Skip to content

Commit 906b6c5

Browse files
committed
fix TestTryKillOne to be platform-agnostic
1 parent 5fcd0b4 commit 906b6c5

File tree

3 files changed

+26
-32
lines changed

3 files changed

+26
-32
lines changed

cmd/minikube/cmd/delete.go

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,8 @@ import (
2222
"os"
2323
"os/exec"
2424
"path/filepath"
25-
"runtime"
2625
"strconv"
2726
"strings"
28-
"syscall"
2927
"time"
3028

3129
"github.com/docker/machine/libmachine/mcnerror"
@@ -676,22 +674,9 @@ func trySigKillProcess(pid int) error {
676674
}
677675

678676
klog.Infof("Killing pid %d ...", pid)
679-
// On non-Windows try to send SIGUP first (graceful), fallback to kill if signaling fails.
680-
// Use numeric syscall.Signal(1) (SIGUP) to avoid referencing platform-specific constant names.
681-
if runtime.GOOS != "windows" {
682-
if err := proc.Signal(syscall.Signal(1)); err != nil {
683-
klog.Infof("SIGUP failed with %v - falling back to kill...", err)
684-
if err := proc.Kill(); err != nil {
685-
klog.Infof("Kill failed with %v - removing probably stale pid...", err)
686-
return errors.Wrapf(err, "removing likely stale unkillable pid: %d", pid)
687-
}
688-
}
689-
} else {
690-
// On Windows, Signal is not meaningful, so we just kill the process
691-
if err := proc.Kill(); err != nil {
692-
klog.Infof("Kill failed with %v - removing probably stale pid...", err)
693-
return errors.Wrapf(err, "removing likely stale unkillable pid: %d", pid)
694-
}
677+
if err := proc.Kill(); err != nil {
678+
klog.Infof("Kill failed with %v - removing probably stale pid...", err)
679+
return errors.Wrapf(err, "removing likely stale unkillable pid: %d", pid)
695680
}
696681

697682
return nil

cmd/minikube/cmd/delete_test.go

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"os"
2222
"os/exec"
2323
"path/filepath"
24-
"runtime"
2524
"testing"
2625

2726
"github.com/docker/machine/libmachine"
@@ -32,6 +31,7 @@ import (
3231
cmdcfg "k8s.io/minikube/cmd/minikube/cmd/config"
3332
"k8s.io/minikube/pkg/minikube/config"
3433
"k8s.io/minikube/pkg/minikube/localpath"
34+
"k8s.io/minikube/pkg/minikube/process"
3535
)
3636

3737
// exclude returns a list of strings, minus the excluded ones
@@ -242,7 +242,7 @@ func main() {
242242
done := make(chan struct{})
243243
defer close(ch)
244244
245-
signal.Notify(ch, syscall.SIGHUP)
245+
signal.Notify(ch, syscall.SIGTERM)
246246
defer signal.Stop(ch)
247247
248248
go func() {
@@ -263,7 +263,7 @@ func main() {
263263
processToKill := exec.Command("go", "run", tmpfile)
264264
err := processToKill.Start()
265265
if err != nil {
266-
t.Fatalf("while execing child process: %v\n", err)
266+
t.Fatalf("while executing child process: %v\n", err)
267267
}
268268
pid := processToKill.Process.Pid
269269

@@ -278,16 +278,11 @@ func main() {
278278

279279
// waiting for process to exit
280280
waitErr := processToKill.Wait()
281-
if runtime.GOOS == "windows" {
282-
// Windows Wait commonly returns a non-nil error (exit status).
283-
// Expect a non-nil Wait error on Windows; fail if we got nil.
284-
if waitErr == nil {
285-
t.Fatalf("expected non-nil Wait error on windows, got nil")
286-
}
287-
} else {
288-
// On POSIX we signal SIGHUP — child should exit cleanly.
289-
if waitErr != nil {
290-
t.Fatalf("unable to kill process: %v\n", waitErr)
291-
}
281+
exists, err := process.ExistsPID(pid)
282+
if err != nil {
283+
t.Fatalf("error checking process existence for pid %d: %v", pid, err)
284+
}
285+
if exists {
286+
t.Fatalf("process %d still exists after trySigKillProcess; waitErr=%v", pid, waitErr)
292287
}
293288
}

pkg/minikube/process/process.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,20 @@ func Exists(pid int, executable string) (bool, error) {
7373
return entry.Executable() == executable, nil
7474
}
7575

76+
// ExistsPID reports whether a process with the given pid exists.
77+
// This is a PID-only check (no executable name matching).
78+
func ExistsPID(pid int) (bool, error) {
79+
if pid <= 0 {
80+
return false, nil
81+
}
82+
83+
entry, err := ps.FindProcess(pid)
84+
if err != nil {
85+
return true, err
86+
}
87+
return entry != nil, nil
88+
}
89+
7690
// Terminate a process with pid and matching name. Returns os.ErrProcessDone if
7791
// the process does not exist, or nil if termination was requested. Caller need
7892
// to wait until the process disappears.

0 commit comments

Comments
 (0)