✨ lb: pick the address family for dedicated server targets

docs/load_balancers.md

@@ -94,6 +94,27 @@ For convenience, you can set the following environment variables as cluster-wide
 * `HCLOUD_LOAD_BALANCERS_DISABLE_PRIVATE_INGRESS`
 * `HCLOUD_LOAD_BALANCERS_USE_PRIVATE_IP`
 * `HCLOUD_LOAD_BALANCERS_ENABLED`
+* `HCLOUD_LOAD_BALANCERS_ROBOT_TARGET_ADDRESS_FAMILY`
+
+## Targets for dedicated servers
+
+A dedicated server has no server ID the Load Balancer can point at, so it is
+added as an IP target.
+`HCLOUD_LOAD_BALANCERS_ROBOT_TARGET_ADDRESS_FAMILY` picks which address is used,
+and the `load-balancer.hetzner.cloud/robot-target-address-family` annotation
+overrides it per service. One of `ipv4`, `ipv6` or `dualstack`, default `ipv4`.
+
+Pick the family the cluster network carries. The Load Balancer reaches a node
+over IPv6 only if the pod network and the node ports are up on IPv6, so on an
+IPv4 cluster an IPv6 target never passes its health check.
+
+`dualstack` adds both addresses of the same server. That registers the server
+twice, so it counts twice against the target limit of the Load Balancer type and
+takes a double share of the traffic compared to a cloud server. Use it only if
+you really want both.
+
+This setting is separate from `HCLOUD_LOAD_BALANCERS_DISABLE_IPV6`, which
+controls the public IPv6 address of the Load Balancer itself.
 
 ## Reference existing Load Balancers
 

hcloud/cloud.go

