Merge pull request #1108 from spencerhance/cp-1092-r-1-9

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

pkg/apis/backendconfig/v1/types.go

@@ -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"`

pkg/healthchecks/healthcheck.go

@@ -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
 	}
 
@@ -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"
 	}
 }
 
@@ -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

pkg/healthchecks/healthchecks.go

@@ -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
 
@@ -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))

pkg/healthchecks/healthchecks_test.go

@@ -78,6 +78,7 @@ func init() {
 						TimeoutSec:         &num,
 						HealthyThreshold:   &num,
 						UnhealthyThreshold: &num,
+						Port:               &num,
 					},
 				} {
 					sp := &utils.ServicePort{
@@ -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)
@@ -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"
@@ -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),