From e4dc9f3db97de53a2e581726ef1df5967dacc780 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Mon, 20 Apr 2026 10:44:12 +0200 Subject: [PATCH] Fix code review issues: replace label, observed names for disconnect/remove - Fix ContainerReplaceLabel detection: use op.Inherited != nil (not op.Container) as signal for recreate in execCreateContainer - Use observed network name (not desired) for DisconnectNetwork and RemoveNetwork operations, in case the name changed - Use observed volume name (not desired) for RemoveVolume operations - Update reconciliation.md with 3 new lessons learned (7.8, 7.9, 7.10) Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Nicolas De Loof --- pkg/compose/executor.go | 2 +- pkg/compose/reconcile.go | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/compose/executor.go b/pkg/compose/executor.go index d0346c17c..c08d55696 100644 --- a/pkg/compose/executor.go +++ b/pkg/compose/executor.go @@ -207,7 +207,7 @@ func (exec *planExecutor) execCreateContainer(ctx context.Context, node *PlanNod } labels := mergeLabels(service.Labels, service.CustomLabels) - if op.Container != nil { + if op.Inherited != nil { // This is a recreate: add the replace label replacedName := op.Service.ContainerName if replacedName == "" { diff --git a/pkg/compose/reconcile.go b/pkg/compose/reconcile.go index e3392fda1..aab5c2b09 100644 --- a/pkg/compose/reconcile.go +++ b/pkg/compose/reconcile.go @@ -148,6 +148,7 @@ func (r *reconciler) planCreateNetwork(key string, nw *types.NetworkConfig) *Pla // planRecreateNetwork adds the full sequence for a diverged network: // stop affected containers → disconnect → remove network → create network. func (r *reconciler) planRecreateNetwork(key string, nw *types.NetworkConfig) error { + observed := r.observed.Networks[key] affectedServices := r.servicesUsingNetwork(key) affectedContainers := r.containersForServices(affectedServices) @@ -164,7 +165,7 @@ func (r *reconciler) planRecreateNetwork(key string, nw *types.NetworkConfig) er stopNodes = append(stopNodes, node) } - // Disconnect all affected containers (each depends on its own stop) + // Disconnect all affected containers from the *observed* network (each depends on its own stop) var disconnectNodes []*PlanNode for i, oc := range affectedContainers { node := r.plan.addNode(Operation{ @@ -172,18 +173,17 @@ func (r *reconciler) planRecreateNetwork(key string, nw *types.NetworkConfig) er ResourceID: fmt.Sprintf("service:%s:%d", oc.Summary.Labels[serviceLabel], oc.Number), Cause: fmt.Sprintf("network %s recreate", key), Container: &affectedContainers[i].Summary, - Name: nw.Name, + Name: observed.Name, }, "", stopNodes[i]) disconnectNodes = append(disconnectNodes, node) } - // Remove network (depends on all disconnects) + // Remove the *observed* network (depends on all disconnects) removeNode := r.plan.addNode(Operation{ Type: OpRemoveNetwork, ResourceID: fmt.Sprintf("network:%s", key), Cause: "config hash diverged", - Name: nw.Name, - Network: nw, + Name: observed.Name, }, "", disconnectNodes...) // Create network (depends on remove) @@ -251,6 +251,7 @@ func (r *reconciler) planCreateVolume(key string, vol *types.VolumeConfig) *Plan // Containers must be removed (not just stopped) because Docker does not allow // removing a volume that is referenced by any container, even a stopped one. func (r *reconciler) planRecreateVolume(key string, vol *types.VolumeConfig) { + observed := r.observed.Volumes[key] affectedServices := r.servicesUsingVolume(key) affectedContainers := r.containersForServices(affectedServices) @@ -279,13 +280,12 @@ func (r *reconciler) planRecreateVolume(key string, vol *types.VolumeConfig) { removeNodes = append(removeNodes, node) } - // Remove volume (depends on all container removals) + // Remove the *observed* volume (depends on all container removals) removeVolNode := r.plan.addNode(Operation{ Type: OpRemoveVolume, ResourceID: fmt.Sprintf("volume:%s", key), Cause: "config hash diverged", - Name: vol.Name, - Volume: vol, + Name: observed.Name, }, "", removeNodes...) // Create volume (depends on remove)