mirror of
https://github.com/docker/compose.git
synced 2026-10-12 02:13:08 +00:00
Nicolas De Loof
eaf172eab2
pre_start hook containers become first-class resources of the reconciliation plan. The reconciler plans one OpCreateHookContainer per declared hook — gated on the same predicate the start phase uses to run the hooks: a replica to start and no running replica SURVIVING the plan (a recreated replica is not running at start time, so its runners are planned too). Runner creation depends on every replica node (the executor resolves the VolumesFrom target against a final live view), on the infrastructure, and on the purge of previously observed runners: names are deterministic (<project>-<service>-pre_start-<i>) so repeated plans converge on the same container instead of accumulating anonymous ones, and a new HookIndexLabel ties each runner to its hook. runPreStart becomes pure execution: it looks up the created-state runner for each hook by label and never creates one. A declared hook without a prepared runner is an actionable error naming the reconciliation command (docker compose up <service>) — the accepted consequence is that stop-then-start of a hooked service errors, since runners are consumed on success (locked by e2e). Success removal, failure retention and cancellation cleanup are unchanged. Runners stay invisible to the start/ps container listings — they carry no ConfigHashLabel, which getDefaultFilters requires — and down keeps removing them by hook label. Rebased onto main, which diverged substantially since this branch was opened (the unrelated container-spec-layering refactor, #14093, restructured compose-go's ServiceConfig/PreStartHook types this PR also touches). Reconciling pre_start.go/pre_start_test.go against that refactor surfaced a real bug: createPreStartContainer builds the hook's config via the shared getCreateConfigs, which unconditionally stamps a ConfigHashLabel — silently contradicting this same commit's own "invisible to ps/start" guarantee above, since that label is exactly what getDefaultFilters requires. Fixed by deleting the label after getCreateConfigs returns, with a test asserting its absence (an independent local review, via the GitHub Copilot CLI, and a fresh-context review agent both converged on catching this during rebase verification). Also restored two security/label tests that only existed on one side of the rebase (SecuritySensitiveFieldsHonored, HookLabelsMerged), ported to call createPreStartContainer directly. Also addresses glours' three review comments on this PR: hook-container create operations for a service no longer chain serially (each depends on the same shared containerNodes+infraDeps+purges set, letting the executor create them in parallel), the corresponding plan-string test assertion, and a clearer missing-runner error message hinting this isn't necessarily a planning bug. Signed-off-by: Nicolas De Loof <[email protected]>