✨ lb: pick the address family for dedicated server targets#87
Conversation
A dedicated server has no server ID the Load Balancer can point at, so it is added as an IP target. Until now the CCM added both the IPv4 and the IPv6 address unless HCLOUD_LOAD_BALANCERS_DISABLE_IPV6 was set, and that same flag also hides the public IPv6 address of the Load Balancer itself. A cluster that wants IPv6 on the Load Balancer had no way to say it does not want IPv6 targets. On an IPv4 cluster the IPv6 target never passes its health check, because the node ports are not up on IPv6. It sits in the console as a permanently unhealthy target. Adding both addresses also registers the same server twice, which counts twice against the target limit of the Load Balancer type and gives it a double share of the traffic next to a cloud server. HCLOUD_LOAD_BALANCERS_ROBOT_TARGET_ADDRESS_FAMILY now picks the family, and load-balancer.hetzner.cloud/robot-target-address-family overrides it per service. One of ipv4, ipv6 or dualstack, default ipv4. The reconcile loop now also removes a target of the other family. Before, any IP target that mapped to a known dedicated server counted as wanted, so changing the setting left the old targets on the Load Balancer for good. Two smaller fixes along the way: a server without an IPv6 network no longer produces the address "1", and a target address is checked with net.ParseIP before it is sent to the API. Note the default: a Load Balancer that has IPv6 targets today drops them on the next reconcile unless the setting asks for ipv6 or dualstack.
| "%v: Invalid value, expected one of: ipv4,ipv6,dualstack", hcloudInstancesAddressFamily) | ||
| f, err := addressfamily.Parse(family) | ||
| if err != nil { | ||
| return -1, fmt.Errorf("%v: %w", hcloudInstancesAddressFamily, err) |
There was a problem hiding this comment.
let's use the pattern "failed to ..." everywhere in error messages
| 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" |
There was a problem hiding this comment.
why not HCLOUD_ADDRESS_FAMILY_ROBOT? This would align with the existing HCLOUD_INSTANCES_ADDRESS_FAMILY which seems to be the equivalent for hcloud. This weird naming pattern that everything starts with "HCLOUD" makes "ROBOT_ADDRESS_FAMILY" impossible
There was a problem hiding this comment.
HCLOUD_INSTANCES_ADDRESS_FAMILY sets node addresses and already covers robot nodes, so it is not really the equivalent, this one only picks LB targets. I would keep the HCLOUD_LOAD_BALANCERS_ prefix, HCLOUD_ROBOT_ADDRESS_FAMILY would clash with that node meaning. if it is just length, drop TARGET
| 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" |
There was a problem hiding this comment.
HCLOUD_INSTANCES_ADDRESS_FAMILY sets node addresses and already covers robot nodes, so it is not really the equivalent, this one only picks LB targets. I would keep the HCLOUD_LOAD_BALANCERS_ prefix, HCLOUD_ROBOT_ADDRESS_FAMILY would clash with that node meaning. if it is just length, drop TARGET
The problem
A dedicated server has no server ID the Load Balancer can point at, so it is added
as an IP target. Today the CCM adds both the IPv4 and the IPv6 address of that
server unless
HCLOUD_LOAD_BALANCERS_DISABLE_IPV6is set, and that same flag alsohides the public IPv6 address of the Load Balancer from the Service status. A
cluster that wants IPv6 on the Load Balancer has no way to say that it does not
want IPv6 targets.
On an IPv4 cluster the IPv6 target can never pass its health check, because the
node ports are not up on IPv6. It sits in the Hetzner console as a permanently
failed check, which is what customers report.
Measured on a live cluster running v2.0.6, two dedicated servers and two cloud
servers, traefik behind a Load Balancer:
Cilium on that cluster runs with
enable-ipv6 = false, so the two v6 targetscannot ever come up. Setting
load-balancer.hetzner.cloud/ipv6-disabled: "true"removed the IPv6 address from the Service status but left all four IP targets in
place, so there was no way out short of deleting the Service.
It is not only cosmetic. The same server registered twice counts twice against the
target limit of the Load Balancer type, 25 on an lb11, and takes a double share of
round_robinnext to a cloud server.The change
HCLOUD_LOAD_BALANCERS_ROBOT_TARGET_ADDRESS_FAMILYpicks the family used for theIP target of a dedicated server. One of
ipv4,ipv6ordualstack, defaultipv4. The annotationload-balancer.hetzner.cloud/robot-target-address-familyoverrides it per service. It is separate from
HCLOUD_LOAD_BALANCERS_DISABLE_IPV6, which keeps doing only its own job on thepublic IPv6 address of the Load Balancer.
The reconcile loop now also removes a target of the other family. Before, any IP
target that mapped to a known dedicated server counted as wanted, so changing the
setting only affected newly created Load Balancers and left the old targets behind
for good.
How this compares to upstream
Upstream
hcloud-cloud-controller-managernever adds an IPv6 target for adedicated server at all. It only builds
robotIDToIPv4, so dual stack does notwork there in the first place.
This change keeps both cases working instead of picking one.
ipv4matchesupstream and is the default, so an IPv4 cluster stops showing failed checks in the
Hetzner UI.
ipv6covers a cluster whose network really does carry IPv6, whichupstream cannot do, and it registers one target per node rather than two, so the
checks pass there as well.
dualstackstays available for anyone who wants theold behavior on purpose.
Lining the default up with upstream also removes one more difference between this
fork and upstream, which the README lists as the goal.
Note on the default
A Load Balancer that has IPv6 targets today drops them on the next reconcile
unless the setting asks for
ipv6ordualstack. That is the intended fix, thosetargets are the failed checks, but it is a behavior change on upgrade and worth
calling out in the release notes.
Also in here
Two small fixes in the same code path:
"1", becausethe code concatenated an empty
ServerIPv6Netwith"1". The empty check didnot catch it, so it reached
net.ParseIPas an invalid address.net.ParseIPbefore it is sent to the API.Structure
addressFamilymoved into a newinternal/addressfamilypackage sohcopscanuse it.
hcloudalready importshcops, so the type could not stay where it was.hcloud/instances.gokeeps the old names through a type alias and constre-exports, so no existing call site changed.
Testing
go build ./...andgo test ./...pass. New cases inTestLoadBalancerOps_ReconcileHCLBTargetscover ipv4 only, ipv6 only, theannotation override, removal of a stale IPv6 target, and a server with no IPv6
network. The new package has its own unit tests.
The failing state above was reproduced on a live cluster with v2.0.6. The new
build has not been rolled out to that cluster yet.