@@ -30,6 +30,7 @@ import (
 
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
 	"github.com/hetznercloud/hcloud-go/v2/hcloud/metadata"
+	"github.com/syself/hetzner-cloud-controller-manager/internal/addressfamily"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/credentials"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/hcops"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/metrics"
@@ -64,6 +65,7 @@ const (
 	hcloudLoadBalancersDisablePrivateIngress = "HCLOUD_LOAD_BALANCERS_DISABLE_PRIVATE_INGRESS"
 	hcloudLoadBalancersUsePrivateIP          = "HCLOUD_LOAD_BALANCERS_USE_PRIVATE_IP"
 	hcloudLoadBalancersDisableIPv6           = "HCLOUD_LOAD_BALANCERS_DISABLE_IPV6"
+	hcloudLoadBalancersRobotTargetFamily     = "HCLOUD_LOAD_BALANCERS_ROBOT_TARGET_ADDRESS_FAMILY"
 	hcloudMetricsEnabledENVVar               = "HCLOUD_METRICS_ENABLED"
 	UseHrobotProviderIDForBaremetalEnvVar    = "HCLOUD_USE_HROBOT_PROVIDER_ID_FOR_BAREMETAL"
 	hcloudMetricsAddress                     = ":8233"
@@ -224,7 +226,10 @@ func newCloud(_ io.Reader) (cloudprovider.Interface, error) {
 
 	klog.Infof("Hetzner Cloud k8s cloud controller %s started\n", ProviderVersion())
 
-	lbOpsDefaults.DisableIPv6 = lbDisableIPv6
+	lbOpsDefaults.RobotTargetFamily, err = robotTargetFamilyFromEnv()
+	if err != nil {
+		return nil, fmt.Errorf("%s: %w", op, err)
+	}
 
 	eventBroadcaster := record.NewBroadcaster()
 	lbRecorder := eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "hetzner-ccm-loadbalancer"})
@@ -372,17 +377,28 @@ func addressFamilyFromEnv() (addressFamily, error) {
 		return AddressFamilyIPv4, nil
 	}
 
-	switch strings.ToLower(family) {
-	case "ipv6":
-		return AddressFamilyIPv6, nil
-	case "ipv4":
-		return AddressFamilyIPv4, nil
-	case "dualstack":
-		return AddressFamilyDualStack, nil
-	default:
-		return -1, fmt.Errorf(
-			"%v: Invalid value, expected one of: ipv4,ipv6,dualstack", hcloudInstancesAddressFamily)
+	f, err := addressfamily.Parse(family)
+	if err != nil {
+		return -1, fmt.Errorf("failed to parse %v: %w", hcloudInstancesAddressFamily, err)
+	}
+	return f, nil
+}
+
+// robotTargetFamilyFromEnv returns the address family used when a dedicated
+// server is added as an IP target of a load balancer. Returns IPv4 if unset,
+// because the load balancer only reaches a node over IPv6 if the cluster
+// network carries IPv6.
+func robotTargetFamilyFromEnv() (addressfamily.Family, error) {
+	family, ok := os.LookupEnv(hcloudLoadBalancersRobotTargetFamily)
+	if !ok {
+		return addressfamily.IPv4, nil
+	}
+
+	f, err := addressfamily.Parse(family)
+	if err != nil {
+		return -1, fmt.Errorf("failed to parse %v: %w", hcloudLoadBalancersRobotTargetFamily, err)
 	}
+	return f, nil
 }
 
 // getEnvBool returns the boolean parsed from the environment variable with the given key and a potential error

hcloud/instances.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
+	"github.com/syself/hetzner-cloud-controller-manager/internal/addressfamily"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/legacydatacenter"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/metrics"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/providerid"
@@ -31,12 +32,15 @@ import (
 	"k8s.io/klog/v2"
 )
 
-type addressFamily int
+// addressFamily and the AddressFamily constants keep the names this package
+// used before the type moved to internal/addressfamily, where the load
+// balancer code can reach it too.
+type addressFamily = addressfamily.Family
 
 const (
-	AddressFamilyDualStack addressFamily = iota
-	AddressFamilyIPv6
-	AddressFamilyIPv4
+	AddressFamilyDualStack = addressfamily.DualStack
+	AddressFamilyIPv6      = addressfamily.IPv6
+	AddressFamilyIPv4      = addressfamily.IPv4
 )
 
 type instances struct {

internal/addressfamily/addressfamily.go

@@ -0,0 +1,47 @@
+// Package addressfamily selects which IP addresses of a server are used. It is
+// shared by the node address code and the load balancer target code.
+package addressfamily
+
+import (
+	"errors"
+	"strings"
+)
+
+// Family selects which IP addresses of a server are used.
+type Family int
+
+const (
+	// DualStack uses both the IPv4 and the IPv6 address.
+	DualStack Family = iota
+	// IPv6 uses the IPv6 address only.
+	IPv6
+	// IPv4 uses the IPv4 address only.
+	IPv4
+)
+
+// ErrInvalid is returned by Parse for a value that names no address family.
+var ErrInvalid = errors.New("invalid value, expected one of: ipv4,ipv6,dualstack")
+
+// Parse reads the value used in environment variables and service annotations.
+func Parse(v string) (Family, error) {
+	switch strings.ToLower(v) {
+	case "ipv6":
+		return IPv6, nil
+	case "ipv4":
+		return IPv4, nil
+	case "dualstack":
+		return DualStack, nil
+	default:
+		return -1, ErrInvalid
+	}
+}
+
+// UsesIPv4 reports whether f covers the IPv4 address.
+func (f Family) UsesIPv4() bool {
+	return f == IPv4 || f == DualStack
+}
+
+// UsesIPv6 reports whether f covers the IPv6 address.
+func (f Family) UsesIPv6() bool {
+	return f == IPv6 || f == DualStack
+}

internal/addressfamily/addressfamily_test.go

@@ -0,0 +1,56 @@
+package addressfamily_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/syself/hetzner-cloud-controller-manager/internal/addressfamily"
+)
+
+func TestParse(t *testing.T) {
+	tests := []struct {
+		in      string
+		want    addressfamily.Family
+		wantErr bool
+	}{
+		{in: "ipv4", want: addressfamily.IPv4},
+		{in: "ipv6", want: addressfamily.IPv6},
+		{in: "dualstack", want: addressfamily.DualStack},
+		{in: "IPv4", want: addressfamily.IPv4},
+		{in: "DualStack", want: addressfamily.DualStack},
+		{in: "", wantErr: true},
+		{in: "both", wantErr: true},
+	}
+
+	for _, test := range tests {
+		t.Run(test.in, func(t *testing.T) {
+			got, err := addressfamily.Parse(test.in)
+			if test.wantErr {
+				assert.ErrorIs(t, err, addressfamily.ErrInvalid)
+				return
+			}
+			assert.NoError(t, err)
+			assert.Equal(t, test.want, got)
+		})
+	}
+}
+
+func TestFamilyUses(t *testing.T) {
+	tests := []struct {
+		name     string
+		family   addressfamily.Family
+		wantIPv4 bool
+		wantIPv6 bool
+	}{
+		{name: "ipv4", family: addressfamily.IPv4, wantIPv4: true},
+		{name: "ipv6", family: addressfamily.IPv6, wantIPv6: true},
+		{name: "dualstack", family: addressfamily.DualStack, wantIPv4: true, wantIPv6: true},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			assert.Equal(t, test.wantIPv4, test.family.UsesIPv4())
+			assert.Equal(t, test.wantIPv6, test.family.UsesIPv6())
+		})
+	}
+}

