reconcile: document the asymmetric removeDeps in planRecreateContainer
Some checks are pending
ci / validate (lint) (push) Waiting to run
ci / validate (validate-docs) (push) Waiting to run
ci / validate (validate-go-mod) (push) Waiting to run
ci / validate (validate-headers) (push) Waiting to run
ci / binary (push) Waiting to run
ci / binary-finalize (push) Blocked by required conditions
ci / bin-image-test (push) Waiting to run
ci / test (push) Waiting to run
ci / e2e (plugin, oldstable) (push) Waiting to run
ci / e2e (standalone, oldstable) (push) Waiting to run
ci / e2e (plugin, stable) (push) Waiting to run
ci / e2e (standalone, stable) (push) Waiting to run
ci / coverage (push) Blocked by required conditions
ci / release (push) Blocked by required conditions
merge / bin-image-prepare (push) Waiting to run
merge / bin-image (push) Blocked by required conditions
merge / module-image (push) Waiting to run
merge / desktop-edge-test (push) Blocked by required conditions
Scorecards supply-chain security / Scorecards analysis (push) Waiting to run

External reviewer noted that the alreadyStopped branch adds createNode
to removeDeps while the !alreadyStopped branch does not — semantically
correct but fragile, since it relies on the implicit invariant that
stopNode.DependsOn contains createNode in the !alreadyStopped path.

Spell out the invariant in a comment so a future maintainer who edits
the stop → create edge in the normal path knows they must also add
createNode unconditionally in the remove deps.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
This commit is contained in:
Guillaume Lours 2026-05-28 17:07:42 +02:00 committed by Guillaume Lours
parent 750eb9b24e
commit 77ec74ee67

View file

@ -640,9 +640,16 @@ func (r *reconciler) planRecreateContainer(service types.ServiceConfig, oc *Obse
r.stoppedByPlan[oc.ID] = stopNode
}
// 3. Remove old container. Depend on the (existing or new) stop and on
// the create node so the new container is in place before the old one
// goes away.
// 3. Remove old container. The invariant is "new container exists before
// old one is removed":
// - !alreadyStopped: stopNode is the one we just created with
// DependsOn=[createNode], so the chain remove → stop → create
// guarantees the invariant transitively.
// - alreadyStopped: stopNode comes from planRecreateNetwork and was
// emitted before createNode. The transitive guarantee no longer
// holds, so we add createNode to removeDeps explicitly. If anyone
// ever drops the stop → create edge from the !alreadyStopped case,
// they must add createNode here unconditionally.
removeDeps := []*PlanNode{stopNode}
if alreadyStopped {
removeDeps = append(removeDeps, createNode)