From b3737d749ac51ed1e93f00b2f147f7a570c4dba5 Mon Sep 17 00:00:00 2001
From: Vince Prignano <vincepri@redhat.com>
Date: Mon, 23 Oct 2023 11:49:44 -0700
Subject: [PATCH] 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 <vincepri@redhat.com>
---
 pkg/internal/testing/controlplane/kubectl.go  |  1 +
 .../testing/process/procattr_other.go         | 28 +++++++++++++++
 pkg/internal/testing/process/procattr_unix.go | 34 +++++++++++++++++++
 pkg/internal/testing/process/process.go       |  4 +++
 4 files changed, 67 insertions(+)
 create mode 100644 pkg/internal/testing/process/procattr_other.go
 create mode 100644 pkg/internal/testing/process/procattr_unix.go

diff --git a/pkg/internal/testing/controlplane/kubectl.go b/pkg/internal/testing/controlplane/kubectl.go
index a27b7a0ff8..a41bb77c4d 100644
--- a/pkg/internal/testing/controlplane/kubectl.go
+++ b/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()
 
diff --git a/pkg/internal/testing/process/procattr_other.go b/pkg/internal/testing/process/procattr_other.go
new file mode 100644
index 0000000000..664c22baaf
--- /dev/null
+++ b/pkg/internal/testing/process/procattr_other.go
@@ -0,0 +1,28 @@
+//go:build (!aix && !darwin && !dragonfly && !freebsd && !linux && !netbsd && !openbsd && !solaris && !zos) || !go1.9
+// +build !aix,!darwin,!dragonfly,!freebsd,!linux,!netbsd,!openbsd,!solaris,!zos !go1.9
+
+/*
+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
+}
diff --git a/pkg/internal/testing/process/procattr_unix.go b/pkg/internal/testing/process/procattr_unix.go
new file mode 100644
index 0000000000..c5531d0a9e
--- /dev/null
+++ b/pkg/internal/testing/process/procattr_unix.go
@@ -0,0 +1,34 @@
+//go:build (aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris || zos) && go1.9
+// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris zos
+// +build go1.9
+
+/*
+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,
+	}
+}
diff --git a/pkg/internal/testing/process/process.go b/pkg/internal/testing/process/process.go
index af83c70a2f..03f252524a 100644
--- a/pkg/internal/testing/process/process.go
+++ b/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