Merge pull request #4801 from brendandburns/default_port

Clean up "default" service port handling.

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 findDefaultPort(pod *api.Pod, servicePort int) (int, bool) {
-func findPort(pod *api.Pod, portName util.IntOrString) (int, error) {
+	foundPorts := []int{}
-	firstContainerPort := 0
+	for _, container := range pod.Spec.Containers {
-	if len(pod.Spec.Containers) > 0 && len(pod.Spec.Containers[0].Ports) > 0 {
+		for _, port := range container.Ports {
-		firstContainerPort = pod.Spec.Containers[0].Ports[0].ContainerPort
+			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 {
+			if port, found := findDefaultPort(pod, service.Spec.Port); found {
-				return firstContainerPort, nil
+				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 {
+			if port, found := findDefaultPort(pod, service.Spec.Port); found {
-				return firstContainerPort, nil
+				return port, nil
 			}
 			break
 		}

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
@@ -133,33 +169,51 @@ func TestFindPort(t *testing.T) {
 			0,
 			true,
 		},
-		{
-			pod,
-			util.IntOrString{Kind: util.IntstrString, StrVal: ""},
-			8080,
-			false,
-		},
-		{
-			pod,
-			util.IntOrString{Kind: util.IntstrInt, IntVal: 0},
-			8080,
-			false,
-		},
 		{
 			emptyPortsPod,
-			util.IntOrString{Kind: util.IntstrString, StrVal: ""},
+			util.IntOrString{Kind: util.IntstrString, StrVal: "foo"},
 			0,
 			true,
 		},
+		{
+			emptyPortsPod,
+			util.IntOrString{Kind: util.IntstrString, StrVal: ""},
+			servicePort,
+			false,
+		},
 		{
 			emptyPortsPod,
 			util.IntOrString{Kind: util.IntstrInt, IntVal: 0},
-			0,
+			servicePort,
-			true,
+			false,
+		},
+		{
+			singlePortPod,
+			util.IntOrString{Kind: util.IntstrString, StrVal: ""},
+			8300,
+			false,
+		},
+		{
+			singlePortPod,
+			util.IntOrString{Kind: util.IntstrInt, IntVal: 0},
+			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)
 		}