Skip to content

runner: clean up containers after termination - #84

Open
chunga-ict wants to merge 4 commits into
strongdm:mainfrom
chunga-ict:fix/sigterm-container-cleanup
Open

chunga-ict wants to merge 4 commits into
strongdm:mainfrom
chunga-ict:fix/sigterm-container-cleanup

Conversation

@chunga-ict

Copy link
Copy Markdown

Summary

  • handle SIGTERM through the same cancellation path as SIGINT
  • reconcile a target container that Docker creates after a canceled docker run client exits
  • surface Docker removal failures after successful commands while preserving non-zero command exit codes
  • add regression coverage for termination signals, late container creation, and cleanup error propagation

Problem

exec.CommandContext can terminate the docker run client while the Docker daemon is still processing the create request. The existing one-shot docker rm -f may run before the container exists, allowing a target container to appear after Leash exits. In addition, the CLI only subscribed to SIGINT, so timeout/process supervisors using SIGTERM did not enter the intended cancellation path.

The reconciliation loop is only enabled when target launch did not complete normally, so normal successful sessions do not pay the added wait.

Tests

  • go test ./internal/runner -count=1
  • go test -race ./internal/runner -run "TestTerminationSignalsIncludeInterruptAndTerminate|TestStopContainers|TestFinalizeSession" -count=1

Both pass.

go test ./... was also attempted in a clean Go container. It is blocked by repository build prerequisites unrelated to this patch: missing generated LSM bindings (loadLsmOpen, loadLsmExec, and loadLsmConnect) and the Control UI embed assets. The runner package itself passes in full. A full runner race run additionally exposes a pre-existing race around the global commandOutput override between parallel upstream tests; the new targeted race tests pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant