Honor FF_TIMESTAMPS and reclassify benign exec outcomes in concrete

What does this MR do?

Honor FF_TIMESTAMPS in StepRunnerStream

Upgrade step-runner to v0.38.0 for the timestamper carriage-return fix. Move innerstream from functions/concrete/run/internal/ to common/buildlogger/ so StepRunnerStream can use it.

StepRunnerStream now picks its mode on the first write:

  • pre-stamped and Timestamping on: passthrough, preserving step-runner's headers and avoiding double-stamping.
  • pre-stamped and Timestamping off: step-runner has no knob to disable its own stamping, so the runner demuxes via innerstream and routes bodies through per-type wrap chains, giving the stamp-free trace the caller asked for.
  • not pre-stamped: bytes flow through one wrap chain with the caller-supplied stream type.

Reclassify benign exec outcomes in concrete env.Command

gracefulexitcmd (wired into functions/concrete/run/env/env.go in b8e2de918) bounds the post-Wait() stdio drain via cmd.WaitDelay. That guarantee is the right one to have, but the std-lib *exec.Cmd reports two benign outcomes as errors that concrete was surfacing as script_failure even though the traditional bash-helper path treats them as success:

  • The foreground script exited 0 but a backgrounded child kept the stdio pipes open past WaitDelay (a CI script doing (sleep 60) & is enough). exec.ErrWaitDelay, exit code 0.
  • The script's outer shell was killed by a non-fatal user signal (SIGUSR1 / SIGUSR2 / SIGHUP / SIGPIPE). scriptwriter's : | (eval body) wrapping means kill -USR1 $$ from a user script lands on the outer wrapping shell, which has no trap installed.

normalizeExitError in functions/concrete/run/env/env.go reclassifies both as success. SIGTERM (raised by gracefulexitcmd.Cmd.Cancel on cancellation) and SIGKILL (e.g. Docker OOM-kill / alpine-id-overflow) deliberately keep surfacing so cancellations are still reported and exit_code=137 still propagates.

Why was this MR needed?

  • Updates step-runner because there was a bug in the timestamper impl.
  • Upgrades the timestamp impl in Runner (it's identical to step-runner, choosing to duplicate the code rather than import for now).
  • Rather than pass FF_TIMESTAMP through to the step-runner and have it disable the timestamps, we just strip them in Runner. Doing it this way because FF_TIMESTAMP will eventually be removed, and we already had an implementation to strip timestamps in Runner.
  • Backbench surfaced two concrete-only regressions where jobs that succeed under traditional execution were failing under concrete: (sleep 60) & (WaitDelay on inherited stdio) and kill -USR1 $$ with a trap installed. Both fixed by normalizeExitError without touching the cancel / process-group / WaitDelay-bound guarantees gracefulexitcmd was added for.

What's the best way to test this MR?

  • Unit tests in functions/concrete/run/env/normalize_exit_error_unix_test.go: nil passthrough, plain exit-1 passthrough, exec.ErrWaitDelay on a clean exit, each of SIGUSR1 / SIGUSR2 / SIGHUP / SIGPIPE, and SIGTERM / SIGKILL passthrough.
  • Backbench coverage in the companion MR (https://gitlab.com/gitlab-org/ci-cd/runner-tools/runner-backbench/-/merge_requests/13) covers the same shapes end-to-end: core_script/background_process_does_not_block and linux_platform/linux_USR1_trap_fires. Both fail on the previous helper image and pass on the one built from this branch.

What are the relevant issue numbers?

Closes step-runner#465 (closed) (step-runner upgrade fixes this)

Edited by Arran Walker

Merge request reports

Loading