From fb7b1220f299a3205ab15e4084987cde33e15ccb Mon Sep 17 00:00:00 2001 From: Joana Hrotko Date: Mon, 21 Oct 2024 12:01:50 +0100 Subject: [PATCH 1/2] Recreate container on volume configuration change Signed-off-by: Joana Hrotko Signed-off-by: Nicolas De Loof --- pkg/compose/convergence.go | 74 ++++++++++++++----- pkg/compose/create.go | 38 ++++++---- pkg/compose/hash.go | 13 ++++ pkg/compose/run.go | 2 +- .../fixtures/recreate-volumes/compose.yaml | 10 +++ .../fixtures/recreate-volumes/compose2.yaml | 10 +++ pkg/e2e/volumes_test.go | 23 ++++++ 7 files changed, 138 insertions(+), 32 deletions(-) create mode 100644 pkg/e2e/fixtures/recreate-volumes/compose.yaml create mode 100644 pkg/e2e/fixtures/recreate-volumes/compose2.yaml diff --git a/pkg/compose/convergence.go b/pkg/compose/convergence.go index 11b4d6f1b38..3518a3ebc19 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,28 +344,63 @@ 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 + 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 } - 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 !found { - // config is up-to-date but container is not connected to network - maybe recreated ? - return true, nil + 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, nil + return false } func getContainerName(projectName string, service types.ServiceConfig, number int) string { diff --git a/pkg/compose/create.go b/pkg/compose/create.go index 0b8d92d071f..0228fecb46e 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, @@ -1426,21 +1429,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 @@ -1451,7 +1454,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 221f439b432..24c6cce2e88 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 0d15600b09c..947572cbfc2 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 00000000000..9da0dcba175 --- /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 00000000000..6d52097f925 --- /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 2c9d47c25a5..d975f9e8a8c 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"}) +} From 4e7b6dfc3d0f725bc561cfe88df68833b5863811 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Thu, 12 Dec 2024 09:36:24 +0100 Subject: [PATCH 2/2] prompt user to confirm volume recreation Signed-off-by: Nicolas De Loof --- cmd/compose/create.go | 3 + cmd/compose/up.go | 2 + docs/reference/compose_create.md | 1 + docs/reference/compose_up.md | 1 + docs/reference/docker_compose_create.yaml | 11 +++ docs/reference/docker_compose_up.yaml | 11 +++ pkg/api/api.go | 2 + pkg/compose/create.go | 86 +++++++++++++++++++---- 8 files changed, 104 insertions(+), 13 deletions(-) diff --git a/cmd/compose/create.go b/cmd/compose/create.go index 1fe05e98dc4..856b911fcff 100644 --- a/cmd/compose/create.go +++ b/cmd/compose/create.go @@ -46,6 +46,7 @@ type createOptions struct { timeout int quietPull bool scale []string + AssumeYes bool } func createCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Service) *cobra.Command { @@ -80,6 +81,7 @@ func createCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Service flags.BoolVar(&opts.noRecreate, "no-recreate", false, "If containers already exist, don't recreate them. Incompatible with --force-recreate.") flags.BoolVar(&opts.removeOrphans, "remove-orphans", false, "Remove containers for services not defined in the Compose file") flags.StringArrayVar(&opts.scale, "scale", []string{}, "Scale SERVICE to NUM instances. Overrides the `scale` setting in the Compose file if present.") + flags.BoolVarP(&opts.AssumeYes, "y", "y", false, `Assume "yes" as answer to all prompts and run non-interactively`) return cmd } @@ -107,6 +109,7 @@ func runCreate(ctx context.Context, _ command.Cli, backend api.Service, createOp Inherit: !createOpts.noInherit, Timeout: createOpts.GetTimeout(), QuietPull: createOpts.quietPull, + AssumeYes: createOpts.AssumeYes, }) } diff --git a/cmd/compose/up.go b/cmd/compose/up.go index 31756d6a846..e41a9549bdf 100644 --- a/cmd/compose/up.go +++ b/cmd/compose/up.go @@ -145,6 +145,7 @@ func upCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Service) *c flags := upCmd.Flags() flags.BoolVarP(&up.Detach, "detach", "d", false, "Detached mode: Run containers in the background") flags.BoolVar(&create.Build, "build", false, "Build images before starting containers") + flags.BoolVarP(&create.AssumeYes, "y", "y", false, `Assume "yes" as answer to all prompts and run non-interactively`) flags.BoolVar(&create.noBuild, "no-build", false, "Don't build an image, even if it's policy") flags.StringVar(&create.Pull, "pull", "policy", `Pull image before running ("always"|"missing"|"never")`) flags.BoolVar(&create.removeOrphans, "remove-orphans", false, "Remove containers for services not defined in the Compose file") @@ -255,6 +256,7 @@ func runUp( Inherit: !createOptions.noInherit, Timeout: createOptions.GetTimeout(), QuietPull: createOptions.quietPull, + AssumeYes: createOptions.AssumeYes, } if upOptions.noStart { diff --git a/docs/reference/compose_create.md b/docs/reference/compose_create.md index 7ae60c549da..b87cce8572b 100644 --- a/docs/reference/compose_create.md +++ b/docs/reference/compose_create.md @@ -16,6 +16,7 @@ Creates containers for a service | `--quiet-pull` | `bool` | | Pull without printing progress information | | `--remove-orphans` | `bool` | | Remove containers for services not defined in the Compose file | | `--scale` | `stringArray` | | Scale SERVICE to NUM instances. Overrides the `scale` setting in the Compose file if present. | +| `-y`, `--y` | `bool` | | Assume "yes" as answer to all prompts and run non-interactively | diff --git a/docs/reference/compose_up.md b/docs/reference/compose_up.md index 410ec84b52d..d1478cdd336 100644 --- a/docs/reference/compose_up.md +++ b/docs/reference/compose_up.md @@ -53,6 +53,7 @@ If the process is interrupted using `SIGINT` (ctrl + C) or `SIGTERM`, the contai | `--wait` | `bool` | | Wait for services to be running\|healthy. Implies detached mode. | | `--wait-timeout` | `int` | `0` | Maximum duration in seconds to wait for the project to be running\|healthy | | `-w`, `--watch` | `bool` | | Watch source code and rebuild/refresh containers when files are updated. | +| `-y`, `--y` | `bool` | | Assume "yes" as answer to all prompts and run non-interactively | diff --git a/docs/reference/docker_compose_create.yaml b/docs/reference/docker_compose_create.yaml index a07e1c88cc5..7cb764098b9 100644 --- a/docs/reference/docker_compose_create.yaml +++ b/docs/reference/docker_compose_create.yaml @@ -88,6 +88,17 @@ options: experimentalcli: false kubernetes: false swarm: false + - option: "y" + shorthand: "y" + value_type: bool + default_value: "false" + description: Assume "yes" as answer to all prompts and run non-interactively + deprecated: false + hidden: false + experimental: false + experimentalcli: false + kubernetes: false + swarm: false inherited_options: - option: dry-run value_type: bool diff --git a/docs/reference/docker_compose_up.yaml b/docs/reference/docker_compose_up.yaml index 1474cfe2ee6..0ed31b3599c 100644 --- a/docs/reference/docker_compose_up.yaml +++ b/docs/reference/docker_compose_up.yaml @@ -309,6 +309,17 @@ options: experimentalcli: false kubernetes: false swarm: false + - option: "y" + shorthand: "y" + value_type: bool + default_value: "false" + description: Assume "yes" as answer to all prompts and run non-interactively + deprecated: false + hidden: false + experimental: false + experimentalcli: false + kubernetes: false + swarm: false inherited_options: - option: dry-run value_type: bool diff --git a/pkg/api/api.go b/pkg/api/api.go index ccd56b74b6a..f0e813803d1 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -207,6 +207,8 @@ type CreateOptions struct { Timeout *time.Duration // QuietPull makes the pulling process quiet QuietPull bool + // AssumeYes assume "yes" as answer to all prompts and run non-interactively + AssumeYes bool } // StartOptions group options of the Start API diff --git a/pkg/compose/create.go b/pkg/compose/create.go index 0228fecb46e..858f8555f70 100644 --- a/pkg/compose/create.go +++ b/pkg/compose/create.go @@ -34,6 +34,7 @@ import ( pathutil "github.com/docker/compose/v2/internal/paths" "github.com/docker/compose/v2/pkg/api" "github.com/docker/compose/v2/pkg/progress" + "github.com/docker/compose/v2/pkg/prompt" "github.com/docker/compose/v2/pkg/utils" moby "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/blkiodev" @@ -92,7 +93,7 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt return err } - volumes, err := s.ensureProjectVolumes(ctx, project) + volumes, err := s.ensureProjectVolumes(ctx, project, options.AssumeYes) if err != nil { return err } @@ -142,13 +143,13 @@ 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) { +func (s *composeService) ensureProjectVolumes(ctx context.Context, project *types.Project, assumeYes bool) (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) - id, err := s.ensureVolume(ctx, volume, project.Name) + 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, assumeYes) if err != nil { return nil, err } @@ -1429,7 +1430,7 @@ func (s *composeService) resolveExternalNetwork(ctx context.Context, n *types.Ne } } -func (s *composeService) ensureVolume(ctx context.Context, volume types.VolumeConfig, project string) (string, error) { +func (s *composeService) ensureVolume(ctx context.Context, name string, volume types.VolumeConfig, project *types.Project, assumeYes bool) (string, error) { inspected, err := s.apiClient().VolumeInspect(ctx, volume.Name) if err != nil { if !errdefs.IsNotFound(err) { @@ -1439,7 +1440,7 @@ func (s *composeService) ensureVolume(ctx context.Context, volume types.VolumeCo return "", fmt.Errorf("external volume %q not found", volume.Name) } err = s.createVolume(ctx, volume) - return "", err + return volume.Name, err } if volume.External { @@ -1451,8 +1452,8 @@ func (s *composeService) ensureVolume(ctx context.Context, volume types.VolumeCo 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 { - 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) + 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) @@ -1461,17 +1462,76 @@ func (s *composeService) ensureVolume(ctx context.Context, volume types.VolumeCo } 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) + var confirm = assumeYes + if !assumeYes { + msg := fmt.Sprintf("Volume %q exists but doesn't match configuration in compose file. Recreate (data will be lost)?", volume.Name) + confirm, err = prompt.NewPrompt(s.stdin(), s.stdout()).Confirm(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.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, + }) + 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 + } + + return s.apiClient().VolumeRemove(ctx, volume.Name, true) +} + func (s *composeService) createVolume(ctx context.Context, volume types.VolumeConfig) error { eventName := fmt.Sprintf("Volume %q", volume.Name) w := progress.ContextWriter(ctx) w.Event(progress.CreatingEvent(eventName)) - _, err := s.apiClient().VolumeCreate(ctx, volumetypes.CreateOptions{ - Labels: volume.Labels, + hash, err := VolumeHash(volume) + if err != nil { + return err + } + volume.CustomLabels.Add(api.ConfigHashLabel, hash) + _, err = s.apiClient().VolumeCreate(ctx, volumetypes.CreateOptions{ + Labels: mergeLabels(volume.Labels, volume.CustomLabels), Name: volume.Name, Driver: volume.Driver, DriverOpts: volume.DriverOpts,