Skip to content

fix(up): enforce healthcheck.timeout and treat start_period as a grace window - #151

Open
Mikimoto wants to merge 1 commit into
Mcrich23:mainfrom
Mikimoto:fix/healthcheck-timeout-start-period
Open

fix(up): enforce healthcheck.timeout and treat start_period as a grace window#151
Mikimoto wants to merge 1 commit into
Mcrich23:mainfrom
Mikimoto:fix/healthcheck-timeout-start-period

Conversation

@Mikimoto

@Mikimoto Mikimoto commented Sep 1, 2026

Copy link
Copy Markdown

fix(up): enforce healthcheck.timeout and treat start_period as a grace window

Summary

depends_on: condition: service_healthy works today (#61, #75, #117), but two of the five
healthcheck fields do not behave as Compose specifies:

  1. timeout is decoded and never applied. A probe that blocks stalls up indefinitely.
  2. start_period is implemented as an unconditional pre-sleep, not as Compose's grace window.
    Every service that declares one pays it in full even when it is ready immediately.

Both are in waitUntilServiceIsHealthy.


1. timeout is parsed but never applied

Healthcheck.timeout is decoded (Codable Structs/Healthcheck.swift:79) and has parsing tests,
but waitUntilServiceIsHealthy (Commands/ComposeUp.swift:1247) never reads it:

let exitCode = try await streamCommand(
    "container",
    args: ["exec", containerName] + execArguments,
    onStdout: { _ in },
    onStderr: { _ in }
)

streamCommand (Commands/ComposeUp.swift:1481) resumes its continuation only from
process.terminationHandler, so the probe is unbounded. grep -rn timeout Sources/ returns hits
only in Healthcheck.swift plus unrelated comments about the start wait.

Reproducer

services:
  hang:
    image: alpine:3
    command: ["sleep", "infinity"]
    healthcheck:
      test: ["CMD", "sleep", "600"]
      interval: 1s
      timeout: 1s
      retries: 3
  app:
    image: alpine:3
    command: ["echo", "ok"]
    depends_on:
      hang:
        condition: service_healthy
behaviour
docker compose up each probe killed at 1s; 3 consecutive failures mark hang unhealthy; fails in ~5s with dependency failed to start
container-compose up on main first probe runs the full 600s, then repeats twice. up is silent for ~30 minutes

This is not synthetic. Healthchecks that talk to a socket block for real:

test: ["CMD", "bash", "-c", "exec 3<>/dev/tcp/127.0.0.1/5432 && exec 4<>/dev/tcp/127.0.0.1/5433"]

bash's /dev/tcp has no connect timeout, so against a filtered (not refused) port it blocks until
the kernel gives up. timeout: 5s is exactly what is supposed to bound it.


2. start_period is a pre-sleep, not a grace window

if startPeriod > 0 {
    try await Task.sleep(nanoseconds: UInt64(startPeriod * 1_000_000_000))
}

Compose semantics are the other way round: probes do run during start_period, their failures
do not consume retries, and the first success ends the window immediately. Today the first
probe cannot run until the whole window has elapsed, so start_period: 40s costs 40 seconds even
for a service that is ready in 200 ms.

up starts services sequentially (#128 tracks parallelising it), so the cost is additive across the
project. Measured on a real 23-service compose file (YAML merge keys resolved, so anchor-inherited
values are counted): 18 services declare a healthcheck — 9 at start_period: 10s, 4 at 15s, 1 at
20s, 3 at 30s, 1 at 40s. That is 300 seconds of mandatory sleep before those services are first
probed, regardless of how quickly they actually become ready.


What this PR changes

  • streamCommand gains an optional timeout:. When it elapses and the child is still running the
    process is terminated; the termination handler then fires with a non-zero status, so the caller
    sees an ordinary failed check. This matches Docker, where a timed-out probe counts as one failure
    rather than as an error.
  • waitUntilServiceIsHealthy passes healthcheck.timeout (Compose default 30s) to every probe.
  • start_period becomes a grace window: probes run at interval inside it, failures there do not
    consume the retry budget, and the first success returns.

timeout: defaults to nil, so the two existing streamCommand call sites (run, and the
foreground service stream) keep their current unbounded behaviour. No other call site changes.


Tests

New Tests/Container-Compose-StaticTests/HealthcheckTimeoutTests.swift:

  • streamCommand terminates a child that overruns its timeout: sleep 30 with timeout: 1
    returns non-zero in ~1.1 s. On main the same call takes 30 s and returns 0.
  • a child that finishes inside its timeout is left alone and its exit code preserved.
  • an omitted timeout resolves to the Compose default of 30 s, not to "no timeout".

These need no running container daemon.

Verification

Run on macOS 27.0 / Swift 6.4, branched from main @ 6e6aaf0:

check result
swift build 0 errors
swift test (static suites) 239 tests in 22 suites passed
new suite 3 tests passed in 1.14 s

Negative control: with process.terminate() deleted from the new timeout branch, the
streamCommand terminates a child that overruns its timeout test fails on both expectations and
takes 30.039 s instead of 1.090 s. The test does discriminate the fix from its absence.

Not covered: an end-to-end container-compose up against a live daemon. The start_period change
has no unit test for the same reason, only the reasoning above and the existing dynamic suite.


Notes and limits

  • Terminating the host-side container exec does not guarantee the process inside the guest is
    reaped. Docker has the same caveat with its own exec-based probes.
  • The grace-window loop probes at interval, so with interval > start_period it can overshoot the
    window by up to one interval. Docker behaves the same way.
  • Dropping the pre-sleep is a behaviour change: a compose file that leaned on start_period as a
    hard floor to mask a startup race will now proceed earlier. That is the Compose-correct behaviour,
    but it is worth a line in the release notes.
  • Not addressed here: Compose keeps probing after a service is healthy and lets an unhealthy
    container recover. Container-Compose still treats the health wait as a one-shot gate during up.
    That is a larger change and belongs with full support: depends_on #68.

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