diff --git a/command/service/create.go b/command/service/create.go index 28790ec8e6..92cf969b4b 100644 --- a/command/service/create.go +++ b/command/service/create.go @@ -35,7 +35,7 @@ func newCreateCommand(dockerCli *command.DockerCli) *cobra.Command { flags.Var(&opts.containerLabels, flagContainerLabel, "Container labels") flags.VarP(&opts.env, flagEnv, "e", "Set environment variables") flags.Var(&opts.envFile, flagEnvFile, "Read in a file of environment variables") - flags.Var(&opts.mounts, flagMount, "Attach a mount to the service") + flags.Var(&opts.mounts, flagMount, "Attach a filesystem mount to the service") flags.StringSliceVar(&opts.constraints, flagConstraint, []string{}, "Placement constraints") flags.StringSliceVar(&opts.networks, flagNetwork, []string{}, "Network attachments") flags.VarP(&opts.endpoint.ports, flagPublish, "p", "Publish a port as a node port") diff --git a/command/service/opts.go b/command/service/opts.go index 43b7b671cb..358185c0b4 100644 --- a/command/service/opts.go +++ b/command/service/opts.go @@ -1,7 +1,6 @@ package service import ( - "encoding/csv" "fmt" "math/big" "strconv" @@ -9,7 +8,6 @@ import ( "time" "github.com/docker/docker/api/types/container" - mounttypes "github.com/docker/docker/api/types/mount" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/opts" runconfigopts "github.com/docker/docker/runconfig/opts" @@ -149,143 +147,6 @@ func (i *Uint64Opt) Value() *uint64 { return i.value } -// MountOpt is a Value type for parsing mounts -type MountOpt struct { - values []mounttypes.Mount -} - -// Set a new mount value -func (m *MountOpt) Set(value string) error { - csvReader := csv.NewReader(strings.NewReader(value)) - fields, err := csvReader.Read() - if err != nil { - return err - } - - mount := mounttypes.Mount{} - - volumeOptions := func() *mounttypes.VolumeOptions { - if mount.VolumeOptions == nil { - mount.VolumeOptions = &mounttypes.VolumeOptions{ - Labels: make(map[string]string), - } - } - if mount.VolumeOptions.DriverConfig == nil { - mount.VolumeOptions.DriverConfig = &mounttypes.Driver{} - } - return mount.VolumeOptions - } - - bindOptions := func() *mounttypes.BindOptions { - if mount.BindOptions == nil { - mount.BindOptions = new(mounttypes.BindOptions) - } - return mount.BindOptions - } - - setValueOnMap := func(target map[string]string, value string) { - parts := strings.SplitN(value, "=", 2) - if len(parts) == 1 { - target[value] = "" - } else { - target[parts[0]] = parts[1] - } - } - - mount.Type = mounttypes.TypeVolume // default to volume mounts - // Set writable as the default - for _, field := range fields { - parts := strings.SplitN(field, "=", 2) - key := strings.ToLower(parts[0]) - - if len(parts) == 1 { - switch key { - case "readonly", "ro": - mount.ReadOnly = true - continue - case "volume-nocopy": - volumeOptions().NoCopy = true - continue - } - } - - if len(parts) != 2 { - return fmt.Errorf("invalid field '%s' must be a key=value pair", field) - } - - value := parts[1] - switch key { - case "type": - mount.Type = mounttypes.Type(strings.ToLower(value)) - case "source", "src": - mount.Source = value - case "target", "dst", "destination": - mount.Target = value - case "readonly", "ro": - mount.ReadOnly, err = strconv.ParseBool(value) - if err != nil { - return fmt.Errorf("invalid value for %s: %s", key, value) - } - case "bind-propagation": - bindOptions().Propagation = mounttypes.Propagation(strings.ToLower(value)) - case "volume-nocopy": - volumeOptions().NoCopy, err = strconv.ParseBool(value) - if err != nil { - return fmt.Errorf("invalid value for populate: %s", value) - } - case "volume-label": - setValueOnMap(volumeOptions().Labels, value) - case "volume-driver": - volumeOptions().DriverConfig.Name = value - case "volume-opt": - if volumeOptions().DriverConfig.Options == nil { - volumeOptions().DriverConfig.Options = make(map[string]string) - } - setValueOnMap(volumeOptions().DriverConfig.Options, value) - default: - return fmt.Errorf("unexpected key '%s' in '%s'", key, field) - } - } - - if mount.Type == "" { - return fmt.Errorf("type is required") - } - - if mount.Target == "" { - return fmt.Errorf("target is required") - } - - if mount.Type == mounttypes.TypeBind && mount.VolumeOptions != nil { - return fmt.Errorf("cannot mix 'volume-*' options with mount type '%s'", mounttypes.TypeBind) - } - if mount.Type == mounttypes.TypeVolume && mount.BindOptions != nil { - return fmt.Errorf("cannot mix 'bind-*' options with mount type '%s'", mounttypes.TypeVolume) - } - - m.values = append(m.values, mount) - return nil -} - -// Type returns the type of this option -func (m *MountOpt) Type() string { - return "mount" -} - -// String returns a string repr of this option -func (m *MountOpt) String() string { - mounts := []string{} - for _, mount := range m.values { - repr := fmt.Sprintf("%s %s %s", mount.Type, mount.Source, mount.Target) - mounts = append(mounts, repr) - } - return strings.Join(mounts, ", ") -} - -// Value returns the mounts -func (m *MountOpt) Value() []mounttypes.Mount { - return m.values -} - type updateOptions struct { parallelism uint64 delay time.Duration @@ -460,7 +321,7 @@ type serviceOptions struct { workdir string user string groups []string - mounts MountOpt + mounts opts.MountOpt resources resourceOptions stopGrace DurationOpt diff --git a/command/service/opts_test.go b/command/service/opts_test.go index 52016cbfc5..26534cf0f5 100644 --- a/command/service/opts_test.go +++ b/command/service/opts_test.go @@ -6,7 +6,6 @@ import ( "time" "github.com/docker/docker/api/types/container" - mounttypes "github.com/docker/docker/api/types/mount" "github.com/docker/docker/pkg/testutil/assert" ) @@ -68,151 +67,6 @@ func TestUint64OptSetAndValue(t *testing.T) { assert.Equal(t, *opt.Value(), uint64(14445)) } -func TestMountOptString(t *testing.T) { - mount := MountOpt{ - values: []mounttypes.Mount{ - { - Type: mounttypes.TypeBind, - Source: "/home/path", - Target: "/target", - }, - { - Type: mounttypes.TypeVolume, - Source: "foo", - Target: "/target/foo", - }, - }, - } - expected := "bind /home/path /target, volume foo /target/foo" - assert.Equal(t, mount.String(), expected) -} - -func TestMountOptSetBindNoErrorBind(t *testing.T) { - for _, testcase := range []string{ - // tests several aliases that should have same result. - "type=bind,target=/target,source=/source", - "type=bind,src=/source,dst=/target", - "type=bind,source=/source,dst=/target", - "type=bind,src=/source,target=/target", - } { - var mount MountOpt - - assert.NilError(t, mount.Set(testcase)) - - mounts := mount.Value() - assert.Equal(t, len(mounts), 1) - assert.Equal(t, mounts[0], mounttypes.Mount{ - Type: mounttypes.TypeBind, - Source: "/source", - Target: "/target", - }) - } -} - -func TestMountOptSetVolumeNoError(t *testing.T) { - for _, testcase := range []string{ - // tests several aliases that should have same result. - "type=volume,target=/target,source=/source", - "type=volume,src=/source,dst=/target", - "type=volume,source=/source,dst=/target", - "type=volume,src=/source,target=/target", - } { - var mount MountOpt - - assert.NilError(t, mount.Set(testcase)) - - mounts := mount.Value() - assert.Equal(t, len(mounts), 1) - assert.Equal(t, mounts[0], mounttypes.Mount{ - Type: mounttypes.TypeVolume, - Source: "/source", - Target: "/target", - }) - } -} - -// TestMountOptDefaultType ensures that a mount without the type defaults to a -// volume mount. -func TestMountOptDefaultType(t *testing.T) { - var mount MountOpt - assert.NilError(t, mount.Set("target=/target,source=/foo")) - assert.Equal(t, mount.values[0].Type, mounttypes.TypeVolume) -} - -func TestMountOptSetErrorNoTarget(t *testing.T) { - var mount MountOpt - assert.Error(t, mount.Set("type=volume,source=/foo"), "target is required") -} - -func TestMountOptSetErrorInvalidKey(t *testing.T) { - var mount MountOpt - assert.Error(t, mount.Set("type=volume,bogus=foo"), "unexpected key 'bogus'") -} - -func TestMountOptSetErrorInvalidField(t *testing.T) { - var mount MountOpt - assert.Error(t, mount.Set("type=volume,bogus"), "invalid field 'bogus'") -} - -func TestMountOptSetErrorInvalidReadOnly(t *testing.T) { - var mount MountOpt - assert.Error(t, mount.Set("type=volume,readonly=no"), "invalid value for readonly: no") - assert.Error(t, mount.Set("type=volume,readonly=invalid"), "invalid value for readonly: invalid") -} - -func TestMountOptDefaultEnableReadOnly(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=true")) - 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.NilError(t, m.Set("type=volume,target=/foo,volume-nocopy")) - assert.Equal(t, m.values[0].Source, "") - - 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") -} - func TestHealthCheckOptionsToHealthConfig(t *testing.T) { dur := time.Second opt := healthCheckOptions{ diff --git a/command/service/update.go b/command/service/update.go index b76a20e97c..a9aa9c9987 100644 --- a/command/service/update.go +++ b/command/service/update.go @@ -404,7 +404,7 @@ func removeItems( func updateMounts(flags *pflag.FlagSet, mounts *[]mounttypes.Mount) { if flags.Changed(flagMountAdd) { - values := flags.Lookup(flagMountAdd).Value.(*MountOpt).Value() + values := flags.Lookup(flagMountAdd).Value.(*opts.MountOpt).Value() *mounts = append(*mounts, values...) } toRemove := buildToRemoveSet(flags, flagMountRemove)