diff --git a/pkg/compose/create.go b/pkg/compose/create.go index 1f979f5b2..5c9d13436 100644 --- a/pkg/compose/create.go +++ b/pkg/compose/create.go @@ -39,7 +39,6 @@ import ( "github.com/docker/docker/api/types/blkiodev" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/filters" - "github.com/docker/docker/api/types/image" "github.com/docker/docker/api/types/mount" "github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/strslice" @@ -828,7 +827,6 @@ func getDependentServiceFromMode(mode string) string { return "" } -//nolint:gocyclo func (s *composeService) buildContainerVolumes( ctx context.Context, p types.Project, @@ -838,13 +836,7 @@ func (s *composeService) buildContainerVolumes( var mounts []mount.Mount var binds []string - img := api.GetImageNameOrDefault(service, p.Name) - imgInspect, err := s.apiClient().ImageInspect(ctx, img) - if err != nil { - return nil, nil, err - } - - mountOptions, err := buildContainerMountOptions(p, service, imgInspect, inherit) + mountOptions, err := s.buildContainerMountOptions(ctx, p, service, inherit) if err != nil { return nil, nil, err } @@ -857,11 +849,10 @@ func (s *composeService) buildContainerVolumes( // see https://github.com/moby/moby/issues/43483 v := findVolumeByTarget(service.Volumes, m.Target) if v != nil { - switch { - case v.Type != types.VolumeTypeBind: + if v.Type != types.VolumeTypeBind { v.Source = m.Source - fallthrough - case !requireMountAPI(v.Bind): + } + if !bindRequiresMountAPI(v.Bind) { source := m.Source if vol := findVolumeByName(p.Volumes, m.Source); vol != nil { source = m.Source @@ -874,8 +865,8 @@ func (s *composeService) buildContainerVolumes( 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 + // Prefer the bind API if no advanced option is used, to preserve backward compatibility + if !volumeRequiresMountAPI(v.Volume) { binds = append(binds, toBindString(vol.Name, v)) continue } @@ -930,9 +921,9 @@ func findVolumeByTarget(volumes []types.ServiceVolumeConfig, target string) *typ return nil } -// requireMountAPI check if Bind declaration can be implemented by the plain old Bind API or uses any of the advanced +// bindRequiresMountAPI 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 { +func bindRequiresMountAPI(bind *types.ServiceVolumeBind) bool { switch { case bind == nil: return false @@ -947,7 +938,24 @@ func requireMountAPI(bind *types.ServiceVolumeBind) bool { } } -func buildContainerMountOptions(p types.Project, s types.ServiceConfig, img image.InspectResponse, inherit *container.Summary) ([]mount.Mount, error) { +// volumeRequiresMountAPI check if Volume declaration can be implemented by the plain old Bind API or uses any of the advanced +// options which require use of Mount API +func volumeRequiresMountAPI(vol *types.ServiceVolumeVolume) bool { + switch { + case vol == nil: + return false + case len(vol.Labels) > 0: + return true + case vol.Subpath != "": + return true + case vol.NoCopy: + return true + default: + return false + } +} + +func (s *composeService) buildContainerMountOptions(ctx context.Context, p types.Project, service types.ServiceConfig, inherit *container.Summary) ([]mount.Mount, error) { mounts := map[string]mount.Mount{} if inherit != nil { for _, m := range inherit.Mounts { @@ -959,6 +967,11 @@ func buildContainerMountOptions(p types.Project, s types.ServiceConfig, img imag src = m.Name } + img, err := s.apiClient().ImageInspect(ctx, api.GetImageNameOrDefault(service, p.Name)) + if err != nil { + return nil, err + } + if img.Config != nil { if _, ok := img.Config.Volumes[m.Destination]; ok { // inherit previous container's anonymous volume @@ -971,7 +984,7 @@ func buildContainerMountOptions(p types.Project, s types.ServiceConfig, img imag } } volumes := []types.ServiceVolumeConfig{} - for _, v := range s.Volumes { + for _, v := range service.Volumes { if v.Target != m.Destination || v.Source != "" { volumes = append(volumes, v) continue @@ -984,11 +997,11 @@ func buildContainerMountOptions(p types.Project, s types.ServiceConfig, img imag ReadOnly: !m.RW, } } - s.Volumes = volumes + service.Volumes = volumes } } - mounts, err := fillBindMounts(p, s, mounts) + mounts, err := fillBindMounts(p, service, mounts) if err != nil { return nil, err } diff --git a/pkg/compose/create_test.go b/pkg/compose/create_test.go index ae7eb12d7..bec42beef 100644 --- a/pkg/compose/create_test.go +++ b/pkg/compose/create_test.go @@ -17,13 +17,16 @@ package compose import ( + "context" "os" "path/filepath" "sort" "testing" + composeloader "github.com/compose-spec/compose-go/v2/loader" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/image" + "go.uber.org/mock/gomock" "gotest.tools/v3/assert/cmp" "github.com/docker/compose/v2/pkg/api" @@ -154,7 +157,16 @@ func TestBuildContainerMountOptions(t *testing.T) { }, } - mounts, err := buildContainerMountOptions(project, project.Services["myService"], image.InspectResponse{}, inherit) + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mock, cli := prepareMocks(mockCtrl) + s := composeService{ + dockerCli: cli, + } + mock.EXPECT().ImageInspect(gomock.Any(), "myProject-myService").AnyTimes().Return(image.InspectResponse{}, nil) + + mounts, err := s.buildContainerMountOptions(context.TODO(), project, project.Services["myService"], inherit) sort.Slice(mounts, func(i, j int) bool { return mounts[i].Target < mounts[j].Target }) @@ -166,7 +178,7 @@ func TestBuildContainerMountOptions(t *testing.T) { assert.Equal(t, mounts[2].VolumeOptions.Subpath, "etc") assert.Equal(t, mounts[3].Target, "\\\\.\\pipe\\docker_engine") - mounts, err = buildContainerMountOptions(project, project.Services["myService"], image.InspectResponse{}, inherit) + mounts, err = s.buildContainerMountOptions(context.TODO(), project, project.Services["myService"], inherit) sort.Slice(mounts, func(i, j int) bool { return mounts[i].Target < mounts[j].Target }) @@ -321,3 +333,123 @@ func TestCreateEndpointSettings(t *testing.T) { IPv6Gateway: "fdb4:7a7f:373a:3f0c::42", })) } + +func Test_buildContainerVolumes(t *testing.T) { + pwd, err := os.Getwd() + assert.NilError(t, err) + + tests := []struct { + name string + yaml string + binds []string + mounts []mountTypes.Mount + }{ + { + name: "bind mount local path", + yaml: ` +services: + test: + volumes: + - ./data:/data +`, + binds: []string{filepath.Join(pwd, "data") + ":/data:rw"}, + mounts: nil, + }, + { + name: "bind mount, not create host path", + yaml: ` +services: + test: + volumes: + - type: bind + source: ./data + target: /data + bind: + create_host_path: false +`, + binds: nil, + mounts: []mountTypes.Mount{ + { + Type: "bind", + Source: filepath.Join(pwd, "data"), + Target: "/data", + BindOptions: &mountTypes.BindOptions{CreateMountpoint: false}, + }, + }, + }, + { + name: "mount volume", + yaml: ` +services: + test: + volumes: + - data:/data +volumes: + data: + name: my_volume +`, + binds: []string{"my_volume:/data:rw"}, + mounts: nil, + }, + { + name: "mount volume, readonly", + yaml: ` +services: + test: + volumes: + - data:/data:ro +volumes: + data: + name: my_volume +`, + binds: []string{"my_volume:/data:ro"}, + mounts: nil, + }, + { + name: "mount volume subpath", + yaml: ` +services: + test: + volumes: + - type: volume + source: data + target: /data + volume: + subpath: test/ +volumes: + data: + name: my_volume +`, + binds: nil, + mounts: []mountTypes.Mount{ + { + Type: "volume", + Source: "my_volume", + Target: "/data", + VolumeOptions: &mountTypes.VolumeOptions{Subpath: "test/"}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p, err := composeloader.LoadWithContext(context.TODO(), composetypes.ConfigDetails{ + ConfigFiles: []composetypes.ConfigFile{ + { + Filename: "test", + Content: []byte(tt.yaml), + }, + }, + }, func(options *composeloader.Options) { + options.SkipValidation = true + options.SkipConsistencyCheck = true + }) + assert.NilError(t, err) + s := &composeService{} + binds, mounts, err := s.buildContainerVolumes(context.TODO(), *p, p.Services["test"], nil) + assert.NilError(t, err) + assert.DeepEqual(t, tt.binds, binds) + assert.DeepEqual(t, tt.mounts, mounts) + }) + } +}