Repository: hetznercloud/hcloud-cloud-controller-manager (go)

Issue:
Not possible to configure the annotation load-balancer.hetzner.cloud/health-check-port
As far as I can tell there is no working way to specify the health-check-port the controller should configure.

* it's not possible to set it as an integer. From what I found only strings are allowed in annotations/labels (https://github.com/kubernetes/kubernetes/issues/57509)
* if it's not an integer the controller doesn't process the annotation

https://github.com/hetznercloud/hcloud-cloud-controller-manager/commit/df90212c1c022a1cb892c19466d6efa38cb925f2#r40335885

Pull request:
Fix health check defaults
Closes #64 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 71068065..950292f3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,7 +6,9 @@ master
 
 * Fix: nil pointer dereference when Network was not found
 * Update hcloud-go to 1.18.1
-* Add `HCLOUD_LOAD_BALANCERS_ENABLED` env variable to disable the Load Balancer feature, per default the Load Balancers are enabled
+* Add `HCLOUD_LOAD_BALANCERS_ENABLED` env variable to disable the Load
+  Balancer feature, per default the Load Balancers are enabled.
+* Use defaults if health check annotations are not set.
 
 v1.6.1
 ------
diff --git a/internal/hcops/load_balancer.go b/internal/hcops/load_balancer.go
index 9fe95e82..9b897310 100644
--- a/internal/hcops/load_balancer.go
+++ b/internal/hcops/load_balancer.go
@@ -459,7 +459,7 @@ type hclbServiceOptsBuilder struct {
 
 	listenPort      int
 	destinationPort int
-	proxyProtocol   bool
+	proxyProtocol   *bool
 	protocol        hcloud.LoadBalancerServiceProtocol
 	httpOpts        struct {
 		CookieName     *string
@@ -471,10 +471,10 @@ type hclbServiceOptsBuilder struct {
 	addHTTP         bool
 	healthCheckOpts struct {
 		Protocol hcloud.LoadBalancerServiceProtocol
-		Port     int
-		Interval time.Duration
-		Timeout  time.Duration
-		Retries  int
+		Port     *int
+		Interval *time.Duration
+		Timeout  *time.Duration
+		Retries  *int
 		httpOpts struct {
 			Domain      *string
 			Path        *string
@@ -503,7 +503,7 @@ func (b *hclbServiceOptsBuilder) extract() {
 		if err != nil {
 			return fmt.Errorf("%s: %w", op, err)
 		}
-		b.proxyProtocol = pp
+		b.proxyProtocol = hcloud.Bool(pp)
 		return nil
 	})
 
@@ -583,6 +583,11 @@ func (b *hclbServiceOptsBuilder) extractHealthCheck() {
 	b.do(func() error {
 		p, err := annotation.LBSvcHealthCheckProtocol.LBSvcProtocolFromService(b.Service)
 		if errors.Is(err, annotation.ErrNotSet) {
+			// Set the service protocol but do not set the addHealthCheck flag.
+			// This way the health check is configured using the service
+			// protocol only if at least one health check annotation is
+			// present.
+			b.healthCheckOpts.Protocol = b.protocol
 			return nil
 		}
 		if err != nil {
@@ -601,7 +606,7 @@ func (b *hclbServiceOptsBuilder) extractHealthCheck() {
 		if err != nil {
 			return fmt.Errorf("%s: %w", op, err)
 		}
-		b.healthCheckOpts.Port = hcPort
+		b.healthCheckOpts.Port = hcloud.Int(hcPort)
 		b.addHealthCheck = true
 		return nil
 	})
@@ -614,7 +619,7 @@ func (b *hclbServiceOptsBuilder) extractHealthCheck() {
 		if err != nil {
 			return fmt.Errorf("%s: %w", op, err)
 		}
-		b.healthCheckOpts.Interval = time.Duration(hcInterval)
+		b.healthCheckOpts.Interval = hcloud.Duration(hcInterval)
 		b.addHealthCheck = true
 		return nil
 	})
@@ -627,7 +632,7 @@ func (b *hclbServiceOptsBuilder) extractHealthCheck() {
 		if err != nil {
 			return fmt.Errorf("%s: %w", op, err)
 		}
-		b.healthCheckOpts.Timeout = time.Duration(t)
+		b.healthCheckOpts.Timeout = hcloud.Duration(t)
 		b.addHealthCheck = true
 		return nil
 	})
@@ -640,7 +645,7 @@ func (b *hclbServiceOptsBuilder) extractHealthCheck() {
 		if err != nil {
 			return fmt.Errorf("%s: %w", op, err)
 		}
-		b.healthCheckOpts.Retries = v
+		b.healthCheckOpts.Retries = hcloud.Int(v)
 		b.addHealthCheck = true
 		return nil
 	})
@@ -705,7 +710,7 @@ func (b *hclbServiceOptsBuilder) buildAddServiceOpts() (hcloud.LoadBalancerAddSe
 		ListenPort:      hcloud.Int(b.listenPort),
 		DestinationPort: hcloud.Int(b.destinationPort),
 		Protocol:        b.protocol,
-		Proxyprotocol:   hcloud.Bool(b.proxyProtocol),
+		Proxyprotocol:   b.proxyProtocol,
 	}
 	if b.addHTTP {
 		opts.HTTP = &hcloud.LoadBalancerAddServiceOptsHTTP{
@@ -719,10 +724,10 @@ func (b *hclbServiceOptsBuilder) buildAddServiceOpts() (hcloud.LoadBalancerAddSe
 	if b.addHealthCheck {
 		opts.HealthCheck = &hcloud.LoadBalancerAddServiceOptsHealthCheck{
 			Protocol: b.healthCheckOpts.Protocol,
-			Interval: &b.healthCheckOpts.Interval,
-			Port:     &b.healthCheckOpts.Port,
-			Retries:  &b.healthCheckOpts.Retries,
-			Timeout:  &b.healthCheckOpts.Timeout,
+			Interval: b.healthCheckOpts.Interval,
+			Port:     b.healthCheckOpts.Port,
+			Retries:  b.healthCheckOpts.Retries,
+			Timeout:  b.healthCheckOpts.Timeout,
 		}
 		if b.healthCheckOpts.Protocol == hcloud.LoadBalancerServiceProtocolHTTP ||
 			b.healthCheckOpts.Protocol == hcloud.LoadBalancerServiceProtocolHTTPS {
@@ -749,7 +754,7 @@ func (b *hclbServiceOptsBuilder) buildUpdateServiceOpts() (hcloud.LoadBalancerUp
 	opts := hcloud.LoadBalancerUpdateServiceOpts{
 		DestinationPort: hcloud.Int(b.destinationPort),
 		Protocol:        b.protocol,
-		Proxyprotocol:   hcloud.Bool(b.proxyProtocol),
+		Proxyprotocol:   b.proxyProtocol,
 	}
 	if b.addHTTP {
 		opts.HTTP = &hcloud.LoadBalancerUpdateServiceOptsHTTP{
@@ -763,10 +768,10 @@ func (b *hclbServiceOptsBuilder) buildUpdateServiceOpts() (hcloud.LoadBalancerUp
 	if b.addHealthCheck {
 		opts.HealthCheck = &hcloud.LoadBalancerUpdateServiceOptsHealthCheck{
 			Protocol: b.healthCheckOpts.Protocol,
-			Interval: &b.healthCheckOpts.Interval,
-			Port:     &b.healthCheckOpts.Port,
-			Retries:  &b.healthCheckOpts.Retries,
-			Timeout:  &b.healthCheckOpts.Timeout,
+			Interval: b.healthCheckOpts.Interval,
+			Port:     b.healthCheckOpts.Port,
+			Retries:  b.healthCheckOpts.Retries,
+			Timeout:  b.healthCheckOpts.Timeout,
 		}
 		if b.healthCheckOpts.Protocol == hcloud.LoadBalancerServiceProtocolHTTP ||
 			b.healthCheckOpts.Protocol == hcloud.LoadBalancerServiceProtocolHTTPS {


Test patch:
diff --git a/internal/hcops/load_balancer_internal_test.go b/internal/hcops/load_balancer_internal_test.go
index 8c7b752f..408b3406 100644
--- a/internal/hcops/load_balancer_internal_test.go
+++ b/internal/hcops/load_balancer_internal_test.go
@@ -25,12 +25,10 @@ func TestHCLBServiceOptsBuilder(t *testing.T) {
 				ListenPort:      hcloud.Int(80),
 				DestinationPort: hcloud.Int(8080),
 				Protocol:        hcloud.LoadBalancerServiceProtocolTCP,
-				Proxyprotocol:   hcloud.Bool(false),
 			},
 			expectedUpdateOpts: hcloud.LoadBalancerUpdateServiceOpts{
 				DestinationPort: hcloud.Int(8080),
 				Protocol:        hcloud.LoadBalancerServiceProtocolTCP,
-				Proxyprotocol:   hcloud.Bool(false),
 			},
 		},
 		{
@@ -66,7 +64,6 @@ func TestHCLBServiceOptsBuilder(t *testing.T) {
 				ListenPort:      hcloud.Int(82),
 				DestinationPort: hcloud.Int(8082),
 				Protocol:        hcloud.LoadBalancerServiceProtocolHTTP,
-				Proxyprotocol:   hcloud.Bool(false),
 				HTTP: &hcloud.LoadBalancerAddServiceOptsHTTP{
 					CookieName:     hcloud.String("my-cookie"),
 					CookieLifetime: hcloud.Duration(time.Hour),
@@ -78,7 +75,6 @@ func TestHCLBServiceOptsBuilder(t *testing.T) {
 			expectedUpdateOpts: hcloud.LoadBalancerUpdateServiceOpts{
 				DestinationPort: hcloud.Int(8082),
 				Protocol:        hcloud.LoadBalancerServiceProtocolHTTP,
-				Proxyprotocol:   hcloud.Bool(false),
 				HTTP: &hcloud.LoadBalancerUpdateServiceOptsHTTP{
 					CookieName:     hcloud.String("my-cookie"),
 					CookieLifetime: hcloud.Duration(time.Hour),
@@ -88,6 +84,31 @@ func TestHCLBServiceOptsBuilder(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:        "add health check with default protocol",
+			servicePort: v1.ServicePort{Port: 83, NodePort: 8083},
+			serviceAnnotations: map[annotation.Name]interface{}{
+				annotation.LBSvcProtocol:        hcloud.LoadBalancerServiceProtocolTCP,
+				annotation.LBSvcHealthCheckPort: 8084,
+			},
+			expectedAddOpts: hcloud.LoadBalancerAddServiceOpts{
+				ListenPort:      hcloud.Int(83),
+				DestinationPort: hcloud.Int(8083),
+				Protocol:        hcloud.LoadBalancerServiceProtocolTCP,
+				HealthCheck: &hcloud.LoadBalancerAddServiceOptsHealthCheck{
+					Protocol: hcloud.LoadBalancerServiceProtocolTCP,
+					Port:     hcloud.Int(8084),
+				},
+			},
+			expectedUpdateOpts: hcloud.LoadBalancerUpdateServiceOpts{
+				DestinationPort: hcloud.Int(8083),
+				Protocol:        hcloud.LoadBalancerServiceProtocolTCP,
+				HealthCheck: &hcloud.LoadBalancerUpdateServiceOptsHealthCheck{
+					Protocol: hcloud.LoadBalancerServiceProtocolTCP,
+					Port:     hcloud.Int(8084),
+				},
+			},
+		},
 		{
 			name:        "add TCP health check",
 			servicePort: v1.ServicePort{Port: 83, NodePort: 8083},
@@ -102,7 +123,6 @@ func TestHCLBServiceOptsBuilder(t *testing.T) {
 				ListenPort:      hcloud.Int(83),
 				DestinationPort: hcloud.Int(8083),
 				Protocol:        hcloud.LoadBalancerServiceProtocolTCP,
-				Proxyprotocol:   hcloud.Bool(false),
 				HealthCheck: &hcloud.LoadBalancerAddServiceOptsHealthCheck{
 					Protocol: hcloud.LoadBalancerServiceProtocolTCP,
 					Port:     hcloud.Int(8084),
@@ -114,7 +134,6 @@ func TestHCLBServiceOptsBuilder(t *testing.T) {
 			expectedUpdateOpts: hcloud.LoadBalancerUpdateServiceOpts{
 				DestinationPort: hcloud.Int(8083),
 				Protocol:        hcloud.LoadBalancerServiceProtocolTCP,
-				Proxyprotocol:   hcloud.Bool(false),
 				HealthCheck: &hcloud.LoadBalancerUpdateServiceOptsHealthCheck{
 					Protocol: hcloud.LoadBalancerServiceProtocolTCP,
 					Port:     hcloud.Int(8084),
@@ -142,7 +161,6 @@ func TestHCLBServiceOptsBuilder(t *testing.T) {
 				ListenPort:      hcloud.Int(84),
 				DestinationPort: hcloud.Int(8084),
 				Protocol:        hcloud.LoadBalancerServiceProtocolTCP,
-				Proxyprotocol:   hcloud.Bool(false),
 				HealthCheck: &hcloud.LoadBalancerAddServiceOptsHealthCheck{
 					Protocol: hcloud.LoadBalancerServiceProtocolHTTP,
 					Port:     hcloud.Int(8085),
@@ -160,7 +178,6 @@ func TestHCLBServiceOptsBuilder(t *testing.T) {
 			expectedUpdateOpts: hcloud.LoadBalancerUpdateServiceOpts{
 				DestinationPort: hcloud.Int(8084),
 				Protocol:        hcloud.LoadBalancerServiceProtocolTCP,
-				Proxyprotocol:   hcloud.Bool(false),
 				HealthCheck: &hcloud.LoadBalancerUpdateServiceOptsHealthCheck{
 					Protocol: hcloud.LoadBalancerServiceProtocolHTTP,
 					Port:     hcloud.Int(8085),
diff --git a/internal/hcops/load_balancer_test.go b/internal/hcops/load_balancer_test.go
index 3cd84b02..c41afe93 100644
--- a/internal/hcops/load_balancer_test.go
+++ b/internal/hcops/load_balancer_test.go
@@ -755,7 +755,6 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) {
 			mock: func(t *testing.T, tt *LBReconcilementTestCase) {
 				opts := hcloud.LoadBalancerAddServiceOpts{
 					Protocol:        hcloud.LoadBalancerServiceProtocolTCP,
-					Proxyprotocol:   hcloud.Bool(false),
 					ListenPort:      hcloud.Int(80),
 					DestinationPort: hcloud.Int(8080),
 				}
@@ -764,7 +763,6 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) {
 
 				opts = hcloud.LoadBalancerAddServiceOpts{
 					Protocol:        hcloud.LoadBalancerServiceProtocolTCP,
-					Proxyprotocol:   hcloud.Bool(false),
 					ListenPort:      hcloud.Int(443),
 					DestinationPort: hcloud.Int(8443),
 				}
@@ -796,7 +794,6 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) {
 			mock: func(t *testing.T, tt *LBReconcilementTestCase) {
 				opts := hcloud.LoadBalancerAddServiceOpts{
 					Protocol:        hcloud.LoadBalancerServiceProtocolHTTP,
-					Proxyprotocol:   hcloud.Bool(false),
 					ListenPort:      hcloud.Int(81),
 					DestinationPort: hcloud.Int(8081),
 				}
@@ -805,7 +802,6 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) {
 
 				opts = hcloud.LoadBalancerAddServiceOpts{
 					Protocol:        hcloud.LoadBalancerServiceProtocolHTTP,
-					Proxyprotocol:   hcloud.Bool(false),
 					ListenPort:      hcloud.Int(444),
 					DestinationPort: hcloud.Int(8444),
 				}
@@ -839,7 +835,6 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) {
 			mock: func(t *testing.T, tt *LBReconcilementTestCase) {
 				opts := hcloud.LoadBalancerUpdateServiceOpts{
 					Protocol:        hcloud.LoadBalancerServiceProtocolTCP,
-					Proxyprotocol:   hcloud.Bool(false),
 					DestinationPort: hcloud.Int(8081),
 				}
 				action := tt.fx.MockUpdateService(opts, tt.initialLB, 80, nil)
@@ -847,7 +842,6 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) {
 
 				opts = hcloud.LoadBalancerUpdateServiceOpts{
 					Protocol:        hcloud.LoadBalancerServiceProtocolTCP,
-					Proxyprotocol:   hcloud.Bool(false),
 					DestinationPort: hcloud.Int(8444),
 				}
 				action = tt.fx.MockUpdateService(opts, tt.initialLB, 443, nil)