envtest: improve process cleanup

When envtest creates new processes, it wasn't setting the process group
to the parent, when the main process is killed or it panics, all the
other processes are orphaned, and need to be killed separately.

In addition, if for some reason we can't shutdown a process cleanly, we
should try to SIGKILL before returning from Stop.

Signed-off-by: Vince Prignano <[email protected]>

Author: vincepri

pkg/internal/testing/controlplane/kubectl.go

@@ -112,6 +112,7 @@ func (k *KubeCtl) Run(args ...string) (stdout, stderr io.Reader, err error) {
 	cmd := exec.Command(k.Path, allArgs...)
 	cmd.Stdout = stdoutBuffer
 	cmd.Stderr = stderrBuffer
+	cmd.SysProcAttr = process.GetSysProcAttr()
 
 	err = cmd.Run()
 

pkg/internal/testing/process/procattr_other.go

@@ -0,0 +1,28 @@
+//go:build !aix && !darwin && !dragonfly && !freebsd && !linux && !netbsd && !openbsd && !solaris && !zos
+// +build !aix,!darwin,!dragonfly,!freebsd,!linux,!netbsd,!openbsd,!solaris,!zos
+
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package process
+
+import "syscall"
+
+// GetSysProcAttr returns the SysProcAttr to use for the process,
+// for non-unix systems this returns nil.
+func GetSysProcAttr() *syscall.SysProcAttr {
+	return nil
+}

pkg/internal/testing/process/procattr_unix.go

@@ -0,0 +1,33 @@
+//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris || zos
+// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris zos
+
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package process
+
+import (
+	"golang.org/x/sys/unix"
+)
+
+// GetSysProcAttr returns the SysProcAttr to use for the process,
+// for unix systems this returns a SysProcAttr with Setpgid set to true,
+// which inherits the parent's process group id.
+func GetSysProcAttr() *unix.SysProcAttr {
+	return &unix.SysProcAttr{
+		Setpgid: true,
+	}
+}

pkg/internal/testing/process/process.go

@@ -155,6 +155,7 @@ func (ps *State) Start(stdout, stderr io.Writer) (err error) {
 	ps.Cmd = exec.Command(ps.Path, ps.Args...)
 	ps.Cmd.Stdout = stdout
 	ps.Cmd.Stderr = stderr
+	ps.Cmd.SysProcAttr = GetSysProcAttr()
 
 	ready := make(chan bool)
 	timedOut := time.After(ps.StartTimeout)
@@ -265,6 +266,9 @@ func (ps *State) Stop() error {
 	case <-ps.waitDone:
 		break
 	case <-timedOut:
+		if err := ps.Cmd.Process.Signal(syscall.SIGKILL); err != nil {
+			return fmt.Errorf("unable to kill process %s: %w", ps.Path, err)
+		}
 		return fmt.Errorf("timeout waiting for process %s to stop", path.Base(ps.Path))
 	}
 	ps.ready = false

pkg/internal/testing/process/process_test.go

@@ -352,16 +352,7 @@ var _ = Describe("Init", func() {
 })
 
 var simpleBashScript = []string{
-	"-c",
-	`
-		i=0
-		while true
-		do
-			echo "loop $i" >&2
-			let 'i += 1'
-			sleep 0.2
-		done
-	`,
+	"-c", "tail -f /dev/null",
 }
 
 func getServerURL(server *ghttp.Server) url.URL {