fix: budget the drain against the container's own healthcheck

[vishr]

Closes #109.

A rolling deploy marks a container unhealthy, waits for the runtime to notice, then stops it — so the proxy has stopped routing before the container goes away. That wait was budgeted as retries × pollEvery, where pollEvery is how often Onebox runs docker inspect. The flip it waits for happens at retries × the interval the container's own healthcheck runs at.

Different jobs: one is a local query, the other a probe inside the container. They agreed only when health.interval was authored. With the shorthand health: /path no interval was written into the generated healthcheck at all, so Docker applied its own 30s default while Onebox budgeted against its 2s poll cadence — a flip needing 90s against a 10s budget. It timed out on every replica of every deploy and stopped each container while the proxy might still be routing to it, which is exactly what the budget exists to prevent.

The budget now comes from the container being drained

Not from the spec. A healthcheck is baked in at creation, so the spec describes the containers being started, never the ones being drained.

This distinction is the whole fix. Budgeting from the spec is correct going forward but leaves the failure intact for one more deploy after any change to the probe timing — including a change to Onebox's own default, which no operator asked for and which would otherwise strand every replica of the first deploy after upgrading. It is read back with the same docker inspect that already checks whether the check is drain-guarded, widened from .Config.Healthcheck.Test to .Config.Healthcheck, with Docker's own defaults for omitted fields.

TestDrainBudgetCoversAContainerBakedBeforeTheChange pins that case: a container whose baked healthcheck has no interval or retries, exactly what Onebox emitted before this change.

Nothing is left to the runtime's defaults

interval, retries, and start_period are all written into the generated healthcheck now, so what Onebox reasons about and what the container does cannot drift apart. An unset interval becomes 5s rather than the runtime's 30s: twelve probes a minute per container, and a drained one leaves rotation in fifteen seconds, which keeps a rolling deploy from being dominated by waiting for the flip.

Writing down a fast interval is only safe with a grace period to match. Before, the runtime's 30s interval gave a booting container roughly 90s before a failed probe could count against it; 5s with no start period would call a slow-starting application unhealthy while it is still coming up — a verdict visible to dependency conditions, restart watchers, and alerting long before the rollout would notice. start_period is therefore written too, at 30s, which is the delay the first probe already had. The runtime leaves the start period at first success, so a fast application pays none of it.

Two adjacent holes this exposed

• The readiness budget. within defaults to 120s, chosen for ordinary probe timings. With interval: 3m the newcomer's first probe lands after it expires, so the rollout would rm -f a container that was never given the chance to answer. The default now stretches to cover one full flip cycle; an authored within is still taken at its word.
• retries had no upper bound. It multiplies the probe interval to give the budget, and a count large enough to overflow that arithmetic yields a negative budget — one that expires immediately, which is the original failure reached by another route. Bounded at 1000, orders of magnitude above any real healthcheck.

Not added, deliberately

The issue suggested validating that retries × interval fits the drain budget. The budget is (retries + 2) × interval against a flip of at most (retries + 1) × interval, so it covers the flip by construction — there is no longer a configuration that can fail this way to warn about.

Testing

A virtual clock that advances only when the engine sleeps, plus a fake that models Docker properly: a drained container keeps reporting healthy until the flip is actually due. The previous fake flipped the instant it was drained, which is why a budget too short for the real flip never failed a test.

Both drain tests use worst-case phase alignment — a real probe cycle is not aligned to the moment the drain file appears, so a budget of exactly retries × interval would pass a best-case test and still strand containers in the field. Both also assert the drain wait actually ran, since a rollout that decides the check cannot be drain-guarded skips the wait entirely and would pass vacuously.

• just check, just lint — pass
• go test -race ./... — pass, 1619 tests
• ONEBOX_E2E=1 go test ./e2e/ with Docker — pass (296s)
• The issue's exact reproducer now renders interval: 5s, retries: 3, start_period: 30s

Compatibility

Every generated runtime that declares a healthcheck changes, because it now carries three keys it previously left to the runtime. Frozen corpus verdicts are regenerated accordingly. Behaviour for anyone who already authored interval, retries, and start_period is unchanged.

🤖 Generated with Claude Code

[vishr]

Four review findings, all real. Fixed and force-pushed as 022601d.

1. A leftover raw echo defeated the normalisation in the same hunk

generate.go set start_period from the model and then immediately overwrote it with the author's literal string. Onebox's duration grammar accepts 14d; Compose's does not know the unit at all, so start_period: 14d validated cleanly and then killed the deploy at the compose step with time: unknown unit "d" — the exact bug this change fixed for interval, left in place one line below. My own test locked it in by asserting "90s" rather than 1m30s.

Every emitted duration now goes through the model. The test asserts both the normalisation and that no emitted duration carries a unit Compose cannot parse.

2. The retries bound did not close the overflow it documented

Bounding one operand does nothing when the other is unbounded: gDur accepts 100000d, so {interval: 100000d, retries: 2} passed validation and wrapped the readiness budget to roughly +237 years. An effectively infinite budget means a crash-looping newcomer that should abort the rollout hangs instead.

interval, start_period, and within are now bounded at 7 days — far past the point where a health check measures anything. Added a test that the budget stays positive at every accepted extreme.

3 + 4. The plan promised a drain step the deploy never takes

DrainWait()'s derived fallback was unreachable: roll.go, recreate.go, and plan.go all check drain.wait was authored before asking for it. Its only effect was that ob plan printed docker kill --signal=USR1 <current web>; wait 15s for a workload with drain: {signal: USR1} and no wait, while execution sent no signal and slept not at all. Pre-existing, but this change doubled the number the plan lied with (6s to 15s).

The plan now gates exactly as recreate does, and DrainWait() returns 0 rather than a value nothing can observe. That required changing a pre-existing test, TestDrainWaitDerivesFromHealthTiming, which asserted the unreachable value — flagging that explicitly rather than burying it. Deriving a real wait would add a sleep to every deploy naming a drain signal, which is a change to make deliberately, not via a fallback nothing calls.

Both halves are mutation-checked: reverting either alone leaves the test passing (they overlap), reverting both fails it.

Re-verified

just check 0, just lint 0, go test -race ./... 0 (1624 tests), ONEBOX_E2E=1 go test ./e2e/ pass (291s). The reproducer still renders interval: 5s, retries: 3, start_period: 30s.