Adjust healthcheck Description when BackendConfig used

kubernetes · Mar 14, 2023 · a8f7844 · a8f7844
1 parent fcdd7d6
commit a8f7844
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 10 deletions.
diff --git a/pkg/healthchecks/healthcheck.go b/pkg/healthchecks/healthcheck.go
@@ -72,6 +72,9 @@ func calculateDiff(old, new *translator.HealthCheck, c *backendconfigv1.HealthCh
 	if c.Port != nil && old.Port != new.Port {
 		changes.add("Port", strconv.FormatInt(old.Port, 10), strconv.FormatInt(new.Port, 10))
 	}
+	if old.Description != new.Description {
+		changes.add("Description", old.Description, new.Description)
+	}
 
 	// TODO(bowei): Host seems to be missing.
 

diff --git a/pkg/healthchecks/healthchecks_test.go b/pkg/healthchecks/healthchecks_test.go
@@ -770,6 +770,7 @@ func (*syncSPFixture) hc() *compute.HealthCheck {
 			Port:        80,
 			RequestPath: "/",
 		},
+		Description: translator.DescriptionForDefaultHealthChecks,
 	}
 }
 
@@ -809,13 +810,15 @@ func (*syncSPFixture) neg() *compute.HealthCheck {
 			PortSpecification: "USE_SERVING_PORT",
 			RequestPath:       "/",
 		},
+		Description: translator.DescriptionForDefaultNEGHealthChecks,
 	}
 }
 
 func (f *syncSPFixture) ilb() *compute.HealthCheck {
 	h := f.neg()
 	h.Region = "us-central1"
 	h.SelfLink = ilbSelfLink
+	h.Description = translator.DescriptionForDefaultILBHealthChecks
 	return h
 }
 
