JSONify healthcheck Description for BackendConfig

cmd/glbc/main.go

@@ -328,12 +328,7 @@ func runControllers(ctx *ingctx.ControllerContext) {
 	// In NonGCP mode, use the zone specified in gce.conf directly.
 	// This overrides the zone/fault-domain label on nodes for NEG controller.
 	if flags.F.EnableNonGCPMode {
-		zone, err := ctx.Cloud.GetZone(context.Background())
-		if err != nil {
-			klog.Errorf("Failed to retrieve zone information from Cloud provider: %v; Please check if local-zone is specified in gce.conf.", err)
-		} else {
-			zoneGetter = negtypes.NewSimpleZoneGetter(zone.FailureDomain)
-		}
+		zoneGetter = negtypes.NewSimpleZoneGetter(ctx.Cloud.LocalZone())
 	}
 
 	enableAsm := false

pkg/healthchecks/healthchecks.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/ingress-gce/pkg/loadbalancers/features"
 	"k8s.io/ingress-gce/pkg/translator"
 	"k8s.io/ingress-gce/pkg/utils"
+	"k8s.io/ingress-gce/pkg/utils/healthcheck"
 	"k8s.io/klog/v2"
 )
 
@@ -45,13 +46,27 @@ type HealthChecks struct {
 	defaultBackendSvc types.NamespacedName
 	recorderGetter    RecorderGetter
 	serviceGetter     ServiceGetter
+	clusterInfo       healthcheck.ClusterInfo
 }
 
 // NewHealthChecker creates a new health checker.
 // cloud: the cloud object implementing SingleHealthCheck.
 // defaultHealthCheckPath: is the HTTP path to use for health checks.
 func NewHealthChecker(cloud HealthCheckProvider, healthCheckPath string, defaultBackendSvc types.NamespacedName, recorderGetter RecorderGetter, serviceGetter ServiceGetter) *HealthChecks {
-	return &HealthChecks{cloud, healthCheckPath, defaultBackendSvc, recorderGetter, serviceGetter}
+	ci := generateClusterInfo(cloud.(*gce.Cloud))
+	return &HealthChecks{cloud, healthCheckPath, defaultBackendSvc, recorderGetter, serviceGetter, ci}
+}
+
+func generateClusterInfo(gceCloud *gce.Cloud) healthcheck.ClusterInfo {
+	var location string
+	regionalCluster := gceCloud.Regional()
+	if regionalCluster {
+		location = gceCloud.Region()
+	} else {
+		location = gceCloud.LocalZone()
+	}
+	name := flags.F.GKEClusterName
+	return healthcheck.ClusterInfo{Name: name, Location: location, Regional: regionalCluster}
 }
 
 // new returns a *HealthCheck with default settings and specified port/protocol
@@ -70,6 +85,7 @@ func (h *HealthChecks) new(sp utils.ServicePort) *translator.HealthCheck {
 	hc.Port = sp.NodePort
 	hc.RequestPath = h.pathFromSvcPort(sp)
 	hc.Service = h.getService(sp)
+	hc.SetHealthcheckInfo(h.generateHealthcheckInfo(sp, hc.ForILB))
 	return hc
 }
 
@@ -94,6 +110,19 @@ func (h *HealthChecks) mainService(sp utils.ServicePort) types.NamespacedName {
 	return service
 }
 
