Repository navigation
Conversation
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 <f@lex.la>
76182d6 to
d6c6ee0
Compare
| } | ||
| // 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) |
There was a problem hiding this comment.
I don't think that this check is required, since we assume that those annotations are not edited by hand anyway.
There was a problem hiding this comment.
@s3rius Both go away with #62, which is stacked on this PR. It removes the robotlb/balancer-id annotation completely. robotlb then finds its balancer by the robotlb/service-uid label it puts on the balancer when it creates it. If you'd rather not have the annotation land here at all, I can drop it from this PR. Until #62 the release would then find the balancer only by name.
| Some(json!({ | ||
| "metadata": { | ||
| "annotations": { | ||
| consts::LB_ID_ANN_NAME: format!("{uid}/{id}") |
There was a problem hiding this comment.
Why do we need service uid? I guess we can just set ID of an actual hetzner's loadbalancer.
There was a problem hiding this comment.
Then we can close this PR and rebase the #62 on top of main.
There was a problem hiding this comment.
@s3rius This PR is more than the annotation. It also fixes #38: robotlb releases the balancer when a Service changes its type or class, and #57 is already merged into this branch. #62 builds on that release logic, so closing #47 would drop the #38 fix.
The annotation is a leftover. I built the stack step by step and didn't get to the final shape right away, so I missed that #62 makes it unnecessary. I'll remove it from this PR, so it never lands.
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 <f@lex.la>
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 <f@lex.la>
96a8ddf to
bea258d
Compare
2 participants
When a Service stops being a robotlb load balancer, its Hetzner balancer is now deleted and the finalizer removed. Until now such a Service was skipped, so the balancer stayed billed and the finalizer stayed.
The API server clears
loadBalancerClasstogether with the type, so robotlb decides ownership by its finalizer. A Service that carries it but is no longer a robotlbLoadBalancer, after a type change or a change back with another class, goes through the same release path as a deleted Service. The status is left alone, since the API server clears it on the type change.Release finds the balancer by name, as before. If the
robotlb/balancerannotation is changed or removed in the same edit as the type, release deletes the balancer under the name the Service has now. That can be the balancer of another Service with the same name, see #41. #62 replaces this lookup with an ownership label. No unit test covers the lookup in the release path, since it sits in async code that talks to Hetzner.A Service with no TCP port that has a node port now keeps its balancer and external IP and gets a warning event. Before, it lost the IP while the balancer stayed.
On upgrade, Services changed from
LoadBalancerbefore this release still carry the finalizer, and their balancer is deleted on the first start. The README has a command to list them. Its balancer is found by name, so a same-named Service in another namespace may share it, see #41.This is stacked on #37 and targets its branch.
Closes #38