Skip to content

Adjust health check Description when BackendConfig used #2008

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
k8s-ci-robot merged 1 commit into from
Mar 15, 2023
Merged

Adjust health check Description when BackendConfig used #2008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/healthchecks/healthcheck.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ func calculateDiff(old, new *translator.HealthCheck, c *backendconfigv1.HealthCh
if c.Port != nil && old.Port != new.Port {
changes.add("Port", strconv.FormatInt(old.Port, 10), strconv.FormatInt(new.Port, 10))
}
if old.Description != new.Description {
changes.add("Description", old.Description, new.Description)
}

// TODO(bowei): Host seems to be missing.

Expand Down
17 changes: 15 additions & 2 deletions pkg/healthchecks/healthchecks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,7 @@ func (*syncSPFixture) hc() *compute.HealthCheck {
Port: 80,
RequestPath: "/",
},
Description: translator.DescriptionForDefaultHealthChecks,
}
}

Expand Down Expand Up @@ -809,13 +810,15 @@ func (*syncSPFixture) neg() *compute.HealthCheck {
PortSpecification: "USE_SERVING_PORT",
RequestPath: "/",
},
Description: translator.DescriptionForDefaultNEGHealthChecks,
}
}

func (f *syncSPFixture) ilb() *compute.HealthCheck {
h := f.neg()
h.Region = "us-central1"
h.SelfLink = ilbSelfLink
h.Description = translator.DescriptionForDefaultILBHealthChecks
return h
}

