Merge pull request containers#19516 from baude/stopgvproxy

Stop gvproxy on hyperv machine stop

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 {

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()
+}

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 {

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 {