فهرست منبع

Merge pull request #28943 from vdemeester/publish-long-short-syntax

Remove --port and update --publish for services to support syntaxes
Sebastiaan van Stijn 8 سال پیش
والد
کامیت
2fe62f2395

+ 1 - 1
CHANGELOG.md

@@ -72,7 +72,7 @@ To manually remove all plugins and resolve this problem, take the following step
 ### Networking
 ### Networking
 
 
 + Add `--attachable` network support to enable `docker run` to work in swarm-mode overlay network [#25962](https://github.com/docker/docker/pull/25962)
 + Add `--attachable` network support to enable `docker run` to work in swarm-mode overlay network [#25962](https://github.com/docker/docker/pull/25962)
-+ Add support for host port PublishMode in services using the `--port` option in `docker service create` [#27917](https://github.com/docker/docker/pull/27917)
++ Add support for host port PublishMode in services using the `--publish` option in `docker service create` [#27917](https://github.com/docker/docker/pull/27917) and [#28943](https://github.com/docker/docker/pull/28943)
 + Add support for Windows server 2016 overlay network driver (requires upcoming ws2016 update) [#28182](https://github.com/docker/docker/pull/28182)
 + Add support for Windows server 2016 overlay network driver (requires upcoming ws2016 update) [#28182](https://github.com/docker/docker/pull/28182)
 * Change the default `FORWARD` policy to `DROP` [#28257](https://github.com/docker/docker/pull/28257)
 * Change the default `FORWARD` policy to `DROP` [#28257](https://github.com/docker/docker/pull/28257)
 + Add support for specifying static IP addresses for predefined network on windows [#22208](https://github.com/docker/docker/pull/22208)
 + Add support for specifying static IP addresses for predefined network on windows [#22208](https://github.com/docker/docker/pull/22208)

+ 0 - 2
cli/command/service/create.go

@@ -40,13 +40,11 @@ func newCreateCommand(dockerCli *command.DockerCli) *cobra.Command {
 	flags.Var(&opts.networks, flagNetwork, "Network attachments")
 	flags.Var(&opts.networks, flagNetwork, "Network attachments")
 	flags.Var(&opts.secrets, flagSecret, "Specify secrets to expose to the service")
 	flags.Var(&opts.secrets, flagSecret, "Specify secrets to expose to the service")
 	flags.VarP(&opts.endpoint.publishPorts, flagPublish, "p", "Publish a port as a node port")
 	flags.VarP(&opts.endpoint.publishPorts, flagPublish, "p", "Publish a port as a node port")
-	flags.MarkHidden(flagPublish)
 	flags.Var(&opts.groups, flagGroup, "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.dns, flagDNS, "Set custom DNS servers")
 	flags.Var(&opts.dnsOption, flagDNSOption, "Set DNS options")
 	flags.Var(&opts.dnsOption, flagDNSOption, "Set DNS options")
 	flags.Var(&opts.dnsSearch, flagDNSSearch, "Set custom DNS search domains")
 	flags.Var(&opts.dnsSearch, flagDNSSearch, "Set custom DNS search domains")
 	flags.Var(&opts.hosts, flagHost, "Set one or more custom host-to-IP mappings (host:ip)")
 	flags.Var(&opts.hosts, flagHost, "Set one or more custom host-to-IP mappings (host:ip)")
-	flags.Var(&opts.endpoint.expandedPorts, flagPort, "Publish a port")
 
 
 	flags.SetInterspersed(false)
 	flags.SetInterspersed(false)
 	return cmd
 	return cmd

+ 10 - 44
cli/command/service/opts.go

@@ -287,45 +287,17 @@ func convertNetworks(networks []string) []swarm.NetworkAttachmentConfig {
 }
 }
 
 
 type endpointOptions struct {
 type endpointOptions struct {
-	mode          string
-	publishPorts  opts.ListOpts
-	expandedPorts opts.PortOpt
+	mode         string
+	publishPorts opts.PortOpt
 }
 }
 
 
 func (e *endpointOptions) ToEndpointSpec() *swarm.EndpointSpec {
 func (e *endpointOptions) ToEndpointSpec() *swarm.EndpointSpec {
-	portConfigs := []swarm.PortConfig{}
-	// We can ignore errors because the format was already validated by ValidatePort
-	ports, portBindings, _ := nat.ParsePortSpecs(e.publishPorts.GetAll())
-
-	for port := range ports {
-		portConfigs = append(portConfigs, ConvertPortToPortConfig(port, portBindings)...)
-	}
-
 	return &swarm.EndpointSpec{
 	return &swarm.EndpointSpec{
 		Mode:  swarm.ResolutionMode(strings.ToLower(e.mode)),
 		Mode:  swarm.ResolutionMode(strings.ToLower(e.mode)),
-		Ports: append(portConfigs, e.expandedPorts.Value()...),
+		Ports: e.publishPorts.Value(),
 	}
 	}
 }
 }
 
 
-// ConvertPortToPortConfig converts ports to the swarm type
-func ConvertPortToPortConfig(
-	port nat.Port,
-	portBindings map[nat.Port][]nat.PortBinding,
-) []swarm.PortConfig {
-	ports := []swarm.PortConfig{}
-
-	for _, binding := range portBindings[port] {
-		hostPort, _ := strconv.ParseUint(binding.HostPort, 10, 16)
-		ports = append(ports, swarm.PortConfig{
-			//TODO Name: ?
-			Protocol:      swarm.PortConfigProtocol(strings.ToLower(port.Proto())),
-			TargetPort:    uint32(port.Int()),
-			PublishedPort: uint32(hostPort),
-		})
-	}
-	return ports
-}
-
 type logDriverOptions struct {
 type logDriverOptions struct {
 	name string
 	name string
 	opts opts.ListOpts
 	opts opts.ListOpts
@@ -459,16 +431,13 @@ func newServiceOptions() *serviceOptions {
 		containerLabels: opts.NewListOpts(runconfigopts.ValidateEnv),
 		containerLabels: opts.NewListOpts(runconfigopts.ValidateEnv),
 		env:             opts.NewListOpts(runconfigopts.ValidateEnv),
 		env:             opts.NewListOpts(runconfigopts.ValidateEnv),
 		envFile:         opts.NewListOpts(nil),
 		envFile:         opts.NewListOpts(nil),
-		endpoint: endpointOptions{
-			publishPorts: opts.NewListOpts(ValidatePort),
-		},
-		groups:    opts.NewListOpts(nil),
-		logDriver: newLogDriverOptions(),
-		dns:       opts.NewListOpts(opts.ValidateIPAddress),
-		dnsOption: opts.NewListOpts(nil),
-		dnsSearch: opts.NewListOpts(opts.ValidateDNSSearch),
-		hosts:     opts.NewListOpts(runconfigopts.ValidateExtraHost),
-		networks:  opts.NewListOpts(nil),
+		groups:          opts.NewListOpts(nil),
+		logDriver:       newLogDriverOptions(),
+		dns:             opts.NewListOpts(opts.ValidateIPAddress),
+		dnsOption:       opts.NewListOpts(nil),
+		dnsSearch:       opts.NewListOpts(opts.ValidateDNSSearch),
+		hosts:           opts.NewListOpts(runconfigopts.ValidateExtraHost),
+		networks:        opts.NewListOpts(nil),
 	}
 	}
 }
 }
 
 
@@ -649,9 +618,6 @@ const (
 	flagPublish               = "publish"
 	flagPublish               = "publish"
 	flagPublishRemove         = "publish-rm"
 	flagPublishRemove         = "publish-rm"
 	flagPublishAdd            = "publish-add"
 	flagPublishAdd            = "publish-add"
-	flagPort                  = "port"
-	flagPortAdd               = "port-add"
-	flagPortRemove            = "port-rm"
 	flagReplicas              = "replicas"
 	flagReplicas              = "replicas"
 	flagReserveCPU            = "reserve-cpu"
 	flagReserveCPU            = "reserve-cpu"
 	flagReserveMemory         = "reserve-memory"
 	flagReserveMemory         = "reserve-memory"

+ 40 - 70
cli/command/service/update.go

@@ -24,7 +24,7 @@ import (
 )
 )
 
 
 func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command {
 func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command {
-	opts := newServiceOptions()
+	serviceOpts := newServiceOptions()
 
 
 	cmd := &cobra.Command{
 	cmd := &cobra.Command{
 		Use:   "update [OPTIONS] SERVICE",
 		Use:   "update [OPTIONS] SERVICE",
@@ -40,36 +40,33 @@ func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command {
 	flags.String("args", "", "Service command args")
 	flags.String("args", "", "Service command args")
 	flags.Bool("rollback", false, "Rollback to previous specification")
 	flags.Bool("rollback", false, "Rollback to previous specification")
 	flags.Bool("force", false, "Force update even if no changes require it")
 	flags.Bool("force", false, "Force update even if no changes require it")
-	addServiceFlags(cmd, opts)
+	addServiceFlags(cmd, serviceOpts)
 
 
 	flags.Var(newListOptsVar(), flagEnvRemove, "Remove an environment variable")
 	flags.Var(newListOptsVar(), flagEnvRemove, "Remove an environment variable")
 	flags.Var(newListOptsVar(), flagGroupRemove, "Remove a previously added supplementary user group from the container")
 	flags.Var(newListOptsVar(), flagGroupRemove, "Remove a previously added supplementary user group from the container")
 	flags.Var(newListOptsVar(), flagLabelRemove, "Remove a label by its key")
 	flags.Var(newListOptsVar(), flagLabelRemove, "Remove a label by its key")
 	flags.Var(newListOptsVar(), flagContainerLabelRemove, "Remove a container label by its key")
 	flags.Var(newListOptsVar(), flagContainerLabelRemove, "Remove a container label by its key")
 	flags.Var(newListOptsVar(), flagMountRemove, "Remove a mount by its target path")
 	flags.Var(newListOptsVar(), flagMountRemove, "Remove a mount by its target path")
-	flags.Var(newListOptsVar().WithValidator(validatePublishRemove), flagPublishRemove, "Remove a published port by its target port")
-	flags.MarkHidden(flagPublishRemove)
-	flags.Var(newListOptsVar(), flagPortRemove, "Remove a port(target-port mandatory)")
+	// flags.Var(newListOptsVar().WithValidator(validatePublishRemove), flagPublishRemove, "Remove a published port by its target port")
+	flags.Var(&opts.PortOpt{}, flagPublishRemove, "Remove a published port by its target port")
 	flags.Var(newListOptsVar(), flagConstraintRemove, "Remove a constraint")
 	flags.Var(newListOptsVar(), flagConstraintRemove, "Remove a constraint")
 	flags.Var(newListOptsVar(), flagDNSRemove, "Remove a custom DNS server")
 	flags.Var(newListOptsVar(), flagDNSRemove, "Remove a custom DNS server")
 	flags.Var(newListOptsVar(), flagDNSOptionRemove, "Remove a DNS option")
 	flags.Var(newListOptsVar(), flagDNSOptionRemove, "Remove a DNS option")
 	flags.Var(newListOptsVar(), flagDNSSearchRemove, "Remove a DNS search domain")
 	flags.Var(newListOptsVar(), flagDNSSearchRemove, "Remove a DNS search domain")
 	flags.Var(newListOptsVar(), flagHostRemove, "Remove a custom host-to-IP mapping (host:ip)")
 	flags.Var(newListOptsVar(), flagHostRemove, "Remove a custom host-to-IP mapping (host:ip)")
-	flags.Var(&opts.labels, flagLabelAdd, "Add or update a service label")
-	flags.Var(&opts.containerLabels, flagContainerLabelAdd, "Add or update a container label")
-	flags.Var(&opts.env, flagEnvAdd, "Add or update an environment variable")
+	flags.Var(&serviceOpts.labels, flagLabelAdd, "Add or update a service label")
+	flags.Var(&serviceOpts.containerLabels, flagContainerLabelAdd, "Add or update a container label")
+	flags.Var(&serviceOpts.env, flagEnvAdd, "Add or update an environment variable")
 	flags.Var(newListOptsVar(), flagSecretRemove, "Remove a secret")
 	flags.Var(newListOptsVar(), flagSecretRemove, "Remove a secret")
-	flags.Var(&opts.secrets, flagSecretAdd, "Add or update a secret on a service")
-	flags.Var(&opts.mounts, flagMountAdd, "Add or update a mount on a service")
-	flags.Var(&opts.constraints, flagConstraintAdd, "Add or update a placement constraint")
-	flags.Var(&opts.endpoint.publishPorts, flagPublishAdd, "Add or update a published port")
-	flags.MarkHidden(flagPublishAdd)
-	flags.Var(&opts.endpoint.expandedPorts, flagPortAdd, "Add or update a port")
-	flags.Var(&opts.groups, flagGroupAdd, "Add an additional supplementary user group to the container")
-	flags.Var(&opts.dns, flagDNSAdd, "Add or update a custom DNS server")
-	flags.Var(&opts.dnsOption, flagDNSOptionAdd, "Add or update a DNS option")
-	flags.Var(&opts.dnsSearch, flagDNSSearchAdd, "Add or update a custom DNS search domain")
-	flags.Var(&opts.hosts, flagHostAdd, "Add or update a custom host-to-IP mapping (host:ip)")
+	flags.Var(&serviceOpts.secrets, flagSecretAdd, "Add or update a secret on a service")
+	flags.Var(&serviceOpts.mounts, flagMountAdd, "Add or update a mount on a service")
+	flags.Var(&serviceOpts.constraints, flagConstraintAdd, "Add or update a placement constraint")
+	flags.Var(&serviceOpts.endpoint.publishPorts, flagPublishAdd, "Add or update a published port")
+	flags.Var(&serviceOpts.groups, flagGroupAdd, "Add an additional supplementary user group to the container")
+	flags.Var(&serviceOpts.dns, flagDNSAdd, "Add or update a custom DNS server")
+	flags.Var(&serviceOpts.dnsOption, flagDNSOptionAdd, "Add or update a DNS option")
+	flags.Var(&serviceOpts.dnsSearch, flagDNSSearchAdd, "Add or update a custom DNS search domain")
+	flags.Var(&serviceOpts.hosts, flagHostAdd, "Add or update a custom host-to-IP mapping (host:ip)")
 
 
 	return cmd
 	return cmd
 }
 }
@@ -276,7 +273,7 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error {
 		}
 		}
 	}
 	}
 
 
-	if anyChanged(flags, flagPublishAdd, flagPublishRemove, flagPortAdd, flagPortRemove) {
+	if anyChanged(flags, flagPublishAdd, flagPublishRemove) {
 		if spec.EndpointSpec == nil {
 		if spec.EndpointSpec == nil {
 			spec.EndpointSpec = &swarm.EndpointSpec{}
 			spec.EndpointSpec = &swarm.EndpointSpec{}
 		}
 		}
@@ -633,18 +630,11 @@ func (r byPortConfig) Less(i, j int) bool {
 
 
 func portConfigToString(portConfig *swarm.PortConfig) string {
 func portConfigToString(portConfig *swarm.PortConfig) string {
 	protocol := portConfig.Protocol
 	protocol := portConfig.Protocol
-	if protocol == "" {
-		protocol = "tcp"
-	}
-
 	mode := portConfig.PublishMode
 	mode := portConfig.PublishMode
-	if mode == "" {
-		mode = "ingress"
-	}
-
 	return fmt.Sprintf("%v:%v/%s/%s", portConfig.PublishedPort, portConfig.TargetPort, protocol, mode)
 	return fmt.Sprintf("%v:%v/%s/%s", portConfig.PublishedPort, portConfig.TargetPort, protocol, mode)
 }
 }
 
 
+// FIXME(vdemeester) port to opts.PortOpt
 // This validation is only used for `--publish-rm`.
 // This validation is only used for `--publish-rm`.
 // The `--publish-rm` takes:
 // The `--publish-rm` takes:
 // <TargetPort>[/<Protocol>] (e.g., 80, 80/tcp, 53/udp)
 // <TargetPort>[/<Protocol>] (e.g., 80, 80/tcp, 53/udp)
@@ -665,58 +655,21 @@ func validatePublishRemove(val string) (string, error) {
 func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) error {
 func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) error {
 	// The key of the map is `port/protocol`, e.g., `80/tcp`
 	// The key of the map is `port/protocol`, e.g., `80/tcp`
 	portSet := map[string]swarm.PortConfig{}
 	portSet := map[string]swarm.PortConfig{}
-	// Check to see if there are any conflict in flags.
-	if flags.Changed(flagPublishAdd) {
-		values := flags.Lookup(flagPublishAdd).Value.(*opts.ListOpts).GetAll()
-		ports, portBindings, _ := nat.ParsePortSpecs(values)
-
-		for port := range ports {
-			newConfigs := ConvertPortToPortConfig(port, portBindings)
-			for _, entry := range newConfigs {
-				if v, ok := portSet[portConfigToString(&entry)]; ok && v != entry {
-					return fmt.Errorf("conflicting port mapping between %v:%v/%s and %v:%v/%s", entry.PublishedPort, entry.TargetPort, entry.Protocol, v.PublishedPort, v.TargetPort, v.Protocol)
-				}
-				portSet[portConfigToString(&entry)] = entry
-			}
-		}
-	}
-
-	if flags.Changed(flagPortAdd) {
-		for _, entry := range flags.Lookup(flagPortAdd).Value.(*opts.PortOpt).Value() {
-			if v, ok := portSet[portConfigToString(&entry)]; ok && v != entry {
-				return fmt.Errorf("conflicting port mapping between %v:%v/%s and %v:%v/%s", entry.PublishedPort, entry.TargetPort, entry.Protocol, v.PublishedPort, v.TargetPort, v.Protocol)
-			}
-			portSet[portConfigToString(&entry)] = entry
-		}
-	}
 
 
-	// Override previous PortConfig in service if there is any duplicate
+	// Build the current list of portConfig
 	for _, entry := range *portConfig {
 	for _, entry := range *portConfig {
 		if _, ok := portSet[portConfigToString(&entry)]; !ok {
 		if _, ok := portSet[portConfigToString(&entry)]; !ok {
 			portSet[portConfigToString(&entry)] = entry
 			portSet[portConfigToString(&entry)] = entry
 		}
 		}
 	}
 	}
 
 
-	toRemove := flags.Lookup(flagPublishRemove).Value.(*opts.ListOpts).GetAll()
-	removePortCSV := flags.Lookup(flagPortRemove).Value.(*opts.ListOpts).GetAll()
-	removePortOpts := &opts.PortOpt{}
-	for _, portCSV := range removePortCSV {
-		if err := removePortOpts.Set(portCSV); err != nil {
-			return err
-		}
-	}
-
 	newPorts := []swarm.PortConfig{}
 	newPorts := []swarm.PortConfig{}
+
+	// Clean current ports
+	toRemove := flags.Lookup(flagPublishRemove).Value.(*opts.PortOpt).Value()
 portLoop:
 portLoop:
 	for _, port := range portSet {
 	for _, port := range portSet {
-		for _, rawTargetPort := range toRemove {
-			targetPort := nat.Port(rawTargetPort)
-			if equalPort(targetPort, port) {
-				continue portLoop
-			}
-		}
-
-		for _, pConfig := range removePortOpts.Value() {
+		for _, pConfig := range toRemove {
 			if equalProtocol(port.Protocol, pConfig.Protocol) &&
 			if equalProtocol(port.Protocol, pConfig.Protocol) &&
 				port.TargetPort == pConfig.TargetPort &&
 				port.TargetPort == pConfig.TargetPort &&
 				equalPublishMode(port.PublishMode, pConfig.PublishMode) {
 				equalPublishMode(port.PublishMode, pConfig.PublishMode) {
@@ -727,6 +680,23 @@ portLoop:
 		newPorts = append(newPorts, port)
 		newPorts = append(newPorts, port)
 	}
 	}
 
 
+	// Check to see if there are any conflict in flags.
+	if flags.Changed(flagPublishAdd) {
+		ports := flags.Lookup(flagPublishAdd).Value.(*opts.PortOpt).Value()
+
+		for _, port := range ports {
+			if v, ok := portSet[portConfigToString(&port)]; ok {
+				if v != port {
+					fmt.Println("v", v)
+					return fmt.Errorf("conflicting port mapping between %v:%v/%s and %v:%v/%s", port.PublishedPort, port.TargetPort, port.Protocol, v.PublishedPort, v.TargetPort, v.Protocol)
+				}
+				continue
+			}
+			//portSet[portConfigToString(&port)] = port
+			newPorts = append(newPorts, port)
+		}
+	}
+
 	// Sort the PortConfig to avoid unnecessary updates
 	// Sort the PortConfig to avoid unnecessary updates
 	sort.Sort(byPortConfig(newPorts))
 	sort.Sort(byPortConfig(newPorts))
 	*portConfig = newPorts
 	*portConfig = newPorts

+ 17 - 20
cli/command/service/update_test.go

@@ -220,28 +220,18 @@ func TestUpdatePorts(t *testing.T) {
 	assert.Equal(t, targetPorts[1], 1000)
 	assert.Equal(t, targetPorts[1], 1000)
 }
 }
 
 
-func TestUpdatePortsDuplicateEntries(t *testing.T) {
+func TestUpdatePortsDuplicate(t *testing.T) {
 	// Test case for #25375
 	// Test case for #25375
 	flags := newUpdateCommand(nil).Flags()
 	flags := newUpdateCommand(nil).Flags()
 	flags.Set("publish-add", "80:80")
 	flags.Set("publish-add", "80:80")
 
 
 	portConfigs := []swarm.PortConfig{
 	portConfigs := []swarm.PortConfig{
-		{TargetPort: 80, PublishedPort: 80},
-	}
-
-	err := updatePorts(flags, &portConfigs)
-	assert.Equal(t, err, nil)
-	assert.Equal(t, len(portConfigs), 1)
-	assert.Equal(t, portConfigs[0].TargetPort, uint32(80))
-}
-
-func TestUpdatePortsDuplicateKeys(t *testing.T) {
-	// Test case for #25375
-	flags := newUpdateCommand(nil).Flags()
-	flags.Set("publish-add", "80:80")
-
-	portConfigs := []swarm.PortConfig{
-		{TargetPort: 80, PublishedPort: 80},
+		{
+			TargetPort:    80,
+			PublishedPort: 80,
+			Protocol:      swarm.PortConfigProtocolTCP,
+			PublishMode:   swarm.PortConfigPublishModeIngress,
+		},
 	}
 	}
 
 
 	err := updatePorts(flags, &portConfigs)
 	err := updatePorts(flags, &portConfigs)
@@ -355,15 +345,22 @@ func TestUpdatePortsRmWithProtocol(t *testing.T) {
 	flags.Set("publish-rm", "82/udp")
 	flags.Set("publish-rm", "82/udp")
 
 
 	portConfigs := []swarm.PortConfig{
 	portConfigs := []swarm.PortConfig{
-		{TargetPort: 80, PublishedPort: 8080, Protocol: swarm.PortConfigProtocolTCP},
+		{
+			TargetPort:    80,
+			PublishedPort: 8080,
+			Protocol:      swarm.PortConfigProtocolTCP,
+			PublishMode:   swarm.PortConfigPublishModeIngress,
+		},
 	}
 	}
 
 
 	err := updatePorts(flags, &portConfigs)
 	err := updatePorts(flags, &portConfigs)
 	assert.Equal(t, err, nil)
 	assert.Equal(t, err, nil)
-	assert.Equal(t, len(portConfigs), 1)
-	assert.Equal(t, portConfigs[0].TargetPort, uint32(82))
+	assert.Equal(t, len(portConfigs), 2)
+	assert.Equal(t, portConfigs[0].TargetPort, uint32(81))
+	assert.Equal(t, portConfigs[1].TargetPort, uint32(82))
 }
 }
 
 
+// FIXME(vdemeester) port to opts.PortOpt
 func TestValidatePort(t *testing.T) {
 func TestValidatePort(t *testing.T) {
 	validPorts := []string{"80/tcp", "80", "80/udp"}
 	validPorts := []string{"80/tcp", "80", "80/udp"}
 	invalidPorts := map[string]string{
 	invalidPorts := map[string]string{

+ 1 - 2
cli/command/stack/deploy.go

@@ -21,7 +21,6 @@ import (
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/cli"
 	"github.com/docker/docker/cli"
 	"github.com/docker/docker/cli/command"
 	"github.com/docker/docker/cli/command"
-	servicecmd "github.com/docker/docker/cli/command/service"
 	dockerclient "github.com/docker/docker/client"
 	dockerclient "github.com/docker/docker/client"
 	"github.com/docker/docker/opts"
 	"github.com/docker/docker/opts"
 	runconfigopts "github.com/docker/docker/runconfig/opts"
 	runconfigopts "github.com/docker/docker/runconfig/opts"
@@ -745,7 +744,7 @@ func convertEndpointSpec(source []string) (*swarm.EndpointSpec, error) {
 	for port := range ports {
 	for port := range ports {
 		portConfigs = append(
 		portConfigs = append(
 			portConfigs,
 			portConfigs,
-			servicecmd.ConvertPortToPortConfig(port, portBindings)...)
+			opts.ConvertPortToPortConfig(port, portBindings)...)
 	}
 	}
 
 
 	return &swarm.EndpointSpec{Ports: portConfigs}, nil
 	return &swarm.EndpointSpec{Ports: portConfigs}, nil

+ 3 - 3
contrib/completion/bash/docker

@@ -2756,7 +2756,7 @@ _docker_service_update() {
 			--host
 			--host
 			--mode
 			--mode
 			--name
 			--name
-			--port
+			--publish
 			--secret
 			--secret
 		"
 		"
 
 
@@ -2803,8 +2803,8 @@ _docker_service_update() {
 			--host-add
 			--host-add
 			--host-rm
 			--host-rm
 			--image
 			--image
-			--port-add
-			--port-rm
+			--publish-add
+			--publish-rm
 			--secret-add
 			--secret-add
 			--secret-rm
 			--secret-rm
 		"
 		"

+ 3 - 3
contrib/completion/zsh/_docker

@@ -1797,7 +1797,7 @@ __docker_service_subcommand() {
                 "($help)*--env-file=[Read environment variables from a file]:environment file:_files" \
                 "($help)*--env-file=[Read environment variables from a file]:environment file:_files" \
                 "($help)--mode=[Service Mode]:mode:(global replicated)" \
                 "($help)--mode=[Service Mode]:mode:(global replicated)" \
                 "($help)--name=[Service name]:name: " \
                 "($help)--name=[Service name]:name: " \
-                "($help)*--port=[Publish a port]:port: " \
+                "($help)*--publish=[Publish a port]:port: " \
                 "($help -): :__docker_complete_images" \
                 "($help -): :__docker_complete_images" \
                 "($help -):command: _command_names -e" \
                 "($help -):command: _command_names -e" \
                 "($help -)*::arguments: _normal" && ret=0
                 "($help -)*::arguments: _normal" && ret=0
@@ -1870,8 +1870,8 @@ __docker_service_subcommand() {
                 "($help)*--group-add=[Add additional supplementary user groups to the container]:group:_groups" \
                 "($help)*--group-add=[Add additional supplementary user groups to the container]:group:_groups" \
                 "($help)*--group-rm=[Remove previously added supplementary user groups from the container]:group:_groups" \
                 "($help)*--group-rm=[Remove previously added supplementary user groups from the container]:group:_groups" \
                 "($help)--image=[Service image tag]:image:__docker_complete_repositories" \
                 "($help)--image=[Service image tag]:image:__docker_complete_repositories" \
-                "($help)*--port-add=[Add or update a port]:port: " \
-                "($help)*--port-rm=[Remove a port(target-port mandatory)]:port: " \
+                "($help)*--publish-add=[Add or update a port]:port: " \
+                "($help)*--publish-rm=[Remove a port(target-port mandatory)]:port: " \
                 "($help)--rollback[Rollback to previous specification]" \
                 "($help)--rollback[Rollback to previous specification]" \
                 "($help -)1:service:__docker_complete_services" && ret=0
                 "($help -)1:service:__docker_complete_services" && ret=0
             ;;
             ;;

+ 41 - 13
integration-cli/docker_cli_swarm_test.go

@@ -235,23 +235,51 @@ func (s *DockerSwarmSuite) TestSwarmNodeTaskListFilter(c *check.C) {
 func (s *DockerSwarmSuite) TestSwarmPublishAdd(c *check.C) {
 func (s *DockerSwarmSuite) TestSwarmPublishAdd(c *check.C) {
 	d := s.AddDaemon(c, true, true)
 	d := s.AddDaemon(c, true, true)
 
 
-	name := "top"
-	out, err := d.Cmd("service", "create", "--name", name, "--label", "x=y", "busybox", "top")
-	c.Assert(err, checker.IsNil)
-	c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "")
+	testCases := []struct {
+		name       string
+		publishAdd []string
+		ports      string
+	}{
+		{
+			name: "simple-syntax",
+			publishAdd: []string{
+				"80:80",
+				"80:80",
+				"80:80",
+				"80:20",
+			},
+			ports: "[{ tcp 80 80 ingress}]",
+		},
+		{
+			name: "complex-syntax",
+			publishAdd: []string{
+				"target=90,published=90,protocol=tcp,mode=ingress",
+				"target=90,published=90,protocol=tcp,mode=ingress",
+				"target=90,published=90,protocol=tcp,mode=ingress",
+				"target=30,published=90,protocol=tcp,mode=ingress",
+			},
+			ports: "[{ tcp 90 90 ingress}]",
+		},
+	}
 
 
-	out, err = d.Cmd("service", "update", "--publish-add", "80:80", name)
-	c.Assert(err, checker.IsNil)
+	for _, tc := range testCases {
+		out, err := d.Cmd("service", "create", "--name", tc.name, "--label", "x=y", "busybox", "top")
+		c.Assert(err, checker.IsNil, check.Commentf(out))
+		c.Assert(strings.TrimSpace(out), checker.Not(checker.Equals), "")
 
 
-	out, err = d.CmdRetryOutOfSequence("service", "update", "--publish-add", "80:80", name)
-	c.Assert(err, checker.IsNil)
+		out, err = d.CmdRetryOutOfSequence("service", "update", "--publish-add", tc.publishAdd[0], tc.name)
+		c.Assert(err, checker.IsNil, check.Commentf(out))
 
 
-	out, err = d.CmdRetryOutOfSequence("service", "update", "--publish-add", "80:80", "--publish-add", "80:20", name)
-	c.Assert(err, checker.NotNil)
+		out, err = d.CmdRetryOutOfSequence("service", "update", "--publish-add", tc.publishAdd[1], tc.name)
+		c.Assert(err, checker.IsNil, check.Commentf(out))
 
 
-	out, err = d.Cmd("service", "inspect", "--format", "{{ .Spec.EndpointSpec.Ports }}", name)
-	c.Assert(err, checker.IsNil)
-	c.Assert(strings.TrimSpace(out), checker.Equals, "[{ tcp 80 80 ingress}]")
+		out, err = d.CmdRetryOutOfSequence("service", "update", "--publish-add", tc.publishAdd[2], "--publish-add", tc.publishAdd[3], tc.name)
+		c.Assert(err, checker.NotNil, check.Commentf(out))
+
+		out, err = d.Cmd("service", "inspect", "--format", "{{ .Spec.EndpointSpec.Ports }}", tc.name)
+		c.Assert(err, checker.IsNil)
+		c.Assert(strings.TrimSpace(out), checker.Equals, tc.ports)
+	}
 }
 }
 
 
 func (s *DockerSwarmSuite) TestSwarmServiceWithGroup(c *check.C) {
 func (s *DockerSwarmSuite) TestSwarmServiceWithGroup(c *check.C) {

+ 85 - 39
opts/port.go

@@ -3,10 +3,12 @@ package opts
 import (
 import (
 	"encoding/csv"
 	"encoding/csv"
 	"fmt"
 	"fmt"
+	"regexp"
 	"strconv"
 	"strconv"
 	"strings"
 	"strings"
 
 
 	"github.com/docker/docker/api/types/swarm"
 	"github.com/docker/docker/api/types/swarm"
+	"github.com/docker/go-connections/nat"
 )
 )
 
 
 const (
 const (
@@ -23,59 +25,83 @@ type PortOpt struct {
 
 
 // Set a new port value
 // Set a new port value
 func (p *PortOpt) Set(value string) error {
 func (p *PortOpt) Set(value string) error {
-	csvReader := csv.NewReader(strings.NewReader(value))
-	fields, err := csvReader.Read()
+	longSyntax, err := regexp.MatchString(`\w+=\w+(,\w+=\w+)*`, value)
 	if err != nil {
 	if err != nil {
 		return err
 		return err
 	}
 	}
-
-	pConfig := swarm.PortConfig{}
-	for _, field := range fields {
-		parts := strings.SplitN(field, "=", 2)
-		if len(parts) != 2 {
-			return fmt.Errorf("invalid field %s", field)
+	if longSyntax {
+		csvReader := csv.NewReader(strings.NewReader(value))
+		fields, err := csvReader.Read()
+		if err != nil {
+			return err
 		}
 		}
 
 
-		key := strings.ToLower(parts[0])
-		value := strings.ToLower(parts[1])
-
-		switch key {
-		case portOptProtocol:
-			if value != string(swarm.PortConfigProtocolTCP) && value != string(swarm.PortConfigProtocolUDP) {
-				return fmt.Errorf("invalid protocol value %s", value)
+		pConfig := swarm.PortConfig{}
+		for _, field := range fields {
+			parts := strings.SplitN(field, "=", 2)
+			if len(parts) != 2 {
+				return fmt.Errorf("invalid field %s", field)
 			}
 			}
 
 
-			pConfig.Protocol = swarm.PortConfigProtocol(value)
-		case portOptMode:
-			if value != string(swarm.PortConfigPublishModeIngress) && value != string(swarm.PortConfigPublishModeHost) {
-				return fmt.Errorf("invalid publish mode value %s", value)
+			key := strings.ToLower(parts[0])
+			value := strings.ToLower(parts[1])
+
+			switch key {
+			case portOptProtocol:
+				if value != string(swarm.PortConfigProtocolTCP) && value != string(swarm.PortConfigProtocolUDP) {
+					return fmt.Errorf("invalid protocol value %s", value)
+				}
+
+				pConfig.Protocol = swarm.PortConfigProtocol(value)
+			case portOptMode:
+				if value != string(swarm.PortConfigPublishModeIngress) && value != string(swarm.PortConfigPublishModeHost) {
+					return fmt.Errorf("invalid publish mode value %s", value)
+				}
+
+				pConfig.PublishMode = swarm.PortConfigPublishMode(value)
+			case portOptTargetPort:
+				tPort, err := strconv.ParseUint(value, 10, 16)
+				if err != nil {
+					return err
+				}
+
+				pConfig.TargetPort = uint32(tPort)
+			case portOptPublishedPort:
+				pPort, err := strconv.ParseUint(value, 10, 16)
+				if err != nil {
+					return err
+				}
+
+				pConfig.PublishedPort = uint32(pPort)
+			default:
+				return fmt.Errorf("invalid field key %s", key)
 			}
 			}
+		}
 
 
-			pConfig.PublishMode = swarm.PortConfigPublishMode(value)
-		case portOptTargetPort:
-			tPort, err := strconv.ParseUint(value, 10, 16)
-			if err != nil {
-				return err
-			}
+		if pConfig.TargetPort == 0 {
+			return fmt.Errorf("missing mandatory field %q", portOptTargetPort)
+		}
 
 
-			pConfig.TargetPort = uint32(tPort)
-		case portOptPublishedPort:
-			pPort, err := strconv.ParseUint(value, 10, 16)
-			if err != nil {
-				return err
-			}
+		if pConfig.PublishMode == "" {
+			pConfig.PublishMode = swarm.PortConfigPublishModeIngress
+		}
 
 
-			pConfig.PublishedPort = uint32(pPort)
-		default:
-			return fmt.Errorf("invalid field key %s", key)
+		if pConfig.Protocol == "" {
+			pConfig.Protocol = swarm.PortConfigProtocolTCP
 		}
 		}
-	}
 
 
-	if pConfig.TargetPort == 0 {
-		return fmt.Errorf("missing mandatory field %q", portOptTargetPort)
-	}
+		p.ports = append(p.ports, pConfig)
+	} else {
+		// short syntax
+		portConfigs := []swarm.PortConfig{}
+		// We can ignore errors because the format was already validated by ValidatePort
+		ports, portBindings, _ := nat.ParsePortSpecs([]string{value})
 
 
-	p.ports = append(p.ports, pConfig)
+		for port := range ports {
+			portConfigs = append(portConfigs, ConvertPortToPortConfig(port, portBindings)...)
+		}
+		p.ports = append(p.ports, portConfigs...)
+	}
 	return nil
 	return nil
 }
 }
 
 
@@ -98,3 +124,23 @@ func (p *PortOpt) String() string {
 func (p *PortOpt) Value() []swarm.PortConfig {
 func (p *PortOpt) Value() []swarm.PortConfig {
 	return p.ports
 	return p.ports
 }
 }
+
+// ConvertPortToPortConfig converts ports to the swarm type
+func ConvertPortToPortConfig(
+	port nat.Port,
+	portBindings map[nat.Port][]nat.PortBinding,
+) []swarm.PortConfig {
+	ports := []swarm.PortConfig{}
+
+	for _, binding := range portBindings[port] {
+		hostPort, _ := strconv.ParseUint(binding.HostPort, 10, 16)
+		ports = append(ports, swarm.PortConfig{
+			//TODO Name: ?
+			Protocol:      swarm.PortConfigProtocol(strings.ToLower(port.Proto())),
+			TargetPort:    uint32(port.Int()),
+			PublishedPort: uint32(hostPort),
+			PublishMode:   swarm.PortConfigPublishModeIngress,
+		})
+	}
+	return ports
+}

+ 259 - 0
opts/port_test.go

@@ -0,0 +1,259 @@
+package opts
+
+import (
+	"testing"
+
+	"github.com/docker/docker/api/types/swarm"
+	"github.com/docker/docker/pkg/testutil/assert"
+)
+
+func TestPortOptValidSimpleSyntax(t *testing.T) {
+	testCases := []struct {
+		value    string
+		expected []swarm.PortConfig
+	}{
+		{
+			value: "80",
+			expected: []swarm.PortConfig{
+				{
+					Protocol:    "tcp",
+					TargetPort:  80,
+					PublishMode: swarm.PortConfigPublishModeIngress,
+				},
+			},
+		},
+		{
+			value: "80:8080",
+			expected: []swarm.PortConfig{
+				{
+					Protocol:      "tcp",
+					TargetPort:    8080,
+					PublishedPort: 80,
+					PublishMode:   swarm.PortConfigPublishModeIngress,
+				},
+			},
+		},
+		{
+			value: "8080:80/tcp",
+			expected: []swarm.PortConfig{
+				{
+					Protocol:      "tcp",
+					TargetPort:    80,
+					PublishedPort: 8080,
+					PublishMode:   swarm.PortConfigPublishModeIngress,
+				},
+			},
+		},
+		{
+			value: "80:8080/udp",
+			expected: []swarm.PortConfig{
+				{
+					Protocol:      "udp",
+					TargetPort:    8080,
+					PublishedPort: 80,
+					PublishMode:   swarm.PortConfigPublishModeIngress,
+				},
+			},
+		},
+		{
+			value: "80-81:8080-8081/tcp",
+			expected: []swarm.PortConfig{
+				{
+					Protocol:      "tcp",
+					TargetPort:    8080,
+					PublishedPort: 80,
+					PublishMode:   swarm.PortConfigPublishModeIngress,
+				},
+				{
+					Protocol:      "tcp",
+					TargetPort:    8081,
+					PublishedPort: 81,
+					PublishMode:   swarm.PortConfigPublishModeIngress,
+				},
+			},
+		},
+		{
+			value: "80-82:8080-8082/udp",
+			expected: []swarm.PortConfig{
+				{
+					Protocol:      "udp",
+					TargetPort:    8080,
+					PublishedPort: 80,
+					PublishMode:   swarm.PortConfigPublishModeIngress,
+				},
+				{
+					Protocol:      "udp",
+					TargetPort:    8081,
+					PublishedPort: 81,
+					PublishMode:   swarm.PortConfigPublishModeIngress,
+				},
+				{
+					Protocol:      "udp",
+					TargetPort:    8082,
+					PublishedPort: 82,
+					PublishMode:   swarm.PortConfigPublishModeIngress,
+				},
+			},
+		},
+	}
+	for _, tc := range testCases {
+		var port PortOpt
+		assert.NilError(t, port.Set(tc.value))
+		assert.Equal(t, len(port.Value()), len(tc.expected))
+		for _, expectedPortConfig := range tc.expected {
+			assertContains(t, port.Value(), expectedPortConfig)
+		}
+	}
+}
+
+func TestPortOptValidComplexSyntax(t *testing.T) {
+	testCases := []struct {
+		value    string
+		expected []swarm.PortConfig
+	}{
+		{
+			value: "target=80",
+			expected: []swarm.PortConfig{
+				{
+					TargetPort:  80,
+					Protocol:    "tcp",
+					PublishMode: swarm.PortConfigPublishModeIngress,
+				},
+			},
+		},
+		{
+			value: "target=80,protocol=tcp",
+			expected: []swarm.PortConfig{
+				{
+					Protocol:    "tcp",
+					TargetPort:  80,
+					PublishMode: swarm.PortConfigPublishModeIngress,
+				},
+			},
+		},
+		{
+			value: "target=80,published=8080,protocol=tcp",
+			expected: []swarm.PortConfig{
+				{
+					Protocol:      "tcp",
+					TargetPort:    80,
+					PublishedPort: 8080,
+					PublishMode:   swarm.PortConfigPublishModeIngress,
+				},
+			},
+		},
+		{
+			value: "published=80,target=8080,protocol=tcp",
+			expected: []swarm.PortConfig{
+				{
+					Protocol:      "tcp",
+					TargetPort:    8080,
+					PublishedPort: 80,
+					PublishMode:   swarm.PortConfigPublishModeIngress,
+				},
+			},
+		},
+		{
+			value: "target=80,published=8080,protocol=tcp,mode=host",
+			expected: []swarm.PortConfig{
+				{
+					Protocol:      "tcp",
+					TargetPort:    80,
+					PublishedPort: 8080,
+					PublishMode:   "host",
+				},
+			},
+		},
+		{
+			value: "target=80,published=8080,mode=host",
+			expected: []swarm.PortConfig{
+				{
+					TargetPort:    80,
+					PublishedPort: 8080,
+					PublishMode:   "host",
+					Protocol:      "tcp",
+				},
+			},
+		},
+		{
+			value: "target=80,published=8080,mode=ingress",
+			expected: []swarm.PortConfig{
+				{
+					TargetPort:    80,
+					PublishedPort: 8080,
+					PublishMode:   "ingress",
+					Protocol:      "tcp",
+				},
+			},
+		},
+	}
+	for _, tc := range testCases {
+		var port PortOpt
+		assert.NilError(t, port.Set(tc.value))
+		assert.Equal(t, len(port.Value()), len(tc.expected))
+		for _, expectedPortConfig := range tc.expected {
+			assertContains(t, port.Value(), expectedPortConfig)
+		}
+	}
+}
+
+func TestPortOptInvalidComplexSyntax(t *testing.T) {
+	testCases := []struct {
+		value         string
+		expectedError string
+	}{
+		{
+			value:         "invalid,target=80",
+			expectedError: "invalid field",
+		},
+		{
+			value:         "invalid=field",
+			expectedError: "invalid field",
+		},
+		{
+			value:         "protocol=invalid",
+			expectedError: "invalid protocol value",
+		},
+		{
+			value:         "target=invalid",
+			expectedError: "invalid syntax",
+		},
+		{
+			value:         "published=invalid",
+			expectedError: "invalid syntax",
+		},
+		{
+			value:         "mode=invalid",
+			expectedError: "invalid publish mode value",
+		},
+		{
+			value:         "published=8080,protocol=tcp,mode=ingress",
+			expectedError: "missing mandatory field",
+		},
+		{
+			value:         `target=80,protocol="tcp,mode=ingress"`,
+			expectedError: "non-quoted-field",
+		},
+		{
+			value:         `target=80,"protocol=tcp,mode=ingress"`,
+			expectedError: "invalid protocol value",
+		},
+	}
+	for _, tc := range testCases {
+		var port PortOpt
+		assert.Error(t, port.Set(tc.value), tc.expectedError)
+	}
+}
+
+func assertContains(t *testing.T, portConfigs []swarm.PortConfig, expected swarm.PortConfig) {
+	var contains = false
+	for _, portConfig := range portConfigs {
+		if portConfig == expected {
+			contains = true
+			break
+		}
+	}
+	if !contains {
+		t.Errorf("expected %v to contain %v, did not", portConfigs, expected)
+	}
+}