From 0c37c10964b1ec79d1fc80d0aa4dd2215eec5775 Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Tue, 8 Apr 2025 19:44:26 +0200 Subject: [PATCH] mount API is not strictly equivalent to bind Signed-off-by: Nicolas De Loof --- pkg/compose/build.go | 3 +- pkg/compose/create.go | 78 +++++++++++++++++++++++++++++++--------- pkg/compose/down.go | 9 ++++- pkg/compose/down_test.go | 2 ++ 4 files changed, 72 insertions(+), 20 deletions(-) diff --git a/pkg/compose/build.go b/pkg/compose/build.go index 1a2c3f94d..a32fe6528 100644 --- a/pkg/compose/build.go +++ b/pkg/compose/build.go @@ -320,8 +320,7 @@ func (s *composeService) ensureImagesExists(ctx context.Context, project *types. func (s *composeService) getLocalImagesDigests(ctx context.Context, project *types.Project) (map[string]api.ImageSummary, error) { imageNames := utils.Set[string]{} for _, s := range project.Services { - imgName := api.GetImageNameOrDefault(s, project.Name) - imageNames.Add(imgName) + imageNames.Add(api.GetImageNameOrDefault(s, project.Name)) for _, volume := range s.Volumes { if volume.Type == types.VolumeTypeImage { imageNames.Add(volume.Source) diff --git a/pkg/compose/create.go b/pkg/compose/create.go index 8e7729b38..999b50f43 100644 --- a/pkg/compose/create.go +++ b/pkg/compose/create.go @@ -828,6 +828,7 @@ func getDependentServiceFromMode(mode string) string { return "" } +//nolint:gocyclo func (s *composeService) buildContainerVolumes( ctx context.Context, p types.Project, @@ -848,30 +849,37 @@ func (s *composeService) buildContainerVolumes( return nil, nil, err } -MOUNTS: for _, m := range mountOptions { - if m.Type == mount.TypeNamedPipe { - mounts = append(mounts, m) - continue - } - if m.Type == mount.TypeBind { + switch m.Type { + case mount.TypeBind: // `Mount` is preferred but does not offer option to created host path if missing // so `Bind` API is used here with raw volume string // see https://github.com/moby/moby/issues/43483 - for _, v := range service.Volumes { - if v.Target == m.Target { - switch { - case string(m.Type) != v.Type: - v.Source = m.Source - fallthrough - case !requireMountAPI(v.Bind): - binds = append(binds, v.String()) - continue MOUNTS + v := findVolumeByTarget(service.Volumes, m.Target) + if v != nil { + switch { + case v.Type != types.VolumeTypeBind: + v.Source = m.Source + fallthrough + case !requireMountAPI(v.Bind): + vol := findVolumeByName(p.Volumes, m.Source) + if vol != nil { + binds = append(binds, toBindString(vol.Name, v)) + continue } } } - } - if m.Type == mount.TypeImage { + case mount.TypeVolume: + v := findVolumeByTarget(service.Volumes, m.Target) + vol := findVolumeByName(p.Volumes, m.Source) + if v != nil && vol != nil { + if _, ok := vol.DriverOpts["device"]; ok && vol.Driver == "local" && vol.DriverOpts["o"] == "bind" { + // Looks like a volume, but actually a bind mount which requires the bind API + binds = append(binds, toBindString(vol.Name, v)) + continue + } + } + case mount.TypeImage: version, err := s.RuntimeVersion(ctx) if err != nil { return nil, nil, err @@ -885,6 +893,42 @@ MOUNTS: return binds, mounts, nil } +func toBindString(name string, v *types.ServiceVolumeConfig) string { + access := "rw" + if v.ReadOnly { + access = "ro" + } + options := []string{access} + if v.Bind != nil && v.Bind.SELinux != "" { + options = append(options, v.Bind.SELinux) + } + if v.Bind != nil && v.Bind.Propagation != "" { + options = append(options, v.Bind.Propagation) + } + if v.Volume != nil && v.Volume.NoCopy { + options = append(options, "nocopy") + } + return fmt.Sprintf("%s:%s:%s", name, v.Target, strings.Join(options, ",")) +} + +func findVolumeByName(volumes types.Volumes, name string) *types.VolumeConfig { + for _, vol := range volumes { + if vol.Name == name { + return &vol + } + } + return nil +} + +func findVolumeByTarget(volumes []types.ServiceVolumeConfig, target string) *types.ServiceVolumeConfig { + for _, v := range volumes { + if v.Target == target { + return &v + } + } + return nil +} + // requireMountAPI check if Bind declaration can be implemented by the plain old Bind API or uses any of the advanced // options which require use of Mount API func requireMountAPI(bind *types.ServiceVolumeBind) bool { diff --git a/pkg/compose/down.go b/pkg/compose/down.go index 00580a50d..07b7e5840 100644 --- a/pkg/compose/down.go +++ b/pkg/compose/down.go @@ -282,8 +282,15 @@ func (s *composeService) removeImage(ctx context.Context, image string, w progre func (s *composeService) removeVolume(ctx context.Context, id string, w progress.Writer) error { resource := fmt.Sprintf("Volume %s", id) + + _, err := s.apiClient().VolumeInspect(ctx, id) + if errdefs.IsNotFound(err) { + // Already gone + return nil + } + w.Event(progress.NewEvent(resource, progress.Working, "Removing")) - err := s.apiClient().VolumeRemove(ctx, id, true) + err = s.apiClient().VolumeRemove(ctx, id, true) if err == nil { w.Event(progress.NewEvent(resource, progress.Done, "Removed")) return nil diff --git a/pkg/compose/down_test.go b/pkg/compose/down_test.go index ac147d653..faeb0a81d 100644 --- a/pkg/compose/down_test.go +++ b/pkg/compose/down_test.go @@ -254,6 +254,8 @@ func TestDownRemoveVolumes(t *testing.T) { Return(volume.ListResponse{ Volumes: []*volume.Volume{{Name: "myProject_volume"}}, }, nil) + api.EXPECT().VolumeInspect(gomock.Any(), "myProject_volume"). + Return(volume.Volume{}, nil) api.EXPECT().NetworkList(gomock.Any(), network.ListOptions{Filters: filters.NewArgs(projectFilter(strings.ToLower(testProject)))}). Return(nil, nil)