Skip to content

Commit b175494

Browse files
committed
feat: reconciler plans the pre_start hook lifecycle
pre_start hooks run in ephemeral containers whose lifecycle — purge of stale runners left by a previous failed run, creation, removal after success — used to live entirely inside the imperative primitive, opaque to the reconciliation plan. The plan now describes it: when pre_start is going to run, each hook chains CreateHookContainer → RunPreStart → RemoveContainer, preceded by best-effort removals of observed stale runners. Only the run node stays imperative (start, wait, log streaming, retain-on-failure) and no longer manages the container it executes: a failed hook aborts before its remove node, so the container is retained for inspection and purged as stale on the next run — the imperative semantics, expressed as graph structure. Observed state learns to tell hook containers apart: they carry no container-number label and previously classified as a service replica numbered 0. They now land in a dedicated HookContainers bucket the reconciler plans purges from. The imperative path is recomposed from the same pieces (createPreStartContainer + execPreStartHook + explicit removal) and behaves identically, as locked by the existing characterization tests. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
1 parent 5439032 commit b175494

6 files changed

Lines changed: 291 additions & 53 deletions

File tree

pkg/compose/observed_state.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ type ObservedState struct {
4646
// others as orphans (see selectNetwork/selectVolume).
4747
Networks map[string][]ObservedNetwork // compose network key → observed
4848
Volumes map[string][]ObservedVolume // compose volume key → observed
49+
// HookContainers are ephemeral lifecycle-hook runners (HookLabel set),
50+
// per service. Any observed at collection time is stale by definition —
51+
// a previous run failed before removing it — and the reconciler plans
52+
// its purge before re-running the hooks.
53+
HookContainers map[string][]ObservedContainer // service name → hook containers
4954
}
5055

5156
// selectNetwork picks, among the live networks recorded for a compose key, the
@@ -148,6 +153,8 @@ func (s *composeService) collectObservedState(ctx context.Context, project *type
148153
Containers: map[string][]ObservedContainer{},
149154
Networks: map[string][]ObservedNetwork{},
150155
Volumes: map[string][]ObservedVolume{},
156+
157+
HookContainers: map[string][]ObservedContainer{},
151158
}
152159

153160
// --- Containers ---
@@ -170,6 +177,15 @@ func (s *composeService) collectObservedState(ctx context.Context, project *type
170177

171178
for _, ctr := range raw {
172179
svcName := ctr.Labels[api.ServiceLabel]
180+
if ctr.Labels[api.HookLabel] != "" {
181+
// lifecycle-hook containers (ephemeral pre_start runners) are
182+
// neither service replicas nor one-offs: classified apart, so
183+
// they never masquerade as a replica (they carry no
184+
// container-number label and would otherwise read as number 0)
185+
// and the reconciler can plan purging stale ones.
186+
state.HookContainers[svcName] = append(state.HookContainers[svcName], toObservedContainer(ctr))
187+
continue
188+
}
173189
if isNotOneOff(ctr) && knownServices[svcName] {
174190
state.Containers[svcName] = append(state.Containers[svcName], toObservedContainer(ctr))
175191
} else if isOrphaned(project)(ctr) {

pkg/compose/observed_state_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,20 @@ func TestCollectObservedState(t *testing.T) {
160160
api.OneoffLabel: "True",
161161
},
162162
},
163+
{
164+
// Stale lifecycle-hook runner (a previous run failed before
165+
// removing it): neither a replica (it has no container-number
166+
// label and must not read as number 0) nor a one-off —
167+
// classified apart so the reconciler can plan its purge.
168+
ID: "c5",
169+
Names: []string{"/hook-runner"},
170+
State: container.StateExited,
171+
Labels: map[string]string{
172+
api.ServiceLabel: "web",
173+
api.ProjectLabel: "myproject",
174+
api.HookLabel: "pre_start",
175+
},
176+
},
163177
},
164178
}, nil)
165179

