From bd906cb3145b19b5de4afdb27711844bc70ac019 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 17 Jun 2024 14:58:52 +0200 Subject: [PATCH 1/2] pkg/machine/wsl: wrap command errors First of some commands ignored cmd.Wait() error which means it was impossible to notice any command errors. And others only returned the wait error as it which when a command fails is just `exit status ` which is not helpful at all. This commit should add proper error wrapping with stderr to get useful strings back hopefully. Signed-off-by: Paul Holzinger --- pkg/machine/wsl/machine.go | 58 ++++++++++++++++++++++++++++------ pkg/machine/wsl/stubber.go | 11 +++++-- pkg/machine/wsl/wutil/wutil.go | 6 +++- 3 files changed, 61 insertions(+), 14 deletions(-) diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index 20afee2dc4..38c1a91261 100644 --- a/pkg/machine/wsl/machine.go +++ b/pkg/machine/wsl/machine.go @@ -4,6 +4,7 @@ package wsl import ( "bufio" + "bytes" "errors" "fmt" "io" @@ -550,7 +551,10 @@ func runCmdPassThrough(name string, arg ...string) error { cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - return cmd.Run() + if err := cmd.Run(); err != nil { + return fmt.Errorf("command %s %v failed: %w", name, arg, err) + } + return nil } func runCmdPassThroughTee(out io.Writer, name string, arg ...string) error { @@ -562,7 +566,10 @@ func runCmdPassThroughTee(out io.Writer, name string, arg ...string) error { cmd.Stdin = os.Stdin cmd.Stdout = io.MultiWriter(os.Stdout, out) cmd.Stderr = io.MultiWriter(os.Stderr, out) - return cmd.Run() + if err := cmd.Run(); err != nil { + return fmt.Errorf("command %s %v failed: %w", name, arg, err) + } + return nil } func pipeCmdPassThrough(name string, input string, arg ...string) error { @@ -571,7 +578,10 @@ func pipeCmdPassThrough(name string, input string, arg ...string) error { cmd.Stdin = strings.NewReader(input) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - return cmd.Run() + if err := cmd.Run(); err != nil { + return fmt.Errorf("command %s %v failed: %w", name, arg, err) + } + return nil } func setupWslProxyEnv() (hasProxy bool) { @@ -638,8 +648,10 @@ func getAllWSLDistros(running bool) (map[string]struct{}, error) { if err != nil { return nil, err } + stderr := &bytes.Buffer{} + cmd.Stderr = stderr if err = cmd.Start(); err != nil { - return nil, err + return nil, fmt.Errorf("failed to start command %s %v: %w", cmd.Path, args, err) } all := make(map[string]struct{}) @@ -651,7 +663,10 @@ func getAllWSLDistros(running bool) (map[string]struct{}, error) { } } - _ = cmd.Wait() + err = cmd.Wait() + if err != nil { + return nil, fmt.Errorf("command %s %v failed: %w (%s)", cmd.Path, args, err, strings.TrimSpace(stderr.String())) + } return all, nil } @@ -663,6 +678,8 @@ func isSystemdRunning(dist string) (bool, error) { if err != nil { return false, err } + stderr := &bytes.Buffer{} + cmd.Stderr = stderr if err = cmd.Start(); err != nil { return false, err } @@ -676,19 +693,30 @@ func isSystemdRunning(dist string) (bool, error) { } } - _ = cmd.Wait() + err = cmd.Wait() + if err != nil { + return false, fmt.Errorf("command %s %v failed: %w (%s)", cmd.Path, cmd.Args, err, strings.TrimSpace(stderr.String())) + } return result, nil } func terminateDist(dist string) error { cmd := exec.Command(wutil.FindWSL(), "--terminate", dist) - return cmd.Run() + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("command %s %v failed: %w (%s)", cmd.Path, cmd.Args, err, strings.TrimSpace(string(out))) + } + return nil } func unregisterDist(dist string) error { cmd := exec.Command(wutil.FindWSL(), "--unregister", dist) - return cmd.Run() + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("command %s %v failed: %w (%s)", cmd.Path, cmd.Args, err, strings.TrimSpace(string(out))) + } + return nil } func isRunning(name string) (bool, error) { @@ -736,6 +764,8 @@ func getCPUs(name string) (uint64, error) { if err != nil { return 0, err } + stderr := &bytes.Buffer{} + cmd.Stderr = stderr if err = cmd.Start(); err != nil { return 0, err } @@ -744,7 +774,10 @@ func getCPUs(name string) (uint64, error) { for scanner.Scan() { result = scanner.Text() } - _ = cmd.Wait() + err = cmd.Wait() + if err != nil { + return 0, fmt.Errorf("command %s %v failed: %w (%s)", cmd.Path, cmd.Args, err, strings.TrimSpace(strings.TrimSpace(stderr.String()))) + } ret, err := strconv.Atoi(result) return uint64(ret), err @@ -761,6 +794,8 @@ func getMem(name string) (strongunits.MiB, error) { if err != nil { return 0, err } + stderr := &bytes.Buffer{} + cmd.Stderr = stderr if err = cmd.Start(); err != nil { return 0, err } @@ -783,7 +818,10 @@ func getMem(name string) (strongunits.MiB, error) { break } } - _ = cmd.Wait() + err = cmd.Wait() + if err != nil { + return 0, fmt.Errorf("command %s %v failed: %w (%s)", cmd.Path, cmd.Args, err, strings.TrimSpace(stderr.String())) + } return strongunits.MiB(total - available), err } diff --git a/pkg/machine/wsl/stubber.go b/pkg/machine/wsl/stubber.go index 433f05eef1..8f2569283b 100644 --- a/pkg/machine/wsl/stubber.go +++ b/pkg/machine/wsl/stubber.go @@ -3,6 +3,7 @@ package wsl import ( + "bytes" "errors" "fmt" "os" @@ -110,7 +111,7 @@ func (w WSLStubber) Remove(mc *vmconfigs.MachineConfig) ([]string, func() error, // of the vm wslRemoveFunc := func() error { if err := runCmdPassThrough(wutil.FindWSL(), "--unregister", env.WithPodmanPrefix(mc.Name)); err != nil { - logrus.Error(err) + return err } return nil } @@ -251,17 +252,21 @@ func (w WSLStubber) StopVM(mc *vmconfigs.MachineConfig, hardStop bool) error { cmd := exec.Command(wutil.FindWSL(), "-u", "root", "-d", dist, "sh") cmd.Stdin = strings.NewReader(waitTerm) + out := &bytes.Buffer{} + cmd.Stderr = out + cmd.Stdout = out + if err = cmd.Start(); err != nil { return fmt.Errorf("executing wait command: %w", err) } exitCmd := exec.Command(wutil.FindWSL(), "-u", "root", "-d", dist, "/usr/local/bin/enterns", "systemctl", "exit", "0") if err = exitCmd.Run(); err != nil { - return fmt.Errorf("stopping sysd: %w", err) + return fmt.Errorf("stopping systemd: %w", err) } if err = cmd.Wait(); err != nil { - return err + return fmt.Errorf("wait for systemd to exit: %w (%s)", err, strings.TrimSpace(out.String())) } return terminateDist(dist) diff --git a/pkg/machine/wsl/wutil/wutil.go b/pkg/machine/wsl/wutil/wutil.go index 3f613ab02c..bc07ffa2a9 100644 --- a/pkg/machine/wsl/wutil/wutil.go +++ b/pkg/machine/wsl/wutil/wutil.go @@ -4,6 +4,7 @@ package wutil import ( "bufio" + "fmt" "io" "os" "os/exec" @@ -74,7 +75,10 @@ func SilentExec(command string, args ...string) error { cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: 0x08000000} cmd.Stdout = nil cmd.Stderr = nil - return cmd.Run() + if err := cmd.Run(); err != nil { + return fmt.Errorf("command %s %v failed: %w", command, args, err) + } + return nil } func SilentExecCmd(command string, args ...string) *exec.Cmd { From 5c1e5cd02645c815a9dc443cff3dfadc2288c77f Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 17 Jun 2024 17:47:37 +0200 Subject: [PATCH 2/2] pkg/machine/wsl: force terminate wsl instance We do a soft stop via systemd to allow graceful shutdown behavior. Hoewever for unknown reason we are hitting such a case in CI right now. Regardless of the CI issue we should always to the hard terminate in such case so only log the timeout as warning. Signed-off-by: Paul Holzinger --- pkg/machine/wsl/stubber.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/machine/wsl/stubber.go b/pkg/machine/wsl/stubber.go index 8f2569283b..3a79f095ca 100644 --- a/pkg/machine/wsl/stubber.go +++ b/pkg/machine/wsl/stubber.go @@ -266,7 +266,7 @@ func (w WSLStubber) StopVM(mc *vmconfigs.MachineConfig, hardStop bool) error { } if err = cmd.Wait(); err != nil { - return fmt.Errorf("wait for systemd to exit: %w (%s)", err, strings.TrimSpace(out.String())) + logrus.Warnf("Failed to wait for systemd to exit: (%s)", strings.TrimSpace(out.String())) } return terminateDist(dist)