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 meanskill -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_TIMESTAMPthrough to the step-runner and have it disable the timestamps, we just strip them in Runner. Doing it this way becauseFF_TIMESTAMPwill 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) andkill -USR1 $$with a trap installed. Both fixed bynormalizeExitErrorwithout touching the cancel / process-group / WaitDelay-bound guaranteesgracefulexitcmdwas 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.ErrWaitDelayon 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_blockandlinux_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)