Expand Down Expand Up @@ -894,6 +897,7 @@ func TestSyncServicePort(t *testing.T) {
chc.HttpHealthCheck.Host = "foo.com"
chc.CheckIntervalSec = 61
chc.TimeoutSec = 1234
chc.Description = translator.DescriptionForHealthChecksFromReadinessProbe
cases = append(cases, &tc{
desc: "create probe",
sp: testSPs["HTTP-80-reg-nil"],
Expand All @@ -913,6 +917,7 @@ func TestSyncServicePort(t *testing.T) {
chc.HttpHealthCheck.Host = "foo.com"
chc.CheckIntervalSec = 1234
chc.TimeoutSec = 5678
chc.Description = translator.DescriptionForHealthChecksFromReadinessProbe
cases = append(cases, &tc{
desc: "create probe neg",
sp: testSPs["HTTP-80-neg-nil"],
Expand All @@ -932,6 +937,7 @@ func TestSyncServicePort(t *testing.T) {
chc.HttpHealthCheck.Host = "foo.com"
chc.CheckIntervalSec = 1234
chc.TimeoutSec = 5678
chc.Description = translator.DescriptionForHealthChecksFromReadinessProbe
cases = append(cases, &tc{
desc: "create probe ilb",
sp: testSPs["HTTP-80-ilb-nil"],
Expand All @@ -949,6 +955,7 @@ func TestSyncServicePort(t *testing.T) {
// BackendConfig
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})

// BackendConfig all
Expand All @@ -961,6 +968,7 @@ func TestSyncServicePort(t *testing.T) {
chc.HttpHealthCheck.Port = 1234
// 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})

i64 := func(i int64) *int64 { return &i }
Expand All @@ -970,6 +978,7 @@ func TestSyncServicePort(t *testing.T) {
chc.HttpHealthCheck.Port = 1234
// PortSpecification is set by the controller
chc.HttpHealthCheck.PortSpecification = "USE_FIXED_PORT"
chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
sp := utils.ServicePort{
NodePort: 80,
Protocol: annotations.ProtocolHTTP,
Expand All @@ -981,6 +990,7 @@ func TestSyncServicePort(t *testing.T) {
// BackendConfig neg
chc = fixture.neg()
chc.HttpHealthCheck.RequestPath = "/foo"
chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
cases = append(cases, &tc{
desc: "create backendconfig neg",
sp: testSPs["HTTP-80-neg-bc"],
Expand All @@ -990,6 +1000,7 @@ func TestSyncServicePort(t *testing.T) {
// BackendConfig ilb
chc = fixture.ilb()
chc.HttpHealthCheck.RequestPath = "/foo"
chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
cases = append(cases, &tc{
desc: "create backendconfig ilb",
sp: testSPs["HTTP-80-ilb-bc"],
Expand All @@ -1003,6 +1014,7 @@ func TestSyncServicePort(t *testing.T) {
chc.HttpHealthCheck.Host = "foo.com"
chc.CheckIntervalSec = 61
chc.TimeoutSec = 1234
chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
cases = append(cases, &tc{
desc: "create probe and backendconfig",
sp: testSPs["HTTP-80-reg-bc"],
Expand Down Expand Up @@ -1149,6 +1161,7 @@ func TestSyncServicePort(t *testing.T) {
wantCHC = fixture.hc()
wantCHC.HttpHealthCheck.RequestPath = "/foo" // from bc
wantCHC.CheckIntervalSec = 1234 // same
wantCHC.Description = translator.DescriptionForHealthChecksFromBackendConfig
cases = append(cases, &tc{
desc: "update preserve and backendconfig (path)",
sp: testSPs["HTTP-80-reg-bc"],
Expand All @@ -1168,6 +1181,7 @@ func TestSyncServicePort(t *testing.T) {
wantCHC.TimeoutSec = 1234
wantCHC.HttpHealthCheck.Port = 1234
wantCHC.HttpHealthCheck.PortSpecification = "USE_FIXED_PORT"
wantCHC.Description = translator.DescriptionForHealthChecksFromBackendConfig
cases = append(cases, &tc{
desc: "update preserve backendconfig all",
setup: fixture.setupExistingHCFunc(chc),
Expand Down Expand Up @@ -1234,10 +1248,9 @@ func TestSyncServicePort(t *testing.T) {
}

gotHC := computeHCs[0]
// Filter out fields that are hard to deal with in the mock and
// Filter out SelfLink because it is hard to deal with in the mock and
// test cases.
filter := func(hc *compute.HealthCheck) {
hc.Description = ""
hc.SelfLink = ""
}
filter(gotHC)
Expand Down
16 changes: 12 additions & 4 deletions pkg/translator/healthchecks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ const (
// used for health checking.
useServingPortSpecification = "USE_SERVING_PORT"

DescriptionForDefaultHealthChecks = "Default kubernetes L7 Loadbalancing health check."
DescriptionForDefaultNEGHealthChecks = "Default kubernetes L7 Loadbalancing health check for NEG."
DescriptionForDefaultILBHealthChecks = "Default kubernetes L7 Loadbalancing health check for ILB."
DescriptionForHealthChecksFromReadinessProbe = "Kubernetes L7 health check generated with readiness probe settings."
DescriptionForHealthChecksFromBackendConfig = "Kubernetes L7 health check generated with BackendConfig CRD."

// TODO: revendor the GCE API go client so that this error will not be hit.
newHealthCheckErrorMessageTemplate = "the %v health check configuration on the existing health check %v is nil. " +
"This is usually caused by an application protocol change on the k8s service spec. " +
Expand Down Expand Up @@ -217,6 +223,8 @@ func (hc *HealthCheck) UpdateFromBackendConfig(c *backendconfigv1.HealthCheckCon
// This override is necessary regardless of type
hc.PortSpecification = "USE_FIXED_PORT"
}

hc.Description = DescriptionForHealthChecksFromBackendConfig
}

// DefaultHealthCheck simply returns the default health check.
Expand All @@ -231,7 +239,7 @@ func DefaultHealthCheck(port int64, protocol annotations.AppProtocol) *HealthChe
HealthyThreshold: defaultHealthyThreshold,
// Number of healthchecks to fail before the vm is deemed unhealthy.
UnhealthyThreshold: defaultUnhealthyThreshold,
Description: "Default kubernetes L7 Loadbalancing health check.",
Description: DescriptionForDefaultHealthChecks,
Type: string(protocol),
}
return &HealthCheck{
Expand All @@ -255,7 +263,7 @@ func DefaultNEGHealthCheck(protocol annotations.AppProtocol) *HealthCheck {
HealthyThreshold: defaultHealthyThreshold,
// Number of healthchecks to fail before the vm is deemed unhealthy.
UnhealthyThreshold: defaultNEGUnhealthyThreshold,
Description: "Default kubernetes L7 Loadbalancing health check for NEG.",
Description: DescriptionForDefaultNEGHealthChecks,
Type: string(protocol),
}
return &HealthCheck{
Expand All @@ -278,7 +286,7 @@ func DefaultILBHealthCheck(protocol annotations.AppProtocol) *HealthCheck {
HealthyThreshold: defaultHealthyThreshold,
// Number of healthchecks to fail before the vm is deemed unhealthy.
UnhealthyThreshold: defaultNEGUnhealthyThreshold,
Description: "Default kubernetes L7 Loadbalancing health check for ILB.",
Description: DescriptionForDefaultILBHealthChecks,
Type: string(protocol),
}

Expand Down Expand Up @@ -326,5 +334,5 @@ func ApplyProbeSettingsToHC(p *v1.Probe, hc *HealthCheck) {
hc.CheckIntervalSec = int64(p.PeriodSeconds) + int64(defaultHealthCheckInterval.Seconds())
}

hc.Description = "Kubernetes L7 health check generated with readiness probe settings."
hc.Description = DescriptionForHealthChecksFromReadinessProbe
}