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) <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
This commit is contained in:
Nicolas De Loof 2026-04-20 10:44:12 +02:00 committed by Guillaume Lours
parent fbea647b9d
commit e4dc9f3db9
2 changed files with 9 additions and 9 deletions

View file

@ -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 == "" {

View file

@ -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)