Disable PROXY protocol for the default health check cases (#739)

If the health check override is specified, then we will default to the setting on the LB.
digitalocean · Jun 12, 2024 · fbaf28d · fbaf28d
1 parent a01758b
commit fbaf28d
Show file tree

Hide file tree

Showing 7 changed files with 40 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,13 @@
 ## unreleased
 
+* Fixes an issue with load balancer health checks when the LB is using PROXY protocol. The new health check 
+  implementation (introduced in v0.1.51), now probes either kube proxy (Cluster) or the health check node port (Local).
+  If the LB enables PROXY protocol, this alters the health check behavior to also use PROXY protocol. Since these Kubernetes
+  components don't support PROXY protocol, this caused worker nodes to be marked as unhealthy. Support was added to the
+  load balancer health check to optionally enable/disable PROXY protocol. When using the default health check implementation
+  the health check will disable PROXY protocol. If the `service.beta.kubernetes.io/do-loadbalancer-override-health-check` is
+  provided, then the health check will default to the setting on the LB.
+
 ## v0.1.53 (beta) - June  7, 2024
 
 * Adding support for internal load balancers (NOTE: this is a closed beta feature, contact DigitalOcean 

diff --git a/cloud-controller-manager/do/loadbalancers.go b/cloud-controller-manager/do/loadbalancers.go
@@ -663,6 +663,8 @@ func buildHealthCheck(service *v1.Service) (*godo.HealthCheck, error) {
 	// Default health check behavior
 	hcPath, hcPort := healthCheckPathAndPort(service)
 	hcProtocol := protocolHTTP
+	// Disable PROXY protocol when health checking kube-proxy or the health check node port (which don't support it)
+	proxyProtocol := godo.PtrTo(false)
 
 	// Permit changing the default health check behavior if the override annotation
 	// is set.
@@ -678,6 +680,8 @@ func buildHealthCheck(service *v1.Service) (*godo.HealthCheck, error) {
 		if err != nil {
 			return nil, err
 		}
+		// Revert to the behaviour on the LB
+		proxyProtocol = nil
 	}
 
 	checkIntervalSecs, err := healthCheckIntervalSeconds(service)
@@ -705,6 +709,7 @@ func buildHealthCheck(service *v1.Service) (*godo.HealthCheck, error) {
 		ResponseTimeoutSeconds: responseTimeoutSecs,
 		UnhealthyThreshold:     unhealthyThreshold,
 		HealthyThreshold:       healthyThreshold,
+		ProxyProtocol:          proxyProtocol,
 	}, nil
 }
 

diff --git a/cloud-controller-manager/do/loadbalancers_test.go b/cloud-controller-manager/do/loadbalancers_test.go
@@ -160,10 +160,10 @@ func createHTTPSLB(lbID, certID, certType string) (*godo.LoadBalancer, *godo.Cer
 }
 
 func defaultHealthCheck(port int) *godo.HealthCheck {
-	return healthCheck(protocolHTTP, port, "/healthz")
+	return healthCheck(protocolHTTP, port, "/healthz", godo.PtrTo(false))
 }
 
-func healthCheck(protocol string, port int, path string) *godo.HealthCheck {
+func healthCheck(protocol string, port int, path string, proxyProtocol *bool) *godo.HealthCheck {
 	svc := &v1.Service{}
 	is, _ := healthCheckIntervalSeconds(svc)
 	rts, _ := healthCheckResponseTimeoutSeconds(svc)
@@ -178,6 +178,7 @@ func healthCheck(protocol string, port int, path string) *godo.HealthCheck {
 		ResponseTimeoutSeconds: rts,
 		UnhealthyThreshold:     ut,
 		HealthyThreshold:       ht,
+		ProxyProtocol:          proxyProtocol,
 	}
 }
 
@@ -2295,6 +2296,7 @@ func Test_buildHealthCheck(t *testing.T) {
 				ResponseTimeoutSeconds: 5,
 				UnhealthyThreshold:     3,
 				HealthyThreshold:       5,
+				ProxyProtocol:          godo.PtrTo(false),
 			},
 		},
 		{
@@ -2318,6 +2320,7 @@ func Test_buildHealthCheck(t *testing.T) {
 				ResponseTimeoutSeconds: 5,
 				UnhealthyThreshold:     3,
 				HealthyThreshold:       5,
+				ProxyProtocol:          godo.PtrTo(false),
 			},
 		},
 		{
@@ -2423,6 +2426,7 @@ func Test_buildHealthCheck(t *testing.T) {
 				ResponseTimeoutSeconds: 5,
 				UnhealthyThreshold:     3,
 				HealthyThreshold:       5,
+				ProxyProtocol:          godo.PtrTo(false),
 			},
 		},
 		{
@@ -2457,6 +2461,7 @@ func Test_buildHealthCheck(t *testing.T) {
 				ResponseTimeoutSeconds: 3,
 				UnhealthyThreshold:     1,
 				HealthyThreshold:       2,
+				ProxyProtocol:          godo.PtrTo(false),
 			},
 		},
 		{
@@ -2596,7 +2601,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("tcp", 30000, ""),
+			healthcheck: healthCheck("tcp", 30000, "", nil),
 		},
 		{
 			name: "default health check with http service protocol",
@@ -2620,7 +2625,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("tcp", 30000, ""),
+			healthcheck: healthCheck("tcp", 30000, "", nil),
 		},
 		{
 			name: "default health check with https service protocol",
@@ -2645,7 +2650,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("tcp", 30000, ""),
+			healthcheck: healthCheck("tcp", 30000, "", nil),
 		},
 		{
 			name: "default health check with TLS passthrough",
@@ -2670,7 +2675,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("tcp", 30000, ""),
+			healthcheck: healthCheck("tcp", 30000, "", nil),
 		},
 		{
 			name: "https health check",
@@ -2695,7 +2700,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("tcp", 30000, ""),
+			healthcheck: healthCheck("tcp", 30000, "", nil),
 		},
 		{
 			name: "http2 health check",
@@ -2720,7 +2725,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("tcp", 30000, ""),
+			healthcheck: healthCheck("tcp", 30000, "", nil),
 		},
 		{
 			name: "https health check with TLS passthrough",
@@ -2745,7 +2750,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("tcp", 30000, ""),
+			healthcheck: healthCheck("tcp", 30000, "", nil),
 		},
 		{
 			name: "http2 health check with TLS passthrough",
@@ -2770,7 +2775,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("tcp", 30000, ""),
+			healthcheck: healthCheck("tcp", 30000, "", nil),
 		},
 		{
 			name: "explicit http health check protocol and tcp payload protocol",
@@ -2795,7 +2800,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("http", 30000, ""),
+			healthcheck: healthCheck("http", 30000, "", nil),
 		},
 		{
 			name: "explicit http health check protocol and https payload protocol",
@@ -2821,7 +2826,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("http", 30000, ""),
+			healthcheck: healthCheck("http", 30000, "", nil),
 		},
 		{
 			name: "explicit http health check protocol and http2 payload protocol",
@@ -2847,7 +2852,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("http", 30000, ""),
+			healthcheck: healthCheck("http", 30000, "", nil),
 		},
 		{
 			name: "explicit https health check protocol and tcp payload protocol",
@@ -2872,7 +2877,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("https", 30000, ""),
+			healthcheck: healthCheck("https", 30000, "", nil),
 		},
 		{
 			name: "http health check with https and certificate",
@@ -2898,7 +2903,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("http", 30000, ""),
+			healthcheck: healthCheck("http", 30000, "", nil),
 		},
 		{
 			name: "http health check with path",
@@ -2923,7 +2928,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("http", 30000, "/health"),
+			healthcheck: healthCheck("http", 30000, "/health", nil),
 		},
 		{
 			name: "invalid health check using protocol override",
@@ -2979,7 +2984,7 @@ func Test_buildHealthCheckOld(t *testing.T) {
 					},
 				},
 			},
-			healthcheck: healthCheck("http", 32000, "/health"),
+			healthcheck: healthCheck("http", 32000, "/health", nil),
 		},
 		{
 			name: "invalid health check using port override with non-existent port",

diff --git a/go.mod b/go.mod
@@ -6,7 +6,7 @@ toolchain go1.22.2
 
 require (
 	github.com/davecgh/go-spew v1.1.1
-	github.com/digitalocean/godo v1.117.0
+	github.com/digitalocean/godo v1.117.1-0.20240611182056-2abf3ae0efbb
 	github.com/go-logr/logr v1.4.2
 	github.com/google/go-cmp v0.6.0
 	github.com/google/uuid v1.6.0

diff --git a/go.sum b/go.sum
@@ -24,8 +24,8 @@ github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/digitalocean/godo v1.117.0 h1:WVlTe09melDYTd7VCVyvHcNWbgB+uI1O115+5LOtdSw=
-github.com/digitalocean/godo v1.117.0/go.mod h1:Vk0vpCot2HOAJwc5WE8wljZGtJ3ZtWIc8MQ8rF38sdo=
+github.com/digitalocean/godo v1.117.1-0.20240611182056-2abf3ae0efbb h1:Xkl/ehYYnrCPvWhXBbYYMTqct1ejGuU/Yfi4cZaa3BA=
+github.com/digitalocean/godo v1.117.1-0.20240611182056-2abf3ae0efbb/go.mod h1:Vk0vpCot2HOAJwc5WE8wljZGtJ3ZtWIc8MQ8rF38sdo=
 github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
 github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
 github.com/emicklei/go-restful/v3 v3.12.0 h1:y2DdzBAURM29NFF94q6RaY4vjIH1rtwDapwQtU84iWk=

diff --git a/vendor/github.com/digitalocean/godo/load_balancers.go b/vendor/github.com/digitalocean/godo/load_balancers.go
diff --git a/vendor/modules.txt b/vendor/modules.txt
@@ -33,7 +33,7 @@ github.com/coreos/go-systemd/v22/journal
 # github.com/davecgh/go-spew v1.1.1
 ## explicit
 github.com/davecgh/go-spew/spew
-# github.com/digitalocean/godo v1.117.0
+# github.com/digitalocean/godo v1.117.1-0.20240611182056-2abf3ae0efbb
 ## explicit; go 1.20
 github.com/digitalocean/godo
 github.com/digitalocean/godo/metrics