CmdRunner Start and Wait Ignore the Group Timeout

[meling]

Group.Run builds a timeout context (context.WithTimeout(..., g.Timeout)) and passes it to each task function. But the CmdRunner "start now, wait later" pattern — Start(cmd) then Wait() — ignores that context entirely: sshCmd.Wait() calls ssh.Session.Wait(), which blocks until the remote process exits, regardless of the deadline. Only RunContext honors the context (it closes the session when the context fires).

So a task that starts a command and waits separately gets no timeout, even though it was handed a deadline-bearing context. One hung remote process blocks the Group task forever. The timeout looks enforced but is not.

How it was hit

The gorums cluster sweep tool (cmd/sweep) launched benchmark nodes with cmd.Start() + cmd.Wait() under a withTimeout(g, 5m+duration) group. A benchmark node hung during teardown; because Wait() ignored the group
timeout, the whole driven sweep wedged for ~14h instead of failing the run after ~5 minutes. It has been fixed on the caller side by switching to RunContext, but the iago footgun remains for the next caller.

Root cause

Start/Wait are the only blocking CmdRunner methods that take no context yet can be reached with one in scope. The interface offers three ways to run a command, and only one of the two blocking-with-a-deadline paths honors it:

• Run(cmd) — blocks, no context. Honest: no deadline is in scope.
• RunContext(ctx, cmd) — blocks, honors ctx. Correct.
• Start(cmd) + Wait() — blocks, silently ignores any ctx. The footgun.

Proposed fix: remove Start and Wait from the CmdRunner interface

RunContext(ctx, cmd) already equals Start(ctx, cmd) + Wait(). Every use of the separate start/wait pair is expressible as RunContext with the pipes wired before the call (drained by goroutines), and run inside a goroutine when concurrency across several commands is needed — which is exactly what Shell.Apply does and what the sweep's launchAndWait now does. So the separate methods buy no capability that RunContext + a goroutine does not.

Proposed interface:

type CmdRunner interface {
        Run(cmd string) error
        RunContext(ctx context.Context, cmd string) error
        StdinPipe() (io.WriteCloser, error)
        StdoutPipe() (io.ReadCloser, error)
        StderrPipe() (io.ReadCloser, error)
}

Keep start/wait as unexported helpers on sshCmd behind RunContext, or inline them. This removes the footgun by construction: the only blocking runners are Run (deadline-free by design) and RunContext (ctx-honoring).

Consider also whether Run should stay or be folded into RunContext(context.Background(), cmd); it is a thin convenience over ssh.Session.Run. Minor, orthogonal.

Compatibility

Breaking change to the CmdRunner interface either way; no compatibility promised. Known in-tree consumer (gorums cmd/sweep) already uses RunContext, so it is unaffected. Audit other relab consumers before merging.

Test

Add a Docker-gated regression test (see iago_test.go / iagotest.CreateSSHGroup): start a long-running remote command (sleep 60) under a 500ms context and assert the wait returns well before the command would finish. Before the fix this hangs; after, it returns promptly. (fix/start-honors-context includes TestStartHonorsContextTimeout in this shape.)