From 162ef5d1582c600baaa57e1f7bde47d63b5f32d1 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Thu, 15 Dec 2016 07:24:43 +0100 Subject: [PATCH] Make --publish-rm precedes --publish-add, so that add wins `--publish-add 8081:81 --publish-add 8082:82 --publish-rm 80 --publish-rm 81/tcp --publish-rm 82/tcp` would thus result in 81 and 82 to be published. Signed-off-by: Vincent Demeester --- cli/command/service/update.go | 43 ++++++++++++------------ cli/command/service/update_test.go | 36 +++++++++----------- integration-cli/docker_cli_swarm_test.go | 6 ++-- opts/port.go | 9 +++++ opts/port_test.go | 26 +++++++++++--- 5 files changed, 70 insertions(+), 50 deletions(-) diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 2ceaf275ab..4bbcf35a8d 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -630,15 +630,7 @@ func (r byPortConfig) Less(i, j int) bool { func portConfigToString(portConfig *swarm.PortConfig) string { protocol := portConfig.Protocol - if protocol == "" { - protocol = "tcp" - } - mode := portConfig.PublishMode - if mode == "" { - mode = "ingress" - } - return fmt.Sprintf("%v:%v/%s/%s", portConfig.PublishedPort, portConfig.TargetPort, protocol, mode) } @@ -663,28 +655,18 @@ func validatePublishRemove(val string) (string, error) { func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) error { // The key of the map is `port/protocol`, e.g., `80/tcp` portSet := map[string]swarm.PortConfig{} - // 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 && 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) - } - portSet[portConfigToString(&port)] = port - } - } - - // Override previous PortConfig in service if there is any duplicate + // Build the current list of portConfig for _, entry := range *portConfig { if _, ok := portSet[portConfigToString(&entry)]; !ok { portSet[portConfigToString(&entry)] = entry } } - toRemove := flags.Lookup(flagPublishRemove).Value.(*opts.PortOpt).Value() - newPorts := []swarm.PortConfig{} + + // Clean current ports + toRemove := flags.Lookup(flagPublishRemove).Value.(*opts.PortOpt).Value() portLoop: for _, port := range portSet { for _, pConfig := range toRemove { @@ -698,6 +680,23 @@ portLoop: 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.Sort(byPortConfig(newPorts)) *portConfig = newPorts diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index 3cb7657996..08fe248769 100644 --- a/cli/command/service/update_test.go +++ b/cli/command/service/update_test.go @@ -220,28 +220,18 @@ func TestUpdatePorts(t *testing.T) { assert.Equal(t, targetPorts[1], 1000) } -func TestUpdatePortsDuplicateEntries(t *testing.T) { +func TestUpdatePortsDuplicate(t *testing.T) { // Test case for #25375 flags := newUpdateCommand(nil).Flags() flags.Set("publish-add", "80:80") 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) @@ -355,13 +345,19 @@ func TestUpdatePortsRmWithProtocol(t *testing.T) { flags.Set("publish-rm", "82/udp") portConfigs := []swarm.PortConfig{ - {TargetPort: 80, PublishedPort: 8080, Protocol: swarm.PortConfigProtocolTCP}, + { + TargetPort: 80, + PublishedPort: 8080, + Protocol: swarm.PortConfigProtocolTCP, + PublishMode: swarm.PortConfigPublishModeIngress, + }, } err := updatePorts(flags, &portConfigs) 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 diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index e91ec4b03d..b0f2de7a0a 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/integration-cli/docker_cli_swarm_test.go @@ -264,13 +264,13 @@ func (s *DockerSwarmSuite) TestSwarmPublishAdd(c *check.C) { 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", tc.publishAdd[0], tc.name) + 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", tc.publishAdd[1], tc.name) + out, err = d.cmdRetryOutOfSequence("service", "update", "--publish-add", tc.publishAdd[1], tc.name) c.Assert(err, checker.IsNil, check.Commentf(out)) - out, err = d.CmdRetryOutOfSequence("service", "update", "--publish-add", tc.publishAdd[2], "--publish-add", tc.publishAdd[3], tc.name) + 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) diff --git a/opts/port.go b/opts/port.go index e1c4449a38..020a5d1e1c 100644 --- a/opts/port.go +++ b/opts/port.go @@ -82,6 +82,14 @@ func (p *PortOpt) Set(value string) error { return fmt.Errorf("missing mandatory field %q", portOptTargetPort) } + if pConfig.PublishMode == "" { + pConfig.PublishMode = swarm.PortConfigPublishModeIngress + } + + if pConfig.Protocol == "" { + pConfig.Protocol = swarm.PortConfigProtocolTCP + } + p.ports = append(p.ports, pConfig) } else { // short syntax @@ -131,6 +139,7 @@ func ConvertPortToPortConfig( Protocol: swarm.PortConfigProtocol(strings.ToLower(port.Proto())), TargetPort: uint32(port.Int()), PublishedPort: uint32(hostPort), + PublishMode: swarm.PortConfigPublishModeIngress, }) } return ports diff --git a/opts/port_test.go b/opts/port_test.go index 8046b4428e..67bcf8f1d9 100644 --- a/opts/port_test.go +++ b/opts/port_test.go @@ -16,8 +16,9 @@ func TestPortOptValidSimpleSyntax(t *testing.T) { value: "80", expected: []swarm.PortConfig{ { - Protocol: "tcp", - TargetPort: 80, + Protocol: "tcp", + TargetPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, }, }, }, @@ -28,6 +29,7 @@ func TestPortOptValidSimpleSyntax(t *testing.T) { Protocol: "tcp", TargetPort: 8080, PublishedPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, }, }, }, @@ -38,6 +40,7 @@ func TestPortOptValidSimpleSyntax(t *testing.T) { Protocol: "tcp", TargetPort: 80, PublishedPort: 8080, + PublishMode: swarm.PortConfigPublishModeIngress, }, }, }, @@ -48,6 +51,7 @@ func TestPortOptValidSimpleSyntax(t *testing.T) { Protocol: "udp", TargetPort: 8080, PublishedPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, }, }, }, @@ -58,11 +62,13 @@ func TestPortOptValidSimpleSyntax(t *testing.T) { Protocol: "tcp", TargetPort: 8080, PublishedPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, }, { Protocol: "tcp", TargetPort: 8081, PublishedPort: 81, + PublishMode: swarm.PortConfigPublishModeIngress, }, }, }, @@ -73,16 +79,19 @@ func TestPortOptValidSimpleSyntax(t *testing.T) { 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, }, }, }, @@ -106,7 +115,9 @@ func TestPortOptValidComplexSyntax(t *testing.T) { value: "target=80", expected: []swarm.PortConfig{ { - TargetPort: 80, + TargetPort: 80, + Protocol: "tcp", + PublishMode: swarm.PortConfigPublishModeIngress, }, }, }, @@ -114,8 +125,9 @@ func TestPortOptValidComplexSyntax(t *testing.T) { value: "target=80,protocol=tcp", expected: []swarm.PortConfig{ { - Protocol: "tcp", - TargetPort: 80, + Protocol: "tcp", + TargetPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, }, }, }, @@ -126,6 +138,7 @@ func TestPortOptValidComplexSyntax(t *testing.T) { Protocol: "tcp", TargetPort: 80, PublishedPort: 8080, + PublishMode: swarm.PortConfigPublishModeIngress, }, }, }, @@ -136,6 +149,7 @@ func TestPortOptValidComplexSyntax(t *testing.T) { Protocol: "tcp", TargetPort: 8080, PublishedPort: 80, + PublishMode: swarm.PortConfigPublishModeIngress, }, }, }, @@ -157,6 +171,7 @@ func TestPortOptValidComplexSyntax(t *testing.T) { TargetPort: 80, PublishedPort: 8080, PublishMode: "host", + Protocol: "tcp", }, }, }, @@ -167,6 +182,7 @@ func TestPortOptValidComplexSyntax(t *testing.T) { TargetPort: 80, PublishedPort: 8080, PublishMode: "ingress", + Protocol: "tcp", }, }, },