mirror of
https://github.com/docker/compose.git
synced 2026-06-28 04:03:48 +00:00
reconcile: stop duplicating volume divergence prompt
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) <noreply@anthropic.com> Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
This commit is contained in:
parent
f41d5c10a6
commit
31abe5f46d
2 changed files with 31 additions and 84 deletions
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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())
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue