From c7bf373d985575ad6f33d235c246d2cd13730df8 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Mon, 18 May 2015 22:18:40 -0700 Subject: [PATCH 1/2] Allow same-hostport-different-protocol --- pkg/api/validation/validation.go | 27 ++++++++++++--------------- pkg/api/validation/validation_test.go | 9 +++++++++ pkg/kubelet/kubelet.go | 5 +---- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index bc47d2be2ee..a89eb0ab7fd 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -673,22 +673,23 @@ func validateProbe(probe *api.Probe) errs.ValidationErrorList { return allErrs } -// AccumulateUniquePorts runs an extraction function on each Port of each Container, +// accumulateUniquePorts runs an extraction function on each Port 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 map[string]bool) 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[str] { + cErrs = append(cErrs, errs.NewFieldDuplicate("port", str)) } else { - accumulator[port] = true + accumulator[str] = true } } allErrs = append(allErrs, cErrs.PrefixIndex(ci)...) @@ -696,11 +697,11 @@ func AccumulateUniquePorts(containers []api.Container, accumulator map[int]bool, return allErrs } -// checkHostPortConflicts checks for colliding Port.HostPort values across +// ValidateHostPorts 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 }) +func ValidateHostPorts(containers []api.Container) errs.ValidationErrorList { + allPorts := map[string]bool{} + return accumulateUniqueHostPorts(containers, allPorts) } func validateExecAction(exec *api.ExecAction) errs.ValidationErrorList { @@ -817,11 +818,7 @@ 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)...) + allErrs = append(allErrs, ValidateHostPorts(containers)...) return allErrs } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 0bf0a40c1fb..8cb44f15e46 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -785,6 +785,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 1320c531d18..d6cb3209894 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1354,14 +1354,11 @@ 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 } - // 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.ValidateHostPorts(pod.Spec.Containers); len(errs) != 0 { glog.Errorf("Pod %q: HostPort is already allocated, ignoring: %v", kubecontainer.GetPodFullName(pod), errs) notFitting = append(notFitting, pod) continue From 711fa2f2c6389cbadf430f091da754989b786dda Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Tue, 19 May 2015 10:17:53 -0700 Subject: [PATCH 2/2] fix --- pkg/api/validation/validation.go | 18 +++++++++--------- pkg/kubelet/kubelet.go | 4 +++- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index a89eb0ab7fd..0ac3a6972c2 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -673,9 +673,9 @@ 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 accumulateUniqueHostPorts(containers []api.Container, accumulator map[string]bool) errs.ValidationErrorList { +func AccumulateUniqueHostPorts(containers []api.Container, accumulator *util.StringSet) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} for ci, ctr := range containers { @@ -686,10 +686,10 @@ func accumulateUniqueHostPorts(containers []api.Container, accumulator map[strin continue } str := fmt.Sprintf("%d/%s", port, ctr.Ports[pi].Protocol) - if accumulator[str] { + if accumulator.Has(str) { cErrs = append(cErrs, errs.NewFieldDuplicate("port", str)) } else { - accumulator[str] = true + accumulator.Insert(str) } } allErrs = append(allErrs, cErrs.PrefixIndex(ci)...) @@ -697,11 +697,11 @@ func accumulateUniqueHostPorts(containers []api.Container, accumulator map[strin return allErrs } -// ValidateHostPorts checks for colliding Port.HostPort values across +// checkHostPortConflicts checks for colliding Port.HostPort values across // a slice of containers. -func ValidateHostPorts(containers []api.Container) errs.ValidationErrorList { - allPorts := map[string]bool{} - return accumulateUniqueHostPorts(containers, allPorts) +func checkHostPortConflicts(containers []api.Container) errs.ValidationErrorList { + allPorts := util.StringSet{} + return AccumulateUniqueHostPorts(containers, &allPorts) } func validateExecAction(exec *api.ExecAction) errs.ValidationErrorList { @@ -818,7 +818,7 @@ func validateContainers(containers []api.Container, volumes util.StringSet) errs allErrs = append(allErrs, cErrs.PrefixIndex(i)...) } // Check for colliding ports across all containers. - allErrs = append(allErrs, ValidateHostPorts(containers)...) + allErrs = append(allErrs, checkHostPortConflicts(containers)...) return allErrs } diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index d6cb3209894..cf4f3b5e7fb 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1354,11 +1354,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 := util.StringSet{} + // Respect the pod creation order when resolving conflicts. sort.Sort(podsByCreationTime(pods)) for _, pod := range pods { - if errs := validation.ValidateHostPorts(pod.Spec.Containers); 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