Repository: hetznercloud/hcloud-cloud-controller-manager
Pull request #514 (2023-09-14 10:18:34)

feat(load-balancer): Add new node-selector annotation
Fixes: https://github.com/hetznercloud/hcloud-cloud-controller-manager/issues/506, https://github.com/hetznercloud/hcloud-cloud-controller-manager/issues/373

apricote: Dont worry about the failing e2e tests. They are currently broken for external contributors. I ran them locally and everything passed :)

Patch:
diff --git a/hcloud/load_balancers.go b/hcloud/load_balancers.go
index a60a3163..09be6ef3 100644
--- a/hcloud/load_balancers.go
+++ b/hcloud/load_balancers.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	cloudprovider "k8s.io/cloud-provider"
 	"k8s.io/klog/v2"
 
@@ -44,6 +45,29 @@ func newLoadBalancers(lbOps LoadBalancerOps, ac hcops.HCloudActionClient, disabl
 	}
 }
 
+func matchNodeSelector(svc *corev1.Service, nodes []*corev1.Node) ([]*corev1.Node, error) {
+	var (
+		err           error
+		selectedNodes []*corev1.Node
+	)
+
+	selector := labels.Everything()
+	if v, ok := annotation.LBNodeSelector.StringFromService(svc); ok {
+		selector, err = labels.Parse(v)
+		if err != nil {
+			return nil, fmt.Errorf("unable to parse the node-selector annotation: %w", err)
+		}
+	}
+
+	for _, n := range nodes {
+		if selector.Matches(labels.Set(n.GetLabels())) {
+			selectedNodes = append(selectedNodes, n)
+		}
+	}
+
+	return selectedNodes, nil
+}
+
 func (l *loadBalancers) GetLoadBalancer(
 	ctx context.Context, _ string, service *corev1.Service,
 ) (status *corev1.LoadBalancerStatus, exists bool, err error) {
@@ -97,13 +121,19 @@ func (l *loadBalancers) EnsureLoadBalancer(
 	metrics.OperationCalled.WithLabelValues(op).Inc()
 
 	var (
-		reload bool
-		lb     *hcloud.LoadBalancer
-		err    error
+		reload        bool
+		lb            *hcloud.LoadBalancer
+		err           error
+		selectedNodes []*corev1.Node
 	)
 
-	nodeNames := make([]string, len(nodes))
-	for i, n := range nodes {
+	selectedNodes, err = matchNodeSelector(svc, nodes)
+	if err != nil {
+		return nil, fmt.Errorf("%s: %w", op, err)
+	}
+
+	nodeNames := make([]string, len(selectedNodes))
+	for i, n := range selectedNodes {
 		nodeNames[i] = n.Name
 	}
 	klog.InfoS("ensure Load Balancer", "op", op, "service", svc.Name, "nodes", nodeNames)
@@ -149,7 +179,7 @@ func (l *loadBalancers) EnsureLoadBalancer(
 	}
 	reload = reload || servicesChanged
 
-	targetsChanged, err := l.lbOps.ReconcileHCLBTargets(ctx, lb, svc, nodes)
+	targetsChanged, err := l.lbOps.ReconcileHCLBTargets(ctx, lb, svc, selectedNodes)
 	if err != nil {
 		return nil, fmt.Errorf("%s: %w", op, err)
 	}
@@ -236,12 +266,18 @@ func (l *loadBalancers) UpdateLoadBalancer(
 	metrics.OperationCalled.WithLabelValues(op).Inc()
 
 	var (
-		lb  *hcloud.LoadBalancer
-		err error
+		lb            *hcloud.LoadBalancer
+		err           error
+		selectedNodes []*corev1.Node
 	)
 
-	nodeNames := make([]string, len(nodes))
-	for i, n := range nodes {
+	selectedNodes, err = matchNodeSelector(svc, nodes)
+	if err != nil {
+		return fmt.Errorf("%s: %w", op, err)
+	}
+
+	nodeNames := make([]string, len(selectedNodes))
+	for i, n := range selectedNodes {
 		nodeNames[i] = n.Name
 	}
 	klog.InfoS("update Load Balancer", "op", op, "service", svc.Name, "nodes", nodeNames)
@@ -263,7 +299,7 @@ func (l *loadBalancers) UpdateLoadBalancer(
 	if _, err = l.lbOps.ReconcileHCLB(ctx, lb, svc); err != nil {
 		return fmt.Errorf("%s: %w", op, err)
 	}
-	if _, err = l.lbOps.ReconcileHCLBTargets(ctx, lb, svc, nodes); err != nil {
+	if _, err = l.lbOps.ReconcileHCLBTargets(ctx, lb, svc, selectedNodes); err != nil {
 		return fmt.Errorf("%s: %w", op, err)
 	}
 	if _, err = l.lbOps.ReconcileHCLBServices(ctx, lb, svc); err != nil {
diff --git a/internal/annotation/load_balancer.go b/internal/annotation/load_balancer.go
index 016f5702..0e5fd866 100644
--- a/internal/annotation/load_balancer.go
+++ b/internal/annotation/load_balancer.go
@@ -98,6 +98,16 @@ const (
 	// Mutually exclusive with LBLocation.
 	LBNetworkZone Name = "load-balancer.hetzner.cloud/network-zone"
 
+	// LBNodeSelector can be set to restrict which Nodes are added as targets to the
+	// Load Balancer. It accepts a Kubernetes label selector string, using either the
+	// set-based or equality-based formats.
+	//
+	// If the selector can not be parsed, the targets in the Load Balancer are not
+	// updated and an Event is created with the error message.
+	//
+	// Format: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors
+	LBNodeSelector Name = "load-balancer.hetzner.cloud/node-selector"
+
 	// LBSvcProxyProtocol specifies if the Load Balancer services should
 	// use the proxy protocol.
 	//


Test patch:
diff --git a/hcloud/load_balancers_test.go b/hcloud/load_balancers_test.go
index db6de804..3ced86a6 100644
--- a/hcloud/load_balancers_test.go
+++ b/hcloud/load_balancers_test.go
@@ -3,16 +3,27 @@ package hcloud
 import (
 	"errors"
 	"net"
+	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/annotation"
 	"github.com/hetznercloud/hcloud-cloud-controller-manager/internal/hcops"
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 )
 
+func newNodeSelectorNode(name string, labels map[string]string) *corev1.Node {
+	return &corev1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:   name,
+			Labels: labels,
+		},
+	}
+}
+
 func TestLoadBalancers_GetLoadBalancer(t *testing.T) {
 	tests := []LoadBalancerTestCase{
 		{
@@ -713,3 +724,111 @@ func TestLoadBalancers_EnsureLoadBalancerDeleted(t *testing.T) {
 
 	RunLoadBalancerTests(t, tests)
 }
+
+func TestLoadBalancer_matchNodeSelector(t *testing.T) {
+	cases := []struct {
+		name     string
+		service  *corev1.Service
+		k8sNodes []*corev1.Node
+		expected []*corev1.Node
+	}{
+		{
+			name: "no node selector",
+			service: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{},
+			},
+			k8sNodes: []*corev1.Node{
+				newNodeSelectorNode("node1", nil),
+				newNodeSelectorNode("node2", nil),
+			},
+			expected: []*corev1.Node{
+				newNodeSelectorNode("node1", nil),
+				newNodeSelectorNode("node2", nil),
+			},
+		},
+		{
+			name: "empty node selector",
+			service: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						string(annotation.LBNodeSelector): "",
+					},
+				},
+			},
+			k8sNodes: []*corev1.Node{
+				newNodeSelectorNode("node1", nil),
+				newNodeSelectorNode("node2", nil),
+			},
+			expected: []*corev1.Node{
+				newNodeSelectorNode("node1", nil),
+				newNodeSelectorNode("node2", nil),
+			},
+		},
+		{
+			name: "single node selector to select all",
+			service: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						string(annotation.LBNodeSelector): "environment=production",
+					},
+				},
+			},
+			k8sNodes: []*corev1.Node{
+				newNodeSelectorNode("node1", map[string]string{"environment": "production"}),
+				newNodeSelectorNode("node2", map[string]string{"environment": "production"}),
+			},
+			expected: []*corev1.Node{
+				newNodeSelectorNode("node1", map[string]string{"environment": "production"}),
+				newNodeSelectorNode("node2", map[string]string{"environment": "production"}),
+			},
+		},
+		{
+			name: "single node selector to select some",
+			service: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						string(annotation.LBNodeSelector): "environment=production",
+					},
+				},
+			},
+			k8sNodes: []*corev1.Node{
+				newNodeSelectorNode("node1", map[string]string{"environment": "production"}),
+				newNodeSelectorNode("node2", map[string]string{"environment": "staging"}),
+			},
+			expected: []*corev1.Node{
+				newNodeSelectorNode("node1", map[string]string{"environment": "production"}),
+			},
+		},
+		{
+			name: "multiple node selector to select all",
+			service: &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						string(annotation.LBNodeSelector): "environment=production,zone=nue",
+					},
+				},
+			},
+			k8sNodes: []*corev1.Node{
+				newNodeSelectorNode("node1", map[string]string{"environment": "production", "zone": "fsn"}),
+				newNodeSelectorNode("node2", map[string]string{"environment": "production", "zone": "nue"}),
+			},
+			expected: []*corev1.Node{
+				newNodeSelectorNode("node2", map[string]string{"environment": "production", "zone": "nue"}),
+			},
+		},
+	}
+
+	for _, c := range cases {
+		c := c // prevent scopelint from complaining
+		t.Run(c.name, func(t *testing.T) {
+			nodes, err := matchNodeSelector(c.service, c.k8sNodes)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if !reflect.DeepEqual(nodes, c.expected) {
+				t.Errorf("expected: %+v got %+v", c.expected, nodes)
+			}
+		})
+	}
+}