From 4bc3a0cce9a89e2c979255ecaf28e9bedbd047d0 Mon Sep 17 00:00:00 2001
From: Minhan Xia <mixia@google.com>
Date: Fri, 14 Dec 2018 10:40:54 -0800
Subject: [PATCH] bug fix: correctly reflect readiness probe in health check
 for NEG enabled ClusterIP service backend

---
 pkg/controller/translator/translator.go      | 8 +++++++-
 pkg/controller/translator/translator_test.go | 6 ++++--
 pkg/neg/controller.go                        | 2 +-
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/pkg/controller/translator/translator.go b/pkg/controller/translator/translator.go
index 831b97b1f2..993bbc08c7 100644
--- a/pkg/controller/translator/translator.go
+++ b/pkg/controller/translator/translator.go
@@ -327,9 +327,15 @@ OuterLoop:
 		service = *as.(*api_v1.Service)
 		for _, sp := range service.Spec.Ports {
 			svcPort = sp
+			// If service is NEG enabled, compare the service name and namespace instead
+			// This is because NEG enabled service is not required to have nodePort
+			if port.NEGEnabled && port.ID.Service.Namespace == service.Namespace && port.ID.Service.Name == service.Name && port.Port == sp.Port {
+				found = true
+				break OuterLoop
+			}
 			// only one Service can match this nodePort, try and look up
 			// the readiness probe of the pods behind it
-			if int32(port.NodePort) == sp.NodePort {
+			if port.NodePort != 0 && int32(port.NodePort) == sp.NodePort {
 				found = true
 				break OuterLoop
 			}
diff --git a/pkg/controller/translator/translator_test.go b/pkg/controller/translator/translator_test.go
index 8916230d4f..24808b20a5 100644
--- a/pkg/controller/translator/translator_test.go
+++ b/pkg/controller/translator/translator_test.go
@@ -326,6 +326,8 @@ func TestGetProbe(t *testing.T) {
 		{NodePort: 3001, Protocol: annotations.ProtocolHTTP}:  "/healthz",
 		{NodePort: 3002, Protocol: annotations.ProtocolHTTPS}: "/foo",
 		{NodePort: 3003, Protocol: annotations.ProtocolHTTP2}: "/http2-check",
+		{NodePort: 0, Protocol: annotations.ProtocolHTTPS, NEGEnabled: true,
+			ID: utils.ServicePortID{Service: types.NamespacedName{Name: "svc0", Namespace: apiv1.NamespaceDefault}}}: "/bar",
 	}
 	for _, svc := range makeServices(nodePortToHealthCheck, apiv1.NamespaceDefault) {
 		translator.ctx.ServiceInformer.GetIndexer().Add(svc)
@@ -431,7 +433,7 @@ func makePods(nodePortToHealthCheck map[utils.ServicePort]string, ns string) []*
 		pod := &apiv1.Pod{
 			ObjectMeta: metav1.ObjectMeta{
 				Labels:            map[string]string{fmt.Sprintf("app-%d", np.NodePort): "test"},
-				Name:              fmt.Sprintf("%d", np.NodePort),
+				Name:              fmt.Sprintf("pod%d", np.NodePort),
 				Namespace:         ns,
 				CreationTimestamp: metav1.NewTime(firstPodCreationTime.Add(delay)),
 			},
@@ -466,7 +468,7 @@ func makeServices(nodePortToHealthCheck map[utils.ServicePort]string, ns string)
 	for np := range nodePortToHealthCheck {
 		svc := &apiv1.Service{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      fmt.Sprintf("%d", np.NodePort),
+				Name:      fmt.Sprintf("svc%d", np.NodePort),
 				Namespace: ns,
 			},
 			Spec: apiv1.ServiceSpec{
diff --git a/pkg/neg/controller.go b/pkg/neg/controller.go
index fb9527ddd3..54d9901eb0 100644
--- a/pkg/neg/controller.go
+++ b/pkg/neg/controller.go
@@ -331,7 +331,7 @@ func (c *Controller) syncNegStatusAnnotation(namespace, name string, portMap neg
 		if _, ok := service.Annotations[annotations.NEGStatusKey]; ok {
 			// TODO: use PATCH to remove annotation
 			delete(service.Annotations, annotations.NEGStatusKey)
-			glog.V(2).Infof("Removing expose NEG annotation from service: %s/%s", namespace, name)
+			glog.V(2).Infof("Removing NEG status annotation from service: %s/%s", namespace, name)
 			_, err = svcClient.Update(service)
 			return err
 		}