[htoyoda18]

What this PR does

When a service waits on a dependency with condition: service_healthy or service_completed_successfully, Compose always waited for one 500ms poll interval before it checked the dependency for the first time, even when the dependency already satisfied the condition. This PR checks these two conditions as soon as the wait starts, so a no-op docker compose up -d goes from ~0.6s to ~0.07s. The running_or_healthy check used by up --wait still waits for the first tick, so --wait keeps reporting a container that exits right after starting as a failure.

Related issue

Fixes #14384

Changes made

• pkg/compose/service_containers.go: waitDependency checks service_healthy and service_completed_successfully on entry, without waiting for a tick. running_or_healthy (used by --wait, and for dependencies of a project rebuilt from container labels) is still first checked on the first tick. It accepts a running container that has no healthcheck, so a check right after start would report a container that exits within the interval as running. If the context is already done on entry, the wait still ends through the existing ctx.Done() branch without inspecting the dependency: a cancellation stays silent, and a deadline keeps its current meaning.
• pkg/compose/service_containers_test.go: add TestWaitDependencyFirstCheck, which uses testing/synctest to lock when the first check of an already satisfied dependency happens: after 0s for service_healthy and service_completed_successfully, and after exactly 500ms for running_or_healthy with a running container that has no healthcheck. TestWaitDependencyDeadline and TestOptionalDependencyInspectFailure assumed that a 50ms timeout always expires before any check. They now let the check on entry find the dependency still starting, so they still cover a deadline that expires between two polls.
• pkg/compose/executor_start_test.go: rewrite a comment that said the wait "never" checks on entry.
• pkg/e2e/testdata/TestUpWithDependencyExit/compose.yaml: give db a healthcheck. Without one, the test's expected exited (1) only won over has no healthcheck configured because the first check came 500ms after start.

Testing done

• Unit tests pass: make test
• E2E tests pass: make e2e-compose or make e2e-compose-standalone
• Linting passes: make lint
• Code formatted: make fmt
• Manually tested (describe):

AI tool used (if applicable)

AI Tool:

Additional context

• PR fix(executor): report the services a failed start interrupts #14376 also includes changes to waitDependency.

(not mandatory) A picture of a cute animal, if possible in relation to what you did

[ndeloof]

LGTM, thanks for the fix and for locking the first-check timing in a test.

I went through the side effects I could think of and found none: the check* functions are untouched, only the instant of their first call changes for service_healthy and service_completed_successfully, and running_or_healthy (--wait) keeps waiting for the first tick, which is the right call.

Could you rebase on main? #14376 has been merged since and it reworked the loop in waitDependency, so the PR now conflicts in service_containers.go. One thing to keep when resolving: on main the select on the ticker and ctx.Done() is followed by an if ctx.Err() != nil, so that a canceled wait is never run through one more check when both are ready together. The check on entry can simply skip the select on the first iteration and fall through to that same ctx.Err() test.

I tried that resolution locally on top of main: the build, go test ./pkg/compose/ -race and the lint (only the two existing errcheck) are all fine, and the wait/executor tests are stable over repeated runs.

[htoyoda18]

@ndeloof Thanks for the review! Rebased on main and resolved the conflict as you suggested: on the first iteration, service_healthy and service_completed_successfully skip the select and fall through to the same ctx.Err() test, so the extra ctx.Err() != nil condition from the previous version is gone. running_or_healthy still waits for the first tick. I also trimmed the comments I had added.

[htoyoda18]

@ndeloof

Follow-up: CI caught something my local run missed. TestUpWithDependencyExit waits with service_healthy on a dependency that has no healthcheck and exits at once. The first check now often runs before the exit, so it reports has no healthcheck configured instead of exited (1). up still fails, only the message differs.

I gave db a healthcheck in the test's compose file, since the scenario is "exits before turning healthy". With it, the test reports exited (1) whatever the timing.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[ndeloof]

Thanks @htoyoda18, the change itself is good and everything else checks out. One blocking point though: this PR makes an existing test flaky.

TestExecutePlanCanceledCallerReportsNothingMore (pkg/compose/executor_start_test.go) now fails intermittently under -race with api: [Starting] instead of no event:

• on main: 0 failures out of 150 runs (-race, GOMAXPROCS=2)
• on this branch: 20 failures out of 300 (same conditions), about 1 in 300 without the CPU constraint

Repro: GOMAXPROCS=2 go test ./pkg/compose/ -race -count=300 -run 'TestExecutePlanCanceledCallerReportsNothingMore$'

Why: the test cancels the caller's context from inside the first ContainerInspect of the wait. Before this PR that inspection only happened after the first 500ms tick, so the api start node was long parked in its dependency select and ctx.Done() woke it up first. Now the inspection happens on entry, so the cancel can land before that goroutine reaches its select. When it does, done and ctx.Done() are both ready, select picks at random, and api emits a Starting event after the cancellation. The test was relying on that timing without saying so.

The race in runNode itself is not introduced by this PR (it is the same pattern waitDependencyReached fixed by letting ctx win), and I will handle it separately. For this PR the test just needs to stop depending on scheduling. One way: run it under synctest.Test as you already do in TestWaitDependencyFirstCheck, make the mocked inspection wait on a channel, call synctest.Wait() from the test goroutine (it returns once the api node is durably blocked on its dependency), then cancel() and release the inspection.

Please also run the test with -race -count=300 and GOMAXPROCS=2 once fixed.

[htoyoda18]

@ndeloof Thanks! I fixed the test under synctest.Test as you suggested, in a separate commit so the re-indentation is easy to review (happy to squash before merge). With GOMAXPROCS=2 -race -count=300, it now passes 600/600 (28/300 failed before).