feat(self-managed): add function autoscaler to self-hosted stack - #512
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds self-managed deployment configuration for function-autoscaler, including runtime settings, static-token authentication, metrics exposure, Helmfile release wiring, configuration validation, version metadata, and ServiceMonitor integration. ChangesFunction Autoscaler Integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Helmfile
participant GlobalValues
participant StateMetrics
participant FunctionAutoscalerChart
participant ServiceMonitor
Helmfile->>Helmfile: Validate autoscaler and observability settings
Helmfile->>GlobalValues: Render functionautoscaler values
Helmfile->>StateMetrics: Wait for nvcf/state-metrics
Helmfile->>FunctionAutoscalerChart: Install enabled release
FunctionAutoscalerChart->>ServiceMonitor: Expose metrics service on TCP 41338
ServiceMonitor->>FunctionAutoscalerChart: Scrape selected metrics endpoint
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
bright-poku
left a comment
There was a problem hiding this comment.
LGTM ✅. tested from scratch on k3d, full stack comes up 2/2 (obs infra + state-metrics + autoscaler), and the new autoscaler ServiceMonitor is scraping its nvcf_autoscaler_* metrics into VM. Looks good
cf9a682 to
cf790c6
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@deploy/stacks/self-managed/helmfile.d/03-observability.yaml.gotmpl`:
- Around line 9-32: Add Helmfile render tests covering invalid observability
modes and value types, missing TIMESERIES_DB__TIMESERIES_DB_URL requirements,
the functionAutoscaler dependency on state-metrics, and disabled ServiceMonitor
behavior. Exercise the validation around observability, functionAutoscaler, and
victoriaMetrics values, and verify rendering fails or succeeds according to the
deployment contract.
- Around line 22-31: Validate the types of the values assigned to
$functionAutoscalerEnabled and $functionAutoscalerTsdbURL before the existing
mode guards use them: require enabled to be a boolean and the Timeseries DB URL
to be a string, failing template rendering for invalid inputs. Preserve the
current default values and validation behavior for correctly typed values.
- Around line 24-31: The validation around functionAutoscalerEnabled must also
require stateMetrics.enabled=true, matching its unconditional dependency on
nvcf/state-metrics. Update the existing Helm template checks near the
functionAutoscaler validation to fail with a clear message when
functionAutoscaler is enabled while stateMetrics is disabled.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: abda161f-616a-4345-8494-9a6cbae5c2fb
📒 Files selected for processing (7)
deploy/helm/function-autoscaler/values.yamldeploy/stacks/observability/charts/nvcf-default-monitors/templates/controlplane-servicemonitors.yamldeploy/stacks/observability/charts/nvcf-default-monitors/values.yamldeploy/stacks/self-managed/environments/base.yamldeploy/stacks/self-managed/global.yaml.gotmpldeploy/stacks/self-managed/helmfile.d/00-observability-infrastructure.yaml.gotmpldeploy/stacks/self-managed/helmfile.d/03-observability.yaml.gotmpl
🚧 Files skipped from review as they are similar to previous changes (3)
- deploy/stacks/self-managed/environments/base.yaml
- deploy/helm/function-autoscaler/values.yaml
- deploy/stacks/self-managed/global.yaml.gotmpl
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/control-plane-services/function-autoscaler/crates/server/src/nvcf_api/oauth2_client.rs (1)
186-189: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid re-entering the static-token branch on every refresh interval.
This path returns the watcher token without populating
token_cache, so the background task continues seeing an empty cache and logs a successful “refresh” every interval. Skip background refresh while a static token is configured, or cache it with an explicit rotation/expiry strategy.🤖 Prompt for 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. In `@src/control-plane-services/function-autoscaler/crates/server/src/nvcf_api/oauth2_client.rs` around lines 186 - 189, Update the access-token handling around the static-token branch in the OAuth2 client so a configured watcher token does not trigger repeated background refresh attempts. Either bypass refresh scheduling while the static token is present, or populate token_cache using an explicit rotation/expiry strategy; preserve dynamic-token refresh behavior.
🤖 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.
Nitpick comments:
In
`@src/control-plane-services/function-autoscaler/crates/server/src/nvcf_api/oauth2_client.rs`:
- Around line 186-189: Update the access-token handling around the static-token
branch in the OAuth2 client so a configured watcher token does not trigger
repeated background refresh attempts. Either bypass refresh scheduling while the
static token is present, or populate token_cache using an explicit
rotation/expiry strategy; preserve dynamic-token refresh behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a8a005c4-b31b-48a2-819e-661cf0a17728
📒 Files selected for processing (5)
deploy/helm/function-autoscaler/values.yamldeploy/helm/function-autoscaler/vault-agent-templates/secrets.json.tmpldeploy/stacks/self-managed/helmfile.d/03-observability.yaml.gotmplsrc/control-plane-services/function-autoscaler/crates/server/src/nvcf_api/oauth2_client.rssrc/control-plane-services/function-autoscaler/crates/server/src/secrets/secrets_config.rs
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 `@docs/user/manifest.md`:
- Line 160: Regenerate the generated manifest block instead of editing
docs/user/manifest.md directly: update the nvcf-openbao-migrations version in
docs/version-catalog/main.yaml, then run the docs version-sync generator and the
specified version-sync and documentation validation commands.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e12ee45c-303a-427a-8eb4-f11305a81584
📒 Files selected for processing (6)
deploy/helm/function-autoscaler/Chart.yamldeploy/stacks/self-managed/environments/base.yamldeploy/stacks/self-managed/helmfile.d/03-observability.yaml.gotmpldocs/user/manifest.mddocs/version-catalog/main.yamlsrc/control-plane-services/function-autoscaler/crates/server/src/nvcf_api/nvcf_client.rs
💤 Files with no reviewable changes (2)
- deploy/stacks/self-managed/environments/base.yaml
- deploy/stacks/self-managed/helmfile.d/03-observability.yaml.gotmpl
8f5c854 to
2e8a202
Compare
🛡️ CodeQL Analysis🚨 Found 2 issue(s) Severity Breakdown:
📋 Top Issues🔗 View full details in Security tab 🕐 Last updated: 2026-07-29 23:53:02 UTC | Commit: 2e8a202 |
Wire the self-managed observability helmfile to deploy the function autoscaler when enabled. Keep runtime defaults in the function autoscaler chart and expose only slim stack-level image and env overrides. Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
033ee0d to
bfd069b
Compare
|
🎉 This PR is included in version nvcf-function-autoscaler-v1.19.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version helm-nvcf-function-autoscaler-v0.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
TL;DR
Add the function autoscaler to the self-hosted stack by default.
Additional Details (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)
function-autoscalerrelease to03-observability.yaml.gotmpl, enabled by default throughfunctionAutoscaler.enabled.For the Reviewer
Please look closely at:
deploy/helm/function-autoscaler/values.yamldeploy/stacks/self-managed/global.yaml.gotmpldeploy/stacks/self-managed/helmfile.d/03-observability.yaml.gotmplThe main intent is to keep the stack-level surface small and leave runtime defaults in the chart.
Issues
Relates to #15
Related context: #480
Checklist
Summary by CodeRabbit