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 7, 2023
1 parent 684710a commit a645166
Show file tree
Hide file tree
Showing 7 changed files with 265 additions and 21 deletions.
34 changes: 33 additions & 1 deletion 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,13 +44,30 @@ 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}
ci := generateClusterInfo(cloud.(*gce.Cloud))
return &HealthChecks{cloud, healthCheckPath, defaultBackendSvc, ci}
}

func generateClusterInfo(gceCloud *gce.Cloud) descutils.ClusterInfo {
var location string
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)
}
return descutils.ClusterInfo{Name: name, Location: location, Regional: regionalCluster}
}

// new returns a *HealthCheck with default settings and specified port/protocol
Expand All @@ -67,9 +85,23 @@ 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(hc))
return hc
}

func (h *HealthChecks) generateHealthcheckInfo(hc *translator.HealthCheck) descutils.HealthcheckInfo {
ingressType := descutils.ExternalLB
if hc.ForILB {
ingressType = descutils.InternalLB
}
return descutils.HealthcheckInfo{
ClusterInfo: h.clusterInfo,
ServiceInfo: descutils.ServiceInfo(h.defaultBackendSvc),
HealthcheckConfig: descutils.DefaultHC,
IngressType: ingressType,
}
}

// SyncServicePort implements HealthChecker.
func (h *HealthChecks) SyncServicePort(sp *utils.ServicePort, probe *v1.Probe) (string, error) {
hc := h.new(*sp)
Expand Down
79 changes: 61 additions & 18 deletions pkg/healthchecks/healthchecks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,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 +505,17 @@ 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(" \"K8sResourceDependency\": \"Ingress External Load Balancer\",\n") +
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 @@ -980,11 +990,12 @@ func TestSyncServicePort(t *testing.T) {
}()

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
Expand Down Expand Up @@ -1066,7 +1077,10 @@ func TestSyncServicePort(t *testing.T) {
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})
cases = append(cases, &tc{
desc: "create backendconfig",
sp: testSPs["HTTP-80-reg-bc"],
wantComputeHC: chc})

// BackendConfig all
chc = fixture.hc()
Expand All @@ -1079,7 +1093,11 @@ func TestSyncServicePort(t *testing.T) {
// 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})
cases = append(cases, &tc{
desc: "create backendconfig all",
sp: testSPs["HTTP-80-reg-bcall"],
regionalCluster: true,
wantComputeHC: chc})

i64 := func(i int64) *int64 { return &i }

Expand Down Expand Up @@ -1325,15 +1343,40 @@ 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"
}
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(" \"K8sResourceDependency\": \"%s\",\n", wantIngressType) +
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.regionalCluster
fakeGCE := gce.NewFakeGCECloud(testClusterValues)

mock := fakeGCE.Compute().(*cloud.MockGCE)
setupMockUpdate(mock)
Expand Down Expand Up @@ -1364,20 +1407,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)
}
72 changes: 71 additions & 1 deletion pkg/utils/descutils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,64 @@ 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 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 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 +90,16 @@ 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()
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)
}
61 changes: 61 additions & 0 deletions pkg/utils/descutils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,64 @@ 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,
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)
}
}
Loading

0 comments on commit a645166

Please sign in to comment.