Browse Source

mount API is not strictly equivalent to bind

Signed-off-by: Nicolas De Loof <[email protected]>
Nicolas De Loof 9 months ago
parent
commit
0c37c10964
4 changed files with 72 additions and 20 deletions
  1. 1 2
      pkg/compose/build.go
  2. 61 17
      pkg/compose/create.go
  3. 8 1
      pkg/compose/down.go
  4. 2 0
      pkg/compose/down_test.go

+ 1 - 2
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)

+ 61 - 17
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 {

+ 8 - 1
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

+ 2 - 0
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)