@@ -202,6 +216,11 @@ func TestCollectObservedState(t *testing.T) {
202216
assert.Equal(t, len(state.Containers["db"]), 1)
203217
assert.Equal(t, state.Containers["db"][0].ID, "c2")
204218

219+
// The hook runner is classified apart — not a "web" replica (asserted
220+
// above: 1 replica), not an orphan
221+
assert.Equal(t, len(state.HookContainers["web"]), 1)
222+
assert.Equal(t, state.HookContainers["web"][0].ID, "c5")
223+
205224
// Orphans: only the model-absent service "old". The running one-off c4 is
206225
// absent everywhere — not in the "web" bucket (asserted above: 1 replica),
207226
// not an orphan: up leaves live `compose run` sessions alone.

pkg/compose/plan.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ const (
5858
OpWaitCondition OperationType = 40
5959
OpRunPreStart OperationType = 41
6060
OpRunPostStart OperationType = 42
61+
// OpCreateHookContainer creates the ephemeral container an OpRunPreStart
62+
// node then executes: the hook's lifecycle (stale-runner purge, creation,
63+
// post-success removal) belongs to the plan, only its execution — start,
64+
// wait, log streaming, retain-on-failure — stays with the imperative
65+
// primitive.
66+
OpCreateHookContainer OperationType = 43
6167
)
6268

6369
// PlanPhase situates a node in the plan lifecycle. The Create phase converges
@@ -105,6 +111,8 @@ func (o OperationType) String() string {
105111
return "RunPreStart"
106112
case OpRunPostStart:
107113
return "RunPostStart"
114+
case OpCreateHookContainer:
115+
return "CreateHookContainer"
108116
default:
109117
return fmt.Sprintf("Unknown(%d)", int(o))
110118
}
@@ -127,6 +135,7 @@ type Operation struct {
127135
Timeout *time.Duration // for stop operations
128136
CreateNodeID int // for OpRenameContainer/start-phase ops: ID of the CreateContainer node whose result to target
129137
Condition string // for OpWaitCondition: depends_on condition to wait for (service_healthy, ...)
138+
HookIndex int // for OpCreateHookContainer/OpRunPreStart: index into the service's PreStart hooks
130139
// BestEffort marks an operation whose failure must not abort the plan. It is
131140
// used for the optional removal of the old network on a rename: if the
132141
// network is still in use (by non-Compose containers) the removal is skipped

pkg/compose/pre_start.go

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -79,26 +79,38 @@ func (s *composeService) runPreStart(ctx context.Context, project *types.Project
7979
logrus.Warnf("service %q: failed to remove stale pre_start hook containers: %v", service.Name, err)
8080
}
8181
for i, hook := range service.PreStart {
82-
if err := s.runPreStartHook(ctx, project, service, ctr, i, hook, listener); err != nil {
82+
created, err := s.createPreStartContainer(ctx, project, service, ctr, hook)
83+
if err != nil {
84+
return err
85+
}
86+
if err := s.execPreStartHook(ctx, service, i, created.ID, listener); err != nil {
8387
return err
8488
}
89+
// Success: remove the hook container, mirroring the old AutoRemove behaviour
90+
// (including its anonymous volumes). A removal failure is logged but does not
91+
// gate service start — the hook already succeeded.
92+
if _, removeErr := s.apiClient().ContainerRemove(ctx, created.ID, client.ContainerRemoveOptions{RemoveVolumes: true}); removeErr != nil {
93+
logrus.Warnf("service %q pre_start[%d]: failed to remove hook container %s: %v", service.Name, i, created.ID, removeErr)
94+
}
8595
}
8696
return nil
8797
}
8898

89-
func (s *composeService) runPreStartHook(
90-
ctx context.Context, project *types.Project, service types.ServiceConfig,
91-
ctr container.Summary, index int, hook types.ServiceHook, listener api.ContainerEventListener,
99+
// execPreStartHook starts an already-created hook container, streams its logs
100+
// and waits for its exit. It owns only execution-failure handling: a container
101+
// that never started or a run cancelled by the user is removed, a genuinely
102+
// failed hook is retained for post-mortem inspection. Removing the container
103+
// after a successful run is the caller's job — the container's lifecycle
104+
// belongs to whoever created it (the imperative runPreStart loop today, the
105+
// reconciliation plan once the executor runs hook nodes).
106+
func (s *composeService) execPreStartHook(
107+
ctx context.Context, service types.ServiceConfig,
108+
index int, containerID string, listener api.ContainerEventListener,
92109
) error {
93-
created, err := s.createPreStartContainer(ctx, project, service, ctr, hook)
94-
if err != nil {
95-
return err
96-
}
97-
98110
// Subscribe to wait before start to avoid missing the exit event for short-lived hooks.
99111
// WaitConditionNotRunning would match immediately because the container is still in
100112
// "created" state, so use WaitConditionNextExit to block until the run actually finishes.
101-
waitRes := s.apiClient().ContainerWait(ctx, created.ID, client.ContainerWaitOptions{
113+
waitRes := s.apiClient().ContainerWait(ctx, containerID, client.ContainerWaitOptions{
102114
Condition: container.WaitConditionNextExit,
103115
})
104116

@@ -108,13 +120,13 @@ func (s *composeService) runPreStartHook(
108120
// open cannot deadlock `<-logsDone`.
109121
logCtx, cancelLogs := context.WithCancel(ctx)
110122
defer cancelLogs()
111-
logsDone, getTail := s.streamPreStartLogs(logCtx, created.ID, service, index, listener)
123+
logsDone, getTail := s.streamPreStartLogs(logCtx, containerID, service, index, listener)
112124

113-
if _, err := s.apiClient().ContainerStart(ctx, created.ID, client.ContainerStartOptions{}); err != nil {
125+
if _, err := s.apiClient().ContainerStart(ctx, containerID, client.ContainerStartOptions{}); err != nil {
114126
// AutoRemove is false, so we must remove the never-started container
115127
// explicitly. A failed removal is logged so the orphan is visible.
116-
if _, removeErr := s.apiClient().ContainerRemove(ctx, created.ID, client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}); removeErr != nil {
117-
logrus.Warnf("service %q pre_start[%d]: failed to remove orphan hook container %s: %v", service.Name, index, created.ID, removeErr)
128+
if _, removeErr := s.apiClient().ContainerRemove(ctx, containerID, client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}); removeErr != nil {
129+
logrus.Warnf("service %q pre_start[%d]: failed to remove orphan hook container %s: %v", service.Name, index, containerID, removeErr)
118130
}
119131
// Drain waitRes so the client's wait goroutine exits without having to
120132
// wait for the parent context to be canceled.
@@ -136,15 +148,15 @@ func (s *composeService) runPreStartHook(
136148
// and return the raw context error without decorating it with the tail or
137149
// retaining the container for post-mortem inspection.
138150
if ctx.Err() != nil {
139-
if _, removeErr := s.apiClient().ContainerRemove(context.Background(), created.ID, client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}); removeErr != nil {
140-
logrus.Warnf("service %q pre_start[%d]: failed to remove hook container %s after cancellation: %v", service.Name, index, created.ID, removeErr)
151+
if _, removeErr := s.apiClient().ContainerRemove(context.Background(), containerID, client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}); removeErr != nil {
152+
logrus.Warnf("service %q pre_start[%d]: failed to remove hook container %s after cancellation: %v", service.Name, index, containerID, removeErr)
141153
}
142154
return waitErr
143155
}
144156
// Genuine hook failure: retain the container so the operator can run
145157
// `docker logs <id>` and `docker inspect <id>` to diagnose the failure.
146158
// Include the short container ID in the error to make it actionable.
147-
shortID := created.ID
159+
shortID := containerID
148160
if len(shortID) > 12 {
149161
shortID = shortID[:12]
150162
}
@@ -153,12 +165,6 @@ func (s *composeService) runPreStartHook(
153165
}
154166
return fmt.Errorf("%w (hook container %s retained for inspection)", waitErr, shortID)
155167
}
156-
// Success: remove the hook container, mirroring the old AutoRemove behaviour
157-
// (including its anonymous volumes). A removal failure is logged but does not
158-
// gate service start — the hook already succeeded.
159-
if _, removeErr := s.apiClient().ContainerRemove(ctx, created.ID, client.ContainerRemoveOptions{RemoveVolumes: true}); removeErr != nil {
160-
logrus.Warnf("service %q pre_start[%d]: failed to remove hook container %s: %v", service.Name, index, created.ID, removeErr)
161-
}
162168
return nil
163169
}
164170

pkg/compose/reconcile.go

Lines changed: 81 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1187,25 +1187,7 @@ func (r *reconciler) planServiceStart(service types.ServiceConfig) error {
11871187
// observation — the imperative gating (initial up, force-recreate, or
11881188
// spec change), decided at plan time
11891189
if len(service.PreStart) > 0 && !anyRunning {
1190-
serviceCopy := service
1191-
first := replicas[0]
1192-
op := Operation{
1193-
Type: OpRunPreStart,
1194-
ResourceID: first.resID,
1195-
Cause: "pre_start hooks",
1196-
Service: &serviceCopy,
1197-
Container: first.container,
1198-
CreateNodeID: first.createNodeID,
1199-
}
1200-
// clone: appending into prev's backing array would silently mutate
1201-
// depNodes' hidden capacity (they share it)
1202-
deps := slices.Clone(prev)
1203-
if first.after != nil {
1204-
deps = append(deps, first.after)
1205-
}
1206-
preStart := r.plan.addNode(op, startGroupID(first.resID), deps...)
1207-
preStart.Phase = PhaseStart
1208-
prev = []*PlanNode{preStart}
1190+
prev = r.planPreStart(service, replicas[0], prev)
12091191
}
12101192

12111193
// the replica chain: inject+start then post_start of replica n+1 waits
@@ -1247,6 +1229,86 @@ func (r *reconciler) planServiceStart(service types.ServiceConfig) error {
12471229
return nil
12481230
}
12491231

1232+
// planPreStart plans the pre_start sequence gating the first replica start.
1233+
// The hook containers' lifecycle belongs to the plan: stale runners left by a
1234+
// previous run that failed before removing them are purged first (best-effort,
1235+
// the imperative warn-only removal), then each hook chains create → run →
1236+
// remove. Only the run stays an imperative primitive — start, wait, log
1237+
// streaming, retain-on-failure — it no longer manages the container it
1238+
// executes: a failed hook aborts the chain before its remove node, so the
1239+
// container is retained for inspection and purged as stale on the next run.
1240+
// Returns the node(s) the first replica start must wait for.
1241+
func (r *reconciler) planPreStart(service types.ServiceConfig, first startReplica, prev []*PlanNode) []*PlanNode {
1242+
serviceCopy := service
1243+
group := startGroupID(first.resID)
1244+
// clone: appending into prev's backing array would silently mutate
1245+
// depNodes' hidden capacity (they share it)
1246+
deps := slices.Clone(prev)
1247+
if first.after != nil {
1248+
deps = append(deps, first.after)
1249+
}
1250+
1251+
// observed stale runners are purged in parallel; the first hook waits for
1252+
// all of them, like the imperative engine purges before running any hook
1253+
stale := slices.Clone(r.observed.HookContainers[service.Name])
1254+
slices.SortFunc(stale, func(a, b ObservedContainer) int { return cmp.Compare(a.ID, b.ID) })
1255+
purges := make([]*PlanNode, 0, len(stale))
1256+
for i := range stale {
1257+
purge := r.plan.addNode(Operation{
1258+
Type: OpRemoveContainer,
1259+
ResourceID: fmt.Sprintf("hook:%s:stale:%s", service.Name, stale[i].ID[:min(12, len(stale[i].ID))]),
1260+
Cause: "stale pre_start hook container",
1261+
Service: &serviceCopy,
1262+
Container: &stale[i].Summary,
1263+
BestEffort: true,
1264+
}, group, deps...)
1265+
purge.Phase = PhaseStart
1266+
purges = append(purges, purge)
1267+
}
1268+
deps = append(deps, purges...)
1269+
1270+
for i := range service.PreStart {
1271+
resID := fmt.Sprintf("hook:%s:pre_start:%d", service.Name, i)
1272+
create := r.plan.addNode(Operation{
1273+
Type: OpCreateHookContainer,
1274+
ResourceID: resID,
1275+
Cause: "pre_start hook",
1276+
Service: &serviceCopy,
1277+
HookIndex: i,
1278+
// the target replica whose volumes the hook shares (VolumesFrom):
1279+
// observed, or resolved from the create-phase node's result
1280+
Container: first.container,
1281+
CreateNodeID: first.createNodeID,
1282+
}, group, deps...)
1283+
create.Phase = PhaseStart
1284+
run := r.plan.addNode(Operation{
1285+
Type: OpRunPreStart,
1286+
ResourceID: resID,
1287+
Cause: "pre_start hook",
1288+
Service: &serviceCopy,
1289+
HookIndex: i,
1290+
// the container to execute is the one the create node materialized
1291+
CreateNodeID: create.ID,
1292+
}, group, create)
1293+
run.Phase = PhaseStart
1294+
remove := r.plan.addNode(Operation{
1295+
Type: OpRemoveContainer,
1296+
ResourceID: resID,
1297+
Cause: "hook completed",
1298+
Service: &serviceCopy,
1299+
CreateNodeID: create.ID,
1300+
// a removal failure does not gate service start: the hook already
1301+
// succeeded (imperative parity — warn-only removal)
1302+
BestEffort: true,
1303+
}, group, run)
1304+
remove.Phase = PhaseStart
1305+
// hooks run sequentially, in declared order: the next hook (and
1306+
// ultimately the first replica start) waits for this one's removal
1307+
deps = []*PlanNode{remove}
1308+
}
1309+
return deps
1310+
}
1311+
12501312
// planRecreateContainer decomposes container recreation into 4 atomic operations:
12511313
// CreateContainer(tmpName) → StopContainer → RemoveContainer → RenameContainer
12521314
func (r *reconciler) planRecreateContainer(service types.ServiceConfig, oc *ObservedContainer, infraDeps []*PlanNode) *PlanNode {

0 commit comments

Comments
 (0)