From b3316c0e0907a218c6778e8ee43778c76f2ccdce Mon Sep 17 00:00:00 2001 From: Wei Fu Date: Sat, 24 Jun 2023 14:51:24 +0000 Subject: [PATCH] *: should return exitCode even if cmd isn't nil For the pkg/expect package, if the process has been stopped but there is no `Close()` call, the `ExitCode()` won't return exit code correctly. The `ExitCode()` should check `exitErr` and return exit code if cmd isn't nil. And introduces `exitCode` to return correct exit code based on the process is signaled or exited. Signed-off-by: Wei Fu --- pkg/expect/expect.go | 28 +++++++++++++++++++++++++++- pkg/expect/expect_test.go | 22 +++++++++++++++++----- tests/framework/e2e/etcd_process.go | 18 +++++++++++++++--- 3 files changed, 59 insertions(+), 9 deletions(-) diff --git a/pkg/expect/expect.go b/pkg/expect/expect.go index 9c56ae30a..9ad2c8a0e 100644 --- a/pkg/expect/expect.go +++ b/pkg/expect/expect.go @@ -229,6 +229,22 @@ func (ep *ExpectProcess) ExitCode() (int, error) { return ep.exitCode, nil } + if ep.exitErr != nil { + // If the child process panics or is killed, for instance, the + // goFailpoint triggers the exit event, the ep.cmd isn't nil and + // the exitCode will describe the case. + if ep.exitCode != 0 { + return ep.exitCode, nil + } + + // If the wait4(2) in waitProcess returns error, the child + // process might be reaped if the process handles the SIGCHILD + // in other goroutine. It's unlikely in this repo. But we + // should return the error for log even if the child process + // is still running. + return 0, ep.exitErr + } + return 0, ErrProcessRunning } @@ -274,7 +290,7 @@ func (ep *ExpectProcess) waitProcess() error { ep.mu.Lock() defer ep.mu.Unlock() - ep.exitCode = state.ExitCode() + ep.exitCode = exitCode(state) if !state.Success() { return fmt.Errorf("unexpected exit code [%d] after running [%s]", ep.exitCode, ep.cmd.String()) @@ -283,6 +299,16 @@ func (ep *ExpectProcess) waitProcess() error { return nil } +// exitCode returns correct exit code for a process based on signaled or exited. +func exitCode(state *os.ProcessState) int { + status := state.Sys().(syscall.WaitStatus) + + if status.Signaled() { + return 128 + int(status.Signal()) + } + return status.ExitStatus() +} + // Wait waits for the process to finish. func (ep *ExpectProcess) Wait() { ep.wg.Wait() diff --git a/pkg/expect/expect_test.go b/pkg/expect/expect_test.go index b918df67c..93dfdc4a4 100644 --- a/pkg/expect/expect_test.go +++ b/pkg/expect/expect_test.go @@ -72,8 +72,8 @@ func TestExpectFuncTimeout(t *testing.T) { } err = ep.Close() - require.ErrorContains(t, err, "unexpected exit code [-1] after running [/usr/bin/tail -f /dev/null]") - require.Equal(t, -1, ep.exitCode) + require.ErrorContains(t, err, "unexpected exit code [143] after running [/usr/bin/tail -f /dev/null]") + require.Equal(t, 143, ep.exitCode) } func TestExpectFuncExitFailure(t *testing.T) { @@ -108,8 +108,9 @@ func TestExpectFuncExitFailureStop(t *testing.T) { }) require.ErrorContains(t, err, "unexpected exit code [1] after running [/usr/bin/tail -x]") exitCode, err := ep.ExitCode() - require.Equal(t, 0, exitCode) - require.Equal(t, err, ErrProcessRunning) + require.Equal(t, 1, exitCode) + require.NoError(t, err) + if err := ep.Stop(); err != nil { t.Fatal(err) } @@ -189,7 +190,7 @@ func TestSignal(t *testing.T) { go func() { defer close(donec) err = ep.Close() - assert.ErrorContains(t, err, "unexpected exit code [-1]") + assert.ErrorContains(t, err, "unexpected exit code [130]") assert.ErrorContains(t, err, "sleep 100") }() select { @@ -198,3 +199,14 @@ func TestSignal(t *testing.T) { case <-donec: } } + +func TestExitCodeAfterKill(t *testing.T) { + ep, err := NewExpect("sleep", "100") + require.NoError(t, err) + + ep.Signal(os.Kill) + ep.Wait() + code, err := ep.ExitCode() + assert.Equal(t, 137, code) + assert.NoError(t, err) +} diff --git a/tests/framework/e2e/etcd_process.go b/tests/framework/e2e/etcd_process.go index 38934f861..1d8f941bf 100644 --- a/tests/framework/e2e/etcd_process.go +++ b/tests/framework/e2e/etcd_process.go @@ -246,7 +246,14 @@ func (ep *EtcdServerProcess) Wait(ctx context.Context) error { defer close(ch) if ep.proc != nil { ep.proc.Wait() - ep.cfg.lg.Info("server exited", zap.String("name", ep.cfg.Name)) + + exitCode, exitErr := ep.proc.ExitCode() + + ep.cfg.lg.Info("server exited", + zap.String("name", ep.cfg.Name), + zap.Int("code", exitCode), + zap.Error(exitErr), + ) } }() select { @@ -262,11 +269,16 @@ func (ep *EtcdServerProcess) IsRunning() bool { if ep.proc == nil { return false } - _, err := ep.proc.ExitCode() + + exitCode, err := ep.proc.ExitCode() if err == expect.ErrProcessRunning { return true } - ep.cfg.lg.Info("server exited", zap.String("name", ep.cfg.Name)) + + ep.cfg.lg.Info("server exited", + zap.String("name", ep.cfg.Name), + zap.Int("code", exitCode), + zap.Error(err)) ep.proc = nil return false }