Skip to content

Commit

Permalink
prefer mount API over bind
Browse files Browse the repository at this point in the history
Signed-off-by: Nicolas De Loof <[email protected]>
  • Loading branch information
ndeloof authored and jhrotko committed Aug 26, 2024
1 parent 6e172d6 commit f9c7a0c
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 42 deletions.
31 changes: 8 additions & 23 deletions pkg/compose/convergence_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
containerType "github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/filters"
"github.com/docker/docker/api/types/network"
"github.com/docker/go-connections/nat"
"go.uber.org/mock/gomock"
"gotest.tools/v3/assert"

Expand Down Expand Up @@ -301,17 +300,10 @@ func TestCreateMobyContainer(t *testing.T) {
},
}

var falseBool bool
apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any(), gomock.Eq(
&containerType.HostConfig{
PortBindings: nat.PortMap{},
ExtraHosts: []string{},
Tmpfs: map[string]string{},
Resources: containerType.Resources{
OomKillDisable: &falseBool,
},
NetworkMode: "b-moby-name",
}), gomock.Eq(
apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any(), gomock.Cond(func(x any) bool {
v := x.(*containerType.HostConfig)
return v.NetworkMode == "b-moby-name"
}), gomock.Eq(
&network.NetworkingConfig{
EndpointsConfig: map[string]*network.EndpointSettings{
"b-moby-name": {
Expand Down Expand Up @@ -390,17 +382,10 @@ func TestCreateMobyContainer(t *testing.T) {
},
}

var falseBool bool
apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any(), gomock.Eq(
&containerType.HostConfig{
PortBindings: nat.PortMap{},
ExtraHosts: []string{},
Tmpfs: map[string]string{},
Resources: containerType.Resources{
OomKillDisable: &falseBool,
},
NetworkMode: "b-moby-name",
}), gomock.Eq(
apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any(), gomock.Cond(func(x any) bool {
v := x.(*containerType.HostConfig)
return v.NetworkMode == "b-moby-name"
}), gomock.Eq(
&network.NetworkingConfig{
EndpointsConfig: map[string]*network.EndpointSettings{
"a-moby-name": {
Expand Down
34 changes: 15 additions & 19 deletions pkg/compose/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -824,23 +824,23 @@ func (s *composeService) buildContainerVolumes(
return nil, nil, err
}

version, err := s.RuntimeVersion(ctx)
if err != nil {
return nil, nil, err
}
if versions.GreaterThan(version, "1.42") {
// We can fully leverage `Mount` API as a replacement for legacy `Bind`
return nil, mountOptions, nil
}

MOUNTS:
for _, m := range mountOptions {
if m.Type == mount.TypeNamedPipe {
mounts = append(mounts, m)
continue
}
if m.Type == mount.TypeBind {
// `Mount` is preferred but does not offer option to created host path if missing
// `Mount` does not offer option to created host path if missing
// so `Bind` API is used here with raw volume string
// see https:/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 v.Bind != nil && v.Bind.CreateHostPath:
if v.Bind != nil && v.Bind.CreateHostPath {
binds = append(binds, v.String())
continue MOUNTS
}
Expand Down Expand Up @@ -1078,7 +1078,7 @@ func buildMount(project types.Project, volume types.ServiceVolumeConfig) (mount.
}
}

bind, vol, tmpfs := buildMountOptions(project, volume)
bind, vol, tmpfs := buildMountOptions(volume)

volume.Target = path.Clean(volume.Target)

Expand All @@ -1098,7 +1098,7 @@ func buildMount(project types.Project, volume types.ServiceVolumeConfig) (mount.
}, nil
}

func buildMountOptions(project types.Project, volume types.ServiceVolumeConfig) (*mount.BindOptions, *mount.VolumeOptions, *mount.TmpfsOptions) {
func buildMountOptions(volume types.ServiceVolumeConfig) (*mount.BindOptions, *mount.VolumeOptions, *mount.TmpfsOptions) {
switch volume.Type {
case "bind":
if volume.Volume != nil {
Expand All @@ -1115,11 +1115,6 @@ func buildMountOptions(project types.Project, volume types.ServiceVolumeConfig)
if volume.Tmpfs != nil {
logrus.Warnf("mount of type `volume` should not define `tmpfs` option")
}
if v, ok := project.Volumes[volume.Source]; ok && v.DriverOpts["o"] == types.VolumeTypeBind {
return buildBindOption(&types.ServiceVolumeBind{
CreateHostPath: true,
}), nil, nil
}
return nil, buildVolumeOptions(volume.Volume), nil
case "tmpfs":
if volume.Bind != nil {
Expand All @@ -1138,7 +1133,8 @@ func buildBindOption(bind *types.ServiceVolumeBind) *mount.BindOptions {
return nil
}
return &mount.BindOptions{
Propagation: mount.Propagation(bind.Propagation),
Propagation: mount.Propagation(bind.Propagation),
CreateMountpoint: bind.CreateHostPath,
// NonRecursive: false, FIXME missing from model ?
}
}
Expand Down

0 comments on commit f9c7a0c

Please sign in to comment.