Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

revert commits link to mount API over bind changes #12141

Merged
merged 1 commit into from
Sep 20, 2024
Merged

revert commits link to mount API over bind changes #12141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions pkg/compose/compose.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,23 +321,6 @@ func (s *composeService) RuntimeVersion(ctx context.Context) (string, error) {

}

var windowsContainer = struct {
once sync.Once
val bool
err error
}{}

func (s *composeService) isWindowsContainer(ctx context.Context) (bool, error) {
windowsContainer.once.Do(func() {
info, err := s.apiClient().Info(ctx)
if err != nil {
windowsContainer.err = err
}
windowsContainer.val = info.OSType == "windows"
})
return windowsContainer.val, windowsContainer.err
}

func (s *composeService) isDesktopIntegrationActive() bool {
return s.desktopCli != nil
}
Expand Down
31 changes: 23 additions & 8 deletions pkg/compose/convergence_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ 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 @@ -300,10 +301,17 @@ func TestCreateMobyContainer(t *testing.T) {
},
}

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(
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(
&network.NetworkingConfig{
EndpointsConfig: map[string]*network.EndpointSettings{
"b-moby-name": {
Expand Down Expand Up @@ -382,10 +390,17 @@ func TestCreateMobyContainer(t *testing.T) {
},
}

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(
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(
&network.NetworkingConfig{
EndpointsConfig: map[string]*network.EndpointSettings{
"a-moby-name": {
Expand Down
102 changes: 46 additions & 56 deletions pkg/compose/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,28 +801,28 @@ func (s *composeService) buildContainerVolumes(
return nil, nil, err
}

mountOptions, err := s.buildContainerMountOptions(ctx, p, service, imgInspect, inherit)
mountOptions, err := buildContainerMountOptions(p, service, imgInspect, inherit)
if err != nil {
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` does not offer option to created host path if missing
// `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 {
if v.Bind != nil && v.Bind.CreateHostPath {
switch {
case string(m.Type) != v.Type:
v.Source = m.Source
fallthrough
case v.Bind != nil && v.Bind.CreateHostPath:
binds = append(binds, v.String())
continue MOUNTS
}
Expand All @@ -834,7 +834,7 @@ MOUNTS:
return binds, mounts, nil
}

func (s *composeService) buildContainerMountOptions(ctx context.Context, p types.Project, service types.ServiceConfig, img moby.ImageInspect, inherit *moby.Container) ([]mount.Mount, error) {
func buildContainerMountOptions(p types.Project, s types.ServiceConfig, img moby.ImageInspect, inherit *moby.Container) ([]mount.Mount, error) {
var mounts = map[string]mount.Mount{}
if inherit != nil {
for _, m := range inherit.Mounts {
Expand All @@ -859,7 +859,7 @@ func (s *composeService) buildContainerMountOptions(ctx context.Context, p types
}
}
volumes := []types.ServiceVolumeConfig{}
for _, v := range service.Volumes {
for _, v := range s.Volumes {
if v.Target != m.Destination || v.Source != "" {
volumes = append(volumes, v)
continue
Expand All @@ -872,11 +872,11 @@ func (s *composeService) buildContainerMountOptions(ctx context.Context, p types
ReadOnly: !m.RW,
}
}
service.Volumes = volumes
s.Volumes = volumes
}
}

mounts, err := s.fillBindMounts(ctx, p, service, mounts)
mounts, err := fillBindMounts(p, s, mounts)
if err != nil {
return nil, err
}
Expand All @@ -888,27 +888,27 @@ func (s *composeService) buildContainerMountOptions(ctx context.Context, p types
return values, nil
}

func (s *composeService) fillBindMounts(ctx context.Context, p types.Project, service types.ServiceConfig, m map[string]mount.Mount) (map[string]mount.Mount, error) {
for _, v := range service.Volumes {
bindMount, err := s.buildMount(ctx, p, v)
func fillBindMounts(p types.Project, s types.ServiceConfig, m map[string]mount.Mount) (map[string]mount.Mount, error) {
for _, v := range s.Volumes {
bindMount, err := buildMount(p, v)
if err != nil {
return nil, err
}
m[bindMount.Target] = bindMount
}

secrets, err := s.buildContainerSecretMounts(ctx, p, service)
secrets, err := buildContainerSecretMounts(p, s)
if err != nil {
return nil, err
}
for _, secret := range secrets {
if _, found := m[secret.Target]; found {
for _, s := range secrets {
if _, found := m[s.Target]; found {
continue
}
m[secret.Target] = secret
m[s.Target] = s
}

configs, err := s.buildContainerConfigMounts(ctx, p, service)
configs, err := buildContainerConfigMounts(p, s)
if err != nil {
return nil, err
}
Expand All @@ -921,11 +921,11 @@ func (s *composeService) fillBindMounts(ctx context.Context, p types.Project, se
return m, nil
}

func (s *composeService) buildContainerConfigMounts(ctx context.Context, p types.Project, service types.ServiceConfig) ([]mount.Mount, error) {
func buildContainerConfigMounts(p types.Project, s types.ServiceConfig) ([]mount.Mount, error) {
var mounts = map[string]mount.Mount{}

configsBaseDir := "/"
for _, config := range service.Configs {
for _, config := range s.Configs {
target := config.Target
if config.Target == "" {
target = configsBaseDir + config.Source
Expand Down Expand Up @@ -953,7 +953,7 @@ func (s *composeService) buildContainerConfigMounts(ctx context.Context, p types
continue
}

bindMount, err := s.buildMount(ctx, p, types.ServiceVolumeConfig{
bindMount, err := buildMount(p, types.ServiceVolumeConfig{
Type: types.VolumeTypeBind,
Source: definedConfig.File,
Target: target,
Expand All @@ -971,11 +971,11 @@ func (s *composeService) buildContainerConfigMounts(ctx context.Context, p types
return values, nil
}

func (s *composeService) buildContainerSecretMounts(ctx context.Context, p types.Project, service types.ServiceConfig) ([]mount.Mount, error) {
func buildContainerSecretMounts(p types.Project, s types.ServiceConfig) ([]mount.Mount, error) {
var mounts = map[string]mount.Mount{}

secretsDir := "/run/secrets/"
for _, secret := range service.Secrets {
for _, secret := range s.Secrets {
target := secret.Target
if secret.Target == "" {
target = secretsDir + secret.Source
Expand Down Expand Up @@ -1003,7 +1003,7 @@ func (s *composeService) buildContainerSecretMounts(ctx context.Context, p types
continue
}

mnt, err := s.buildMount(ctx, p, types.ServiceVolumeConfig{
mnt, err := buildMount(p, types.ServiceVolumeConfig{
Type: types.VolumeTypeBind,
Source: definedSecret.File,
Target: target,
Expand Down Expand Up @@ -1039,7 +1039,7 @@ func isWindowsAbs(p string) bool {
return false
}

func (s *composeService) buildMount(ctx context.Context, project types.Project, volume types.ServiceVolumeConfig) (mount.Mount, error) {
func buildMount(project types.Project, volume types.ServiceVolumeConfig) (mount.Mount, error) {
source := volume.Source
// on windows, filepath.IsAbs(source) is false for unix style abs path like /var/run/docker.sock.
// do not replace these with filepath.Abs(source) that will include a default drive.
Expand All @@ -1060,10 +1060,7 @@ func (s *composeService) buildMount(ctx context.Context, project types.Project,
}
}

bind, vol, tmpfs, err := s.buildMountOptions(ctx, volume)
if err != nil {
return mount.Mount{}, err
}
bind, vol, tmpfs := buildMountOptions(project, volume)

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

Expand All @@ -1083,7 +1080,7 @@ func (s *composeService) buildMount(ctx context.Context, project types.Project,
}, nil
}

func (s *composeService) buildMountOptions(ctx context.Context, volume types.ServiceVolumeConfig) (*mount.BindOptions, *mount.VolumeOptions, *mount.TmpfsOptions, error) {
func buildMountOptions(project types.Project, volume types.ServiceVolumeConfig) (*mount.BindOptions, *mount.VolumeOptions, *mount.TmpfsOptions) {
switch volume.Type {
case "bind":
if volume.Volume != nil {
Expand All @@ -1092,47 +1089,40 @@ func (s *composeService) buildMountOptions(ctx context.Context, volume types.Ser
if volume.Tmpfs != nil {
logrus.Warnf("mount of type `bind` should not define `tmpfs` option")
}
option, err := s.buildBindOption(ctx, volume.Bind)
return option, nil, nil, err
return buildBindOption(volume.Bind), nil, nil
case "volume":
if volume.Bind != nil {
logrus.Warnf("mount of type `volume` should not define `bind` option")
}
if volume.Tmpfs != nil {
logrus.Warnf("mount of type `volume` should not define `tmpfs` option")
}
return nil, buildVolumeOptions(volume.Volume), nil, nil
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 {
logrus.Warnf("mount of type `tmpfs` should not define `bind` option")
}
if volume.Volume != nil {
logrus.Warnf("mount of type `tmpfs` should not define `volume` option")
}
return nil, nil, buildTmpfsOptions(volume.Tmpfs), nil
return nil, nil, buildTmpfsOptions(volume.Tmpfs)
}
return nil, nil, nil, nil
return nil, nil, nil
}

func (s *composeService) buildBindOption(ctx context.Context, bind *types.ServiceVolumeBind) (*mount.BindOptions, error) {
func buildBindOption(bind *types.ServiceVolumeBind) *mount.BindOptions {
if bind == nil {
return nil, nil
}

propagation := bind.Propagation
isWindowsContainer, err := s.isWindowsContainer(ctx)
if err != nil {
return nil, err
}
if propagation == "" && !isWindowsContainer {
propagation = types.PropagationRPrivate
return nil
}

return &mount.BindOptions{
Propagation: mount.Propagation(propagation),
CreateMountpoint: bind.CreateHostPath,
Propagation: mount.Propagation(bind.Propagation),
// NonRecursive: false, FIXME missing from model ?
}, nil
}
}

func buildVolumeOptions(vol *types.ServiceVolumeVolume) *mount.VolumeOptions {
Expand Down
16 changes: 6 additions & 10 deletions pkg/compose/create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package compose

import (
"context"
"os"
"path/filepath"
"sort"
Expand All @@ -42,8 +41,7 @@ func TestBuildBindMount(t *testing.T) {
Source: "",
Target: "/data",
}
s := composeService{}
mount, err := s.buildMount(context.TODO(), project, volume)
mount, err := buildMount(project, volume)
assert.NilError(t, err)
assert.Assert(t, filepath.IsAbs(mount.Source))
_, err = os.Stat(mount.Source)
Expand All @@ -58,8 +56,7 @@ func TestBuildNamedPipeMount(t *testing.T) {
Source: "\\\\.\\pipe\\docker_engine_windows",
Target: "\\\\.\\pipe\\docker_engine",
}
s := composeService{}
mount, err := s.buildMount(context.TODO(), project, volume)
mount, err := buildMount(project, volume)
assert.NilError(t, err)
assert.Equal(t, mount.Type, mountTypes.TypeNamedPipe)
}
Expand All @@ -78,8 +75,7 @@ func TestBuildVolumeMount(t *testing.T) {
Source: "myVolume",
Target: "/data",
}
s := composeService{}
mount, err := s.buildMount(context.TODO(), project, volume)
mount, err := buildMount(project, volume)
assert.NilError(t, err)
assert.Equal(t, mount.Source, "myProject_myVolume")
assert.Equal(t, mount.Type, mountTypes.TypeVolume)
Expand Down Expand Up @@ -156,8 +152,8 @@ func TestBuildContainerMountOptions(t *testing.T) {
},
},
}
s := composeService{}
mounts, err := s.buildContainerMountOptions(context.TODO(), project, project.Services["myService"], moby.ImageInspect{}, inherit)

mounts, err := buildContainerMountOptions(project, project.Services["myService"], moby.ImageInspect{}, inherit)
sort.Slice(mounts, func(i, j int) bool {
return mounts[i].Target < mounts[j].Target
})
Expand All @@ -169,7 +165,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 = s.buildContainerMountOptions(context.TODO(), project, project.Services["myService"], moby.ImageInspect{}, inherit)
mounts, err = buildContainerMountOptions(project, project.Services["myService"], moby.ImageInspect{}, inherit)
sort.Slice(mounts, func(i, j int) bool {
return mounts[i].Target < mounts[j].Target
})
Expand Down
Loading