mirror of
https://github.com/coder/coder.git
synced 2026-09-21 12:44:32 +08:00
fix: propagate correct agent exit code (#21718)
The reaper (PID 1) now returns the child's exit code instead of always exiting 0. Signal termination uses the standard Unix convention of 128 + signal number. fixes #21661
This commit is contained in:
@@ -7,6 +7,6 @@ func IsInitProcess() bool {
|
||||
return false
|
||||
}
|
||||
|
||||
func ForkReap(_ ...Option) error {
|
||||
return nil
|
||||
func ForkReap(_ ...Option) (int, error) {
|
||||
return 0, nil
|
||||
}
|
||||
|
||||
@@ -32,12 +32,13 @@ func TestReap(t *testing.T) {
|
||||
}
|
||||
|
||||
pids := make(reap.PidCh, 1)
|
||||
err := reaper.ForkReap(
|
||||
exitCode, err := reaper.ForkReap(
|
||||
reaper.WithPIDCallback(pids),
|
||||
// Provide some argument that immediately exits.
|
||||
reaper.WithExecArgs("/bin/sh", "-c", "exit 0"),
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, 0, exitCode)
|
||||
|
||||
cmd := exec.Command("tail", "-f", "/dev/null")
|
||||
err = cmd.Start()
|
||||
@@ -65,6 +66,36 @@ func TestReap(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
//nolint:paralleltest
|
||||
func TestForkReapExitCodes(t *testing.T) {
|
||||
if testutil.InCI() {
|
||||
t.Skip("Detected CI, skipping reaper tests")
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
command string
|
||||
expectedCode int
|
||||
}{
|
||||
{"exit 0", "exit 0", 0},
|
||||
{"exit 1", "exit 1", 1},
|
||||
{"exit 42", "exit 42", 42},
|
||||
{"exit 255", "exit 255", 255},
|
||||
{"SIGKILL", "kill -9 $$", 128 + 9},
|
||||
{"SIGTERM", "kill -15 $$", 128 + 15},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
exitCode, err := reaper.ForkReap(
|
||||
reaper.WithExecArgs("/bin/sh", "-c", tt.command),
|
||||
)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, tt.expectedCode, exitCode, "exit code mismatch for %q", tt.command)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
//nolint:paralleltest // Signal handling.
|
||||
func TestReapInterrupt(t *testing.T) {
|
||||
// Don't run the reaper test in CI. It does weird
|
||||
@@ -84,13 +115,17 @@ func TestReapInterrupt(t *testing.T) {
|
||||
defer signal.Stop(usrSig)
|
||||
|
||||
go func() {
|
||||
errC <- reaper.ForkReap(
|
||||
exitCode, err := reaper.ForkReap(
|
||||
reaper.WithPIDCallback(pids),
|
||||
reaper.WithCatchSignals(os.Interrupt),
|
||||
// Signal propagation does not extend to children of children, so
|
||||
// we create a little bash script to ensure sleep is interrupted.
|
||||
reaper.WithExecArgs("/bin/sh", "-c", fmt.Sprintf("pid=0; trap 'kill -USR2 %d; kill -TERM $pid' INT; sleep 10 &\npid=$!; kill -USR1 %d; wait", os.Getpid(), os.Getpid())),
|
||||
)
|
||||
// The child exits with 128 + SIGTERM (15) = 143, but the trap catches
|
||||
// SIGINT and sends SIGTERM to the sleep process, so exit code varies.
|
||||
_ = exitCode
|
||||
errC <- err
|
||||
}()
|
||||
|
||||
require.Equal(t, <-usrSig, syscall.SIGUSR1)
|
||||
|
||||
@@ -40,7 +40,10 @@ func catchSignals(pid int, sigs []os.Signal) {
|
||||
// the reaper and an exec.Command waiting for its process to complete.
|
||||
// The provided 'pids' channel may be nil if the caller does not care about the
|
||||
// reaped children PIDs.
|
||||
func ForkReap(opt ...Option) error {
|
||||
//
|
||||
// Returns the child's exit code (using 128+signal for signal termination)
|
||||
// and any error from Wait4.
|
||||
func ForkReap(opt ...Option) (int, error) {
|
||||
opts := &options{
|
||||
ExecArgs: os.Args,
|
||||
}
|
||||
@@ -53,7 +56,7 @@ func ForkReap(opt ...Option) error {
|
||||
|
||||
pwd, err := os.Getwd()
|
||||
if err != nil {
|
||||
return xerrors.Errorf("get wd: %w", err)
|
||||
return 1, xerrors.Errorf("get wd: %w", err)
|
||||
}
|
||||
|
||||
pattrs := &syscall.ProcAttr{
|
||||
@@ -72,7 +75,7 @@ func ForkReap(opt ...Option) error {
|
||||
//#nosec G204
|
||||
pid, err := syscall.ForkExec(opts.ExecArgs[0], opts.ExecArgs, pattrs)
|
||||
if err != nil {
|
||||
return xerrors.Errorf("fork exec: %w", err)
|
||||
return 1, xerrors.Errorf("fork exec: %w", err)
|
||||
}
|
||||
|
||||
go catchSignals(pid, opts.CatchSignals)
|
||||
@@ -82,5 +85,18 @@ func ForkReap(opt ...Option) error {
|
||||
for xerrors.Is(err, syscall.EINTR) {
|
||||
_, err = syscall.Wait4(pid, &wstatus, 0, nil)
|
||||
}
|
||||
return err
|
||||
|
||||
// Convert wait status to exit code using standard Unix conventions:
|
||||
// - Normal exit: use the exit code
|
||||
// - Signal termination: use 128 + signal number
|
||||
var exitCode int
|
||||
switch {
|
||||
case wstatus.Exited():
|
||||
exitCode = wstatus.ExitStatus()
|
||||
case wstatus.Signaled():
|
||||
exitCode = 128 + int(wstatus.Signal())
|
||||
default:
|
||||
exitCode = 1
|
||||
}
|
||||
return exitCode, err
|
||||
}
|
||||
|
||||
+3
-3
@@ -136,7 +136,7 @@ func workspaceAgent() *serpent.Command {
|
||||
// to do this else we fork bomb ourselves.
|
||||
//nolint:gocritic
|
||||
args := append(os.Args, "--no-reap")
|
||||
err := reaper.ForkReap(
|
||||
exitCode, err := reaper.ForkReap(
|
||||
reaper.WithExecArgs(args...),
|
||||
reaper.WithCatchSignals(StopSignals...),
|
||||
)
|
||||
@@ -145,8 +145,8 @@ func workspaceAgent() *serpent.Command {
|
||||
return xerrors.Errorf("fork reap: %w", err)
|
||||
}
|
||||
|
||||
logger.Info(ctx, "reaper process exiting")
|
||||
return nil
|
||||
logger.Info(ctx, "reaper child process exited", slog.F("exit_code", exitCode))
|
||||
return ExitError(exitCode, nil)
|
||||
}
|
||||
|
||||
// Handle interrupt signals to allow for graceful shutdown,
|
||||
|
||||
Reference in New Issue
Block a user