From 31abe5f46d06ca4e88471bc112ae855a0f78d891 Mon Sep 17 00:00:00 2001 From: Guillaume Lours Date: Thu, 28 May 2026 11:02:25 +0200 Subject: [PATCH] reconcile: stop duplicating volume divergence prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ensureProjectVolumes already prompts the user when a volume's config hash diverges from the compose file (create.go:1626) and recreates it on confirm. The reconciler ran after ensureProjectVolumes and prompted again with the exact same message — so a user who declined the first prompt was asked the same question a second time. Drop the prompt + recreate call from reconcileVolumes(). Recreation of diverged volumes stays owned by ensureProjectVolumes; the reconciler only plans the creation of missing volumes. If the user declined recreation, the existing container's mounts still match the existing volume name and hasVolumeMismatch correctly returns false, so containers are not falsely flagged as obsolete. Keep the supporting infrastructure available for future use, when divergence detection migrates fully into the reconciler: - reconciler.prompt field - prompt parameter on reconcile() - planRecreateVolume function (//nolint:unused) - servicesUsingVolume function (//nolint:unused) - noPrompt test helper The reframed test (TestReconcileVolumes_DivergedIsIgnored) asserts the new contract: a diverged volume produces no plan operations from the reconciler. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Guillaume Lours --- pkg/compose/reconcile.go | 55 +++++++++++++++----------------- pkg/compose/reconcile_test.go | 60 ++++------------------------------- 2 files changed, 31 insertions(+), 84 deletions(-) diff --git a/pkg/compose/reconcile.go b/pkg/compose/reconcile.go index d7eb40b34..2ea603cba 100644 --- a/pkg/compose/reconcile.go +++ b/pkg/compose/reconcile.go @@ -60,8 +60,12 @@ type reconciler struct { project *types.Project observed *ObservedState options ReconcileOptions - prompt Prompt - plan *Plan + // prompt is wired through for future use: when divergence detection for + // volumes/networks migrates fully into the reconciler (today it lives in + // ensureProjectVolumes/ensureNetworks), prompts will fire from here. Kept + // available now so call sites do not have to change later. + prompt Prompt + plan *Plan // networkNodes and volumeNodes track the last plan node for each // network/volume, so container creation nodes can depend on them. @@ -73,7 +77,8 @@ type reconciler struct { } // reconcile is the main entry point: it builds a Plan from desired vs observed state. -// The prompt function is called for interactive decisions (e.g. volume divergence). +// The prompt function is reserved for future interactive decisions (see the +// reconciler.prompt field). func reconcile(_ context.Context, project *types.Project, observed *ObservedState, options ReconcileOptions, prompt Prompt) (*Plan, error) { r := &reconciler{ project: project, @@ -90,9 +95,7 @@ func reconcile(_ context.Context, project *types.Project, observed *ObservedStat return nil, err } - if err := r.reconcileVolumes(); err != nil { - return nil, err - } + r.reconcileVolumes() if err := r.reconcileContainers(); err != nil { return nil, err @@ -199,38 +202,20 @@ func (r *reconciler) planRecreateNetwork(key string, nw *types.NetworkConfig) er return nil } -// reconcileVolumes adds plan nodes for volume creation or recreation. -func (r *reconciler) reconcileVolumes() error { +// reconcileVolumes adds plan nodes for volume creation. Recreation of a +// diverged volume is handled by ensureProjectVolumes (which already prompts +// the user) before reconcile runs, so the reconciler does not duplicate that +// decision here. +func (r *reconciler) reconcileVolumes() { for _, key := range sortedKeys(r.project.Volumes) { desired := r.project.Volumes[key] if desired.External { continue } - observed, exists := r.observed.Volumes[key] - if !exists { + if _, exists := r.observed.Volumes[key]; !exists { r.planCreateVolume(key, &desired) - continue } - - expectedHash, err := VolumeHash(desired) - if err != nil { - return err - } - if observed.ConfigHash != "" && observed.ConfigHash != expectedHash { - confirmed, err := r.prompt( - fmt.Sprintf("Volume %q exists but doesn't match configuration in compose file. Recreate (data will be lost)?", desired.Name), - false, - ) - if err != nil { - return err - } - if confirmed { - r.planRecreateVolume(key, &desired) - } - } - // else: volume exists and config matches, nothing to do } - return nil } // planCreateVolume adds a single CreateVolume node and records it for dependency tracking. @@ -250,6 +235,12 @@ func (r *reconciler) planCreateVolume(key string, vol *types.VolumeConfig) *Plan // stop affected containers → remove containers → remove volume → create volume. // 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. +// +// Currently unused: divergence detection and recreation live in +// ensureProjectVolumes (see create.go:1626). Kept in place so the reconciler +// can take over that responsibility when the seam is consolidated. +// +//nolint:unused func (r *reconciler) planRecreateVolume(key string, vol *types.VolumeConfig) { observed := r.observed.Volumes[key] affectedServices := r.servicesUsingVolume(key) @@ -314,6 +305,10 @@ func (r *reconciler) servicesUsingNetwork(networkKey string) []string { // servicesUsingVolume returns the names of services that mount the given // compose volume key, sorted for deterministic plan output. +// +// Currently used only by planRecreateVolume (also unused — see its doc). +// +//nolint:unused func (r *reconciler) servicesUsingVolume(volumeKey string) []string { var names []string for _, key := range sortedKeys(r.project.Services) { diff --git a/pkg/compose/reconcile_test.go b/pkg/compose/reconcile_test.go index ff0c8516a..c7379322d 100644 --- a/pkg/compose/reconcile_test.go +++ b/pkg/compose/reconcile_test.go @@ -32,9 +32,6 @@ func noPrompt(msg string, _ bool) (bool, error) { panic("unexpected prompt call: " + msg) } -func alwaysYesPrompt(string, bool) (bool, error) { return true, nil } -func alwaysNoPrompt(string, bool) (bool, error) { return false, nil } - func defaultReconcileOptions() ReconcileOptions { return ReconcileOptions{ Recreate: api.RecreateDiverged, @@ -282,56 +279,11 @@ func TestReconcileVolumes_ExternalSkipped(t *testing.T) { assert.Assert(t, plan.IsEmpty()) } -func TestReconcileVolumes_DivergedConfirmed(t *testing.T) { - project := &types.Project{ - Name: "myproject", - Volumes: types.Volumes{"data": {Name: "myproject_data", Driver: "local"}}, - Services: types.Services{ - "db": { - Name: "db", - Scale: intPtr(1), - Volumes: []types.ServiceVolumeConfig{ - {Source: "data", Type: "volume"}, - }, - }, - }, - } - observed := &ObservedState{ - ProjectName: "myproject", - Containers: map[string][]ObservedContainer{ - "db": {{ - ID: "c1aabbccddee", Number: 1, State: container.StateRunning, - Summary: container.Summary{ - ID: "c1aabbccddee", - Labels: map[string]string{ - api.ServiceLabel: "db", - api.ContainerNumberLabel: "1", - }, - }, - }}, - }, - Networks: map[string]ObservedNetwork{}, - Volumes: map[string]ObservedVolume{ - "data": {Name: "myproject_data", ConfigHash: "oldhash"}, - }, - } - - plan, err := reconcile(t.Context(), project, observed, defaultReconcileOptions(), alwaysYesPrompt) - assert.NilError(t, err) - - assert.Equal(t, plan.String(), strings.TrimSpace(` -[] -> #1 service:db:1, StopContainer, volume data config changed -[1] -> #2 service:db:1, RemoveContainer, volume data config changed -[2] -> #3 volume:data, RemoveVolume, config hash diverged -[3] -> #4 volume:data, CreateVolume, recreate after config change -[4] -> #5 service:db:1, CreateContainer, config changed (tmpName) [recreate:db:1] -[5] -> #6 service:db:1, StopContainer, replaced by #5 [recreate:db:1] -[6] -> #7 service:db:1, RemoveContainer, replaced by #5 [recreate:db:1] -[7] -> #8 service:db:1, RenameContainer, finalize recreate [recreate:db:1] -`)+"\n") -} - -func TestReconcileVolumes_DivergedDeclined(t *testing.T) { +// TestReconcileVolumes_DivergedIsIgnored verifies that a diverged volume +// produces no plan operations: recreation of diverged volumes is owned by +// ensureProjectVolumes (which prompts the user) and runs before reconcile, +// so the reconciler must not duplicate that decision. +func TestReconcileVolumes_DivergedIsIgnored(t *testing.T) { vol := types.VolumeConfig{Name: "myproject_data", Driver: "local"} project := &types.Project{ @@ -371,7 +323,7 @@ func TestReconcileVolumes_DivergedDeclined(t *testing.T) { }, } - plan, err := reconcile(t.Context(), project, observed, defaultReconcileOptions(), alwaysNoPrompt) + plan, err := reconcile(t.Context(), project, observed, defaultReconcileOptions(), noPrompt) assert.NilError(t, err) assert.Assert(t, plan.IsEmpty()) }