From 0c58170ad0d55445e6c5caf1640cab94e0ea05c4 Mon Sep 17 00:00:00 2001 From: kargakis Date: Fri, 6 Nov 2015 12:34:42 +0100 Subject: [PATCH] Move port utility out of endpoints controller --- pkg/api/pod/util.go | 47 ++++++++ pkg/api/pod/util_test.go | 110 ++++++++++++++++++ .../endpoint/endpoints_controller.go | 28 +---- .../endpoint/endpoints_controller_test.go | 86 -------------- 4 files changed, 159 insertions(+), 112 deletions(-) create mode 100644 pkg/api/pod/util.go create mode 100644 pkg/api/pod/util_test.go diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go new file mode 100644 index 00000000000..8e70104aae9 --- /dev/null +++ b/pkg/api/pod/util.go @@ -0,0 +1,47 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package pod + +import ( + "fmt" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/util/intstr" +) + +// FindPort locates the container port for the given pod and portName. If the +// targetPort is a number, use that. If the targetPort is a string, look that +// string up in all named ports in all containers in the target pod. If no +// match is found, fail. +func FindPort(pod *api.Pod, svcPort *api.ServicePort) (int, error) { + portName := svcPort.TargetPort + switch portName.Type { + case intstr.String: + name := portName.StrVal + for _, container := range pod.Spec.Containers { + for _, port := range container.Ports { + if port.Name == name && port.Protocol == svcPort.Protocol { + return port.ContainerPort, nil + } + } + } + case intstr.Int: + return portName.IntValue(), nil + } + + return 0, fmt.Errorf("no suitable port for manifest: %s", pod.UID) +} diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go new file mode 100644 index 00000000000..428f2bdcdbc --- /dev/null +++ b/pkg/api/pod/util_test.go @@ -0,0 +1,110 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package pod + +import ( + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/util/intstr" +) + +func TestFindPort(t *testing.T) { + testCases := []struct { + name string + containers []api.Container + port intstr.IntOrString + expected int + pass bool + }{{ + name: "valid int, no ports", + containers: []api.Container{{}}, + port: intstr.FromInt(93), + expected: 93, + pass: true, + }, { + name: "valid int, with ports", + containers: []api.Container{{Ports: []api.ContainerPort{{ + Name: "", + ContainerPort: 11, + Protocol: "TCP", + }, { + Name: "p", + ContainerPort: 22, + Protocol: "TCP", + }}}}, + port: intstr.FromInt(93), + expected: 93, + pass: true, + }, { + name: "valid str, no ports", + containers: []api.Container{{}}, + port: intstr.FromString("p"), + expected: 0, + pass: false, + }, { + name: "valid str, one ctr with ports", + containers: []api.Container{{Ports: []api.ContainerPort{{ + Name: "", + ContainerPort: 11, + Protocol: "UDP", + }, { + Name: "p", + ContainerPort: 22, + Protocol: "TCP", + }, { + Name: "q", + ContainerPort: 33, + Protocol: "TCP", + }}}}, + port: intstr.FromString("q"), + expected: 33, + pass: true, + }, { + name: "valid str, two ctr with ports", + containers: []api.Container{{}, {Ports: []api.ContainerPort{{ + Name: "", + ContainerPort: 11, + Protocol: "UDP", + }, { + Name: "p", + ContainerPort: 22, + Protocol: "TCP", + }, { + Name: "q", + ContainerPort: 33, + Protocol: "TCP", + }}}}, + port: intstr.FromString("q"), + expected: 33, + pass: true, + }} + + for _, tc := range testCases { + port, err := FindPort(&api.Pod{Spec: api.PodSpec{Containers: tc.containers}}, + &api.ServicePort{Protocol: "TCP", TargetPort: tc.port}) + if err != nil && tc.pass { + t.Errorf("unexpected error for %s: %v", tc.name, err) + } + if err == nil && !tc.pass { + t.Errorf("unexpected non-error for %s: %d", tc.name, port) + } + if port != tc.expected { + t.Errorf("wrong result for %s: expected %d, got %d", tc.name, tc.expected, port) + } + } +} diff --git a/pkg/controller/endpoint/endpoints_controller.go b/pkg/controller/endpoint/endpoints_controller.go index 41654ce2c69..9e5fb1f7227 100644 --- a/pkg/controller/endpoint/endpoints_controller.go +++ b/pkg/controller/endpoint/endpoints_controller.go @@ -19,13 +19,13 @@ limitations under the License. package endpoint import ( - "fmt" "reflect" "time" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/endpoints" "k8s.io/kubernetes/pkg/api/errors" + podutil "k8s.io/kubernetes/pkg/api/pod" "k8s.io/kubernetes/pkg/client/cache" client "k8s.io/kubernetes/pkg/client/unversioned" "k8s.io/kubernetes/pkg/controller" @@ -33,7 +33,6 @@ import ( "k8s.io/kubernetes/pkg/labels" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util" - "k8s.io/kubernetes/pkg/util/intstr" "k8s.io/kubernetes/pkg/util/sets" "k8s.io/kubernetes/pkg/util/workqueue" "k8s.io/kubernetes/pkg/watch" @@ -302,7 +301,7 @@ func (e *EndpointController) syncService(key string) { portName := servicePort.Name portProto := servicePort.Protocol - portNum, err := findPort(pod, servicePort) + portNum, err := podutil.FindPort(pod, servicePort) if err != nil { glog.V(4).Infof("Failed to find port for service %s/%s: %v", service.Namespace, service.Name, err) continue @@ -399,26 +398,3 @@ func (e *EndpointController) checkLeftoverEndpoints() { e.queue.Add(key) } } - -// findPort locates the container port for the given pod and portName. If the -// targetPort is a number, use that. If the targetPort is a string, look that -// string up in all named ports in all containers in the target pod. If no -// match is found, fail. -func findPort(pod *api.Pod, svcPort *api.ServicePort) (int, error) { - portName := svcPort.TargetPort - switch portName.Type { - case intstr.String: - name := portName.StrVal - for _, container := range pod.Spec.Containers { - for _, port := range container.Ports { - if port.Name == name && port.Protocol == svcPort.Protocol { - return port.ContainerPort, nil - } - } - } - case intstr.Int: - return portName.IntValue(), nil - } - - return 0, fmt.Errorf("no suitable port for manifest: %s", pod.UID) -} diff --git a/pkg/controller/endpoint/endpoints_controller_test.go b/pkg/controller/endpoint/endpoints_controller_test.go index 1c5d38e52d9..96401e5bf45 100644 --- a/pkg/controller/endpoint/endpoints_controller_test.go +++ b/pkg/controller/endpoint/endpoints_controller_test.go @@ -68,92 +68,6 @@ func addPods(store cache.Store, namespace string, nPods int, nPorts int, nNotRea } } -func TestFindPort(t *testing.T) { - testCases := []struct { - name string - containers []api.Container - port intstr.IntOrString - expected int - pass bool - }{{ - name: "valid int, no ports", - containers: []api.Container{{}}, - port: intstr.FromInt(93), - expected: 93, - pass: true, - }, { - name: "valid int, with ports", - containers: []api.Container{{Ports: []api.ContainerPort{{ - Name: "", - ContainerPort: 11, - Protocol: "TCP", - }, { - Name: "p", - ContainerPort: 22, - Protocol: "TCP", - }}}}, - port: intstr.FromInt(93), - expected: 93, - pass: true, - }, { - name: "valid str, no ports", - containers: []api.Container{{}}, - port: intstr.FromString("p"), - expected: 0, - pass: false, - }, { - name: "valid str, one ctr with ports", - containers: []api.Container{{Ports: []api.ContainerPort{{ - Name: "", - ContainerPort: 11, - Protocol: "UDP", - }, { - Name: "p", - ContainerPort: 22, - Protocol: "TCP", - }, { - Name: "q", - ContainerPort: 33, - Protocol: "TCP", - }}}}, - port: intstr.FromString("q"), - expected: 33, - pass: true, - }, { - name: "valid str, two ctr with ports", - containers: []api.Container{{}, {Ports: []api.ContainerPort{{ - Name: "", - ContainerPort: 11, - Protocol: "UDP", - }, { - Name: "p", - ContainerPort: 22, - Protocol: "TCP", - }, { - Name: "q", - ContainerPort: 33, - Protocol: "TCP", - }}}}, - port: intstr.FromString("q"), - expected: 33, - pass: true, - }} - - for _, tc := range testCases { - port, err := findPort(&api.Pod{Spec: api.PodSpec{Containers: tc.containers}}, - &api.ServicePort{Protocol: "TCP", TargetPort: tc.port}) - if err != nil && tc.pass { - t.Errorf("unexpected error for %s: %v", tc.name, err) - } - if err == nil && !tc.pass { - t.Errorf("unexpected non-error for %s: %d", tc.name, port) - } - if port != tc.expected { - t.Errorf("wrong result for %s: expected %d, got %d", tc.name, tc.expected, port) - } - } -} - type serverResponse struct { statusCode int obj interface{}