@@ -894,6 +897,7 @@ func TestSyncServicePort(t *testing.T) {
 	chc.HttpHealthCheck.Host = "foo.com"
 	chc.CheckIntervalSec = 61
 	chc.TimeoutSec = 1234
+	chc.Description = translator.DescriptionForHealthChecksFromReadinessProbe
 	cases = append(cases, &tc{
 		desc: "create probe",
 		sp:   testSPs["HTTP-80-reg-nil"],
@@ -913,6 +917,7 @@ func TestSyncServicePort(t *testing.T) {
 	chc.HttpHealthCheck.Host = "foo.com"
 	chc.CheckIntervalSec = 1234
 	chc.TimeoutSec = 5678
+	chc.Description = translator.DescriptionForHealthChecksFromReadinessProbe
 	cases = append(cases, &tc{
 		desc: "create probe neg",
 		sp:   testSPs["HTTP-80-neg-nil"],
@@ -932,6 +937,7 @@ func TestSyncServicePort(t *testing.T) {
 	chc.HttpHealthCheck.Host = "foo.com"
 	chc.CheckIntervalSec = 1234
 	chc.TimeoutSec = 5678
+	chc.Description = translator.DescriptionForHealthChecksFromReadinessProbe
 	cases = append(cases, &tc{
 		desc:     "create probe ilb",
 		sp:       testSPs["HTTP-80-ilb-nil"],
@@ -949,6 +955,7 @@ func TestSyncServicePort(t *testing.T) {
 	// BackendConfig
 	chc = fixture.hc()
 	chc.HttpHealthCheck.RequestPath = "/foo"
+	chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
 	cases = append(cases, &tc{desc: "create backendconfig", sp: testSPs["HTTP-80-reg-bc"], wantComputeHC: chc})
 
 	// BackendConfig all
@@ -961,6 +968,7 @@ func TestSyncServicePort(t *testing.T) {
 	chc.HttpHealthCheck.Port = 1234
 	// PortSpecification is set by the controller
 	chc.HttpHealthCheck.PortSpecification = "USE_FIXED_PORT"
+	chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
 	cases = append(cases, &tc{desc: "create backendconfig all", sp: testSPs["HTTP-80-reg-bcall"], wantComputeHC: chc})
 
 	i64 := func(i int64) *int64 { return &i }
@@ -970,6 +978,7 @@ func TestSyncServicePort(t *testing.T) {
 	chc.HttpHealthCheck.Port = 1234
 	// PortSpecification is set by the controller
 	chc.HttpHealthCheck.PortSpecification = "USE_FIXED_PORT"
+	chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
 	sp := utils.ServicePort{
 		NodePort:      80,
 		Protocol:      annotations.ProtocolHTTP,
@@ -981,6 +990,7 @@ func TestSyncServicePort(t *testing.T) {
 	// BackendConfig neg
 	chc = fixture.neg()
 	chc.HttpHealthCheck.RequestPath = "/foo"
+	chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
 	cases = append(cases, &tc{
 		desc:          "create backendconfig neg",
 		sp:            testSPs["HTTP-80-neg-bc"],
@@ -990,6 +1000,7 @@ func TestSyncServicePort(t *testing.T) {
 	// BackendConfig ilb
 	chc = fixture.ilb()
 	chc.HttpHealthCheck.RequestPath = "/foo"
+	chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
 	cases = append(cases, &tc{
 		desc:          "create backendconfig ilb",
 		sp:            testSPs["HTTP-80-ilb-bc"],
@@ -1003,6 +1014,7 @@ func TestSyncServicePort(t *testing.T) {
 	chc.HttpHealthCheck.Host = "foo.com"
 	chc.CheckIntervalSec = 61
 	chc.TimeoutSec = 1234
+	chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
 	cases = append(cases, &tc{
 		desc: "create probe and backendconfig",
 		sp:   testSPs["HTTP-80-reg-bc"],
@@ -1149,6 +1161,7 @@ func TestSyncServicePort(t *testing.T) {
 	wantCHC = fixture.hc()
 	wantCHC.HttpHealthCheck.RequestPath = "/foo" // from bc
 	wantCHC.CheckIntervalSec = 1234              // same
+	wantCHC.Description = translator.DescriptionForHealthChecksFromBackendConfig
 	cases = append(cases, &tc{
 		desc:          "update preserve and backendconfig (path)",
 		sp:            testSPs["HTTP-80-reg-bc"],
@@ -1168,6 +1181,7 @@ func TestSyncServicePort(t *testing.T) {
 	wantCHC.TimeoutSec = 1234
 	wantCHC.HttpHealthCheck.Port = 1234
 	wantCHC.HttpHealthCheck.PortSpecification = "USE_FIXED_PORT"
+	wantCHC.Description = translator.DescriptionForHealthChecksFromBackendConfig
 	cases = append(cases, &tc{
 		desc:          "update preserve backendconfig all",
 		setup:         fixture.setupExistingHCFunc(chc),
@@ -1237,14 +1251,16 @@ func TestSyncServicePort(t *testing.T) {
 				// Filter out fields that are hard to deal with in the mock and
 				// test cases.
 				filter := func(hc *compute.HealthCheck) {
-					hc.Description = ""
 					hc.SelfLink = ""
 				}
-				filter(gotHC)
-				filter(tc.wantComputeHC)
-
-				if !reflect.DeepEqual(gotHC, tc.wantComputeHC) {
-					t.Fatalf("Compute healthcheck is:\n%s, want:\n%s", pretty.Sprint(gotHC), pretty.Sprint(tc.wantComputeHC))
+				// Create copies to be modified by the above func filter as otherwise resync may try to revert modifications.
+				gotHCCopy := *gotHC
+				wantComputeHCCopy := *(tc.wantComputeHC)
+				filter(&gotHCCopy)
+				filter(&wantComputeHCCopy)
+
+				if !reflect.DeepEqual(&gotHCCopy, &wantComputeHCCopy) {
+					t.Fatalf("Compute healthcheck is:\n%s, want:\n%s", pretty.Sprint(&gotHCCopy), pretty.Sprint(&wantComputeHCCopy))
 				}
 			}
 

diff --git a/pkg/translator/healthchecks.go b/pkg/translator/healthchecks.go
@@ -65,6 +65,12 @@ const (
 	// used for health checking.
 	useServingPortSpecification = "USE_SERVING_PORT"
 
+	DescriptionForDefaultHealthChecks            = "Default kubernetes L7 Loadbalancing health check."
+	DescriptionForDefaultNEGHealthChecks         = "Default kubernetes L7 Loadbalancing health check for NEG."
+	DescriptionForDefaultILBHealthChecks         = "Default kubernetes L7 Loadbalancing health check for ILB."
+	DescriptionForHealthChecksFromReadinessProbe = "Kubernetes L7 health check generated with readiness probe settings."
+	DescriptionForHealthChecksFromBackendConfig  = "Kubernetes L7 health check generated with BackendConfig CRD."
+
 	// TODO: revendor the GCE API go client so that this error will not be hit.
 	newHealthCheckErrorMessageTemplate = "the %v health check configuration on the existing health check %v is nil. " +
 		"This is usually caused by an application protocol change on the k8s service spec. " +
@@ -217,6 +223,8 @@ func (hc *HealthCheck) UpdateFromBackendConfig(c *backendconfigv1.HealthCheckCon
 		// This override is necessary regardless of type
 		hc.PortSpecification = "USE_FIXED_PORT"
 	}
+
+	hc.Description = DescriptionForHealthChecksFromBackendConfig
 }
 
 // DefaultHealthCheck simply returns the default health check.
@@ -231,7 +239,7 @@ func DefaultHealthCheck(port int64, protocol annotations.AppProtocol) *HealthChe
 		HealthyThreshold: defaultHealthyThreshold,
 		// Number of healthchecks to fail before the vm is deemed unhealthy.
 		UnhealthyThreshold: defaultUnhealthyThreshold,
-		Description:        "Default kubernetes L7 Loadbalancing health check.",
+		Description:        DescriptionForDefaultHealthChecks,
 		Type:               string(protocol),
 	}
 	return &HealthCheck{
@@ -255,7 +263,7 @@ func DefaultNEGHealthCheck(protocol annotations.AppProtocol) *HealthCheck {
 		HealthyThreshold: defaultHealthyThreshold,
 		// Number of healthchecks to fail before the vm is deemed unhealthy.
 		UnhealthyThreshold: defaultNEGUnhealthyThreshold,
-		Description:        "Default kubernetes L7 Loadbalancing health check for NEG.",
+		Description:        DescriptionForDefaultNEGHealthChecks,
 		Type:               string(protocol),
 	}
 	return &HealthCheck{
@@ -278,7 +286,7 @@ func DefaultILBHealthCheck(protocol annotations.AppProtocol) *HealthCheck {
 		HealthyThreshold: defaultHealthyThreshold,
 		// Number of healthchecks to fail before the vm is deemed unhealthy.
 		UnhealthyThreshold: defaultNEGUnhealthyThreshold,
-		Description:        "Default kubernetes L7 Loadbalancing health check for ILB.",
+		Description:        DescriptionForDefaultILBHealthChecks,
 		Type:               string(protocol),
 	}
 
@@ -326,5 +334,5 @@ func ApplyProbeSettingsToHC(p *v1.Probe, hc *HealthCheck) {
 		hc.CheckIntervalSec = int64(p.PeriodSeconds) + int64(defaultHealthCheckInterval.Seconds())
 	}
 
-	hc.Description = "Kubernetes L7 health check generated with readiness probe settings."
+	hc.Description = DescriptionForHealthChecksFromReadinessProbe
 }