From d0f793711587ded4996ec47a365ce34e8580e046 Mon Sep 17 00:00:00 2001 From: Kirill Ilin Date: Tue, 15 Sep 2026 12:16:30 +0500 Subject: [PATCH 01/10] fix(nodes): select target nodes by externalTrafficPolicy Target nodes were always restricted to the nodes running the service's pods, which is the behaviour of externalTrafficPolicy: Local. Services using the default Cluster policy lost every node that kube-proxy could have forwarded from, so a service whose only pod sits on a node where its nodePort is unreachable ends up with no healthy target at all. Follow the policy declared by the service instead, and cover the port mapping and the policy check with unit tests. Assisted-By: LLM Signed-off-by: Kirill Ilin --- src/main.rs | 159 +++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 138 insertions(+), 21 deletions(-) diff --git a/src/main.rs b/src/main.rs index b237572..f9339cf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -297,6 +297,47 @@ async fn get_nodes_by_selector( Ok(nodes) } +/// Get every node of the cluster. +async fn get_all_nodes(context: &Arc) -> RobotLBResult> { + let nodes_api = kube::Api::::all(context.client.clone()); + Ok(nodes_api.list(&ListParams::default()).await?.items) +} + +/// Whether the service asks for traffic to reach only the nodes that host its endpoints. +/// Under the default `Cluster` policy every node is a valid target, because kube-proxy +/// forwards the traffic to a node that actually hosts a pod. +/// +/// +fn is_local_traffic_policy(svc: &Service) -> bool { + svc.spec + .as_ref() + .and_then(|spec| spec.external_traffic_policy.as_deref()) + == Some("Local") +} + +/// Map the ports of a service onto load balancer services, +/// as pairs of a listen port and the node port behind it. +fn collect_lb_services(svc: &Service) -> Vec<(i32, i32)> { + let mut services = Vec::new(); + for port in svc.spec.iter().flat_map(|spec| spec.ports.iter().flatten()) { + let protocol = port.protocol.as_deref().unwrap_or("TCP"); + if protocol != "TCP" { + tracing::warn!("Protocol {} is not supported. Skipping...", protocol); + continue; + } + let Some(node_port) = port.node_port else { + tracing::warn!( + "Service port {} has no nodePort allocated. Hetzner load balancers forward \ + traffic to node IPs, so such a port cannot be exposed. Skipping...", + port.port + ); + continue; + }; + services.push((port.port, node_port)); + } + services +} + /// Reconcile the `LoadBalancer` type of service. /// This function will find the nodes based on the node selector /// and create or update the load balancer. @@ -311,7 +352,11 @@ pub async fn reconcile_load_balancer( } let nodes = if context.config.dynamic_node_selector { - get_nodes_dynamically(&svc, &context).await? + if is_local_traffic_policy(&svc) { + get_nodes_dynamically(&svc, &context).await? + } else { + get_all_nodes(&context).await? + } } else { get_nodes_by_selector(&svc, &context).await? }; @@ -330,26 +375,8 @@ pub async fn reconcile_load_balancer( } } - for port in svc - .spec - .clone() - .unwrap_or_default() - .ports - .unwrap_or_default() - { - let protocol = port.protocol.unwrap_or_else(|| "TCP".to_string()); - if protocol != "TCP" { - tracing::warn!("Protocol {} is not supported. Skipping...", protocol); - continue; - } - let Some(node_port) = port.node_port else { - tracing::warn!( - "Node port is not set for target_port {}. Skipping...", - port.port - ); - continue; - }; - lb.add_service(port.port, node_port); + for (listen_port, node_port) in collect_lb_services(&svc) { + lb.add_service(listen_port, node_port); } let svc_api = kube::Api::::namespaced( @@ -411,3 +438,93 @@ fn on_error(_: Arc, error: &RobotLBError, _context: Arc _ => Action::requeue(Duration::from_secs(30)), } } + +#[cfg(test)] +mod tests { + use super::{collect_lb_services, is_local_traffic_policy}; + use k8s_openapi::api::core::v1::{Service, ServicePort, ServiceSpec}; + + fn service(spec: ServiceSpec) -> Service { + Service { + spec: Some(spec), + ..Default::default() + } + } + + #[test] + fn cluster_policy_is_not_local() { + let svc = service(ServiceSpec { + external_traffic_policy: Some("Cluster".into()), + ..Default::default() + }); + assert!(!is_local_traffic_policy(&svc)); + } + + #[test] + fn unset_policy_defaults_to_cluster() { + assert!(!is_local_traffic_policy(&service(ServiceSpec::default()))); + } + + #[test] + fn local_policy_is_local() { + let svc = service(ServiceSpec { + external_traffic_policy: Some("Local".into()), + ..Default::default() + }); + assert!(is_local_traffic_policy(&svc)); + } + + #[test] + fn ports_map_listen_port_to_node_port() { + let svc = service(ServiceSpec { + ports: Some(vec![ServicePort { + port: 22, + node_port: Some(30821), + protocol: Some("TCP".into()), + ..Default::default() + }]), + ..Default::default() + }); + assert_eq!(collect_lb_services(&svc), vec![(22, 30821)]); + } + + #[test] + fn ports_without_protocol_are_treated_as_tcp() { + let svc = service(ServiceSpec { + ports: Some(vec![ServicePort { + port: 80, + node_port: Some(31571), + ..Default::default() + }]), + ..Default::default() + }); + assert_eq!(collect_lb_services(&svc), vec![(80, 31571)]); + } + + #[test] + fn udp_ports_are_skipped() { + let svc = service(ServiceSpec { + ports: Some(vec![ServicePort { + port: 53, + node_port: Some(30053), + protocol: Some("UDP".into()), + ..Default::default() + }]), + ..Default::default() + }); + assert!(collect_lb_services(&svc).is_empty()); + } + + #[test] + fn ports_without_node_port_are_skipped() { + let svc = service(ServiceSpec { + ports: Some(vec![ServicePort { + port: 22, + protocol: Some("TCP".into()), + ..Default::default() + }]), + ..Default::default() + }); + assert!(collect_lb_services(&svc).is_empty()); + } +} From d5fc1ad84d1bf40cd58bb8c630c9dba27cf56503 Mon Sep 17 00:00:00 2001 From: Kirill Ilin Date: Tue, 15 Sep 2026 12:16:39 +0500 Subject: [PATCH 02/10] fix(targets): keep reconciling after a rejected target Hetzner refuses IP targets outside the vSwitch subnet of the attached network. Such a rejection aborted the whole reconciliation, so a single node the API declines kept every other node out of the load balancer. Log the rejected target and carry on with the rest. Assisted-By: LLM Signed-off-by: Kirill Ilin --- src/lb.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/lb.rs b/src/lb.rs index bac19f1..c868ab5 100644 --- a/src/lb.rs +++ b/src/lb.rs @@ -327,7 +327,7 @@ impl LoadBalancer { .any(|t| t.ip.as_ref().map(|i| i.ip.as_str()) == Some(ip)) { tracing::info!("Adding target {}", ip); - hcloud::apis::load_balancers_api::add_target( + let added = hcloud::apis::load_balancers_api::add_target( &self.hcloud_config, AddTargetParams { id: hcloud_balancer.id, @@ -339,7 +339,12 @@ impl LoadBalancer { }), }, ) - .await?; + .await; + // Hetzner rejects IPs outside the vSwitch subnet of the attached network, + // which must not keep the remaining nodes out of the load balancer. + if let Err(error) = added { + tracing::error!("Cannot add target {ip}: {error}"); + } } } Ok(()) From 7734dd716cc7c0dd4cb14d0cf0d5978cf76d5732 Mon Sep 17 00:00:00 2001 From: Kirill Ilin Date: Tue, 15 Sep 2026 12:16:42 +0500 Subject: [PATCH 03/10] docs(readme): describe node selection and the nodePort requirement Assisted-By: LLM Signed-off-by: Kirill Ilin --- README.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index b3b0585..1537801 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,14 @@ After the chart is installed, you should be able to create `LoadBalancer` servic The operator listens to the Kubernetes API for services of type `LoadBalancer` and creates Hetzner load balancers that point to nodes based on `node-ip`. -Nodes are selected based on where the service's target pods are deployed, which is determined by searching for pods with the service's selector. This behavior can be configured. +Target nodes are selected according to the service's `externalTrafficPolicy`: + +- `Cluster`, the Kubernetes default: every node of the cluster becomes a target, since kube-proxy forwards the traffic to a node that hosts a pod. +- `Local`: only the nodes where the service's target pods run, found through the service selector, or through the service's `EndpointSlice` resources when it has no selector. + +Setting `ROBOTLB_DYNAMIC_NODE_SELECTOR` to `false` replaces both with the node selector from the `robotlb/node-selector` annotation. + +Every port of the service needs an allocated `nodePort`. A Hetzner load balancer forwards traffic to the IP of a node, so a port is reachable only through its `nodePort`: ports without one are skipped, and `allocateLoadBalancerNodePorts: false` is not supported. ## Configuration From 8351284de6c37cace0039d6f4b06ba8871e1dc0d Mon Sep 17 00:00:00 2001 From: Kirill Ilin Date: Tue, 15 Sep 2026 12:29:34 +0500 Subject: [PATCH 04/10] fix(nodes): keep excluded and cordoned nodes out of the balancer Following externalTrafficPolicy made every node a target under the Cluster policy, including nodes a cluster marks as unfit for external load balancers: kubeadm labels control-plane nodes with node.kubernetes.io/exclude-from-external-load-balancers, and a cordoned node is draining. Filter both out. A service left without a single exposable port no longer gets an external IP patched onto its status, so a load balancer that forwards nothing is not reported as ready. Port mapping now carries the existing LBService type instead of a bare pair of integers. Assisted-By: LLM Signed-off-by: Kirill Ilin --- src/consts.rs | 5 +++ src/main.rs | 110 +++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 101 insertions(+), 14 deletions(-) diff --git a/src/consts.rs b/src/consts.rs index 820fa02..0cedb34 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -22,5 +22,10 @@ pub const DEFAULT_LB_LOCATION: &str = "hel1"; pub const DEFAULT_LB_ALGORITHM: &str = "least-connections"; pub const DEFAULT_LB_BALANCER_TYPE: &str = "lb11"; +/// Well-known Kubernetes label marking a node that must never be a load balancer target. +/// kubeadm applies it to every control-plane node. +pub const EXCLUDE_FROM_LB_LABEL_NAME: &str = + "node.kubernetes.io/exclude-from-external-load-balancers"; + pub const FINALIZER_NAME: &str = "robotlb/finalizer"; pub const ROBOTLB_LB_CLASS: &str = "robotlb"; diff --git a/src/main.rs b/src/main.rs index f9339cf..fb1576a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -34,7 +34,7 @@ use kube::{ Resource, ResourceExt, }; use label_filter::LabelFilter; -use lb::LoadBalancer; +use lb::{LBService, LoadBalancer}; use std::{collections::HashSet, str::FromStr, sync::Arc, time::Duration}; pub mod config; @@ -297,10 +297,34 @@ async fn get_nodes_by_selector( Ok(nodes) } -/// Get every node of the cluster. +/// Get every node of the cluster that may serve load balancer traffic. async fn get_all_nodes(context: &Arc) -> RobotLBResult> { let nodes_api = kube::Api::::all(context.client.clone()); - Ok(nodes_api.list(&ListParams::default()).await?.items) + let nodes = nodes_api + .list(&ListParams::default()) + .await? + .into_iter() + .filter(is_lb_eligible_node) + .collect::>(); + Ok(nodes) +} + +/// Whether a node may be used as a load balancer target. +/// Cordoned nodes are draining, and the exclusion label is how a cluster declares +/// that a node must stay out of external load balancers. +fn is_lb_eligible_node(node: &Node) -> bool { + if node + .labels() + .contains_key(consts::EXCLUDE_FROM_LB_LABEL_NAME) + { + tracing::debug!("Node {} is excluded from load balancers", node.name_any()); + return false; + } + if node.spec.as_ref().and_then(|spec| spec.unschedulable) == Some(true) { + tracing::debug!("Node {} is unschedulable", node.name_any()); + return false; + } + true } /// Whether the service asks for traffic to reach only the nodes that host its endpoints. @@ -315,9 +339,8 @@ fn is_local_traffic_policy(svc: &Service) -> bool { == Some("Local") } -/// Map the ports of a service onto load balancer services, -/// as pairs of a listen port and the node port behind it. -fn collect_lb_services(svc: &Service) -> Vec<(i32, i32)> { +/// Map the ports of a service onto load balancer services. +fn collect_lb_services(svc: &Service) -> Vec { let mut services = Vec::new(); for port in svc.spec.iter().flat_map(|spec| spec.ports.iter().flatten()) { let protocol = port.protocol.as_deref().unwrap_or("TCP"); @@ -333,7 +356,10 @@ fn collect_lb_services(svc: &Service) -> Vec<(i32, i32)> { ); continue; }; - services.push((port.port, node_port)); + services.push(LBService { + listen_port: port.port, + target_port: node_port, + }); } services } @@ -375,8 +401,14 @@ pub async fn reconcile_load_balancer( } } - for (listen_port, node_port) in collect_lb_services(&svc) { - lb.add_service(listen_port, node_port); + for service in collect_lb_services(&svc) { + lb.add_service(service.listen_port, service.target_port); + } + + if lb.services.is_empty() { + tracing::warn!( + "Service has no port that can be exposed. Leaving it without an external IP." + ); } let svc_api = kube::Api::::namespaced( @@ -411,7 +443,7 @@ pub async fn reconcile_load_balancer( } } - if !ingress.is_empty() { + if !ingress.is_empty() && !lb.services.is_empty() { svc_api .patch_status( svc.name_any().as_str(), @@ -441,8 +473,12 @@ fn on_error(_: Arc, error: &RobotLBError, _context: Arc #[cfg(test)] mod tests { - use super::{collect_lb_services, is_local_traffic_policy}; - use k8s_openapi::api::core::v1::{Service, ServicePort, ServiceSpec}; + use super::{collect_lb_services, consts, is_lb_eligible_node, is_local_traffic_policy}; + use k8s_openapi::{ + api::core::v1::{Node, NodeSpec, Service, ServicePort, ServiceSpec}, + apimachinery::pkg::apis::meta::v1::ObjectMeta, + }; + use std::collections::BTreeMap; fn service(spec: ServiceSpec) -> Service { Service { @@ -451,6 +487,25 @@ mod tests { } } + fn node(labels: &[(&str, &str)], unschedulable: bool) -> Node { + Node { + metadata: ObjectMeta { + labels: Some( + labels + .iter() + .map(|(key, value)| ((*key).to_string(), (*value).to_string())) + .collect::>(), + ), + ..Default::default() + }, + spec: Some(NodeSpec { + unschedulable: Some(unschedulable), + ..Default::default() + }), + ..Default::default() + } + } + #[test] fn cluster_policy_is_not_local() { let svc = service(ServiceSpec { @@ -485,7 +540,10 @@ mod tests { }]), ..Default::default() }); - assert_eq!(collect_lb_services(&svc), vec![(22, 30821)]); + let services = collect_lb_services(&svc); + assert_eq!(services.len(), 1); + assert_eq!(services[0].listen_port, 22); + assert_eq!(services[0].target_port, 30821); } #[test] @@ -498,7 +556,10 @@ mod tests { }]), ..Default::default() }); - assert_eq!(collect_lb_services(&svc), vec![(80, 31571)]); + let services = collect_lb_services(&svc); + assert_eq!(services.len(), 1); + assert_eq!(services[0].listen_port, 80); + assert_eq!(services[0].target_port, 31571); } #[test] @@ -527,4 +588,25 @@ mod tests { }); assert!(collect_lb_services(&svc).is_empty()); } + + #[test] + fn plain_node_is_eligible() { + assert!(is_lb_eligible_node(&node( + &[("kubernetes.io/hostname", "ws1")], + false + ))); + } + + #[test] + fn excluded_node_is_not_eligible() { + assert!(!is_lb_eligible_node(&node( + &[(consts::EXCLUDE_FROM_LB_LABEL_NAME, "")], + false + ))); + } + + #[test] + fn cordoned_node_is_not_eligible() { + assert!(!is_lb_eligible_node(&node(&[], true))); + } } From 94b31f50edcbf0820506e83fb710025489e47182 Mon Sep 17 00:00:00 2001 From: Kirill Ilin Date: Tue, 15 Sep 2026 12:29:43 +0500 Subject: [PATCH 05/10] fix(targets): fail reconciliation when no target can be added Tolerating a rejected target also hid the case where every target is rejected: reconciliation reported success and the service was patched with an external IP for a load balancer that had nowhere to forward to. Fail instead when no target could be added at all, so the controller requeues and the service stays unready. Warn as well when more nodes are selected than the balancer type holds, using the limit the API reports for that type. Assisted-By: LLM Signed-off-by: Kirill Ilin --- src/lb.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/lb.rs b/src/lb.rs index c868ab5..01d6608 100644 --- a/src/lb.rs +++ b/src/lb.rs @@ -320,6 +320,19 @@ impl LoadBalancer { } } + let max_targets = hcloud_balancer.load_balancer_type.max_targets; + if i64::try_from(self.targets.len()).unwrap_or(i64::MAX) > max_targets { + tracing::warn!( + "Selected {} target(s), but a {} balancer holds at most {}. \ + Use a bigger balancer type or externalTrafficPolicy: Local.", + self.targets.len(), + hcloud_balancer.load_balancer_type.name, + max_targets, + ); + } + + let mut attempted = 0_usize; + let mut failed = 0_usize; for ip in &self.targets { if !hcloud_balancer .targets @@ -340,13 +353,23 @@ impl LoadBalancer { }, ) .await; + attempted += 1; // Hetzner rejects IPs outside the vSwitch subnet of the attached network, // which must not keep the remaining nodes out of the load balancer. if let Err(error) = added { - tracing::error!("Cannot add target {ip}: {error}"); + failed += 1; + tracing::warn!("Cannot add target {ip}: {error}"); } } } + // Losing every single target is a failure of the whole reconciliation, not a + // rejected node: the service must not be reported as ready in that case. + if attempted > 0 && attempted == failed { + return Err(RobotLBError::HCloudError(format!( + "None of the {attempted} target(s) could be added to load balancer {}", + self.name + ))); + } Ok(()) } From 978372b64dc9c269341e3dd26a1ec61b145ca36a Mon Sep 17 00:00:00 2001 From: Kirill Ilin Date: Tue, 15 Sep 2026 12:29:43 +0500 Subject: [PATCH 06/10] ci(pre-commit): run cargo test Assisted-By: LLM Signed-off-by: Kirill Ilin --- .pre-commit-config.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 69ed78b..3c57201 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -27,6 +27,17 @@ repos: - clippy - --all + - id: test + types: + - rust + name: cargo test + language: system + pass_filenames: false + entry: cargo + args: + - test + - --all + - id: check types: - rust From 23d33c08542cff215f7fff435c2d94b782f76fa1 Mon Sep 17 00:00:00 2001 From: Kirill Ilin Date: Tue, 15 Sep 2026 12:29:43 +0500 Subject: [PATCH 07/10] docs(readme): note the node exclusions, target limits and upgrade impact Assisted-By: LLM Signed-off-by: Kirill Ilin --- README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 1537801..39e0dca 100644 --- a/README.md +++ b/README.md @@ -37,12 +37,16 @@ The operator listens to the Kubernetes API for services of type `LoadBalancer` a Target nodes are selected according to the service's `externalTrafficPolicy`: -- `Cluster`, the Kubernetes default: every node of the cluster becomes a target, since kube-proxy forwards the traffic to a node that hosts a pod. +- `Cluster`, the Kubernetes default: every node of the cluster becomes a target, since kube-proxy forwards the traffic to a node that hosts a pod. Cordoned nodes and nodes labelled `node.kubernetes.io/exclude-from-external-load-balancers`, which kubeadm puts on control-plane nodes, are left out. - `Local`: only the nodes where the service's target pods run, found through the service selector, or through the service's `EndpointSlice` resources when it has no selector. Setting `ROBOTLB_DYNAMIC_NODE_SELECTOR` to `false` replaces both with the node selector from the `robotlb/node-selector` annotation. -Every port of the service needs an allocated `nodePort`. A Hetzner load balancer forwards traffic to the IP of a node, so a port is reachable only through its `nodePort`: ports without one are skipped, and `allocateLoadBalancerNodePorts: false` is not supported. +Under the `Cluster` policy the number of targets grows with the cluster, while a balancer type caps how many it holds: `lb11`, the default type, holds 25. Pick a bigger type through `ROBOTLB_DEFAULT_LB_TYPE` or the `robotlb/balancer-type` annotation on larger clusters. + +Every port of the service needs an allocated `nodePort`. A Hetzner load balancer forwards traffic to the IP of a node, so a port is reachable only through its `nodePort`: ports without one are skipped, and `allocateLoadBalancerNodePorts: false` is not supported. A service whose ports are all skipped is left without an external IP. + +> Earlier releases treated every service as if it had the `Local` policy. Services that leave `externalTrafficPolicy` unset therefore get the full node list on upgrade, which changes the targets of their existing balancers. ## Configuration From d1f2472f63a0cccc747ad5474a5dc5d1f65e761f Mon Sep 17 00:00:00 2001 From: Kirill Ilin Date: Tue, 15 Sep 2026 12:41:47 +0500 Subject: [PATCH 08/10] fix(targets): plan targets instead of failing on every rejected one Counting the attempts of a single run treated a permanently rejected node as a total failure: once the acceptable targets were in place, every following run retried only the rejected ones and reported failure forever. Count the targets that are actually live instead, and carry the reason of the last rejection into the error. Targets are now planned up front: deduplicated, sorted, and trimmed to what the balancer type holds, so a cluster larger than the limit stops retrying doomed calls and keeps a stable set of targets. The limit is not reported while a type change is still in flight, when the balancer answers with the type it is leaving. Assisted-By: LLM Signed-off-by: Kirill Ilin --- src/lb.rs | 140 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 99 insertions(+), 41 deletions(-) diff --git a/src/lb.rs b/src/lb.rs index 01d6608..8725314 100644 --- a/src/lb.rs +++ b/src/lb.rs @@ -300,11 +300,28 @@ impl LoadBalancer { &self, hcloud_balancer: &hcloud::models::LoadBalancer, ) -> RobotLBResult<()> { + let max_targets = + usize::try_from(hcloud_balancer.load_balancer_type.max_targets).unwrap_or(usize::MAX); + let planned = plan_targets(&self.targets, max_targets); + if planned.len() < self.targets.len() + // While a type change is in flight the balancer still reports the old type, + // whose limit says nothing about the type the service asked for. + && hcloud_balancer.load_balancer_type.name == self.balancer_type + { + tracing::warn!( + "Selected {} node(s), but a {} balancer holds at most {}. \ + Use a bigger balancer type or externalTrafficPolicy: Local.", + self.targets.len(), + hcloud_balancer.load_balancer_type.name, + max_targets, + ); + } + for target in &hcloud_balancer.targets { let Some(target_ip) = target.ip.clone() else { continue; }; - if !self.targets.contains(&target_ip.ip) { + if !planned.contains(&target_ip.ip.as_str()) { tracing::info!("Removing target {}", target_ip.ip); hcloud::apis::load_balancers_api::remove_target( &self.hcloud_config, @@ -320,54 +337,49 @@ impl LoadBalancer { } } - let max_targets = hcloud_balancer.load_balancer_type.max_targets; - if i64::try_from(self.targets.len()).unwrap_or(i64::MAX) > max_targets { - tracing::warn!( - "Selected {} target(s), but a {} balancer holds at most {}. \ - Use a bigger balancer type or externalTrafficPolicy: Local.", - self.targets.len(), - hcloud_balancer.load_balancer_type.name, - max_targets, - ); - } - - let mut attempted = 0_usize; - let mut failed = 0_usize; - for ip in &self.targets { - if !hcloud_balancer + let mut live = 0_usize; + let mut last_error = None; + for ip in &planned { + if hcloud_balancer .targets .iter() - .any(|t| t.ip.as_ref().map(|i| i.ip.as_str()) == Some(ip)) + .any(|t| t.ip.as_ref().map(|i| i.ip.as_str()) == Some(*ip)) { - tracing::info!("Adding target {}", ip); - let added = hcloud::apis::load_balancers_api::add_target( - &self.hcloud_config, - AddTargetParams { - id: hcloud_balancer.id, - body: Some(LoadBalancerAddTarget { - ip: Some(Box::new(hcloud::models::LoadBalancerTargetIp { - ip: ip.clone(), - })), - ..Default::default() - }), - }, - ) - .await; - attempted += 1; - // Hetzner rejects IPs outside the vSwitch subnet of the attached network, - // which must not keep the remaining nodes out of the load balancer. - if let Err(error) = added { - failed += 1; + live += 1; + continue; + } + tracing::info!("Adding target {}", ip); + let added = hcloud::apis::load_balancers_api::add_target( + &self.hcloud_config, + AddTargetParams { + id: hcloud_balancer.id, + body: Some(LoadBalancerAddTarget { + ip: Some(Box::new(hcloud::models::LoadBalancerTargetIp { + ip: (*ip).to_string(), + })), + ..Default::default() + }), + }, + ) + .await; + // Hetzner rejects IPs outside the vSwitch subnet of the attached network, + // which must not keep the remaining nodes out of the load balancer. + match added { + Ok(_) => live += 1, + Err(error) => { tracing::warn!("Cannot add target {ip}: {error}"); + last_error = Some(error.to_string()); } } } - // Losing every single target is a failure of the whole reconciliation, not a - // rejected node: the service must not be reported as ready in that case. - if attempted > 0 && attempted == failed { + // A balancer left without a single target forwards nothing, so the service + // must not be reported as ready. Counting what is live rather than what this + // run attempted keeps a permanently rejected node from failing every run. + if !planned.is_empty() && live == 0 { return Err(RobotLBError::HCloudError(format!( - "None of the {attempted} target(s) could be added to load balancer {}", - self.name + "No target could be added to load balancer {}: {}", + self.name, + last_error.unwrap_or_else(|| "no reason reported".to_string()), ))); } Ok(()) @@ -659,6 +671,17 @@ impl LoadBalancer { } } +/// The targets a balancer should end up with: deduplicated, and trimmed to what the +/// balancer type holds. Sorted, so that a cluster larger than the limit keeps the same +/// targets from one reconciliation to the next instead of trading them back and forth. +fn plan_targets(desired: &[String], max_targets: usize) -> Vec<&str> { + let mut planned = desired.iter().map(String::as_str).collect::>(); + planned.sort_unstable(); + planned.dedup(); + planned.truncate(max_targets); + planned +} + impl FromStr for LBAlgorithm { type Err = RobotLBError; fn from_str(s: &str) -> Result { @@ -681,3 +704,38 @@ impl From for LoadBalancerAlgorithm { Self { r#type } } } + +#[cfg(test)] +mod tests { + use super::plan_targets; + + #[test] + fn targets_are_sorted_and_deduplicated() { + let desired = vec![ + "192.168.100.4".to_string(), + "192.168.100.2".to_string(), + "192.168.100.4".to_string(), + ]; + assert_eq!( + plan_targets(&desired, 25), + vec!["192.168.100.2", "192.168.100.4"] + ); + } + + #[test] + fn targets_beyond_the_balancer_limit_are_dropped() { + let desired = (1..=30) + .map(|host| format!("192.168.100.{host:03}")) + .collect::>(); + let planned = plan_targets(&desired, 25); + assert_eq!(planned.len(), 25); + assert_eq!(planned[0], "192.168.100.001"); + assert_eq!(planned[24], "192.168.100.025"); + } + + #[test] + fn a_plan_within_the_limit_keeps_every_target() { + let desired = vec!["192.168.100.2".to_string(), "192.168.100.3".to_string()]; + assert_eq!(plan_targets(&desired, 25).len(), 2); + } +} From 4df46e4046727cf3d295ce20348fc647fb14d3c9 Mon Sep 17 00:00:00 2001 From: Kirill Ilin Date: Tue, 15 Sep 2026 12:41:54 +0500 Subject: [PATCH 09/10] fix(service): stop billing for a balancer that forwards nothing A service without a single exposable port still got a Hetzner load balancer created for it, charged by the hour, with no service behind it and nothing but a log line to say so. Skip the balancer entirely in that case, and drop the external IP the service advertises, so that clients and ExternalDNS stop pointing at a balancer that no longer forwards. Node selection moves behind a NodeSource decision that can be tested on its own, not-ready nodes no longer take up target slots, and the exclusion label now holds under every traffic policy, the way upstream cloud providers treat it. Assisted-By: LLM Signed-off-by: Kirill Ilin --- src/main.rs | 178 ++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 151 insertions(+), 27 deletions(-) diff --git a/src/main.rs b/src/main.rs index fb1576a..164cc6f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -220,7 +220,7 @@ async fn get_nodes_dynamically( .list(&ListParams::default()) .await? .into_iter() - .filter(|node| target_nodes.contains(&node.name_any())) + .filter(|node| target_nodes.contains(&node.name_any()) && !is_excluded_from_lb(node)) .collect::>(); Ok(nodes) @@ -268,7 +268,7 @@ async fn get_nodes_from_endpointslices( .list(&ListParams::default()) .await? .into_iter() - .filter(|node| target_nodes.contains(&node.name_any())) + .filter(|node| target_nodes.contains(&node.name_any()) && !is_excluded_from_lb(node)) .collect::>(); Ok(nodes) @@ -292,7 +292,7 @@ async fn get_nodes_by_selector( .list(&ListParams::default()) .await? .into_iter() - .filter(|node| label_filter.check(node.labels())) + .filter(|node| label_filter.check(node.labels()) && !is_excluded_from_lb(node)) .collect::>(); Ok(nodes) } @@ -309,14 +309,18 @@ async fn get_all_nodes(context: &Arc) -> RobotLBResult Ok(nodes) } -/// Whether a node may be used as a load balancer target. -/// Cordoned nodes are draining, and the exclusion label is how a cluster declares -/// that a node must stay out of external load balancers. -fn is_lb_eligible_node(node: &Node) -> bool { - if node - .labels() +/// Whether the cluster declares that a node must stay out of external load balancers. +/// The label holds under every traffic policy, the way upstream cloud providers treat it. +fn is_excluded_from_lb(node: &Node) -> bool { + node.labels() .contains_key(consts::EXCLUDE_FROM_LB_LABEL_NAME) - { +} + +/// Whether a node may be used as a load balancer target under the `Cluster` policy, +/// where any node can carry the traffic and a draining or unhealthy one only takes +/// up a target slot. +fn is_lb_eligible_node(node: &Node) -> bool { + if is_excluded_from_lb(node) { tracing::debug!("Node {} is excluded from load balancers", node.name_any()); return false; } @@ -324,9 +328,40 @@ fn is_lb_eligible_node(node: &Node) -> bool { tracing::debug!("Node {} is unschedulable", node.name_any()); return false; } + let ready = node + .status + .as_ref() + .and_then(|status| status.conditions.as_ref()) + .and_then(|conditions| conditions.iter().find(|cond| cond.type_ == "Ready")) + .map(|cond| cond.status.as_str()); + if matches!(ready, Some(status) if status != "True") { + tracing::debug!("Node {} is not ready", node.name_any()); + return false; + } true } +/// Where the target nodes of a service come from. +#[derive(Debug, PartialEq, Eq)] +enum NodeSource { + /// The `robotlb/node-selector` annotation of the service. + Annotation, + /// The nodes hosting the endpoints of the service. + ServiceEndpoints, + /// Every node that may serve load balancer traffic. + AllNodes, +} + +fn node_source(svc: &Service, dynamic_node_selector: bool) -> NodeSource { + if !dynamic_node_selector { + return NodeSource::Annotation; + } + if is_local_traffic_policy(svc) { + return NodeSource::ServiceEndpoints; + } + NodeSource::AllNodes +} + /// Whether the service asks for traffic to reach only the nodes that host its endpoints. /// Under the default `Cluster` policy every node is a valid target, because kube-proxy /// forwards the traffic to a node that actually hosts a pod. @@ -377,14 +412,10 @@ pub async fn reconcile_load_balancer( node_ip_type = "ExternalIP"; } - let nodes = if context.config.dynamic_node_selector { - if is_local_traffic_policy(&svc) { - get_nodes_dynamically(&svc, &context).await? - } else { - get_all_nodes(&context).await? - } - } else { - get_nodes_by_selector(&svc, &context).await? + let nodes = match node_source(&svc, context.config.dynamic_node_selector) { + NodeSource::Annotation => get_nodes_by_selector(&svc, &context).await?, + NodeSource::ServiceEndpoints => get_nodes_dynamically(&svc, &context).await?, + NodeSource::AllNodes => get_all_nodes(&context).await?, }; for node in nodes { @@ -405,12 +436,6 @@ pub async fn reconcile_load_balancer( lb.add_service(service.listen_port, service.target_port); } - if lb.services.is_empty() { - tracing::warn!( - "Service has no port that can be exposed. Leaving it without an external IP." - ); - } - let svc_api = kube::Api::::namespaced( context.client.clone(), svc.namespace() @@ -418,6 +443,14 @@ pub async fn reconcile_load_balancer( .as_str(), ); + // A balancer without a single service forwards nothing while still being billed, + // so none is created until the service has a port that can be exposed. + if lb.services.is_empty() { + tracing::warn!("Service has no port that can be exposed. Skipping the load balancer."); + clear_ingress_status(&svc_api, &svc).await?; + return Ok(Action::requeue(Duration::from_secs(30))); + } + let hcloud_lb = lb.reconcile().await?; let mut ingress = vec![]; @@ -443,7 +476,7 @@ pub async fn reconcile_load_balancer( } } - if !ingress.is_empty() && !lb.services.is_empty() { + if !ingress.is_empty() { svc_api .patch_status( svc.name_any().as_str(), @@ -462,6 +495,35 @@ pub async fn reconcile_load_balancer( Ok(Action::requeue(Duration::from_secs(30))) } +/// Drop the external IP a service advertises, so that nothing keeps sending +/// traffic to a load balancer that no longer forwards it. +async fn clear_ingress_status(svc_api: &kube::Api, svc: &Service) -> RobotLBResult<()> { + let advertises_ingress = svc + .status + .as_ref() + .and_then(|status| status.load_balancer.as_ref()) + .and_then(|lb| lb.ingress.as_ref()) + .is_some_and(|ingress| !ingress.is_empty()); + if !advertises_ingress { + return Ok(()); + } + tracing::info!("Removing the external IP from the service status"); + svc_api + .patch_status( + svc.name_any().as_str(), + &PatchParams::default(), + &kube::api::Patch::Merge(json!({ + "status": { + "loadBalancer": { + "ingress": null + } + } + })), + ) + .await?; + Ok(()) +} + /// Handle the error during reconcilation. #[allow(clippy::needless_pass_by_value)] fn on_error(_: Arc, error: &RobotLBError, _context: Arc) -> Action { @@ -473,9 +535,14 @@ fn on_error(_: Arc, error: &RobotLBError, _context: Arc #[cfg(test)] mod tests { - use super::{collect_lb_services, consts, is_lb_eligible_node, is_local_traffic_policy}; + use super::{ + collect_lb_services, consts, is_excluded_from_lb, is_lb_eligible_node, + is_local_traffic_policy, node_source, NodeSource, + }; use k8s_openapi::{ - api::core::v1::{Node, NodeSpec, Service, ServicePort, ServiceSpec}, + api::core::v1::{ + Node, NodeCondition, NodeSpec, NodeStatus, Service, ServicePort, ServiceSpec, + }, apimachinery::pkg::apis::meta::v1::ObjectMeta, }; use std::collections::BTreeMap; @@ -506,6 +573,20 @@ mod tests { } } + fn node_with_ready_condition(status: &str) -> Node { + Node { + status: Some(NodeStatus { + conditions: Some(vec![NodeCondition { + type_: "Ready".to_string(), + status: status.to_string(), + ..Default::default() + }]), + ..Default::default() + }), + ..node(&[], false) + } + } + #[test] fn cluster_policy_is_not_local() { let svc = service(ServiceSpec { @@ -609,4 +690,47 @@ mod tests { fn cordoned_node_is_not_eligible() { assert!(!is_lb_eligible_node(&node(&[], true))); } + + #[test] + fn ready_node_is_eligible() { + assert!(is_lb_eligible_node(&node_with_ready_condition("True"))); + } + + #[test] + fn not_ready_node_is_not_eligible() { + assert!(!is_lb_eligible_node(&node_with_ready_condition("False"))); + } + + #[test] + fn exclusion_label_is_recognised_on_its_own() { + assert!(is_excluded_from_lb(&node( + &[(consts::EXCLUDE_FROM_LB_LABEL_NAME, "")], + false + ))); + assert!(!is_excluded_from_lb(&node(&[], true))); + } + + #[test] + fn cluster_policy_takes_every_node() { + let svc = service(ServiceSpec::default()); + assert_eq!(node_source(&svc, true), NodeSource::AllNodes); + } + + #[test] + fn local_policy_takes_endpoint_nodes() { + let svc = service(ServiceSpec { + external_traffic_policy: Some("Local".into()), + ..Default::default() + }); + assert_eq!(node_source(&svc, true), NodeSource::ServiceEndpoints); + } + + #[test] + fn static_selector_wins_over_the_policy() { + let svc = service(ServiceSpec { + external_traffic_policy: Some("Local".into()), + ..Default::default() + }); + assert_eq!(node_source(&svc, false), NodeSource::Annotation); + } } From c37b2b615ad4e3010fa292a26aec0b787d93917a Mon Sep 17 00:00:00 2001 From: Kirill Ilin Date: Tue, 15 Sep 2026 12:41:55 +0500 Subject: [PATCH 10/10] docs(readme): describe target trimming and services left without a balancer Assisted-By: LLM Signed-off-by: Kirill Ilin --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 39e0dca..fb3afc7 100644 --- a/README.md +++ b/README.md @@ -37,14 +37,16 @@ The operator listens to the Kubernetes API for services of type `LoadBalancer` a Target nodes are selected according to the service's `externalTrafficPolicy`: -- `Cluster`, the Kubernetes default: every node of the cluster becomes a target, since kube-proxy forwards the traffic to a node that hosts a pod. Cordoned nodes and nodes labelled `node.kubernetes.io/exclude-from-external-load-balancers`, which kubeadm puts on control-plane nodes, are left out. +- `Cluster`, the Kubernetes default: every node of the cluster becomes a target, since kube-proxy forwards the traffic to a node that hosts a pod. Cordoned and not-ready nodes are left out, as they would only take up target slots. - `Local`: only the nodes where the service's target pods run, found through the service selector, or through the service's `EndpointSlice` resources when it has no selector. +Nodes labelled `node.kubernetes.io/exclude-from-external-load-balancers`, which kubeadm puts on control-plane nodes, stay out of every balancer under either policy. + Setting `ROBOTLB_DYNAMIC_NODE_SELECTOR` to `false` replaces both with the node selector from the `robotlb/node-selector` annotation. -Under the `Cluster` policy the number of targets grows with the cluster, while a balancer type caps how many it holds: `lb11`, the default type, holds 25. Pick a bigger type through `ROBOTLB_DEFAULT_LB_TYPE` or the `robotlb/balancer-type` annotation on larger clusters. +A balancer type caps how many targets it holds: `lb11`, the default type, holds 25. When more nodes are selected than the type holds, the extra ones are dropped in a stable order and a warning names the limit. Pick a bigger type through `ROBOTLB_DEFAULT_LB_TYPE` or the `robotlb/balancer-type` annotation to use the whole cluster. -Every port of the service needs an allocated `nodePort`. A Hetzner load balancer forwards traffic to the IP of a node, so a port is reachable only through its `nodePort`: ports without one are skipped, and `allocateLoadBalancerNodePorts: false` is not supported. A service whose ports are all skipped is left without an external IP. +Every port of the service needs an allocated `nodePort`. A Hetzner load balancer forwards traffic to the IP of a node, so a port is reachable only through its `nodePort`: ports without one are skipped, and `allocateLoadBalancerNodePorts: false` is not supported. When no port of a service can be exposed, no balancer is created for it, and a service that already advertises an external IP loses it. > Earlier releases treated every service as if it had the `Local` policy. Services that leave `externalTrafficPolicy` unset therefore get the full node list on upgrade, which changes the targets of their existing balancers.