From 7d709d5e93ee32339375fc0f5f864d17711bff5c Mon Sep 17 00:00:00 2001
From: bprashanth <beeps@google.com>
Date: Fri, 27 Jan 2017 14:22:44 -0800
Subject: [PATCH] Match named port between container and probe

We were previous matching the target port with the readiness probe, and
hence dropping the case where the container port and the probe had the
same name, but the target port did not.
---
 controllers/gce/controller/util_test.go | 25 +++++++++++++++++++++++-
 controllers/gce/controller/utils.go     | 26 +++++++++++++------------
 2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/controllers/gce/controller/util_test.go b/controllers/gce/controller/util_test.go
index fbc7f0022..a3bbbe120 100644
--- a/controllers/gce/controller/util_test.go
+++ b/controllers/gce/controller/util_test.go
@@ -109,6 +109,29 @@ func TestProbeGetter(t *testing.T) {
 	}
 }
 
+func TestProbeGetterNamedPort(t *testing.T) {
+	cm := NewFakeClusterManager(DefaultClusterUID)
+	lbc := newLoadBalancerController(t, cm, "")
+	nodePortToHealthCheck := map[int64]string{
+		3001: "/healthz",
+	}
+	addPods(lbc, nodePortToHealthCheck, api.NamespaceDefault)
+	for _, p := range lbc.podLister.Indexer.List() {
+		pod := p.(*api.Pod)
+		pod.Spec.Containers[0].Ports[0].Name = "test"
+		pod.Spec.Containers[0].ReadinessProbe.Handler.HTTPGet.Port = intstr.IntOrString{Type: intstr.String, StrVal: "test"}
+	}
+	for p, exp := range nodePortToHealthCheck {
+		got, err := lbc.tr.HealthCheck(p)
+		if err != nil {
+			t.Errorf("Failed to get health check for node port %v: %v", p, err)
+		} else if got.RequestPath != exp {
+			t.Errorf("Wrong health check for node port %v, got %v expected %v", p, got.RequestPath, exp)
+		}
+	}
+
+}
+
 func TestProbeGetterCrossNamespace(t *testing.T) {
 	cm := NewFakeClusterManager(DefaultClusterUID)
 	lbc := newLoadBalancerController(t, cm, "")
@@ -191,7 +214,7 @@ func addPods(lbc *LoadBalancerController, nodePortToHealthCheck map[int64]string
 			Spec: api.PodSpec{
 				Containers: []api.Container{
 					{
-						Ports: []api.ContainerPort{{ContainerPort: 80}},
+						Ports: []api.ContainerPort{{Name: "test", ContainerPort: 80}},
 						ReadinessProbe: &api.Probe{
 							Handler: api.Handler{
 								HTTPGet: &api.HTTPGetAction{
diff --git a/controllers/gce/controller/utils.go b/controllers/gce/controller/utils.go
index b86e811f0..617ce5fad 100644
--- a/controllers/gce/controller/utils.go
+++ b/controllers/gce/controller/utils.go
@@ -410,14 +410,6 @@ func (t *GCETranslator) ListZones() ([]string, error) {
 	return zones.List(), nil
 }
 
-// isPortEqual compares the given IntOrString ports
-func isPortEqual(port, targetPort intstr.IntOrString) bool {
-	if targetPort.Type == intstr.Int {
-		return port.IntVal == targetPort.IntVal
-	}
-	return port.StrVal == targetPort.StrVal
-}
-
 // geHTTPProbe returns the http readiness probe from the first container
 // that matches targetPort, from the set of pods matching the given labels.
 func (t *GCETranslator) getHTTPProbe(svc api.Service, targetPort intstr.IntOrString) (*api.Probe, error) {
@@ -443,11 +435,21 @@ func (t *GCETranslator) getHTTPProbe(svc api.Service, targetPort intstr.IntOrStr
 				continue
 			}
 			for _, p := range c.Ports {
-				cPort := intstr.IntOrString{IntVal: p.ContainerPort, StrVal: p.Name}
-				if isPortEqual(cPort, targetPort) {
-					if isPortEqual(c.ReadinessProbe.Handler.HTTPGet.Port, targetPort) {
-						return c.ReadinessProbe, nil
+				if targetPort.Type == intstr.Int && targetPort.IntVal == p.ContainerPort ||
+					targetPort.Type == intstr.String && targetPort.StrVal == p.Name {
+
+					readinessProbePort := c.ReadinessProbe.Handler.HTTPGet.Port
+					switch readinessProbePort.Type {
+					case intstr.Int:
+						if readinessProbePort.IntVal == p.ContainerPort {
+							return c.ReadinessProbe, nil
+						}
+					case intstr.String:
+						if readinessProbePort.StrVal == p.Name {
+							return c.ReadinessProbe, nil
+						}
 					}
+
 					glog.Infof("%v: found matching targetPort on container %v, but not on readinessProbe (%+v)",
 						logStr, c.Name, c.ReadinessProbe.Handler.HTTPGet.Port)
 				}