feat(reconcile): model volume recreation in the plan

AI_AGENT_DISCLOSURE.md

@@ -0,0 +1,2 @@
+This contribution was prepared by an AI agent acting on a human's behalf.
+The human submitter may not have independently reviewed or tested the change.

pkg/compose/create.go

@@ -92,7 +92,8 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt
 		return err
 	}
 
-	volumes, err := s.ensureProjectVolumes(ctx, project)
+	prepareVolumes(project)
+	externalVolumes, err := s.checkExternalVolumes(ctx, project)
 	if err != nil {
 		return err
 	}
@@ -108,7 +109,8 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt
 		return err
 	}
 	observed.setResolvedNetworks(networks, project)
-	observed.setResolvedVolumes(volumes)
+	observed.setResolvedVolumes(externalVolumes)
+	warnUnmanagedVolumes(project, observed)
 
 	if len(observed.Orphans) > 0 && !options.IgnoreOrphans && !options.RemoveOrphans {
 		logrus.Warnf("Found orphan containers (%s) for this project. If "+
@@ -152,20 +154,65 @@ func (s *composeService) ensureNetworks(ctx context.Context, project *types.Proj
 	return networks, nil
 }
 
-func (s *composeService) ensureProjectVolumes(ctx context.Context, project *types.Project) (map[string]string, error) {
-	ids := map[string]string{}
+// prepareVolumes injects the compose-managed labels onto every project volume so
+// that createVolume (executed later as a plan operation) persists them and the
+// volume can be matched back to the project on the next run. It mirrors
+// prepareNetworks and performs no I/O.
+func prepareVolumes(project *types.Project) {
 	for k, volume := range project.Volumes {
-		volume.CustomLabels = volume.CustomLabels.Add(api.VolumeLabel, k)
-		volume.CustomLabels = volume.CustomLabels.Add(api.ProjectLabel, project.Name)
-		volume.CustomLabels = volume.CustomLabels.Add(api.VersionLabel, api.ComposeVersion)
-		id, err := s.ensureVolume(ctx, k, volume, project)
-		if err != nil {
+		volume.CustomLabels = volume.CustomLabels.
+			Add(api.VolumeLabel, k).
+			Add(api.ProjectLabel, project.Name).
+			Add(api.VersionLabel, api.ComposeVersion)
+		project.Volumes[k] = volume
+	}
+}
+
+// checkExternalVolumes validates that every external volume exists and returns
+// their resolved names. External volumes carry no compose label and are
+// therefore absent from the label-scoped observed state, so the reconciler needs
+// them injected via setResolvedVolumes.
+//
+// Managed and legacy (unlabeled, name-matched) volumes are discovered by
+// collectObservedState; their lifecycle is owned by the reconciliation plan, so
+// this function performs no mutation on them.
+func (s *composeService) checkExternalVolumes(ctx context.Context, project *types.Project) (map[string]string, error) {
+	external := map[string]string{}
+	for k, volume := range project.Volumes {
+		if !volume.External {
+			continue
+		}
+		if _, err := s.apiClient().VolumeInspect(ctx, volume.Name, client.VolumeInspectOptions{}); err != nil {
+			if errdefs.IsNotFound(err) {
+				return nil, fmt.Errorf("external volume %q not found", volume.Name)
+			}
 			return nil, err
 		}
-		ids[k] = id
+		external[k] = volume.Name
 	}
+	return external, nil
+}
 
-	return ids, nil
+// warnUnmanagedVolumes warns about declared volumes backed by a live volume that
+// this project does not own — either created outside Compose (no project label)
+// or by another project. Such volumes are matched by name and reused untouched
+// (see collectObservedState); the warning tells the user to set `external: true`
+// to make the intent explicit.
+func warnUnmanagedVolumes(project *types.Project, observed *ObservedState) {
+	for k, volume := range project.Volumes {
+		if volume.External {
+			continue
+		}
+		obs, ok := observed.Volumes[k]
+		if !ok || obs.ProjectName == project.Name {
+			continue
+		}
+		if obs.ProjectName == "" {
+			logrus.Warnf("volume %q already exists but was not created by Docker Compose. Use `external: true` to use an existing volume", volume.Name)
+		} else {
+			logrus.Warnf("volume %q already exists but was created for project %q (expected %q). Use `external: true` to use an existing volume", volume.Name, obs.ProjectName, project.Name)
+		}
+	}
 }
 
 //nolint:gocyclo
@@ -1594,105 +1641,14 @@ func (s *composeService) resolveExternalNetwork(ctx context.Context, n *types.Ne
 	}
 }
 
-func (s *composeService) ensureVolume(ctx context.Context, name string, volume types.VolumeConfig, project *types.Project) (string, error) {
-	inspected, err := s.apiClient().VolumeInspect(ctx, volume.Name, client.VolumeInspectOptions{})
-	if err != nil {
-		if !errdefs.IsNotFound(err) {
-			return "", err
-		}
-		if volume.External {
-			return "", fmt.Errorf("external volume %q not found", volume.Name)
-		}
-		err = s.createVolume(ctx, volume)
-		return volume.Name, err
-	}
-
-	if volume.External {
-		return volume.Name, nil
-	}
-
-	// Volume exists with name, but let's double-check this is the expected one
-	p, ok := inspected.Volume.Labels[api.ProjectLabel]
-	if !ok {
-		logrus.Warnf("volume %q already exists but was not created by Docker Compose. Use `external: true` to use an existing volume", volume.Name)
-	}
-	if ok && p != project.Name {
-		logrus.Warnf("volume %q already exists but was created for project %q (expected %q). Use `external: true` to use an existing volume", volume.Name, p, project.Name)
-	}
-
-	expected, err := VolumeHash(volume)
-	if err != nil {
-		return "", err
-	}
-	actual, ok := inspected.Volume.Labels[api.ConfigHashLabel]
-	if ok && actual != expected {
-		msg := fmt.Sprintf("Volume %q exists but doesn't match configuration in compose file. Recreate (data will be lost)?", volume.Name)
-		confirm, err := s.prompt(msg, false)
-		if err != nil {
-			return "", err
-		}
-		if confirm {
-			err = s.removeDivergedVolume(ctx, name, volume, project)
-			if err != nil {
-				return "", err
-			}
-			return volume.Name, s.createVolume(ctx, volume)
-		}
-	}
-	return inspected.Volume.Name, nil
-}
-
-func (s *composeService) removeDivergedVolume(ctx context.Context, name string, volume types.VolumeConfig, project *types.Project) error {
-	// Remove services mounting divergent volume
-	var services []string
-	for _, service := range project.Services.Filter(func(config types.ServiceConfig) bool {
-		for _, cfg := range config.Volumes {
-			if cfg.Source == name {
-				return true
-			}
-		}
-		return false
-	}) {
-		services = append(services, service.Name)
-	}
-
-	err := s.stop(ctx, project.Name, api.StopOptions{
-		Services: services,
-		Project:  project,
-	}, nil)
-	if err != nil {
-		return err
-	}
-
-	containers, err := s.getContainers(ctx, project.Name, oneOffExclude, true, services...)
-	if err != nil {
-		return err
-	}
-
-	// FIXME (ndeloof) we have to remove container so we can recreate volume
-	// but doing so we can't inherit anonymous volumes from previous instance
-	err = s.remove(ctx, containers, api.RemoveOptions{
-		Services: services,
-		Project:  project,
-	})
-	if err != nil {
-		return err
-	}
-
-	_, err = s.apiClient().VolumeRemove(ctx, volume.Name, client.VolumeRemoveOptions{
-		Force: true,
-	})
-	return err
-}
-
 func (s *composeService) createVolume(ctx context.Context, volume types.VolumeConfig) error {
 	eventName := fmt.Sprintf("Volume %s", volume.Name)
 	s.events.On(creatingEvent(eventName))
 	hash, err := VolumeHash(volume)
 	if err != nil {
 		return err
 	}
-	volume.CustomLabels.Add(api.ConfigHashLabel, hash)
+	volume.CustomLabels = volume.CustomLabels.Add(api.ConfigHashLabel, hash)
 	_, err = s.apiClient().VolumeCreate(ctx, client.VolumeCreateOptions{
 		Labels:     mergeLabels(volume.Labels, volume.CustomLabels),
 		Name:       volume.Name,

pkg/compose/executor_test.go

@@ -253,6 +253,69 @@ func TestExecutePlanConcurrentRemovesCacheCoherence(t *testing.T) {
 		"all removed containers should be dropped from the live view")
 }
 
+// TestExecutePlanRecreateVolume drives the destructive core of a volume
+// recreation — stop container → remove container → remove volume → create
+// volume — end to end through the executor, asserting each Docker API call
+// fires. The dependency edges force the destructive order: the volume can only
+// be removed once the container referencing it is gone.
+func TestExecutePlanRecreateVolume(t *testing.T) {
+	svc, apiClient := newTestService(t)
+
+	ctr := container.Summary{
+		ID:    "c1",
+		Names: []string{"/test-db-1"},
+		Labels: map[string]string{
+			api.ServiceLabel:         "db",
+			api.ContainerNumberLabel: "1",
+		},
+	}
+
+	apiClient.EXPECT().ContainerStop(gomock.Any(), "c1", gomock.Any()).
+		Return(client.ContainerStopResult{}, nil)
+	apiClient.EXPECT().ContainerRemove(gomock.Any(), "c1", gomock.Any()).
+		Return(client.ContainerRemoveResult{}, nil)
+	apiClient.EXPECT().VolumeRemove(gomock.Any(), "recreate_data", gomock.Any()).
+		Return(client.VolumeRemoveResult{}, nil)
+	apiClient.EXPECT().VolumeCreate(gomock.Any(), gomock.Any()).
+		Return(client.VolumeCreateResult{}, nil)
+
+	vol := types.VolumeConfig{Name: "recreate_data", Driver: "local"}
+	project := &types.Project{
+		Name:    "recreate",
+		Volumes: types.Volumes{"data": vol},
+	}
+
+	plan := &Plan{}
+	stopNode := plan.addNode(Operation{
+		Type:       OpStopContainer,
+		ResourceID: "service:db:1",
+		Cause:      "mounted volume config changed",
+		Container:  &ctr,
+	}, "")
+	removeNode := plan.addNode(Operation{
+		Type:       OpRemoveContainer,
+		ResourceID: "service:db:1",
+		Cause:      "mounted volume config changed",
+		Container:  &ctr,
+	}, "", stopNode)
+	removeVolNode := plan.addNode(Operation{
+		Type:       OpRemoveVolume,
+		ResourceID: "volume:data",
+		Cause:      "config hash diverged",
+		Name:       vol.Name,
+	}, "", removeNode)
+	plan.addNode(Operation{
+		Type:       OpCreateVolume,
+		ResourceID: "volume:data",
+		Cause:      "recreate after config change",
+		Name:       vol.Name,
+		Volume:     &vol,
+	}, "", removeVolNode)
+
+	err := svc.executePlan(t.Context(), project, emptyObservedState("recreate"), plan)
+	assert.NilError(t, err)
+}
+
 // notFoundError implements the errdefs.ErrNotFound interface for test mocks.
 type notFoundError struct{}
 

pkg/compose/observed_state.go

@@ -22,6 +22,7 @@ import (
 	"strings"
 
 	"github.com/compose-spec/compose-go/v2/types"
+	"github.com/containerd/errdefs"
 	"github.com/moby/moby/api/types/container"
 	"github.com/moby/moby/client"
 
@@ -155,9 +156,46 @@ func (s *composeService) collectObservedState(ctx context.Context, project *type
 		}
 	}
 
+	if err := s.discoverUnmanagedVolumes(ctx, project, state); err != nil {
+		return nil, err
+	}
+
 	return state, nil
 }
 
+// discoverUnmanagedVolumes augments the observed state with volumes that match a
+// declared volume by name but carry no compose label — pre-label Compose or
+// manually created volumes, missed by the label-filtered VolumeList. Each is
+// recorded as an unmanaged match with an empty ConfigHash: the reconciler then
+// reuses it untouched instead of scheduling a (possibly failing) VolumeCreate.
+// See warnUnmanagedVolumes for the accompanying user warning.
+func (s *composeService) discoverUnmanagedVolumes(ctx context.Context, project *types.Project, state *ObservedState) error {
+	for _, key := range project.VolumeNames() {
+		vol := project.Volumes[key]
+		if vol.External {
+			continue
+		}
+		if _, ok := state.Volumes[key]; ok {
+			continue
+		}
+		inspected, err := s.apiClient().VolumeInspect(ctx, vol.Name, client.VolumeInspectOptions{})
+		if err != nil {
+			if errdefs.IsNotFound(err) {
+				continue // absent: it will be created by the reconciliation plan
+			}
+			return err
+		}
+		state.Volumes[key] = ObservedVolume{
+			Name:        inspected.Volume.Name,
+			ProjectName: inspected.Volume.Labels[api.ProjectLabel],
+			Driver:      inspected.Volume.Driver,
+			// ConfigHash intentionally left empty: the volume is not owned by
+			// this project, so we must not treat it as diverged and recreate it.
+		}
+	}
+	return nil
+}
+
 // toObservedContainer extracts the relevant fields from a container.Summary,
 // parsing labels into typed values.
 func toObservedContainer(c container.Summary) ObservedContainer {
@@ -197,8 +235,9 @@ func (s *ObservedState) setResolvedNetworks(networks map[string]string, project
 	}
 }
 
-// setResolvedVolumes injects volume names already resolved by ensureProjectVolumes
-// into the observed state.
+// setResolvedVolumes injects volume names already resolved by checkVolumes
+// (external volumes) into the observed state. Managed volumes are discovered
+// directly by collectObservedState, so only external ones need injecting.
 func (s *ObservedState) setResolvedVolumes(volumes map[string]string) {
 	for key, id := range volumes {
 		if obs, exists := s.Volumes[key]; exists {