internal/annotation/load_balancer.go

@@ -36,6 +36,18 @@ const (
 	// Default: false.
 	LBIPv6Disabled Name = "load-balancer.hetzner.cloud/ipv6-disabled"
 
+	// LBRobotTargetAddressFamily picks the address family used when a dedicated
+	// server is added as an IP target of the Load Balancer.
+	//
+	// The Load Balancer only reaches a node over IPv6 if the cluster network
+	// carries IPv6, so on an IPv4 cluster an IPv6 target never passes its
+	// health check. Adding both families also registers the same server twice,
+	// which counts twice against the target limit and gives it a double share
+	// of the traffic.
+	//
+	// One of: ipv4, ipv6, dualstack. Default: ipv4.
+	LBRobotTargetAddressFamily Name = "load-balancer.hetzner.cloud/robot-target-address-family"
+
 	// LBName is the name of the Load Balancer. The name will be visible in
 	// the Hetzner Cloud API console.
 	LBName Name = "load-balancer.hetzner.cloud/name"

internal/hcops/load_balancer.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
+	"github.com/syself/hetzner-cloud-controller-manager/internal/addressfamily"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/annotation"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/metrics"
 	"github.com/syself/hetzner-cloud-controller-manager/internal/providerid"
@@ -86,7 +87,12 @@ type LoadBalancerDefaults struct {
 	Location     string
 	NetworkZone  string
 	UsePrivateIP bool
-	DisableIPv6  bool
+
+	// RobotTargetFamily picks the address family used when a dedicated server
+	// is added as an IP target. It is unrelated to the public IPv6 address of
+	// the load balancer itself, which HCLOUD_LOAD_BALANCERS_DISABLE_IPV6
+	// controls.
+	RobotTargetFamily addressfamily.Family
 }
 
 // GetByK8SServiceUID tries to find a Load Balancer by its Kubernetes service
@@ -567,15 +573,32 @@ func (l *LoadBalancerOps) togglePublicInterface(ctx context.Context, lb *hcloud.
 	return true, nil
 }
 
-func (l *LoadBalancerOps) getDisableIPv6(svc *corev1.Service) (bool, error) {
-	disable, err := annotation.LBIPv6Disabled.BoolFromService(svc)
-	if err == nil {
-		return disable, nil
+func (l *LoadBalancerOps) getRobotTargetFamily(svc *corev1.Service) (addressfamily.Family, error) {
+	v, ok := annotation.LBRobotTargetAddressFamily.StringFromService(svc)
+	if !ok {
+		return l.Defaults.RobotTargetFamily, nil
 	}
-	if errors.Is(err, annotation.ErrNotSet) {
-		return l.Defaults.DisableIPv6, nil
+	family, err := addressfamily.Parse(v)
+	if err != nil {
+		return -1, fmt.Errorf("failed to parse %s: %w", annotation.LBRobotTargetAddressFamily, err)
+	}
+	return family, nil
+}
+
+// robotTargetIPs returns the addresses of a dedicated server that are used as
+// IP targets of a load balancer. A server contributes at most one address per
+// family, and none if the address is missing.
+func robotTargetIPs(family addressfamily.Family, s models.Server) []string {
+	var ips []string
+	if family.UsesIPv4() && s.ServerIP != "" {
+		ips = append(ips, s.ServerIP)
 	}
-	return false, err
+	// The robot API reports the IPv6 network of a server, for example
+	// 2a01:f48:111:4221::. The server answers on its first address.
+	if family.UsesIPv6() && s.ServerIPv6Net != "" {
+		ips = append(ips, s.ServerIPv6Net+"1")
+	}
+	return ips
 }
 
 // ReconcileHCLBTargets adds or removes target nodes from the Hetzner Cloud
@@ -594,8 +617,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
 		k8sNodeNames     = make(map[int64]string)
 
 		robotIPsToIDs = make(map[string]int)
-		robotIDToIPv4 = make(map[int]string)
-		robotIDToIPv6 = make(map[int]string)
+		robotIDToIPs  = make(map[int][]string)
 		// Set of server IDs assigned as targets to the HC Load Balancer. Some
 		// of the entries may get deleted during reconcilement. In this case
 		// the hclbTargetIDs[id] is always false. If hclbTargetIDs[id] is true,
@@ -611,7 +633,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
 		changed bool
 	)
 
-	disableIPv6, err := l.getDisableIPv6(svc)
+	robotTargetFamily, err := l.getRobotTargetFamily(svc)
 	if err != nil {
 		return changed, fmt.Errorf("%s: %w", op, err)
 	}
@@ -661,11 +683,15 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
 		}
 	}
 
+	// Only the addresses of the configured family count as wanted targets. An
+	// existing target of the other family is not found below, so it is removed
+	// from the load balancer.
 	for _, s := range dedicatedServers {
-		robotIPsToIDs[s.ServerIP] = s.ServerNumber
-		robotIPsToIDs[s.ServerIPv6Net+"1"] = s.ServerNumber
-		robotIDToIPv4[s.ServerNumber] = s.ServerIP
-		robotIDToIPv6[s.ServerNumber] = s.ServerIPv6Net + "1"
+		ips := robotTargetIPs(robotTargetFamily, s)
+		robotIDToIPs[s.ServerNumber] = ips
+		for _, ip := range ips {
+			robotIPsToIDs[ip] = s.ServerNumber
+		}
 	}
 
 	numberOfTargets := len(lb.Targets)
@@ -771,26 +797,22 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
 	// Assign the dedicated servers which are currently assigned as nodes
 	// to the K8S Load Balancer as IP targets to the HC Load Balancer.
 	for id := range k8sNodeIDsRobot {
-		var arr []string
-		if disableIPv6 {
-			arr = []string{
-				robotIDToIPv4[id],
-			}
-		} else {
-			arr = []string{
-				robotIDToIPv4[id],
-				robotIDToIPv6[id],
-			}
+		ips := robotIDToIPs[id]
+		if len(ips) == 0 {
+			klog.InfoS("k8s node found but no corresponding server in robot", "id", id)
+			continue
 		}
 
-		for _, ip := range arr {
+		for _, ip := range ips {
 			// Don't assign the node again if it is already assigned to the HC load
 			// balancer.
 			if hclbTargetIPs[ip] {
 				continue
 			}
-			if ip == "" {
-				klog.InfoS("k8s node found but no corresponding server in robot", "id", id)
+
+			targetIP := net.ParseIP(ip)
+			if targetIP == nil {
+				klog.InfoS("robot server has an address that is not an IP", "op", op, "id", id, "ip", ip)
 				continue
 			}
 
@@ -806,7 +828,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
 
 			klog.InfoS("add target", "op", op, "service", svc.Name, "targetName", k8sNodeNames[int64(id)], "ip", ip)
 			opts := hcloud.LoadBalancerAddIPTargetOpts{
-				IP: net.ParseIP(ip),
+				IP: targetIP,
 			}
 			a, _, err := l.LBClient.AddIPTarget(ctx, lb, opts)
 			if err != nil {