diff --git a/manager/allocator/networkallocator/portallocator.go b/manager/allocator/networkallocator/portallocator.go index 1275b08b07..c950ac18bf 100644 --- a/manager/allocator/networkallocator/portallocator.go +++ b/manager/allocator/networkallocator/portallocator.go @@ -91,13 +91,17 @@ func reconcilePortConfigs(s *api.Service) []*api.PortConfig { return s.Spec.Endpoint.Ports } - allocatedPorts := make(map[api.PortConfig]*api.PortConfig) + allocatedPorts := make(map[api.PortConfig]map[uint32]*api.PortConfig) for _, portState := range s.Endpoint.Ports { if portState.PublishMode != api.PublishModeIngress { continue } - allocatedPorts[getPortConfigKey(portState)] = portState + portKey := getPortConfigKey(portState) + if _, ok := allocatedPorts[portKey]; !ok { + allocatedPorts[portKey] = make(map[uint32]*api.PortConfig) + } + allocatedPorts[portKey][portState.PublishedPort] = portState } var portConfigs []*api.PortConfig @@ -109,18 +113,27 @@ func reconcilePortConfigs(s *api.Service) []*api.PortConfig { continue } - portState, ok := allocatedPorts[getPortConfigKey(portConfig)] - // If the portConfig is exactly the same as portState // except if SwarmPort is not user-define then prefer // portState to ensure sticky allocation of the same // port that was allocated before. - if ok && portConfig.Name == portState.Name && - portConfig.TargetPort == portState.TargetPort && - portConfig.Protocol == portState.Protocol && - portConfig.PublishedPort == 0 { - portConfigs = append(portConfigs, portState) - continue + if portConfig.PublishedPort == 0 { + portKey := getPortConfigKey(portConfig) + + if portStateMap, ok := allocatedPorts[portKey]; ok { + // Find the non-zero port for portState + var portState *api.PortConfig + for publishedPort, v := range portStateMap { + if publishedPort != 0 { + portState = v + break + } + } + if portState != nil { + portConfigs = append(portConfigs, portState) + continue + } + } } // For all other cases prefer the portConfig @@ -213,13 +226,17 @@ func (pa *portAllocator) isPortsAllocated(s *api.Service) bool { return false } - allocatedPorts := make(map[api.PortConfig]*api.PortConfig) + allocatedPorts := make(map[api.PortConfig]map[uint32]*api.PortConfig) for _, portState := range s.Endpoint.Ports { if portState.PublishMode != api.PublishModeIngress { continue } - allocatedPorts[getPortConfigKey(portState)] = portState + portKey := getPortConfigKey(portState) + if _, ok := allocatedPorts[portKey]; !ok { + allocatedPorts[portKey] = make(map[uint32]*api.PortConfig) + } + allocatedPorts[portKey][portState.PublishedPort] = portState } for _, portConfig := range s.Spec.Endpoint.Ports { @@ -228,7 +245,9 @@ func (pa *portAllocator) isPortsAllocated(s *api.Service) bool { continue } - portState, ok := allocatedPorts[getPortConfigKey(portConfig)] + portKey := getPortConfigKey(portConfig) + + portStateMap, ok := allocatedPorts[portKey] // If name, port, protocol values don't match then we // are not allocated. @@ -236,18 +255,27 @@ func (pa *portAllocator) isPortsAllocated(s *api.Service) bool { return false } - // If SwarmPort was user defined but the port state - // SwarmPort doesn't match we are not allocated. - if portConfig.PublishedPort != portState.PublishedPort && - portConfig.PublishedPort != 0 { - return false - } + if portConfig.PublishedPort != 0 { + // If SwarmPort was user defined but the port state + // SwarmPort doesn't match we are not allocated. + _, ok := portStateMap[portConfig.PublishedPort] - // If SwarmPort was not defined by user and port state - // is not initialized with a valid SwarmPort value then - // we are not allocated. - if portConfig.PublishedPort == 0 && portState.PublishedPort == 0 { - return false + if !ok { + return false + } + } else { + // If PublishedPort == 0 and we don't have non-zero port + // then we are not allocated + ok := false + for publishedPort := range portStateMap { + if publishedPort != 0 { + ok = true + break + } + } + if !ok { + return false + } } } diff --git a/manager/allocator/networkallocator/portallocator_test.go b/manager/allocator/networkallocator/portallocator_test.go index ed20d05934..18d95a4b6d 100644 --- a/manager/allocator/networkallocator/portallocator_test.go +++ b/manager/allocator/networkallocator/portallocator_test.go @@ -550,6 +550,43 @@ func TestIsPortsAllocated(t *testing.T) { }, expect: true, }, + { + // Endpoint and Spec.Endpoint have multiple PublishedPort + // See docker/docker#29730 + input: &api.Service{ + Spec: api.ServiceSpec{ + Endpoint: &api.EndpointSpec{ + Ports: []*api.PortConfig{ + { + Protocol: api.ProtocolTCP, + TargetPort: 80, + PublishedPort: 5000, + }, + { + Protocol: api.ProtocolTCP, + TargetPort: 80, + PublishedPort: 5001, + }, + }, + }, + }, + Endpoint: &api.Endpoint{ + Ports: []*api.PortConfig{ + { + Protocol: api.ProtocolTCP, + TargetPort: 80, + PublishedPort: 5000, + }, + { + Protocol: api.ProtocolTCP, + TargetPort: 80, + PublishedPort: 5001, + }, + }, + }, + }, + expect: true, + }, } for _, singleTest := range testCases {