diff --git a/pkg/healthchecks/healthchecks.go b/pkg/healthchecks/healthchecks.go index cb7ae56451..3ebf188fef 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/descutils" "k8s.io/klog/v2" ) @@ -43,17 +44,31 @@ 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 descutils.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} + var location string + gceCloud := cloud.(*gce.Cloud) + regionalCluster := gceCloud.Regional() + if regionalCluster { + location = gceCloud.Region() + } else { + location = gceCloud.Zone() + } + name, err := gceCloud.ClusterID.GetID() + if err != nil { + klog.Errorf("Failed to obtain cluster name, %+v", err) + } + ci := descutils.ClusterInfo{Name: name, Location: location, Regional: regionalCluster} + return &HealthChecks{cloud, healthCheckPath, defaultBackendSvc, ci} } // new returns a *HealthCheck with default settings and specified port/protocol -func (h *HealthChecks) new(sp utils.ServicePort) *translator.HealthCheck { +func (h *HealthChecks) new(sp utils.ServicePort, i descutils.ClusterInfo) *translator.HealthCheck { var hc *translator.HealthCheck if sp.NEGEnabled && !sp.L7ILBEnabled { hc = translator.DefaultNEGHealthCheck(sp.Protocol) @@ -67,12 +82,17 @@ func (h *HealthChecks) new(sp utils.ServicePort) *translator.HealthCheck { hc.Name = sp.BackendName() hc.Port = sp.NodePort hc.RequestPath = h.pathFromSvcPort(sp) + info := descutils.HealthcheckInfo{ + ClusterInfo: i, + ServiceInfo: descutils.ServiceInfo(h.defaultBackendSvc), + HealthcheckConfig: descutils.DefaultHC} // TODO: IngressType: ??? + hc.SetHealthcheckInfo(info) return hc } // SyncServicePort implements HealthChecker. func (h *HealthChecks) SyncServicePort(sp *utils.ServicePort, probe *v1.Probe) (string, error) { - hc := h.new(*sp) + hc := h.new(*sp, h.clusterInfo) if probe != nil { klog.V(2).Infof("Applying httpGet settings of readinessProbe to health check on port %+v", sp) translator.ApplyProbeSettingsToHC(probe, hc) diff --git a/pkg/healthchecks/healthchecks_test.go b/pkg/healthchecks/healthchecks_test.go index f23e7637b1..44f0885b7e 100644 --- a/pkg/healthchecks/healthchecks_test.go +++ b/pkg/healthchecks/healthchecks_test.go @@ -279,6 +279,7 @@ func TestHTTP2HealthCheckDelete(t *testing.T) { func TestRegionalHealthCheckDelete(t *testing.T) { fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc) + ci := gce.DefaultClusterInfo() hc := healthChecks.new( utils.ServicePort{ @@ -294,6 +295,7 @@ func TestRegionalHealthCheckDelete(t *testing.T) { L7ILBEnabled: true, BackendNamer: testNamer, }, + ci, ) hcName := testNamer.NEG("ns2", "svc2", 80) @@ -453,7 +455,9 @@ func TestRolloutUpdateCustomHCDescription(t *testing.T) { flags.F.EnableUpdateCustomHealthCheckDescription = oldUpdateDescription }() - fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) + testClusterValues := gce.DefaultTestClusterValues() + testClusterValues.Regional = true + fakeGCE := gce.NewFakeGCECloud(testClusterValues) var ( defaultSP *utils.ServicePort = testSPs["HTTP-80-reg-nil"] @@ -503,9 +507,16 @@ func TestRolloutUpdateCustomHCDescription(t *testing.T) { outputBCHCWithFlag := getSingletonHealthcheck(t, fakeGCE) - if outputBCHCWithFlag.Description != translator.DescriptionForHealthChecksFromBackendConfig { + wantDescription := "" + + fmt.Sprintf("{\n") + + fmt.Sprintf(" \"K8sCluster\": \"/locations/%s/clusters/%s\",\n", gce.DefaultTestClusterValues().Region, gce.DefaultTestClusterValues().ClusterID) + + fmt.Sprintf(" \"K8sResource\": \"/namespaces/%s/services/%s\",\n", defaultBackendSvc.Namespace, defaultBackendSvc.Name) + + fmt.Sprintf(" \"Config\": \"BackendConfig\"\n") + + fmt.Sprintf("}") + + 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. @@ -1325,15 +1336,34 @@ 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.regional { + wantLocation = gce.DefaultTestClusterValues().Region + } else { + wantLocation = gce.DefaultTestClusterValues().ZoneName + } + copyOfWant.Description = "" + + fmt.Sprintf("{\n") + + fmt.Sprintf(" \"K8sCluster\": \"/locations/%s/clusters/%s\",\n", wantLocation, gce.DefaultTestClusterValues().ClusterID) + + fmt.Sprintf(" \"K8sResource\": \"/namespaces/%s/services/%s\",\n", defaultBackendSvc.Namespace, defaultBackendSvc.Name) + + fmt.Sprintf(" \"Config\": \"BackendConfig\"\n") + + fmt.Sprintf("}") + } + 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.regional + fakeGCE := gce.NewFakeGCECloud(testClusterValues) mock := fakeGCE.Compute().(*cloud.MockGCE) setupMockUpdate(mock) @@ -1364,20 +1394,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/translator/healthchecks.go b/pkg/translator/healthchecks.go index caa541559c..028abe21e8 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/descutils" "k8s.io/klog/v2" ) @@ -90,6 +91,19 @@ type HealthCheck struct { // compute struct back. computealpha.HTTPHealthCheck computealpha.HealthCheck + healthcheckInfo descutils.HealthcheckInfo +} + +func (hc *HealthCheck) UpdateHealthcheckConfig(c descutils.HealthcheckConfig) { + hc.healthcheckInfo.HealthcheckConfig = c + hc.SetHealthcheckInfo(hc.healthcheckInfo) // This is not a no-op. +} + +func (hc *HealthCheck) SetHealthcheckInfo(i descutils.HealthcheckInfo) { + hc.healthcheckInfo = i + if flags.F.EnableUpdateCustomHealthCheckDescription && i.HealthcheckConfig == descutils.BackendConfigHC { + hc.Description = hc.healthcheckInfo.GenerateHealthcheckDescription() + } } // NewHealthCheck creates a HealthCheck which abstracts nested structs away @@ -227,6 +241,7 @@ func (hc *HealthCheck) UpdateFromBackendConfig(c *backendconfigv1.HealthCheckCon if flags.F.EnableUpdateCustomHealthCheckDescription { hc.Description = DescriptionForHealthChecksFromBackendConfig } + hc.UpdateHealthcheckConfig(descutils.BackendConfigHC) } // DefaultHealthCheck simply returns the default health check. @@ -337,4 +352,5 @@ func ApplyProbeSettingsToHC(p *v1.Probe, hc *HealthCheck) { } hc.Description = DescriptionForHealthChecksFromReadinessProbe + hc.UpdateHealthcheckConfig(descutils.ReadinessProbeHC) } diff --git a/pkg/utils/descutils/utils.go b/pkg/utils/descutils/utils.go index fdb371e726..998e36b31c 100644 --- a/pkg/utils/descutils/utils.go +++ b/pkg/utils/descutils/utils.go @@ -16,7 +16,57 @@ limitations under the License. package descutils -import "fmt" +import ( + "encoding/json" + "fmt" + + "k8s.io/apimachinery/pkg/types" + "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 HealthcheckInfo struct { + ClusterInfo + ServiceInfo + HealthcheckConfig + // TODO: IngressType string +} + +type HealthcheckDesc struct { + K8sCluster string + K8sResource string + // TODO: K8ResourceDependency string + Config string +} + +func (i *ClusterInfo) generateClusterDescription() string { + // locType here differs from locType in 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 GenerateK8sResourceLink(i.Namespace, "services", i.Name) +} func GenerateClusterLink(name, location string, regional bool) string { if name == "" || location == "" { @@ -33,3 +83,17 @@ func GenerateClusterLink(name, location string, regional bool) string { func GenerateK8sResourceLink(namespace, resourceType, resourceName string) string { return fmt.Sprintf("/namespaces/%s/%s/%s", namespace, resourceType, resourceName) } + +func (i *HealthcheckInfo) GenerateHealthcheckDescription() string { + desc := HealthcheckDesc{} + desc.K8sCluster = i.ClusterInfo.generateClusterDescription() + desc.K8sResource = i.ServiceInfo.generateK8sResourceDescription() + // TODO: + // desc.K8ResourceDependency = i.IngressType + desc.Config = string(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/descutils/utils_test.go b/pkg/utils/descutils/utils_test.go index 8d29a7912d..6928489442 100644 --- a/pkg/utils/descutils/utils_test.go +++ b/pkg/utils/descutils/utils_test.go @@ -78,3 +78,58 @@ func TestK8sResourceLink(t *testing.T) { t.Errorf("unexpected cluster link: wanted %s, but got %s", expectedLink, generatedLink) } } + +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} + expectedDescription := "" + + "{\n" + + " \"K8sCluster\": \"/locations/pl-central7/clusters/cluster-name\",\n" + + " \"K8sResource\": \"/namespaces/testNamespace/services/service-name\",\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/k8s.io/cloud-provider-gcp/providers/gce/gce.go b/vendor/k8s.io/cloud-provider-gcp/providers/gce/gce.go index 9ebadc19e2..45e8c1bdf7 100644 --- a/vendor/k8s.io/cloud-provider-gcp/providers/gce/gce.go +++ b/vendor/k8s.io/cloud-provider-gcp/providers/gce/gce.go @@ -43,7 +43,7 @@ import ( "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/informers" @@ -55,6 +55,7 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/client-go/util/flowcontrol" cloudprovider "k8s.io/cloud-provider" + "k8s.io/ingress-gce/pkg/utils/descutils" "k8s.io/klog/v2" ) @@ -726,6 +727,16 @@ func (g *Cloud) Region() string { return g.region } +// Regional returns true if the cluster is regional. +func (g *Cloud) Regional() bool { + return g.regional +} + +// Zone returns the zone. +func (g *Cloud) Zone() string { + return g.localZone +} + // OnXPN returns true if the cluster is running on a cross project network (XPN) func (g *Cloud) OnXPN() bool { return g.onXPN @@ -976,3 +987,20 @@ func (manager *gceServiceManager) getProjectsAPIEndpoint() string { return projectsAPIEndpoint } + +func (cloud *Cloud) ExtractClusterInfo() (descutils.ClusterInfo, error) { + var ans descutils.ClusterInfo + name, err := cloud.ClusterID.GetID() + if err != nil { + return ans, fmt.Errorf("Error getting cluster name: %w", err) + } + regional := cloud.Regional() + var location string + if regional { + location = cloud.Region() + } else { + location = cloud.Zone() + } + ans = descutils.ClusterInfo{Name: name, Location: location, Regional: regional} + return ans, nil +} diff --git a/vendor/k8s.io/cloud-provider-gcp/providers/gce/gce_fake.go b/vendor/k8s.io/cloud-provider-gcp/providers/gce/gce_fake.go index d9aca0742f..8f4456bbf8 100644 --- a/vendor/k8s.io/cloud-provider-gcp/providers/gce/gce_fake.go +++ b/vendor/k8s.io/cloud-provider-gcp/providers/gce/gce_fake.go @@ -26,6 +26,7 @@ import ( compute "google.golang.org/api/compute/v1" option "google.golang.org/api/option" "k8s.io/client-go/tools/cache" + "k8s.io/ingress-gce/pkg/utils/descutils" ) // TestClusterValues holds the config values for the fake/test gce cloud object. @@ -37,6 +38,7 @@ type TestClusterValues struct { ClusterID string ClusterName string OnXPN bool + Regional bool } // DefaultTestClusterValues Creates a reasonable set of default cluster values @@ -49,9 +51,17 @@ func DefaultTestClusterValues() TestClusterValues { SecondaryZoneName: "us-central1-c", ClusterID: "test-cluster-id", ClusterName: "Test Cluster Name", + Regional: false, } } +// DefaultClusterInfo creates descutils.ClusterInfo based on the output of +// DefaultTestClusterValues(). +func DefaultClusterInfo() descutils.ClusterInfo { + values := DefaultTestClusterValues() + return descutils.ClusterInfo{Name: values.ClusterID, Location: values.Region, Regional: true} +} + // Stubs ClusterID so that ClusterID.getOrInitialize() does not require calling // gce.Initialize() func fakeClusterID(clusterID string) ClusterID { @@ -73,12 +83,14 @@ func NewFakeGCECloud(vals TestClusterValues) *Cloud { region: vals.Region, service: service, managedZones: []string{vals.ZoneName}, + localZone: vals.ZoneName, projectID: vals.ProjectID, networkProjectID: vals.ProjectID, ClusterID: fakeClusterID(vals.ClusterID), onXPN: vals.OnXPN, metricsCollector: newLoadBalancerMetrics(), projectsBasePath: getProjectsBasePath(service.BasePath), + regional: vals.Regional, } c := cloud.NewMockGCE(&gceProjectRouter{gce}) gce.c = c