Repository navigation
fix(platform): spread co-located HA controllers across nodes - #2125
Merged
Merged
Conversation
Coroot flagged several controllers running >=2 replicas as "all instances on one node - not resilient to node failure". Add soft (ScheduleAnyway) topologySpreadConstraints across kubernetes.io/hostname so the replicas prefer separate nodes, without ever blocking scheduling. Pure spread change: no replica counts or resource requests are touched. Workloads: - kyverno backgroundController / reportsController / cleanupController (chart topologySpreadConstraints, mirroring the existing admissionController idiom) - cilium hubble.relay (chart topologySpreadConstraints) - snapshot-controller controller (chart topologySpreadConstraints) - vertical-pod-autoscaler admissionController + updater — the FairwindsOps vpa chart exposes no topologySpreadConstraints/affinity values for these components, so the spread is injected via a postRenderers Kustomize patch on each Deployment (the same mechanism flagger already uses). The VPA recommender is left untouched (not flagged by Coroot). Each chart's support for the key (or absence, for vpa) was confirmed with `helm show values`/`helm template`, and every selector matches the chart's Deployment .spec.selector.matchLabels. `kubectl kustomize` builds clean for all changed dirs and for the merged hetzner controllers overlay. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
devantler
marked this pull request as ready for review
June 17, 2026 22:27
devantler
enabled auto-merge
June 17, 2026 22:27
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Jun 17, 2026
Contributor
|
🎉 This PR is included in version 1.64.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
5 tasks done
This branch was previously deployed
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
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.
What
Coroot flagged several control-plane controllers that already run ≥2 replicas as "all instances on one node — not resilient to node failure" (the replicas are co-located on a single node, so one node loss takes the whole controller down).
This PR adds soft
topologySpreadConstraints(maxSkew: 1,topologyKey: kubernetes.io/hostname,whenUnsatisfiable: ScheduleAnyway) so the replicas prefer landing on different nodes. Because it isScheduleAnyway, it never blocks scheduling — it only influences placement when a second node is available.This is a pure spread change — no replica counts and no resource requests are touched, so there is no capacity/cost impact.
Workloads changed
backgroundControllerk8s/bases/infrastructure/controllers/kyverno/helm-release.yamltopologySpreadConstraintscomponent=background-controller, instance=kyvernoreportsControllertopologySpreadConstraintscomponent=reports-controller, instance=kyvernocleanupControllertopologySpreadConstraintscomponent=cleanup-controller, instance=kyvernok8s/bases/infrastructure/controllers/cilium/helm-release.yamlhubble.relay.topologySpreadConstraintsk8s-app=hubble-relayk8s/providers/hetzner/infrastructure/controllers/snapshot-controller/helm-release.yamlcontroller.topologySpreadConstraintsname=snapshot-controller, instance=snapshot-controlleradmissionControllerk8s/bases/infrastructure/controllers/vertical-pod-autoscaler/helm-release.yamlcomponent=admission-controller, name=vpa, instance=vertical-pod-autoscalerupdatercomponent=updater, name=vpa, instance=vertical-pod-autoscalerThe Kyverno change mirrors the already-present
admissionControllertopologySpread idiom in the same file, applied to the three other ≥2-replica controllers.Chart that needed a different approach (postRenderer)
vertical-pod-autoscaler (FairwindsOps
vpachart v4.11.0) exposes notopologySpreadConstraints(noraffinity) values for its components — confirmed withhelm show values. So for the two flagged components (admissionController,updater) the spread is injected with apostRenderersKustomize patch on each rendered Deployment — the same mechanism theflaggerHelmRelease already uses in this repo. The VPA recommender is intentionally left untouched (Coroot did not flag it).Validation
helm show values/helm templateat the exact chart versions/URLs in eachhelm-release.yaml.labelSelectormatches the chart's Deployment.spec.selector.matchLabels.kubectl kustomizebuilds clean for all four changed dirs and for the mergedk8s/providers/hetzner/infrastructure/controllersoverlay — verified the relay spread coexists with the hetzner WireGuard strictMode patch, Kyverno shows all 4 spread blocks, and the VPA postRenderer survives the merge.helm template→kustomize) and confirmed both Deployments gettopologySpreadConstraintswith correct selectors while the recommender stays unmodified.All flagged ≥2-replica controllers in scope were spread; nothing was left unspread.
🤖 Generated with Claude Code