From c06b9e2da0fc1edf21f66528f5ec67e297d7ad22 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Tue, 29 Sep 2026 19:56:40 +0300 Subject: [PATCH] fix(service): release a service without reading its annotations The release path built the full load balancer from the service annotations before cleaning up. An annotation that did not parse, such as a non-numeric retry count, failed the release before the balancer was deleted, so the finalizer stayed and the service could not be deleted or leave the LoadBalancer type. A release finds the balancer by the service UID label alone, so it now builds the load balancer from the UID and the Hetzner client only. Reconciliation still parses every annotation as before. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin --- src/lb.rs | 32 ++++++++++++++++++++++++++++++-- src/main.rs | 44 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/src/lb.rs b/src/lb.rs index f5d5949..f2a61c7 100644 --- a/src/lb.rs +++ b/src/lb.rs @@ -40,7 +40,7 @@ enum LBAlgorithm { /// Struct representing a load balancer /// It holds all the necessary information to manage the load balancer /// in Hetzner Cloud. -#[derive(Debug)] +#[derive(Debug, Default)] pub struct LoadBalancer { pub name: String, /// The name earlier releases gave the balancer, when it differs from `name`. @@ -162,6 +162,17 @@ impl LoadBalancer { }) } + /// A load balancer that is only good for `cleanup`, which finds the balancer by the + /// service UID alone. The annotations are not read: one that does not parse must + /// not keep the service from being released. + pub fn for_release(svc: &Service, hcloud_config: HcloudConfig) -> RobotLBResult { + Ok(Self { + service_uid: svc.uid().ok_or(RobotLBError::SkipService)?, + hcloud_config, + ..Default::default() + }) + } + /// Add a service to the load balancer. /// The service will listen on the `listen_port` and forward the /// traffic to the `target_port` to all targets. @@ -830,13 +841,15 @@ impl From for LoadBalancerAlgorithm { mod tests { use super::{ candidate_names, decide, default_name, owner_labels, owner_selector, plan_targets, single, - Decision, Purpose, + Decision, LoadBalancer, Purpose, }; use crate::{consts, error::RobotLBError}; + use hcloud::apis::configuration::Configuration as HcloudConfig; use hcloud::models::{ load_balancer_target, LoadBalancer as HcloudBalancer, LoadBalancerTarget, LoadBalancerTargetIp, }; + use k8s_openapi::{api::core::v1::Service, apimachinery::pkg::apis::meta::v1::ObjectMeta}; use std::collections::HashMap; #[test] @@ -1003,4 +1016,19 @@ mod tests { Err(RobotLBError::AmbiguousBalancer(_)) )); } + #[test] + fn a_release_ignores_annotations_that_do_not_parse() { + let svc = Service { + metadata: ObjectMeta { + uid: Some("uid-1".to_string()), + annotations: Some( + [(consts::LB_RETRIES_ANN_NAME.to_string(), "abc".to_string())].into(), + ), + ..Default::default() + }, + ..Default::default() + }; + let lb = LoadBalancer::for_release(&svc, HcloudConfig::default()).unwrap(); + assert_eq!(lb.service_uid, "uid-1"); + } } diff --git a/src/main.rs b/src/main.rs index cae620c..16d441f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -232,6 +232,18 @@ fn service_role(svc: &Service) -> ServiceRole { } } +fn balancer_for( + role: &ServiceRole, + svc: &Service, + context: &CurrentContext, +) -> RobotLBResult { + if *role == ServiceRole::Release { + LoadBalancer::for_release(svc, context.hcloud_config.clone()) + } else { + LoadBalancer::try_from_svc(svc, context) + } +} + async fn sync_service(svc: Arc, context: Arc) -> RobotLBResult { let role = service_role(&svc); if role == ServiceRole::Skip { @@ -247,7 +259,7 @@ async fn sync_service(svc: Arc, context: Arc) -> RobotL tracing::info!("Starting service reconcilation"); - let lb = LoadBalancer::try_from_svc(&svc, &context)?; + let lb = balancer_for(&role, &svc, &context)?; if role == ServiceRole::Release { tracing::info!("Service no longer needs a load balancer. Cleaning up resources."); @@ -617,10 +629,11 @@ fn error_action( #[cfg(test)] mod tests { use super::{ - collect_lb_services, consts, error_action, event_note, is_excluded_from_lb, + balancer_for, 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, + CurrentContext, HCloudConfig, NodeSource, OperatorConfig, RobotLBError, ServiceRole, }; + use clap::Parser; use k8s_openapi::{ api::core::v1::{ Node, NodeCondition, NodeSpec, NodeStatus, Service, ServicePort, ServiceSpec, @@ -629,6 +642,31 @@ mod tests { }; use std::collections::BTreeMap; + #[tokio::test] + async fn only_a_reconciliation_reads_the_annotations() { + let config = OperatorConfig::try_parse_from(["robotlb", "--hcloud-token", "t"]).unwrap(); + let client = + kube::Client::try_from(kube::Config::new("http://127.0.0.1:1".parse().unwrap())) + .unwrap(); + let context = CurrentContext::new(client, config, HCloudConfig::default()); + let svc = Service { + metadata: ObjectMeta { + uid: Some("uid-1".to_string()), + annotations: Some( + [(consts::LB_RETRIES_ANN_NAME.to_string(), "abc".to_string())].into(), + ), + ..Default::default() + }, + ..Default::default() + }; + assert!(balancer_for(&ServiceRole::Release, &svc, &context).is_ok()); + assert!(balancer_for(&ServiceRole::Reconcile, &svc, &context).is_err()); + assert!(matches!( + balancer_for(&ServiceRole::Release, &Service::default(), &context), + Err(RobotLBError::SkipService) + )); + } + fn service(spec: ServiceSpec) -> Service { Service { spec: Some(spec),