fix(controller): skip Hetzner calls when nothing robotlb acts on changed

README.md

@@ -35,10 +35,14 @@ After the chart is installed, you should be able to create `LoadBalancer` servic
 
 The operator listens to the Kubernetes API for services of type `LoadBalancer` and creates Hetzner load balancers that point to nodes based on `node-ip`.
 
+robotlb handles every `LoadBalancer` service that has no `spec.loadBalancerClass` or has it set to `robotlb`, and ignores services of any other class. Another LoadBalancer controller that also takes services without a class, such as MetalLB started without `--lb-class`, handles the same services: the two keep overwriting each other's `status.loadBalancer`, and every status write starts another reconcile. Set `loadBalancerClass: robotlb` on the services robotlb should handle to keep other controllers away from them. Kubernetes lets you set the field only when the service is created or its type is changed to `LoadBalancer`. Recreating a service deletes its Hetzner balancer, so the service gets a new balancer with a new public IP. Set the class when the service is created.
+
+A balancer is updated when its service changes, when a node is added, removed, relabelled, cordoned or changes readiness or addresses, and, for services with `externalTrafficPolicy: Local` while `ROBOTLB_DYNAMIC_NODE_SELECTOR` is on, when the nodes their endpoints serve traffic from change; a deleted endpoint slice of such a service rechecks every balancer. Apart from that robotlb checks each balancer every `ROBOTLB_RESYNC_INTERVAL` seconds, 300 by default, which bounds how long a change made to the balancer in Hetzner survives. Each check costs one or two Hetzner API requests per service. A reconcile that runs before the next check is due, and finds the spec, the `robotlb/` annotations, the targets and the ports as the last successful reconcile left them, makes no Hetzner requests and does not write the status, so a change made in Hetzner, or a status another controller cleared, stays until that check. When Hetzner refuses a target for a reason other than the rate limit, the service is checked again within 30 seconds instead, so a node it refuses for good, such as one outside the vSwitch subnet, keeps its service on that 30-second cycle.
+
 Target nodes are selected according to the service's `externalTrafficPolicy`:
 
 - `Cluster`, the Kubernetes default: every node of the cluster becomes a target, since kube-proxy forwards the traffic to a node that hosts a pod. Cordoned and not-ready nodes are left out, as they would only take up target slots.
-- `Local`: only the nodes where the service's target pods run, found through the service selector, or through the service's `EndpointSlice` resources when it has no selector.
+- `Local`: only the nodes kube-proxy serves the traffic from, read from the service's `EndpointSlice` resources whether or not the service has a selector. These are the nodes with a ready endpoint, or with a terminating endpoint that still serves while the node has no ready one. Endpoint conditions decide, not whether a pod runs on the node: a node whose pods are not ready is left out, and a cordoned or not-ready node stays a target while it has such an endpoint.
 
 Nodes labelled `node.kubernetes.io/exclude-from-external-load-balancers`, which kubeadm puts on control-plane nodes, stay out of every balancer under either policy.
 
@@ -70,7 +74,7 @@ Options:
       --default-network <DEFAULT_NETWORK>
           Default network to use for load balancers. If not set, then only network from the service annotation will be used [env: ROBOTLB_DEFAULT_NETWORK=]
       --dynamic-node-selector
-          If enabled, the operator will try to find target nodes based on where the target pods are actually deployed. If disabled, the operator will try to find target nodes based on the node selector [env: ROBOTLB_DYNAMIC_NODE_SELECTOR=]
+          If enabled, the operator will try to find target nodes based on the service's traffic policy, and under `Local` on the nodes serving the service's endpoints. If disabled, the operator will try to find target nodes based on the node selector [env: ROBOTLB_DYNAMIC_NODE_SELECTOR=]
       --default-lb-retries <DEFAULT_LB_RETRIES>
           Default load balancer healthcheck retries cound [env: ROBOTLB_DEFAULT_LB_RETRIES=] [default: 3]
       --default-lb-timeout <DEFAULT_LB_TIMEOUT>
@@ -87,6 +91,8 @@ Options:
           Default load balancer proxy mode. If enabled, the load balancer will act as a proxy for the target servers. The default value is `false`. https://docs.hetzner.com/cloud/load-balancers/faq/#what-does-proxy-protocol-mean-and-should-i-enable-it [env: ROBOTLB_DEFAULT_LB_PROXY_MODE_ENABLED=]
       --ipv6-ingress
           Whether to enable IPv6 ingress for the load balancer. If enabled, the load balancer's IPv6 will be attached to the service as an external IP along with IPv4 [env: ROBOTLB_IPV6_INGRESS=]
