What
Send one heartbeat before each step, so the lease is renewed at every step boundary and not only inside a step that happens to outlive one heartbeat interval.
Why
#243. A step heartbeats while it runs, but the ticker starts with the step
(ci/agent/agent.go, runCommand), so a step shorter than lease/3 never heartbeats at
all, and nothing renewed the lease between steps. An attempt whose steps add up to more
than lease_seconds expired while its cheap steps were running, and the first heartbeat of
the long step at the end was answered ok=false:
01:26:49 execStep "test"
01:27:09 heartbeat sent, took 7ms, ok=false
vole runner: lease lost while running "test"; stopping
Twelve short steps fit in that gap. Attempts died at 61 s, 65 s and 77 s with no result, each retry re-ran every step, and five of them failed the run.
How it is proven
On a real runner, lease_seconds at its default 60, with a nine-step workflow whose steps
are each shorter than the interval (sleep 10 × 8, then sleep 60):
- Unfixed binary: the run died at the boundary four times, no attempt recorded.
- Fixed binary: the same run completed inside one lease in 3m12s and recorded an attempt,
failing only on
TestTrimDropsTheOldestNamespaceWhole— theci/cachebug that #236 and #242 cover, which is not onmain.
The instrumented run also showed the renewal working where it used to be missing:
heartbeat ok=true at every boundary.
Tests
TestHeartbeatBetweenStepsNamesTheRun— the boundary request isop=heartbeatwith the run it belongs to, and the answer is what the step loop acts on.TestHeartbeatWithoutAConnectionIsNotHeld— a heartbeat that cannot be sent reports the lease as gone, so an attempt stops instead of running a step it may no longer own.
What the tests cannot cover is the step loop itself, which needs a container runtime; that is what the runner run above is for.
What I ran
./vole workflow run verify—ok: 1 job(s)in the job image via docker.go vet ./ci/agent,go test ./ci/agent -count=2,./vole workflow check.
Risk
One extra request per step: a workflow with twenty steps makes twenty more round trips, each a few milliseconds on the same channel the log already uses. The step ticker and the build ticker are untouched, so a long step still renews while it runs.
lease_seconds is set to 300 on the instance as a temporary workaround for the unfixed
runner; it can go back to the default once this lands and the runner is upgraded.