Merge pull request moby#24053 from cpuguy83/mounts_default_writable

Change defaults for mount writable and volume copying

api/client/service/create.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/docker/docker/api/client"
 	"github.com/docker/docker/cli"
+	"github.com/docker/engine-api/types"
 	"github.com/spf13/cobra"
 	"golang.org/x/net/context"
 )
@@ -33,7 +34,7 @@ func newCreateCommand(dockerCli *client.DockerCli) *cobra.Command {
 
 func runCreate(dockerCli *client.DockerCli, opts *serviceOptions) error {
 	apiClient := dockerCli.Client()
-	headers := map[string][]string{}
+	createOpts := types.ServiceCreateOptions{}
 
 	service, err := opts.ToService()
 	if err != nil {
@@ -49,10 +50,10 @@ func runCreate(dockerCli *client.DockerCli, opts *serviceOptions) error {
 		if err != nil {
 			return err
 		}
-		headers["X-Registry-Auth"] = []string{encodedAuth}
+		createOpts.EncodedRegistryAuth = encodedAuth
 	}
 
-	response, err := apiClient.ServiceCreate(ctx, service, headers)
+	response, err := apiClient.ServiceCreate(ctx, service, createOpts)
 	if err != nil {
 		return err
 	}

api/client/service/inspect.go

@@ -169,7 +169,7 @@ func printContainerSpec(out io.Writer, containerSpec swarm.ContainerSpec) {
 		for _, v := range containerSpec.Mounts {
 			fmt.Fprintf(out, "  Target = %s\n", v.Target)
 			fmt.Fprintf(out, "  Source = %s\n", v.Source)
-			fmt.Fprintf(out, "  Writable = %v\n", v.Writable)
+			fmt.Fprintf(out, "  ReadOnly = %v\n", v.ReadOnly)
 			fmt.Fprintf(out, "  Type = %v\n", v.Type)
 		}
 	}

api/client/service/opts.go

@@ -176,10 +176,16 @@ func (m *MountOpt) Set(value string) error {
 		}
 	}
 
+	// Set writable as the default
 	for _, field := range fields {
 		parts := strings.SplitN(field, "=", 2)
-		if len(parts) == 1 && strings.ToLower(parts[0]) == "writable" {
-			mount.Writable = true
+		if len(parts) == 1 && strings.ToLower(parts[0]) == "readonly" {
+			mount.ReadOnly = true
+			continue
+		}
+
+		if len(parts) == 1 && strings.ToLower(parts[0]) == "volume-nocopy" {
+			volumeOptions().NoCopy = true
 			continue
 		}
 
@@ -195,15 +201,16 @@ func (m *MountOpt) Set(value string) error {
 			mount.Source = value
 		case "target":
 			mount.Target = value
-		case "writable":
-			mount.Writable, err = strconv.ParseBool(value)
+		case "readonly":
+			ro, err := strconv.ParseBool(value)
 			if err != nil {
-				return fmt.Errorf("invalid value for writable: %s", value)
+				return fmt.Errorf("invalid value for readonly: %s", value)
 			}
+			mount.ReadOnly = ro
 		case "bind-propagation":
 			bindOptions().Propagation = swarm.MountPropagation(strings.ToUpper(value))
-		case "volume-populate":
-			volumeOptions().Populate, err = strconv.ParseBool(value)
+		case "volume-nocopy":
+			volumeOptions().NoCopy, err = strconv.ParseBool(value)
 			if err != nil {
 				return fmt.Errorf("invalid value for populate: %s", value)
 			}
@@ -229,6 +236,17 @@ func (m *MountOpt) Set(value string) error {
 		return fmt.Errorf("target is required")
 	}
 
+	if mount.VolumeOptions != nil && mount.Source == "" {
+		return fmt.Errorf("source is required when specifying volume-* options")
+	}
+
+	if mount.Type == swarm.MountType("BIND") && mount.VolumeOptions != nil {
+		return fmt.Errorf("cannot mix 'volume-*' options with mount type '%s'", swarm.MountTypeBind)
+	}
+	if mount.Type == swarm.MountType("VOLUME") && mount.BindOptions != nil {
+		return fmt.Errorf("cannot mix 'bind-*' options with mount type '%s'", swarm.MountTypeVolume)
+	}
+
 	m.values = append(m.values, mount)
 	return nil
 }

api/client/service/opts_test.go

@@ -111,5 +111,53 @@ func TestMountOptSetErrorInvalidField(t *testing.T) {
 
 func TestMountOptSetErrorInvalidWritable(t *testing.T) {
 	var mount MountOpt
-	assert.Error(t, mount.Set("type=VOLUME,writable=yes"), "invalid value for writable: yes")
+	assert.Error(t, mount.Set("type=VOLUME,readonly=no"), "invalid value for readonly: no")
+}
+
+func TestMountOptDefaultEnableWritable(t *testing.T) {
+	var m MountOpt
+	assert.NilError(t, m.Set("type=bind,target=/foo,source=/foo"))
+	assert.Equal(t, m.values[0].ReadOnly, false)
+
+	m = MountOpt{}
+	assert.NilError(t, m.Set("type=bind,target=/foo,source=/foo,readonly"))
+	assert.Equal(t, m.values[0].ReadOnly, true)
+
+	m = MountOpt{}
+	assert.NilError(t, m.Set("type=bind,target=/foo,source=/foo,readonly=1"))
+	assert.Equal(t, m.values[0].ReadOnly, true)
+
+	m = MountOpt{}
+	assert.NilError(t, m.Set("type=bind,target=/foo,source=/foo,readonly=0"))
+	assert.Equal(t, m.values[0].ReadOnly, false)
+}
+
+func TestMountOptVolumeNoCopy(t *testing.T) {
+	var m MountOpt
+	assert.Error(t, m.Set("type=volume,target=/foo,volume-nocopy"), "source is required")
+
+	m = MountOpt{}
+	assert.NilError(t, m.Set("type=volume,target=/foo,source=foo"))
+	assert.Equal(t, m.values[0].VolumeOptions == nil, true)
+
+	m = MountOpt{}
+	assert.NilError(t, m.Set("type=volume,target=/foo,source=foo,volume-nocopy=true"))
+	assert.Equal(t, m.values[0].VolumeOptions != nil, true)
+	assert.Equal(t, m.values[0].VolumeOptions.NoCopy, true)
+
+	m = MountOpt{}
+	assert.NilError(t, m.Set("type=volume,target=/foo,source=foo,volume-nocopy"))
+	assert.Equal(t, m.values[0].VolumeOptions != nil, true)
+	assert.Equal(t, m.values[0].VolumeOptions.NoCopy, true)
+
+	m = MountOpt{}
+	assert.NilError(t, m.Set("type=volume,target=/foo,source=foo,volume-nocopy=1"))
+	assert.Equal(t, m.values[0].VolumeOptions != nil, true)
+	assert.Equal(t, m.values[0].VolumeOptions.NoCopy, true)
+}
+
+func TestMountOptTypeConflict(t *testing.T) {
+	var m MountOpt
+	assert.Error(t, m.Set("type=bind,target=/foo,source=/foo,volume-nocopy=true"), "cannot mix")
+	assert.Error(t, m.Set("type=volume,target=/foo,source=/foo,bind-propagation=rprivate"), "cannot mix")
 }

api/client/service/scale.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/docker/docker/api/client"
 	"github.com/docker/docker/cli"
+	"github.com/docker/engine-api/types"
 	"github.com/spf13/cobra"
 )
 
@@ -77,7 +78,7 @@ func runServiceScale(dockerCli *client.DockerCli, serviceID string, scale string
 	}
 	serviceMode.Replicated.Replicas = &uintScale
 
-	err = client.ServiceUpdate(ctx, service.ID, service.Version, service.Spec, nil)
+	err = client.ServiceUpdate(ctx, service.ID, service.Version, service.Spec, types.ServiceUpdateOptions{})
 	if err != nil {
 		return err
 	}

api/client/service/update.go

@@ -10,6 +10,7 @@ import (
 	"github.com/docker/docker/cli"
 	"github.com/docker/docker/opts"
 	runconfigopts "github.com/docker/docker/runconfig/opts"
+	"github.com/docker/engine-api/types"
 	"github.com/docker/engine-api/types/swarm"
 	"github.com/docker/go-connections/nat"
 	"github.com/spf13/cobra"
@@ -39,7 +40,7 @@ func newUpdateCommand(dockerCli *client.DockerCli) *cobra.Command {
 func runUpdate(dockerCli *client.DockerCli, flags *pflag.FlagSet, serviceID string) error {
 	apiClient := dockerCli.Client()
 	ctx := context.Background()
-	headers := map[string][]string{}
+	updateOpts := types.ServiceUpdateOptions{}
 
 	service, _, err := apiClient.ServiceInspectWithRaw(ctx, serviceID)
 	if err != nil {
@@ -64,10 +65,10 @@ func runUpdate(dockerCli *client.DockerCli, flags *pflag.FlagSet, serviceID stri
 		if err != nil {
 			return err
 		}
-		headers["X-Registry-Auth"] = []string{encodedAuth}
+		updateOpts.EncodedRegistryAuth = encodedAuth
 	}
 
-	err = apiClient.ServiceUpdate(ctx, service.ID, service.Version, service.Spec, headers)
+	err = apiClient.ServiceUpdate(ctx, service.ID, service.Version, service.Spec, updateOpts)
 	if err != nil {
 		return err
 	}

api/client/stack/deploy.go

@@ -184,21 +184,21 @@ func deployServices(
 		if service, exists := existingServiceMap[name]; exists {
 			fmt.Fprintf(out, "Updating service %s (id: %s)\n", name, service.ID)
 
-			// TODO(nishanttotla): Pass headers with X-Registry-Auth
+			// TODO(nishanttotla): Pass auth token
 			if err := apiClient.ServiceUpdate(
 				ctx,
 				service.ID,
 				service.Version,
 				serviceSpec,
-				nil,
+				types.ServiceUpdateOptions{},
 			); err != nil {
 				return err
 			}
 		} else {
 			fmt.Fprintf(out, "Creating service %s\n", name)
 
 			// TODO(nishanttotla): Pass headers with X-Registry-Auth
-			if _, err := apiClient.ServiceCreate(ctx, serviceSpec, nil); err != nil {
+			if _, err := apiClient.ServiceCreate(ctx, serviceSpec, types.ServiceCreateOptions{}); err != nil {
 				return err
 			}
 		}

daemon/cluster/convert/container.go

@@ -26,7 +26,7 @@ func containerSpecFromGRPC(c *swarmapi.ContainerSpec) types.ContainerSpec {
 			Target:   m.Target,
 			Source:   m.Source,
 			Type:     types.MountType(strings.ToLower(swarmapi.Mount_MountType_name[int32(m.Type)])),
-			Writable: m.Writable,
+			ReadOnly: m.ReadOnly,
 		}
 
 		if m.BindOptions != nil {
@@ -37,8 +37,8 @@ func containerSpecFromGRPC(c *swarmapi.ContainerSpec) types.ContainerSpec {
 
 		if m.VolumeOptions != nil {
 			mount.VolumeOptions = &types.VolumeOptions{
-				Populate: m.VolumeOptions.Populate,
-				Labels:   m.VolumeOptions.Labels,
+				NoCopy: m.VolumeOptions.NoCopy,
+				Labels: m.VolumeOptions.Labels,
 			}
 			if m.VolumeOptions.DriverConfig != nil {
 				mount.VolumeOptions.DriverConfig = &types.Driver{
@@ -77,7 +77,7 @@ func containerToGRPC(c types.ContainerSpec) (*swarmapi.ContainerSpec, error) {
 		mount := swarmapi.Mount{
 			Target:   m.Target,
 			Source:   m.Source,
-			Writable: m.Writable,
+			ReadOnly: m.ReadOnly,
 		}
 
 		if mountType, ok := swarmapi.Mount_MountType_value[strings.ToUpper(string(m.Type))]; ok {
@@ -98,8 +98,8 @@ func containerToGRPC(c types.ContainerSpec) (*swarmapi.ContainerSpec, error) {
 
 		if m.VolumeOptions != nil {
 			mount.VolumeOptions = &swarmapi.Mount_VolumeOptions{
-				Populate: m.VolumeOptions.Populate,
-				Labels:   m.VolumeOptions.Labels,
+				NoCopy: m.VolumeOptions.NoCopy,
+				Labels: m.VolumeOptions.Labels,
 			}
 			if m.VolumeOptions.DriverConfig != nil {
 				mount.VolumeOptions.DriverConfig = &swarmapi.Driver{

daemon/cluster/executor/container/container.go

@@ -163,19 +163,23 @@ func (c *containerConfig) bindMounts() []string {
 	var r []string
 
 	for _, val := range c.spec().Mounts {
-		mask := getMountMask(&val)
 		if val.Type == api.MountTypeBind || (val.Type == api.MountTypeVolume && val.Source != "") {
-			r = append(r, fmt.Sprintf("%s:%s:%s", val.Source, val.Target, mask))
+			mask := getMountMask(&val)
+			spec := fmt.Sprintf("%s:%s", val.Source, val.Target)
+			if mask != "" {
+				spec = fmt.Sprintf("%s:%s", spec, mask)
+			}
+			r = append(r, spec)
 		}
 	}
 
 	return r
 }
 
 func getMountMask(m *api.Mount) string {
-	maskOpts := []string{"ro"}
-	if m.Writable {
-		maskOpts[0] = "rw"
+	var maskOpts []string
+	if m.ReadOnly {
+		maskOpts = append(maskOpts, "ro")
 	}
 
 	if m.BindOptions != nil {
@@ -196,7 +200,7 @@ func getMountMask(m *api.Mount) string {
 	}
 
 	if m.VolumeOptions != nil {
-		if !m.VolumeOptions.Populate {
+		if m.VolumeOptions.NoCopy {
 			maskOpts = append(maskOpts, "nocopy")
 		}
 	}

docs/reference/api/docker_remote_api_v1.24.md

@@ -3985,11 +3985,11 @@ JSON Parameters:
             - **Target** – Container path.
             - **Source** – Mount source (e.g. a volume name, a host path).
             - **Type** – The mount type (`bind`, or `volume`).
-            - **Writable** – A boolean indicating whether the mount should be writable.
+            - **ReadOnly** – A boolean indicating whether the mount should be read-only.
             - **BindOptions** - Optional configuration for the `bind` type.
               - **Propagation** – A propagation mode with the value `[r]private`, `[r]shared`, or `[r]slave`.
             - **VolumeOptions** – Optional configuration for the `volume` type.
-                - **Populate** – A boolean indicating if volume should be
+                - **NoCopy** – A boolean indicating if volume should be
                   populated with the data from the target. (Default false)
                 - **Labels** – User-defined name and labels for the volume.
                 - **DriverConfig** – Map of driver-specific options.
@@ -4203,11 +4203,12 @@ Update the service `id`.
             - **Target** – Container path.
             - **Source** – Mount source (e.g. a volume name, a host path).
             - **Type** – The mount type (`bind`, or `volume`).
-            - **Writable** – A boolean indicating whether the mount should be writable.
+            - **ReadOnly** – A boolean indicating whether the mount should be read-only.
             - **BindOptions** - Optional configuration for the `bind` type
               - **Propagation** – A propagation mode with the value `[r]private`, `[r]shared`, or `[r]slave`.
             - **VolumeOptions** – Optional configuration for the `volume` type.
-                - **Populate** – A boolean indicating if volume should be populated with the data from the target. (Default false)
+                - **NoCopy** – A boolean indicating if volume should be
+                  populated with the data from the target. (Default false)
                 - **Labels** – User-defined name and labels for the volume.
                 - **DriverConfig** – Map of driver-specific options.
                   - **Name** - Name of the driver to use to create the volume

docs/reference/api/docker_remote_api_v1.25.md

@@ -3986,11 +3986,11 @@ JSON Parameters:
             - **Target** – Container path.
             - **Source** – Mount source (e.g. a volume name, a host path).
             - **Type** – The mount type (`bind`, or `volume`).
-            - **Writable** – A boolean indicating whether the mount should be writable.
+            - **ReadOnly** – A boolean indicating whether the mount should be read-only.
             - **BindOptions** - Optional configuration for the `bind` type.
               - **Propagation** – A propagation mode with the value `[r]private`, `[r]shared`, or `[r]slave`.
             - **VolumeOptions** – Optional configuration for the `volume` type.
-                - **Populate** – A boolean indicating if volume should be
+                - **NoCopy** – A boolean indicating if volume should be
                   populated with the data from the target. (Default false)
                 - **Labels** – User-defined name and labels for the volume.
                 - **DriverConfig** – Map of driver-specific options.
@@ -4204,11 +4204,12 @@ Update the service `id`.
             - **Target** – Container path.
             - **Source** – Mount source (e.g. a volume name, a host path).
             - **Type** – The mount type (`bind`, or `volume`).
-            - **Writable** – A boolean indicating whether the mount should be writable.
+            - **ReadOnly** – A boolean indicating whether the mount should be read-only.
             - **BindOptions** - Optional configuration for the `bind` type
               - **Propagation** – A propagation mode with the value `[r]private`, `[r]shared`, or `[r]slave`.
             - **VolumeOptions** – Optional configuration for the `volume` type.
-                - **Populate** – A boolean indicating if volume should be populated with the data from the target. (Default false)
+                - **NoCopy** – A boolean indicating if volume should be
+                  populated with the data from the target. (Default false)
                 - **Labels** – User-defined name and labels for the volume.
                 - **DriverConfig** – Map of driver-specific options.
                   - **Name** - Name of the driver to use to create the volume

hack/vendor.sh

@@ -60,7 +60,7 @@ clone git golang.org/x/net 2beffdc2e92c8a3027590f898fe88f69af48a3f8 https://gith
 clone git golang.org/x/sys eb2c74142fd19a79b3f237334c7384d5167b1b46 https://github.com/golang/sys.git
 clone git github.com/docker/go-units 651fc226e7441360384da338d0fd37f2440ffbe3
 clone git github.com/docker/go-connections fa2850ff103453a9ad190da0df0af134f0314b3d
-clone git github.com/docker/engine-api 62043eb79d581a32ea849645277023c550732e52
+clone git github.com/docker/engine-api 139c221fcbe6e67dfac3c8807870e7136884a45b
 clone git github.com/RackSec/srslog 259aed10dfa74ea2961eddd1d9847619f6e98837
 clone git github.com/imdario/mergo 0.2.1
 
@@ -139,7 +139,7 @@ clone git github.com/docker/docker-credential-helpers v0.3.0
 clone git github.com/docker/containerd 1b3a81545ca79456086dc2aa424357be98b962ee
 
 # cluster
-clone git github.com/docker/swarmkit 036a4a1e934bd1bbb35c3ec7f85dea2ba6d4e336
+clone git github.com/docker/swarmkit 24eaf0021a2eea7938fce7493ce4731f53c2b87c
 clone git github.com/golang/mock bd3c8e81be01eef76d4b503f5e687d2d1354d2d9
 clone git github.com/gogo/protobuf 43a2e0b1c32252bfbbdf81f7faa7a88fb3fa4028
 clone git github.com/cloudflare/cfssl b895b0549c0ff676f92cf09ba971ae02bb41367b

integration-cli/docker_cli_service_create_hack_test.go

@@ -41,5 +41,5 @@ func (s *DockerSwarmSuite) TestServiceCreateMountVolume(c *check.C) {
 
 	c.Assert(mounts[0].Name, checker.Equals, "foo")
 	c.Assert(mounts[0].Destination, checker.Equals, "/foo")
-	c.Assert(mounts[0].RW, checker.Equals, false)
+	c.Assert(mounts[0].RW, checker.Equals, true)
 }