Remove `-ptr` from the help output of `service create`

This fix is based on the comment:
https://github.com/docker/docker/pull/28147#discussion_r86996347

Previously the output string of the `DurationOpt` is `duration-ptr`
and `Uint64Opt` is `uint64-ptr`. While it is clear to developers,
for a normal user `-ptr` might not be very informative.

On the other hand, the default value of `DurationOpt` and `Uint64Opt`
has already been quite informative: `none`. That means if no flag
provided, the value will be treated as none.
(like a ptr with nil as the default)

For that reason this fix removes the `-ptr`.

Also, the output in the docs of `service create` has been quite
out-of-sync with the true output. So this fix updates the docs
to have the most up-to-date help output of `service create --help`.

This fix is related to #28147.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

command/service/create.go

@@ -37,10 +37,10 @@ func newCreateCommand(dockerCli *command.DockerCli) *cobra.Command {
 	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 filesystem mount to the service")
-	flags.StringSliceVar(&opts.constraints, flagConstraint, []string{}, "Placement constraints")
+	flags.Var(&opts.constraints, flagConstraint, "Placement constraints")
-	flags.StringSliceVar(&opts.networks, flagNetwork, []string{}, "Network attachments")
+	flags.Var(&opts.networks, flagNetwork, "Network attachments")
 	flags.VarP(&opts.endpoint.ports, flagPublish, "p", "Publish a port as a node port")
-	flags.StringSliceVar(&opts.groups, flagGroup, []string{}, "Set one or more supplementary user groups for the container")
+	flags.Var(&opts.groups, flagGroup, "Set one or more supplementary user groups for the container")
 	flags.Var(&opts.dns, flagDNS, "Set custom DNS servers")
 	flags.Var(&opts.dnsOptions, flagDNSOptions, "Set DNS options")
 	flags.Var(&opts.dnsSearch, flagDNSSearch, "Set custom DNS search domains")

command/service/opts.go

@@ -32,7 +32,7 @@ func (m *memBytes) Set(value string) error {
 }
 
 func (m *memBytes) Type() string {
-	return "MemoryBytes"
+	return "bytes"
 }
 
 func (m *memBytes) Value() int64 {
@@ -71,9 +71,9 @@ func (d *DurationOpt) Set(s string) error {
 	return err
 }
 
-// Type returns the type of this option
+// Type returns the type of this option, which will be displayed in `--help` output
 func (d *DurationOpt) Type() string {
-	return "duration-ptr"
+	return "duration"
 }
 
 // String returns a string repr of this option
@@ -101,9 +101,9 @@ func (i *Uint64Opt) Set(s string) error {
 	return err
 }
 
-// Type returns the type of this option
+// Type returns the type of this option, which will be displayed in `--help` output
 func (i *Uint64Opt) Type() string {
-	return "uint64-ptr"
+	return "uint"
 }
 
 // String returns a string repr of this option
@@ -119,12 +119,32 @@ func (i *Uint64Opt) Value() *uint64 {
 	return i.value
 }
 
+type floatValue float32
+
+func (f *floatValue) Set(s string) error {
+	v, err := strconv.ParseFloat(s, 32)
+	*f = floatValue(v)
+	return err
+}
+
+func (f *floatValue) Type() string {
+	return "float"
+}
+
+func (f *floatValue) String() string {
+	return strconv.FormatFloat(float64(*f), 'g', -1, 32)
+}
+
+func (f *floatValue) Value() float32 {
+	return float32(*f)
+}
+
 type updateOptions struct {
 	parallelism     uint64
 	delay           time.Duration
 	monitor         time.Duration
 	onFailure       string
-	maxFailureRatio float32
+	maxFailureRatio floatValue
 }
 
 type resourceOptions struct {
@@ -293,7 +313,7 @@ type serviceOptions struct {
 	envFile         opts.ListOpts
 	workdir         string
 	user            string
-	groups          []string
+	groups          opts.ListOpts
 	tty             bool
 	mounts          opts.MountOpt
 	dns             opts.ListOpts
@@ -307,9 +327,9 @@ type serviceOptions struct {
 	mode     string
 
 	restartPolicy restartPolicyOptions
-	constraints   []string
+	constraints   opts.ListOpts
 	update        updateOptions
-	networks      []string
+	networks      opts.ListOpts
 	endpoint      endpointOptions
 
 	registryAuth bool
@@ -322,16 +342,19 @@ type serviceOptions struct {
 func newServiceOptions() *serviceOptions {
 	return &serviceOptions{
 		labels:          opts.NewListOpts(runconfigopts.ValidateEnv),
+		constraints:     opts.NewListOpts(nil),
 		containerLabels: opts.NewListOpts(runconfigopts.ValidateEnv),
 		env:             opts.NewListOpts(runconfigopts.ValidateEnv),
 		envFile:         opts.NewListOpts(nil),
 		endpoint: endpointOptions{
 			ports: opts.NewListOpts(ValidatePort),
 		},
+		groups:     opts.NewListOpts(nil),
 		logDriver:  newLogDriverOptions(),
 		dns:        opts.NewListOpts(opts.ValidateIPAddress),
 		dnsOptions: opts.NewListOpts(nil),
 		dnsSearch:  opts.NewListOpts(opts.ValidateDNSSearch),
+		networks:   opts.NewListOpts(nil),
 	}
 }
 
@@ -371,7 +394,7 @@ func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) {
 				Labels:   runconfigopts.ConvertKVStringsToMap(opts.containerLabels.GetAll()),
 				Dir:      opts.workdir,
 				User:     opts.user,
-				Groups:   opts.groups,
+				Groups:   opts.groups.GetAll(),
 				TTY:      opts.tty,
 				Mounts:   opts.mounts.Value(),
 				DNSConfig: &swarm.DNSConfig{
@@ -381,22 +404,22 @@ func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) {
 				},
 				StopGracePeriod: opts.stopGrace.Value(),
 			},
-			Networks:      convertNetworks(opts.networks),
+			Networks:      convertNetworks(opts.networks.GetAll()),
 			Resources:     opts.resources.ToResourceRequirements(),
 			RestartPolicy: opts.restartPolicy.ToRestartPolicy(),
 			Placement: &swarm.Placement{
-				Constraints: opts.constraints,
+				Constraints: opts.constraints.GetAll(),
 			},
 			LogDriver: opts.logDriver.toLogDriver(),
 		},
-		Networks: convertNetworks(opts.networks),
+		Networks: convertNetworks(opts.networks.GetAll()),
 		Mode:     swarm.ServiceMode{},
 		UpdateConfig: &swarm.UpdateConfig{
 			Parallelism:     opts.update.parallelism,
 			Delay:           opts.update.delay,
 			Monitor:         opts.update.monitor,
 			FailureAction:   opts.update.onFailure,
-			MaxFailureRatio: opts.update.maxFailureRatio,
+			MaxFailureRatio: opts.update.maxFailureRatio.Value(),
 		},
 		EndpointSpec: opts.endpoint.ToEndpointSpec(),
 	}
@@ -449,7 +472,7 @@ func addServiceFlags(cmd *cobra.Command, opts *serviceOptions) {
 	flags.DurationVar(&opts.update.delay, flagUpdateDelay, time.Duration(0), "Delay between updates (ns|us|ms|s|m|h) (default 0s)")
 	flags.DurationVar(&opts.update.monitor, flagUpdateMonitor, time.Duration(0), "Duration after each task update to monitor for failure (ns|us|ms|s|m|h) (default 0s)")
 	flags.StringVar(&opts.update.onFailure, flagUpdateFailureAction, "pause", "Action on update failure (pause|continue)")
-	flags.Float32Var(&opts.update.maxFailureRatio, flagUpdateMaxFailureRatio, 0, "Failure rate to tolerate during an update")
+	flags.Var(&opts.update.maxFailureRatio, flagUpdateMaxFailureRatio, "Failure rate to tolerate during an update")
 
 	flags.StringVar(&opts.endpoint.mode, flagEndpointMode, "", "Endpoint mode (vip or dnsrr)")
 

command/service/update.go

@@ -55,9 +55,9 @@ func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command {
 	flags.Var(&opts.containerLabels, flagContainerLabelAdd, "Add or update a container label")
 	flags.Var(&opts.env, flagEnvAdd, "Add or update an environment variable")
 	flags.Var(&opts.mounts, flagMountAdd, "Add or update a mount on a service")
-	flags.StringSliceVar(&opts.constraints, flagConstraintAdd, []string{}, "Add or update a placement constraint")
+	flags.Var(&opts.constraints, flagConstraintAdd, "Add or update a placement constraint")
 	flags.Var(&opts.endpoint.ports, flagPublishAdd, "Add or update a published port")
-	flags.StringSliceVar(&opts.groups, flagGroupAdd, []string{}, "Add an additional supplementary user group to the container")
+	flags.Var(&opts.groups, flagGroupAdd, "Add an additional supplementary user group to the container")
 	flags.Var(&opts.dns, flagDNSAdd, "Add or update custom DNS servers")
 	flags.Var(&opts.dnsOptions, flagDNSOptionsAdd, "Add or update DNS options")
 	flags.Var(&opts.dnsSearch, flagDNSSearchAdd, "Add or update custom DNS search domains")
@@ -139,9 +139,9 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error {
 		}
 	}
 
-	updateFloat32 := func(flag string, field *float32) {
+	updateFloatValue := func(flag string, field *float32) {
 		if flags.Changed(flag) {
-			*field, _ = flags.GetFloat32(flag)
+			*field = flags.Lookup(flag).Value.(*floatValue).Value()
 		}
 	}
 
@@ -238,7 +238,7 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error {
 		updateDuration(flagUpdateDelay, &spec.UpdateConfig.Delay)
 		updateDuration(flagUpdateMonitor, &spec.UpdateConfig.Monitor)
 		updateString(flagUpdateFailureAction, &spec.UpdateConfig.FailureAction)
-		updateFloat32(flagUpdateMaxFailureRatio, &spec.UpdateConfig.MaxFailureRatio)
+		updateFloatValue(flagUpdateMaxFailureRatio, &spec.UpdateConfig.MaxFailureRatio)
 	}
 
 	if flags.Changed(flagEndpointMode) {
@@ -322,11 +322,22 @@ func anyChanged(flags *pflag.FlagSet, fields ...string) bool {
 }
 
 func updatePlacement(flags *pflag.FlagSet, placement *swarm.Placement) {
-	field, _ := flags.GetStringSlice(flagConstraintAdd)
+	if flags.Changed(flagConstraintAdd) {
-	placement.Constraints = append(placement.Constraints, field...)
+		values := flags.Lookup(flagConstraintAdd).Value.(*opts.ListOpts).GetAll()
-
+		placement.Constraints = append(placement.Constraints, values...)
+	}
 	toRemove := buildToRemoveSet(flags, flagConstraintRemove)
-	placement.Constraints = removeItems(placement.Constraints, toRemove, itemKey)
+
+	newConstraints := []string{}
+	for _, constraint := range placement.Constraints {
+		if _, exists := toRemove[constraint]; !exists {
+			newConstraints = append(newConstraints, constraint)
+		}
+	}
+	// Sort so that result is predictable.
+	sort.Strings(newConstraints)
+
+	placement.Constraints = newConstraints
 }
 
 func updateContainerLabels(flags *pflag.FlagSet, field *map[string]string) {
@@ -479,10 +490,7 @@ func updateMounts(flags *pflag.FlagSet, mounts *[]mounttypes.Mount) error {
 
 func updateGroups(flags *pflag.FlagSet, groups *[]string) error {
 	if flags.Changed(flagGroupAdd) {
-		values, err := flags.GetStringSlice(flagGroupAdd)
+		values := flags.Lookup(flagGroupAdd).Value.(*opts.ListOpts).GetAll()
-		if err != nil {
-			return err
-		}
 		*groups = append(*groups, values...)
 	}
 	toRemove := buildToRemoveSet(flags, flagGroupRemove)