feat(autoscaler): add scale-down eligibility knobs (ignore-daemonsets-utilization, skip-nodes-with-*) - #5756
Conversation
…-utilization, skip-nodes-with-*) The Hetzner Cluster Autoscaler judges scale-down on requested utilization and, by default, counts DaemonSet requests and refuses to remove nodes running pods with local storage or non-DaemonSet system pods. On a cluster whose per-node system DaemonSets (CNI, CSI, observability, security agents) already exceed the 50% scale-down threshold, overflow nodes become permanently unremovable — a one-way ratchet that exhausts the Hetzner server quota. Expose three cluster-autoscaler knobs on spec.cluster.autoscaler.node so the autoscaler can actually reclaim idle overflow nodes: - ignoreDaemonsetsUtilization (bool, default false) → --ignore-daemonsets-utilization - skipNodesWithLocalStorage (*bool, upstream default true) → --skip-nodes-with-local-storage - skipNodesWithSystemPods (*bool, upstream default true) → --skip-nodes-with-system-pods The two skip flags are pointers so an explicit false is preserved (nil inherits the upstream default, leaving existing releases free of values drift). Regenerated schema, CRD, deepcopy, reference docs, chat docs, and web UI types. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds three cluster-autoscaler configuration options to the API type, Helm rendering path, CRD and JSON schemas, documentation, and installer tests. ChangesCluster autoscaler flag additions
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
✅MegaLinter analysis: Success✅ Linters with no issuesactionlint, bash-exec, git_diff, hadolint, jscpd, jsonlint, lychee, markdown-table-formatter, markdownlint, prettier, prettier, shellcheck, shfmt, stylelint, syft, trivy-sbom, trufflehog, v8r, v8r, yamllint Notices📣 MegaLinter 9.5.0 is out! Discover the new features and security recommendations in the release announcement. (Skip this info by defining See detailed reports in MegaLinter artifacts
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/apis/cluster/v1alpha1/autoscaler.go`:
- Around line 47-54: The IgnoreDaemonsetsUtilization field is using a
comma-delimited jsonschema description, so the text for the Cluster Autoscaler
option gets truncated at the comma in the flag name. Update the Autoscaler
struct field in autoscaler.go to follow the same pattern as the sibling fields
by moving the full description into jsonschema_description instead of embedding
it in jsonschema:"description=...". After changing the tag on
IgnoreDaemonsetsUtilization, regenerate the schema/docs so the updated
description is reflected everywhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fa8141b7-d268-43a9-a3e9-83fb1187bc34
⛔ Files ignored due to path filters (1)
web/ui/src/generated/ksail-config.tsis excluded by!**/generated/**
📒 Files selected for processing (8)
charts/ksail-operator/crds/ksail.io_clusters.yamldocs/src/content/docs/configuration/declarative-configuration.mdxpkg/apis/cluster/v1alpha1/autoscaler.gopkg/apis/cluster/v1alpha1/zz_generated.deepcopy.gopkg/svc/chat/docs_generated.gopkg/svc/installer/clusterautoscaler/installer.gopkg/svc/installer/clusterautoscaler/installer_test.goschemas/ksail-config.schema.json
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: 🧪 Test
- GitHub Check: 🧹 Lint - mega-linter
- GitHub Check: 🔍 Dead Code Analysis
- GitHub Check: 🧹 Lint - golangci-lint
- GitHub Check: 📊 Code Coverage
- GitHub Check: 📦 Tidy
- GitHub Check: 🏗️ Build
- GitHub Check: 🛡️ Vulnerability Scan
- GitHub Check: ⛵ Operator Chart E2E
- GitHub Check: 🏠 Home Isolation Guard
- GitHub Check: 🏗️ Build KSail Binary
- GitHub Check: 🔍 License Check
- GitHub Check: 🏗️ Build (Linux)
- GitHub Check: 🏗️ Build + cask (macOS)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🧰 Additional context used
📓 Path-based instructions (2)
docs/src/content/docs/configuration/declarative-configuration.mdx
📄 CodeRabbit inference engine (AGENTS.md)
Keep
docs/src/content/docs/configuration/declarative-configuration.mdxsynchronized with the generator output; do not hand-edit generated configuration docs.
Files:
docs/src/content/docs/configuration/declarative-configuration.mdx
schemas/ksail-config.schema.json
📄 CodeRabbit inference engine (AGENTS.md)
Do not hand-edit
schemas/ksail-config.schema.json; regenerate it from the schema generator instead.
Files:
schemas/ksail-config.schema.json
🧠 Learnings (1)
📚 Learning: 2026-06-30T02:35:51.945Z
Learnt from: devantler
Repo: devantler-tech/ksail PR: 5561
File: pkg/apis/cluster/v1alpha1/options.go:95-108
Timestamp: 2026-06-30T02:35:51.945Z
Learning: For kubebuilder API fields typed as `metav1.Duration`, avoid using `+kubebuilder:validation:Pattern`. `controller-gen` rejects `Pattern` markers on `metav1.Duration` (even though the field may render as a string). Use a CEL `+kubebuilder:validation:XValidation` rule instead to enforce validation constraints, e.g. on duration format/range, so code generation and schema validation both succeed.
Applied to files:
pkg/apis/cluster/v1alpha1/zz_generated.deepcopy.gopkg/apis/cluster/v1alpha1/autoscaler.go
🔇 Additional comments (9)
schemas/ksail-config.schema.json (2)
539-542: 🎯 Functional CorrectnessTruncated description — downstream of a source struct-tag bug.
The
ignoreDaemonsetsUtilizationdescription is cut off mid-sentence at "--ignore-daemonsets-utilization". This is a generator-output symptom of a comma-splitting issue in thejsonschematag onIgnoreDaemonsetsUtilizationinpkg/apis/cluster/v1alpha1/autoscaler.go; see the root-cause comment there. Per coding guidelines this file should not be hand-edited — fix the source tag and regenerate.
543-550: LGTM!docs/src/content/docs/configuration/declarative-configuration.mdx (2)
417-417: 🎯 Functional CorrectnessTruncated description — downstream of a source struct-tag bug.
Same truncation as
schemas/ksail-config.schema.jsonforignoreDaemonsetsUtilization, tracing back to thejsonschematag comma issue inpkg/apis/cluster/v1alpha1/autoscaler.go(see comment there). Per coding guidelines this doc should stay synchronized with the generator output — fix the source and regenerate rather than hand-editing this line.
418-419: LGTM!pkg/apis/cluster/v1alpha1/autoscaler.go (1)
55-72: LGTM!pkg/apis/cluster/v1alpha1/zz_generated.deepcopy.go (1)
513-523: LGTM!charts/ksail-operator/crds/ksail.io_clusters.yaml (1)
115-124: LGTM!Also applies to: 203-224
pkg/svc/installer/clusterautoscaler/installer.go (1)
151-162: LGTM!Also applies to: 296-305
pkg/svc/installer/clusterautoscaler/installer_test.go (1)
652-758: LGTM!
github-project-automation
Bot
moved this from 🫴 Ready
to 🏃🏻♂️ In Progress
in 🌊 Project Board
Code Coverage OverviewLanguages: Go Go / code-coverage/goThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
The failing |
…zation The jsonschema:"description=..." tag is comma-delimited, so the comma in "--ignore-daemonsets-utilization, off by default" truncated the generated schema/doc description at that point. Switch to the dedicated jsonschema_description tag (as the sibling scale-down fields already do) and regenerate schema + reference docs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-project-automation
Bot
moved this from 🏃🏻♂️ In Progress
to ✅ Done
in 🌊 Project Board
1 participant
Why
On the Hetzner prod cluster, overflow (autoscaler) nodes could never scale back down: the Cluster Autoscaler counts each node’s system-DaemonSet requests (CNI, CSI, observability, security agents) toward utilization, and those alone exceed the 50% scale-down threshold — a one-way ratchet that pinned idle nodes and exhausted the Hetzner server quota (which in turn wedges the deploy pipeline). KSail exposed no way to relax this.
What
Adds three cluster-autoscaler scale-down knobs to
spec.cluster.autoscaler.nodeso operators can let the autoscaler reclaim genuinely-idle overflow nodes:ignoreDaemonsetsUtilization,skipNodesWithLocalStorage, andskipNodesWithSystemPods. All default to the upstream behaviour (no change for existing clusters); the skip flags are opt-out (explicitfalseis preserved). Regenerated schema/CRD/docs/types accordingly.Unblocks the platform node right-sizing work; the platform side sets
ignoreDaemonsetsUtilization: trueonce this ships in a release. Complements dropping the too-small cx33 overflow type (platform#2420).