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
DamianSawicki committed Apr 5, 2023
1 parent 684710a commit 151877a
Show file tree
Hide file tree
Showing 9 changed files with 249 additions and 31 deletions.
6 changes: 5 additions & 1 deletion pkg/backends/syncer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,11 @@ func (s *backendSyncer) ensureHealthCheck(sp utils.ServicePort) (string, error)
return "", fmt.Errorf("Error getting prober: %w", err)
}
}
return s.healthChecker.SyncServicePort(&sp, probe)
i, err := s.cloud.ExtractClusterInfo()
if err != nil {
return "", fmt.Errorf("Error extracting cluster information: %w", err)
}
return s.healthChecker.SyncServicePort(&sp, probe, &i)
}

// getHealthCheckLink gets the Healthcheck link off the BackendService
Expand Down
9 changes: 6 additions & 3 deletions pkg/healthchecks/healthchecks.go
View file
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 @@ -53,7 +54,7 @@ func NewHealthChecker(cloud HealthCheckProvider, healthCheckPath string, default
}

// 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 +68,14 @@ 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), HealthcheckType: descutils.DefaultHC} // 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)
func (h *HealthChecks) SyncServicePort(sp *utils.ServicePort, probe *v1.Probe, i *descutils.ClusterInfo) (string, error) {
hc := h.new(*sp, *i)
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
72 changes: 48 additions & 24 deletions pkg/healthchecks/healthchecks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,10 @@ func init() {
func TestHealthCheckAdd(t *testing.T) {
fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc)
ci := gce.DefaultClusterInfo()

sp := &utils.ServicePort{NodePort: 80, Protocol: annotations.ProtocolHTTP, NEGEnabled: false, BackendNamer: testNamer}
_, err := healthChecks.SyncServicePort(sp, nil)
_, err := healthChecks.SyncServicePort(sp, nil, &ci)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -125,7 +126,7 @@ func TestHealthCheckAdd(t *testing.T) {
}

sp = &utils.ServicePort{NodePort: 443, Protocol: annotations.ProtocolHTTPS, NEGEnabled: false, BackendNamer: testNamer}
_, err = healthChecks.SyncServicePort(sp, nil)
_, err = healthChecks.SyncServicePort(sp, nil, &ci)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -136,7 +137,7 @@ func TestHealthCheckAdd(t *testing.T) {
}

sp = &utils.ServicePort{NodePort: 3000, Protocol: annotations.ProtocolHTTP2, NEGEnabled: false, BackendNamer: testNamer}
_, err = healthChecks.SyncServicePort(sp, nil)
_, err = healthChecks.SyncServicePort(sp, nil, &ci)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -150,6 +151,7 @@ func TestHealthCheckAdd(t *testing.T) {
func TestHealthCheckAddExisting(t *testing.T) {
fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc)
ci := gce.DefaultClusterInfo()

// HTTP
// Manually insert a health check
Expand All @@ -164,7 +166,7 @@ func TestHealthCheckAddExisting(t *testing.T) {

sp := &utils.ServicePort{NodePort: 3000, Protocol: annotations.ProtocolHTTP, NEGEnabled: false, BackendNamer: testNamer}
// Should not fail adding the same type of health check
_, err = healthChecks.SyncServicePort(sp, nil)
_, err = healthChecks.SyncServicePort(sp, nil, &ci)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -186,7 +188,7 @@ func TestHealthCheckAddExisting(t *testing.T) {
fakeGCE.CreateHealthCheck(v1hc)

sp = &utils.ServicePort{NodePort: 4000, Protocol: annotations.ProtocolHTTPS, NEGEnabled: false, BackendNamer: testNamer}
_, err = healthChecks.SyncServicePort(sp, nil)
_, err = healthChecks.SyncServicePort(sp, nil, &ci)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -208,7 +210,7 @@ func TestHealthCheckAddExisting(t *testing.T) {
fakeGCE.CreateHealthCheck(v1hc)

sp = &utils.ServicePort{NodePort: 5000, Protocol: annotations.ProtocolHTTPS, NEGEnabled: false, BackendNamer: testNamer}
_, err = healthChecks.SyncServicePort(sp, nil)
_, err = healthChecks.SyncServicePort(sp, nil, &ci)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand Down Expand Up @@ -279,6 +281,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 +297,7 @@ func TestRegionalHealthCheckDelete(t *testing.T) {
L7ILBEnabled: true,
BackendNamer: testNamer,
},
ci,
)
hcName := testNamer.NEG("ns2", "svc2", 80)

Expand Down Expand Up @@ -454,6 +458,7 @@ func TestRolloutUpdateCustomHCDescription(t *testing.T) {
}()

fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
ci := gce.DefaultClusterInfo()

var (
defaultSP *utils.ServicePort = testSPs["HTTP-80-reg-nil"]
Expand All @@ -467,7 +472,7 @@ func TestRolloutUpdateCustomHCDescription(t *testing.T) {

healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc)

_, err := healthChecks.SyncServicePort(defaultSP, nil)
_, err := healthChecks.SyncServicePort(defaultSP, nil, &ci)
if err != nil {
t.Fatalf("unexpected err while syncing healthcheck, err %v", err)
}
Expand All @@ -479,7 +484,7 @@ func TestRolloutUpdateCustomHCDescription(t *testing.T) {
outputDefaultHC.Description, translator.DescriptionForDefaultHealthChecks)
}

_, err = healthChecks.SyncServicePort(backendConfigSP, nil)
_, err = healthChecks.SyncServicePort(backendConfigSP, nil, &ci)
if err != nil {
t.Fatalf("unexpected err while syncing healthcheck, err %v", err)
}
Expand All @@ -496,16 +501,23 @@ func TestRolloutUpdateCustomHCDescription(t *testing.T) {
// Modify the flag and see what happens.
flags.F.EnableUpdateCustomHealthCheckDescription = true

_, err = healthChecks.SyncServicePort(backendConfigSP, nil)
_, err = healthChecks.SyncServicePort(backendConfigSP, nil, &ci)
if err != nil {
t.Fatalf("unexpected err while syncing healthcheck, err %v", err)
}

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 +1337,27 @@ 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 {
copyOfWant.Description = "" +
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("}")
}
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())
ci := gce.DefaultClusterInfo()

mock := fakeGCE.Compute().(*cloud.MockGCE)
setupMockUpdate(mock)
Expand All @@ -1344,12 +1368,12 @@ func TestSyncServicePort(t *testing.T) {

hcs := NewHealthChecker(fakeGCE, "/", defaultBackendSvc)

gotSelfLink, err := hcs.SyncServicePort(tc.sp, tc.probe)
gotSelfLink, err := hcs.SyncServicePort(tc.sp, tc.probe, &ci)
if gotErr := err != nil; gotErr != tc.wantErr {
t.Errorf("hcs.SyncServicePort(tc.sp, tc.probe) = _, %v; gotErr = %t, want %t\nsp = %s\nprobe = %s", err, gotErr, tc.wantErr, pretty.Sprint(tc.sp), pretty.Sprint(tc.probe))
t.Errorf("hcs.SyncServicePort(tc.sp, tc.probe, &ci) = _, %v; gotErr = %t, want %t\nsp = %s\nprobe = %s\nci = %s", err, gotErr, tc.wantErr, pretty.Sprint(tc.sp), pretty.Sprint(tc.probe), pretty.Sprint(ci))
}
if tc.wantSelfLink != "" && gotSelfLink != tc.wantSelfLink {
t.Errorf("hcs.SyncServicePort(tc.sp, tc.probe) = %q, _; want = %q", gotSelfLink, tc.wantSelfLink)
t.Errorf("hcs.SyncServicePort(tc.sp, tc.probe, &ci) = %q, _; want = %q", gotSelfLink, tc.wantSelfLink)
}

verify := func() {
Expand All @@ -1364,20 +1388,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 All @@ -1390,12 +1414,12 @@ func TestSyncServicePort(t *testing.T) {
return nil
}

gotSelfLink, err = hcs.SyncServicePort(tc.sp, tc.probe)
gotSelfLink, err = hcs.SyncServicePort(tc.sp, tc.probe, &ci)
if gotErr := err != nil; gotErr != tc.wantErr {
t.Errorf("hcs.SyncServicePort(tc.sp, tc.probe) = %v; gotErr = %t, want %t\nsp = %s\nprobe = %s", err, gotErr, tc.wantErr, pretty.Sprint(tc.sp), pretty.Sprint(tc.probe))
t.Errorf("hcs.SyncServicePort(tc.sp, tc.probe, &ci) = %v; gotErr = %t, want %t\nsp = %s\nprobe = %s\nci = %s", err, gotErr, tc.wantErr, pretty.Sprint(tc.sp), pretty.Sprint(tc.probe), pretty.Sprint(ci))
}
if tc.wantSelfLink != "" && gotSelfLink != tc.wantSelfLink {
t.Errorf("hcs.SyncServicePort(tc.sp, tc.probe) = %q, _; want = %q", gotSelfLink, tc.wantSelfLink)
t.Errorf("hcs.SyncServicePort(tc.sp, tc.probe, &ci) = %q, _; want = %q", gotSelfLink, tc.wantSelfLink)
}
verify()
})
Expand Down
3 changes: 2 additions & 1 deletion pkg/healthchecks/interfaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
v1 "k8s.io/api/core/v1"
"k8s.io/ingress-gce/pkg/translator"
"k8s.io/ingress-gce/pkg/utils"
"k8s.io/ingress-gce/pkg/utils/descutils"
)

// HealthCheckProvider is an interface to manage a single GCE health check.
Expand Down Expand Up @@ -52,7 +53,7 @@ type HealthChecker interface {
// ServicePort and Pod Probe definition.
//
// `probe` can be nil if no probe exists.
SyncServicePort(sp *utils.ServicePort, probe *v1.Probe) (string, error)
SyncServicePort(sp *utils.ServicePort, probe *v1.Probe, i *descutils.ClusterInfo) (string, error)
Delete(name string, scope meta.KeyType) error
Get(name string, version meta.Version, scope meta.KeyType) (*translator.HealthCheck, error)
}
16 changes: 16 additions & 0 deletions pkg/translator/healthchecks.go
View file
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) UpdateHealthCheckType(t descutils.HealthcheckType) {
hc.healthcheckInfo.HealthcheckType = t
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.HealthcheckType == descutils.BackendConfigHC {
hc.Description = descutils.GenerateHealthcheckDescriptionFromHealthcheckInfo(hc.healthcheckInfo)
}
}

// 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.UpdateHealthCheckType(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.UpdateHealthCheckType(descutils.ReadinessProbeHC)
}
Loading

0 comments on commit 151877a

Please sign in to comment.