diff --git a/pkg/compose/convergence.go b/pkg/compose/convergence.go index 11b4d6f1b..3518a3ebc 100644 --- a/pkg/compose/convergence.go +++ b/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 - for net := range expected.Networks { - id := c.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 - maybe recreated ? - return true, nil - } + if checkExpectedNetworks(expected, actual, c.networks) { + return true, nil + } + } + + if c.volumes != nil { + if checkExpectedVolumes(expected, actual, c.volumes) { + 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 != "" { diff --git a/pkg/compose/create.go b/pkg/compose/create.go index 035492db6..02a436b43 100644 --- a/pkg/compose/create.go +++ b/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) - return err + err = s.createVolume(ctx, volume) + 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 { diff --git a/pkg/compose/hash.go b/pkg/compose/hash.go index 221f439b4..24c6cce2e 100644 --- a/pkg/compose/hash.go +++ b/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 +} diff --git a/pkg/compose/run.go b/pkg/compose/run.go index 0d15600b0..947572cbf 100644 --- a/pkg/compose/run.go +++ b/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 } diff --git a/pkg/e2e/fixtures/recreate-volumes/compose.yaml b/pkg/e2e/fixtures/recreate-volumes/compose.yaml new file mode 100644 index 000000000..9da0dcba1 --- /dev/null +++ b/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 diff --git a/pkg/e2e/fixtures/recreate-volumes/compose2.yaml b/pkg/e2e/fixtures/recreate-volumes/compose2.yaml new file mode 100644 index 000000000..6d52097f9 --- /dev/null +++ b/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 diff --git a/pkg/e2e/volumes_test.go b/pkg/e2e/volumes_test.go index 2c9d47c25..d975f9e8a 100644 --- a/pkg/e2e/volumes_test.go +++ b/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"}) +}