diff --git a/Makefile b/Makefile index 24755491..2680cbeb 100644 --- a/Makefile +++ b/Makefile @@ -39,7 +39,7 @@ build: $(current_binary) ## Build the func-e binary test_packages := . ./internal/... test: ## Run all unit tests @printf "$(ansi_format_dark)" test "running unit tests" - @go test . $(test_packages) + @go test -timeout 30s . $(test_packages) @printf "$(ansi_format_bright)" test "ok" coverage: ## Generate test coverage diff --git a/internal/moreos/moreos_test.go b/internal/moreos/moreos_test.go index c7837319..1eff879d 100644 --- a/internal/moreos/moreos_test.go +++ b/internal/moreos/moreos_test.go @@ -15,6 +15,7 @@ package moreos import ( + "bufio" "bytes" "errors" "fmt" @@ -23,6 +24,7 @@ import ( "path/filepath" "runtime" "testing" + "time" "github.com/shirou/gopsutil/v3/process" "github.com/stretchr/testify/require" @@ -120,7 +122,7 @@ func TestSprintf_IdiomaticPerOS(t *testing.T) { func TestProcessGroupAttr_Interrupt(t *testing.T) { // Fork a process that hangs - cmd := exec.Command("cat" + Exe) + cmd := exec.Command("sleep"+Exe, "1000") cmd.SysProcAttr = ProcessGroupAttr() require.NoError(t, cmd.Start()) @@ -140,7 +142,7 @@ func TestProcessGroupAttr_Interrupt(t *testing.T) { func Test_EnsureProcessDone(t *testing.T) { // Fork a process that hangs - cmd := exec.Command("cat" + Exe) + cmd := exec.Command("sleep"+Exe, "1000") cmd.SysProcAttr = ProcessGroupAttr() require.NoError(t, cmd.Start()) @@ -155,6 +157,65 @@ func Test_EnsureProcessDone(t *testing.T) { require.NoError(t, EnsureProcessDone(cmd.Process)) } +func Test_EnsureChildProcessDone(t *testing.T) { + // There are 3 processes spawned by this test target, with the following relationship: + // + // runner (go run testdata/main/main.go) + // └── main (the executable, compiled from testdata/main/main.go. This is a fake func-e) + // └── child (sleep, this is a fake envoy) + cmd := exec.Command("go"+Exe, "run", filepath.Join("testdata", "main", "main.go")) + stdout, err := cmd.StdoutPipe() + require.NoError(t, err) + cmd.Start() + + buf := bufio.NewReader(stdout) + for { + line, _, readErr := buf.ReadLine() + require.NoError(t, readErr) + // As soon as we detect output from the main program, we know the child process in main program + // is started. See: testdata/main/main.go#34. + if string(line) == "ok" { + break + } + } + + // This is the "go run" process. + runner, err := process.NewProcess(int32(cmd.Process.Pid)) + require.NoError(t, err) + + parents, err := runner.Children() + require.NoError(t, err) + require.Equal(t, len(parents), 1) + + // This is the main process executed by "go run". + main, err := process.NewProcess(parents[0].Pid) + require.NoError(t, err) + + children, err := main.Children() + require.NoError(t, err) + require.Equal(t, len(children), 1) + + // Kill the main process. + require.NoError(t, EnsureProcessDone(&os.Process{Pid: int(main.Pid)})) + + // Wait and check the status of child and main processes. + // The following check is here to make sure the child and main processes are not killed because + // of the runner is died. + time.Sleep(time.Millisecond) + child := children[0] + require.Error(t, findProcess(&os.Process{Pid: int(child.Pid)})) + require.Error(t, findProcess(&os.Process{Pid: int(main.Pid)})) + + // The runner needs to be killed too. + require.NoError(t, EnsureProcessDone(&os.Process{Pid: int(runner.Pid)})) + cmd.Wait() //nolint + + require.Error(t, findProcess(&os.Process{Pid: int(runner.Pid)})) + + // Ensure killing it again doesn't error. + require.NoError(t, EnsureProcessDone(&os.Process{Pid: int(main.Pid)})) +} + func findProcess(proc *os.Process) error { _, err := process.NewProcess(int32(proc.Pid)) // because os.FindProcess is no-op in Linux! return err diff --git a/internal/moreos/proc_darwin.go b/internal/moreos/proc_darwin.go index 5fcf5c64..d72702ef 100644 --- a/internal/moreos/proc_darwin.go +++ b/internal/moreos/proc_darwin.go @@ -17,6 +17,8 @@ package moreos import ( "os" "syscall" + + "github.com/shirou/gopsutil/v3/process" ) const exe = "" @@ -33,7 +35,24 @@ func interrupt(p *os.Process) error { } func ensureProcessDone(p *os.Process) error { - if err := p.Kill(); err != nil && err != os.ErrProcessDone { + proc, err := process.NewProcess(int32(p.Pid)) + if err != nil { + if err == process.ErrorProcessNotRunning { + return nil + } + return err + } + + // on macOS, when a process doesn't have children, pgrep gives "exit status 1", hence we ignore + // the error here. + children, _ := proc.Children() + for _, child := range children { + if err := child.Kill(); err != nil && err != process.ErrorProcessNotRunning { + return err + } + } + + if err := proc.Kill(); err != nil && err != os.ErrProcessDone { return err } return nil diff --git a/internal/moreos/testdata/main/main.go b/internal/moreos/testdata/main/main.go new file mode 100644 index 00000000..9e2a0220 --- /dev/null +++ b/internal/moreos/testdata/main/main.go @@ -0,0 +1,37 @@ +// Copyright 2021 Tetrate +// +// 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 main + +import ( + "context" + "fmt" + "os" + "os/exec" + "os/signal" + "syscall" + + "github.com/tetratelabs/func-e/internal/moreos" +) + +func main() { + ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGKILL) + defer stop() + cmd := exec.Command("sleep"+moreos.Exe, "1000") + cmd.SysProcAttr = moreos.ProcessGroupAttr() + cmd.Start() + fmt.Fprintln(os.Stdout, "ok") + cmd.Wait() + <-ctx.Done() +}