+      --resync-interval <RESYNC_INTERVAL>
+          Seconds between reconciliations of a service that nothing changed. Node changes, and endpoint changes of Local services, trigger a reconciliation on their own; this interval bounds how long a change made to a balancer outside robotlb survives. A service whose balancer refused a target is retried within 30 seconds [env: ROBOTLB_RESYNC_INTERVAL=] [default: 300]
       --log-level <LOG_LEVEL>
           [env: ROBOTLB_LOG_LEVEL=] [default: INFO]
   -h, --help
@@ -142,8 +148,9 @@ metadata:
     robotlb/balancer-type: "lb11"
 spec:
   type: LoadBalancer
-  # If dynamic node selector is enabled, nodes will be found
-  # using this property.
+  # The selector fills the service's endpoints; with
+  # externalTrafficPolicy: Local and the dynamic node selector
+  # enabled, the nodes serving them become the targets.
   selector:
     app: target
   ports:

helm/values.yaml

@@ -28,17 +28,20 @@ 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]
     verbs: [get, list, patch, update, watch]
   - apiGroups: [""]
-    resources: [nodes, pods]
+    resources: [nodes]
     verbs: [get, list, watch]
   - apiGroups: [discovery.k8s.io]
     resources: [endpointslices]
     verbs: [get, list, watch]
+  - apiGroups: [events.k8s.io]
+    resources: [events]
+    verbs: [create]
 
 podAnnotations: {}
 podLabels: {}

src/config.rs

@@ -12,7 +12,7 @@ pub struct OperatorConfig {
     #[arg(long, env = "ROBOTLB_DEFAULT_NETWORK", default_value = None)]
     pub default_network: Option<String>,
 
-    /// If enabled, the operator will try to find target nodes based on where the target pods are actually deployed.
+    /// If enabled, the operator will try to find target nodes based on the service's traffic policy, and under `Local` on the nodes serving the service's endpoints.
     /// If disabled, the operator will try to find target nodes based on the node selector.
     #[arg(long, env = "ROBOTLB_DYNAMIC_NODE_SELECTOR", default_value = "true")]
     pub dynamic_node_selector: bool,
@@ -67,7 +67,46 @@ pub struct OperatorConfig {
     #[arg(long, env = "ROBOTLB_IPV6_INGRESS", default_value = "false")]
     pub ipv6_ingress: bool,
 
+    /// Seconds between reconciliations of a service that nothing changed. Node changes,
+    /// and endpoint changes of Local services, trigger a reconciliation on their own;
+    /// this interval bounds how long a change made to a balancer outside robotlb survives.
+    /// A service whose balancer refused a target is retried within 30 seconds.
+    #[arg(
+        long,
+        env = "ROBOTLB_RESYNC_INTERVAL",
+        default_value = "300",
+        value_parser = clap::value_parser!(u64).range(1..=31_536_000)
+    )]
+    pub resync_interval: u64,
+
     // Log level of the operator.
     #[arg(long, env = "ROBOTLB_LOG_LEVEL", default_value = "INFO")]
     pub log_level: LevelFilter,
 }
+
+#[cfg(test)]
+mod tests {
+    use super::OperatorConfig;
+    use clap::Parser;
+
+    fn parse(resync: &str) -> Result<OperatorConfig, clap::Error> {
+        OperatorConfig::try_parse_from([
+            "robotlb",
+            "--hcloud-token",
+            "t",
+            "--resync-interval",
+            resync,
+        ])
+    }
+
+    // Zero would requeue every successful reconcile right away, spending Hetzner
+    // API requests on every service all the time. The controller's delay queue
+    // panics on delays past about two years.
+    #[test]
+    fn the_resync_interval_must_be_between_a_second_and_a_year() {
+        assert!(parse("0").is_err());
+        assert!(parse("31536001").is_err());
+        assert_eq!(parse("31536000").unwrap().resync_interval, 31_536_000);
+        assert_eq!(parse("1").unwrap().resync_interval, 1);
+    }
+}

src/error.rs

@@ -22,65 +22,229 @@ pub enum RobotLBError {
     UnknownLBAlgorithm,
     #[error("Cannot get target nodes, because the service has no selector")]
     ServiceWithoutSelector,
+    #[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
-    #[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<hcloud::apis::load_balancers_api::AttachLoadBalancerToNetworkError>,
     ),
