From 75bf18c9f01d050e5fccab98fb671306c7c5f6d2 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Thu, 8 Dec 2016 22:32:10 +0100 Subject: [PATCH] Remove --port and update --publish for services to support syntaxes Add support for simple and complex syntax to `--publish` through the use of `PortOpt`. Signed-off-by: Vincent Demeester --- CHANGELOG.md | 2 +- cli/command/service/create.go | 2 - cli/command/service/opts.go | 54 +---- cli/command/service/update.go | 79 +++----- cli/command/service/update_test.go | 1 + cli/command/stack/deploy.go | 3 +- contrib/completion/bash/docker | 6 +- contrib/completion/zsh/_docker | 6 +- integration-cli/docker_cli_swarm_test.go | 54 +++-- opts/port.go | 117 +++++++---- opts/port_test.go | 243 +++++++++++++++++++++++ 11 files changed, 405 insertions(+), 162 deletions(-) create mode 100644 opts/port_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ed97157ac7..3fa05d42ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,7 +72,7 @@ To manually remove all plugins and resolve this problem, take the following step ### 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 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) * 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) diff --git a/cli/command/service/create.go b/cli/command/service/create.go index ea078e43ad..a8382835a0 100644 --- a/cli/command/service/create.go +++ b/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.secrets, flagSecret, "Specify secrets to expose to the service") 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.dns, flagDNS, "Set custom DNS servers") flags.Var(&opts.dnsOption, flagDNSOption, "Set DNS options") 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.endpoint.expandedPorts, flagPort, "Publish a port") flags.SetInterspersed(false) return cmd diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index 023b922a15..c7518e5976 100644 --- a/cli/command/service/opts.go +++ b/cli/command/service/opts.go @@ -287,45 +287,17 @@ func convertNetworks(networks []string) []swarm.NetworkAttachmentConfig { } type endpointOptions struct { - mode string - publishPorts opts.ListOpts - expandedPorts opts.PortOpt + mode string + publishPorts opts.PortOpt } 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{ 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 { name string opts opts.ListOpts @@ -459,16 +431,13 @@ func newServiceOptions() *serviceOptions { containerLabels: opts.NewListOpts(runconfigopts.ValidateEnv), env: opts.NewListOpts(runconfigopts.ValidateEnv), 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" flagPublishRemove = "publish-rm" flagPublishAdd = "publish-add" - flagPort = "port" - flagPortAdd = "port-add" - flagPortRemove = "port-rm" flagReplicas = "replicas" flagReserveCPU = "reserve-cpu" flagReserveMemory = "reserve-memory" diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 200f58c3a6..2ceaf275ab 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -24,7 +24,7 @@ import ( ) func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command { - opts := newServiceOptions() + serviceOpts := newServiceOptions() cmd := &cobra.Command{ Use: "update [OPTIONS] SERVICE", @@ -40,36 +40,33 @@ func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command { flags.String("args", "", "Service command args") flags.Bool("rollback", false, "Rollback to previous specification") 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(), flagGroupRemove, "Remove a previously added supplementary user group from the container") flags.Var(newListOptsVar(), flagLabelRemove, "Remove a 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().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(), flagDNSRemove, "Remove a custom DNS server") flags.Var(newListOptsVar(), flagDNSOptionRemove, "Remove a DNS option") flags.Var(newListOptsVar(), flagDNSSearchRemove, "Remove a DNS search domain") 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(&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 } @@ -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 { spec.EndpointSpec = &swarm.EndpointSpec{} } @@ -645,6 +642,7 @@ func portConfigToString(portConfig *swarm.PortConfig) string { 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`. // The `--publish-rm` takes: // [/] (e.g., 80, 80/tcp, 53/udp) @@ -667,26 +665,13 @@ func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) error { 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) + ports := flags.Lookup(flagPublishAdd).Value.(*opts.PortOpt).Value() - 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 + for _, port := range ports { + if v, ok := portSet[portConfigToString(&port)]; ok && v != port { + 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) } - } - } - - 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 + portSet[portConfigToString(&port)] = port } } @@ -697,26 +682,12 @@ func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) error { } } - 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 - } - } + toRemove := flags.Lookup(flagPublishRemove).Value.(*opts.PortOpt).Value() newPorts := []swarm.PortConfig{} portLoop: 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) && port.TargetPort == pConfig.TargetPort && equalPublishMode(port.PublishMode, pConfig.PublishMode) { diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index bb2e9c1073..3cb7657996 100644 --- a/cli/command/service/update_test.go +++ b/cli/command/service/update_test.go @@ -364,6 +364,7 @@ func TestUpdatePortsRmWithProtocol(t *testing.T) { assert.Equal(t, portConfigs[0].TargetPort, uint32(82)) } +// FIXME(vdemeester) port to opts.PortOpt func TestValidatePort(t *testing.T) { validPorts := []string{"80/tcp", "80", "80/udp"} invalidPorts := map[string]string{ diff --git a/cli/command/stack/deploy.go b/cli/command/stack/deploy.go index 1f41cb7d89..00a7634a0a 100644 --- a/cli/command/stack/deploy.go +++ b/cli/command/stack/deploy.go @@ -21,7 +21,6 @@ import ( "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/cli" "github.com/docker/docker/cli/command" - servicecmd "github.com/docker/docker/cli/command/service" dockerclient "github.com/docker/docker/client" "github.com/docker/docker/opts" runconfigopts "github.com/docker/docker/runconfig/opts" @@ -745,7 +744,7 @@ func convertEndpointSpec(source []string) (*swarm.EndpointSpec, error) { for port := range ports { portConfigs = append( portConfigs, - servicecmd.ConvertPortToPortConfig(port, portBindings)...) + opts.ConvertPortToPortConfig(port, portBindings)...) } return &swarm.EndpointSpec{Ports: portConfigs}, nil diff --git a/contrib/completion/bash/docker b/contrib/completion/bash/docker index efaffdc509..afe360872c 100644 --- a/contrib/completion/bash/docker +++ b/contrib/completion/bash/docker @@ -2752,7 +2752,7 @@ _docker_service_update() { --host --mode --name - --port + --publish --secret " @@ -2799,8 +2799,8 @@ _docker_service_update() { --host-add --host-rm --image - --port-add - --port-rm + --publish-add + --publish-rm --secret-add --secret-rm " diff --git a/contrib/completion/zsh/_docker b/contrib/completion/zsh/_docker index 3afcb8977c..f6028e7414 100644 --- a/contrib/completion/zsh/_docker +++ b/contrib/completion/zsh/_docker @@ -1795,7 +1795,7 @@ __docker_service_subcommand() { "($help)*--env-file=[Read environment variables from a file]:environment file:_files" \ "($help)--mode=[Service Mode]:mode:(global replicated)" \ "($help)--name=[Service name]:name: " \ - "($help)*--port=[Publish a port]:port: " \ + "($help)*--publish=[Publish a port]:port: " \ "($help -): :__docker_complete_images" \ "($help -):command: _command_names -e" \ "($help -)*::arguments: _normal" && ret=0 @@ -1868,8 +1868,8 @@ __docker_service_subcommand() { "($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)--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 -)1:service:__docker_complete_services" && ret=0 ;; diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index daa8a02dde..8604ea9f72 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/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) { 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) { diff --git a/opts/port.go b/opts/port.go index d337cb1a43..e1c4449a38 100644 --- a/opts/port.go +++ b/opts/port.go @@ -3,10 +3,12 @@ package opts import ( "encoding/csv" "fmt" + "regexp" "strconv" "strings" "github.com/docker/docker/api/types/swarm" + "github.com/docker/go-connections/nat" ) const ( @@ -23,59 +25,75 @@ type PortOpt struct { // Set a new port value 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 { 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]) - pConfig.PublishMode = swarm.PortConfigPublishMode(value) - case portOptTargetPort: - tPort, err := strconv.ParseUint(value, 10, 16) - if err != nil { - return err - } + switch key { + case portOptProtocol: + if value != string(swarm.PortConfigProtocolTCP) && value != string(swarm.PortConfigProtocolUDP) { + return fmt.Errorf("invalid protocol value %s", value) + } - pConfig.TargetPort = uint32(tPort) - case portOptPublishedPort: - pPort, err := strconv.ParseUint(value, 10, 16) - if err != nil { - return err - } + 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.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 + } + + 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) + } } - } - if pConfig.TargetPort == 0 { - return fmt.Errorf("missing mandatory field %q", portOptTargetPort) - } + if pConfig.TargetPort == 0 { + return fmt.Errorf("missing mandatory field %q", portOptTargetPort) + } - p.ports = append(p.ports, pConfig) + 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}) + + for port := range ports { + portConfigs = append(portConfigs, ConvertPortToPortConfig(port, portBindings)...) + } + p.ports = append(p.ports, portConfigs...) + } return nil } @@ -98,3 +116,22 @@ func (p *PortOpt) String() string { func (p *PortOpt) Value() []swarm.PortConfig { 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), + }) + } + return ports +} diff --git a/opts/port_test.go b/opts/port_test.go new file mode 100644 index 0000000000..8046b4428e --- /dev/null +++ b/opts/port_test.go @@ -0,0 +1,243 @@ +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, + }, + }, + }, + { + value: "80:8080", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 8080, + PublishedPort: 80, + }, + }, + }, + { + value: "8080:80/tcp", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 80, + PublishedPort: 8080, + }, + }, + }, + { + value: "80:8080/udp", + expected: []swarm.PortConfig{ + { + Protocol: "udp", + TargetPort: 8080, + PublishedPort: 80, + }, + }, + }, + { + value: "80-81:8080-8081/tcp", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 8080, + PublishedPort: 80, + }, + { + Protocol: "tcp", + TargetPort: 8081, + PublishedPort: 81, + }, + }, + }, + { + value: "80-82:8080-8082/udp", + expected: []swarm.PortConfig{ + { + Protocol: "udp", + TargetPort: 8080, + PublishedPort: 80, + }, + { + Protocol: "udp", + TargetPort: 8081, + PublishedPort: 81, + }, + { + Protocol: "udp", + TargetPort: 8082, + PublishedPort: 82, + }, + }, + }, + } + 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, + }, + }, + }, + { + value: "target=80,protocol=tcp", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 80, + }, + }, + }, + { + value: "target=80,published=8080,protocol=tcp", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 80, + PublishedPort: 8080, + }, + }, + }, + { + value: "published=80,target=8080,protocol=tcp", + expected: []swarm.PortConfig{ + { + Protocol: "tcp", + TargetPort: 8080, + PublishedPort: 80, + }, + }, + }, + { + 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", + }, + }, + }, + { + value: "target=80,published=8080,mode=ingress", + expected: []swarm.PortConfig{ + { + TargetPort: 80, + PublishedPort: 8080, + PublishMode: "ingress", + }, + }, + }, + } + 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) + } +}