Repository navigation
fix: port upstream controller safety and reconciliation improvements - #62
Merged
Merged
Conversation
Signed-off-by: Dev Jadeja <dev@treets.com>
…olicy (#31) * fix(nodes): select target nodes by externalTrafficPolicy Target nodes were always restricted to the nodes running the service's pods, which is the behaviour of externalTrafficPolicy: Local. Services using the default Cluster policy lost every node that kube-proxy could have forwarded from, so a service whose only pod sits on a node where its nodePort is unreachable ends up with no healthy target at all. Follow the policy declared by the service instead, and cover the port mapping and the policy check with unit tests. Assisted-By: LLM Signed-off-by: Kirill Ilin <stitch14@yandex.ru> * fix(targets): keep reconciling after a rejected target Hetzner refuses IP targets outside the vSwitch subnet of the attached network. Such a rejection aborted the whole reconciliation, so a single node the API declines kept every other node out of the load balancer. Log the rejected target and carry on with the rest. Assisted-By: LLM Signed-off-by: Kirill Ilin <stitch14@yandex.ru> * docs(readme): describe node selection and the nodePort requirement Assisted-By: LLM Signed-off-by: Kirill Ilin <stitch14@yandex.ru> * fix(nodes): keep excluded and cordoned nodes out of the balancer Following externalTrafficPolicy made every node a target under the Cluster policy, including nodes a cluster marks as unfit for external load balancers: kubeadm labels control-plane nodes with node.kubernetes.io/exclude-from-external-load-balancers, and a cordoned node is draining. Filter both out. A service left without a single exposable port no longer gets an external IP patched onto its status, so a load balancer that forwards nothing is not reported as ready. Port mapping now carries the existing LBService type instead of a bare pair of integers. Assisted-By: LLM Signed-off-by: Kirill Ilin <stitch14@yandex.ru> * fix(targets): fail reconciliation when no target can be added Tolerating a rejected target also hid the case where every target is rejected: reconciliation reported success and the service was patched with an external IP for a load balancer that had nowhere to forward to. Fail instead when no target could be added at all, so the controller requeues and the service stays unready. Warn as well when more nodes are selected than the balancer type holds, using the limit the API reports for that type. Assisted-By: LLM Signed-off-by: Kirill Ilin <stitch14@yandex.ru> * ci(pre-commit): run cargo test Assisted-By: LLM Signed-off-by: Kirill Ilin <stitch14@yandex.ru> * docs(readme): note the node exclusions, target limits and upgrade impact Assisted-By: LLM Signed-off-by: Kirill Ilin <stitch14@yandex.ru> * fix(targets): plan targets instead of failing on every rejected one Counting the attempts of a single run treated a permanently rejected node as a total failure: once the acceptable targets were in place, every following run retried only the rejected ones and reported failure forever. Count the targets that are actually live instead, and carry the reason of the last rejection into the error. Targets are now planned up front: deduplicated, sorted, and trimmed to what the balancer type holds, so a cluster larger than the limit stops retrying doomed calls and keeps a stable set of targets. The limit is not reported while a type change is still in flight, when the balancer answers with the type it is leaving. Assisted-By: LLM Signed-off-by: Kirill Ilin <stitch14@yandex.ru> * fix(service): stop billing for a balancer that forwards nothing A service without a single exposable port still got a Hetzner load balancer created for it, charged by the hour, with no service behind it and nothing but a log line to say so. Skip the balancer entirely in that case, and drop the external IP the service advertises, so that clients and ExternalDNS stop pointing at a balancer that no longer forwards. Node selection moves behind a NodeSource decision that can be tested on its own, not-ready nodes no longer take up target slots, and the exclusion label now holds under every traffic policy, the way upstream cloud providers treat it. Assisted-By: LLM Signed-off-by: Kirill Ilin <stitch14@yandex.ru> * docs(readme): describe target trimming and services left without a balancer Assisted-By: LLM Signed-off-by: Kirill Ilin <stitch14@yandex.ru> --------- Signed-off-by: Kirill Ilin <stitch14@yandex.ru>
Bumps [openssl](https://github.com/sfackler/rust-openssl) from 0.10.70 to 0.10.72. - [Release notes](https://github.com/sfackler/rust-openssl/releases) - [Commits](rust-openssl/rust-openssl@openssl-v0.10.70...openssl-v0.10.72) --- updated-dependencies: - dependency-name: openssl dependency-version: 0.10.72 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [tokio](https://github.com/tokio-rs/tokio) from 1.41.1 to 1.43.1. - [Release notes](https://github.com/tokio-rs/tokio/releases) - [Commits](tokio-rs/tokio@tokio-1.41.1...tokio-1.43.1) --- updated-dependencies: - dependency-name: tokio dependency-version: 1.43.1 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [ring](https://github.com/briansmith/ring) from 0.17.8 to 0.17.14. - [Changelog](https://github.com/briansmith/ring/blob/main/RELEASES.md) - [Commits](https://github.com/briansmith/ring/commits) --- updated-dependencies: - dependency-name: ring dependency-version: 0.17.14 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [serde_with](https://github.com/jonasbb/serde_with) from 3.11.0 to 3.24.0. - [Release notes](https://github.com/jonasbb/serde_with/releases) - [Commits](jonasbb/serde_with@v3.11.0...v3.24.0) --- updated-dependencies: - dependency-name: serde_with dependency-version: 3.24.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [openssl](https://github.com/rust-openssl/rust-openssl) from 0.10.72 to 0.10.81. - [Release notes](https://github.com/rust-openssl/rust-openssl/releases) - [Commits](rust-openssl/rust-openssl@openssl-v0.10.72...openssl-v0.10.81) --- updated-dependencies: - dependency-name: openssl dependency-version: 0.10.81 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [rand](https://github.com/rust-random/rand) from 0.8.5 to 0.8.8. - [Release notes](https://github.com/rust-random/rand/releases) - [Changelog](https://github.com/rust-random/rand/blob/0.8.8/CHANGELOG.md) - [Commits](rust-random/rand@0.8.5...0.8.8) --- updated-dependencies: - dependency-name: rand dependency-version: 0.8.8 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Adding the robotlb finalizer used a JSON merge patch that set metadata.finalizers to a one-element list. A merge patch replaces a list as a whole, so every finalizer another controller had put on the Service was dropped. Removal wrote back the list read from the cache, so a finalizer added or removed in between was lost or came back. metadata.finalizers is merged by value in a strategic merge patch, so adding now appends only robotlb's entry and removal deletes only that entry with a deleteFromPrimitiveList directive. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
Bumps [tracing-subscriber](https://github.com/tokio-rs/tracing) from 0.3.18 to 0.3.20. - [Release notes](https://github.com/tokio-rs/tracing/releases) - [Commits](tokio-rs/tracing@tracing-subscriber-0.3.18...tracing-subscriber-0.3.20) --- updated-dependencies: - dependency-name: tracing-subscriber dependency-version: 0.3.20 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [bytes](https://github.com/tokio-rs/bytes) from 1.8.0 to 1.12.1. - [Release notes](https://github.com/tokio-rs/bytes/releases) - [Changelog](https://github.com/tokio-rs/bytes/blob/master/CHANGELOG.md) - [Commits](tokio-rs/bytes@v1.8.0...v1.12.1) --- updated-dependencies: - dependency-name: bytes dependency-version: 1.12.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Preserve upstream master as a merge parent without replacing Forkline's refactored controller, Helm deployment or observability additions. Adapt upstream finalizer and exact-name lookup fixes to Forkline's code layout. Never issue an unversioned last-chance finalizer merge. Upstream: Treetscom/robotlb@7268ba2c Ported-from: Treetscom#53, Treetscom#76
Use ready eligible cluster nodes for Cluster traffic policy and endpoint-local nodes for Local policy, falling back to EndpointSlice when a Service has no selector. Exclude control-plane-designated nodes. Avoid creating a billed load balancer without an exposable NodePort, and cap target planning to the Hetzner LB target limit. Ported-from: Treetscom#31 Ported-from: Treetscom#25
Associate newly provisioned Hetzner balancers with the immutable Service UID. Resolve or delete resources by ownership instead of name. Conservatively adopt recognizable pre-upgrade balancers in place, keeping public IPs and existing labels. Ignore foreign balancers. Release on deletion or type/class change without parsing annotations; reject unrecognized name collisions rather than taking over LBs. Ported-from: Treetscom#62 Ported-from: Treetscom#47 Ported-from: Treetscom#64
Apply rustfmt changes reported by GitHub Actions, remove obsolete unreferenced API helpers, and satisfy newer Clippy diagnostics.
Guard Hetzner API calls across all managed Services after a 429, using a 60-second to 16-minute exponential pause. Avoid extending the cooldown for already-in-flight requests. Preserve typed errors from balancer creation so 429s are distinguishable. Ported-from: Treetscom#37
…ions Cover namespaced resource names, invalid annotations and conservative adoption rules. Document upgrade/cleanup risks and how to audit existing unlabelled Hetzner balancers. Ported-from: Treetscom#62
Prevent repeated Service watch events caused by writing the same ingress status after every reconcile. Adapt the API-call suppression principle from upstream while retaining Forkline's controller structure. Related-to: Treetscom#72
|
✅ Fix pushed: Fixed the CI failure by applying rustfmt formatting to the |
Retry 409/423/5xx and robot_unavailable responses on target updates, balancer type and algorithm changes with 1s/2s delay. Treat a target already added by a previous attempt as success. 429 stays governed by the shared rate-limit gate. Ported-from: Treetscom#59 Related-to: Treetscom#66
|
⏸️ CI Auto-Fix paused Maximum consecutive fix attempts (3/3) reached for this PR without success. You can:
To configure the maximum attempts, go to Repository Settings > Automation > CI Auto-Fix. |
Extend transient retry handling to service add/update/delete and network attach/detach, as in upstream busy-balancer handling. Ported-from: Treetscom#66
![forkline[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
Treat max_targets=0 as unlimited to prevent truncating all targets when LoadBalancerType uses default values in tests.
|
Applied rustfmt formatting to |
pando85
marked this pull request as ready for review
October 13, 2026 08:45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Single integration PR preserving the original upstream commits while adapting functionality to Forkline. The upstream merge keeps Forkline
master(b1792f4) as its first parent and Treetscom upstreammaster(7268ba2) as its second; upstream authors and SHAs remain reachable.What is included
externalTrafficPolicy; selectorlessLocalServices use ready EndpointSlices; selector-basedLocalServices retain simple pod-placement target selection, leaving target health to Hetzner TCP checks.LoadBalancerIngressvalues and avoid redundant status writes; remove the unsupported customdnsfield.cargo test --all-targets --lockedin PR CI.Explicit decisions
<service>.<namespace>.Localendpoint discovery for selector-based Services. Hetzner's TCP health checks determine target health; this keeps discovery simple.Not included
Full desired-state fingerprint memoization, Node-change-driven watches, complete token redaction/Event parity from upstream #37, optional per-cluster prefixes, lockfile dependency refresh and dedicated transient deletion retries remain separate follow-ups.
Rollout and validation
The existing Helm chart has opt-in Prometheus alert rules (
metrics.prometheusRules.enabled); Kubernetes Events alone do not send notifications. A production alert also requires a configured Prometheus/Alertmanager pipeline.Validate the latest Actions run before merging. Deploy first against disposable Hetzner load balancers, checking unchanged status patches, ambiguous ownership/name collisions, oversized capacity and invalid type downgrades. Existing unlabelled balancers require a separate audit if their IPs or continued billing matter.