+func (h *HealthChecks) generateHealthcheckInfo(sp utils.ServicePort, iLB bool) healthcheck.HealthcheckInfo {
+	serviceInfo := healthcheck.ServiceInfo(h.defaultBackendSvc)
+	if sp.ID.Service.Name != "" {
+		serviceInfo = healthcheck.ServiceInfo(sp.ID.Service)
+	}
+
+	return healthcheck.HealthcheckInfo{
+		ClusterInfo:       h.clusterInfo,
+		ServiceInfo:       serviceInfo,
+		HealthcheckConfig: healthcheck.DefaultHC,
+	}
+}
+
 // SyncServicePort implements HealthChecker.
 func (h *HealthChecks) SyncServicePort(sp *utils.ServicePort, probe *v1.Probe) (string, error) {
 	hc := h.new(*sp)

pkg/healthchecks/healthchecks_test.go

@@ -18,6 +18,7 @@ package healthchecks
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"net/http"
 	"reflect"
@@ -44,6 +45,7 @@ import (
 	"k8s.io/ingress-gce/pkg/loadbalancers/features"
 	"k8s.io/ingress-gce/pkg/translator"
 	"k8s.io/ingress-gce/pkg/utils"
+	"k8s.io/ingress-gce/pkg/utils/healthcheck"
 	namer_util "k8s.io/ingress-gce/pkg/utils/namer"
 	"k8s.io/klog/v2"
 )
@@ -444,18 +446,28 @@ func getSingletonHealthcheck(t *testing.T, c *gce.Cloud) *compute.HealthCheck {
 
 // Test changing the value of the flag EnableUpdateCustomHealthCheckDescription from false to true.
 func TestRolloutUpdateCustomHCDescription(t *testing.T) {
-	// No parallel() because we modify the value of the flags EnableBackendConfigHealthCheck and EnableUpdateCustomHealthCheckDescription.
+	// No parallel() because we modify the value of the flags:
+	// - EnableBackendConfigHealthCheck,
+	// - EnableUpdateCustomHealthCheckDescription,
+	// - GKEClusterName.
+
+	testClusterValues := gce.DefaultTestClusterValues()
+
 	oldEnableBC := flags.F.EnableBackendConfigHealthCheck
 	oldUpdateDescription := flags.F.EnableUpdateCustomHealthCheckDescription
+	oldGKEClusterName := flags.F.GKEClusterName
 	flags.F.EnableBackendConfigHealthCheck = true
 	// Start with EnableUpdateCustomHealthCheckDescription = false.
 	flags.F.EnableUpdateCustomHealthCheckDescription = false
+	flags.F.GKEClusterName = testClusterValues.ClusterName
 	defer func() {
 		flags.F.EnableBackendConfigHealthCheck = oldEnableBC
 		flags.F.EnableUpdateCustomHealthCheckDescription = oldUpdateDescription
+		flags.F.GKEClusterName = oldGKEClusterName
 	}()
 
-	fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
+	testClusterValues.Regional = true
+	fakeGCE := gce.NewFakeGCECloud(testClusterValues)
 
 	var (
 		defaultSP       *utils.ServicePort = testSPs["HTTP-80-reg-nil"]
@@ -506,9 +518,20 @@ func TestRolloutUpdateCustomHCDescription(t *testing.T) {
 
 	outputBCHCWithFlag := getSingletonHealthcheck(t, fakeGCE)
 
-	if outputBCHCWithFlag.Description != translator.DescriptionForHealthChecksFromBackendConfig {
+	wantDesc := healthcheck.HealthcheckDesc{
+		K8sCluster:  fmt.Sprintf("/locations/%s/clusters/%s", gce.DefaultTestClusterValues().Region, gce.DefaultTestClusterValues().ClusterName),
+		K8sResource: fmt.Sprintf("/namespaces/%s/services/%s", defaultBackendSvc.Namespace, defaultBackendSvc.Name),
+		Config:      "BackendConfig",
+	}
+	bytes, err := json.MarshalIndent(wantDesc, "", "    ")
+	if err != nil {
+		t.Fatalf("Error while generating the wantDesc JSON.")
+	}
+	wantDescription := string(bytes)
+
+	if outputBCHCWithFlag.Description != wantDescription {
 		t.Fatalf("incorrect Description, is: \"%v\", want: \"%v\"",
-			outputBCHCWithFlag.Description, translator.DescriptionForHealthChecksFromBackendConfig)
+			outputBCHCWithFlag.Description, wantDescription)
 	}
 
 	// Verify that only the Description is modified after rollout.
@@ -984,21 +1007,28 @@ func setupMockUpdate(mock *cloud.MockGCE) {
 }
 
 func TestSyncServicePort(t *testing.T) {
-	// No parallel() because we modify the value of the flags EnableBackendConfigHealthCheck and EnableUpdateCustomHealthCheckDescription.
+	// No parallel() because we modify the value of the flags:
+	// - EnableBackendConfigHealthCheck,
+	// - EnableUpdateCustomHealthCheckDescription,
+	// - GKEClusterName.	oldEnableBC := flags.F.EnableBackendConfigHealthCheck
 	oldEnableBC := flags.F.EnableBackendConfigHealthCheck
-	oldUpdateDescription := flags.F.EnableUpdateCustomHealthCheckDescription
 	flags.F.EnableBackendConfigHealthCheck = true
+	oldUpdateDescription := flags.F.EnableUpdateCustomHealthCheckDescription
+	oldGKEClusterName := flags.F.GKEClusterName
+	flags.F.GKEClusterName = gce.DefaultTestClusterValues().ClusterName
 	defer func() {
 		flags.F.EnableBackendConfigHealthCheck = oldEnableBC
 		flags.F.EnableUpdateCustomHealthCheckDescription = oldUpdateDescription
+		flags.F.GKEClusterName = oldGKEClusterName
 	}()
 
 	type tc struct {
-		desc     string
-		setup    func(*cloud.MockGCE)
-		sp       *utils.ServicePort
-		probe    *v1.Probe
-		regional bool
+		desc            string
+		setup           func(*cloud.MockGCE)
+		sp              *utils.ServicePort
+		probe           *v1.Probe
+		regional        bool
+		regionalCluster bool
 
 		wantSelfLink        string
 		wantErr             bool
@@ -1132,6 +1162,18 @@ func TestSyncServicePort(t *testing.T) {
 		wantComputeHC: chc,
 	})
 
+	// BackendConfig ilb, regional cluster
+	chc = fixture.ilb()
+	chc.HttpHealthCheck.RequestPath = "/foo"
+	chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
+	cases = append(cases, &tc{
+		desc:            "create backendconfig ilb regional cluster",
+		sp:              testSPs["HTTP-80-ilb-bc"],
+		regional:        true,
+		regionalCluster: true,
+		wantComputeHC:   chc,
+	})
+
 	// Probe and BackendConfig override
 	chc = fixture.hc()
 	chc.HttpHealthCheck.RequestPath = "/foo"
@@ -1339,15 +1381,38 @@ func TestSyncServicePort(t *testing.T) {
 	// settings.
 
 	for _, tc := range cases {
+		tc := *tc
 		tc.updateHCDescription = true
 		tc.desc = tc.desc + " with updateHCDescription"
-		cases = append(cases, tc)
+		copyOfWant := *tc.wantComputeHC
+		if tc.sp.BackendConfig != nil {
+			var wantLocation string
+			if tc.regionalCluster {
+				wantLocation = gce.DefaultTestClusterValues().Region
+			} else {
+				wantLocation = gce.DefaultTestClusterValues().ZoneName
+			}
+			wantDesc := healthcheck.HealthcheckDesc{
+				K8sCluster:  fmt.Sprintf("/locations/%s/clusters/%s", wantLocation, gce.DefaultTestClusterValues().ClusterName),
+				K8sResource: fmt.Sprintf("/namespaces/%s/services/%s", defaultBackendSvc.Namespace, defaultBackendSvc.Name),
+				Config:      "BackendConfig",
+			}
+			bytes, err := json.MarshalIndent(wantDesc, "", "    ")
+			if err != nil {
+				t.Fatalf("Error while generating the wantDesc JSON.")
+			}
+			copyOfWant.Description = string(bytes)
+		}
+		tc.wantComputeHC = &copyOfWant
+		cases = append(cases, &tc)
 	}
 
 	for _, tc := range cases {
 		t.Run(tc.desc, func(t *testing.T) {
 			flags.F.EnableUpdateCustomHealthCheckDescription = tc.updateHCDescription
-			fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
+			testClusterValues := gce.DefaultTestClusterValues()
+			testClusterValues.Regional = tc.regionalCluster
+			fakeGCE := gce.NewFakeGCECloud(testClusterValues)
 
 			mock := fakeGCE.Compute().(*cloud.MockGCE)
 			setupMockUpdate(mock)
@@ -1378,20 +1443,20 @@ func TestSyncServicePort(t *testing.T) {
 					t.Fatalf("Got %d healthchecks, want 1\n%s", len(computeHCs), pretty.Sprint(computeHCs))
 				}
 
-				gotHC := computeHCs[0]
 				// Filter out SelfLink because it is hard to deal with in the mock and
 				// test cases.
-				filter := func(hc *compute.HealthCheck) {
+				filter := func(hc compute.HealthCheck) compute.HealthCheck {
 					hc.SelfLink = ""
 					if !tc.updateHCDescription {
 						hc.Description = ""
 					}
+					return hc
 				}
-				filter(gotHC)
-				filter(tc.wantComputeHC)
+				gotHC := filter(*computeHCs[0])
+				wantHC := 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))
+				if !reflect.DeepEqual(gotHC, wantHC) {
+					t.Fatalf("Compute healthcheck is:\n%s, want:\n%s", pretty.Sprint(gotHC), pretty.Sprint(wantHC))
 				}
 			}
 

pkg/psc/controller.go

@@ -128,11 +128,7 @@ func NewController(ctx *context.ControllerContext) *Controller {
 	if controller.regionalCluster {
 		controller.clusterLoc = controller.cloud.Region()
 	} else {
-		zone, err := controller.cloud.GetZone(context2.Background())
-		if err != nil {
-			klog.Errorf("Failed to retrieve zone information from cloud provider: %q", err)
-		}
-		controller.clusterLoc = zone.FailureDomain
+		controller.clusterLoc = controller.cloud.LocalZone()
 	}
 
 	ctx.SAInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{

pkg/psc/controller_test.go

@@ -301,11 +301,8 @@ func TestServiceAttachmentCreation(t *testing.T) {
 					t.Errorf("%s", err)
 				}
 
-				zone, err := fakeCloud.GetZone(context2.TODO())
-				if err != nil {
-					t.Errorf("failed to get zone %q", err)
-				}
-				desc := sautils.NewServiceAttachmentDesc(testNamespace, saName, ClusterName, zone.FailureDomain, false)
+				zone := fakeCloud.LocalZone()
+				desc := sautils.NewServiceAttachmentDesc(testNamespace, saName, ClusterName, zone, false)
 
 				expectedSA := &ga.ServiceAttachment{
 					ConnectionPreference: tc.connectionPreference,

pkg/translator/healthchecks.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/ingress-gce/pkg/flags"
 	"k8s.io/ingress-gce/pkg/loadbalancers/features"
 	"k8s.io/ingress-gce/pkg/utils"
+	"k8s.io/ingress-gce/pkg/utils/healthcheck"
 	"k8s.io/klog/v2"
 )
 
@@ -91,7 +92,19 @@ type HealthCheck struct {
 	computealpha.HTTPHealthCheck
 	computealpha.HealthCheck
 
-	Service *v1.Service
+	Service         *v1.Service
+	healthcheckInfo healthcheck.HealthcheckInfo
+}
+
+func (hc *HealthCheck) SetHealthcheckInfo(i healthcheck.HealthcheckInfo) {
+	hc.healthcheckInfo = i
+	hc.reconcileHCDescription()
+}
+
+func (hc *HealthCheck) reconcileHCDescription() {
+	if flags.F.EnableUpdateCustomHealthCheckDescription && hc.healthcheckInfo.HealthcheckConfig == healthcheck.BackendConfigHC {
+		hc.Description = hc.healthcheckInfo.GenerateHealthcheckDescription()
+	}
 }
 
 // NewHealthCheck creates a HealthCheck which abstracts nested structs away
@@ -227,8 +240,10 @@ func (hc *HealthCheck) UpdateFromBackendConfig(c *backendconfigv1.HealthCheckCon
 		hc.PortSpecification = "USE_FIXED_PORT"
 	}
 	if flags.F.EnableUpdateCustomHealthCheckDescription {
-		hc.Description = DescriptionForHealthChecksFromBackendConfig
+		hc.Description = hc.healthcheckInfo.GenerateHealthcheckDescription()
 	}
+	hc.healthcheckInfo.HealthcheckConfig = healthcheck.BackendConfigHC
+	hc.reconcileHCDescription()
 }
 
 // DefaultHealthCheck simply returns the default health check.
@@ -339,4 +354,6 @@ func ApplyProbeSettingsToHC(p *v1.Probe, hc *HealthCheck) {
 	}
 
 	hc.Description = DescriptionForHealthChecksFromReadinessProbe
+	hc.healthcheckInfo.HealthcheckConfig = healthcheck.ReadinessProbeHC
+	hc.reconcileHCDescription()
 }