From 0772d8ddb0faf93cbb575c1f9c6bdd56136cdcc0 Mon Sep 17 00:00:00 2001 From: Brent Baude Date: Fri, 4 Aug 2023 10:03:56 -0500 Subject: [PATCH] Stop gvproxy on hyperv machine stop when we stop a machine, we need to also stop the gvproxy process that is running. JIRA: RUN-1828 also, remove unused applehv function for ssh Signed-off-by: Brent Baude [NO NEW TESTS NEEDED] Signed-off-by: Brent Baude --- pkg/machine/applehv/machine.go | 60 +++----------------------- pkg/machine/gvproxy.go | 79 ++++++++++++++++++++++++++++++++++ pkg/machine/hyperv/config.go | 13 ++++++ pkg/machine/hyperv/machine.go | 9 ++++ 4 files changed, 106 insertions(+), 55 deletions(-) create mode 100644 pkg/machine/gvproxy.go diff --git a/pkg/machine/applehv/machine.go b/pkg/machine/applehv/machine.go index 7564f85553..08992f5f3b 100644 --- a/pkg/machine/applehv/machine.go +++ b/pkg/machine/applehv/machine.go @@ -11,7 +11,6 @@ import ( "io/fs" "net" "os" - "os/exec" "path/filepath" "strconv" "strings" @@ -692,34 +691,6 @@ func (m *MacMachine) State(_ bool) (machine.Status, error) { return vmStatus, nil } -// cleanupGVProxy finds the gvproxy process, kills it, and deletes the -// pidfile -func (m *MacMachine) cleanupGVProxy() { - gvPid, err := m.GvProxyPid.Read() - if err != nil { - logrus.Error(fmt.Errorf("unable to read gvproxy pid file %s: %v", m.GvProxyPid.GetPath(), err)) - return - } - proxyPid, err := strconv.Atoi(string(gvPid)) - if err != nil { - logrus.Error(fmt.Errorf("unable to convert pid to integer: %v", err)) - return - } - proxyProc, err := os.FindProcess(proxyPid) - if proxyProc == nil && err != nil { - logrus.Error("unable to find process: %v", err) - return - } - if err := proxyProc.Kill(); err != nil { - logrus.Error("unable to kill gvproxy: %v", err) - return - } - // gvproxy does not clean up its pid file on exit - if err := m.GvProxyPid.Delete(); err != nil { - logrus.Error("unable to delete gvproxy pid file: %v", err) - } -} - func (m *MacMachine) Stop(name string, opts machine.StopOptions) error { vmState, err := m.State(false) if err != nil { @@ -730,7 +701,11 @@ func (m *MacMachine) Stop(name string, opts machine.StopOptions) error { return machine.ErrWrongState } - defer m.cleanupGVProxy() + defer func() { + if err := machine.CleanupGVProxy(m.GvProxyPid); err != nil { + logrus.Error(err) + } + }() return m.Vfkit.stop(false, true) } @@ -915,31 +890,6 @@ func (m *MacMachine) startHostNetworking(ioEater *os.File) (string, machine.APIF return forwardSock, state, nil } -// AppleHVSSH is a temporary function for applehv until we decide how the networking will work -// for certain. -func AppleHVSSH(username, identityPath, name string, sshPort int, inputArgs []string) error { - sshDestination := username + "@192.168.64.2" - port := strconv.Itoa(sshPort) - - args := []string{"-i", identityPath, "-p", port, sshDestination, - "-o", "IdentitiesOnly=yes", - "-o", "StrictHostKeyChecking=no", "-o", "LogLevel=ERROR", "-o", "SetEnv=LC_ALL="} - if len(inputArgs) > 0 { - args = append(args, inputArgs...) - } else { - fmt.Printf("Connecting to vm %s. To close connection, use `~.` or `exit`\n", name) - } - - cmd := exec.Command("ssh", args...) - logrus.Debugf("Executing: ssh %v\n", args) - - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - cmd.Stdin = os.Stdin - - return cmd.Run() -} - func (m *MacMachine) setupAPIForwarding(cmd []string) ([]string, string, machine.APIForwardingState) { socket, err := m.forwardSocketPath() if err != nil { diff --git a/pkg/machine/gvproxy.go b/pkg/machine/gvproxy.go new file mode 100644 index 0000000000..8cc8f00443 --- /dev/null +++ b/pkg/machine/gvproxy.go @@ -0,0 +1,79 @@ +package machine + +import ( + "fmt" + "os" + "strconv" + "syscall" + "time" +) + +const ( + loops = 5 + sleepTime = time.Millisecond * 1 +) + +// backoffForProcess checks if the process still exists, for something like +// sigterm. If the process still exists after loops and sleep time are exhausted, +// an error is returned +func backoffForProcess(pid int) error { + sleepInterval := sleepTime + for i := 0; i < loops; i++ { + proxyProc, err := os.FindProcess(pid) + if proxyProc == nil && err != nil { + // process is killed, gone + return nil //nolint: nilerr + } + time.Sleep(sleepInterval) + // double the time + sleepInterval += sleepInterval + } + return fmt.Errorf("process %d has not ended", pid) +} + +// waitOnProcess takes a pid and sends a sigterm to it. it then waits for the +// process to not exist. if the sigterm does not end the process after an interval, +// then sigkill is sent. it also waits for the process to exit after the sigkill too. +func waitOnProcess(processID int) error { + proxyProc, err := os.FindProcess(processID) + if err != nil { + return err + } + + // Try to kill the pid with sigterm + if err := proxyProc.Signal(syscall.SIGTERM); err != nil { + return err + } + + if err := backoffForProcess(processID); err == nil { + return nil + } + + // sigterm has not killed it yet, lets send a sigkill + proxyProc, err = os.FindProcess(processID) + if proxyProc == nil && err != nil { + // process is killed, gone + return nil //nolint: nilerr + } + if err := proxyProc.Signal(syscall.SIGKILL); err != nil { + // lets assume it is dead in this case + return nil //nolint: nilerr + } + return backoffForProcess(processID) +} + +// CleanupGVProxy reads the --pid-file for gvproxy attempts to stop it +func CleanupGVProxy(f VMFile) error { + gvPid, err := f.Read() + if err != nil { + return fmt.Errorf("unable to read gvproxy pid file %s: %v", f.GetPath(), err) + } + proxyPid, err := strconv.Atoi(string(gvPid)) + if err != nil { + return fmt.Errorf("unable to convert pid to integer: %v", err) + } + if err := waitOnProcess(proxyPid); err == nil { + return nil + } + return f.Delete() +} diff --git a/pkg/machine/hyperv/config.go b/pkg/machine/hyperv/config.go index adec7626c5..6decd1b1af 100644 --- a/pkg/machine/hyperv/config.go +++ b/pkg/machine/hyperv/config.go @@ -115,6 +115,7 @@ func (v HyperVVirtualization) NewMachine(opts machine.InitOptions) (machine.VM, if err != nil { return nil, err } + m.ConfigPath = *configPath ignitionPath, err := machine.NewMachineFile(filepath.Join(configDir, m.Name)+".ign", nil) @@ -126,6 +127,18 @@ func (v HyperVVirtualization) NewMachine(opts machine.InitOptions) (machine.VM, // Set creation time m.Created = time.Now() + dataDir, err := machine.GetDataDir(machine.HyperVVirt) + if err != nil { + return nil, err + } + + // Set the proxy pid file + gvProxyPid, err := machine.NewMachineFile(filepath.Join(dataDir, "gvproxy.pid"), nil) + if err != nil { + return nil, err + } + m.GvProxyPid = *gvProxyPid + // Acquire the image imagePath, imageStream, err := v.acquireVMImage(opts) if err != nil { diff --git a/pkg/machine/hyperv/machine.go b/pkg/machine/hyperv/machine.go index 0c654d9566..4919f5bd31 100644 --- a/pkg/machine/hyperv/machine.go +++ b/pkg/machine/hyperv/machine.go @@ -62,6 +62,8 @@ type HyperVMachine struct { Created time.Time // LastUp contains the last recorded uptime LastUp time.Time + // GVProxy will write its PID here + GvProxyPid machine.VMFile } // addNetworkAndReadySocketsToRegistry adds the Network and Ready sockets to the @@ -510,7 +512,13 @@ func (m *HyperVMachine) Stop(name string, opts machine.StopOptions) error { if vm.State() != hypervctl.Enabled { return hypervctl.ErrMachineStateInvalid } + + if err := machine.CleanupGVProxy(m.GvProxyPid); err != nil { + logrus.Error(err) + } + return vm.Stop() + } func (m *HyperVMachine) jsonConfigPath() (string, error) { @@ -615,6 +623,7 @@ func (m *HyperVMachine) startHostNetworking() (string, machine.APIForwardingStat // Add the ssh port cmd = append(cmd, []string{"-ssh-port", fmt.Sprintf("%d", m.Port)}...) cmd = append(cmd, []string{"-listen", fmt.Sprintf("vsock://%s", m.NetworkHVSock.KeyName)}...) + cmd = append(cmd, "-pid-file", m.GvProxyPid.GetPath()) cmd, forwardSock, state = m.setupAPIForwarding(cmd) if logrus.GetLevel() == logrus.DebugLevel {