feat(self-managed): wire autoscaler observability#384
Conversation
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
📝 WalkthroughWalkthroughThe change adds function autoscaler metrics exposure and monitoring, introduces self-managed observability and autoscaler configuration, wires derived environment values, validates deployment prerequisites, conditionally deploys observability components and the autoscaler through Helmfile, and supports optional static NVCF API tokens from Vault. ChangesObservability and function autoscaler
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Helmfile
participant Observability
participant MetricsBackends
participant FunctionAutoscaler
Helmfile->>Observability: Validate settings and define releases
Observability->>MetricsBackends: Install enabled backends
Observability->>FunctionAutoscaler: Deploy when enabled
FunctionAutoscaler->>MetricsBackends: Depend on enabled metrics backends
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![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 `@deploy/stacks/observability/charts/nvcf-default-monitors/values.yaml`:
- Around line 57-70: Update the api monitor definition in values.yaml to target
the existing Service port named http instead of management-port. Leave the sis
monitor and other api settings unchanged.
🪄 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: e44c9508-d7db-4027-be45-82aec9f69ee4
📒 Files selected for processing (5)
deploy/helm/function-autoscaler/values.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/03-observability.yaml.gotmpl
| - name: api | ||
| enabled: true | ||
| path: /actuator/prometheus | ||
| port: management-port | ||
| selector: | ||
| app.kubernetes.io/name: helm-nvcf-api | ||
| - name: sis | ||
| enabled: true | ||
| namespaces: | ||
| - sis | ||
| path: /actuator/prometheus | ||
| port: management-port | ||
| selector: | ||
| app.kubernetes.io/name: helm-nvcf-sis |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm api/sis services expose a named 'management-port' and inspect metrics gating.
fd -t f -e yaml -e gotmpl -e tpl . deploy/helm 2>/dev/null | xargs rg -nP -C2 'management-port|actuator/prometheus' 2>/dev/null
rg -nP -C2 'MANAGEMENT_PROMETHEUS_METRICS_EXPORT_ENABLED|observability\.metrics\.enabled' deploy/stacks/self-managedRepository: NVIDIA/nvcf
Length of output: 4962
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the observability monitor chart and the api/sis service templates.
fd -t f 'values.yaml' deploy/stacks/observability/charts/nvcf-default-monitors deploy/helm 2>/dev/null
printf '\n---\n'
fd -t f -g 'deployment*.yaml' -g 'service*.yaml' deploy/helm 2>/dev/null | sort
printf '\n--- nvcf-default-monitors values excerpt ---\n'
sed -n '1,140p' deploy/stacks/observability/charts/nvcf-default-monitors/values.yaml
printf '\n--- api/sis service/deployment excerpts ---\n'
for f in \
deploy/helm/api-keys-colocated/api-keys/templates/deployment.yaml \
deploy/helm/api-keys-colocated/api-keys/templates/service.yaml \
deploy/helm/api-keys-colocated/sis/templates/deployment.yaml \
deploy/helm/api-keys-colocated/sis/templates/service.yaml
do
if [ -f "$f" ]; then
echo "### $f"
sed -n '1,180p' "$f"
echo
fi
done
printf '\n--- metrics gating references ---\n'
rg -n -C2 'MANAGEMENT_PROMETHEUS_METRICS_EXPORT_ENABLED|MANAGEMENT_METRICS_ENABLE_ALL|observability\.metrics\.enabled|management-port' deploy/helm deploy/stacks/self-managed deploy/stacks/observability 2>/dev/nullRepository: NVIDIA/nvcf
Length of output: 16110
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focus on the exact files and line ranges implied by the review comment.
echo '--- deploy/stacks/observability/charts/nvcf-default-monitors/values.yaml ---'
nl -ba deploy/stacks/observability/charts/nvcf-default-monitors/values.yaml | sed -n '45,90p'
echo
echo '--- deploy/stacks/self-managed/global.yaml.gotmpl ---'
nl -ba deploy/stacks/self-managed/global.yaml.gotmpl | sed -n '332,345p'
echo
echo '--- api-keys service deployment/service ---'
for f in \
deploy/helm/api-keys-colocated/api-keys/templates/deployment.yaml \
deploy/helm/api-keys-colocated/api-keys/templates/service.yaml \
deploy/helm/api-keys-colocated/sis/templates/deployment.yaml \
deploy/helm/api-keys-colocated/sis/templates/service.yaml
do
if [ -f "$f" ]; then
echo "### $f"
nl -ba "$f" | sed -n '1,140p'
echo
fi
done
echo '--- search for management-port service names in relevant charts ---'
rg -n -C2 'name:\s*management-port|port:\s*management-port|management-port:' deploy/helm/api-keys-colocated deploy/stacks/observability deploy/stacks/self-managed 2>/dev/nullRepository: NVIDIA/nvcf
Length of output: 266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the exact service definitions for api and sis, plus the chart mode wiring.
for f in \
deploy/helm/api-keys-colocated/api-keys/templates/service.yaml \
deploy/helm/api-keys-colocated/sis/templates/service.yaml \
deploy/stacks/observability/charts/nvcf-default-monitors/values.yaml \
deploy/stacks/observability/charts/nvcf-default-monitors/templates/*.yaml
do
[ -f "$f" ] || continue
echo "### $f"
nl -ba "$f" | sed -n '1,220p'
echo
done
echo '--- grep for observability mode and metrics defaults ---'
rg -n -C3 'observability\.mode|metrics\.enabled|enabled:\s*true|enabled:\s*false' deploy/stacks/self-managed deploy/stacks/observability 2>/dev/nullRepository: NVIDIA/nvcf
Length of output: 257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for the actual Service definitions and any management-port service name on api/sis charts.
rg -n -C3 'kind:\s*Service|name:\s*management-port|targetPort:\s*management-port|port:\s*management-port' \
deploy/helm/api-keys-colocated deploy/helm 2>/dev/null
printf '\n--- files mentioning helm-nvcf-sis ---\n'
rg -n -C2 'helm-nvcf-sis|app.kubernetes.io/name:\s*helm-nvcf-sis|sis' deploy/helm 2>/dev/null
printf '\n--- service templates under api-keys-colocated ---\n'
fd -t f 'service.yaml' deploy/helm/api-keys-colocated 2>/dev/nullRepository: NVIDIA/nvcf
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the sis chart/service definitions and limit output to the relevant lines.
fd -t f -g '*sis*' deploy/helm 2>/dev/null | sort
printf '\n--- sis references with service/port names ---\n'
rg -n -C2 'kind:\s*Service|name:\s*management-port|targetPort:|port: management-port|app.kubernetes.io/name:\s*helm-nvcf-sis|helm-nvcf-sis' deploy/helm 2>/dev/null | head -n 200Repository: NVIDIA/nvcf
Length of output: 15848
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how the nvcf-default-monitors chart enables ServiceMonitors and what it keys off.
fd -t f -g '*.yaml' -g '*.yml' -g '*.gotmpl' deploy/stacks/observability/charts/nvcf-default-monitors 2>/dev/null | sort
printf '\n--- chart templates mentioning controlPlane/mode/enabled ---\n'
rg -n -C3 'controlPlane|install|existing|disabled|enabled:|namespaceSelector|ServiceMonitor' deploy/stacks/observability/charts/nvcf-default-monitors 2>/dev/nullRepository: NVIDIA/nvcf
Length of output: 11698
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the observability stack wiring for mode -> chart enablement.
rg -n -C3 'observability\.mode|nvcf-default-monitors|controlPlane\.enabled|default-monitors|install|existing|disabled' \
deploy/stacks/observability deploy/stacks/self-managed 2>/dev/nullRepository: NVIDIA/nvcf
Length of output: 42627
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the sis service/deployment templates and any management port exposure.
rg -n -C3 'helm-nvcf-sis|management-port|managementPort|kind:\s*Service|ports:' deploy/helm/api-keys-colocated deploy/helm 2>/dev/null | head -n 250Repository: NVIDIA/nvcf
Length of output: 19568
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact service port names used by api-keys and sis-related templates.
for f in \
deploy/helm/api-keys-colocated/api-keys/templates/service.yaml \
deploy/helm/api-keys-colocated/api-keys/templates/deployment.yaml \
deploy/helm/api-keys-colocated/api-keys/values.yaml \
deploy/helm/api-keys-colocated/sis/templates/service.yaml \
deploy/helm/api-keys-colocated/sis/templates/deployment.yaml \
deploy/helm/api-keys-colocated/sis/values.yaml
do
[ -f "$f" ] || continue
echo "### $f"
sed -n '1,140p' "$f"
echo
doneRepository: NVIDIA/nvcf
Length of output: 7266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether api-keys or sis consume the global metrics-export env vars.
rg -n -C3 'MANAGEMENT_PROMETHEUS_METRICS_EXPORT_ENABLED|MANAGEMENT_METRICS_ENABLE_ALL|global\.observability\.metrics|actuator/prometheus|management-port' \
deploy/helm/api-keys-colocated deploy/helm 2>/dev/null | head -n 250Repository: NVIDIA/nvcf
Length of output: 5941
Retarget api to an existing Service port. The api Service only exposes http, so port: management-port won’t match unless that Service adds a named management-port port.
🤖 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 `@deploy/stacks/observability/charts/nvcf-default-monitors/values.yaml` around
lines 57 - 70, Update the api monitor definition in values.yaml to target the
existing Service port named http instead of management-port. Leave the sis
monitor and other api settings unchanged.
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
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 `@deploy/stacks/observability/charts/nvcf-default-monitors/values.yaml`:
- Around line 75-79: Update the invocation-service monitor configuration in
nvcf-default-monitors values so its enabled state matches an available metrics
Service endpoint: add and wire a metrics port for helm-nvcf-invocation-service
if metrics are exposed there, or set enabled to false while no metrics Service
port exists. Preserve the monitor’s metrics port/path defaults.
🪄 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: ee8c0f77-cf66-467a-974a-e640323718fa
📒 Files selected for processing (4)
deploy/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/03-observability.yaml.gotmpl
🚧 Files skipped from review as they are similar to previous changes (3)
- deploy/stacks/self-managed/global.yaml.gotmpl
- deploy/stacks/self-managed/helmfile.d/03-observability.yaml.gotmpl
- deploy/stacks/self-managed/environments/base.yaml
| - name: invocation-service | ||
| enabled: false | ||
| enabled: true | ||
| selector: | ||
| app.kubernetes.io/instance: invocation-service | ||
| app.kubernetes.io/name: helm-nvcf-invocation-service |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 'invocation-service|name:\s*metrics|targetPort:\s*metrics|/metrics' deploy/helm deploy/stacks/self-managedRepository: NVIDIA/nvcf
Length of output: 43866
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the invocation-service chart for its metrics service/endpoint contract
printf '\n== values ==\n'
sed -n '130,165p' deploy/helm/http-invocation/values.yaml
printf '\n== deployment ==\n'
sed -n '1,140p' deploy/helm/http-invocation/templates/deployment.yaml
printf '\n== service ==\n'
sed -n '1,120p' deploy/helm/http-invocation/templates/service.yaml
printf '\n== default monitor values ==\n'
sed -n '60,95p' deploy/stacks/observability/charts/nvcf-default-monitors/values.yamlRepository: NVIDIA/nvcf
Length of output: 7163
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== default monitor templates ==\n'
fd -a . deploy/stacks/observability/charts/nvcf-default-monitors/templates
printf '\n== monitor template references ==\n'
rg -n -C3 'path:|port:|ServiceMonitor|PodMonitor|invocation-service' deploy/stacks/observability/charts/nvcf-default-monitors/templates deploy/stacks/observability/charts/nvcf-default-monitors/values.yamlRepository: NVIDIA/nvcf
Length of output: 9066
Add a metrics Service port before enabling the invocation-service monitor.
deploy/helm/http-invocation only publishes grpc and http service ports, so this monitor’s default port: metrics/path: /metrics has no Service endpoint to scrape. If metrics are exposed elsewhere, wire that port here; otherwise keep the monitor disabled.
🤖 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 `@deploy/stacks/observability/charts/nvcf-default-monitors/values.yaml` around
lines 75 - 79, Update the invocation-service monitor configuration in
nvcf-default-monitors values so its enabled state matches an available metrics
Service endpoint: add and wire a metrics port for helm-nvcf-invocation-service
if metrics are exposed there, or set enabled to false while no metrics Service
port exists. Preserve the monitor’s metrics port/path defaults.
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
Signed-off-by: Bora Oztekin <boztekin@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/helm/function-autoscaler/vault-agent-templates/secrets.json.tmpl`:
- Around line 10-12: Make the static-token block around secret path
services/nvcf-api/jwt/sign/nvcf-autoscaler-service conditional on the dedicated
static-token configuration, so deployments without that setting skip the secret
lookup and allow OAuth2 fallback. Preserve nvcfApiToken rendering when
static-token mode is enabled, and add coverage for both enabled and disabled
modes.
In
`@src/control-plane-services/function-autoscaler/crates/server/src/nvcf_api/oauth2_client.rs`:
- Around line 171-173: Update the refresh-loop logic around get_nvcf_api_token
and token_cache so a present static NVCF API token prevents repeated background
refresh attempts without caching it as a refreshable token. Continue checking
get_nvcf_api_token on each interval so token rotation or removal is detected,
and only perform dynamic refresh when no static token is present.
🪄 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: da5f9ee6-7b32-4909-a01f-1a458b54aa7b
📒 Files selected for processing (6)
deploy/helm/function-autoscaler/vault-agent-templates/secrets.json.tmpldeploy/stacks/self-managed/global.yaml.gotmpldeploy/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.rssrc/control-plane-services/function-autoscaler/crates/server/src/secrets/secrets_file_watcher.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- deploy/stacks/self-managed/global.yaml.gotmpl
- deploy/stacks/self-managed/helmfile.d/03-observability.yaml.gotmpl
| {{- with secret `services/nvcf-api/jwt/sign/nvcf-autoscaler-service` }} | ||
| "nvcfApiToken": "{{ .Data.token }}", | ||
| {{- end }} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- AGENTS ---'
find .. -name AGENTS.md -print
printf '\n%s\n' '--- target file ---'
sed -n '1,120p' deploy/helm/function-autoscaler/vault-agent-templates/secrets.json.tmpl
printf '\n%s\n' '--- related references ---'
rg -n "nvcf-autoscaler-service|nvcfApiToken|static token|oauth2|secret `services/nvcf-api/jwt/sign/nvcf-autoscaler-service`" deploy/helm -SRepository: NVIDIA/nvcf
Length of output: 22408
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- root AGENTS ---'
sed -n '1,240p' AGENTS.md
printf '\n%s\n' '--- function-autoscaler values ---'
sed -n '1,220p' deploy/helm/function-autoscaler/values.yaml
printf '\n%s\n' '--- function-autoscaler templates/manifests referencing vault agent ---'
rg -n "vault|openbao|secrets.json|nvcfApiToken|oauth2|static token|secret path|agent" deploy/helm/function-autoscaler -S
printf '\n%s\n' '--- openbao-related template examples in repo ---'
sed -n '1,80p' deploy/helm/llm-request-router/llm-request-router/vault-agent-templates/secrets.json.tmpl
sed -n '1,80p' deploy/helm/grpc-proxy/grpc-proxy/vault-agent-templates/secrets.json.tmplRepository: NVIDIA/nvcf
Length of output: 21198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- function-autoscaler AGENTS ---'
sed -n '1,220p' deploy/helm/function-autoscaler/AGENTS.md
printf '\n%s\n' '--- search for nvcfApiToken / oauth2 / fallback in autoscaler code ---'
rg -n "nvcfApiToken|oauth2|OAuth2|bearer token|static token|SECRETS_PATH|secrets.json" src/control-plane-services/function-autoscaler deploy/helm/function-autoscaler -S
printf '\n%s\n' '--- file list around autoscaler source ---'
git ls-files src/control-plane-services/function-autoscaler | sed -n '1,200p'Repository: NVIDIA/nvcf
Length of output: 269
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- autoscaler source files ---'
git ls-files src/control-plane-services/function-autoscaler | sed -n '1,200p'
printf '\n%s\n' '--- autoscaler source search ---'
rg -n "nvcfApiToken|SECRETS_PATH|oauth2|OAuth2|bearer token|static token|token" src/control-plane-services/function-autoscaler -S
printf '\n%s\n' '--- helm template/search ---'
rg -n "vault.enabled|agent-inject-template-file-secrets.json|nvcfApiToken|oauth2|static token|secret `services/nvcf-api/jwt/sign/nvcf-autoscaler-service`" deploy/helm/function-autoscaler -SRepository: NVIDIA/nvcf
Length of output: 50367
🌐 Web query:
OpenBao Vault Agent template missing secret path with secret render error retry semantics
💡 Result:
By default, OpenBao Agent (and Vault Agent) templates are designed to retry indefinitely when secret rendering fails, including cases where a secret path is missing or non-existent [1][2][3]. The Agent employs an exponential backoff mechanism for these retries [1][3]. To modify this behavior and cause the Agent to exit after a failed number of attempts rather than looping indefinitely, you must explicitly configure specific settings within your Agent configuration file [1][2]: 1. set exit_on_retry_failure to true within the template_config stanza of your global configuration [1][2]. 2. For cases involving missing keys within a secret, you should also set error_on_missing_key = true inside the specific template stanza [1][2]. Note that these two settings function differently depending on the nature of the error [1][2]: - A missing key (e.g., accessing a field that does not exist in a secret response) will only cause an error if error_on_missing_key is set to true [1][2]. - A missing or non-existent secret path will always cause the templating engine to error [1][2]. - Whether the Agent exits upon these errors is controlled solely by exit_on_retry_failure [1][2]. If you do not set exit_on_retry_failure to true, the Agent will continue to retry the template rendering indefinitely, regardless of whether the error is due to a missing secret path or a missing key [1][3]. While the Agent has a retry stanza for general OpenBao requests, requests originating from the template subsystem are retried regardless of the failure until the process is terminated or exit_on_retry_failure is triggered [3].
Citations:
- 1: https://openbao.org/docs/agent-and-proxy/agent/template/
- 2: https://developer.hashicorp.com/vault/docs/agent-and-proxy/agent/template
- 3: https://github.com/openbao/openbao/blob/main/website/content/docs/agent-and-proxy/agent/index.mdx
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- secrets config ---'
sed -n '1,120p' src/control-plane-services/function-autoscaler/crates/server/src/secrets/secrets_config.rs
printf '\n%s\n' '--- secrets file watcher ---'
sed -n '130,220p' src/control-plane-services/function-autoscaler/crates/server/src/secrets/secrets_file_watcher.rs
printf '\n%s\n' '--- oauth2 client ---'
sed -n '130,220p' src/control-plane-services/function-autoscaler/crates/server/src/nvcf_api/oauth2_client.rs
printf '\n%s\n' '--- server wiring ---'
sed -n '380,500p' src/control-plane-services/function-autoscaler/crates/server/src/server.rs
printf '\n%s\n' '--- vault template configmap ---'
sed -n '1,120p' deploy/helm/function-autoscaler/templates/configmap-vault-agent-template.yamlRepository: NVIDIA/nvcf
Length of output: 10977
Make the static-token template path optional.
nvcfApiToken is already optional in the app and falls back to OAuth2, but with secret ... still hard-requires the OpenBao path. Deployments that don’t provision services/nvcf-api/jwt/sign/nvcf-autoscaler-service will keep retrying template render instead of starting with OAuth2. Gate this on a dedicated static-token setting or ensure the path always exists; add coverage for both modes.
🤖 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 `@deploy/helm/function-autoscaler/vault-agent-templates/secrets.json.tmpl`
around lines 10 - 12, Make the static-token block around secret path
services/nvcf-api/jwt/sign/nvcf-autoscaler-service conditional on the dedicated
static-token configuration, so deployments without that setting skip the secret
lookup and allow OAuth2 fallback. Preserve nvcfApiToken rendering when
static-token mode is enabled, and add coverage for both enabled and disabled
modes.
Sources: Coding guidelines, MCP tools
| if let Some(token) = secrets_watcher.get_nvcf_api_token() { | ||
| return Ok(token); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Avoid leaving the refresh loop in a permanent “needs refresh” state.
Lines [171]-[173] return the static token without populating token_cache. The background task therefore continues seeing None as “needs refresh” and invokes this path on every refresh interval, logging a successful refresh each time. Skip background refresh while get_nvcf_api_token() is present, while continuing to re-check it for rotation or removal.
🤖 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 171 - 173, Update the refresh-loop logic around get_nvcf_api_token
and token_cache so a present static NVCF API token prevents repeated background
refresh attempts without caching it as a refreshable token. Continue checking
get_nvcf_api_token on each interval so token rotation or removal is detected,
and only perform dynamic refresh when no static token is present.
1 participant
TL;DR
Wire the function autoscaler into the self-managed stack with the managed observability path it needs to read VictoriaMetrics metrics.
Additional Details (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)
observability.mode: install, or an explicit external TSDB URL inobservability.mode: existing.For the Reviewer
Please review the deployment-mode validation and chart ownership boundaries in:
deploy/stacks/self-managed/helmfile.d/03-observability.yaml.gotmpldeploy/stacks/self-managed/environments/base.yamldeploy/stacks/self-managed/global.yaml.gotmplFor QA (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)
git diff --check origin/main...HEADhelmfile listwith managed observability, state metrics, and function autoscaler enabled renders the full metrics path.observability.mode: existingwith autoscaler enabled requires an explicit TSDB URL.helmfile buildagainst real OCI mirrors, because this local environment uses placeholder registry settings.Issues
Relates to #15
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests