Skip to content

Draft: Add context support to process.NewOSCmd

Arran Walker requested to merge ajwalker/commander-ctx into main

What does this MR do?

Rather than a separate Killer that callers need to be aware of, a process can be terminated by canceling the provided context. This puts it closer to https://golang.org/pkg/os/exec/#CommandContext in functionality and use.

  • It keeps KillerWaiter, as it's quite handy for mock tests.
  • The mock tests using KillWaiter outside of process package have been removed, as they now only need to concern themselves with process.Commander, and not KillWaiter directly.
  • The Logger, GracefulKillTimeout, ForceKillTimeout already existed, but were not really being used. Conveniently, they're now needed.

Why was this MR needed?

Eventually, new process management/kill strategies need to be implemented, and the existing Killer functionality will need to be aware of how a process is also setup. Right now, Killer cannot be made aware of this due to the command and killer setup being too detached from one another.

What's the best way to test this MR?

  • There's an existing integration test in process: TestKiller that kills a real process.

I haven't tested this myself yet, I think to test both graceful/force kill, two manual tests would be:

  • Canceling a job should graceful terminate processes when the shell executor is used.
  • Forcefully exiting Runner (Ctrl+C twice) should SIGKILL processes when the shell executor is used.

What are the relevant issue numbers?

#27624

Edited by Elliot Rushton

Merge request reports