[3.5]backport: *: 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 <[email protected]>

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()

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)
 	}
@@ -188,11 +189,23 @@ func TestSignal(t *testing.T) {
 	go func() {
 		defer close(donec)
 		err = ep.Close()
-		assert.ErrorContains(t, err, "unexpected exit code [-1] after running [/usr/bin/sleep 100]")
+		assert.ErrorContains(t, err, "unexpected exit code [130]")
+		assert.ErrorContains(t, err, "sleep 100")
 	}()
 	select {
 	case <-time.After(5 * time.Second):
 		t.Fatalf("signal test timed out")
 	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)
+}

tests/framework/e2e/etcd_process.go

@@ -252,9 +252,45 @@ func (ep *EtcdServerProcess) Kill() error {
 	return ep.proc.Signal(syscall.SIGKILL)
 }
 
-func (ep *EtcdServerProcess) Wait() error {
-	ep.proc.Wait()
-	ep.cfg.lg.Info("server exited", zap.String("name", ep.cfg.Name))
+func (ep *EtcdServerProcess) Wait(ctx context.Context) error {
+	ch := make(chan struct{})
+	go func() {
+		defer close(ch)
+		if ep.proc != nil {
+			ep.proc.Wait()
+
+			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 {
+	case <-ch:
+		ep.proc = nil
+		return nil
+	case <-ctx.Done():
+		return ctx.Err()
+	}
+}
+
+func (ep *EtcdServerProcess) IsRunning() bool {
+	if ep.proc == nil {
+		return false
+	}
+
+	exitCode, err := ep.proc.ExitCode()
+	if err == expect.ErrProcessRunning {
+		return true
+	}
+
+	ep.cfg.lg.Info("server exited",
+		zap.String("name", ep.cfg.Name),
+		zap.Int("code", exitCode),
+		zap.Error(err))
 	ep.proc = nil
 	return nil
 }