Merge pull request #26833 from cpuguy83/use_mounts_api_for_services

Use container Mounts API for Swarm containers.

daemon/cluster/executor/container/container.go

@@ -14,6 +14,7 @@ import (
 	enginecontainer "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/events"
 	"github.com/docker/docker/api/types/filters"
+	enginemount "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/api/types/network"
 	volumetypes "github.com/docker/docker/api/types/volume"
 	clustertypes "github.com/docker/docker/daemon/cluster/provider"
@@ -191,7 +192,6 @@ func (c *containerConfig) config() *enginecontainer.Config {
 		Hostname:     c.spec().Hostname,
 		WorkingDir:   c.spec().Dir,
 		Image:        c.image(),
-		Volumes:      c.volumes(),
 		ExposedPorts: c.exposedPorts(),
 		Healthcheck:  c.healthcheck(),
 	}
@@ -243,49 +243,79 @@ func (c *containerConfig) labels() map[string]string {
 	return labels
 }
 
-// volumes gets placed into the Volumes field on the containerConfig.
-func (c *containerConfig) volumes() map[string]struct{} {
-	r := make(map[string]struct{})
-	// Volumes *only* creates anonymous volumes. The rest is mixed in with
-	// binds, which aren't actually binds. Basically, any volume that
-	// results in a single component must be added here.
-	//
-	// This is reversed engineered from the behavior of the engine API.
+func (c *containerConfig) mounts() []enginemount.Mount {
+	var r []enginemount.Mount
 	for _, mount := range c.spec().Mounts {
-		if mount.Type == api.MountTypeVolume && mount.Source == "" {
-			r[mount.Target] = struct{}{}
-		}
+		r = append(r, convertMount(mount))
 	}
 	return r
 }
 
-func (c *containerConfig) tmpfs() map[string]string {
-	r := make(map[string]string)
-
-	for _, spec := range c.spec().Mounts {
-		if spec.Type != api.MountTypeTmpfs {
-			continue
-		}
-
-		r[spec.Target] = getMountMask(&spec)
+func convertMount(m api.Mount) enginemount.Mount {
+	mount := enginemount.Mount{
+		Source:   m.Source,
+		Target:   m.Target,
+		ReadOnly: m.ReadOnly,
 	}
 
-	return r
-}
+	switch m.Type {
+	case api.MountTypeBind:
+		mount.Type = enginemount.TypeBind
+	case api.MountTypeVolume:
+		mount.Type = enginemount.TypeVolume
+	case api.MountTypeTmpfs:
+		mount.Type = enginemount.TypeTmpfs
+	}
 
-func (c *containerConfig) binds() []string {
-	var r []string
-	for _, mount := range c.spec().Mounts {
-		if mount.Type == api.MountTypeBind || (mount.Type == api.MountTypeVolume && mount.Source != "") {
-			spec := fmt.Sprintf("%s:%s", mount.Source, mount.Target)
-			mask := getMountMask(&mount)
-			if mask != "" {
-				spec = fmt.Sprintf("%s:%s", spec, mask)
+	if m.BindOptions != nil {
+		mount.BindOptions = &enginemount.BindOptions{}
+		switch m.BindOptions.Propagation {
+		case api.MountPropagationRPrivate:
+			mount.BindOptions.Propagation = enginemount.PropagationRPrivate
+		case api.MountPropagationPrivate:
+			mount.BindOptions.Propagation = enginemount.PropagationPrivate
+		case api.MountPropagationRSlave:
+			mount.BindOptions.Propagation = enginemount.PropagationRSlave
+		case api.MountPropagationSlave:
+			mount.BindOptions.Propagation = enginemount.PropagationSlave
+		case api.MountPropagationRShared:
+			mount.BindOptions.Propagation = enginemount.PropagationRShared
+		case api.MountPropagationShared:
+			mount.BindOptions.Propagation = enginemount.PropagationShared
+		}
+	}
+
+	if m.VolumeOptions != nil {
+		mount.VolumeOptions = &enginemount.VolumeOptions{
+			NoCopy: m.VolumeOptions.NoCopy,
+		}
+		if m.VolumeOptions.Labels != nil {
+			mount.VolumeOptions.Labels = make(map[string]string, len(m.VolumeOptions.Labels))
+			for k, v := range m.VolumeOptions.Labels {
+				mount.VolumeOptions.Labels[k] = v
+			}
+		}
+		if m.VolumeOptions.DriverConfig != nil {
+			mount.VolumeOptions.DriverConfig = &enginemount.Driver{
+				Name: m.VolumeOptions.DriverConfig.Name,
+			}
+			if m.VolumeOptions.DriverConfig.Options != nil {
+				mount.VolumeOptions.DriverConfig.Options = make(map[string]string, len(m.VolumeOptions.DriverConfig.Options))
+				for k, v := range m.VolumeOptions.DriverConfig.Options {
+					mount.VolumeOptions.DriverConfig.Options[k] = v
+				}
 			}
-			r = append(r, spec)
 		}
 	}
-	return r
+
+	if m.TmpfsOptions != nil {
+		mount.TmpfsOptions = &enginemount.TmpfsOptions{
+			SizeBytes: m.TmpfsOptions.SizeBytes,
+			Mode:      m.TmpfsOptions.Mode,
+		}
+	}
+
+	return mount
 }
 
 func (c *containerConfig) healthcheck() *enginecontainer.HealthConfig {
@@ -303,88 +333,12 @@ func (c *containerConfig) healthcheck() *enginecontainer.HealthConfig {
 	}
 }
 
-func getMountMask(m *api.Mount) string {
-	var maskOpts []string
-	if m.ReadOnly {
-		maskOpts = append(maskOpts, "ro")
-	}
-
-	switch m.Type {
-	case api.MountTypeVolume:
-		if m.VolumeOptions != nil && m.VolumeOptions.NoCopy {
-			maskOpts = append(maskOpts, "nocopy")
-		}
-	case api.MountTypeBind:
-		if m.BindOptions == nil {
-			break
-		}
-
-		switch m.BindOptions.Propagation {
-		case api.MountPropagationPrivate:
-			maskOpts = append(maskOpts, "private")
-		case api.MountPropagationRPrivate:
-			maskOpts = append(maskOpts, "rprivate")
-		case api.MountPropagationShared:
-			maskOpts = append(maskOpts, "shared")
-		case api.MountPropagationRShared:
-			maskOpts = append(maskOpts, "rshared")
-		case api.MountPropagationSlave:
-			maskOpts = append(maskOpts, "slave")
-		case api.MountPropagationRSlave:
-			maskOpts = append(maskOpts, "rslave")
-		}
-	case api.MountTypeTmpfs:
-		if m.TmpfsOptions == nil {
-			break
-		}
-
-		if m.TmpfsOptions.Mode != 0 {
-			maskOpts = append(maskOpts, fmt.Sprintf("mode=%o", m.TmpfsOptions.Mode))
-		}
-
-		if m.TmpfsOptions.SizeBytes != 0 {
-			// calculate suffix here, making this linux specific, but that is
-			// okay, since API is that way anyways.
-
-			// we do this by finding the suffix that divides evenly into the
-			// value, returing the value itself, with no suffix, if it fails.
-			//
-			// For the most part, we don't enforce any semantic to this values.
-			// The operating system will usually align this and enforce minimum
-			// and maximums.
-			var (
-				size   = m.TmpfsOptions.SizeBytes
-				suffix string
-			)
-			for _, r := range []struct {
-				suffix  string
-				divisor int64
-			}{
-				{"g", 1 << 30},
-				{"m", 1 << 20},
-				{"k", 1 << 10},
-			} {
-				if size%r.divisor == 0 {
-					size = size / r.divisor
-					suffix = r.suffix
-					break
-				}
-			}
-
-			maskOpts = append(maskOpts, fmt.Sprintf("size=%d%s", size, suffix))
-		}
-	}
-
-	return strings.Join(maskOpts, ",")
-}
-
 func (c *containerConfig) hostConfig() *enginecontainer.HostConfig {
 	hc := &enginecontainer.HostConfig{
 		Resources:    c.resources(),
-		Binds:        c.binds(),
-		Tmpfs:        c.tmpfs(),
 		GroupAdd:     c.spec().Groups,
 		PortBindings: c.portBindings(),
+		Mounts:       c.mounts(),
 	}
 
 	if c.spec().DNSConfig != nil {

integration-cli/docker_cli_service_create_test.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/pkg/integration/checker"
 	"github.com/go-check/check"
@@ -15,7 +16,7 @@ import (
 
 func (s *DockerSwarmSuite) TestServiceCreateMountVolume(c *check.C) {
 	d := s.AddDaemon(c, true, true)
-	out, err := d.Cmd("service", "create", "--mount", "type=volume,source=foo,target=/foo", "busybox", "top")
+	out, err := d.Cmd("service", "create", "--mount", "type=volume,source=foo,target=/foo,volume-nocopy", "busybox", "top")
 	c.Assert(err, checker.IsNil, check.Commentf(out))
 	id := strings.TrimSpace(out)
 
@@ -33,6 +34,21 @@ func (s *DockerSwarmSuite) TestServiceCreateMountVolume(c *check.C) {
 		return task.NodeID != "" && task.Status.ContainerStatus.ContainerID != "", nil
 	}, checker.Equals, true)
 
+	// check container mount config
+	out, err = s.nodeCmd(c, task.NodeID, "inspect", "--format", "{{json .HostConfig.Mounts}}", task.Status.ContainerStatus.ContainerID)
+	c.Assert(err, checker.IsNil, check.Commentf(out))
+
+	var mountConfig []mount.Mount
+	c.Assert(json.Unmarshal([]byte(out), &mountConfig), checker.IsNil)
+	c.Assert(mountConfig, checker.HasLen, 1)
+
+	c.Assert(mountConfig[0].Source, checker.Equals, "foo")
+	c.Assert(mountConfig[0].Target, checker.Equals, "/foo")
+	c.Assert(mountConfig[0].Type, checker.Equals, mount.TypeVolume)
+	c.Assert(mountConfig[0].VolumeOptions, checker.NotNil)
+	c.Assert(mountConfig[0].VolumeOptions.NoCopy, checker.True)
+
+	// check container mounts actual
 	out, err = s.nodeCmd(c, task.NodeID, "inspect", "--format", "{{json .Mounts}}", task.Status.ContainerStatus.ContainerID)
 	c.Assert(err, checker.IsNil, check.Commentf(out))
 
@@ -40,6 +56,7 @@ func (s *DockerSwarmSuite) TestServiceCreateMountVolume(c *check.C) {
 	c.Assert(json.Unmarshal([]byte(out), &mounts), checker.IsNil)
 	c.Assert(mounts, checker.HasLen, 1)
 
+	c.Assert(mounts[0].Type, checker.Equals, mount.TypeVolume)
 	c.Assert(mounts[0].Name, checker.Equals, "foo")
 	c.Assert(mounts[0].Destination, checker.Equals, "/foo")
 	c.Assert(mounts[0].RW, checker.Equals, true)
@@ -103,3 +120,53 @@ func (s *DockerSwarmSuite) TestServiceCreateWithSecretSourceTarget(c *check.C) {
 	c.Assert(refs[0].File, checker.Not(checker.IsNil))
 	c.Assert(refs[0].File.Name, checker.Equals, testTarget)
 }
+
+func (s *DockerSwarmSuite) TestServiceCreateMountTmpfs(c *check.C) {
+	d := s.AddDaemon(c, true, true)
+	out, err := d.Cmd("service", "create", "--mount", "type=tmpfs,target=/foo", "busybox", "sh", "-c", "mount | grep foo; tail -f /dev/null")
+	c.Assert(err, checker.IsNil, check.Commentf(out))
+	id := strings.TrimSpace(out)
+
+	var tasks []swarm.Task
+	waitAndAssert(c, defaultReconciliationTimeout, func(c *check.C) (interface{}, check.CommentInterface) {
+		tasks = d.getServiceTasks(c, id)
+		return len(tasks) > 0, nil
+	}, checker.Equals, true)
+
+	task := tasks[0]
+	waitAndAssert(c, defaultReconciliationTimeout, func(c *check.C) (interface{}, check.CommentInterface) {
+		if task.NodeID == "" || task.Status.ContainerStatus.ContainerID == "" {
+			task = d.getTask(c, task.ID)
+		}
+		return task.NodeID != "" && task.Status.ContainerStatus.ContainerID != "", nil
+	}, checker.Equals, true)
+
+	// check container mount config
+	out, err = s.nodeCmd(c, task.NodeID, "inspect", "--format", "{{json .HostConfig.Mounts}}", task.Status.ContainerStatus.ContainerID)
+	c.Assert(err, checker.IsNil, check.Commentf(out))
+
+	var mountConfig []mount.Mount
+	c.Assert(json.Unmarshal([]byte(out), &mountConfig), checker.IsNil)
+	c.Assert(mountConfig, checker.HasLen, 1)
+
+	c.Assert(mountConfig[0].Source, checker.Equals, "")
+	c.Assert(mountConfig[0].Target, checker.Equals, "/foo")
+	c.Assert(mountConfig[0].Type, checker.Equals, mount.TypeTmpfs)
+
+	// check container mounts actual
+	out, err = s.nodeCmd(c, task.NodeID, "inspect", "--format", "{{json .Mounts}}", task.Status.ContainerStatus.ContainerID)
+	c.Assert(err, checker.IsNil, check.Commentf(out))
+
+	var mounts []types.MountPoint
+	c.Assert(json.Unmarshal([]byte(out), &mounts), checker.IsNil)
+	c.Assert(mounts, checker.HasLen, 1)
+
+	c.Assert(mounts[0].Type, checker.Equals, mount.TypeTmpfs)
+	c.Assert(mounts[0].Name, checker.Equals, "")
+	c.Assert(mounts[0].Destination, checker.Equals, "/foo")
+	c.Assert(mounts[0].RW, checker.Equals, true)
+
+	out, err = s.nodeCmd(c, task.NodeID, "logs", task.Status.ContainerStatus.ContainerID)
+	c.Assert(err, checker.IsNil, check.Commentf(out))
+	c.Assert(strings.TrimSpace(out), checker.HasPrefix, "tmpfs on /foo type tmpfs")
+}

volume/volume_linux.go

@@ -11,8 +11,6 @@ import (
 
 // ConvertTmpfsOptions converts *mounttypes.TmpfsOptions to the raw option string
 // for mount(2).
-// The logic is copy-pasted from daemon/cluster/executer/container.getMountMask.
-// It will be deduplicated when we migrated the cluster to the new mount scheme.
 func ConvertTmpfsOptions(opt *mounttypes.TmpfsOptions, readOnly bool) (string, error) {
 	var rawOpts []string
 	if readOnly {