diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index f4fae7f13e3..9d57e8e41f7 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -685,22 +685,23 @@ func validateProbe(probe *api.Probe) errs.ValidationErrorList { return allErrs } -// AccumulateUniquePorts runs an extraction function on each Port of each Container, +// AccumulateUniqueHostPorts extracts each HostPort of each Container, // accumulating the results and returning an error if any ports conflict. -func AccumulateUniquePorts(containers []api.Container, accumulator map[int]bool, extract func(*api.ContainerPort) int) errs.ValidationErrorList { +func AccumulateUniqueHostPorts(containers []api.Container, accumulator *util.StringSet) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} for ci, ctr := range containers { cErrs := errs.ValidationErrorList{} for pi := range ctr.Ports { - port := extract(&ctr.Ports[pi]) + port := ctr.Ports[pi].HostPort if port == 0 { continue } - if accumulator[port] { - cErrs = append(cErrs, errs.NewFieldDuplicate("port", port)) + str := fmt.Sprintf("%d/%s", port, ctr.Ports[pi].Protocol) + if accumulator.Has(str) { + cErrs = append(cErrs, errs.NewFieldDuplicate("port", str)) } else { - accumulator[port] = true + accumulator.Insert(str) } } allErrs = append(allErrs, cErrs.PrefixIndex(ci)...) @@ -711,8 +712,8 @@ func AccumulateUniquePorts(containers []api.Container, accumulator map[int]bool, // checkHostPortConflicts checks for colliding Port.HostPort values across // a slice of containers. func checkHostPortConflicts(containers []api.Container) errs.ValidationErrorList { - allPorts := map[int]bool{} - return AccumulateUniquePorts(containers, allPorts, func(p *api.ContainerPort) int { return p.HostPort }) + allPorts := util.StringSet{} + return AccumulateUniqueHostPorts(containers, &allPorts) } func validateExecAction(exec *api.ExecAction) errs.ValidationErrorList { @@ -829,10 +830,6 @@ func validateContainers(containers []api.Container, volumes util.StringSet) errs allErrs = append(allErrs, cErrs.PrefixIndex(i)...) } // Check for colliding ports across all containers. - // TODO(thockin): This really is dependent on the network config of the host (IP per pod?) - // and the config of the new manifest. But we have not specced that out yet, so we'll just - // make some assumptions for now. As of now, pods share a network namespace, which means that - // every Port.HostPort across the whole pod must be unique. allErrs = append(allErrs, checkHostPortConflicts(containers)...) return allErrs diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index f7978e53120..2f212de4839 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -814,6 +814,15 @@ func TestValidateContainers(t *testing.T) { }, ImagePullPolicy: "IfNotPresent", }, + { + Name: "same-host-port-different-protocol", + Image: "image", + Ports: []api.ContainerPort{ + {ContainerPort: 80, HostPort: 80, Protocol: "TCP"}, + {ContainerPort: 80, HostPort: 80, Protocol: "UDP"}, + }, + ImagePullPolicy: "IfNotPresent", + }, {Name: "abc-1234", Image: "image", ImagePullPolicy: "IfNotPresent", SecurityContext: fakeValidSecurityContext(true)}, } if errs := validateContainers(successCase, volumes); len(errs) != 0 { diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 6cb89206e5c..3293015ce78 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1374,14 +1374,13 @@ func (s podsByCreationTime) Less(i, j int) bool { // checkHostPortConflicts detects pods with conflicted host ports. func checkHostPortConflicts(pods []*api.Pod) (fitting []*api.Pod, notFitting []*api.Pod) { - ports := map[int]bool{} - extract := func(p *api.ContainerPort) int { return p.HostPort } + ports := util.StringSet{} // Respect the pod creation order when resolving conflicts. sort.Sort(podsByCreationTime(pods)) for _, pod := range pods { - if errs := validation.AccumulateUniquePorts(pod.Spec.Containers, ports, extract); len(errs) != 0 { + if errs := validation.AccumulateUniqueHostPorts(pod.Spec.Containers, &ports); len(errs) != 0 { glog.Errorf("Pod %q: HostPort is already allocated, ignoring: %v", kubecontainer.GetPodFullName(pod), errs) notFitting = append(notFitting, pod) continue