From 4b7b199847ae5cb02006305716aa45ccde0ca2b9 Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Tue, 29 Sep 2026 19:13:14 +0300 Subject: [PATCH] fix(lb): act only on balancers labelled for the service A balancer was found only by its name, and without the robotlb/balancer annotation that name was the service name without the namespace. Services with the same name in two namespaces shared one balancer, overwrote each other's ports and targets, and releasing one deleted the other's. Any balancer whose name a service resolved to, including one named in the annotation, could be reconfigured or deleted, even one robotlb never created. New balancers are named . and carry the robotlb/service-uid label from the create request on. The balancer of a service is found by that label, and only a labelled balancer is changed or deleted. A release never goes by name, so a service released again cannot hit a balancer another service created under the old name since. The label replaces the robotlb/balancer-id annotation, which recorded the balancer ID for the release: the label identifies the balancer the same way, cannot go stale between creating a balancer and recording it, and saves a patch of the service. The annotation was never released. Balancers from earlier releases have no label. A reconcile adopts an unlabelled balancer under the old name or the name the service asks for only when every target is an IP and one of them is a node of the service, so a balancer pointing at other hosts is never taken over. While the service has no target nodes, an unlabelled balancer with IP targets cannot be judged, and the service waits with a warning event instead of replacing it. Otherwise, under the old name a balancer that is not adopted is skipped, and the service gets a new balancer under . without an event. Under the name the service asks for it is left alone with a warning event: one labelled for another service names that service's UID, an unlabelled one names the command to hand it over. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin --- README.md | 14 +- src/consts.rs | 2 + src/error.rs | 57 ++++++++ src/lb.rs | 391 ++++++++++++++++++++++++++++++++++++++++++++------ src/main.rs | 14 ++ 5 files changed, 431 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 7f3b221..76fa2f3 100644 --- a/README.md +++ b/README.md @@ -46,13 +46,21 @@ 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 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. +robotlb labels every balancer it creates with `robotlb/service-uid` set to the UID of the service, finds the balancer of a service by that label, and changes or deletes only a balancer labelled for that service. A balancer is never deleted by its name. Without the `robotlb/balancer` annotation the balancer is named `.`. The name is only used to create the balancer: changing the annotation later does not rename it. When the name is taken by a balancer labelled for another service, or by an unlabelled balancer that does not target the nodes of the service, robotlb leaves that balancer alone and reports it in a warning event on the service. Two services therefore cannot share a balancer through the same `robotlb/balancer` annotation: the second one gets the warning. A service recreated with a new UID, for example restored from a backup, is another service to robotlb, because its old balancer is labelled with the old UID. When that balancer has the name the service asks for, the warning names the old UID. When it has the name of an earlier release, robotlb creates a new balancer with a new IP instead, without a warning, and the old one stays in the project and keeps being billed; `hcloud load-balancer list --selector robotlb/service-uid=` finds it. After checking that no service with the old UID is left, hand the balancer over with `hcloud load-balancer add-label --overwrite '' robotlb/service-uid=`, or delete it. An unlabelled balancer whose targets are all IPs, at least one of them a node of the service, is adopted: robotlb adds its label, keeps the name, and from then on manages and deletes it like its own. This is how balancers of earlier releases are taken over, and it applies to any balancer created later under that name that matches. 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. 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: +> Earlier releases created balancers without a label and named them after the service without its namespace. On the first reconcile after the upgrade, a service with no labelled balancer adopts the balancer under its old name, or under the name from its `robotlb/balancer` annotation, if every target of that balancer is an IP and at least one of them is a node of the service: robotlb adds its label and keeps the name. Otherwise the balancer is left alone: under the old name robotlb creates a new balancer with a new IP under `.`, under an annotated name the service gets a warning event instead. Services with the same name in different namespaces used to share one balancer: one of them keeps it, the others get a new balancer and a new IP. When they reconcile at the same moment, the others may configure the shared balancer and report its address once more before they get their own. A balancer without targets, for example because Hetzner refused every node, does not look like the service's. Under the old name it is replaced by a new balancer with a new IP and stays in the project, unlabelled and billed; under an annotated name the service gets a warning event. To hand an unlabelled balancer to a service yourself, label it: `hcloud load-balancer add-label '' robotlb/service-uid=`. While a service has no target nodes, for example a `Local` service without ready pods, robotlb cannot tell whether an unlabelled balancer is its own, so it waits and reports that in a warning event. +> +> A balancer is deleted only when it carries the label, so the balancer of a service deleted or changed from `LoadBalancer` before its first successful reconcile on this release stays in the project: a reconcile that stops early, for example because the service has no port to expose or no target nodes, adopts nothing. This includes services that earlier releases kept a balancer for after their type changed from `LoadBalancer` or they moved to another load balancer class: they still carry the `robotlb/finalizer` finalizer, robotlb removes it on the first start, and their balancers stay. List the balancers without the label, check which of them are still used, and delete the rest: +> +> ```bash +> hcloud load-balancer list --selector '!robotlb/service-uid' +> ``` +> +> The services that lose their finalizer this way can be listed before the upgrade: > > ```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)"' @@ -111,7 +119,7 @@ kind: Service metadata: name: target annotations: - # Custom name of the balancer to create on Hetzner. Defaults to service name. + # Name of the balancer to create on Hetzner. Defaults to .. robotlb/balancer: "custom name" # 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. diff --git a/src/consts.rs b/src/consts.rs index 0cedb34..5f5b01a 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -1,4 +1,6 @@ pub const LB_NAME_LABEL_NAME: &str = "robotlb/balancer"; +/// Hetzner label on every balancer robotlb manages: the UID of the service it serves. +pub const LB_OWNER_LABEL: &str = "robotlb/service-uid"; 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 573719b..e82953d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -26,6 +26,22 @@ pub enum RobotLBError { "No TCP port of the service has a nodePort, so the load balancer has nothing to forward" )] NoExposablePorts, + #[error( + "Load balancer '{name}' is labelled for the service with UID {owner}, so robotlb leaves it alone. Set another name through the robotlb/balancer annotation" + )] + ForeignBalancer { name: String, owner: String }, + #[error( + "Load balancer '{name}' has no {label} label and does not look like a balancer robotlb made for this service (IP targets only, at least one of them a node of the service), so robotlb leaves it alone. If it belongs to this service: hcloud load-balancer add-label '{name}' {label}={uid}", + label = crate::consts::LB_OWNER_LABEL + )] + UnrecognisedBalancer { name: String, uid: String }, + #[error( + "Load balancer '{0}' has no {label} label, and whether it belongs to this service cannot be told before the service has target nodes", + label = crate::consts::LB_OWNER_LABEL + )] + NoNodesToRecogniseBalancer(String), + #[error("More than one load balancer matches {0}")] + AmbiguousBalancer(String), #[error("Hetzner Cloud API rate limit reached, the pause ends in {}s", .0.as_millis().div_ceil(1000))] RateLimited(std::time::Duration), @@ -81,6 +97,10 @@ pub enum RobotLBError { HcloudLBChangeAlgorithm( #[from] hcloud::apis::Error, ), + #[error("Cannot label load balancer. Reason: {}", describe(.0))] + HcloudLBReplaceError( + #[from] hcloud::apis::Error, + ), #[error("Cannot list networks. Reason: {}", describe(.0))] HcloudListNetworksError( #[from] hcloud::apis::Error, @@ -109,6 +129,7 @@ impl RobotLBError { 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::HcloudLBReplaceError(error) => is_rate_limit_response(error), Self::HcloudListNetworksError(error) => is_rate_limit_response(error), Self::HcloudListLoadBalancersError(error) => is_rate_limit_response(error), Self::InvalidNodeFilter(_) @@ -121,6 +142,10 @@ impl RobotLBError { | Self::UnknownLBAlgorithm | Self::ServiceWithoutSelector | Self::NoExposablePorts + | Self::ForeignBalancer { .. } + | Self::UnrecognisedBalancer { .. } + | Self::NoNodesToRecogniseBalancer(_) + | Self::AmbiguousBalancer(_) | Self::RateLimited(_) => false, } } @@ -251,5 +276,37 @@ mod tests { fn other_statuses_are_not_a_rate_limit() { assert!(!RobotLBError::from(response_error(500, "")).is_rate_limited()); assert!(!RobotLBError::SkipService.is_rate_limited()); + assert!(!RobotLBError::AmbiguousBalancer("web".to_string()).is_rate_limited()); + } + + #[test] + fn an_unrecognised_balancer_names_the_handover_command() { + let error = RobotLBError::UnrecognisedBalancer { + name: "custom name".to_string(), + uid: "uid-1".to_string(), + }; + assert!(!error.is_rate_limited()); + assert!(error + .to_string() + .contains("hcloud load-balancer add-label 'custom name' robotlb/service-uid=uid-1")); + } + + // Relabelling would take the balancer from a service that may still use it. + #[test] + fn a_balancer_of_another_service_names_its_owner_and_no_handover() { + let error = RobotLBError::ForeignBalancer { + name: "web".to_string(), + owner: "uid-2".to_string(), + }; + assert!(!error.is_rate_limited()); + assert!(error.to_string().contains("uid-2")); + assert!(!error.to_string().contains("add-label")); + } + + #[test] + fn a_service_without_nodes_is_told_to_wait() { + let error = RobotLBError::NoNodesToRecogniseBalancer("web".to_string()); + assert!(!error.is_rate_limited()); + assert!(!error.to_string().contains("add-label")); } } diff --git a/src/lb.rs b/src/lb.rs index 2da9c40..f5d5949 100644 --- a/src/lb.rs +++ b/src/lb.rs @@ -5,15 +5,15 @@ use hcloud::{ AddServiceParams, AddTargetParams, AttachLoadBalancerToNetworkParams, ChangeAlgorithmParams, ChangeTypeOfLoadBalancerParams, DeleteLoadBalancerParams, DeleteServiceParams, DetachLoadBalancerFromNetworkParams, ListLoadBalancersParams, - RemoveTargetParams, UpdateServiceParams, + RemoveTargetParams, ReplaceLoadBalancerParams, UpdateServiceParams, }, networks_api::ListNetworksParams, }, models::{ - AttachLoadBalancerToNetworkRequest, ChangeTypeOfLoadBalancerRequest, DeleteServiceRequest, - DetachLoadBalancerFromNetworkRequest, LoadBalancerAddTarget, LoadBalancerAlgorithm, - LoadBalancerService, LoadBalancerServiceHealthCheck, RemoveTargetRequest, - UpdateLoadBalancerService, + load_balancer_target, AttachLoadBalancerToNetworkRequest, ChangeTypeOfLoadBalancerRequest, + DeleteServiceRequest, DetachLoadBalancerFromNetworkRequest, LoadBalancerAddTarget, + LoadBalancerAlgorithm, LoadBalancerService, LoadBalancerServiceHealthCheck, + RemoveTargetRequest, ReplaceLoadBalancerRequest, UpdateLoadBalancerService, }, }; use k8s_openapi::api::core::v1::Service; @@ -43,6 +43,9 @@ enum LBAlgorithm { #[derive(Debug)] pub struct LoadBalancer { pub name: String, + /// The name earlier releases gave the balancer, when it differs from `name`. + legacy_name: Option, + service_uid: String, pub services: HashMap, pub targets: Vec, pub private_ip: Option, @@ -127,11 +130,13 @@ impl LoadBalancer { .or(context.config.default_network.as_ref()) .cloned(); - let name = svc - .annotations() - .get(consts::LB_NAME_LABEL_NAME) + let annotated_name = svc.annotations().get(consts::LB_NAME_LABEL_NAME); + let legacy_name = annotated_name.is_none().then(|| svc.name_any()); + let name = annotated_name .cloned() - .unwrap_or(svc.name_any()); + .unwrap_or_else(|| default_name(&svc.name_any(), &svc.namespace().unwrap_or_default())); + // The API server sets the UID on every object it stores. + let service_uid = svc.uid().ok_or(RobotLBError::SkipService)?; let private_ip = svc .annotations() @@ -140,6 +145,8 @@ impl LoadBalancer { Ok(Self { name, + legacy_name, + service_uid, private_ip, balancer_type, check_interval, @@ -171,9 +178,11 @@ impl LoadBalancer { } /// Reconcile the load balancer to match the desired configuration. - #[tracing::instrument(skip(self), fields(lb_name=self.name))] + #[tracing::instrument(skip(self), fields(lb_name = tracing::field::Empty))] pub async fn reconcile(&self) -> RobotLBResult { let hcloud_balancer = self.get_or_create_hcloud_lb().await?; + // An adopted balancer keeps its name, which may differ from `self.name`. + tracing::Span::current().record("lb_name", hcloud_balancer.name.as_str()); self.reconcile_algorithm(&hcloud_balancer).await?; self.reconcile_lb_type(&hcloud_balancer).await?; self.reconcile_network(&hcloud_balancer).await?; @@ -382,7 +391,7 @@ impl LoadBalancer { if !planned.is_empty() && live == 0 { return Err(RobotLBError::HCloudError(format!( "No target could be added to load balancer {}: {}", - self.name, + hcloud_balancer.name, last_error.unwrap_or_else(|| "no reason reported".to_string()), ))); } @@ -517,9 +526,10 @@ impl LoadBalancer { Ok(()) } - /// Delete the balancer of the service, found by its name. + /// Delete the balancer labelled for the service. A balancer is never deleted by + /// its name: another service may have created one under it in the meantime. pub async fn cleanup(&self) -> RobotLBResult<()> { - let Some(hcloud_balancer) = self.get_hcloud_lb().await? else { + let Some(hcloud_balancer) = self.find_hcloud_lb(Purpose::Release).await? else { return Ok(()); }; hcloud::apis::load_balancers_api::delete_load_balancer( @@ -532,42 +542,92 @@ impl LoadBalancer { Ok(()) } - /// Get the load balancer from Hetzner Cloud. - /// This method will try to find the load balancer with the name - /// specified in the `LoadBalancer` struct. - /// - /// The method might return an error if the load balancer is not found - /// or if there are multiple load balancers with the same name. - async fn get_hcloud_lb(&self) -> RobotLBResult> { - let hcloud_balancers = hcloud::apis::load_balancers_api::list_load_balancers( - &self.hcloud_config, - ListLoadBalancersParams { - name: Some(self.name.to_string()), + /// Find the balancer of the service: the one labelled with its UID, or, when + /// reconciling, an unlabelled balancer from an earlier release that it adopts. + async fn find_hcloud_lb( + &self, + purpose: Purpose, + ) -> RobotLBResult> { + let selector = owner_selector(&self.service_uid); + let labelled = self + .list_hcloud_lbs(ListLoadBalancersParams { + label_selector: Some(selector.clone()), ..Default::default() + }) + .await?; + if let Some(balancer) = single(labelled, &selector)? { + return Ok(Some(balancer)); + } + for (name, legacy) in candidate_names(purpose, &self.name, self.legacy_name.as_deref()) { + let named = self + .list_hcloud_lbs(ListLoadBalancersParams { + name: Some(name.to_string()), + ..Default::default() + }) + .await?; + let Some(balancer) = single(named, name)? else { + continue; + }; + match decide(&balancer, &self.service_uid, &self.targets, legacy) { + Decision::Use => return Ok(Some(balancer)), + Decision::Adopt => return self.adopt(balancer).await.map(Some), + Decision::Skip => { + tracing::info!("Load balancer {name} is not this service's, skipping"); + } + Decision::Foreign(owner) => { + return Err(RobotLBError::ForeignBalancer { + name: name.to_string(), + owner, + }) + } + Decision::Unrecognised => { + return Err(RobotLBError::UnrecognisedBalancer { + name: name.to_string(), + uid: self.service_uid.clone(), + }) + } + Decision::NoNodes => { + return Err(RobotLBError::NoNodesToRecogniseBalancer(name.to_string())) + } + } + } + Ok(None) + } + + async fn list_hcloud_lbs( + &self, + params: ListLoadBalancersParams, + ) -> RobotLBResult> { + Ok( + hcloud::apis::load_balancers_api::list_load_balancers(&self.hcloud_config, params) + .await? + .load_balancers, + ) + } + + async fn adopt( + &self, + balancer: hcloud::models::LoadBalancer, + ) -> RobotLBResult { + tracing::info!("Adopting load balancer {}", balancer.name); + let response = hcloud::apis::load_balancers_api::replace_load_balancer( + &self.hcloud_config, + ReplaceLoadBalancerParams { + id: balancer.id, + replace_load_balancer_request: Some(ReplaceLoadBalancerRequest { + labels: Some(owner_labels(&balancer.labels, &self.service_uid)), + name: None, + }), }, ) .await?; - if hcloud_balancers.load_balancers.len() > 1 { - tracing::warn!( - "Found more than one balancer with name {}, skipping", - self.name - ); - return Err(RobotLBError::SkipService); - } - // Here we just return the first load balancer, - // if it exists, otherwise we return None - Ok(hcloud_balancers.load_balancers.into_iter().next()) + Ok(*response.load_balancer) } - /// Get or create the load balancer in Hetzner Cloud. - /// - /// this method will try to find the load balancer with the name - /// specified in the `LoadBalancer` struct. If the load balancer - /// is not found, the method will create a new load balancer - /// with the specified configuration in service's annotations. + /// Get or create the load balancer in Hetzner Cloud. A new balancer carries the + /// service UID label from the start. async fn get_or_create_hcloud_lb(&self) -> RobotLBResult { - let hcloud_lb = self.get_hcloud_lb().await?; - if let Some(balancer) = hcloud_lb { + if let Some(balancer) = self.find_hcloud_lb(Purpose::Reconcile).await? { return Ok(balancer); } @@ -576,7 +636,7 @@ impl LoadBalancer { hcloud::apis::load_balancers_api::CreateLoadBalancerParams { create_load_balancer_request: Some(hcloud::models::CreateLoadBalancerRequest { algorithm: Some(Box::new(self.algorithm.clone())), - labels: None, + labels: Some(owner_labels(&HashMap::new(), &self.service_uid)), load_balancer_type: self.balancer_type.clone(), location: Some(self.location.clone()), name: self.name.clone(), @@ -633,6 +693,105 @@ impl LoadBalancer { } } +/// Service names and namespaces are DNS labels: at most 63 characters and no dots, +/// so the result fits the 128 Hetzner allows and cannot be split two ways. +fn default_name(service: &str, namespace: &str) -> String { + format!("{service}.{namespace}") +} + +fn owner_selector(uid: &str) -> String { + format!("{}={uid}", consts::LB_OWNER_LABEL) +} + +/// Hetzner replaces the whole label set of a balancer, so the existing labels go along. +fn owner_labels(existing: &HashMap, uid: &str) -> HashMap { + let mut labels = existing.clone(); + labels.insert(consts::LB_OWNER_LABEL.to_string(), uid.to_string()); + labels +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Purpose { + Reconcile, + Release, +} + +/// Names to look a balancer up by when none carries the service UID, each marked +/// whether it is the legacy name. A release never goes by name. +fn candidate_names<'a>( + purpose: Purpose, + name: &'a str, + legacy_name: Option<&'a str>, +) -> Vec<(&'a str, bool)> { + if purpose == Purpose::Release { + return vec![]; + } + legacy_name + .map(|legacy| (legacy, true)) + .into_iter() + .chain([(name, false)]) + .collect() +} + +#[derive(Debug, PartialEq, Eq)] +enum Decision { + Use, + Adopt, + /// Look for the balancer under the next name. + Skip, + /// Labelled for the service with this UID. + Foreign(String), + Unrecognised, + /// Unlabelled, and the service has no nodes to compare its targets with. + NoNodes, +} + +/// Balancers of earlier releases carry no label. One is taken over only when it looks +/// like robotlb made it for this service: IP targets only, at least one of them a node +/// of the service. A balancer of another team or cluster points elsewhere. +fn decide( + balancer: &hcloud::models::LoadBalancer, + uid: &str, + desired_targets: &[String], + legacy: bool, +) -> Decision { + match balancer.labels.get(consts::LB_OWNER_LABEL) { + Some(owner) if owner == uid => return Decision::Use, + Some(_) if legacy => return Decision::Skip, + Some(owner) => return Decision::Foreign(owner.clone()), + None => {} + } + let ip_only = balancer + .targets + .iter() + .all(|target| target.r#type == load_balancer_target::Type::Ip); + if ip_only && desired_targets.is_empty() { + // Skipping an old balancer here would replace it with a new one and a new + // address while the service merely waits for its pods. + return Decision::NoNodes; + } + let on_service_nodes = balancer.targets.iter().any(|target| { + target + .ip + .as_ref() + .is_some_and(|ip| desired_targets.contains(&ip.ip)) + }); + if ip_only && on_service_nodes { + Decision::Adopt + } else if legacy { + Decision::Skip + } else { + Decision::Unrecognised + } +} + +fn single(mut found: Vec, what: &str) -> RobotLBResult> { + if found.len() > 1 { + return Err(RobotLBError::AmbiguousBalancer(what.to_string())); + } + Ok(found.pop()) +} + /// 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. @@ -669,7 +828,16 @@ impl From for LoadBalancerAlgorithm { #[cfg(test)] mod tests { - use super::plan_targets; + use super::{ + candidate_names, decide, default_name, owner_labels, owner_selector, plan_targets, single, + Decision, Purpose, + }; + use crate::{consts, error::RobotLBError}; + use hcloud::models::{ + load_balancer_target, LoadBalancer as HcloudBalancer, LoadBalancerTarget, + LoadBalancerTargetIp, + }; + use std::collections::HashMap; #[test] fn targets_are_sorted_and_deduplicated() { @@ -700,4 +868,139 @@ 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); } + + #[test] + fn the_default_name_carries_the_namespace() { + assert_eq!(default_name("web", "shop"), "web.shop"); + assert_ne!(default_name("web", "shop"), default_name("web", "blog")); + } + + // Both parts are DNS labels of at most 63 characters, Hetzner takes 128. + #[test] + fn the_longest_default_name_fits_hetzner() { + let part = "a".repeat(63); + assert_eq!(default_name(&part, &part).len(), 127); + } + + #[test] + fn the_owner_selector_matches_the_service_uid() { + assert_eq!(owner_selector("uid-1"), "robotlb/service-uid=uid-1"); + } + + // Hetzner replaces the whole label set, so labels set by others must be sent back. + #[test] + fn owner_labels_keep_existing_labels() { + let existing = HashMap::from([ + ("team".to_string(), "web".to_string()), + (consts::LB_OWNER_LABEL.to_string(), "uid-2".to_string()), + ]); + let labels = owner_labels(&existing, "uid-1"); + assert_eq!(labels.len(), 2); + assert_eq!(labels["team"], "web"); + assert_eq!(labels[consts::LB_OWNER_LABEL], "uid-1"); + } + + fn ip_target(ip: &str) -> LoadBalancerTarget { + LoadBalancerTarget { + r#type: load_balancer_target::Type::Ip, + ip: Some(Box::new(LoadBalancerTargetIp { ip: ip.to_string() })), + ..Default::default() + } + } + + fn balancer(owner: Option<&str>, targets: Vec) -> HcloudBalancer { + HcloudBalancer { + labels: owner + .map(|uid| HashMap::from([(consts::LB_OWNER_LABEL.to_string(), uid.to_string())])) + .unwrap_or_default(), + targets, + ..Default::default() + } + } + + fn nodes() -> Vec { + vec!["192.0.2.1".to_string(), "192.0.2.2".to_string()] + } + + #[test] + fn a_balancer_labelled_for_the_service_is_used() { + let lb = balancer(Some("uid-1"), vec![]); + assert_eq!(decide(&lb, "uid-1", &nodes(), true), Decision::Use); + assert_eq!(decide(&lb, "uid-1", &nodes(), false), Decision::Use); + } + + #[test] + fn a_balancer_labelled_for_another_service_is_never_taken() { + let lb = balancer(Some("uid-2"), vec![ip_target("192.0.2.1")]); + assert_eq!(decide(&lb, "uid-1", &nodes(), true), Decision::Skip); + assert_eq!( + decide(&lb, "uid-1", &nodes(), false), + Decision::Foreign("uid-2".to_string()) + ); + } + + #[test] + fn an_unlabelled_balancer_on_the_service_nodes_is_adopted() { + let lb = balancer(None, vec![ip_target("192.0.2.9"), ip_target("192.0.2.2")]); + assert_eq!(decide(&lb, "uid-1", &nodes(), true), Decision::Adopt); + assert_eq!(decide(&lb, "uid-1", &nodes(), false), Decision::Adopt); + } + + #[test] + fn an_unlabelled_balancer_elsewhere_is_not_adopted() { + let lb = balancer(None, vec![ip_target("198.51.100.1")]); + assert_eq!(decide(&lb, "uid-1", &nodes(), true), Decision::Skip); + assert_eq!( + decide(&lb, "uid-1", &nodes(), false), + Decision::Unrecognised + ); + let empty = balancer(None, vec![]); + assert_eq!(decide(&empty, "uid-1", &nodes(), true), Decision::Skip); + } + + // robotlb only ever adds IP targets. + #[test] + fn an_unlabelled_balancer_with_server_targets_is_not_adopted() { + let server = LoadBalancerTarget { + r#type: load_balancer_target::Type::Server, + ..Default::default() + }; + let lb = balancer(None, vec![ip_target("192.0.2.1"), server]); + assert_eq!( + decide(&lb, "uid-1", &nodes(), false), + Decision::Unrecognised + ); + // No node could make it robotlb's, so a service without nodes need not wait. + assert_eq!(decide(&lb, "uid-1", &[], true), Decision::Skip); + } + + #[test] + fn an_unlabelled_balancer_is_not_skipped_while_the_service_has_no_nodes() { + let lb = balancer(None, vec![ip_target("192.0.2.1")]); + assert_eq!(decide(&lb, "uid-1", &[], true), Decision::NoNodes); + assert_eq!(decide(&lb, "uid-1", &[], false), Decision::NoNodes); + } + + #[test] + fn a_release_never_looks_a_balancer_up_by_name() { + assert!(candidate_names(Purpose::Release, "web.shop", Some("web")).is_empty()); + assert_eq!( + candidate_names(Purpose::Reconcile, "web.shop", Some("web")), + vec![("web", true), ("web.shop", false)] + ); + assert_eq!( + candidate_names(Purpose::Reconcile, "custom", None), + vec![("custom", false)] + ); + } + + #[test] + fn more_than_one_match_is_an_error() { + assert!(single(Vec::::new(), "x").unwrap().is_none()); + assert_eq!(single(vec![1], "x").unwrap(), Some(1)); + assert!(matches!( + single(vec![1, 2], "x"), + Err(RobotLBError::AmbiguousBalancer(_)) + )); + } } diff --git a/src/main.rs b/src/main.rs index e3c4475..cae620c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -895,6 +895,20 @@ mod tests { assert!(publishes_event(&RobotLBError::HCloudError( "boom".to_string() ))); + assert!(publishes_event(&RobotLBError::UnrecognisedBalancer { + name: "web".to_string(), + uid: "uid-1".to_string(), + })); + assert!(publishes_event(&RobotLBError::ForeignBalancer { + name: "web".to_string(), + owner: "uid-2".to_string(), + })); + assert!(publishes_event(&RobotLBError::NoNodesToRecogniseBalancer( + "web".to_string() + ))); + assert!(publishes_event(&RobotLBError::AmbiguousBalancer( + "web".to_string() + ))); } fn owned_service(type_: &str, class: Option<&str>, deleting: bool) -> Service {