Merge pull request kubernetes#4801 from brendandburns/default_port

Clean up "default" service port handling.
duda · Mar 5, 2015 · a1dd6a9 · a1dd6a9
2 parents 8b627f5 + f8dd4fc
commit a1dd6a9
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 21 deletions.
diff --git a/pkg/service/endpoints_controller.go b/pkg/service/endpoints_controller.go
@@ -71,7 +71,7 @@ func (e *EndpointController) SyncServiceEndpoints() error {
 			// assume that service.Spec.ContainerPort is populated.
 			_ = v1beta1.Dependency
 			_ = v1beta2.Dependency
-			port, err := findPort(&pod, service.Spec.ContainerPort)
+			port, err := findPort(&pod, &service)
 			if err != nil {
 				glog.Errorf("Failed to find port for service %s/%s: %v", service.Namespace, service.Name, err)
 				continue
@@ -156,18 +156,30 @@ func endpointsEqual(eps *api.Endpoints, endpoints []api.Endpoint) bool {
 	return true
 }
 
-// findPort locates the container port for the given manifest and portName.
-func findPort(pod *api.Pod, portName util.IntOrString) (int, error) {
-	firstContainerPort := 0
-	if len(pod.Spec.Containers) > 0 && len(pod.Spec.Containers[0].Ports) > 0 {
-		firstContainerPort = pod.Spec.Containers[0].Ports[0].ContainerPort
+func findDefaultPort(pod *api.Pod, servicePort int) (int, bool) {
+	foundPorts := []int{}
+	for _, container := range pod.Spec.Containers {
+		for _, port := range container.Ports {
+			foundPorts = append(foundPorts, port.ContainerPort)
+		}
+	}
+	if len(foundPorts) == 0 {
+		return servicePort, true
 	}
+	if len(foundPorts) == 1 {
+		return foundPorts[0], true
+	}
+	return 0, false
+}
 
+// findPort locates the container port for the given manifest and portName.
+func findPort(pod *api.Pod, service *api.Service) (int, error) {
+	portName := service.Spec.ContainerPort
 	switch portName.Kind {
 	case util.IntstrString:
 		if len(portName.StrVal) == 0 {
-			if firstContainerPort != 0 {
-				return firstContainerPort, nil
+			if port, found := findDefaultPort(pod, service.Spec.Port); found {
+				return port, nil
 			}
 			break
 		}
@@ -181,8 +193,8 @@ func findPort(pod *api.Pod, portName util.IntOrString) (int, error) {
 		}
 	case util.IntstrInt:
 		if portName.IntVal == 0 {
-			if firstContainerPort != 0 {
-				return firstContainerPort, nil
+			if port, found := findDefaultPort(pod, service.Spec.Port); found {
+				return port, nil
 			}
 			break
 		}

diff --git a/pkg/service/endpoints_controller_test.go b/pkg/service/endpoints_controller_test.go
@@ -80,6 +80,11 @@ func TestFindPort(t *testing.T) {
 							ContainerPort: 8000,
 							HostPort:      9000,
 						},
+						{
+							Name:          "default",
+							ContainerPort: 8100,
+							HostPort:      9200,
+						},
 					},
 				},
 			},
@@ -96,6 +101,37 @@ func TestFindPort(t *testing.T) {
 		},
 	}
 
+	singlePortPod := api.Pod{
+		Spec: api.PodSpec{
+			Containers: []api.Container{
+				{
+					Ports: []api.ContainerPort{
+						{
+							ContainerPort: 8300,
+						},
+					},
+				},
+			},
+		},
+	}
+
+	noDefaultPod := api.Pod{
+		Spec: api.PodSpec{
+			Containers: []api.Container{
+				{
+					Ports: []api.ContainerPort{
+						{
+							Name:          "foo",
+							ContainerPort: 8300,
+						},
+					},
+				},
+			},
+		},
+	}
+
+	servicePort := 999
+
 	tests := []struct {
 		pod      api.Pod
 		portName util.IntOrString
@@ -134,32 +170,50 @@ func TestFindPort(t *testing.T) {
 			true,
 		},
 		{
-			pod,
+			emptyPortsPod,
+			util.IntOrString{Kind: util.IntstrString, StrVal: "foo"},
+			0,
+			true,
+		},
+		{
+			emptyPortsPod,
 			util.IntOrString{Kind: util.IntstrString, StrVal: ""},
-			8080,
+			servicePort,
 			false,
 		},
 		{
-			pod,
+			emptyPortsPod,
 			util.IntOrString{Kind: util.IntstrInt, IntVal: 0},
-			8080,
+			servicePort,
 			false,
 		},
 		{
-			emptyPortsPod,
+			singlePortPod,
 			util.IntOrString{Kind: util.IntstrString, StrVal: ""},
-			0,
-			true,
+			8300,
+			false,
 		},
 		{
-			emptyPortsPod,
+			singlePortPod,
 			util.IntOrString{Kind: util.IntstrInt, IntVal: 0},
-			0,
-			true,
+			8300,
+			false,
+		},
+		{
+			noDefaultPod,
+			util.IntOrString{Kind: util.IntstrString, StrVal: ""},
+			8300,
+			false,
+		},
+		{
+			noDefaultPod,
+			util.IntOrString{Kind: util.IntstrInt, IntVal: 0},
+			8300,
+			false,
 		},
 	}
 	for _, test := range tests {
-		port, err := findPort(&test.pod, test.portName)
+		port, err := findPort(&test.pod, &api.Service{Spec: api.ServiceSpec{Port: servicePort, ContainerPort: test.portName}})
 		if port != test.wport {
 			t.Errorf("Expected port %d, Got %d", test.wport, port)
 		}