diff --git a/cmd/glbc/main.go b/cmd/glbc/main.go index afc247e602..ace44573b9 100644 --- a/cmd/glbc/main.go +++ b/cmd/glbc/main.go @@ -319,12 +319,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 diff --git a/pkg/healthchecks/healthchecks.go b/pkg/healthchecks/healthchecks.go index cb7ae56451..3360ab3073 100644 --- a/pkg/healthchecks/healthchecks.go +++ b/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" ) @@ -43,13 +44,27 @@ type HealthChecks struct { // This is a workaround which allows us to not have to maintain // a separate health checker for the default backend. defaultBackendSvc types.NamespacedName + 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) *HealthChecks { - return &HealthChecks{cloud, healthCheckPath, defaultBackendSvc} + ci := generateClusterInfo(cloud.(*gce.Cloud)) + return &HealthChecks{cloud, healthCheckPath, defaultBackendSvc, 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 @@ -67,9 +82,29 @@ func (h *HealthChecks) new(sp utils.ServicePort) *translator.HealthCheck { hc.Name = sp.BackendName() hc.Port = sp.NodePort hc.RequestPath = h.pathFromSvcPort(sp) + hc.SetHealthcheckInfo(h.generateHealthcheckInfo(sp, hc.ForILB)) return hc } +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) + } + + ingressType := healthcheck.ExternalLB + if iLB { + ingressType = healthcheck.InternalLB + } + + return healthcheck.HealthcheckInfo{ + ClusterInfo: h.clusterInfo, + ServiceInfo: serviceInfo, + HealthcheckConfig: healthcheck.DefaultHC, + IngressType: ingressType, + } +} + // SyncServicePort implements HealthChecker. func (h *HealthChecks) SyncServicePort(sp *utils.ServicePort, probe *v1.Probe) (string, error) { hc := h.new(*sp) diff --git a/pkg/healthchecks/healthchecks_test.go b/pkg/healthchecks/healthchecks_test.go index f23e7637b1..3746dcb6a9 100644 --- a/pkg/healthchecks/healthchecks_test.go +++ b/pkg/healthchecks/healthchecks_test.go @@ -18,6 +18,7 @@ package healthchecks import ( "context" + "encoding/json" "fmt" "net/http" "reflect" @@ -42,6 +43,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" ) @@ -442,18 +444,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"] @@ -503,9 +515,21 @@ 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), + K8sResourceDependency: "Ingress External Load Balancer", + 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. @@ -970,21 +994,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 @@ -1118,6 +1149,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" @@ -1325,15 +1368,44 @@ 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, wantIngressType string + if tc.regionalCluster { + wantLocation = gce.DefaultTestClusterValues().Region + } else { + wantLocation = gce.DefaultTestClusterValues().ZoneName + } + if tc.sp.L7ILBEnabled { + wantIngressType = "Ingress Internal Load Balancer" + } else { + wantIngressType = "Ingress External Load Balancer" + } + 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), + K8sResourceDependency: healthcheck.IngressType(wantIngressType), + Config: "BackendConfig", + } + bytes, err := json.MarshalIndent(wantDesc, "", " ") + if err != nil { + t.Fatalf("Error while generating the wantDesc JSON.") + } + copyOfWant.Description = string(bytes) + } + tc.wantComputeHC = ©OfWant + 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) @@ -1364,20 +1436,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)) } } diff --git a/pkg/psc/controller.go b/pkg/psc/controller.go index 44e196c80a..4517dcbefd 100644 --- a/pkg/psc/controller.go +++ b/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{ diff --git a/pkg/psc/controller_test.go b/pkg/psc/controller_test.go index ddda14acdd..1631f38bfc 100644 --- a/pkg/psc/controller_test.go +++ b/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, diff --git a/pkg/translator/healthchecks.go b/pkg/translator/healthchecks.go index caa541559c..a28215ec9c 100644 --- a/pkg/translator/healthchecks.go +++ b/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" ) @@ -90,6 +91,18 @@ type HealthCheck struct { // compute struct back. computealpha.HTTPHealthCheck computealpha.HealthCheck + 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 @@ -225,8 +238,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. @@ -337,4 +352,6 @@ func ApplyProbeSettingsToHC(p *v1.Probe, hc *HealthCheck) { } hc.Description = DescriptionForHealthChecksFromReadinessProbe + hc.healthcheckInfo.HealthcheckConfig = healthcheck.ReadinessProbeHC + hc.reconcileHCDescription() } diff --git a/pkg/utils/healthcheck/healthcheckdesc.go b/pkg/utils/healthcheck/healthcheckdesc.go new file mode 100644 index 0000000000..b30ea06b15 --- /dev/null +++ b/pkg/utils/healthcheck/healthcheckdesc.go @@ -0,0 +1,90 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package healthcheck + +import ( + "encoding/json" + "fmt" + + "k8s.io/apimachinery/pkg/types" + "k8s.io/ingress-gce/pkg/utils/descutils" + "k8s.io/klog/v2" +) + +type ClusterInfo struct { + Name string + Location string + Regional bool +} + +type ServiceInfo types.NamespacedName + +type HealthcheckConfig string + +const ( + DefaultHC HealthcheckConfig = "Default" + ReadinessProbeHC HealthcheckConfig = "ReadinessProbe" + BackendConfigHC HealthcheckConfig = "BackendConfig" +) + +type IngressType string + +const ( + InternalLB IngressType = "Ingress Internal Load Balancer" + ExternalLB IngressType = "Ingress External Load Balancer" +) + +type HealthcheckInfo struct { + ClusterInfo + ServiceInfo + HealthcheckConfig + IngressType +} + +type HealthcheckDesc struct { + K8sCluster string + K8sResource string + K8sResourceDependency IngressType + Config HealthcheckConfig +} + +func (i *ClusterInfo) generateClusterDescription() string { + // locType here differs from locType in descutils.GenerateClusterLink(). + locType := "locations" + return fmt.Sprintf("/%s/%s/clusters/%s", locType, i.Location, i.Name) +} + +func NewServiceInfo(namespace, resourceName string) ServiceInfo { + return ServiceInfo{namespace, resourceName} +} + +func (i *ServiceInfo) generateK8sResourceDescription() string { + return descutils.GenerateK8sResourceLink(i.Namespace, "services", i.Name) +} + +func (i *HealthcheckInfo) GenerateHealthcheckDescription() string { + desc := HealthcheckDesc{} + desc.K8sCluster = i.ClusterInfo.generateClusterDescription() + desc.K8sResource = i.ServiceInfo.generateK8sResourceDescription() + desc.K8sResourceDependency = i.IngressType + desc.Config = i.HealthcheckConfig + json, err := json.MarshalIndent(desc, "", " ") + if err != nil { + klog.Error("Failed to marshall HealthcheckDesc %s: %v", desc, err) + } + return string(json) +} diff --git a/pkg/utils/healthcheck/healthcheckdesc_test.go b/pkg/utils/healthcheck/healthcheckdesc_test.go new file mode 100644 index 0000000000..07fe1d8aa0 --- /dev/null +++ b/pkg/utils/healthcheck/healthcheckdesc_test.go @@ -0,0 +1,82 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package healthcheck + +import ( + "testing" +) + +func TestGenerateClusterDescription(t *testing.T) { + testName := "cluster-name" + testRegion := "pl-central7" + clusterInfo := ClusterInfo{testName, testRegion, true} + expectedDescription := "/locations/pl-central7/clusters/cluster-name" + + generatedDescription := clusterInfo.generateClusterDescription() + if generatedDescription != expectedDescription { + t.Errorf("unexpected cluster description: wanted %s, but got %s", expectedDescription, generatedDescription) + } + + testZone := "pl-central7-g" + clusterInfo = ClusterInfo{testName, testZone, false} + expectedDescription = "/locations/pl-central7-g/clusters/cluster-name" + + generatedDescription = clusterInfo.generateClusterDescription() + if generatedDescription != expectedDescription { + t.Errorf("unexpected cluster description: wanted %s, but got %s", expectedDescription, generatedDescription) + } +} + +func TestGenerateK8sResourceDescription(t *testing.T) { + testName := "service-name" + testNamespace := "testNamespace" + serviceInfo := NewServiceInfo(testNamespace, testName) + expectedDescription := "/namespaces/testNamespace/services/service-name" + + generatedDescription := serviceInfo.generateK8sResourceDescription() + if generatedDescription != expectedDescription { + t.Errorf("unexpected service description: wanted %v, but got %v", expectedDescription, generatedDescription) + } +} + +func TestGenerateHealthcheckDescription(t *testing.T) { + testCluster := "cluster-name" + testRegion := "pl-central7" + clusterInfo := ClusterInfo{testCluster, testRegion, true} + testService := "service-name" + testNamespace := "testNamespace" + serviceInfo := NewServiceInfo(testNamespace, testService) + hcConfig := DefaultHC + i := HealthcheckInfo{ + ClusterInfo: clusterInfo, + ServiceInfo: serviceInfo, + HealthcheckConfig: hcConfig, + IngressType: InternalLB, + } + expectedDescription := "" + + "{\n" + + " \"K8sCluster\": \"/locations/pl-central7/clusters/cluster-name\",\n" + + " \"K8sResource\": \"/namespaces/testNamespace/services/service-name\",\n" + + " \"K8sResourceDependency\": \"Ingress Internal Load Balancer\",\n" + + " \"Config\": \"Default\"\n" + + "}" + + generatedDescription := i.GenerateHealthcheckDescription() + if generatedDescription != expectedDescription { + t.Errorf("unexpected healthcheck description: wanted %v, but got %v", expectedDescription, generatedDescription) + } +} diff --git a/vendor/modules.txt b/vendor/modules.txt index a8024c12d4..1322634344 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -805,7 +805,7 @@ k8s.io/cloud-provider-gcp/crd/client/gcpfirewall/clientset/versioned/typed/gcpfi k8s.io/cloud-provider-gcp/crd/client/gcpfirewall/informers/externalversions/gcpfirewall/v1beta1 k8s.io/cloud-provider-gcp/crd/client/gcpfirewall/informers/externalversions/internalinterfaces k8s.io/cloud-provider-gcp/crd/client/gcpfirewall/listers/gcpfirewall/v1beta1 -# k8s.io/cloud-provider-gcp/providers v0.26.3-0.20230322202223-f7280f55e36f +# k8s.io/cloud-provider-gcp/providers v0.26.3-0.20230414153241-658d4df496a9 ## explicit; go 1.19 k8s.io/cloud-provider-gcp/providers/gce # k8s.io/component-base v0.26.2