From 8a113662042e0d75b12e480765a926feecad419b Mon Sep 17 00:00:00 2001
From: Akshay Gupta <aksgupta@microsoft.com>
Date: Thu, 23 May 2019 16:45:20 -0700
Subject: [PATCH] health_probes: Fix probe generation to return nil when
 service doesn't exists (#206)

---
 pkg/appgw/health_probes.go      | 64 +++++++++++++++++----------------
 pkg/appgw/health_probes_test.go | 44 +++++++++++++++++++++++
 2 files changed, 77 insertions(+), 31 deletions(-)

diff --git a/pkg/appgw/health_probes.go b/pkg/appgw/health_probes.go
index 6304ff6c1..9e03e5924 100644
--- a/pkg/appgw/health_probes.go
+++ b/pkg/appgw/health_probes.go
@@ -70,40 +70,42 @@ func (builder *appGwConfigBuilder) HealthProbesCollection(ingressList [](*v1beta
 }
 
 func (builder *appGwConfigBuilder) generateHealthProbe(backendID backendIdentifier) *network.ApplicationGatewayProbe {
-	probe := defaultProbe()
 	service := builder.k8sContext.GetService(backendID.serviceKey())
-	if service != nil {
-		probe.Name = to.StringPtr(generateProbeName(backendID.Path.Backend.ServiceName, backendID.Path.Backend.ServicePort.String(), backendID.Ingress.Name))
-		if backendID.Rule != nil && len(backendID.Rule.Host) != 0 {
-			probe.Host = to.StringPtr(backendID.Rule.Host)
-		}
+	if service == nil {
+		return nil
+	}
 
-		if len(annotations.BackendPathPrefix(backendID.Ingress)) != 0 {
-			probe.Path = to.StringPtr(annotations.BackendPathPrefix(backendID.Ingress))
-		} else if backendID.Path != nil && len(backendID.Path.Path) != 0 {
-			probe.Path = to.StringPtr(backendID.Path.Path)
-		}
+	probe := defaultProbe()
+	probe.Name = to.StringPtr(generateProbeName(backendID.Path.Backend.ServiceName, backendID.Path.Backend.ServicePort.String(), backendID.Ingress.Name))
+	if backendID.Rule != nil && len(backendID.Rule.Host) != 0 {
+		probe.Host = to.StringPtr(backendID.Rule.Host)
+	}
 
-		k8sProbeForServiceContainer := builder.getProbeForServiceContainer(service, backendID)
-		if k8sProbeForServiceContainer != nil {
-			if len(k8sProbeForServiceContainer.Handler.HTTPGet.Host) != 0 {
-				probe.Host = to.StringPtr(k8sProbeForServiceContainer.Handler.HTTPGet.Host)
-			}
-			if len(k8sProbeForServiceContainer.Handler.HTTPGet.Path) != 0 {
-				probe.Path = to.StringPtr(k8sProbeForServiceContainer.Handler.HTTPGet.Path)
-			}
-			if k8sProbeForServiceContainer.Handler.HTTPGet.Scheme == v1.URISchemeHTTPS {
-				probe.Protocol = network.HTTPS
-			}
-			if k8sProbeForServiceContainer.PeriodSeconds != 0 {
-				probe.Interval = to.Int32Ptr(k8sProbeForServiceContainer.PeriodSeconds)
-			}
-			if k8sProbeForServiceContainer.TimeoutSeconds != 0 {
-				probe.Timeout = to.Int32Ptr(k8sProbeForServiceContainer.TimeoutSeconds)
-			}
-			if k8sProbeForServiceContainer.FailureThreshold != 0 {
-				probe.UnhealthyThreshold = to.Int32Ptr(k8sProbeForServiceContainer.FailureThreshold)
-			}
+	if len(annotations.BackendPathPrefix(backendID.Ingress)) != 0 {
+		probe.Path = to.StringPtr(annotations.BackendPathPrefix(backendID.Ingress))
+	} else if backendID.Path != nil && len(backendID.Path.Path) != 0 {
+		probe.Path = to.StringPtr(backendID.Path.Path)
+	}
+
+	k8sProbeForServiceContainer := builder.getProbeForServiceContainer(service, backendID)
+	if k8sProbeForServiceContainer != nil {
+		if len(k8sProbeForServiceContainer.Handler.HTTPGet.Host) != 0 {
+			probe.Host = to.StringPtr(k8sProbeForServiceContainer.Handler.HTTPGet.Host)
+		}
+		if len(k8sProbeForServiceContainer.Handler.HTTPGet.Path) != 0 {
+			probe.Path = to.StringPtr(k8sProbeForServiceContainer.Handler.HTTPGet.Path)
+		}
+		if k8sProbeForServiceContainer.Handler.HTTPGet.Scheme == v1.URISchemeHTTPS {
+			probe.Protocol = network.HTTPS
+		}
+		if k8sProbeForServiceContainer.PeriodSeconds != 0 {
+			probe.Interval = to.Int32Ptr(k8sProbeForServiceContainer.PeriodSeconds)
+		}
+		if k8sProbeForServiceContainer.TimeoutSeconds != 0 {
+			probe.Timeout = to.Int32Ptr(k8sProbeForServiceContainer.TimeoutSeconds)
+		}
+		if k8sProbeForServiceContainer.FailureThreshold != 0 {
+			probe.UnhealthyThreshold = to.Int32Ptr(k8sProbeForServiceContainer.FailureThreshold)
 		}
 	}
 
diff --git a/pkg/appgw/health_probes_test.go b/pkg/appgw/health_probes_test.go
index afa6eb365..120ef11a1 100644
--- a/pkg/appgw/health_probes_test.go
+++ b/pkg/appgw/health_probes_test.go
@@ -110,4 +110,48 @@ var _ = Describe("configure App Gateway health probes", func() {
 			Expect(*actual).To(ContainElement(probeForOtherHost))
 		})
 	})
+
+	Context("use default probe when service doesn't exists", func() {
+		cb := newConfigBuilderFixture(nil)
+
+		pod := newPodFixture(testFixturesServiceName, testFixturesNamespace, testFixturesContainerName, testFixturesContainerPort)
+		_ = cb.k8sContext.Caches.Pods.Add(pod)
+
+		ingressList := []*v1beta1.Ingress{
+			newIngressFixture(),
+		}
+
+		// !! Action !!
+		_, _ = cb.HealthProbesCollection(ingressList)
+		actual := cb.appGwConfig.Probes
+
+		// We expect our health probe configurator to have arrived at this final setup
+		defaultProbe := network.ApplicationGatewayProbe{
+
+			ApplicationGatewayProbePropertiesFormat: &network.ApplicationGatewayProbePropertiesFormat{
+				Protocol:                            network.HTTP,
+				Host:                                to.StringPtr("localhost"),
+				Path:                                to.StringPtr("/"),
+				Interval:                            to.Int32Ptr(30),
+				Timeout:                             to.Int32Ptr(30),
+				UnhealthyThreshold:                  to.Int32Ptr(3),
+				PickHostNameFromBackendHTTPSettings: nil,
+				MinServers:                          nil,
+				Match:                               nil,
+				ProvisioningState:                   nil,
+			},
+			Name: to.StringPtr(agPrefix + "defaultprobe"),
+			Etag: nil,
+			Type: nil,
+			ID:   nil,
+		}
+
+		It("should have exactly 1 record", func() {
+			Expect(len(*actual)).To(Equal(1))
+		})
+
+		It("should have created 1 default probe", func() {
+			Expect(*actual).To(ContainElement(defaultProbe))
+		})
+	})
 })