diff --git a/src/consts.rs b/src/consts.rs index cb001df..7155e1d 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -4,32 +4,32 @@ //! for load balancer configuration. /// Annotation key for specifying the load balancer name. -pub const LB_NAME_LABEL_NAME: &str = "robotlb/balancer"; +pub const LB_NAME_ANNOTATION: &str = "robotlb/balancer"; /// Annotation key for custom node selector. -pub const LB_NODE_SELECTOR: &str = "robotlb/node-selector"; +pub const LB_NODE_SELECTOR_ANNOTATION: &str = "robotlb/node-selector"; /// Annotation key for specifying node IP. -pub const LB_NODE_IP_LABEL_NAME: &str = "robotlb/node-ip"; +pub const LB_NODE_IP_ANNOTATION: &str = "robotlb/node-ip"; // LB config /// Annotation key for health check interval (seconds). -pub const LB_CHECK_INTERVAL_ANN_NAME: &str = "robotlb/lb-check-interval"; +pub const LB_CHECK_INTERVAL_ANNOTATION: &str = "robotlb/lb-check-interval"; /// Annotation key for health check timeout (seconds). -pub const LB_TIMEOUT_ANN_NAME: &str = "robotlb/lb-timeout"; +pub const LB_TIMEOUT_ANNOTATION: &str = "robotlb/lb-timeout"; /// Annotation key for health check retries. -pub const LB_RETRIES_ANN_NAME: &str = "robotlb/lb-retries"; +pub const LB_RETRIES_ANNOTATION: &str = "robotlb/lb-retries"; /// Annotation key for enabling proxy mode. -pub const LB_PROXY_MODE_LABEL_NAME: &str = "robotlb/lb-proxy-mode"; +pub const LB_PROXY_MODE_ANNOTATION: &str = "robotlb/lb-proxy-mode"; /// Annotation key for network name. -pub const LB_NETWORK_LABEL_NAME: &str = "robotlb/lb-network"; +pub const LB_NETWORK_ANNOTATION: &str = "robotlb/lb-network"; /// Annotation key for private IP mode. -pub const LB_PRIVATE_IP_LABEL_NAME: &str = "robotlb/lb-private-ip"; +pub const LB_PRIVATE_IP_ANNOTATION: &str = "robotlb/lb-private-ip"; /// Annotation key for load balancer location. -pub const LB_LOCATION_LABEL_NAME: &str = "robotlb/lb-location"; +pub const LB_LOCATION_ANNOTATION: &str = "robotlb/lb-location"; /// Annotation key for load balancing algorithm. -pub const LB_ALGORITHM_LABEL_NAME: &str = "robotlb/lb-algorithm"; +pub const LB_ALGORITHM_ANNOTATION: &str = "robotlb/lb-algorithm"; /// Annotation key for load balancer type. -pub const LB_BALANCER_TYPE_LABEL_NAME: &str = "robotlb/balancer-type"; +pub const LB_BALANCER_TYPE_ANNOTATION: &str = "robotlb/balancer-type"; /// Default number of health check retries. pub const DEFAULT_LB_RETRIES: i32 = 3; diff --git a/src/controller/mod.rs b/src/controller/mod.rs index 7d28679..ed12b8f 100644 --- a/src/controller/mod.rs +++ b/src/controller/mod.rs @@ -233,35 +233,7 @@ pub async fn run( #[cfg(test)] mod tests { use super::*; - use k8s_openapi::{ - api::core::v1::{ServicePort, ServiceSpec}, - apimachinery::pkg::apis::meta::v1::ObjectMeta, - }; - - fn service_with_spec(spec: ServiceSpec) -> Service { - Service { - metadata: ObjectMeta { - name: Some("svc".to_string()), - namespace: Some("default".to_string()), - ..Default::default() - }, - spec: Some(spec), - ..Default::default() - } - } - - fn service_spec( - service_type: &str, - lb_class: Option<&str>, - ports: Vec, - ) -> ServiceSpec { - ServiceSpec { - type_: Some(service_type.to_string()), - load_balancer_class: lb_class.map(str::to_string), - ports: Some(ports), - ..Default::default() - } - } + use crate::test_utils::fixtures::{service_spec, service_with_spec}; #[test] fn service_filter_rejects_non_load_balancer_type() { diff --git a/src/controller/nodes.rs b/src/controller/nodes.rs index d0c2623..ee3a766 100644 --- a/src/controller/nodes.rs +++ b/src/controller/nodes.rs @@ -168,7 +168,7 @@ async fn get_nodes_by_selector( ) -> RobotLBResult> { let node_selector = svc .annotations() - .get(consts::LB_NODE_SELECTOR) + .get(consts::LB_NODE_SELECTOR_ANNOTATION) .map(String::as_str) .ok_or(RobotLBError::ServiceWithoutSelector)?; let label_filter = LabelFilter::from_str(node_selector)?; @@ -185,35 +185,8 @@ async fn get_nodes_by_selector( #[cfg(test)] mod tests { use super::*; - use k8s_openapi::{ - api::core::v1::{NodeAddress, NodeStatus, ServicePort, ServiceSpec}, - apimachinery::pkg::apis::meta::v1::ObjectMeta, - }; - - fn service_with_spec(spec: ServiceSpec) -> Service { - Service { - metadata: ObjectMeta { - name: Some("svc".to_string()), - namespace: Some("default".to_string()), - ..Default::default() - }, - spec: Some(spec), - ..Default::default() - } - } - - fn service_spec( - service_type: &str, - lb_class: Option<&str>, - ports: Vec, - ) -> ServiceSpec { - ServiceSpec { - type_: Some(service_type.to_string()), - load_balancer_class: lb_class.map(str::to_string), - ports: Some(ports), - ..Default::default() - } - } + use crate::test_utils::fixtures::{service_spec, service_with_spec}; + use k8s_openapi::api::core::v1::{NodeAddress, NodeStatus, ServicePort}; #[test] fn derive_targets_picks_matching_address_type() { diff --git a/src/lb/config.rs b/src/lb/config.rs index 35b4298..a50521b 100644 --- a/src/lb/config.rs +++ b/src/lb/config.rs @@ -30,26 +30,26 @@ pub fn parse_load_balancer_config( config: &OperatorConfig, ) -> RobotLBResult { let retries = - parse_annotation(svc, consts::LB_RETRIES_ANN_NAME)?.unwrap_or(config.default_lb_retries); + parse_annotation(svc, consts::LB_RETRIES_ANNOTATION)?.unwrap_or(config.default_lb_retries); let timeout = - parse_annotation(svc, consts::LB_TIMEOUT_ANN_NAME)?.unwrap_or(config.default_lb_timeout); + parse_annotation(svc, consts::LB_TIMEOUT_ANNOTATION)?.unwrap_or(config.default_lb_timeout); - let check_interval = parse_annotation(svc, consts::LB_CHECK_INTERVAL_ANN_NAME)? + let check_interval = parse_annotation(svc, consts::LB_CHECK_INTERVAL_ANNOTATION)? .unwrap_or(config.default_lb_interval); - let proxy_mode = parse_annotation(svc, consts::LB_PROXY_MODE_LABEL_NAME)? + let proxy_mode = parse_annotation(svc, consts::LB_PROXY_MODE_ANNOTATION)? .unwrap_or(config.default_lb_proxy_mode_enabled); let location = svc .annotations() - .get(consts::LB_LOCATION_LABEL_NAME) + .get(consts::LB_LOCATION_ANNOTATION) .cloned() .unwrap_or_else(|| config.default_lb_location.clone()); let balancer_type = svc .annotations() - .get(consts::LB_BALANCER_TYPE_LABEL_NAME) + .get(consts::LB_BALANCER_TYPE_ANNOTATION) .cloned() .unwrap_or_else(|| config.default_balancer_type.clone()); @@ -57,19 +57,19 @@ pub fn parse_load_balancer_config( let network_name = svc .annotations() - .get(consts::LB_NETWORK_LABEL_NAME) + .get(consts::LB_NETWORK_ANNOTATION) .or(config.default_network.as_ref()) .cloned(); let name = svc .annotations() - .get(consts::LB_NAME_LABEL_NAME) + .get(consts::LB_NAME_ANNOTATION) .cloned() .unwrap_or_else(|| svc.name_any()); let private_ip = svc .annotations() - .get(consts::LB_PRIVATE_IP_LABEL_NAME) + .get(consts::LB_PRIVATE_IP_ANNOTATION) .cloned(); Ok(ParsedLoadBalancerConfig { @@ -103,7 +103,7 @@ where /// Parse the algorithm annotation or fall back to default. fn parse_algorithm(svc: &Service, config: &OperatorConfig) -> RobotLBResult { svc.annotations() - .get(consts::LB_ALGORITHM_LABEL_NAME) + .get(consts::LB_ALGORITHM_ANNOTATION) .map(String::as_str) .or(Some(&config.default_lb_algorithm)) .map(LBAlgorithm::from_str) @@ -184,16 +184,16 @@ mod tests { let mut config = base_config(); config.default_network = None; let svc = service_with_annotations([ - (consts::LB_NAME_LABEL_NAME, "custom-lb"), - (consts::LB_RETRIES_ANN_NAME, "5"), - (consts::LB_TIMEOUT_ANN_NAME, "8"), - (consts::LB_CHECK_INTERVAL_ANN_NAME, "20"), - (consts::LB_PROXY_MODE_LABEL_NAME, "true"), - (consts::LB_LOCATION_LABEL_NAME, "nbg1"), - (consts::LB_BALANCER_TYPE_LABEL_NAME, "lb31"), - (consts::LB_ALGORITHM_LABEL_NAME, "round-robin"), - (consts::LB_NETWORK_LABEL_NAME, "private-net"), - (consts::LB_PRIVATE_IP_LABEL_NAME, "10.10.0.5"), + (consts::LB_NAME_ANNOTATION, "custom-lb"), + (consts::LB_RETRIES_ANNOTATION, "5"), + (consts::LB_TIMEOUT_ANNOTATION, "8"), + (consts::LB_CHECK_INTERVAL_ANNOTATION, "20"), + (consts::LB_PROXY_MODE_ANNOTATION, "true"), + (consts::LB_LOCATION_ANNOTATION, "nbg1"), + (consts::LB_BALANCER_TYPE_ANNOTATION, "lb31"), + (consts::LB_ALGORITHM_ANNOTATION, "round-robin"), + (consts::LB_NETWORK_ANNOTATION, "private-net"), + (consts::LB_PRIVATE_IP_ANNOTATION, "10.10.0.5"), ]); let parsed = parse_load_balancer_config(&svc, &config).expect("parse should succeed"); @@ -216,7 +216,7 @@ mod tests { #[test] fn returns_error_for_invalid_algorithm_annotation() { let config = base_config(); - let svc = service_with_annotations([(consts::LB_ALGORITHM_LABEL_NAME, "weighted")]); + let svc = service_with_annotations([(consts::LB_ALGORITHM_ANNOTATION, "weighted")]); let result = parse_load_balancer_config(&svc, &config); assert!(matches!(result, Err(RobotLBError::UnknownLBAlgorithm))); diff --git a/src/lb/mod.rs b/src/lb/mod.rs index c0a72cf..d0d4a2b 100644 --- a/src/lb/mod.rs +++ b/src/lb/mod.rs @@ -27,10 +27,7 @@ use crate::{ pub(crate) use api::{HcloudLoadBalancerApi, LiveHcloudLoadBalancerApi}; pub(crate) use config::parse_load_balancer_config; -pub use types::LBService; -pub(crate) use types::{ - ServiceReconcileAction, TargetReconcileAction, normalize_ip, service_matches_desired, -}; +use types::{ServiceReconcileAction, TargetReconcileAction, normalize_ip, service_matches_desired}; /// Struct representing a load balancer. /// @@ -412,45 +409,17 @@ impl LoadBalancer { &self, hcloud_balancer: &hcloud::models::LoadBalancer, ) -> RobotLBResult<()> { - // If the network name is not provided, and load balancer is not attached to any network, - // we can skip this step. if self.network_name.is_none() && hcloud_balancer.private_net.is_empty() { return Ok(()); } let desired_network = self.get_network().await?.map(|network| network.id); - // If the network name is not provided, but the load balancer is attached to a network, - // we need to detach it from the network. - let mut contain_desired_network = false; - if !hcloud_balancer.private_net.is_empty() { - for private_net in &hcloud_balancer.private_net { - let Some(private_net_id) = private_net.network else { - continue; - }; - // The load balancer is attached to a target network. - if desired_network == Some(private_net_id) { - // Specific IP was provided, we need to check if the IP is the same. - if self.private_ip.is_some() { - // if IPs match, we can leave everything as it is. - if private_net.ip == self.private_ip { - contain_desired_network = true; - continue; - } - } else { - // No specific IP was provided, we can leave everything as it is. - contain_desired_network = true; - continue; - } - } - tracing::info!("Detaching balancer from network {}", private_net_id); - api::detach_from_network(&self.hcloud_config, hcloud_balancer.id, private_net_id) - .await?; - } - } - if !contain_desired_network { - let Some(network_id) = desired_network else { - return Ok(()); - }; + + let is_attached = self + .detach_unwanted_networks(hcloud_balancer, desired_network) + .await?; + + if !is_attached && let Some(network_id) = desired_network { tracing::info!("Attaching balancer to network {}", network_id); api::attach_to_network( &self.hcloud_config, @@ -463,6 +432,42 @@ impl LoadBalancer { Ok(()) } + async fn detach_unwanted_networks( + &self, + hcloud_balancer: &hcloud::models::LoadBalancer, + desired_network: Option, + ) -> RobotLBResult { + let mut is_attached = false; + + for private_net in &hcloud_balancer.private_net { + let Some(private_net_id) = private_net.network else { + continue; + }; + + if desired_network == Some(private_net_id) + && private_net + .ip + .as_ref() + .is_some_and(|ip| self.matches_desired_ip(ip)) + { + is_attached = true; + continue; + } + + tracing::info!("Detaching balancer from network {}", private_net_id); + api::detach_from_network(&self.hcloud_config, hcloud_balancer.id, private_net_id) + .await?; + } + + Ok(is_attached) + } + + fn matches_desired_ip(&self, current_ip: &str) -> bool { + self.private_ip + .as_ref() + .is_none_or(|desired_ip| desired_ip == current_ip) + } + /// Cleanup the load balancer. /// /// This method will remove all the services and targets from the diff --git a/src/lb/types.rs b/src/lb/types.rs index 8f58e7f..877f79a 100644 --- a/src/lb/types.rs +++ b/src/lb/types.rs @@ -5,17 +5,8 @@ use hcloud::models::{LoadBalancerAlgorithm, LoadBalancerService}; -/// Represents a service exposed by the load balancer. -#[derive(Debug)] -pub struct LBService { - /// Port the load balancer listens on. - pub listen_port: i32, - /// Port on the target servers to forward traffic to. - pub target_port: i32, -} - /// Load balancing algorithm types. -pub enum LBAlgorithm { +pub(super) enum LBAlgorithm { /// Round-robin load balancing. RoundRobin, /// Least connections load balancing. @@ -47,22 +38,23 @@ impl From for LoadBalancerAlgorithm { /// Parsed configuration for a load balancer. #[derive(Debug)] -pub struct ParsedLoadBalancerConfig { - pub(crate) name: String, - pub(crate) private_ip: Option, - pub(crate) balancer_type: String, - pub(crate) check_interval: i32, - pub(crate) timeout: i32, - pub(crate) retries: i32, - pub(crate) location: String, - pub(crate) proxy_mode: bool, - pub(crate) network_name: Option, - pub(crate) algorithm: LoadBalancerAlgorithm, +#[allow(clippy::redundant_pub_crate)] +pub(crate) struct ParsedLoadBalancerConfig { + pub(super) name: String, + pub(super) private_ip: Option, + pub(super) balancer_type: String, + pub(super) check_interval: i32, + pub(super) timeout: i32, + pub(super) retries: i32, + pub(super) location: String, + pub(super) proxy_mode: bool, + pub(super) network_name: Option, + pub(super) algorithm: LoadBalancerAlgorithm, } /// Actions that can be performed on a load balancer service. #[derive(Debug, Clone, PartialEq, Eq)] -pub enum ServiceReconcileAction { +pub(super) enum ServiceReconcileAction { /// Update an existing service configuration. Update { listen_port: i32, @@ -79,7 +71,7 @@ pub enum ServiceReconcileAction { /// Actions that can be performed on a load balancer target. #[derive(Debug, Clone, PartialEq, Eq)] -pub enum TargetReconcileAction { +pub(super) enum TargetReconcileAction { /// Remove a target from the load balancer. Remove { target_ip: String }, /// Add a target to the load balancer. @@ -88,7 +80,7 @@ pub enum TargetReconcileAction { /// Check if a service matches the desired configuration. #[must_use] -pub fn service_matches_desired( +pub(super) fn service_matches_desired( service: &LoadBalancerService, destination_port: i32, check_interval: i32, @@ -108,6 +100,6 @@ pub fn service_matches_desired( } /// Normalize an IP address by removing any CIDR suffix. -pub fn normalize_ip(ip: &str) -> String { +pub(super) fn normalize_ip(ip: &str) -> String { ip.split('/').next().unwrap_or(ip).to_string() } diff --git a/src/main.rs b/src/main.rs index 3caaaec..df63ab9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -53,6 +53,9 @@ pub mod metrics; pub mod otel_tracing; pub mod prometheus_exporter; +#[cfg(test)] +pub mod test_utils; + /// Shared context for the operator. #[derive(Clone)] pub struct CurrentContext { diff --git a/src/metrics.rs b/src/metrics.rs index 97c5e96..0fc5ea2 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -1,3 +1,8 @@ +//! Metrics collection for the robotlb operator. +//! +//! This module provides Prometheus-compatible metrics for monitoring +//! the operator's performance and behavior. + use std::{sync::Arc, time::Instant}; use opentelemetry::{ @@ -9,6 +14,9 @@ use tracing::debug; use crate::consts; +/// Metrics collector for the robotlb operator. +/// +/// Tracks reconciliation operations, failures, duration, and API interactions. pub struct Metrics { controller: String, reconcile_operations: Counter, @@ -21,6 +29,7 @@ pub struct Metrics { } impl Metrics { + /// Create a new metrics instance with the given OpenTelemetry meter. pub fn new(meter: &Meter) -> Self { debug!("Initializing robotlb metrics"); @@ -120,6 +129,9 @@ impl Metrics { } } +/// RAII guard for measuring reconcile duration. +/// +/// Automatically records the duration when dropped. pub struct ReconcileMeasurer { start: TokioInstant, controller: String, @@ -136,6 +148,9 @@ impl Drop for ReconcileMeasurer { } } +/// Timer for tracking reconcile operation success/failure. +/// +/// Records metrics on drop, including failure status. pub struct ReconcileTimer { metrics: Arc, start: Instant, diff --git a/src/test_utils.rs b/src/test_utils.rs new file mode 100644 index 0000000..5e3a477 --- /dev/null +++ b/src/test_utils.rs @@ -0,0 +1,38 @@ +//! Shared test utilities for the robotlb operator. +//! +//! This module provides common test helpers to reduce duplication +//! across test modules. + +#[cfg(test)] +pub mod fixtures { + use k8s_openapi::{ + api::core::v1::{Service, ServicePort, ServiceSpec}, + apimachinery::pkg::apis::meta::v1::ObjectMeta, + }; + + #[must_use] + pub fn service_with_spec(spec: ServiceSpec) -> Service { + Service { + metadata: ObjectMeta { + name: Some("svc".to_string()), + namespace: Some("default".to_string()), + ..Default::default() + }, + spec: Some(spec), + ..Default::default() + } + } + + pub fn service_spec( + service_type: &str, + lb_class: Option<&str>, + ports: Vec, + ) -> ServiceSpec { + ServiceSpec { + type_: Some(service_type.to_string()), + load_balancer_class: lb_class.map(str::to_string), + ports: Some(ports), + ..Default::default() + } + } +}