-    #[error("Cannot detach load balancer from network. Reason: {0}")]
+    #[error("Cannot detach load balancer from network. Reason: {}", describe(.0))]
     HcloudLBDetachFromNetworkError(
         #[from]
         hcloud::apis::Error<hcloud::apis::load_balancers_api::DetachLoadBalancerFromNetworkError>,
     ),
-    #[error("Cannot add load balancer target. Reason: {0}")]
+    #[error("Cannot add load balancer target. Reason: {}", describe(.0))]
     HcloudLBAddTargetError(
         #[from] hcloud::apis::Error<hcloud::apis::load_balancers_api::AddTargetError>,
     ),
-    #[error("Cannot remove load balancer target. Reason: {0}")]
+    #[error("Cannot remove load balancer target. Reason: {}", describe(.0))]
     HcloudLBRemoveTargetError(
         #[from] hcloud::apis::Error<hcloud::apis::load_balancers_api::RemoveTargetError>,
     ),
-    #[error("Cannot add service to load balancer. Reason: {0}")]
+    #[error("Cannot add service to load balancer. Reason: {}", describe(.0))]
     HcloudLBAddServiceError(
         #[from] hcloud::apis::Error<hcloud::apis::load_balancers_api::AddServiceError>,
     ),
-    #[error("Cannot remove service from load balancer. Reason: {0}")]
+    #[error("Cannot remove service from load balancer. Reason: {}", describe(.0))]
     HcloudLBRemoveServiceError(
         #[from] hcloud::apis::Error<hcloud::apis::load_balancers_api::DeleteServiceError>,
     ),
-    #[error("Cannot create load balancer. Reason: {0}")]
+    #[error("Cannot create load balancer. Reason: {}", describe(.0))]
     HcloudLBCreateError(
         #[from] hcloud::apis::Error<hcloud::apis::load_balancers_api::CreateLoadBalancerError>,
     ),
-    #[error("Cannot delete load balancer. Reason: {0}")]
+    #[error("Cannot delete load balancer. Reason: {}", describe(.0))]
     HcloudLBDeleteError(
         #[from] hcloud::apis::Error<hcloud::apis::load_balancers_api::DeleteLoadBalancerError>,
     ),
-    #[error("Cannot get load balancer. Reason: {0}")]
+    #[error("Cannot get load balancer. Reason: {}", describe(.0))]
     HcloudLBGetError(
         #[from] hcloud::apis::Error<hcloud::apis::load_balancers_api::GetLoadBalancerError>,
     ),
-    #[error("Cannot update service. Reason: {0}")]
+    #[error("Cannot update service. Reason: {}", describe(.0))]
     HcloudLBUpdateServiceError(
         #[from] hcloud::apis::Error<hcloud::apis::load_balancers_api::UpdateServiceError>,
     ),
-    #[error("Cannot change type of load balancer. Reason: {0}")]
+    #[error("Cannot change type of load balancer. Reason: {}", describe(.0))]
     HcloudLBChangeType(
         #[from]
         hcloud::apis::Error<hcloud::apis::load_balancers_api::ChangeTypeOfLoadBalancerError>,
     ),
-    #[error("Cannot change algorithm of load balancer. Reason: {0}")]
+    #[error("Cannot change algorithm of load balancer. Reason: {}", describe(.0))]
     HcloudLBChangeAlgorithm(
         #[from] hcloud::apis::Error<hcloud::apis::load_balancers_api::ChangeAlgorithmError>,
     ),
-    #[error("Cannot list networks. Reason: {0}")]
+    #[error("Cannot list networks. Reason: {}", describe(.0))]
     HcloudListNetworksError(
         #[from] hcloud::apis::Error<hcloud::apis::networks_api::ListNetworksError>,
     ),
-    #[error("Cannot list load balancers. Reason: {0}")]
+    #[error("Cannot list load balancers. Reason: {}", describe(.0))]
     HcloudListLoadBalancersError(
         #[from] hcloud::apis::Error<hcloud::apis::load_balancers_api::ListLoadBalancersError>,
     ),
 }
+
+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<T>(error: &hcloud::apis::Error<T>) -> 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<T>(error: &hcloud::apis::Error<T>) -> String {
+    let hcloud::apis::Error::ResponseError(response) = error else {
+        return error.to_string();
+    };
+    let body =
+        k8s_openapi::serde_json::from_str::<k8s_openapi::serde_json::Value>(&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::<Vec<_>>().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<ListLoadBalancersError> {
+        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, "<html>bad gateway</html>");
+        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, ""));
+        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());
+    }
+}