Skip to content
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.

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

Cherry-Pick #1092 [Add support for Port to BackendConfig HealthCheckConfig] to release-1.9 #1108

Merged
merged 1 commit into from
May 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion pkg/apis/backendconfig/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,10 @@ type HealthCheckConfig struct {
// Type is a health check parameter. See
// https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.
Type *string `json:"type,omitempty"`
Port *int64 `json:"port,omitempty"`
// Port is a health check parameter. See
// https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.
// If Port is used, the controller updates portSpecification as well
Port *int64 `json:"port,omitempty"`
// RequestPath is a health check parameter. See
// https://cloud.google.com/compute/docs/reference/rest/v1/healthChecks.
RequestPath *string `json:"requestPath,omitempty"`
Expand Down
13 changes: 10 additions & 3 deletions pkg/healthchecks/healthcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,9 @@ func (hc *HealthCheck) ToAlphaComputeHealthCheck() *computealpha.HealthCheck {
}

func (hc *HealthCheck) merge() {
// Cannot specify both portSpecification and port field.
if hc.PortSpecification != "" {
// Cannot specify both portSpecification and port field unless fixed port is specified.
// This can happen if the user overrides the port using backendconfig
if hc.PortSpecification != "" && hc.PortSpecification != "USE_FIXED_PORT" {
hc.Port = 0
}

Expand Down Expand Up @@ -239,7 +240,9 @@ func (hc *HealthCheck) updateFromBackendConfig(c *backendconfigv1.HealthCheckCon
hc.RequestPath = *c.RequestPath
}
if c.Port != nil {
klog.Warningf("Setting Port is not supported (healthcheck %q, backendconfig = %+v)", hc.Name, c)
hc.Port = *c.Port
// This override is necessary regardless of type
hc.PortSpecification = "USE_FIXED_PORT"
}
}

Expand Down Expand Up @@ -287,6 +290,10 @@ func calculateDiff(old, new *HealthCheck, c *backendconfigv1.HealthCheckConfig)
if c.RequestPath != nil && old.RequestPath != new.RequestPath {
changes.add("RequestPath", old.RequestPath, new.RequestPath)
}
if c.Port != nil && old.Port != new.Port {
changes.add("Port", strconv.FormatInt(old.Port, 10), strconv.FormatInt(new.Port, 10))
}

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

return &changes
Expand Down
2 changes: 2 additions & 0 deletions pkg/healthchecks/healthchecks.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ func (h *HealthChecks) pathFromSvcPort(sp utils.ServicePort) string {
return h.path
}

// formatBackendConfigHC returns a human readable string version of the HealthCheckConfig
func formatBackendConfigHC(b *backendconfigv1.HealthCheckConfig) string {
var ret []string

Expand All @@ -413,6 +414,7 @@ func formatBackendConfigHC(b *backendconfigv1.HealthCheckConfig) string {
{k: "healthyThreshold", v: b.HealthyThreshold},
{k: "unhealthyThreshold", v: b.UnhealthyThreshold},
{k: "timeoutSec", v: b.TimeoutSec},
{k: "port", v: b.Port},
} {
if e.v != nil {
ret = append(ret, fmt.Sprintf("%s=%d", e.k, *e.v))
Expand Down
31 changes: 31 additions & 0 deletions pkg/healthchecks/healthchecks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ func init() {
TimeoutSec: &num,
HealthyThreshold: &num,
UnhealthyThreshold: &num,
Port: &num,
},
} {
sp := &utils.ServicePort{
Expand Down Expand Up @@ -675,6 +676,16 @@ func TestCalculateDiff(t *testing.T) {
hasDiff: true,
})

newHC = DefaultHealthCheck(500, annotations.ProtocolHTTP)
newHC.Port = 500
cases = append(cases, tc{
desc: "Backendconfig Port",
old: DefaultHealthCheck(8080, annotations.ProtocolHTTP),
new: newHC,
c: &backendconfigv1.HealthCheckConfig{Port: i64(500)},
hasDiff: true,
})

for _, tc := range cases {
t.Run(tc.desc, func(t *testing.T) {
diffs := calculateDiff(tc.old, tc.new, tc.c)
Expand Down Expand Up @@ -895,8 +906,26 @@ func TestSyncServicePort(t *testing.T) {
chc.HealthyThreshold = 1234
chc.UnhealthyThreshold = 1234
chc.TimeoutSec = 1234
chc.HttpHealthCheck.Port = 1234
// PortSpecification is set by the controller
chc.HttpHealthCheck.PortSpecification = "USE_FIXED_PORT"
cases = append(cases, &tc{desc: "create backendconfig all", sp: testSPs["HTTP-80-reg-bcall"], wantComputeHC: chc})

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

// BackendConfig port
chc = fixture.hc()
chc.HttpHealthCheck.Port = 1234
// PortSpecification is set by the controller
chc.HttpHealthCheck.PortSpecification = "USE_FIXED_PORT"
sp := utils.ServicePort{
NodePort: 80,
Protocol: annotations.ProtocolHTTP,
BackendNamer: testNamer,
BackendConfig: &backendconfigv1.BackendConfig{Spec: backendconfigv1.BackendConfigSpec{HealthCheck: &backendconfigv1.HealthCheckConfig{Port: i64(1234)}}},
}
cases = append(cases, &tc{desc: "create backendconfig port", sp: &sp, wantComputeHC: chc})

// BackendConfig neg
chc = fixture.neg()
chc.HttpHealthCheck.RequestPath = "/foo"
Expand Down Expand Up @@ -1085,6 +1114,8 @@ func TestSyncServicePort(t *testing.T) {
wantCHC.HealthyThreshold = 1234
wantCHC.UnhealthyThreshold = 1234
wantCHC.TimeoutSec = 1234
wantCHC.HttpHealthCheck.Port = 1234
wantCHC.HttpHealthCheck.PortSpecification = "USE_FIXED_PORT"
cases = append(cases, &tc{
desc: "update preserve backendconfig all",
setup: fixture.setupExistingHCFunc(chc),
Expand Down