From 7fbc616b4779d6290afcb7b97709eb784424df0b 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 --- command/service/create.go | 2 - command/service/opts.go | 54 +++++------------------ command/service/update.go | 79 +++++++++++----------------------- command/service/update_test.go | 1 + command/stack/deploy.go | 3 +- 5 files changed, 37 insertions(+), 102 deletions(-) diff --git a/command/service/create.go b/command/service/create.go index ea078e43ad..a8382835a0 100644 --- a/command/service/create.go +++ b/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/command/service/opts.go b/command/service/opts.go index 023b922a15..c7518e5976 100644 --- a/command/service/opts.go +++ b/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/command/service/update.go b/command/service/update.go index 200f58c3a6..2ceaf275ab 100644 --- a/command/service/update.go +++ b/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/command/service/update_test.go b/command/service/update_test.go index bb2e9c1073..3cb7657996 100644 --- a/command/service/update_test.go +++ b/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/command/stack/deploy.go b/command/stack/deploy.go index 1f41cb7d89..00a7634a0a 100644 --- a/command/stack/deploy.go +++ b/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