Skip to content

fix(service): release a service without reading its annotations - #64

Open
lexfrei wants to merge 1 commit into
fix/balancer-ownershipfrom
fix/release-without-annotations
Open

lexfrei wants to merge 1 commit into
fix/balancer-ownershipfrom
fix/release-without-annotations

Conversation

@lexfrei

@lexfrei lexfrei commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

A Service with a robotlb annotation that does not parse can't be deleted. With robotlb/lb-retries: abc, sync_service failed on the annotation before it got to the release. The balancer was never deleted, the finalizer stayed, and the Service hung in Terminating. Changing such a Service away from type LoadBalancer got stuck the same way.

Release doesn't need the annotations. It finds the balancer by the robotlb/service-uid label, so the Service UID and the Hetzner client are enough. LoadBalancer::for_release builds only that. balancer_for picks the constructor by role: for_release for a release, try_from_svc for a reconcile. Reconcile is unchanged and still fails on a bad annotation. The role picks the constructor in this one place, so the for_release value (all other fields at default) never reaches reconcile.

The rest of the release path doesn't read annotations either. That covers role detection, the rate limit gate, finalizer removal and the failure event.

Two unit tests. The one on balancer_for checks the routing: with lb-retries: abc a release works and a reconcile fails, and a release of a Service without a UID is skipped. The one on for_release checks that it keeps the UID even with that annotation. Cleanup looks the balancer up by the UID only, so an empty one would silently find nothing and leave the balancer behind. cargo test, cargo clippy --all-targets and cargo fmt pass, with no new clippy warnings.

One possible follow-up: LoadBalancer derives Default only to fill for_release. Listing the fields there would drop the public LoadBalancer::default(), which has an empty UID.

Stacked on #62.

Closes #42

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 <f@lex.la>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant