From 4c6faa434071b87a55256e86020cb78495e9951d Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 29 Jun 2016 12:28:33 -0400 Subject: [PATCH] Change the add/update flags to include 'add' Signed-off-by: Daniel Nephin --- api/client/service/create.go | 10 +++- api/client/service/opts.go | 25 ++++----- api/client/service/update.go | 56 +++++++++++-------- api/client/service/update_test.go | 24 ++++---- .../docker_cli_service_update_test.go | 2 +- 5 files changed, 68 insertions(+), 49 deletions(-) diff --git a/api/client/service/create.go b/api/client/service/create.go index be707c5f5d..f6210be288 100644 --- a/api/client/service/create.go +++ b/api/client/service/create.go @@ -28,7 +28,15 @@ func newCreateCommand(dockerCli *client.DockerCli) *cobra.Command { flags := cmd.Flags() flags.StringVar(&opts.mode, flagMode, "replicated", "Service mode (replicated or global)") addServiceFlags(cmd, opts) - cmd.Flags().SetInterspersed(false) + + flags.VarP(&opts.labels, flagLabel, "l", "Service labels") + flags.VarP(&opts.env, flagEnv, "e", "Set environment variables") + flags.Var(&opts.mounts, flagMount, "Attach a 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") + + flags.SetInterspersed(false) return cmd } diff --git a/api/client/service/opts.go b/api/client/service/opts.go index 93dbc3ba6c..7c0b0761c6 100644 --- a/api/client/service/opts.go +++ b/api/client/service/opts.go @@ -459,12 +459,9 @@ func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) { func addServiceFlags(cmd *cobra.Command, opts *serviceOptions) { flags := cmd.Flags() flags.StringVar(&opts.name, flagName, "", "Service name") - flags.VarP(&opts.labels, flagLabel, "l", "Service labels") - flags.VarP(&opts.env, flagEnv, "e", "Set environment variables") flags.StringVarP(&opts.workdir, "workdir", "w", "", "Working directory inside the container") flags.StringVarP(&opts.user, flagUser, "u", "", "Username or UID") - flags.Var(&opts.mounts, flagMount, "Attach a mount to the service") flags.Var(&opts.resources.limitCPU, flagLimitCPU, "Limit CPUs") flags.Var(&opts.resources.limitMemBytes, flagLimitMemory, "Limit Memory") @@ -479,36 +476,38 @@ func addServiceFlags(cmd *cobra.Command, opts *serviceOptions) { flags.Var(&opts.restartPolicy.maxAttempts, flagRestartMaxAttempts, "Maximum number of restarts before giving up") flags.Var(&opts.restartPolicy.window, flagRestartWindow, "Window used to evaluate the restart policy") - flags.StringSliceVar(&opts.constraints, flagConstraint, []string{}, "Placement constraints") - flags.Uint64Var(&opts.update.parallelism, flagUpdateParallelism, 0, "Maximum number of tasks updated simultaneously") flags.DurationVar(&opts.update.delay, flagUpdateDelay, time.Duration(0), "Delay between updates") - flags.StringSliceVar(&opts.networks, flagNetwork, []string{}, "Network attachments") flags.StringVar(&opts.endpoint.mode, flagEndpointMode, "", "Endpoint mode (vip or dnsrr)") - flags.VarP(&opts.endpoint.ports, flagPublish, "p", "Publish a port as a node port") flags.BoolVar(&opts.registryAuth, flagRegistryAuth, false, "Send registry authentication details to Swarm agents") } const ( flagConstraint = "constraint" - flagConstraintRemove = "remove-constraint" + flagConstraintRemove = "constraint-rm" + flagConstraintAdd = "constraint-add" flagEndpointMode = "endpoint-mode" flagEnv = "env" - flagEnvRemove = "remove-env" + flagEnvRemove = "env-rm" + flagEnvAdd = "env-add" flagLabel = "label" - flagLabelRemove = "remove-label" + flagLabelRemove = "label-rm" + flagLabelAdd = "label-add" flagLimitCPU = "limit-cpu" flagLimitMemory = "limit-memory" flagMode = "mode" flagMount = "mount" - flagMountRemove = "remove-mount" + flagMountRemove = "mount-rm" + flagMountAdd = "mount-add" flagName = "name" flagNetwork = "network" - flagNetworkRemove = "remove-network" + flagNetworkRemove = "network-rm" + flagNetworkAdd = "network-add" flagPublish = "publish" - flagPublishRemove = "remove-publish" + flagPublishRemove = "publish-rm" + flagPublishAdd = "publish-add" flagReplicas = "replicas" flagReserveCPU = "reserve-cpu" flagReserveMemory = "reserve-memory" diff --git a/api/client/service/update.go b/api/client/service/update.go index 6af1a47d9d..74b0c85dfc 100644 --- a/api/client/service/update.go +++ b/api/client/service/update.go @@ -35,15 +35,26 @@ func newUpdateCommand(dockerCli *client.DockerCli) *cobra.Command { flags.String("image", "", "Service image tag") flags.String("args", "", "Service command args") addServiceFlags(cmd, opts) - flags.StringSlice(flagEnvRemove, []string{}, "Remove an environment variable") - flags.StringSlice(flagLabelRemove, []string{}, "Remove a label by its key") - flags.StringSlice(flagMountRemove, []string{}, "Remove a mount by its target path") - flags.StringSlice(flagPublishRemove, []string{}, "Remove a published port by its target port") - flags.StringSlice(flagNetworkRemove, []string{}, "Remove a network by name") - flags.StringSlice(flagConstraintRemove, []string{}, "Remove a constraint") + + flags.Var(newListOptsVar(), flagEnvRemove, "Remove an environment variable") + flags.Var(newListOptsVar(), flagLabelRemove, "Remove a label by its key") + flags.Var(newListOptsVar(), flagMountRemove, "Remove a mount by its target path") + flags.Var(newListOptsVar(), flagPublishRemove, "Remove a published port by its target port") + flags.Var(newListOptsVar(), flagNetworkRemove, "Remove a network by name") + flags.Var(newListOptsVar(), flagConstraintRemove, "Remove a constraint") + flags.Var(&opts.labels, flagLabelAdd, "Add or update service labels") + flags.Var(&opts.env, flagEnvAdd, "Add or update environment variables") + flags.Var(&opts.mounts, flagMountAdd, "Add or update a mount on a service") + flags.StringSliceVar(&opts.constraints, flagConstraintAdd, []string{}, "Add or update placement constraints") + flags.StringSliceVar(&opts.networks, flagNetworkAdd, []string{}, "Add or update network attachments") + flags.Var(&opts.endpoint.ports, flagPublishAdd, "Add or update a published port") return cmd } +func newListOptsVar() *opts.ListOpts { + return opts.NewListOptsRef(&[]string{}, nil) +} + func runUpdate(dockerCli *client.DockerCli, flags *pflag.FlagSet, serviceID string) error { apiClient := dockerCli.Client() ctx := context.Background() @@ -176,7 +187,7 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error { updateDurationOpt((flagRestartWindow), task.RestartPolicy.Window) } - if flags.Changed(flagConstraint) { + if anyChanged(flags, flagConstraintAdd, flagConstraintRemove) { if task.Placement == nil { task.Placement = &swarm.Placement{} } @@ -201,7 +212,7 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error { spec.EndpointSpec.Mode = swarm.ResolutionMode(value) } - if flags.Changed(flagPublish) { + if anyChanged(flags, flagPublishAdd, flagPublishRemove) { if spec.EndpointSpec == nil { spec.EndpointSpec = &swarm.EndpointSpec{} } @@ -231,7 +242,7 @@ func anyChanged(flags *pflag.FlagSet, fields ...string) bool { } func updatePlacement(flags *pflag.FlagSet, placement *swarm.Placement) { - field, _ := flags.GetStringSlice(flagConstraint) + field, _ := flags.GetStringSlice(flagConstraintAdd) constraints := &placement.Constraints placement.Constraints = append(placement.Constraints, field...) @@ -244,19 +255,19 @@ func updatePlacement(flags *pflag.FlagSet, placement *swarm.Placement) { } func updateLabels(flags *pflag.FlagSet, field *map[string]string) { - if flags.Changed(flagLabel) { + if flags.Changed(flagLabelAdd) { if field == nil { *field = map[string]string{} } - values := flags.Lookup(flagLabel).Value.(*opts.ListOpts).GetAll() + values := flags.Lookup(flagLabelAdd).Value.(*opts.ListOpts).GetAll() for key, value := range runconfigopts.ConvertKVStringsToMap(values) { (*field)[key] = value } } if field != nil && flags.Changed(flagLabelRemove) { - toRemove, _ := flags.GetStringSlice(flagLabelRemove) + toRemove := flags.Lookup(flagLabelRemove).Value.(*opts.ListOpts).GetAll() for _, label := range toRemove { delete(*field, label) } @@ -264,8 +275,8 @@ func updateLabels(flags *pflag.FlagSet, field *map[string]string) { } func updateEnvironment(flags *pflag.FlagSet, field *[]string) { - if flags.Changed(flagEnv) { - value := flags.Lookup(flagEnv).Value.(*opts.ListOpts) + if flags.Changed(flagEnvAdd) { + value := flags.Lookup(flagEnvAdd).Value.(*opts.ListOpts) *field = append(*field, value.GetAll()...) } toRemove := buildToRemoveSet(flags, flagEnvRemove) @@ -290,7 +301,7 @@ func buildToRemoveSet(flags *pflag.FlagSet, flag string) map[string]struct{} { return toRemove } - toRemoveSlice, _ := flags.GetStringSlice(flag) + toRemoveSlice := flags.Lookup(flag).Value.(*opts.ListOpts).GetAll() for _, key := range toRemoveSlice { toRemove[key] = empty } @@ -298,8 +309,8 @@ func buildToRemoveSet(flags *pflag.FlagSet, flag string) map[string]struct{} { } func updateMounts(flags *pflag.FlagSet, mounts *[]swarm.Mount) { - if flags.Changed(flagMount) { - values := flags.Lookup(flagMount).Value.(*MountOpt).Value() + if flags.Changed(flagMountAdd) { + values := flags.Lookup(flagMountAdd).Value.(*MountOpt).Value() *mounts = append(*mounts, values...) } toRemove := buildToRemoveSet(flags, flagMountRemove) @@ -311,8 +322,8 @@ func updateMounts(flags *pflag.FlagSet, mounts *[]swarm.Mount) { } func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) { - if flags.Changed(flagPublish) { - values := flags.Lookup(flagPublish).Value.(*opts.ListOpts).GetAll() + if flags.Changed(flagPublishAdd) { + values := flags.Lookup(flagPublishAdd).Value.(*opts.ListOpts).GetAll() ports, portBindings, _ := nat.ParsePortSpecs(values) for port := range ports { @@ -321,13 +332,14 @@ func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) { } if flags.Changed(flagPublishRemove) { - toRemove, _ := flags.GetStringSlice(flagPublishRemove) + toRemove := flags.Lookup(flagPublishRemove).Value.(*opts.ListOpts).GetAll() for _, rawTargetPort := range toRemove { targetPort := nat.Port(rawTargetPort) for i, port := range *portConfig { if string(port.Protocol) == targetPort.Proto() && port.TargetPort == uint32(targetPort.Int()) { *portConfig = append((*portConfig)[:i], (*portConfig)[i+1:]...) + break } } } @@ -335,8 +347,8 @@ func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) { } func updateNetworks(flags *pflag.FlagSet, attachments *[]swarm.NetworkAttachmentConfig) { - if flags.Changed(flagNetwork) { - networks, _ := flags.GetStringSlice(flagNetwork) + if flags.Changed(flagNetworkAdd) { + networks, _ := flags.GetStringSlice(flagNetworkAdd) for _, network := range networks { *attachments = append(*attachments, swarm.NetworkAttachmentConfig{Target: network}) } diff --git a/api/client/service/update_test.go b/api/client/service/update_test.go index 4a2ccc6169..86321e3de7 100644 --- a/api/client/service/update_test.go +++ b/api/client/service/update_test.go @@ -21,8 +21,8 @@ func TestUpdateServiceArgs(t *testing.T) { func TestUpdateLabels(t *testing.T) { flags := newUpdateCommand(nil).Flags() - flags.Set("label", "toadd=newlabel") - flags.Set("remove-label", "toremove") + flags.Set("label-add", "toadd=newlabel") + flags.Set("label-rm", "toremove") labels := map[string]string{ "toremove": "thelabeltoremove", @@ -37,8 +37,8 @@ func TestUpdateLabels(t *testing.T) { func TestUpdatePlacement(t *testing.T) { flags := newUpdateCommand(nil).Flags() - flags.Set("constraint", "node=toadd") - flags.Set("remove-constraint", "node!=toremove") + flags.Set("constraint-add", "node=toadd") + flags.Set("constraint-rm", "node!=toremove") placement := &swarm.Placement{ Constraints: []string{"node!=toremove", "container=tokeep"}, @@ -52,8 +52,8 @@ func TestUpdatePlacement(t *testing.T) { func TestUpdateEnvironment(t *testing.T) { flags := newUpdateCommand(nil).Flags() - flags.Set("env", "toadd=newenv") - flags.Set("remove-env", "toremove") + flags.Set("env-add", "toadd=newenv") + flags.Set("env-rm", "toremove") envs := []string{"toremove=theenvtoremove", "tokeep=value"} @@ -65,8 +65,8 @@ func TestUpdateEnvironment(t *testing.T) { func TestUpdateMounts(t *testing.T) { flags := newUpdateCommand(nil).Flags() - flags.Set("mount", "type=volume,target=/toadd") - flags.Set("remove-mount", "/toremove") + flags.Set("mount-add", "type=volume,target=/toadd") + flags.Set("mount-rm", "/toremove") mounts := []swarm.Mount{ {Target: "/toremove", Type: swarm.MountType("BIND")}, @@ -81,8 +81,8 @@ func TestUpdateMounts(t *testing.T) { func TestUpdateNetworks(t *testing.T) { flags := newUpdateCommand(nil).Flags() - flags.Set("network", "toadd") - flags.Set("remove-network", "toremove") + flags.Set("network-add", "toadd") + flags.Set("network-rm", "toremove") attachments := []swarm.NetworkAttachmentConfig{ {Target: "toremove", Aliases: []string{"foo"}}, @@ -97,8 +97,8 @@ func TestUpdateNetworks(t *testing.T) { func TestUpdatePorts(t *testing.T) { flags := newUpdateCommand(nil).Flags() - flags.Set("publish", "1000:1000") - flags.Set("remove-publish", "333/udp") + flags.Set("publish-add", "1000:1000") + flags.Set("publish-rm", "333/udp") portConfigs := []swarm.PortConfig{ {TargetPort: 333, Protocol: swarm.PortConfigProtocol("udp")}, diff --git a/integration-cli/docker_cli_service_update_test.go b/integration-cli/docker_cli_service_update_test.go index 680cb16245..67d1641747 100644 --- a/integration-cli/docker_cli_service_update_test.go +++ b/integration-cli/docker_cli_service_update_test.go @@ -22,7 +22,7 @@ func (s *DockerSwarmSuite) TestServiceUpdatePort(c *check.C) { waitAndAssert(c, defaultReconciliationTimeout, d.checkActiveContainerCount, checker.Equals, 1) // Update the service: changed the port mapping from 8080:8081 to 8082:8083. - _, err = d.Cmd("service", "update", "-p", "8082:8083", "--remove-publish", "8081", serviceName) + _, err = d.Cmd("service", "update", "--publish-add", "8082:8083", "--publish-rm", "8081", serviceName) c.Assert(err, checker.IsNil) // Inspect the service and verify port mapping