From 0b8d7948079d84f4c035590c56fd38ffdd850f53 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Tue, 29 Sep 2026 00:50:26 +0300 Subject: [PATCH 1/7] fix(hcloud): back off on rate limits and report failures on the service Once the Hetzner Cloud API budget ran out, every service was retried every 30 seconds, and each retry spent the budget being waited for. The budget is per project, so after a 429 all services now pause together, starting at one minute and doubling up to 16 minutes while the limit keeps being hit. Adding targets stops at the first 429 instead of calling the API for every remaining node. The generated client drops response headers, so RateLimit-Reset cannot be used. A failed reconciliation now leaves a SyncLoadBalancerFailed warning event on the service, so kubectl describe shows the last error. Errors are logged on one line with the status and the message Hetzner reported, and the API token, which Hetzner quotes in some messages, is redacted from logs and events. The chart role gains permission to create events. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin --- helm/values.yaml | 3 + src/error.rs | 186 +++++++++++++++++++++++++++++++++++++--- src/lb.rs | 17 ++-- src/main.rs | 213 ++++++++++++++++++++++++++++++++++++++++------ src/rate_limit.rs | 141 ++++++++++++++++++++++++++++++ 5 files changed, 512 insertions(+), 48 deletions(-) create mode 100644 src/rate_limit.rs diff --git a/helm/values.yaml b/helm/values.yaml index 2524ad9..85c559a 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -39,6 +39,9 @@ serviceAccount: - apiGroups: [discovery.k8s.io] resources: [endpointslices] verbs: [get, list, watch] + - apiGroups: [events.k8s.io] + resources: [events] + verbs: [create] podAnnotations: {} podLabels: {} diff --git a/src/error.rs b/src/error.rs index dae2e3f..d36c3ec 100644 --- a/src/error.rs +++ b/src/error.rs @@ -22,65 +22,223 @@ pub enum RobotLBError { UnknownLBAlgorithm, #[error("Cannot get target nodes, because the service has no selector")] ServiceWithoutSelector, + #[error("Hetzner Cloud API rate limit reached, next attempt in {}s", .0.as_secs())] + RateLimited(std::time::Duration), // HCloud API errors - #[error("Cannot attach load balancer to a network. Reason: {0}")] + #[error("Cannot attach load balancer to a network. Reason: {}", describe(.0))] HCloudLBAttachToNetworkError( #[from] hcloud::apis::Error, ), - #[error("Cannot detach load balancer from network. Reason: {0}")] + #[error("Cannot detach load balancer from network. Reason: {}", describe(.0))] HcloudLBDetachFromNetworkError( #[from] hcloud::apis::Error, ), - #[error("Cannot add load balancer target. Reason: {0}")] + #[error("Cannot add load balancer target. Reason: {}", describe(.0))] HcloudLBAddTargetError( #[from] hcloud::apis::Error, ), - #[error("Cannot remove load balancer target. Reason: {0}")] + #[error("Cannot remove load balancer target. Reason: {}", describe(.0))] HcloudLBRemoveTargetError( #[from] hcloud::apis::Error, ), - #[error("Cannot add service to load balancer. Reason: {0}")] + #[error("Cannot add service to load balancer. Reason: {}", describe(.0))] HcloudLBAddServiceError( #[from] hcloud::apis::Error, ), - #[error("Cannot remove service from load balancer. Reason: {0}")] + #[error("Cannot remove service from load balancer. Reason: {}", describe(.0))] HcloudLBRemoveServiceError( #[from] hcloud::apis::Error, ), - #[error("Cannot create load balancer. Reason: {0}")] + #[error("Cannot create load balancer. Reason: {}", describe(.0))] HcloudLBCreateError( #[from] hcloud::apis::Error, ), - #[error("Cannot delete load balancer. Reason: {0}")] + #[error("Cannot delete load balancer. Reason: {}", describe(.0))] HcloudLBDeleteError( #[from] hcloud::apis::Error, ), - #[error("Cannot get load balancer. Reason: {0}")] + #[error("Cannot get load balancer. Reason: {}", describe(.0))] HcloudLBGetError( #[from] hcloud::apis::Error, ), - #[error("Cannot update service. Reason: {0}")] + #[error("Cannot update service. Reason: {}", describe(.0))] HcloudLBUpdateServiceError( #[from] hcloud::apis::Error, ), - #[error("Cannot change type of load balancer. Reason: {0}")] + #[error("Cannot change type of load balancer. Reason: {}", describe(.0))] HcloudLBChangeType( #[from] hcloud::apis::Error, ), - #[error("Cannot change algorithm of load balancer. Reason: {0}")] + #[error("Cannot change algorithm of load balancer. Reason: {}", describe(.0))] HcloudLBChangeAlgorithm( #[from] hcloud::apis::Error, ), - #[error("Cannot list networks. Reason: {0}")] + #[error("Cannot list networks. Reason: {}", describe(.0))] HcloudListNetworksError( #[from] hcloud::apis::Error, ), - #[error("Cannot list load balancers. Reason: {0}")] + #[error("Cannot list load balancers. Reason: {}", describe(.0))] HcloudListLoadBalancersError( #[from] hcloud::apis::Error, ), } + +impl RobotLBError { + /// Whether Hetzner rejected the call because the project ran out of API requests. + #[must_use] + pub fn is_rate_limited(&self) -> bool { + // No wildcard arm: a new variant must be sorted into one of the two groups. + match self { + Self::HCloudLBAttachToNetworkError(error) => is_rate_limit_response(error), + Self::HcloudLBDetachFromNetworkError(error) => is_rate_limit_response(error), + Self::HcloudLBAddTargetError(error) => is_rate_limit_response(error), + Self::HcloudLBRemoveTargetError(error) => is_rate_limit_response(error), + Self::HcloudLBAddServiceError(error) => is_rate_limit_response(error), + Self::HcloudLBRemoveServiceError(error) => is_rate_limit_response(error), + Self::HcloudLBCreateError(error) => is_rate_limit_response(error), + Self::HcloudLBDeleteError(error) => is_rate_limit_response(error), + Self::HcloudLBGetError(error) => is_rate_limit_response(error), + Self::HcloudLBUpdateServiceError(error) => is_rate_limit_response(error), + Self::HcloudLBChangeType(error) => is_rate_limit_response(error), + Self::HcloudLBChangeAlgorithm(error) => is_rate_limit_response(error), + Self::HcloudListNetworksError(error) => is_rate_limit_response(error), + Self::HcloudListLoadBalancersError(error) => is_rate_limit_response(error), + Self::InvalidNodeFilter(_) + | Self::UnsupportedServiceType + | Self::SkipService + | Self::PaseIntError(_) + | Self::PaseBoolError(_) + | Self::HCloudError(_) + | Self::KubeError(_) + | Self::UnknownLBAlgorithm + | Self::ServiceWithoutSelector + | Self::RateLimited(_) => false, + } + } +} + +/// Whether Hetzner answered 429 because the project ran out of API requests. +#[must_use] +pub fn is_rate_limit_response(error: &hcloud::apis::Error) -> bool { + matches!(error, hcloud::apis::Error::ResponseError(response) if response.status.as_u16() == 429) +} + +/// One line with the HTTP status and the error Hetzner reported, if the body carries one. +#[must_use] +pub fn describe(error: &hcloud::apis::Error) -> String { + let hcloud::apis::Error::ResponseError(response) = error else { + return error.to_string(); + }; + let body = + k8s_openapi::serde_json::from_str::(&response.content).ok(); + let reported = body.as_ref().and_then(|body| { + let error = body.get("error")?; + Some(format!( + "{}: {}", + error.get("code")?.as_str()?, + error.get("message")?.as_str()? + )) + }); + reported.map_or_else( + || response.status.to_string(), + |reported| { + let one_line = reported.split_whitespace().collect::>().join(" "); + format!("{}: {one_line}", response.status) + }, + ) +} + +/// Replace the API token, or any prefix of it long enough to identify it, with a marker. +/// Hetzner quotes the start of the token in some error messages. +#[must_use] +pub fn redact(message: &str, token: &str) -> String { + const SHORTEST_PREFIX: usize = 8; + let mut redacted = message.to_string(); + for len in (SHORTEST_PREFIX..=token.len()).rev() { + if let Some(prefix) = token.get(..len) { + redacted = redacted.replace(prefix, "[REDACTED]"); + } + } + redacted +} + +#[cfg(test)] +mod tests { + use super::{describe, is_rate_limit_response, redact, RobotLBError}; + use hcloud::apis::{load_balancers_api::ListLoadBalancersError, Error, ResponseContent}; + + const TOKEN: &str = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ01"; + + fn response_error(status: u16, content: &str) -> Error { + Error::ResponseError(ResponseContent { + status: status.try_into().unwrap(), + content: content.to_string(), + entity: None, + }) + } + + #[test] + fn a_token_prefix_is_redacted() { + let message = format!("limit reached for token {}", &TOKEN[..32]); + assert_eq!( + redact(&message, TOKEN), + "limit reached for token [REDACTED]" + ); + } + + #[test] + fn the_whole_token_is_redacted() { + assert_eq!(redact(&format!("x{TOKEN}y"), TOKEN), "x[REDACTED]y"); + } + + #[test] + fn a_message_without_the_token_is_kept() { + assert_eq!(redact("status 500", TOKEN), "status 500"); + } + + #[test] + fn an_empty_token_redacts_nothing() { + assert_eq!(redact("status 500", ""), "status 500"); + } + + #[test] + fn the_hetzner_message_is_described_on_one_line() { + let error = response_error( + 429, + r#"{"error": {"code": "rate_limit_exceeded", "message": "limit\n reached"}}"#, + ); + assert_eq!( + describe(&error), + "429 Too Many Requests: rate_limit_exceeded: limit reached" + ); + } + + #[test] + fn a_body_that_is_not_json_falls_back_to_the_status() { + let error = response_error(502, "bad gateway"); + assert_eq!(describe(&error), "502 Bad Gateway"); + } + + #[test] + fn a_429_is_a_rate_limit() { + let error = RobotLBError::from(response_error(429, "")); + assert!(error.is_rate_limited()); + } + + #[test] + fn only_a_429_response_is_a_rate_limit() { + assert!(is_rate_limit_response(&response_error(429, ""))); + // Hetzner answers 422 for a target outside the vSwitch subnet. + assert!(!is_rate_limit_response(&response_error(422, ""))); + } + + #[test] + fn other_statuses_are_not_a_rate_limit() { + assert!(!RobotLBError::from(response_error(500, "")).is_rate_limited()); + assert!(!RobotLBError::SkipService.is_rate_limited()); + } +} diff --git a/src/lb.rs b/src/lb.rs index 8725314..e1cc5f3 100644 --- a/src/lb.rs +++ b/src/lb.rs @@ -366,9 +366,13 @@ impl LoadBalancer { // which must not keep the remaining nodes out of the load balancer. match added { Ok(_) => live += 1, + // Every further call would be rejected too and only drain the budget. + Err(error) if crate::error::is_rate_limit_response(&error) => { + return Err(error.into()); + } Err(error) => { tracing::warn!("Cannot add target {ip}: {error}"); - last_error = Some(error.to_string()); + last_error = Some(crate::error::describe(&error)); } } } @@ -619,16 +623,9 @@ impl LoadBalancer { }), }, ) - .await; - if let Err(e) = response { - tracing::error!("Failed to create load balancer: {:?}", e); - return Err(RobotLBError::HCloudError(format!( - "Failed to create load balancer: {:?}", - e - ))); - } + .await?; - Ok(*response.unwrap().load_balancer) + Ok(*response.load_balancer) } /// Get the network from Hetzner Cloud. diff --git a/src/main.rs b/src/main.rs index 164cc6f..0f6e964 100644 --- a/src/main.rs +++ b/src/main.rs @@ -18,7 +18,7 @@ use clap::Parser; use config::OperatorConfig; -use error::{RobotLBError, RobotLBResult}; +use error::{redact, RobotLBError, RobotLBResult}; use futures::StreamExt; use hcloud::apis::configuration::Configuration as HCloudConfig; use k8s_openapi::{ @@ -30,12 +30,22 @@ use k8s_openapi::{ }; use kube::{ api::{ListParams, PatchParams}, - runtime::{controller::Action, watcher, Controller}, + runtime::{ + controller::{self, Action}, + events::{Event, EventType, Recorder, Reporter}, + watcher, Controller, + }, Resource, ResourceExt, }; use label_filter::LabelFilter; use lb::{LBService, LoadBalancer}; -use std::{collections::HashSet, str::FromStr, sync::Arc, time::Duration}; +use rate_limit::RateLimitGate; +use std::{ + collections::HashSet, + str::FromStr, + sync::Arc, + time::{Duration, Instant}, +}; pub mod config; pub mod consts; @@ -43,6 +53,7 @@ pub mod error; pub mod finalizers; pub mod label_filter; pub mod lb; +pub mod rate_limit; #[cfg(not(target_env = "msvc"))] #[global_allocator] @@ -69,27 +80,38 @@ async fn main() -> RobotLBResult<()> { hcloud_conf, )); tracing::info!("Starting the controller"); + let token = operator_config.hcloud_token; Controller::new( kube::Api::::all(kube_client), watcher::Config::default(), ) .run(reconcile_service, on_error, context) - .for_each(|reconcilation_result| async move { - match reconcilation_result { - Ok((service, _action)) => { - tracing::info!("Reconcilation of a service {} was successful", service.name); - } - Err(err) => match err { + .for_each(|reconcilation_result| { + let token = token.clone(); + async move { + match reconcilation_result { + Ok((service, _action)) => { + tracing::info!("Reconcilation of a service {} was successful", service.name); + } // During reconcilation process, // the controller has decided to skip the service. - kube::runtime::controller::Error::ReconcilerFailed( - RobotLBError::SkipService, - _, - ) => {} - _ => { - tracing::error!("Error reconciling service: {:#?}", err); + Err(controller::Error::ReconcilerFailed(RobotLBError::SkipService, _)) => {} + Err(controller::Error::ReconcilerFailed( + error @ RobotLBError::RateLimited(_), + service, + )) => { + tracing::debug!("Service {service}: {error}"); } - }, + Err(controller::Error::ReconcilerFailed(error, service)) => { + tracing::error!( + "Reconcilation of service {service} failed: {}", + redact(&error.to_string(), &token) + ); + } + Err(error) => { + tracing::error!("Controller error: {}", redact(&error.to_string(), &token)); + } + } } }) .await; @@ -101,18 +123,16 @@ pub struct CurrentContext { pub client: kube::Client, pub config: OperatorConfig, pub hcloud_config: HCloudConfig, + pub rate_limit: Arc, } impl CurrentContext { #[must_use] - pub const fn new( - client: kube::Client, - config: OperatorConfig, - hcloud_config: HCloudConfig, - ) -> Self { + pub fn new(client: kube::Client, config: OperatorConfig, hcloud_config: HCloudConfig) -> Self { Self { client, config, hcloud_config, + rate_limit: Arc::default(), } } } @@ -126,6 +146,63 @@ pub async fn reconcile_service( svc: Arc, context: Arc, ) -> RobotLBResult { + let result = sync_service(svc.clone(), context.clone()).await; + if let Err(error) = &result { + if publishes_event(error) { + report_failure(&svc, &context, error).await; + } + } + result +} + +/// Skipped services are every service robotlb does not own, and a gated one has not +/// called the API at all, so neither has a failure to show. +const fn publishes_event(error: &RobotLBError) -> bool { + !matches!( + error, + RobotLBError::SkipService | RobotLBError::RateLimited(_) + ) +} + +/// Put the error on the service as a warning event, where `kubectl describe` shows it. +async fn report_failure(svc: &Service, context: &CurrentContext, error: &RobotLBError) { + let recorder = Recorder::new( + context.client.clone(), + Reporter { + controller: "robotlb".to_string(), + instance: None, + }, + svc.object_ref(&()), + ); + let published = recorder + .publish(Event { + type_: EventType::Warning, + reason: "SyncLoadBalancerFailed".to_string(), + note: Some(event_note(error, &context.config.hcloud_token)), + action: "Reconcile".to_string(), + secondary: None, + }) + .await; + if let Err(publish_error) = published { + tracing::warn!("Cannot publish an event for the service: {publish_error}"); + } +} + +/// The error as an event note: token redacted and cut to the size the API accepts. +fn event_note(error: &RobotLBError, token: &str) -> String { + const MAX_NOTE_BYTES: usize = 1024; + let mut note = redact(&error.to_string(), token); + if note.len() > MAX_NOTE_BYTES { + let mut end = MAX_NOTE_BYTES; + while !note.is_char_boundary(end) { + end -= 1; + } + note.truncate(end); + } + note +} + +async fn sync_service(svc: Arc, context: Arc) -> RobotLBResult { let svc_type = svc .spec .as_ref() @@ -148,6 +225,12 @@ pub async fn reconcile_service( return Err(RobotLBError::SkipService); } + // Hetzner counts requests per project, so while one service is rate limited + // every other one waits too instead of spending the budget being waited for. + if let Some(wait) = context.rate_limit.remaining(Instant::now()) { + return Err(RobotLBError::RateLimited(wait)); + } + tracing::info!("Starting service reconcilation"); let lb = LoadBalancer::try_from_svc(&svc, &context)?; @@ -526,9 +609,15 @@ async fn clear_ingress_status(svc_api: &kube::Api, svc: &Service) -> Ro /// Handle the error during reconcilation. #[allow(clippy::needless_pass_by_value)] -fn on_error(_: Arc, error: &RobotLBError, _context: Arc) -> Action { +fn on_error(_: Arc, error: &RobotLBError, context: Arc) -> Action { + error_action(error, &context.rate_limit, Instant::now()) +} + +fn error_action(error: &RobotLBError, rate_limit: &RateLimitGate, now: Instant) -> Action { match error { RobotLBError::SkipService => Action::await_change(), + RobotLBError::RateLimited(wait) => Action::requeue(*wait), + error if error.is_rate_limited() => Action::requeue(rate_limit.on_rate_limited(now)), _ => Action::requeue(Duration::from_secs(30)), } } @@ -536,8 +625,8 @@ fn on_error(_: Arc, error: &RobotLBError, _context: Arc #[cfg(test)] mod tests { use super::{ - collect_lb_services, consts, is_excluded_from_lb, is_lb_eligible_node, - is_local_traffic_policy, node_source, NodeSource, + collect_lb_services, consts, error_action, event_note, is_excluded_from_lb, + is_lb_eligible_node, is_local_traffic_policy, node_source, publishes_event, NodeSource, }; use k8s_openapi::{ api::core::v1::{ @@ -733,4 +822,80 @@ mod tests { }); assert_eq!(node_source(&svc, false), NodeSource::Annotation); } + + #[test] + fn an_event_note_is_redacted_and_bounded() { + let token = "0123456789abcdef"; + let error = crate::error::RobotLBError::HCloudError(format!( + "rejected token {} {}", + &token[..10], + "é".repeat(600) + )); + let note = event_note(&error, token); + assert!(!note.contains(&token[..10])); + assert!(note.contains("[REDACTED]")); + assert!(note.len() <= 1024); + } + + #[test] + fn a_gated_service_waits_out_the_pause() { + let gate = crate::rate_limit::RateLimitGate::default(); + let wait = std::time::Duration::from_secs(42); + let error = crate::error::RobotLBError::RateLimited(wait); + assert_eq!( + error_action(&error, &gate, std::time::Instant::now()), + kube::runtime::controller::Action::requeue(wait) + ); + } + + #[test] + fn a_rate_limited_call_closes_the_gate() { + let gate = crate::rate_limit::RateLimitGate::default(); + let now = std::time::Instant::now(); + let error = crate::error::RobotLBError::from(hcloud::apis::Error::< + hcloud::apis::load_balancers_api::AddTargetError, + >::ResponseError( + hcloud::apis::ResponseContent { + status: 429_u16.try_into().unwrap(), + content: String::new(), + entity: None, + }, + )); + assert_eq!( + error_action(&error, &gate, now), + kube::runtime::controller::Action::requeue(std::time::Duration::from_secs(60)) + ); + assert!(gate.remaining(now).is_some()); + } + + #[test] + fn other_errors_retry_in_30_seconds() { + let gate = crate::rate_limit::RateLimitGate::default(); + let error = crate::error::RobotLBError::HCloudError("boom".to_string()); + assert_eq!( + error_action(&error, &gate, std::time::Instant::now()), + kube::runtime::controller::Action::requeue(std::time::Duration::from_secs(30)) + ); + } + + #[test] + fn only_real_failures_publish_an_event() { + use crate::error::RobotLBError; + assert!(!publishes_event(&RobotLBError::SkipService)); + assert!(!publishes_event(&RobotLBError::RateLimited( + std::time::Duration::from_secs(1) + ))); + assert!(publishes_event(&RobotLBError::from(hcloud::apis::Error::< + hcloud::apis::load_balancers_api::ListLoadBalancersError, + >::ResponseError( + hcloud::apis::ResponseContent { + status: 429_u16.try_into().unwrap(), + content: String::new(), + entity: None, + } + )))); + assert!(publishes_event(&RobotLBError::HCloudError( + "boom".to_string() + ))); + } } diff --git a/src/rate_limit.rs b/src/rate_limit.rs new file mode 100644 index 0000000..bfa2e80 --- /dev/null +++ b/src/rate_limit.rs @@ -0,0 +1,141 @@ +use std::{ + sync::Mutex, + time::{Duration, Instant}, +}; + +const FIRST_DELAY: Duration = Duration::from_secs(60); +const MAX_DOUBLINGS: u32 = 4; + +/// Pause shared by every service. Hetzner counts API requests per project, so once one +/// reconciliation is rate limited, any other call would only spend the budget being waited for. +/// +/// The generated `hcloud` client drops response headers, so `RateLimit-Reset` is not +/// available and the pause grows exponentially instead. +#[derive(Debug, Default)] +pub struct RateLimitGate { + state: Mutex, +} + +#[derive(Debug, Default)] +struct State { + closed_until: Option, + last_delay: Duration, + consecutive: u32, +} + +impl State { + fn remaining(&self, now: Instant) -> Option { + self.closed_until + .and_then(|until| until.checked_duration_since(now)) + .filter(|remaining| !remaining.is_zero()) + } +} + +impl RateLimitGate { + /// How long calls to the API must still wait, if they must. + pub fn remaining(&self, now: Instant) -> Option { + let state = self + .state + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + state.remaining(now) + } + + /// Record a rate-limited call and return how long to wait before the next one. + /// A call rejected while the gate is already closed was sent before it closed, + /// so it keeps the current pause instead of lengthening it. The pause starts short + /// again only after the API went without a 429 for as long as the last pause: + /// a reconciliation that succeeds may not have called the API at all. + pub fn on_rate_limited(&self, now: Instant) -> Duration { + let mut state = self + .state + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if let Some(remaining) = state.remaining(now) { + return remaining; + } + if let Some(reopened) = state.closed_until { + if now >= reopened + state.last_delay { + state.consecutive = 0; + } + } + let delay = FIRST_DELAY * 2_u32.pow(state.consecutive.min(MAX_DOUBLINGS)); + state.consecutive += 1; + state.closed_until = Some(now + delay); + state.last_delay = delay; + delay + } +} + +#[cfg(test)] +mod tests { + use super::RateLimitGate; + use std::time::{Duration, Instant}; + + #[test] + fn an_untouched_gate_is_open() { + assert_eq!(RateLimitGate::default().remaining(Instant::now()), None); + } + + #[test] + fn a_rate_limit_closes_the_gate_for_the_returned_delay() { + let gate = RateLimitGate::default(); + let now = Instant::now(); + let delay = gate.on_rate_limited(now); + assert_eq!(delay, Duration::from_secs(60)); + assert_eq!(gate.remaining(now), Some(delay)); + assert_eq!(gate.remaining(now + delay), None); + } + + #[test] + fn repeated_rate_limits_double_the_delay_up_to_the_cap() { + let gate = RateLimitGate::default(); + let mut now = Instant::now(); + let mut delays = Vec::new(); + for _ in 0..8 { + let delay = gate.on_rate_limited(now); + delays.push(delay.as_secs()); + now += delay; + } + assert_eq!(delays, vec![60, 120, 240, 480, 960, 960, 960, 960]); + } + + #[test] + fn a_quiet_period_as_long_as_the_last_pause_resets_the_delay() { + let gate = RateLimitGate::default(); + let now = Instant::now(); + let first = gate.on_rate_limited(now); + let second = gate.on_rate_limited(now + first); + let reopened = now + first + second; + assert_eq!( + gate.on_rate_limited(reopened + second), + Duration::from_secs(60) + ); + } + + #[test] + fn a_shorter_quiet_period_keeps_doubling() { + let gate = RateLimitGate::default(); + let now = Instant::now(); + let first = gate.on_rate_limited(now); + let second = gate.on_rate_limited(now + first); + let reopened = now + first + second; + assert_eq!( + gate.on_rate_limited(reopened + second.checked_sub(Duration::from_secs(1)).unwrap()), + Duration::from_secs(240) + ); + } + + #[test] + fn calls_already_in_flight_do_not_lengthen_the_pause() { + let gate = RateLimitGate::default(); + let now = Instant::now(); + gate.on_rate_limited(now); + let later = now + Duration::from_secs(10); + assert_eq!(gate.on_rate_limited(later), Duration::from_secs(50)); + assert_eq!( + gate.on_rate_limited(later + Duration::from_secs(50)), + Duration::from_secs(120) + ); + } +} From 851e4a3d4706e5463e20bc8a6fa0932cf33ba7ff Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Tue, 29 Sep 2026 12:18:40 +0300 Subject: [PATCH 2/7] fix(hcloud): spread wakeups after a rate limit pause Services paused by the rate limit gate were all requeued for the moment it reopened, so they hit the API in one burst while the budget was still nearly empty. Each wait now grows by up to a quarter, derived from the service name, so services wake up spread over that window. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin --- src/main.rs | 33 ++++++++++++++++++++++----------- src/rate_limit.rs | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/src/main.rs b/src/main.rs index 0f6e964..6ec1ef9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -39,7 +39,7 @@ use kube::{ }; use label_filter::LabelFilter; use lb::{LBService, LoadBalancer}; -use rate_limit::RateLimitGate; +use rate_limit::{spread, RateLimitGate}; use std::{ collections::HashSet, str::FromStr, @@ -609,15 +609,23 @@ async fn clear_ingress_status(svc_api: &kube::Api, svc: &Service) -> Ro /// Handle the error during reconcilation. #[allow(clippy::needless_pass_by_value)] -fn on_error(_: Arc, error: &RobotLBError, context: Arc) -> Action { - error_action(error, &context.rate_limit, Instant::now()) +fn on_error(svc: Arc, error: &RobotLBError, context: Arc) -> Action { + let service = format!("{}/{}", svc.namespace().unwrap_or_default(), svc.name_any()); + error_action(error, &context.rate_limit, Instant::now(), &service) } -fn error_action(error: &RobotLBError, rate_limit: &RateLimitGate, now: Instant) -> Action { +fn error_action( + error: &RobotLBError, + rate_limit: &RateLimitGate, + now: Instant, + service: &str, +) -> Action { match error { RobotLBError::SkipService => Action::await_change(), - RobotLBError::RateLimited(wait) => Action::requeue(*wait), - error if error.is_rate_limited() => Action::requeue(rate_limit.on_rate_limited(now)), + RobotLBError::RateLimited(wait) => Action::requeue(spread(*wait, service)), + error if error.is_rate_limited() => { + Action::requeue(spread(rate_limit.on_rate_limited(now), service)) + } _ => Action::requeue(Duration::from_secs(30)), } } @@ -843,8 +851,8 @@ mod tests { let wait = std::time::Duration::from_secs(42); let error = crate::error::RobotLBError::RateLimited(wait); assert_eq!( - error_action(&error, &gate, std::time::Instant::now()), - kube::runtime::controller::Action::requeue(wait) + error_action(&error, &gate, std::time::Instant::now(), "shop/web"), + kube::runtime::controller::Action::requeue(crate::rate_limit::spread(wait, "shop/web")) ); } @@ -862,8 +870,11 @@ mod tests { }, )); assert_eq!( - error_action(&error, &gate, now), - kube::runtime::controller::Action::requeue(std::time::Duration::from_secs(60)) + error_action(&error, &gate, now, "shop/web"), + kube::runtime::controller::Action::requeue(crate::rate_limit::spread( + std::time::Duration::from_secs(60), + "shop/web" + )) ); assert!(gate.remaining(now).is_some()); } @@ -873,7 +884,7 @@ mod tests { let gate = crate::rate_limit::RateLimitGate::default(); let error = crate::error::RobotLBError::HCloudError("boom".to_string()); assert_eq!( - error_action(&error, &gate, std::time::Instant::now()), + error_action(&error, &gate, std::time::Instant::now(), "shop/web"), kube::runtime::controller::Action::requeue(std::time::Duration::from_secs(30)) ); } diff --git a/src/rate_limit.rs b/src/rate_limit.rs index bfa2e80..2792e08 100644 --- a/src/rate_limit.rs +++ b/src/rate_limit.rs @@ -1,4 +1,6 @@ use std::{ + collections::hash_map::DefaultHasher, + hash::{Hash, Hasher}, sync::Mutex, time::{Duration, Instant}, }; @@ -67,9 +69,21 @@ impl RateLimitGate { } } +/// Push a wait out by up to a quarter, differently for each service, so services +/// paused together do not all call the API the moment the pause ends. +#[must_use] +pub fn spread(wait: Duration, service: &str) -> Duration { + let mut hasher = DefaultHasher::new(); + service.hash(&mut hasher); + let window = u64::try_from((wait / 4).as_millis()) + .unwrap_or(u64::MAX) + .max(1); + wait + Duration::from_millis(hasher.finish() % window) +} + #[cfg(test)] mod tests { - use super::RateLimitGate; + use super::{spread, RateLimitGate}; use std::time::{Duration, Instant}; #[test] @@ -138,4 +152,25 @@ mod tests { Duration::from_secs(120) ); } + + #[test] + fn spread_adds_up_to_a_quarter_of_the_pause() { + let wait = Duration::from_secs(60); + for name in ["web", "api", "dns", "ingress-nginx-controller"] { + let spread_wait = spread(wait, name); + assert!(spread_wait >= wait); + assert!(spread_wait < wait + wait / 4); + } + } + + #[test] + fn spread_is_stable_per_service_and_differs_between_services() { + let wait = Duration::from_secs(60); + assert_eq!(spread(wait, "web"), spread(wait, "web")); + let wakeups = ["a", "b", "c", "d", "e", "f"] + .map(|name| spread(wait, name)) + .into_iter() + .collect::>(); + assert!(wakeups.len() > 1); + } } From d39dc1cd077e37bef9e74cff0041f929207b8c5e Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Tue, 29 Sep 2026 12:41:05 +0300 Subject: [PATCH 3/7] fix(hcloud): report services waiting at the rate limit gate With wakeups spread by service name, the same service wakes first after every pause, hits the limit and closes the gate again. Every other service only ever saw the closed gate, which published no event and logged at debug level, so after the event TTL they showed nothing at all. A service that wakes up to a closed gate now gets an event and an info line. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin --- src/error.rs | 8 +++++++- src/main.rs | 17 ++++++++--------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/error.rs b/src/error.rs index d36c3ec..ec91801 100644 --- a/src/error.rs +++ b/src/error.rs @@ -22,7 +22,7 @@ pub enum RobotLBError { UnknownLBAlgorithm, #[error("Cannot get target nodes, because the service has no selector")] ServiceWithoutSelector, - #[error("Hetzner Cloud API rate limit reached, next attempt in {}s", .0.as_secs())] + #[error("Hetzner Cloud API rate limit reached, the pause ends in {}s", .0.as_millis().div_ceil(1000))] RateLimited(std::time::Duration), // HCloud API errors @@ -223,6 +223,12 @@ mod tests { assert_eq!(describe(&error), "502 Bad Gateway"); } + #[test] + fn a_pause_shorter_than_a_second_is_not_reported_as_over() { + let error = RobotLBError::RateLimited(std::time::Duration::from_millis(300)); + assert!(error.to_string().ends_with("in 1s")); + } + #[test] fn a_429_is_a_rate_limit() { let error = RobotLBError::from(response_error(429, "")); diff --git a/src/main.rs b/src/main.rs index 6ec1ef9..edcfa73 100644 --- a/src/main.rs +++ b/src/main.rs @@ -100,7 +100,7 @@ async fn main() -> RobotLBResult<()> { error @ RobotLBError::RateLimited(_), service, )) => { - tracing::debug!("Service {service}: {error}"); + tracing::info!("Service {service}: {error}"); } Err(controller::Error::ReconcilerFailed(error, service)) => { tracing::error!( @@ -155,13 +155,10 @@ pub async fn reconcile_service( result } -/// Skipped services are every service robotlb does not own, and a gated one has not -/// called the API at all, so neither has a failure to show. +/// Skipped services are every service robotlb does not own. A service waiting at the +/// rate limit gate still gets an event each time it wakes up to a closed gate. const fn publishes_event(error: &RobotLBError) -> bool { - !matches!( - error, - RobotLBError::SkipService | RobotLBError::RateLimited(_) - ) + !matches!(error, RobotLBError::SkipService) } /// Put the error on the service as a warning event, where `kubectl describe` shows it. @@ -890,10 +887,12 @@ mod tests { } #[test] - fn only_real_failures_publish_an_event() { + fn every_failure_except_a_skip_publishes_an_event() { use crate::error::RobotLBError; assert!(!publishes_event(&RobotLBError::SkipService)); - assert!(!publishes_event(&RobotLBError::RateLimited( + // Jitter wakes the same service first after every pause, so the others only + // ever see the closed gate, and without an event of their own they go silent. + assert!(publishes_event(&RobotLBError::RateLimited( std::time::Duration::from_secs(1) ))); assert!(publishes_event(&RobotLBError::from(hcloud::apis::Error::< From 785e5bf1ee8ad4452889170aba4d2773504a9905 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Tue, 29 Sep 2026 12:41:06 +0300 Subject: [PATCH 4/7] docs(helm): describe the default service account permissions The comment claimed the default grants all permissions, while it lists only what robotlb needs. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin --- helm/values.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/values.yaml b/helm/values.yaml index 85c559a..865062a 100644 --- a/helm/values.yaml +++ b/helm/values.yaml @@ -28,7 +28,7 @@ serviceAccount: # If not set and create is true, a name is generated using the fullname template name: "" # This is a list of cluster permissions to apply to the service account. - # By default it grants all permissions. + # The default is what robotlb needs; a custom list replaces it entirely. permissions: - apiGroups: [""] resources: [services, services/status] From d6c6ee0d1b02be7fe59dd479f9585ee14ec7da1a Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Tue, 29 Sep 2026 11:25:46 +0300 Subject: [PATCH 5/7] fix(service): release the balancer when a service stops needing it A service changed from LoadBalancer to another type was skipped, so its Hetzner balancer kept running and its finalizer stayed. The API server clears loadBalancerClass with the type, so ownership is now decided by the finalizer: a service robotlb no longer serves has its balancer deleted and the finalizer removed, the same path a deleted service takes. The balancer to delete is found by the Hetzner ID robotlb now records on the service, since the name annotation may be gone by then. The ID is bound to the service UID, so a manifest exported and applied under another name cannot release the original's balancer. Without a recorded ID, or when no balancer has it, the balancer is looked up by name as before. A service with no TCP port that has a nodePort used to lose its external IP while the balancer behind it stayed. The balancer and the address are now kept, and the service gets a warning event instead, since a missing nodePort is more likely a mistake than a request to delete the balancer. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin --- README.md | 11 +- src/consts.rs | 2 + src/error.rs | 18 +++- src/lb.rs | 47 ++++++-- src/main.rs | 288 ++++++++++++++++++++++++++++++++++++++++---------- 5 files changed, 299 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index 02c9aae..87e1b21 100644 --- a/README.md +++ b/README.md @@ -46,10 +46,18 @@ Setting `ROBOTLB_DYNAMIC_NODE_SELECTOR` to `false` replaces both with the node s 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. 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. +robotlb records the Hetzner ID of the balancer on the service in the `robotlb/balancer-id` annotation, and deletes the balancer by that ID when the service is deleted or stops being a `LoadBalancer`, since the name annotation may be gone by then. When no balancer has that ID, the balancer is looked up by name. Services that share a balancer name share one balancer, and the ID does not change that: releasing either of them deletes it. Without the `robotlb/balancer` annotation the name is the service name without its namespace. + +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, an existing balancer and the service's external IP are kept as they are, and a warning event on the service reports the problem. > 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. +> Earlier releases kept the balancer of a service whose type was changed from `LoadBalancer`, or that moved to another load balancer class. Such a service still carries the `robotlb/finalizer` finalizer, and its Hetzner balancer is deleted on the first start after the upgrade. These services have no balancer ID recorded yet, so their balancer is found by name. Without the `robotlb/balancer` annotation the name is the service name without its namespace, so a `LoadBalancer` service with the same name in another namespace may be using that balancer. List the affected services before upgrading and check their balancers: +> +> ```bash +> kubectl get services --all-namespaces --output json | jq --raw-output '.items[] | select(((.metadata.finalizers // []) | index("robotlb/finalizer")) and (.spec.type != "LoadBalancer" or (.spec.loadBalancerClass // "robotlb") != "robotlb")) | "\(.metadata.namespace)/\(.metadata.name)"' +> ``` + ## Configuration @@ -105,6 +113,7 @@ metadata: annotations: # Custom name of the balancer to create on Hetzner. Defaults to service name. robotlb/balancer: "custom name" + # robotlb/balancer-id is written by robotlb, do not set or copy it. # Hetzner cloud network. If this annotation is missing, the operator will try to # assign external IPs to the load balancer if available. Otherwise, the update won't happen. robotlb/lb-network: "my-net" diff --git a/src/consts.rs b/src/consts.rs index 0cedb34..bc659a7 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -1,4 +1,6 @@ pub const LB_NAME_LABEL_NAME: &str = "robotlb/balancer"; +/// Written by robotlb: the Hetzner ID of the balancer it created for the service. +pub const LB_ID_ANN_NAME: &str = "robotlb/balancer-id"; pub const LB_NODE_SELECTOR: &str = "robotlb/node-selector"; pub const LB_NODE_IP_LABEL_NAME: &str = "robotlb/node-ip"; diff --git a/src/error.rs b/src/error.rs index ec91801..1317eec 100644 --- a/src/error.rs +++ b/src/error.rs @@ -22,6 +22,10 @@ pub enum RobotLBError { UnknownLBAlgorithm, #[error("Cannot get target nodes, because the service has no selector")] ServiceWithoutSelector, + #[error( + "No TCP port of the service has a nodePort, so the load balancer has nothing to forward" + )] + NoExposablePorts, #[error("Hetzner Cloud API rate limit reached, the pause ends in {}s", .0.as_millis().div_ceil(1000))] RateLimited(std::time::Duration), @@ -116,11 +120,17 @@ impl RobotLBError { | Self::KubeError(_) | Self::UnknownLBAlgorithm | Self::ServiceWithoutSelector + | Self::NoExposablePorts | Self::RateLimited(_) => false, } } } +#[must_use] +pub fn is_not_found_response(error: &hcloud::apis::Error) -> bool { + matches!(error, hcloud::apis::Error::ResponseError(response) if response.status.as_u16() == 404) +} + /// Whether Hetzner answered 429 because the project ran out of API requests. #[must_use] pub fn is_rate_limit_response(error: &hcloud::apis::Error) -> bool { @@ -168,7 +178,7 @@ pub fn redact(message: &str, token: &str) -> String { #[cfg(test)] mod tests { - use super::{describe, is_rate_limit_response, redact, RobotLBError}; + use super::{describe, is_not_found_response, is_rate_limit_response, redact, RobotLBError}; use hcloud::apis::{load_balancers_api::ListLoadBalancersError, Error, ResponseContent}; const TOKEN: &str = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ01"; @@ -235,6 +245,12 @@ mod tests { assert!(error.is_rate_limited()); } + #[test] + fn only_a_404_response_is_not_found() { + assert!(is_not_found_response(&response_error(404, ""))); + assert!(!is_not_found_response(&response_error(429, ""))); + } + #[test] fn only_a_429_response_is_a_rate_limit() { assert!(is_rate_limit_response(&response_error(429, ""))); diff --git a/src/lb.rs b/src/lb.rs index e1cc5f3..cee831f 100644 --- a/src/lb.rs +++ b/src/lb.rs @@ -4,8 +4,8 @@ use hcloud::{ load_balancers_api::{ AddServiceParams, AddTargetParams, AttachLoadBalancerToNetworkParams, ChangeAlgorithmParams, ChangeTypeOfLoadBalancerParams, DeleteLoadBalancerParams, - DeleteServiceParams, DetachLoadBalancerFromNetworkParams, ListLoadBalancersParams, - RemoveTargetParams, UpdateServiceParams, + DeleteServiceParams, DetachLoadBalancerFromNetworkParams, GetLoadBalancerParams, + ListLoadBalancersParams, RemoveTargetParams, UpdateServiceParams, }, networks_api::ListNetworksParams, }, @@ -517,11 +517,29 @@ impl LoadBalancer { Ok(()) } - /// Cleanup the load balancer. - /// This method will remove all the services and targets from the - /// load balancer. - pub async fn cleanup(&self) -> RobotLBResult<()> { - let Some(hcloud_balancer) = self.get_hcloud_lb().await? else { + /// Delete the balancer of the service. The recorded ID is preferred over the name, + /// which may have changed since the balancer was found; the name is still tried + /// when no balancer has the recorded ID. + pub async fn cleanup(&self, recorded_id: Option) -> RobotLBResult<()> { + let mut hcloud_balancer = match recorded_id { + Some(id) => { + match hcloud::apis::load_balancers_api::get_load_balancer( + &self.hcloud_config, + GetLoadBalancerParams { id }, + ) + .await + { + Ok(response) => Some(*response.load_balancer), + Err(error) if crate::error::is_not_found_response(&error) => None, + Err(error) => return Err(error.into()), + } + } + None => None, + }; + if falls_back_to_name(recorded_id, hcloud_balancer.is_some()) { + hcloud_balancer = self.get_hcloud_lb().await?; + } + let Some(hcloud_balancer) = hcloud_balancer else { return Ok(()); }; for service in &hcloud_balancer.services { @@ -668,6 +686,10 @@ impl LoadBalancer { } } +const fn falls_back_to_name(recorded_id: Option, found_by_id: bool) -> bool { + recorded_id.is_none() || !found_by_id +} + /// 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. @@ -704,7 +726,7 @@ impl From for LoadBalancerAlgorithm { #[cfg(test)] mod tests { - use super::plan_targets; + use super::{falls_back_to_name, plan_targets}; #[test] fn targets_are_sorted_and_deduplicated() { @@ -735,4 +757,13 @@ mod tests { let desired = vec!["192.168.100.2".to_string(), "192.168.100.3".to_string()]; assert_eq!(plan_targets(&desired, 25).len(), 2); } + + // A recorded ID goes stale when a reconcile creates a new balancer and fails + // before recording it, and the balancer under the name must still be found. + #[test] + fn a_missing_recorded_balancer_falls_back_to_the_name() { + assert!(falls_back_to_name(Some(4711), false)); + assert!(!falls_back_to_name(Some(4711), true)); + assert!(falls_back_to_name(None, false)); + } } diff --git a/src/main.rs b/src/main.rs index edcfa73..dca50da 100644 --- a/src/main.rs +++ b/src/main.rs @@ -199,26 +199,43 @@ fn event_note(error: &RobotLBError, token: &str) -> String { note } -async fn sync_service(svc: Arc, context: Arc) -> RobotLBResult { - let svc_type = svc - .spec - .as_ref() - .and_then(|s| s.type_.as_ref()) - .map(String::as_str) - .unwrap_or("ClusterIP"); - if svc_type != "LoadBalancer" { - tracing::debug!("Service type is not LoadBalancer. Skipping..."); - return Err(RobotLBError::SkipService); - } +#[derive(Debug, PartialEq, Eq)] +enum ServiceRole { + /// The service is not robotlb's. + Skip, + /// The service needs its load balancer created or updated. + Reconcile, + /// The service had a load balancer and no longer needs it. + Release, +} - let lb_type = svc +/// The API server wipes `loadBalancerClass` together with the `LoadBalancer` type, +/// so once the type changes only the finalizer still marks the service as robotlb's, +/// and a service with the finalizer that robotlb no longer serves gets released. +fn service_role(svc: &Service) -> ServiceRole { + let is_load_balancer = + svc.spec.as_ref().and_then(|s| s.type_.as_deref()) == Some("LoadBalancer"); + let class = svc .spec .as_ref() - .and_then(|s| s.load_balancer_class.as_ref()) - .map(String::as_str) + .and_then(|s| s.load_balancer_class.as_deref()) .unwrap_or(consts::ROBOTLB_LB_CLASS); - if lb_type != consts::ROBOTLB_LB_CLASS { - tracing::debug!("Load balancer class is not robotlb. Skipping..."); + let is_robotlb = is_load_balancer && class == consts::ROBOTLB_LB_CLASS; + let owned = finalizers::check(svc); + let deleting = svc.meta().deletion_timestamp.is_some(); + if (deleting && is_robotlb) || (owned && !is_robotlb) { + ServiceRole::Release + } else if is_robotlb && !deleting { + ServiceRole::Reconcile + } else { + ServiceRole::Skip + } +} + +async fn sync_service(svc: Arc, context: Arc) -> RobotLBResult { + let role = service_role(&svc); + if role == ServiceRole::Skip { + tracing::debug!("Service is not a robotlb load balancer. Skipping..."); return Err(RobotLBError::SkipService); } @@ -232,10 +249,9 @@ async fn sync_service(svc: Arc, context: Arc) -> RobotL let lb = LoadBalancer::try_from_svc(&svc, &context)?; - // If the service is being deleted, we need to clean up the resources. - if svc.meta().deletion_timestamp.is_some() { - tracing::info!("Service deletion detected. Cleaning up resources."); - lb.cleanup().await?; + if role == ServiceRole::Release { + tracing::info!("Service no longer needs a load balancer. Cleaning up resources."); + lb.cleanup(recorded_balancer_id(&svc)).await?; finalizers::remove(context.client.clone(), &svc).await?; return Ok(Action::await_change()); } @@ -516,6 +532,16 @@ pub async fn reconcile_load_balancer( lb.add_service(service.listen_port, service.target_port); } + // 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. + // An existing balancer and the address it gave the service are kept: a missing + // nodePort is far more likely a mistake than a request to delete the balancer. + if lb.services.is_empty() { + return Err(RobotLBError::NoExposablePorts); + } + + let hcloud_lb = lb.reconcile().await?; + let svc_api = kube::Api::::namespaced( context.client.clone(), svc.namespace() @@ -523,16 +549,18 @@ 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))); + // Releasing the balancer later goes by this ID, since by then the name annotation + // may be gone. + if let Some(patch) = balancer_id_patch(&svc, hcloud_lb.id) { + svc_api + .patch( + svc.name_any().as_str(), + &PatchParams::default(), + &kube::api::Patch::Merge(patch), + ) + .await?; } - let hcloud_lb = lb.reconcile().await?; - let mut ingress = vec![]; let dns_ipv4 = hcloud_lb.public_net.ipv4.dns_ptr.flatten(); @@ -575,33 +603,34 @@ 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(()) +/// The value is `/`: a manifest exported and applied again carries the +/// annotation along, and only the object it was written for may act on it. +fn recorded_balancer_id(svc: &Service) -> Option { + let (uid, id) = svc + .annotations() + .get(consts::LB_ID_ANN_NAME)? + .split_once('/')?; + if svc.uid().as_deref() != Some(uid) { + return None; + } + // Hetzner IDs are positive; anything else was edited by hand and must not + // make every cleanup attempt fail. + id.parse().ok().filter(|id| *id > 0) +} + +/// A patch recording the balancer ID, unless the service already carries it. +fn balancer_id_patch(svc: &Service, id: i64) -> Option { + let uid = svc.uid()?; + if recorded_balancer_id(svc) == Some(id) { + return None; + } + Some(json!({ + "metadata": { + "annotations": { + consts::LB_ID_ANN_NAME: format!("{uid}/{id}") + } + } + })) } /// Handle the error during reconcilation. @@ -630,8 +659,9 @@ fn error_action( #[cfg(test)] mod tests { use super::{ - collect_lb_services, consts, error_action, event_note, is_excluded_from_lb, - is_lb_eligible_node, is_local_traffic_policy, node_source, publishes_event, NodeSource, + balancer_id_patch, collect_lb_services, consts, error_action, event_note, + is_excluded_from_lb, is_lb_eligible_node, is_local_traffic_policy, node_source, + publishes_event, recorded_balancer_id, service_role, NodeSource, ServiceRole, }; use k8s_openapi::{ api::core::v1::{ @@ -908,4 +938,148 @@ mod tests { "boom".to_string() ))); } + + fn owned_service(type_: &str, class: Option<&str>, deleting: bool) -> Service { + Service { + metadata: ObjectMeta { + finalizers: Some(vec![consts::FINALIZER_NAME.to_string()]), + deletion_timestamp: deleting.then(|| { + k8s_openapi::apimachinery::pkg::apis::meta::v1::Time( + k8s_openapi::chrono::Utc::now(), + ) + }), + ..Default::default() + }, + spec: Some(ServiceSpec { + type_: Some(type_.to_string()), + load_balancer_class: class.map(str::to_string), + ..Default::default() + }), + ..Default::default() + } + } + + #[test] + fn a_robotlb_load_balancer_is_reconciled() { + let svc = service(ServiceSpec { + type_: Some("LoadBalancer".to_string()), + ..Default::default() + }); + assert_eq!(service_role(&svc), ServiceRole::Reconcile); + let svc = owned_service("LoadBalancer", Some(consts::ROBOTLB_LB_CLASS), false); + assert_eq!(service_role(&svc), ServiceRole::Reconcile); + } + + #[test] + fn services_robotlb_does_not_own_are_skipped() { + let other_class = service(ServiceSpec { + type_: Some("LoadBalancer".to_string()), + load_balancer_class: Some("example.com/other".to_string()), + ..Default::default() + }); + assert_eq!(service_role(&other_class), ServiceRole::Skip); + let cluster_ip = service(ServiceSpec { + type_: Some("ClusterIP".to_string()), + ..Default::default() + }); + assert_eq!(service_role(&cluster_ip), ServiceRole::Skip); + let mut deleting_cluster_ip = owned_service("ClusterIP", None, true); + deleting_cluster_ip.metadata.finalizers = None; + assert_eq!(service_role(&deleting_cluster_ip), ServiceRole::Skip); + let mut deleting_other_class = + owned_service("LoadBalancer", Some("example.com/other"), true); + deleting_other_class.metadata.finalizers = None; + assert_eq!(service_role(&deleting_other_class), ServiceRole::Skip); + } + + // A type change and a change back with another class can both land before robotlb + // reconciles, for example while the rate limit gate is closed. + #[test] + fn an_owned_service_taken_over_by_another_class_is_released() { + let svc = owned_service("LoadBalancer", Some("example.com/other"), false); + assert_eq!(service_role(&svc), ServiceRole::Release); + } + + // Reachable when the finalizer was removed by hand while another controller's + // finalizer still holds the object. + #[test] + fn a_deleted_robotlb_load_balancer_without_the_finalizer_is_released() { + let mut svc = owned_service("LoadBalancer", None, true); + svc.metadata.finalizers = None; + assert_eq!(service_role(&svc), ServiceRole::Release); + } + + #[test] + fn an_owned_service_that_stopped_being_a_load_balancer_is_released() { + assert_eq!( + service_role(&owned_service("ClusterIP", None, false)), + ServiceRole::Release + ); + } + + #[test] + fn a_deleted_owned_service_is_released() { + assert_eq!( + service_role(&owned_service("LoadBalancer", None, true)), + ServiceRole::Release + ); + assert_eq!( + service_role(&owned_service("ClusterIP", None, true)), + ServiceRole::Release + ); + } + + #[test] + fn a_service_without_exposable_ports_reports_an_event() { + let error = crate::error::RobotLBError::NoExposablePorts; + assert!(publishes_event(&error)); + assert!(error.to_string().starts_with("No TCP port")); + } + + fn annotated(value: &str) -> Service { + Service { + metadata: ObjectMeta { + uid: Some("uid-1".to_string()), + annotations: Some(BTreeMap::from([( + consts::LB_ID_ANN_NAME.to_string(), + value.to_string(), + )])), + ..Default::default() + }, + ..Default::default() + } + } + + #[test] + fn the_recorded_balancer_id_is_read_back() { + assert_eq!(recorded_balancer_id(&annotated("uid-1/4711")), Some(4711)); + assert_eq!(recorded_balancer_id(&annotated("uid-1/not-a-number")), None); + assert_eq!(recorded_balancer_id(&annotated("uid-1/0")), None); + assert_eq!(recorded_balancer_id(&annotated("uid-1/-1")), None); + assert_eq!(recorded_balancer_id(&annotated("4711")), None); + assert_eq!(recorded_balancer_id(&Service::default()), None); + } + + // A manifest exported with kubectl and applied under another name keeps the + // annotation, and must not release the original service's balancer. + #[test] + fn an_id_recorded_for_another_object_is_ignored() { + assert_eq!(recorded_balancer_id(&annotated("uid-2/4711")), None); + } + + #[test] + fn the_balancer_id_is_recorded_only_when_it_changes() { + assert!(balancer_id_patch(&annotated("uid-1/4711"), 4711).is_none()); + let patch = balancer_id_patch(&annotated("uid-1/4711"), 4712).unwrap(); + assert_eq!( + patch["metadata"]["annotations"][consts::LB_ID_ANN_NAME], + "uid-1/4712" + ); + let copied = balancer_id_patch(&annotated("uid-2/4711"), 4711).unwrap(); + assert_eq!( + copied["metadata"]["annotations"][consts::LB_ID_ANN_NAME], + "uid-1/4711" + ); + assert!(balancer_id_patch(&Service::default(), 1).is_none()); + } } From baea7e14faa831cdd487cba2c7fca966cdfe4385 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Tue, 6 Oct 2026 11:51:10 +0300 Subject: [PATCH 6/7] fix(lb): delete the balancer with a single request (#57) cleanup removed every service and every target one request at a time and only then deleted the balancer, so a balancer with 3 ports and 10 targets cost 14 API calls instead of 1. That matters when the project is close to the hourly rate limit. The API reference only says "Deletes a Load Balancer" and does not describe what happens to its services and targets. The change follows how Hetzner's own cloud controller manager and CLI delete a balancer: one delete call, with no prior removal of services or targets and no network detach. Network handling is left as it was. A failure part-way no longer leaves a balancer with some services already removed. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin --- src/lb.rs | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/src/lb.rs b/src/lb.rs index cee831f..8fa5c29 100644 --- a/src/lb.rs +++ b/src/lb.rs @@ -542,39 +542,6 @@ impl LoadBalancer { let Some(hcloud_balancer) = hcloud_balancer else { return Ok(()); }; - for service in &hcloud_balancer.services { - tracing::info!( - "Deleting service that listens for port {} from load-balancer {}", - service.listen_port, - hcloud_balancer.name, - ); - hcloud::apis::load_balancers_api::delete_service( - &self.hcloud_config, - DeleteServiceParams { - id: hcloud_balancer.id, - delete_service_request: Some(DeleteServiceRequest { - listen_port: service.listen_port, - }), - }, - ) - .await?; - } - for target in &hcloud_balancer.targets { - if let Some(target_ip) = target.ip.clone() { - tracing::info!("Removing target {}", target_ip.ip); - hcloud::apis::load_balancers_api::remove_target( - &self.hcloud_config, - RemoveTargetParams { - id: hcloud_balancer.id, - remove_target_request: Some(RemoveTargetRequest { - ip: Some(target_ip), - ..Default::default() - }), - }, - ) - .await?; - } - } hcloud::apis::load_balancers_api::delete_load_balancer( &self.hcloud_config, DeleteLoadBalancerParams { From bea258d680a565ccf0c99d49a81e267fa5c6c76e Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Tue, 6 Oct 2026 12:49:37 +0300 Subject: [PATCH 7/7] fix(service): find the released balancer by name only Recording the balancer ID on the service as an annotation adds state that users can copy between objects or edit by hand, and the operator has to guard against both. Drop the annotation and look the balancer up by name when releasing it, as robotlb did before. A balancer whose name annotation was changed or removed before the release is not found, and the README now says so. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin --- README.md | 5 ++- src/consts.rs | 2 -- src/error.rs | 13 +------ src/lb.rs | 45 ++++-------------------- src/main.rs | 97 +++------------------------------------------------ 5 files changed, 13 insertions(+), 149 deletions(-) diff --git a/README.md b/README.md index 87e1b21..7f3b221 100644 --- a/README.md +++ b/README.md @@ -46,13 +46,13 @@ Setting `ROBOTLB_DYNAMIC_NODE_SELECTOR` to `false` replaces both with the node s 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. -robotlb records the Hetzner ID of the balancer on the service in the `robotlb/balancer-id` annotation, and deletes the balancer by that ID when the service is deleted or stops being a `LoadBalancer`, since the name annotation may be gone by then. When no balancer has that ID, the balancer is looked up by name. Services that share a balancer name share one balancer, and the ID does not change that: releasing either of them deletes it. Without the `robotlb/balancer` annotation the name is the service name without its namespace. +robotlb deletes the balancer when the service is deleted or stops being a `LoadBalancer`, and finds it by the name the service has at that moment. When the `robotlb/balancer` annotation was changed or removed before that, the balancer under the old name stays in the project, and the balancer under the current name is deleted, even when another service uses it. Services that share a balancer name share one balancer: releasing either of them deletes it. Without the `robotlb/balancer` annotation the name is the service name without its namespace. 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, an existing balancer and the service's external IP are kept as they are, and a warning event on the service reports the problem. > 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. -> Earlier releases kept the balancer of a service whose type was changed from `LoadBalancer`, or that moved to another load balancer class. Such a service still carries the `robotlb/finalizer` finalizer, and its Hetzner balancer is deleted on the first start after the upgrade. These services have no balancer ID recorded yet, so their balancer is found by name. Without the `robotlb/balancer` annotation the name is the service name without its namespace, so a `LoadBalancer` service with the same name in another namespace may be using that balancer. List the affected services before upgrading and check their balancers: +> Earlier releases kept the balancer of a service whose type was changed from `LoadBalancer`, or that moved to another load balancer class. Such a service still carries the `robotlb/finalizer` finalizer, and its Hetzner balancer is deleted on the first start after the upgrade. Its balancer is found by name. Without the `robotlb/balancer` annotation the name is the service name without its namespace, so a `LoadBalancer` service with the same name in another namespace may be using that balancer. List the affected services before upgrading and check their balancers: > > ```bash > kubectl get services --all-namespaces --output json | jq --raw-output '.items[] | select(((.metadata.finalizers // []) | index("robotlb/finalizer")) and (.spec.type != "LoadBalancer" or (.spec.loadBalancerClass // "robotlb") != "robotlb")) | "\(.metadata.namespace)/\(.metadata.name)"' @@ -113,7 +113,6 @@ metadata: annotations: # Custom name of the balancer to create on Hetzner. Defaults to service name. robotlb/balancer: "custom name" - # robotlb/balancer-id is written by robotlb, do not set or copy it. # Hetzner cloud network. If this annotation is missing, the operator will try to # assign external IPs to the load balancer if available. Otherwise, the update won't happen. robotlb/lb-network: "my-net" diff --git a/src/consts.rs b/src/consts.rs index bc659a7..0cedb34 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -1,6 +1,4 @@ pub const LB_NAME_LABEL_NAME: &str = "robotlb/balancer"; -/// Written by robotlb: the Hetzner ID of the balancer it created for the service. -pub const LB_ID_ANN_NAME: &str = "robotlb/balancer-id"; pub const LB_NODE_SELECTOR: &str = "robotlb/node-selector"; pub const LB_NODE_IP_LABEL_NAME: &str = "robotlb/node-ip"; diff --git a/src/error.rs b/src/error.rs index 1317eec..573719b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -126,11 +126,6 @@ impl RobotLBError { } } -#[must_use] -pub fn is_not_found_response(error: &hcloud::apis::Error) -> bool { - matches!(error, hcloud::apis::Error::ResponseError(response) if response.status.as_u16() == 404) -} - /// Whether Hetzner answered 429 because the project ran out of API requests. #[must_use] pub fn is_rate_limit_response(error: &hcloud::apis::Error) -> bool { @@ -178,7 +173,7 @@ pub fn redact(message: &str, token: &str) -> String { #[cfg(test)] mod tests { - use super::{describe, is_not_found_response, is_rate_limit_response, redact, RobotLBError}; + use super::{describe, is_rate_limit_response, redact, RobotLBError}; use hcloud::apis::{load_balancers_api::ListLoadBalancersError, Error, ResponseContent}; const TOKEN: &str = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ01"; @@ -245,12 +240,6 @@ mod tests { assert!(error.is_rate_limited()); } - #[test] - fn only_a_404_response_is_not_found() { - assert!(is_not_found_response(&response_error(404, ""))); - assert!(!is_not_found_response(&response_error(429, ""))); - } - #[test] fn only_a_429_response_is_a_rate_limit() { assert!(is_rate_limit_response(&response_error(429, ""))); diff --git a/src/lb.rs b/src/lb.rs index 8fa5c29..2da9c40 100644 --- a/src/lb.rs +++ b/src/lb.rs @@ -4,8 +4,8 @@ use hcloud::{ load_balancers_api::{ AddServiceParams, AddTargetParams, AttachLoadBalancerToNetworkParams, ChangeAlgorithmParams, ChangeTypeOfLoadBalancerParams, DeleteLoadBalancerParams, - DeleteServiceParams, DetachLoadBalancerFromNetworkParams, GetLoadBalancerParams, - ListLoadBalancersParams, RemoveTargetParams, UpdateServiceParams, + DeleteServiceParams, DetachLoadBalancerFromNetworkParams, ListLoadBalancersParams, + RemoveTargetParams, UpdateServiceParams, }, networks_api::ListNetworksParams, }, @@ -517,29 +517,9 @@ impl LoadBalancer { Ok(()) } - /// Delete the balancer of the service. The recorded ID is preferred over the name, - /// which may have changed since the balancer was found; the name is still tried - /// when no balancer has the recorded ID. - pub async fn cleanup(&self, recorded_id: Option) -> RobotLBResult<()> { - let mut hcloud_balancer = match recorded_id { - Some(id) => { - match hcloud::apis::load_balancers_api::get_load_balancer( - &self.hcloud_config, - GetLoadBalancerParams { id }, - ) - .await - { - Ok(response) => Some(*response.load_balancer), - Err(error) if crate::error::is_not_found_response(&error) => None, - Err(error) => return Err(error.into()), - } - } - None => None, - }; - if falls_back_to_name(recorded_id, hcloud_balancer.is_some()) { - hcloud_balancer = self.get_hcloud_lb().await?; - } - let Some(hcloud_balancer) = hcloud_balancer else { + /// Delete the balancer of the service, found by its name. + pub async fn cleanup(&self) -> RobotLBResult<()> { + let Some(hcloud_balancer) = self.get_hcloud_lb().await? else { return Ok(()); }; hcloud::apis::load_balancers_api::delete_load_balancer( @@ -653,10 +633,6 @@ impl LoadBalancer { } } -const fn falls_back_to_name(recorded_id: Option, found_by_id: bool) -> bool { - recorded_id.is_none() || !found_by_id -} - /// 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. @@ -693,7 +669,7 @@ impl From for LoadBalancerAlgorithm { #[cfg(test)] mod tests { - use super::{falls_back_to_name, plan_targets}; + use super::plan_targets; #[test] fn targets_are_sorted_and_deduplicated() { @@ -724,13 +700,4 @@ mod tests { let desired = vec!["192.168.100.2".to_string(), "192.168.100.3".to_string()]; assert_eq!(plan_targets(&desired, 25).len(), 2); } - - // A recorded ID goes stale when a reconcile creates a new balancer and fails - // before recording it, and the balancer under the name must still be found. - #[test] - fn a_missing_recorded_balancer_falls_back_to_the_name() { - assert!(falls_back_to_name(Some(4711), false)); - assert!(!falls_back_to_name(Some(4711), true)); - assert!(falls_back_to_name(None, false)); - } } diff --git a/src/main.rs b/src/main.rs index dca50da..e3c4475 100644 --- a/src/main.rs +++ b/src/main.rs @@ -251,7 +251,7 @@ async fn sync_service(svc: Arc, context: Arc) -> RobotL if role == ServiceRole::Release { tracing::info!("Service no longer needs a load balancer. Cleaning up resources."); - lb.cleanup(recorded_balancer_id(&svc)).await?; + lb.cleanup().await?; finalizers::remove(context.client.clone(), &svc).await?; return Ok(Action::await_change()); } @@ -549,18 +549,6 @@ pub async fn reconcile_load_balancer( .as_str(), ); - // Releasing the balancer later goes by this ID, since by then the name annotation - // may be gone. - if let Some(patch) = balancer_id_patch(&svc, hcloud_lb.id) { - svc_api - .patch( - svc.name_any().as_str(), - &PatchParams::default(), - &kube::api::Patch::Merge(patch), - ) - .await?; - } - let mut ingress = vec![]; let dns_ipv4 = hcloud_lb.public_net.ipv4.dns_ptr.flatten(); @@ -603,36 +591,6 @@ pub async fn reconcile_load_balancer( Ok(Action::requeue(Duration::from_secs(30))) } -/// The value is `/`: a manifest exported and applied again carries the -/// annotation along, and only the object it was written for may act on it. -fn recorded_balancer_id(svc: &Service) -> Option { - let (uid, id) = svc - .annotations() - .get(consts::LB_ID_ANN_NAME)? - .split_once('/')?; - if svc.uid().as_deref() != Some(uid) { - return None; - } - // Hetzner IDs are positive; anything else was edited by hand and must not - // make every cleanup attempt fail. - id.parse().ok().filter(|id| *id > 0) -} - -/// A patch recording the balancer ID, unless the service already carries it. -fn balancer_id_patch(svc: &Service, id: i64) -> Option { - let uid = svc.uid()?; - if recorded_balancer_id(svc) == Some(id) { - return None; - } - Some(json!({ - "metadata": { - "annotations": { - consts::LB_ID_ANN_NAME: format!("{uid}/{id}") - } - } - })) -} - /// Handle the error during reconcilation. #[allow(clippy::needless_pass_by_value)] fn on_error(svc: Arc, error: &RobotLBError, context: Arc) -> Action { @@ -659,9 +617,9 @@ fn error_action( #[cfg(test)] mod tests { use super::{ - balancer_id_patch, collect_lb_services, consts, error_action, event_note, - is_excluded_from_lb, is_lb_eligible_node, is_local_traffic_policy, node_source, - publishes_event, recorded_balancer_id, service_role, NodeSource, ServiceRole, + collect_lb_services, consts, error_action, event_note, is_excluded_from_lb, + is_lb_eligible_node, is_local_traffic_policy, node_source, publishes_event, service_role, + NodeSource, ServiceRole, }; use k8s_openapi::{ api::core::v1::{ @@ -1035,51 +993,4 @@ mod tests { assert!(publishes_event(&error)); assert!(error.to_string().starts_with("No TCP port")); } - - fn annotated(value: &str) -> Service { - Service { - metadata: ObjectMeta { - uid: Some("uid-1".to_string()), - annotations: Some(BTreeMap::from([( - consts::LB_ID_ANN_NAME.to_string(), - value.to_string(), - )])), - ..Default::default() - }, - ..Default::default() - } - } - - #[test] - fn the_recorded_balancer_id_is_read_back() { - assert_eq!(recorded_balancer_id(&annotated("uid-1/4711")), Some(4711)); - assert_eq!(recorded_balancer_id(&annotated("uid-1/not-a-number")), None); - assert_eq!(recorded_balancer_id(&annotated("uid-1/0")), None); - assert_eq!(recorded_balancer_id(&annotated("uid-1/-1")), None); - assert_eq!(recorded_balancer_id(&annotated("4711")), None); - assert_eq!(recorded_balancer_id(&Service::default()), None); - } - - // A manifest exported with kubectl and applied under another name keeps the - // annotation, and must not release the original service's balancer. - #[test] - fn an_id_recorded_for_another_object_is_ignored() { - assert_eq!(recorded_balancer_id(&annotated("uid-2/4711")), None); - } - - #[test] - fn the_balancer_id_is_recorded_only_when_it_changes() { - assert!(balancer_id_patch(&annotated("uid-1/4711"), 4711).is_none()); - let patch = balancer_id_patch(&annotated("uid-1/4711"), 4712).unwrap(); - assert_eq!( - patch["metadata"]["annotations"][consts::LB_ID_ANN_NAME], - "uid-1/4712" - ); - let copied = balancer_id_patch(&annotated("uid-2/4711"), 4711).unwrap(); - assert_eq!( - copied["metadata"]["annotations"][consts::LB_ID_ANN_NAME], - "uid-1/4711" - ); - assert!(balancer_id_patch(&Service::default(), 1).is_none()); - } }