feat(deploy): add K8s autoscaling and NGINX load balancing for NemoClaw - #7459
Closed
maggiezha wants to merge 44 commits into
Closed
feat(deploy): add K8s autoscaling and NGINX load balancing for NemoClaw#7459maggiezha wants to merge 44 commits into
maggiezha wants to merge 44 commits into
Conversation
Provide a tested Kubernetes deployment with GPU-utilization HPA, monitoring, load validation, and optional NGINX routing. Signed-off-by: maggiezha <maggiez@nvidia.com>
…cript re-runs metrics.serviceMonitor.enabled defaulted to false, so every plain `helm upgrade` from install-hpa.sh/hpa-load-test.sh/hpa-reset.sh (none of which use --reuse-values) silently reset it and deleted the ServiceMonitor, breaking per-pod request-count metrics in Grafana even though GPU utilization (scraped by a separate always-on ServiceMonitor) kept working. Also trims the README: drop the now-redundant manual "enable scraping" helm upgrade snippet, add a short troubleshooting note for this failure mode, and fix stale script/path references.
- Export OLLAMA_HOST (was computed but never applied) before `ollama
serve`, and drop other genuinely unused shellcheck-flagged locals in
hpa-common.sh; annotate the one false positive (nameref var) in
hpa-load-test.sh.
- chmod +x on files with a shebang that weren't marked executable
(ollama-start.sh, agent-server.mjs, load-generator.mjs).
- Fix markdownlint (extra blank lines) in README.md.
- Exclude Helm templates/ from the check-yaml hook: Go templating
({{ ... }}) in .yaml files isn't valid standalone YAML.
Signed-off-by: maggiezha <maggiez@nvidia.com>
Contributor
|
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 a GPU autoscaling Helm chart with configurable HPA metrics, Prometheus integration, inference serving, ingress, operational recovery tools, load generation, documentation, and static contract validation. ChangesGPU autoscaling deployment
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Installer
participant Helm
participant Prometheus
participant Adapter
participant KubernetesHPA
participant AgentPods
Installer->>Helm: Install monitoring and GPU chart
Prometheus->>AgentPods: Scrape GPU and agent metrics
Adapter->>Prometheus: Query custom metric series
KubernetesHPA->>Adapter: Read GPU or agent custom metrics
KubernetesHPA->>AgentPods: Adjust replica count
Installer->>KubernetesHPA: Verify rollout and HPA bounds
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
PR Review Advisor — Blocking findings reportedAdvisor assessment: Blockers require maintainer review Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: 2 optional E2E recommendations
Blockers
|
…s on NVIDIA#7459 - hpa-common.sh: hpa_common_release_fullname took $1/$2 overrides that no caller ever passed (ShellCheck: "references arguments, but none are ever passed"). Both callers rely entirely on RELEASE/CHART_NAME env vars, so drop the dead positional-arg fallback instead of just forwarding "$@". - install-hpa.sh: group `kick_deployment && helm_install` before `|| true` so the no-op fallback unambiguously covers the whole compound command, not just helm_install (ShellCheck: "A && B || C is not if-then-else"). - agent-server.mjs: stop echoing raw fetch errors (String(err)) back to HTTP clients (CodeQL: information exposure through a stack trace/error message); log server-side, return a generic message instead. - load-generator.mjs: remove SERVICE_FALLBACK, an unused leftover from an earlier single-URL design now fully superseded by per-pod-IP discovery (CodeQL: unused variable). Add comments on the two "file data in outbound network request" findings (in-cluster SA token/CA, and the bundled sample-questions payload) clarifying both are the intended, non-attacker-controlled data flow for this load generator. Signed-off-by: maggiezha <maggiez@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
deploy/helm/gpu_autoscaling_k8s/files/load-generator.mjs (1)
14-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDead
SERVICE_FALLBACK/TARGET_URLenv contract.
SERVICE_FALLBACKis computed but never referenced — the generator only ever targets per-pod IPs and exits fatally inrequirePodTargetswhen discovery yields nothing. As a result theTARGET_URLenv thathpa-load-test.shsets (lines 250-251) is silently ignored. Either wire it up as an actual fallback target when pod discovery fails, or drop both the constant and the env to avoid a misleading config knob.🤖 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/gpu_autoscaling_k8s/files/load-generator.mjs` around lines 14 - 17, Resolve the unused SERVICE_FALLBACK contract in the load generator: either update requirePodTargets to use SERVICE_FALLBACK as the target when pod discovery returns no pods, preserving the existing fatal path otherwise, or remove SERVICE_FALLBACK and the corresponding TARGET_URL configuration from hpa-load-test.sh. Keep the chosen behavior consistent with the generator’s target selection flow.Source: Linters/SAST tools
🤖 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/gpu_autoscaling_k8s/files/agent-server.mjs`:
- Around line 24-30: Update readBody to enforce a configurable maximum
request-body size while accumulating chunks, stopping or terminating the request
when the limit is exceeded and surfacing a payload-too-large result so the
handler returns HTTP 413. Ensure oversized requests are drained or destroyed to
prevent continued buffering, while preserving normal UTF-8 body parsing and
existing error propagation for bodies within the limit.
- Around line 64-66: Update the catch handler around the backend request in
agent-server.mjs to log the diagnostic error server-side, then return a fixed
generic JSON 502 response instead of exposing String(err) to callers. Preserve
the existing 502 status and JSON content type while removing raw backend details
from the response.
- Around line 61-63: Update the upstream response handling around hubRes and the
proxy response to preserve streaming: pipe hubRes.body directly into res instead
of buffering with hubRes.text(), and forward hubRes.headers.get("content-type")
rather than forcing application/json. Keep the upstream status code unchanged.
In `@deploy/helm/gpu_autoscaling_k8s/monitoring/dcgm-servicemonitor.yaml`:
- Line 1: Prepend the standard two-line SPDX header, using YAML comment syntax,
to deploy/helm/gpu_autoscaling_k8s/monitoring/dcgm-servicemonitor.yaml (lines
1-1), deploy/helm/gpu_autoscaling_k8s/monitoring/kube-prometheus-microk8s.yaml
(lines 1-1), and
deploy/helm/gpu_autoscaling_k8s/monitoring/prometheus-adapter-gpu-values.yaml
(lines 1-1), with the Apache-2.0 identifier and applicable copyright text.
In `@deploy/helm/gpu_autoscaling_k8s/scripts/install-hpa.sh`:
- Line 175: Update the recovery command following hpa_common_kick_deployment to
use the same kick-or-reinstall pattern as hpa-reset.sh: invoke helm_install only
when hpa_common_kick_deployment returns non-zero, while preserving successful
kick behavior and avoiding a trailing unconditional success mask.
In `@deploy/helm/gpu_autoscaling_k8s/templates/_helpers.tpl`:
- Around line 72-80: Update the nemoclaw-gpu.hpaMaxReplicas helper so that when
gpuScaling.oneReplicaPerGpu is enabled, it uses the lower positive limit between
autoscaling.maxReplicas and autoscaling.maxGpus; retain the existing fallback
behavior when those values are not applicable, ensuring the resulting HPA
replica cap never exceeds maxGpus in GPU mode.
In `@deploy/helm/gpu_autoscaling_k8s/templates/deployment.yaml`:
- Around line 42-113: Harden the ollama and agent container definitions with
non-root user/group identities, allowPrivilegeEscalation disabled, and all Linux
capabilities dropped. Enable read-only root filesystems for images that support
them, then add explicit emptyDir mounts for any required writable paths while
preserving the existing ollama-data hostPath mount. Ensure the selected
identities can read mounted application/scripts data and write only to the
intended storage.
In `@deploy/helm/gpu_autoscaling_k8s/templates/service.yaml`:
- Around line 1-4: Add the applicable SPDX copyright and license header to
deploy/helm/gpu_autoscaling_k8s/templates/service.yaml lines 1-4 and
deploy/helm/gpu_autoscaling_k8s/templates/servicemonitor.yaml lines 1-3 using
YAML # comments; add the same header to
deploy/helm/gpu_autoscaling_k8s/templates/NOTES.txt lines 1-4 inside a Helm
template comment so it is not rendered in installation notes.
In `@deploy/helm/gpu_autoscaling_k8s/values-step2-hpa.yaml`:
- Line 1: Add the applicable SPDX copyright and license headers to
deploy/helm/gpu_autoscaling_k8s/values-step2-hpa.yaml,
values-step2-hpa-performance.yaml, values-step2-hpa-latency.yaml, and
values-load-test-hpa.yaml using YAML comments; add equivalent Helm-template
comment headers to templates/configmap.yaml, templates/deployment.yaml,
templates/hpa.yaml, templates/namespace.yaml, and templates/pvc.yaml.
In `@deploy/helm/gpu_autoscaling_k8s/values.yaml`:
- Around line 24-27: Update the image.tag value in the values.yaml image
configuration to an immutable Ollama version tag or digest instead of latest,
while preserving the existing repository and pullPolicy settings.
---
Nitpick comments:
In `@deploy/helm/gpu_autoscaling_k8s/files/load-generator.mjs`:
- Around line 14-17: Resolve the unused SERVICE_FALLBACK contract in the load
generator: either update requirePodTargets to use SERVICE_FALLBACK as the target
when pod discovery returns no pods, preserving the existing fatal path
otherwise, or remove SERVICE_FALLBACK and the corresponding TARGET_URL
configuration from hpa-load-test.sh. Keep the chosen behavior consistent with
the generator’s target selection flow.
🪄 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: 153d6e56-5c8a-4ec7-8d48-2234cf572746
📒 Files selected for processing (35)
.pre-commit-config.yamldeploy/helm/gpu_autoscaling_k8s/.helmignoredeploy/helm/gpu_autoscaling_k8s/Chart.yamldeploy/helm/gpu_autoscaling_k8s/README.mddeploy/helm/gpu_autoscaling_k8s/files/agent-metrics.mjsdeploy/helm/gpu_autoscaling_k8s/files/agent-server.mjsdeploy/helm/gpu_autoscaling_k8s/files/load-generator.mjsdeploy/helm/gpu_autoscaling_k8s/files/ollama-start.shdeploy/helm/gpu_autoscaling_k8s/files/questions-sample.txtdeploy/helm/gpu_autoscaling_k8s/monitoring/dcgm-servicemonitor.yamldeploy/helm/gpu_autoscaling_k8s/monitoring/kube-prometheus-microk8s.yamldeploy/helm/gpu_autoscaling_k8s/monitoring/prometheus-adapter-gpu-values.yamldeploy/helm/gpu_autoscaling_k8s/scripts/cluster-recover.shdeploy/helm/gpu_autoscaling_k8s/scripts/get-agent-pods.shdeploy/helm/gpu_autoscaling_k8s/scripts/get-hpa.shdeploy/helm/gpu_autoscaling_k8s/scripts/hpa-common.shdeploy/helm/gpu_autoscaling_k8s/scripts/hpa-load-test.shdeploy/helm/gpu_autoscaling_k8s/scripts/hpa-reset.shdeploy/helm/gpu_autoscaling_k8s/scripts/hpa-watch.shdeploy/helm/gpu_autoscaling_k8s/scripts/install-hpa.shdeploy/helm/gpu_autoscaling_k8s/templates/NOTES.txtdeploy/helm/gpu_autoscaling_k8s/templates/_helpers.tpldeploy/helm/gpu_autoscaling_k8s/templates/configmap.yamldeploy/helm/gpu_autoscaling_k8s/templates/deployment.yamldeploy/helm/gpu_autoscaling_k8s/templates/hpa.yamldeploy/helm/gpu_autoscaling_k8s/templates/ingress.yamldeploy/helm/gpu_autoscaling_k8s/templates/namespace.yamldeploy/helm/gpu_autoscaling_k8s/templates/pvc.yamldeploy/helm/gpu_autoscaling_k8s/templates/service.yamldeploy/helm/gpu_autoscaling_k8s/templates/servicemonitor.yamldeploy/helm/gpu_autoscaling_k8s/values-load-test-hpa.yamldeploy/helm/gpu_autoscaling_k8s/values-step2-hpa-latency.yamldeploy/helm/gpu_autoscaling_k8s/values-step2-hpa-performance.yamldeploy/helm/gpu_autoscaling_k8s/values-step2-hpa.yamldeploy/helm/gpu_autoscaling_k8s/values.yaml
…VIDIA#7459 Blockers: - hpa-reset.sh: forward INGRESS_HOST to hpa_common_gpu_helm_upgrade like install-hpa.sh does, so reset no longer silently resets a custom Ingress host back to values.yaml's nemoclaw.local default. - agent-server.mjs: cap request body size (MAX_BODY_BYTES, 413) and add a body-read timeout (REQUEST_BODY_TIMEOUT_MS, 408) plus server-level requestTimeout/headersTimeout, so an unauthenticated client sending a large or never-ending body can no longer exhaust pod memory or hold a connection open indefinitely. - Ingress: refuse to render at all unless ingress.tls is configured or ingress.allowInsecureHttp is explicitly set (new templates/ingress.yaml guard) — the install/load-test/reset scripts set that flag themselves as a documented, explicit acknowledgment for their private/dev-cluster use case. Mandatory basic auth (new templates/ingress-auth-secret.yaml) is on by default, auto-generating a password on first install and reusing it across `helm upgrade` re-runs via `lookup`, since none of this chart's scripts pass --reuse-values. - Pin ollama/ollama, node:22-bookworm-slim (agent + load-test images) to digests, and prometheus-community/kube-prometheus-stack, prometheus-community/prometheus-adapter, ingress-nginx/ingress-nginx to explicit chart --version pins in install-hpa.sh, so a later install or reset can no longer silently pull an unreviewed newer artifact. Warnings: - cluster-recover.sh/hpa-reset.sh/hpa_common_clear_stuck_pods: scope destructive pod/Deployment/ReplicaSet/HPA deletes to the chart's selector label or job-name, never a blanket `--all`, so a namespace accidentally shared with unrelated workloads is left alone; documented the recovery boundary (NAMESPACE is assumed dedicated to this chart). - deployment.yaml: add pod/container securityContext — seccomp RuntimeDefault for both containers, dropped capabilities and no privilege escalation for both, and full non-root/read-only-rootfs for the agent sidecar (the ollama container's GPU device-plugin access is left otherwise unconstrained since non-root there is untested here). - New scripts/test-render-contract.sh: static (no cluster) helm template check that HPA scaleTargetRef/metric and Service/ServiceMonitor selectors still agree with the Deployment. - hpa-load-test.sh/hpa-reset.sh: also delete the load test's ServiceAccount/Role/RoleBinding on exit/reset, not just the Job and ConfigMap, so repeated runs don't accumulate unused RBAC objects. Signed-off-by: maggiezha <maggiez@nvidia.com>
maggiezha
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deploy/helm/gpu_autoscaling_k8s/templates/NOTES.txt (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the applicable SPDX license header.
This Helm template has no SPDX header. Add the repository-standard identifier using a Helm template comment so it does not appear in rendered notes.
As per coding guidelines, every source file must include the applicable SPDX license header.
🤖 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/gpu_autoscaling_k8s/templates/NOTES.txt` at line 1, Add the repository-standard SPDX license identifier at the top of the Helm NOTES template using a Helm template comment, ensuring it is omitted from rendered output while preserving the existing HPA command.Source: Coding guidelines
🤖 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/gpu_autoscaling_k8s/scripts/cluster-recover.sh`:
- Around line 5-14: Update cluster recovery cleanup to avoid unconditional
namespace-wide Job deletion: label load-test Jobs with the chart selector and
delete only matching Jobs, or require an explicit safety acknowledgment before
allowing namespace-wide cleanup. Apply this consistently to every kubectl delete
job invocation in the recovery script, while preserving cleanup of chart-owned
resources.
- Line 30: Update the cleanup logic in cluster-recover.sh to wait synchronously,
or explicitly poll until all selected Deployments, Services, HPAs, and
ReplicaSets are gone before reinstalling. Remove the blanket error suppression
so Kubernetes API deletion failures cause recovery to fail, and ensure the
second deletion pass includes Services alongside the other resource kinds.
In `@deploy/helm/gpu_autoscaling_k8s/scripts/test-render-contract.sh`:
- Around line 30-31: Update the preflight checks near require_cmd helm and
require_cmd python3 to validate that Python can import the yaml module before
running the embedded script. Add a quick python3 import check and fail clearly
when PyYAML is unavailable.
In `@deploy/helm/gpu_autoscaling_k8s/templates/NOTES.txt`:
- Line 2: Update the command shown in the Helm NOTES template to use a path or
equivalent kubectl invocation that remains runnable from the installation
context, rather than relying on the user being inside the chart directory.
Preserve the namespace interpolation from the existing get-agent-pods.sh
command.
---
Outside diff comments:
In `@deploy/helm/gpu_autoscaling_k8s/templates/NOTES.txt`:
- Line 1: Add the repository-standard SPDX license identifier at the top of the
Helm NOTES template using a Helm template comment, ensuring it is omitted from
rendered output while preserving the existing HPA command.
🪄 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: 965d8368-b185-4a0a-963c-6b975d8d883d
📒 Files selected for processing (14)
deploy/helm/gpu_autoscaling_k8s/README.mddeploy/helm/gpu_autoscaling_k8s/files/agent-server.mjsdeploy/helm/gpu_autoscaling_k8s/scripts/cluster-recover.shdeploy/helm/gpu_autoscaling_k8s/scripts/hpa-common.shdeploy/helm/gpu_autoscaling_k8s/scripts/hpa-load-test.shdeploy/helm/gpu_autoscaling_k8s/scripts/hpa-reset.shdeploy/helm/gpu_autoscaling_k8s/scripts/install-hpa.shdeploy/helm/gpu_autoscaling_k8s/scripts/test-render-contract.shdeploy/helm/gpu_autoscaling_k8s/templates/NOTES.txtdeploy/helm/gpu_autoscaling_k8s/templates/_helpers.tpldeploy/helm/gpu_autoscaling_k8s/templates/deployment.yamldeploy/helm/gpu_autoscaling_k8s/templates/ingress-auth-secret.yamldeploy/helm/gpu_autoscaling_k8s/templates/ingress.yamldeploy/helm/gpu_autoscaling_k8s/values.yaml
🚧 Files skipped from review as they are similar to previous changes (10)
- deploy/helm/gpu_autoscaling_k8s/templates/ingress.yaml
- deploy/helm/gpu_autoscaling_k8s/templates/deployment.yaml
- deploy/helm/gpu_autoscaling_k8s/templates/_helpers.tpl
- deploy/helm/gpu_autoscaling_k8s/README.md
- deploy/helm/gpu_autoscaling_k8s/values.yaml
- deploy/helm/gpu_autoscaling_k8s/scripts/hpa-reset.sh
- deploy/helm/gpu_autoscaling_k8s/scripts/install-hpa.sh
- deploy/helm/gpu_autoscaling_k8s/files/agent-server.mjs
- deploy/helm/gpu_autoscaling_k8s/scripts/hpa-load-test.sh
- deploy/helm/gpu_autoscaling_k8s/scripts/hpa-common.sh
Rename agent-server, agent-metrics, and load-generator from .mjs to .ts to satisfy the repo's codebase-growth-guardrails check, which blocks newly added .js/.cjs/.mjs source files in favor of TypeScript. The pinned node:22-bookworm-slim image runs .ts files with no build step or flag needed (Node's built-in type-stripping), so behavior is unchanged; only the extension and internal import path changed. Signed-off-by: maggiezha <maggiez@nvidia.com>
Comment on lines
+97
to
+105
| { | ||
| hostname: process.env.KUBERNETES_SERVICE_HOST, | ||
| port: process.env.KUBERNETES_SERVICE_PORT || 443, | ||
| path, | ||
| method: "GET", | ||
| headers: { Authorization: `Bearer ${token}` }, | ||
| ca, | ||
| rejectUnauthorized: true, | ||
| }, |
Comment on lines
+471
to
+475
| body: JSON.stringify({ | ||
| messages: [{ role: "user", content: q }], | ||
| max_tokens: MAX_TOKENS, | ||
| stream: false, | ||
| }), |
Contributor
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/gpu_autoscaling_k8s/files/agent-metrics.ts`:
- Line 6: Update the LLM_LATENCY_WINDOW initialization to parse
LLM_LATENCY_WINDOW_SIZE as a positive finite integer, reject invalid or
non-positive values, and cap accepted values at a safe maximum before
agent-metrics uses it for the llmDurationsMs trim condition.
- Around line 16-23: Update the duration handling in the request metrics flow to
normalize the input once by clamping invalid or negative duration values, then
reuse that normalized duration for both cumulative metrics and the
llmDurationsMs rolling window. Ensure p50, p95, and average calculations consume
the same finite normalized value as llmDurationSumSec and llmDurationCount.
🪄 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: 51d5f078-d837-46e0-a35a-81a4489adf37
📒 Files selected for processing (7)
deploy/helm/gpu_autoscaling_k8s/README.mddeploy/helm/gpu_autoscaling_k8s/files/agent-metrics.tsdeploy/helm/gpu_autoscaling_k8s/files/agent-server.tsdeploy/helm/gpu_autoscaling_k8s/files/load-generator.tsdeploy/helm/gpu_autoscaling_k8s/scripts/hpa-load-test.shdeploy/helm/gpu_autoscaling_k8s/templates/configmap.yamldeploy/helm/gpu_autoscaling_k8s/templates/deployment.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- deploy/helm/gpu_autoscaling_k8s/templates/configmap.yaml
- deploy/helm/gpu_autoscaling_k8s/templates/deployment.yaml
- deploy/helm/gpu_autoscaling_k8s/README.md
- deploy/helm/gpu_autoscaling_k8s/scripts/hpa-load-test.sh
Change values.yaml's default ingress.auth.password from an empty string to YAML null: the empty string tripped CodeQL's js/empty-password-in-configuration-file check even though it's an intentional "auto-generate on install" sentinel, not a real blank credential. Template behavior (`if not $password`) is unchanged for both null and "". Add codeql[js/file-access-to-http] suppression comments (with justification) in load-generator.ts for the two flagged spots: the in-cluster K8s API token/CA send and the bundled sample-question payload. Both are the load generator's intended, non-attacker- controlled data flow; the .mjs->.ts rename reset CodeQL's alert fingerprints so these reappeared as "new" on the new path. Signed-off-by: maggiezha <maggiez@nvidia.com>
…#7459 - agent-server.ts: stream the upstream chat-completions response via pipe instead of buffering with hubRes.text(), and forward its real content-type instead of forcing application/json, so "stream": true requests reach clients incrementally (e.g. text/event-stream). - install-hpa.sh: fix inverted kick/reinstall recovery — helm_install must run when hpa_common_kick_deployment fails (it tore down an unrecoverable Deployment), not when it succeeds. Matches the correct kick-or-reinstall pattern already used in hpa-reset.sh. - _helpers.tpl: cap HPA maxReplicas by maxGpus when gpuScaling.oneReplicaPerGpu is set, so a maxReplicas override can't exceed the GPU count and create unschedulable Pending pods. - hpa-load-test.sh: drop the unused TARGET_URL env var — the load generator only ever targets per-pod IPs via k8s API discovery. - Add missing SPDX headers to 15 chart YAML/tpl/txt files (the spdx-headers pre-commit hook only covers .ts/.py/.sh, not these). Signed-off-by: maggiezha <maggiez@nvidia.com>
agent-metrics.ts: parse LLM_LATENCY_WINDOW_SIZE as a positive finite integer capped at 10000, falling back to the 128 default on invalid input (NaN/Infinity previously disabled the llmDurationsMs trim, letting it grow unbounded). Also normalize durationMs once (finite, non-negative) and reuse that value for both the rolling window and the cumulative sum/count/histogram, so p50/p95/avg and the counters always agree on the same value instead of diverging on bad input. Signed-off-by: maggiezha <maggiez@nvidia.com>
Author
CI fixes since initial reviewAll automated review findings (GitHub Advanced Security/CodeQL, CodeRabbit, and the PR review advisor) have been addressed. Summary of what failed and how it was fixed: Security (CodeQL)
Code quality / robustness (CodeRabbit)
Result: all Remaining (not code issues, need maintainer action):
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
deploy/helm/gpu_autoscaling_k8s/scripts/install-hpa.sh (2)
151-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the duplicate
INFERENCE_MODELdefault.Line 43 already applies the same default. Line 151 has no effect.
♻️ Proposed change
-INFERENCE_MODEL="${INFERENCE_MODEL:-llama3.2:3b}" - ensure_ingress_nginx() {🤖 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/gpu_autoscaling_k8s/scripts/install-hpa.sh` at line 151, Remove the duplicate INFERENCE_MODEL default assignment near the later configuration block; retain the existing default assignment earlier in install-hpa.sh so the variable continues using llama3.2:3b when unset.
107-123: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSurface the Prometheus install failure.
helm upgrade --installredirects stdout and stderr to/dev/nulland ends with|| true. If the chart install fails, the operator sees only the later messagePrometheus not found. Capture stderr and print it when the command fails.♻️ Proposed change
- --wait >/dev/null 2>&1 || true + --wait >/dev/null 2>"${prom_err}" || { + echo "kube-prometheus-stack install did not complete:" >&2 + cat "${prom_err}" >&2 + }Declare
prom_err="$(mktemp)"before the call and remove it afterwards.🤖 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/gpu_autoscaling_k8s/scripts/install-hpa.sh` around lines 107 - 123, Update the Prometheus installation block around helm upgrade --install to capture stderr in a temporary file, report the captured error when the command fails, and remove the temporary file afterward. Preserve the existing successful flow while ensuring installation failures are surfaced instead of silently ignored.deploy/helm/gpu_autoscaling_k8s/scripts/hpa-common.sh (2)
126-137: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe script-style header assumes GPU mode.
The header always prints
GPU utilization rate (avg per pod)and theGPU UTIL %column.templates/hpa.yamlsupportsgpu,performance,latency, and CPU modes. In the non-GPU modes the header mislabels the values. Derive the column label from the first spec metric, or use a neutralTARGETSlabel.🤖 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/gpu_autoscaling_k8s/scripts/hpa-common.sh` around lines 126 - 137, The non-kubectl header in the header-printing logic assumes GPU metrics and mislabels performance, latency, and CPU modes. Update the relevant script-style output to derive its metric description and column label from the first spec metric, or use the neutral TARGETS label, while preserving the existing GPU output.
652-676: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe
maxparameter is unused.
hpa_common_verify_hpa_boundsacceptsmaxat line 657 but never reads it.install-hpa.shline 228 passesMAX_REPLICAS, so the caller expects an upper-bound check. Either verify the HPAspec.maxReplicasagainstmaxand report a mismatch, or drop the parameter and update the callers.🤖 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/gpu_autoscaling_k8s/scripts/hpa-common.sh` around lines 652 - 676, The max parameter in hpa_common_verify_hpa_bounds is unused despite callers passing MAX_REPLICAS. Implement an upper-bound validation by reading the HPA spec.maxReplicas, comparing it with max, and reporting a mismatch with a nonzero return; preserve the existing lower-bound enforcement and successful behavior when the configured maximum matches.deploy/helm/gpu_autoscaling_k8s/templates/configmap.yaml (1)
11-16: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
.Files.Getfails silently for a missing or renamed file.If a path under
files/changes,.Files.Getreturns an empty string. The ConfigMap then renders an empty key, and the container starts with an empty script instead of failing the install. Wrap each read withrequiredso a rename breaks the render.♻️ Proposed fail-fast guard
agent-server.ts: | -{{ .Files.Get "files/agent-server.ts" | indent 4 }} +{{ required "files/agent-server.ts is missing from the chart" (.Files.Get "files/agent-server.ts") | indent 4 }} agent-metrics.ts: | -{{ .Files.Get "files/agent-metrics.ts" | indent 4 }} +{{ required "files/agent-metrics.ts is missing from the chart" (.Files.Get "files/agent-metrics.ts") | indent 4 }} ollama-start.sh: | -{{ .Files.Get "files/ollama-start.sh" | indent 4 }} +{{ required "files/ollama-start.sh is missing from the chart" (.Files.Get "files/ollama-start.sh") | indent 4 }}🤖 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/gpu_autoscaling_k8s/templates/configmap.yaml` around lines 11 - 16, Update the ConfigMap entries for agent-server.ts, agent-metrics.ts, and ollama-start.sh to wrap each .Files.Get result with Helm’s required function, using a clear non-empty error message so missing or renamed files fail template rendering instead of producing empty scripts.deploy/helm/gpu_autoscaling_k8s/files/load-generator.ts (1)
500-539: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRetired target workers spin until the test ends.
syncTargetWorkerssetsworker.limit = 0for targets that disappear, andworkerPromiseskeeps the promise.runTargetWorkerthen loops withsleep(20)untilendAt. After a scale-down, each removed pod leaves an idle 50 Hz loop for the rest of the run. Add an exit condition when the limit is 0 and the target is no longer active, and delete the entry fromworkerPromises.🤖 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/gpu_autoscaling_k8s/files/load-generator.ts` around lines 500 - 539, Update runTargetWorker and syncTargetWorkers so a worker whose limit is 0 and target is no longer active exits immediately instead of sleeping until endAt. When retiring such a worker, also remove its promise from workerPromises while preserving normal completion and active-worker behavior.deploy/helm/gpu_autoscaling_k8s/templates/ingress-auth-secret.yaml (1)
29-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEach render produces a new bcrypt hash.
bcryptgenerates a random salt, soauthchanges on everyhelm upgradeeven when the password is unchanged. The Secret is patched each time, and ingress-nginx reloads the auth file. Authentication still succeeds. If you want a stable Secret, reuse theauthvalue from$existingwhen the password did not change.🤖 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/gpu_autoscaling_k8s/templates/ingress-auth-secret.yaml` around lines 29 - 32, The ingress auth template currently regenerates the bcrypt hash on every render. Update the auth value logic around the existing `$existing` Secret to reuse its current auth content when the configured username and password are unchanged, and only generate a new bcrypt hash when the password changes or no existing value is available.deploy/helm/gpu_autoscaling_k8s/files/agent-server.ts (1)
96-128: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winLatency metric includes client stream time.
recordLlmLatencyruns after the response finishes streaming to the client. For"stream": truerequests, the recorded value mixes upstream inference latency and client read time. The HPAlatencymode targetsnemoclaw_llm_latency_p95_milliseconds, so slow clients can inflate the scaling signal. Consider recording the time to the first upstream byte, or the time tohubResheaders.♻️ Proposed change
llmOk = hubRes.ok; + // Record inference latency at upstream response headers, before client streaming. + recordLlmLatency(performance.now() - llmStart, llmOk); + llmRecorded = true;Then guard the
finallyblock withif (!llmRecorded).🤖 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/gpu_autoscaling_k8s/files/agent-server.ts` around lines 96 - 128, Update the chat completion timing in the request handler around llmStart and recordLlmLatency so it captures upstream response-header or first-byte latency instead of waiting for pipeline to finish streaming to the client. Add an llmRecorded guard and ensure the finally block records latency only when it has not already been recorded, while preserving metric recording for errors and non-streaming responses.
🤖 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/gpu_autoscaling_k8s/scripts/hpa-common.sh`:
- Around line 52-83: Update targets() so non-GPU Pods metrics include their
metric name in the formatted output, matching the Resource branch, while
preserving GPU-specific formatting for gpu_utilization_percent. Also update
hpa_common_format_hpa at deploy/helm/gpu_autoscaling_k8s/scripts/hpa-common.sh
lines 126-137 to derive the column label and subtitle from the first spec metric
instead of hard-coding GPU UTIL %.
- Around line 640-650: Update hpa_common_enforce_replica_floor to validate that
spec is a numeric integer before comparing it with min; treat empty or
non-numeric values as requiring the existing replica-floor patch, while
preserving the current comparison and patch behavior for valid values.
In `@deploy/helm/gpu_autoscaling_k8s/scripts/test-render-contract.sh`:
- Around line 37-41: Update the helm template assertion in
test-render-contract.sh to capture the command’s output and verify it contains
the specific no-TLS error emitted by templates/ingress.yaml, rather than
accepting any nonzero exit status. Keep the failure path for successful
rendering and unrelated render errors distinct so only the expected TLS-policy
rejection passes.
In `@deploy/helm/gpu_autoscaling_k8s/templates/deployment.yaml`:
- Around line 1-2: Replace the Helm template comments at
deploy/helm/gpu_autoscaling_k8s/templates/deployment.yaml lines 1-2 with
equivalent YAML comments using # while preserving both SPDX header texts; make
the same replacement at deploy/helm/gpu_autoscaling_k8s/templates/pvc.yaml lines
1-2.
- Line 102: Update the deployment configuration around the node command and
mounted TypeScript files so Node recognizes the agent entry point as ESM. Add a
package.json under /app with "type": "module", or consistently rename the entry
point and related imports to .mts, or compile the files to JavaScript before
execution; preserve the existing agent-server startup behavior.
---
Nitpick comments:
In `@deploy/helm/gpu_autoscaling_k8s/files/agent-server.ts`:
- Around line 96-128: Update the chat completion timing in the request handler
around llmStart and recordLlmLatency so it captures upstream response-header or
first-byte latency instead of waiting for pipeline to finish streaming to the
client. Add an llmRecorded guard and ensure the finally block records latency
only when it has not already been recorded, while preserving metric recording
for errors and non-streaming responses.
In `@deploy/helm/gpu_autoscaling_k8s/files/load-generator.ts`:
- Around line 500-539: Update runTargetWorker and syncTargetWorkers so a worker
whose limit is 0 and target is no longer active exits immediately instead of
sleeping until endAt. When retiring such a worker, also remove its promise from
workerPromises while preserving normal completion and active-worker behavior.
In `@deploy/helm/gpu_autoscaling_k8s/scripts/hpa-common.sh`:
- Around line 126-137: The non-kubectl header in the header-printing logic
assumes GPU metrics and mislabels performance, latency, and CPU modes. Update
the relevant script-style output to derive its metric description and column
label from the first spec metric, or use the neutral TARGETS label, while
preserving the existing GPU output.
- Around line 652-676: The max parameter in hpa_common_verify_hpa_bounds is
unused despite callers passing MAX_REPLICAS. Implement an upper-bound validation
by reading the HPA spec.maxReplicas, comparing it with max, and reporting a
mismatch with a nonzero return; preserve the existing lower-bound enforcement
and successful behavior when the configured maximum matches.
In `@deploy/helm/gpu_autoscaling_k8s/scripts/install-hpa.sh`:
- Line 151: Remove the duplicate INFERENCE_MODEL default assignment near the
later configuration block; retain the existing default assignment earlier in
install-hpa.sh so the variable continues using llama3.2:3b when unset.
- Around line 107-123: Update the Prometheus installation block around helm
upgrade --install to capture stderr in a temporary file, report the captured
error when the command fails, and remove the temporary file afterward. Preserve
the existing successful flow while ensuring installation failures are surfaced
instead of silently ignored.
In `@deploy/helm/gpu_autoscaling_k8s/templates/configmap.yaml`:
- Around line 11-16: Update the ConfigMap entries for agent-server.ts,
agent-metrics.ts, and ollama-start.sh to wrap each .Files.Get result with Helm’s
required function, using a clear non-empty error message so missing or renamed
files fail template rendering instead of producing empty scripts.
In `@deploy/helm/gpu_autoscaling_k8s/templates/ingress-auth-secret.yaml`:
- Around line 29-32: The ingress auth template currently regenerates the bcrypt
hash on every render. Update the auth value logic around the existing
`$existing` Secret to reuse its current auth content when the configured
username and password are unchanged, and only generate a new bcrypt hash when
the password changes or no existing value is available.
🪄 Autofix
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: 675d8dd4-78fd-46da-81d3-5f6151682d96
📒 Files selected for processing (38)
.pre-commit-config.yamldeploy/helm/gpu_autoscaling_k8s/.helmignoredeploy/helm/gpu_autoscaling_k8s/Chart.yamldeploy/helm/gpu_autoscaling_k8s/README.mddeploy/helm/gpu_autoscaling_k8s/files/agent-metrics.tsdeploy/helm/gpu_autoscaling_k8s/files/agent-server.tsdeploy/helm/gpu_autoscaling_k8s/files/load-generator.tsdeploy/helm/gpu_autoscaling_k8s/files/ollama-start.shdeploy/helm/gpu_autoscaling_k8s/files/questions-sample.txtdeploy/helm/gpu_autoscaling_k8s/monitoring/dcgm-servicemonitor.yamldeploy/helm/gpu_autoscaling_k8s/monitoring/kube-prometheus-microk8s.yamldeploy/helm/gpu_autoscaling_k8s/monitoring/prometheus-adapter-gpu-values.yamldeploy/helm/gpu_autoscaling_k8s/scripts/cluster-recover.shdeploy/helm/gpu_autoscaling_k8s/scripts/get-agent-pods.shdeploy/helm/gpu_autoscaling_k8s/scripts/get-hpa.shdeploy/helm/gpu_autoscaling_k8s/scripts/hpa-common.shdeploy/helm/gpu_autoscaling_k8s/scripts/hpa-load-test.shdeploy/helm/gpu_autoscaling_k8s/scripts/hpa-reset.shdeploy/helm/gpu_autoscaling_k8s/scripts/hpa-watch.shdeploy/helm/gpu_autoscaling_k8s/scripts/install-hpa.shdeploy/helm/gpu_autoscaling_k8s/scripts/test-render-contract.shdeploy/helm/gpu_autoscaling_k8s/scripts/test-script-security-contract.shdeploy/helm/gpu_autoscaling_k8s/templates/NOTES.txtdeploy/helm/gpu_autoscaling_k8s/templates/_helpers.tpldeploy/helm/gpu_autoscaling_k8s/templates/configmap.yamldeploy/helm/gpu_autoscaling_k8s/templates/deployment.yamldeploy/helm/gpu_autoscaling_k8s/templates/hpa.yamldeploy/helm/gpu_autoscaling_k8s/templates/ingress-auth-secret.yamldeploy/helm/gpu_autoscaling_k8s/templates/ingress.yamldeploy/helm/gpu_autoscaling_k8s/templates/namespace.yamldeploy/helm/gpu_autoscaling_k8s/templates/pvc.yamldeploy/helm/gpu_autoscaling_k8s/templates/service.yamldeploy/helm/gpu_autoscaling_k8s/templates/servicemonitor.yamldeploy/helm/gpu_autoscaling_k8s/values-load-test-hpa.yamldeploy/helm/gpu_autoscaling_k8s/values-step2-hpa-latency.yamldeploy/helm/gpu_autoscaling_k8s/values-step2-hpa-performance.yamldeploy/helm/gpu_autoscaling_k8s/values-step2-hpa.yamldeploy/helm/gpu_autoscaling_k8s/values.yaml
🚧 Files skipped from review as they are similar to previous changes (20)
- deploy/helm/gpu_autoscaling_k8s/.helmignore
- deploy/helm/gpu_autoscaling_k8s/scripts/hpa-watch.sh
- deploy/helm/gpu_autoscaling_k8s/values-step2-hpa-performance.yaml
- deploy/helm/gpu_autoscaling_k8s/files/questions-sample.txt
- deploy/helm/gpu_autoscaling_k8s/scripts/get-hpa.sh
- deploy/helm/gpu_autoscaling_k8s/values-step2-hpa-latency.yaml
- deploy/helm/gpu_autoscaling_k8s/files/ollama-start.sh
- deploy/helm/gpu_autoscaling_k8s/monitoring/kube-prometheus-microk8s.yaml
- deploy/helm/gpu_autoscaling_k8s/monitoring/prometheus-adapter-gpu-values.yaml
- .pre-commit-config.yaml
- deploy/helm/gpu_autoscaling_k8s/files/agent-metrics.ts
- deploy/helm/gpu_autoscaling_k8s/values-step2-hpa.yaml
- deploy/helm/gpu_autoscaling_k8s/Chart.yaml
- deploy/helm/gpu_autoscaling_k8s/scripts/hpa-reset.sh
- deploy/helm/gpu_autoscaling_k8s/monitoring/dcgm-servicemonitor.yaml
- deploy/helm/gpu_autoscaling_k8s/scripts/hpa-load-test.sh
- deploy/helm/gpu_autoscaling_k8s/values.yaml
- deploy/helm/gpu_autoscaling_k8s/values-load-test-hpa.yaml
- deploy/helm/gpu_autoscaling_k8s/templates/_helpers.tpl
- deploy/helm/gpu_autoscaling_k8s/scripts/get-agent-pods.sh
Author
|
@apurvvkumaria Confirmed resolved on newer head
Verification on exact head
Please re-review exact head |
cv
dismissed
apurvvkumaria’s stale review
August 7, 2026 08:00
Commit d4ce708 scopes recovery to chart-owned resources and the named load-test Job, defaults ingress to TLS, and requires an isolation preflight for explicit cleartext use. Later commits add TLS enforcement tests.
cv
requested changes
Aug 7, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
Product scope blocks this PR. It has no linked accepted issue or design decision, while NemoClaw's canonical platform documentation explicitly classifies native Kubernetes and OpenShift deployments as unsupported. Merging this Helm chart, autoscaling contract, NGINX ingress, persistence modes, operational scripts, and documentation would create a supported deployment surface without defined ownership, lifecycle, compatibility, security, or validation expectations. Route this independent solution through NemoClaw Community, or first obtain an accepted product decision that defines those requirements. Passing chart tests and partial security hardening do not establish product approval.
29 tasks
Author
Thank you! Moved it to: NVIDIA/nemoclaw-community#111 |
Collaborator
|
Closing this as NVIDIA/nemoclaw-community#111 is its new home. Thanks @maggiezha! |
apurvvkumaria
pushed a commit
to NVIDIA/nemoclaw-community
that referenced
this pull request
Aug 13, 2026
## Related Issue This community recipe continues and re-homes the public work from [NVIDIA/NemoClaw#7459](NVIDIA/NemoClaw#7459) into `examples/recipes/nvidia/kubernetes-gpu-autoscaling/`. Architecture follow-ups in this PR (beyond the imported baseline): 1. **OpenShell CPU sandbox** — NemoClaw/OpenClaw runs as a CPU-only sandbox under OpenShell + Agent Sandbox. Only Ollama inference pods request GPUs. The sandbox policy removes NVIDIA-hosted inference before agent start. 2. **Optional Envoy Gateway LeastRequest** — users may enable Envoy (default) for LeastRequest balancing across HPA replicas, or disable it (`ENABLE_ENVOY_LB=0`) and use the agent Service only. Replaces ingress-nginx when enabled. Envoy dataplane stays ClusterIP while the OpenShell cleartext HTTP listener is present. 3. **Inference API key** — Chart-generated local Secret for Bearer auth on model/chat; OpenShell injects it (users need not supply one). Not for Ollama model pulls; not OpenAI/`NVIDIA_API_KEY`. 4. **HPA metrics (Pods `AverageValue`)** — two documented examples: average per-pod **GPU utilization > 40%** and average per-pod **LLM chat proxy latency > 3000 ms**. These are examples; operators can choose other metrics or define their own customized Prometheus → Adapter → HPA metrics. ## Description Add an experimental NVIDIA-authored Kubernetes recipe that runs a CPU-only NemoClaw/OpenClaw agent in an OpenShell sandbox and sends inference through **Envoy Gateway (LeastRequest)** to authenticated Ollama replicas on NVIDIA GPUs in the same cluster. A Kubernetes Horizontal Pod Autoscaler scales only the Ollama inference pods using Pods **`AverageValue`** custom metrics — documented examples are per-pod **GPU utilization** (`gpu_utilization_percent` via DCGM → Prometheus → Adapter) and per-pod **LLM latency** (`nemoclaw_llm_latency_avg_milliseconds` via metrics-proxy `/metrics` → Prometheus → Adapter). Latency is the metrics-proxy chat/completions proxy duration (from just before the in-pod inference call until the full response is written to the client). The runtime path is entirely on-premises. HPA scales to **N** Ollama pods (1 GPU each); Envoy only changes load balancing (LeastRequest vs the agent ClusterIP Service when `ENABLE_ENVOY_LB=0`): ```text OpenShell CPU sandbox ↓ Envoy Gateway — LeastRequest (or metrics-proxy Service when ENABLE_ENVOY_LB=0) ↓ Authenticated inference endpoints ├─ Ollama pod → GPU 1 ├─ Ollama pod → GPU 2 ├─ … └─ Ollama pod → GPU N ↑ HPA (examples: GPU util >40% or latency >3000 ms) ``` Operators can toggle Envoy with `ENABLE_ENVOY_LB` / `ingress.gateway.enabled`, verify the sandbox path with `./scripts/verify-nemoclaw-sandbox.sh`, and keep the Envoy dataplane as ClusterIP while the OpenShell cleartext HTTP listener exists. **Inference API key.** The chart generates a local Secret for Bearer auth on model/chat; users do not supply a cloud key. OpenShell injects it for the sandbox path — not for Ollama model pulls, and not an OpenAI/`NVIDIA_API_KEY` credential. The sandbox policy removes and verifies removal of the inherited `integrate.api.nvidia.com` endpoint before the agent starts. ### Source migration | Source | Destination | | --- | --- | | [`NVIDIA/NemoClaw` `deploy/helm/gpu_autoscaling_k8s/` @ `77334cc`](NVIDIA/NemoClaw@77334cc) ([#7459](NVIDIA/NemoClaw#7459)) | `NVIDIA/nemoclaw-community: examples/recipes/nvidia/kubernetes-gpu-autoscaling/` | - Imported chart files from exact source head [`77334ccbbadba2c5079fe9e99fe80a3cddc846b5`](NVIDIA/NemoClaw@77334cc). - Preserve compatibility-sensitive Helm release, namespace, Service, metric, and label identifiers. - Make the community directory the canonical source after migration. Future compatibility changes should be reviewed and applied here rather than mirrored from the old pull-request branch. - Pin the experimental integration to NemoClaw `v0.0.104` at `f389c9d872775006ae069473f58250fa8f3ad40f`, OpenShell `0.0.85`, and Agent Sandbox `v0.5.0`. - Follow-up on this branch replaces ingress-nginx with Envoy Gateway and pins LeastRequest on the OpenShell and external inference routes. ### NemoClaw and OpenShell path - Build and push a deployment-specific NemoClaw image without embedding credentials. - Install an internal-only OpenShell Kubernetes gateway after the operator explicitly installs the pinned Agent Sandbox controller. - Require OIDC by default. The unauthenticated mode requires a separate dedicated-cluster, port-forward-only acknowledgement. - Configure the OpenShell inference provider to the in-cluster **Envoy Gateway** dataplane (`LeastRequest`), which forwards Bearer-authenticated requests to the Ollama inference pods. - Create the NemoClaw sandbox without a GPU request, remove the hosted-inference endpoint, and run NemoClaw in a foreground OpenShell exec session. - Document the OpenShell `0.0.85` idle-sandbox lifecycle, capabilities, privilege-separation limitation, cleanup, and coordinated pin-update procedure. ### Security and lifecycle boundaries - **Inference API key:** chart-generated local Secret; OpenShell injects Bearer for sandbox traffic (users need not supply a key). Not for Ollama pulls; not OpenAI/`NVIDIA_API_KEY`. Timing-safe check on model/completion traffic (`Authorization: Bearer` on the OpenShell/Envoy path, or `X-Api-Key` for external clients when Gateway Basic auth owns `Authorization`). Health, readiness, and metrics remain unauthenticated. - Generate and retain a random inference API key, or preserve an operator-managed Secret name and key end to end. Operational scripts discover the effective Secret contract from the installed Helm release. - Keep TLS required by default. Enable Gateway Basic authentication on the **external** HTTPS HTTPRoute; the OpenShell HTTPRoute does not use Gateway Basic auth so Bearer can pass through Envoy. - Keep the Envoy dataplane Service as **ClusterIP** while the hostname-unrestricted OpenShell cleartext HTTP listener is present; reject `NodePort`/`LoadBalancer` in the chart and installer so that listener cannot bypass hostname-scoped HTTPS redirect and Basic auth when externally exposed. - State that direct Service access bypasses Gateway TLS and Basic authentication but not the application API-key check, and that the chart creates no NetworkPolicy. - Document Prometheus Adapter, **Envoy Gateway**, DCGM ServiceMonitor, Agent Sandbox, OpenShell, and MicroK8s mutations and retained resources. - Default `MAX_REPLICAS` / `TARGET_PODS` to allocatable GPU count **N** (override only for an intentional lower ceiling). - Synchronize the HPA replica maximum and GPU safety ceiling so explicit targets are not capped by an outdated lower `maxGpus` default. - Make Helm uninstall the default cleanup. Namespace deletion requires an explicit exclusive-ownership check. This remains an unsupported, non-production community experiment; it does not establish native Kubernetes as a supported NemoClaw product surface. ## Verification - [x] `python3 scripts/check_license_headers.py --check` - [x] `python3 scripts/check_label_taxonomy.py` - [x] `python3 -m unittest scripts.tests.test_governance_taxonomy` - [x] `python3 scripts/check_pr_title.py --advisory 'feat(examples): add Kubernetes GPU autoscaling recipe'` - [x] `git diff --check` - [x] Bash syntax validation for the recipe scripts - [x] `bash scripts/test-script-security-contract.sh` - [x] `bash scripts/test-nemoclaw-k8s-contract.sh` - [x] `node scripts/test-inference-auth-contract.mjs` - [x] `bash scripts/test-render-contract.sh` with Helm 3.20.2 on `PATH` - [x] `helm lint . --set ingress.allowInsecureHttp=true --set-string ingress.auth.htpasswd='demo:{SHA}<base64-sha1>'` (Apache `{SHA}` htpasswd required by Envoy Gateway Basic auth) - [x] Render coverage for mandatory inference authentication, TLS redirect, operator-managed Secrets, long Secret names, scalar-like and dotted Secret keys, RWX persistence, and single-node `hostPath` - [x] Current branch is rebased onto the latest fetched NVIDIA `main`; it is zero commits behind - [x] Current head includes OpenShell sandbox verification and Envoy LeastRequest/ClusterIP hardening commits on this branch - [x] Latency HPA idle-expire: `metrics.llmLatencyIdleExpireMs` / `LLM_LATENCY_IDLE_EXPIRE_MS` (default 60s) resets `nemoclaw_llm_latency_avg_milliseconds` after idle so HPA can scale down; covered by `scripts/test-metrics-proxy-metrics-contract.mjs` - [x] Built-in HPA modes restricted to live-validated `gpu_utilization` and `latency_avg` (retired `latency_p50` / `latency_p95` / `request_rate`; render contract rejects them). Mode-aware `hpa-load-test.sh` via `HPA_METRIC=...` - [x] Legacy `*-agent` → `*-metrics-proxy` migration: `hpa_common_migrate_pre_metrics_proxy_resources` detects leftovers by basename and label, deletes them before ensure/Helm (install/reset/load-test); contract-tested; live-validated on 4× L40S (injected competing `nemoclaw-gpu-agent`, removed, then install + auth inference + HPA 1→2→1 + Envoy LeastRequest on `nemoclaw-gpu-metrics-proxy` only) - [x] GPU front-door renamed from confusing “agent” to `metrics-proxy` (Ollama + metrics-proxy containers); OpenShell/NemoClaw AI agent remains CPU-only in the sandbox - [x] Live Kubernetes/GPU validation on [**Brev**](https://brev.nvidia.com) (AWS), single-node MicroK8s, **4× NVIDIA L40S** (48 GB GDDR6 each): chart deploy, Envoy Gateway LeastRequest, authenticated inference (external Basic + `X-Api-Key`; OpenShell-path Bearer through Envoy), HPA on **GPU utilization** (`gpu_utilization_percent`, average per-pod util > 40%) and **LLM latency** (`nemoclaw_llm_latency_avg_milliseconds`, average per-pod latency > 3000 ms) via `hpa-load-test.sh`, Envoy LeastRequest distribution check (concurrent chat completions across Ready GPU pods), OpenShell sandbox creation (`nemoclaw-onprem`), and authenticated model + chat/completions requests from the sandbox through `https://inference.local/v1` (OpenShell → Envoy LeastRequest → Ollama). - [ ] **H100 validation (non-blocking follow-up)** — optional later work after time-slicing is removed; 4× L40S evidence is sufficient for this experimental recipe. ## Documentation Writer Review - [x] Documentation writer review completed for the final changes - Result: `docs-updated` - Evidence or justification: Independent review covered the README, catalog, notices, Helm notes, scripts, templates, and tests. It confirmed the pinned lifecycle and capability claims, CPU-only sandbox boundary, authenticated on-premises Envoy route, hosted-endpoint removal, operator-managed Secret behavior, retained-resource cleanup, synchronized GPU ceiling, dual HPA examples (GPU util >40% and latency >3000 ms) with custom-metric guidance, latency proxy timing, and L40S validation guidance. Bash syntax, Helm render, native Kubernetes, recovery/security, inference authentication, SPDX, and diff checks passed. - Reviewer: Independent Codex documentation-writer review - Reviewed head: `970c1db890dc5fde2fcfb9a4dac4489ae396d72d` (restores sanitized README screenshots; migration before ensure/helm in load-test/reset; security-contract cleanup exit 0; metrics-proxy naming + pre-metrics-proxy migration) - [x] Changed user-facing text follows the [writing guide](https://github.com/NVIDIA/nemoclaw-community/blob/main/WRITING.md) and [controlled-word list](https://github.com/NVIDIA/nemoclaw-community/blob/main/.agents/skills/_shared/controlled-words.md). - [x] A public contributor can understand the changed text without internal company context. - [x] I reviewed any agent-generated text before submission. Contributor review is required before merge. ## Release And Compliance - [x] No secrets or credentials are included, including API keys, access tokens, passwords, local `.env` files, private certificates, or token caches. - [x] No nonpublic project names, environment names, hostnames, URLs, ticket identifiers, workspace paths, logs, screenshots, or configuration values are included. - [x] Third-party dependency changes are reflected in `THIRD-PARTY-NOTICES`. - [x] Public content uses sanitized examples and placeholders instead of private values. - [x] I added my DCO sign-off declaration to this pull request description. Signed-off-by: maggiezha <maggiez@nvidia.com> --------- Signed-off-by: maggiezha <42832776+maggiezha@users.noreply.github.com> Signed-off-by: maggiezha <maggiez@nvidia.com>
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
7 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
Adds a Helm chart (
deploy/helm/gpu_autoscaling_k8s) that autoscales NemoClaw on Kubernetes using the per-podgpu_utilization_percentHPA metric, with an NGINX Ingress in front of the agent pods.The current revision also scopes recovery to chart-owned resources, requires TLS by default, validates the configured readiness model, and rejects shared PVC persistence that is not
ReadWriteMany.Related Issue
Changes
nvidia.com/gpu.ssl-redirect: "true"for TLS Ingress, and keep cleartext HTTP behind an explicit isolation preflight.ReadWriteManystorage class for shared PVC persistence while preservingemptyDirand single-nodehostPathmodes.gpu_utilization_percentPods metric and remove unsupported performance, latency, CPU, and memory HPA modes.Type of Change
Quality Gates
Documentation Writer Review
deploy/helm/gpu_autoscaling_k8s/README.md,deploy/helm/gpu_autoscaling_k8s/templates/NOTES.txt, and script help; exact-head review passed for RWX persistence, HTTPS redirect, working-directory-independent Helm NOTES, and the GPU-only HPA support boundary, including observability-only request and latency metrics; no documentation change was required for the internal malformed replica-count guard, test-only TLS-policy assertion, YAML SPDX correction, or internal Node ESM metadata.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every new commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailable — normal hooks passed for the signed commits and fast-forward pushes.test-render-contract.shandtest-script-security-contract.shpass onab009dcb; Bash syntax andgit diff --checkalso pass.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — not applicable to the chart-scoped change;npm run checks:repositorypasses.npm run docsbuilds without warnings (doc changes only) — the command passes; Fern reports two warnings.Signed-off-by: maggiezha maggiez@nvidia.com
Summary by CodeRabbit
New Features
Documentation
Tests
Chores