Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit a3f88a0

Browse filesBrowse files
ndeloofglours
ndeloof
authored and
glours
committed
test to cover preference for bind API
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
1 parent c83f128 commit a3f88a0
Copy full SHA for a3f88a0

File tree

Expand file treeCollapse file tree

2 files changed

+168
-23
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+168
-23
lines changed

‎pkg/compose/create.go

Copy file name to clipboardExpand all lines: pkg/compose/create.go
+34-21Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ import (
3939
"github.com/docker/docker/api/types/blkiodev"
4040
"github.com/docker/docker/api/types/container"
4141
"github.com/docker/docker/api/types/filters"
42-
"github.com/docker/docker/api/types/image"
4342
"github.com/docker/docker/api/types/mount"
4443
"github.com/docker/docker/api/types/network"
4544
"github.com/docker/docker/api/types/strslice"
@@ -828,7 +827,6 @@ func getDependentServiceFromMode(mode string) string {
828827
return ""
829828
}
830829

831-
//nolint:gocyclo
832830
func (s *composeService) buildContainerVolumes(
833831
ctx context.Context,
834832
p types.Project,
@@ -838,13 +836,7 @@ func (s *composeService) buildContainerVolumes(
838836
var mounts []mount.Mount
839837
var binds []string
840838

841-
img := api.GetImageNameOrDefault(service, p.Name)
842-
imgInspect, err := s.apiClient().ImageInspect(ctx, img)
843-
if err != nil {
844-
return nil, nil, err
845-
}
846-
847-
mountOptions, err := buildContainerMountOptions(p, service, imgInspect, inherit)
839+
mountOptions, err := s.buildContainerMountOptions(ctx, p, service, inherit)
848840
if err != nil {
849841
return nil, nil, err
850842
}
@@ -857,11 +849,10 @@ func (s *composeService) buildContainerVolumes(
857849
// see https://github.com/moby/moby/issues/43483
858850
v := findVolumeByTarget(service.Volumes, m.Target)
859851
if v != nil {
860-
switch {
861-
case v.Type != types.VolumeTypeBind:
852+
if v.Type != types.VolumeTypeBind {
862853
v.Source = m.Source
863-
fallthrough
864-
case !requireMountAPI(v.Bind):
854+
}
855+
if !bindRequiresMountAPI(v.Bind) {
865856
source := m.Source
866857
if vol := findVolumeByName(p.Volumes, m.Source); vol != nil {
867858
source = m.Source
@@ -874,8 +865,8 @@ func (s *composeService) buildContainerVolumes(
874865
v := findVolumeByTarget(service.Volumes, m.Target)
875866
vol := findVolumeByName(p.Volumes, m.Source)
876867
if v != nil && vol != nil {
877-
if _, ok := vol.DriverOpts["device"]; ok && vol.Driver == "local" && vol.DriverOpts["o"] == "bind" {
878-
// Looks like a volume, but actually a bind mount which requires the bind API
868+
// Prefer the bind API if no advanced option is used, to preserve backward compatibility
869+
if !volumeRequiresMountAPI(v.Volume) {
879870
binds = append(binds, toBindString(vol.Name, v))
880871
continue
881872
}
@@ -930,9 +921,9 @@ func findVolumeByTarget(volumes []types.ServiceVolumeConfig, target string) *typ
930921
return nil
931922
}
932923

933-
// requireMountAPI check if Bind declaration can be implemented by the plain old Bind API or uses any of the advanced
924+
// bindRequiresMountAPI check if Bind declaration can be implemented by the plain old Bind API or uses any of the advanced
934925
// options which require use of Mount API
935-
func requireMountAPI(bind *types.ServiceVolumeBind) bool {
926+
func bindRequiresMountAPI(bind *types.ServiceVolumeBind) bool {
936927
switch {
937928
case bind == nil:
938929
return false
@@ -947,7 +938,24 @@ func requireMountAPI(bind *types.ServiceVolumeBind) bool {
947938
}
948939
}
949940

950-
func buildContainerMountOptions(p types.Project, s types.ServiceConfig, img image.InspectResponse, inherit *container.Summary) ([]mount.Mount, error) {
941+
// volumeRequiresMountAPI check if Volume declaration can be implemented by the plain old Bind API or uses any of the advanced
942+
// options which require use of Mount API
943+
func volumeRequiresMountAPI(vol *types.ServiceVolumeVolume) bool {
944+
switch {
945+
case vol == nil:
946+
return false
947+
case len(vol.Labels) > 0:
948+
return true
949+
case vol.Subpath != "":
950+
return true
951+
case vol.NoCopy:
952+
return true
953+
default:
954+
return false
955+
}
956+
}
957+
958+
func (s *composeService) buildContainerMountOptions(ctx context.Context, p types.Project, service types.ServiceConfig, inherit *container.Summary) ([]mount.Mount, error) {
951959
mounts := map[string]mount.Mount{}
952960
if inherit != nil {
953961
for _, m := range inherit.Mounts {
@@ -959,6 +967,11 @@ func buildContainerMountOptions(p types.Project, s types.ServiceConfig, img imag
959967
src = m.Name
960968
}
961969

970+
img, err := s.apiClient().ImageInspect(ctx, api.GetImageNameOrDefault(service, p.Name))
971+
if err != nil {
972+
return nil, err
973+
}
974+
962975
if img.Config != nil {
963976
if _, ok := img.Config.Volumes[m.Destination]; ok {
964977
// inherit previous container's anonymous volume
@@ -971,7 +984,7 @@ func buildContainerMountOptions(p types.Project, s types.ServiceConfig, img imag
971984
}
972985
}
973986
volumes := []types.ServiceVolumeConfig{}
974-
for _, v := range s.Volumes {
987+
for _, v := range service.Volumes {
975988
if v.Target != m.Destination || v.Source != "" {
976989
volumes = append(volumes, v)
977990
continue
@@ -984,11 +997,11 @@ func buildContainerMountOptions(p types.Project, s types.ServiceConfig, img imag
984997
ReadOnly: !m.RW,
985998
}
986999
}
987-
s.Volumes = volumes
1000+
service.Volumes = volumes
9881001
}
9891002
}
9901003

991-
mounts, err := fillBindMounts(p, s, mounts)
1004+
mounts, err := fillBindMounts(p, service, mounts)
9921005
if err != nil {
9931006
return nil, err
9941007
}

‎pkg/compose/create_test.go

Copy file name to clipboardExpand all lines: pkg/compose/create_test.go
+134-2Lines changed: 134 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,16 @@
1717
package compose
1818

1919
import (
20+
"context"
2021
"os"
2122
"path/filepath"
2223
"sort"
2324
"testing"
2425

26+
composeloader "github.com/compose-spec/compose-go/v2/loader"
2527
"github.com/docker/docker/api/types/container"
2628
"github.com/docker/docker/api/types/image"
29+
"go.uber.org/mock/gomock"
2730
"gotest.tools/v3/assert/cmp"
2831

2932
"github.com/docker/compose/v2/pkg/api"
@@ -154,7 +157,16 @@ func TestBuildContainerMountOptions(t *testing.T) {
154157
},
155158
}
156159

157-
mounts, err := buildContainerMountOptions(project, project.Services["myService"], image.InspectResponse{}, inherit)
160+
mockCtrl := gomock.NewController(t)
161+
defer mockCtrl.Finish()
162+
163+
mock, cli := prepareMocks(mockCtrl)
164+
s := composeService{
165+
dockerCli: cli,
166+
}
167+
mock.EXPECT().ImageInspect(gomock.Any(), "myProject-myService").AnyTimes().Return(image.InspectResponse{}, nil)
168+
169+
mounts, err := s.buildContainerMountOptions(context.TODO(), project, project.Services["myService"], inherit)
158170
sort.Slice(mounts, func(i, j int) bool {
159171
return mounts[i].Target < mounts[j].Target
160172
})
@@ -166,7 +178,7 @@ func TestBuildContainerMountOptions(t *testing.T) {
166178
assert.Equal(t, mounts[2].VolumeOptions.Subpath, "etc")
167179
assert.Equal(t, mounts[3].Target, "\\\\.\\pipe\\docker_engine")
168180

169-
mounts, err = buildContainerMountOptions(project, project.Services["myService"], image.InspectResponse{}, inherit)
181+
mounts, err = s.buildContainerMountOptions(context.TODO(), project, project.Services["myService"], inherit)
170182
sort.Slice(mounts, func(i, j int) bool {
171183
return mounts[i].Target < mounts[j].Target
172184
})
@@ -321,3 +333,123 @@ func TestCreateEndpointSettings(t *testing.T) {
321333
IPv6Gateway: "fdb4:7a7f:373a:3f0c::42",
322334
}))
323335
}
336+
337+
func Test_buildContainerVolumes(t *testing.T) {
338+
pwd, err := os.Getwd()
339+
assert.NilError(t, err)
340+
341+
tests := []struct {
342+
name string
343+
yaml string
344+
binds []string
345+
mounts []mountTypes.Mount
346+
}{
347+
{
348+
name: "bind mount local path",
349+
yaml: `
350+
services:
351+
test:
352+
volumes:
353+
- ./data:/data
354+
`,
355+
binds: []string{filepath.Join(pwd, "data") + ":/data:rw"},
356+
mounts: nil,
357+
},
358+
{
359+
name: "bind mount, not create host path",
360+
yaml: `
361+
services:
362+
test:
363+
volumes:
364+
- type: bind
365+
source: ./data
366+
target: /data
367+
bind:
368+
create_host_path: false
369+
`,
370+
binds: nil,
371+
mounts: []mountTypes.Mount{
372+
{
373+
Type: "bind",
374+
Source: filepath.Join(pwd, "data"),
375+
Target: "/data",
376+
BindOptions: &mountTypes.BindOptions{CreateMountpoint: false},
377+
},
378+
},
379+
},
380+
{
381+
name: "mount volume",
382+
yaml: `
383+
services:
384+
test:
385+
volumes:
386+
- data:/data
387+
volumes:
388+
data:
389+
name: my_volume
390+
`,
391+
binds: []string{"my_volume:/data:rw"},
392+
mounts: nil,
393+
},
394+
{
395+
name: "mount volume, readonly",
396+
yaml: `
397+
services:
398+
test:
399+
volumes:
400+
- data:/data:ro
401+
volumes:
402+
data:
403+
name: my_volume
404+
`,
405+
binds: []string{"my_volume:/data:ro"},
406+
mounts: nil,
407+
},
408+
{
409+
name: "mount volume subpath",
410+
yaml: `
411+
services:
412+
test:
413+
volumes:
414+
- type: volume
415+
source: data
416+
target: /data
417+
volume:
418+
subpath: test/
419+
volumes:
420+
data:
421+
name: my_volume
422+
`,
423+
binds: nil,
424+
mounts: []mountTypes.Mount{
425+
{
426+
Type: "volume",
427+
Source: "my_volume",
428+
Target: "/data",
429+
VolumeOptions: &mountTypes.VolumeOptions{Subpath: "test/"},
430+
},
431+
},
432+
},
433+
}
434+
for _, tt := range tests {
435+
t.Run(tt.name, func(t *testing.T) {
436+
p, err := composeloader.LoadWithContext(context.TODO(), composetypes.ConfigDetails{
437+
ConfigFiles: []composetypes.ConfigFile{
438+
{
439+
Filename: "test",
440+
Content: []byte(tt.yaml),
441+
},
442+
},
443+
}, func(options *composeloader.Options) {
444+
options.SkipValidation = true
445+
options.SkipConsistencyCheck = true
446+
})
447+
assert.NilError(t, err)
448+
s := &composeService{}
449+
binds, mounts, err := s.buildContainerVolumes(context.TODO(), *p, p.Services["test"], nil)
450+
assert.NilError(t, err)
451+
assert.DeepEqual(t, tt.binds, binds)
452+
assert.DeepEqual(t, tt.mounts, mounts)
453+
})
454+
}
455+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.