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/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",
},
},
},