Recreate container on volume configuration change

Signed-off-by: Joana Hrotko <joana.hrotko@docker.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

pkg/compose/convergence.go

@@ -35,6 +35,7 @@ import (
 	"github.com/docker/compose/v2/internal/tracing"
 	moby "github.com/docker/docker/api/types"
 	containerType "github.com/docker/docker/api/types/container"
+	mmount "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/api/types/versions"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/sirupsen/logrus"
@@ -60,6 +61,7 @@ type convergence struct {
 	service    *composeService
 	services   map[string]Containers
 	networks   map[string]string
+	volumes    map[string]string
 	stateMutex sync.Mutex
 }
 
@@ -75,7 +77,7 @@ func (c *convergence) setObservedState(serviceName string, containers Containers
 	c.services[serviceName] = containers
 }
 
-func newConvergence(services []string, state Containers, networks map[string]string, s *composeService) *convergence {
+func newConvergence(services []string, state Containers, networks map[string]string, volumes map[string]string, s *composeService) *convergence {
 	observedState := map[string]Containers{}
 	for _, s := range services {
 		observedState[s] = Containers{}
@@ -88,6 +90,7 @@ func newConvergence(services []string, state Containers, networks map[string]str
 		service:  s,
 		services: observedState,
 		networks: networks,
+		volumes:  volumes,
 	}
 }
 
@@ -341,30 +344,65 @@ func (c *convergence) mustRecreate(expected types.ServiceConfig, actual moby.Con
 	}
 
 	if c.networks != nil && actual.State == "running" {
-		// check the networks container is connected to are the expected ones
+		if checkExpectedNetworks(expected, actual, c.networks) {
-		for net := range expected.Networks {
+			return true, nil
-			id := c.networks[net]
+		}
-			if id == "swarm" {
+	}
-				// corner-case : swarm overlay network isn't visible until a container is attached
+
-				continue
+	if c.volumes != nil {
-			}
+		if checkExpectedVolumes(expected, actual, c.volumes) {
-			found := false
+			return true, nil
-			for _, settings := range actual.NetworkSettings.Networks {
-				if settings.NetworkID == id {
-					found = true
-					break
-				}
-			}
-			if !found {
-				// config is up-to-date but container is not connected to network - maybe recreated ?
-				return true, nil
-			}
 		}
 	}
 
 	return false, nil
 }
 
+func checkExpectedNetworks(expected types.ServiceConfig, actual moby.Container, networks map[string]string) bool {
+	// check the networks container is connected to are the expected ones
+	for net := range expected.Networks {
+		id := networks[net]
+		if id == "swarm" {
+			// corner-case : swarm overlay network isn't visible until a container is attached
+			continue
+		}
+		found := false
+		for _, settings := range actual.NetworkSettings.Networks {
+			if settings.NetworkID == id {
+				found = true
+				break
+			}
+		}
+		if !found {
+			// config is up-to-date but container is not connected to network
+			return true
+		}
+	}
+	return false
+}
+
+func checkExpectedVolumes(expected types.ServiceConfig, actual moby.Container, volumes map[string]string) bool {
+	// check container's volume mounts and search for the expected ones
+	for _, vol := range expected.Volumes {
+		id := volumes[vol.Source]
+		found := false
+		for _, mount := range actual.Mounts {
+			if mount.Type != mmount.TypeVolume {
+				continue
+			}
+			if mount.Name == id {
+				found = true
+				break
+			}
+		}
+		if !found {
+			// config is up-to-date but container doesn't have volume mounted
+			return true
+		}
+	}
+	return false
+}
+
 func getContainerName(projectName string, service types.ServiceConfig, number int) string {
 	name := getDefaultContainerName(projectName, service.Name, strconv.Itoa(number))
 	if service.ContainerName != "" {

pkg/compose/create.go

@@ -92,7 +92,8 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt
 		return err
 	}
 
-	if err := s.ensureProjectVolumes(ctx, project); err != nil {
+	volumes, err := s.ensureProjectVolumes(ctx, project)
+	if err != nil {
 		return err
 	}
 
@@ -115,7 +116,7 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt
 				"--remove-orphans flag to clean it up.", orphans.names())
 		}
 	}
-	return newConvergence(options.Services, observedState, networks, s).apply(ctx, project, options)
+	return newConvergence(options.Services, observedState, networks, volumes, s).apply(ctx, project, options)
 }
 
 func prepareNetworks(project *types.Project) {
@@ -141,15 +142,17 @@ func (s *composeService) ensureNetworks(ctx context.Context, project *types.Proj
 	return networks, nil
 }
 
-func (s *composeService) ensureProjectVolumes(ctx context.Context, project *types.Project) error {
+func (s *composeService) ensureProjectVolumes(ctx context.Context, project *types.Project) (map[string]string, error) {
+	ids := map[string]string{}
 	for k, volume := range project.Volumes {
 		volume.Labels = volume.Labels.Add(api.VolumeLabel, k)
 		volume.Labels = volume.Labels.Add(api.ProjectLabel, project.Name)
 		volume.Labels = volume.Labels.Add(api.VersionLabel, api.ComposeVersion)
-		err := s.ensureVolume(ctx, volume, project.Name)
+		id, err := s.ensureVolume(ctx, volume, project.Name)
 		if err != nil {
-			return err
+			return nil, err
 		}
+		ids[k] = id
 	}
 
 	err := func() error {
@@ -205,7 +208,7 @@ func (s *composeService) ensureProjectVolumes(ctx context.Context, project *type
 	if err != nil {
 		progress.ContextWriter(ctx).TailMsgf("Failed to prepare Synchronized file shares: %v", err)
 	}
-	return nil
+	return ids, nil
 }
 
 func (s *composeService) getCreateConfigs(ctx context.Context,
@@ -1431,21 +1434,21 @@ func (s *composeService) resolveExternalNetwork(ctx context.Context, n *types.Ne
 	}
 }
 
-func (s *composeService) ensureVolume(ctx context.Context, volume types.VolumeConfig, project string) error {
+func (s *composeService) ensureVolume(ctx context.Context, volume types.VolumeConfig, project string) (string, error) {
 	inspected, err := s.apiClient().VolumeInspect(ctx, volume.Name)
 	if err != nil {
 		if !errdefs.IsNotFound(err) {
-			return err
+			return "", err
 		}
 		if volume.External {
-			return fmt.Errorf("external volume %q not found", volume.Name)
+			return "", fmt.Errorf("external volume %q not found", volume.Name)
 		}
-		err := s.createVolume(ctx, volume)
+		err = s.createVolume(ctx, volume)
-		return err
+		return "", err
 	}
 
 	if volume.External {
-		return nil
+		return volume.Name, nil
 	}
 
 	// Volume exists with name, but let's double-check this is the expected one
@@ -1456,7 +1459,16 @@ func (s *composeService) ensureVolume(ctx context.Context, volume types.VolumeCo
 	if ok && p != project {
 		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)
 	}
-	return nil
+
+	expected, err := VolumeHash(volume)
+	if err != nil {
+		return "", err
+	}
+	actual, ok := inspected.Labels[api.ConfigHashLabel]
+	if ok && actual != expected {
+		logrus.Warnf("volume %q exists but doesn't match configuration in compose file. You should remove it so it get recreated", volume.Name)
+	}
+	return inspected.Name, nil
 }
 
 func (s *composeService) createVolume(ctx context.Context, volume types.VolumeConfig) error {

pkg/compose/hash.go

@@ -42,6 +42,7 @@ func ServiceHash(o types.ServiceConfig) (string, error) {
 	return digest.SHA256.FromBytes(bytes).Encoded(), nil
 }
 
+// NetworkHash computes the configuration hash for a network.
 func NetworkHash(o *types.NetworkConfig) (string, error) {
 	bytes, err := json.Marshal(o)
 	if err != nil {
@@ -49,3 +50,15 @@ func NetworkHash(o *types.NetworkConfig) (string, error) {
 	}
 	return digest.SHA256.FromBytes(bytes).Encoded(), nil
 }
+
+// VolumeHash computes the configuration hash for a volume.
+func VolumeHash(o types.VolumeConfig) (string, error) {
+	if o.Driver == "" { // (TODO: jhrotko) This probably should be fixed in compose-go
+		o.Driver = "local"
+	}
+	bytes, err := json.Marshal(o)
+	if err != nil {
+		return "", err
+	}
+	return digest.SHA256.FromBytes(bytes).Encoded(), nil
+}

pkg/compose/run.go

@@ -104,7 +104,7 @@ func (s *composeService) prepareRun(ctx context.Context, project *types.Project,
 		Labels:            mergeLabels(service.Labels, service.CustomLabels),
 	}
 
-	err = newConvergence(project.ServiceNames(), observedState, nil, s).resolveServiceReferences(&service)
+	err = newConvergence(project.ServiceNames(), observedState, nil, nil, s).resolveServiceReferences(&service)
 	if err != nil {
 		return "", err
 	}

pkg/e2e/fixtures/recreate-volumes/compose.yaml

@@ -0,0 +1,10 @@
+services:
+  app:
+    image: alpine
+    volumes:
+      - my_vol:/my_vol
+
+volumes:
+  my_vol:
+    external: true
+    name: test_external_volume

pkg/e2e/fixtures/recreate-volumes/compose2.yaml

@@ -0,0 +1,10 @@
+services:
+  app:
+    image: alpine
+    volumes:
+      - my_vol:/my_vol
+
+volumes:
+  my_vol:
+    external: true
+    name: test_external_volume_2

pkg/e2e/volumes_test.go

@@ -17,6 +17,7 @@
 package e2e
 
 import (
+	"fmt"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -121,3 +122,25 @@ func TestProjectVolumeBind(t *testing.T) {
 		assert.Assert(t, strings.Contains(ret.Stdout(), "SUCCESS"))
 	})
 }
+
+func TestUpRecreateVolumes(t *testing.T) {
+	c := NewCLI(t)
+	const projectName = "compose-e2e-recreate-volumes"
+	t.Cleanup(func() {
+		c.cleanupWithDown(t, projectName)
+		c.RunDockerCmd(t, "volume", "rm", "-f", "test_external_volume")
+		c.RunDockerCmd(t, "volume", "rm", "-f", "test_external_volume_2")
+	})
+
+	c.RunDockerCmd(t, "volume", "create", "test_external_volume")
+	c.RunDockerCmd(t, "volume", "create", "test_external_volume_2")
+
+	c.RunDockerComposeCmd(t, "-f", "./fixtures/recreate-volumes/compose.yaml", "--project-name", projectName, "up", "-d")
+
+	res := c.RunDockerCmd(t, "inspect", fmt.Sprintf("%s-app-1", projectName), "-f", "{{ (index .Mounts 0).Name }}")
+	res.Assert(t, icmd.Expected{Out: "test_external_volume"})
+
+	c.RunDockerComposeCmd(t, "-f", "./fixtures/recreate-volumes/compose2.yaml", "--project-name", projectName, "up", "-d")
+	res = c.RunDockerCmd(t, "inspect", fmt.Sprintf("%s-app-1", projectName), "-f", "{{ (index .Mounts 0).Name }}")
+	res.Assert(t, icmd.Expected{Out: "test_external_volume_2"})
+}