feat(autoscaler): compute GOMEMLIMIT per node group#370
Merged
Conversation
Signed-off-by: Ben <ben@armosec.io>
Signed-off-by: Ben <ben@armosec.io>
📝 WalkthroughWalkthroughThe pull request adds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
Summary:
|
…ateRenderer Signed-off-by: Ben <ben@armosec.io>
Signed-off-by: Ben <ben@armosec.io>
|
Summary:
|
Signed-off-by: Ben <ben@armosec.io>
matthyx
approved these changes
Apr 29, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
nodeagentautoscaler/templaterenderer.go (1)
211-213: Consider clamping very small positive results to at least1MiB.For very small memory limits, flooring may produce
0MiB; a minimum floor can avoid pathological output while preserving intent.Proposed tweak
limitBytes := resources.Limits.Memory.Value() goMemLimitMiB := int64(float64(limitBytes) * tr.goMemLimitPercentage / (1024 * 1024)) +if limitBytes > 0 && goMemLimitMiB == 0 { + goMemLimitMiB = 1 +}Also applies to: 227-227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nodeagentautoscaler/templaterenderer.go` around lines 211 - 213, The computation of goMemLimitMiB can yield 0 for very small memory limits; clamp the result to at least 1MiB by taking the max between the computed value and 1. Locate the calculation that reads limitBytes := resources.Limits.Memory.Value() and goMemLimitMiB := int64(float64(limitBytes) * tr.goMemLimitPercentage / (1024 * 1024)), then replace the direct assignment with logic that ensures goMemLimitMiB = max(computedValue, 1) so tiny positive limits floor to 1MiB (apply the same clamp where the same pattern occurs around the other instance at the referenced location).config/config.go (1)
320-320: Consider fail-fast range validation at config-validation stage.Right now invalid percentages are rejected later in renderer construction; validating in config validation would centralize and surface misconfiguration earlier.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/config.go` at line 320, Add fail-fast range validation for the nodeAgentAutoscaler.goMemLimitPercentage config key during the central config validation step: after loading defaults (where viper.SetDefault("nodeAgentAutoscaler.goMemLimitPercentage", 0.8) is set) read the configured value (viper.GetFloat64("nodeAgentAutoscaler.goMemLimitPercentage")) in the config validation function (e.g., ValidateConfig/Validate) and return an error if the value is outside the valid range (0.0 <= value <= 1.0), so misconfiguration is caught early rather than later in renderer construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@config/config.go`:
- Line 320: Add fail-fast range validation for the
nodeAgentAutoscaler.goMemLimitPercentage config key during the central config
validation step: after loading defaults (where
viper.SetDefault("nodeAgentAutoscaler.goMemLimitPercentage", 0.8) is set) read
the configured value
(viper.GetFloat64("nodeAgentAutoscaler.goMemLimitPercentage")) in the config
validation function (e.g., ValidateConfig/Validate) and return an error if the
value is outside the valid range (0.0 <= value <= 1.0), so misconfiguration is
caught early rather than later in renderer construction.
In `@nodeagentautoscaler/templaterenderer.go`:
- Around line 211-213: The computation of goMemLimitMiB can yield 0 for very
small memory limits; clamp the result to at least 1MiB by taking the max between
the computed value and 1. Locate the calculation that reads limitBytes :=
resources.Limits.Memory.Value() and goMemLimitMiB := int64(float64(limitBytes) *
tr.goMemLimitPercentage / (1024 * 1024)), then replace the direct assignment
with logic that ensures goMemLimitMiB = max(computedValue, 1) so tiny positive
limits floor to 1MiB (apply the same clamp where the same pattern occurs around
the other instance at the referenced location).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ffa5a517-5be0-444b-946d-15b32cff0f4c
📒 Files selected for processing (8)
.github/workflows/pr-created.yamlconfig/config.goconfig/config_test.gonodeagentautoscaler/autoscaler.gonodeagentautoscaler/autoscaler_test.gonodeagentautoscaler/integration_test.gonodeagentautoscaler/templaterenderer.gonodeagentautoscaler/templaterenderer_test.go
|
Summary:
|
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
2 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
GoMemLimitPercentagefield toNodeAgentAutoscalerConfig(default 0.8 = 80%)GOMEMLIMITper node group inTemplateRendereras a percentage of the memory limit, and inject it asGoMemLimitinto the DaemonSet templategoMemLimitPercentageis in the valid range(0, 1.0]at construction timeMotivation
In autoscaler mode the operator creates per-node-group DaemonSets with dynamically sized resource requests/limits. Previously the DaemonSet template had a static
GOMEMLIMITset at Helm install time. This PR makes the operator computeGOMEMLIMITat reconcile time from the actual computed memory limit, so it always stays at the configured percentage (default 80%) of whatever the operator decided the limit should be for that node group.Design note: main container vs. sidecar
The operator computes
GOMEMLIMITonly for the main node-agent container, whose resources scale per node group. When the SBOM scanner sidecar is enabled, its resources are static (configured via Helm values, uniform across node groups) and itsGOMEMLIMITis computed once by Helm at install time — the operator passes the sidecar definition through unchanged. Sidecar resource limits can still be adjusted viahelm upgrade --set nodeAgent.sbomScanner.resources.limits.memory=....Changes
config/config.goGoMemLimitPercentage float64toNodeAgentAutoscalerConfig; set viper default to 0.8nodeagentautoscaler/templaterenderer.gogoMemLimitPercentagefield; computeGoMemLimitinRenderDaemonSet; validate percentage in constructornodeagentautoscaler/autoscaler.gocfg.GoMemLimitPercentagetoNewTemplateRenderernodeagentautoscaler/templaterenderer_test.goTest plan
TestTemplateRenderer_RenderDaemonSet_GoMemLimitcovers 450Mi→360MiB, 1Gi→819MiB, 1800Mi→1440MiB, 900Mi→720MiB at 80%TestTemplateRenderer_NewTemplateRenderer_InvalidPercentageverifies rejection of 0, negative, >1.0 and acceptance of 1.0go test ./...)Summary by CodeRabbit
Release Notes
New Features
nodeAgentAutoscaler.goMemLimitPercentageconfiguration option (defaults to 0.8) with validation for values between 0 and 1.0Tests