Skip to content

Commit

Permalink
JSONify healthcheck Description for BackendConfig
Browse files Browse the repository at this point in the history
  • Loading branch information
DamianSawicki committed Apr 6, 2023
1 parent 684710a commit 829c504
Show file tree
Hide file tree
Showing 7 changed files with 241 additions and 16 deletions.
26 changes: 23 additions & 3 deletions pkg/healthchecks/healthchecks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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)
Expand All @@ -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)
Expand Down
52 changes: 41 additions & 11 deletions pkg/healthchecks/healthchecks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -294,6 +295,7 @@ func TestRegionalHealthCheckDelete(t *testing.T) {
L7ILBEnabled: true,
BackendNamer: testNamer,
},
ci,
)
hcName := testNamer.NEG("ns2", "svc2", 80)

Expand Down Expand Up @@ -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"]
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 = &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.regional
fakeGCE := gce.NewFakeGCECloud(testClusterValues)

mock := fakeGCE.Compute().(*cloud.MockGCE)
setupMockUpdate(mock)
Expand Down Expand Up @@ -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))
}
}

Expand Down
16 changes: 16 additions & 0 deletions pkg/translator/healthchecks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -337,4 +352,5 @@ func ApplyProbeSettingsToHC(p *v1.Probe, hc *HealthCheck) {
}

hc.Description = DescriptionForHealthChecksFromReadinessProbe
hc.UpdateHealthcheckConfig(descutils.ReadinessProbeHC)
}
66 changes: 65 additions & 1 deletion pkg/utils/descutils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "" {
Expand All @@ -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)
}
55 changes: 55 additions & 0 deletions pkg/utils/descutils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Loading

0 comments on commit 829c504

Please sign in to comment.