feat(power-agent): add DCGM dual actuator (opt-in; NVML remains default)

[kaim-eng]

Power Agent DCGM actuator (PR 1b of the per-node power-cap stack).

Predecessor: #9682 ΓÇö Power Agent NVML DaemonSet ΓÇö has landed on main (squash 82f5389f51). Base is main. The branch has been rebased onto current main (head bd3da174a2); it is conflict-free and GitHub reports MERGEABLE.

Summary

Adds an opt-in DCGM actuator to the Power Agent introduced in #9682. Operators running the GPU Operator with dcgm.enabled=true can set agent.actuator=dcgm so caps are written through the cluster nvidia-dcgm hostengine. Default NVML behavior is unchanged for operators that do not opt in; the implementation is refactored behind an Actuator protocol and a shared managed-state module, with a transaction-verified GPU UUID anchoring every DCGM cap write against re-enumeration.

Branch shape: 34 files changed vs main, in 7 commits (head bd3da174a2). No docs/ changes.

Commits (post-rebase SHAs):

1. 943fc660 ΓÇö feat(power-agent): add DCGM dual actuator (opt-in; NVML remains default) ΓÇö the actuator protocol, NVML/DCGM implementations, managed-state module, Helm wiring, and tests.

2. afab6220 ΓÇö fix(power-agent): fail closed on unverifiable GPU identity in DCGM cap writes ΓÇö every DCGM cap write/restore is anchored to a transaction-verified GPU UUID and fails closed when identity cannot be established; the reconcile loop restores caps on shutdown; ownership retirement is durable across a persistence failure without repeating the hardware release.

3. bae46171 ΓÇö fix(power-agent): make orphan recovery identity-conclusive and persistence durable ΓÇö cold-start orphan recovery resolves identities from one conclusive scan_uuid_index_map() snapshot, transient read failures no longer erase persisted state, and acquisition/retirement persistence is retried by the reconcile loop.

4. ed2de2e6 ΓÇö chore(power-agent): address PR #9790 review feedback ΓÇö bumps the vendored DCGM image to 4.5.3-1-ubuntu24.04 (reviewer asked for ubuntu24; validated the ubuntu24.04 libdcgm.so.4 loads on the python:3.12-slim-bookworm runtime and the in-image suite passes on the new base) and removes the three now-stale .lycheeignore deployment-guide entries (net-zero vs main).

5. b20a2c66 ΓÇö chore(power-agent): remove inert DCGM logger shim; clarify vendor ABI note ΓÇö the vendored pydcgm bindings guard import logger behind __DCGM_TESTING_FRAMEWORK_ACTIVE (never set by the agent), so the logger.py shim was never imported (dead code); removed it and its COPY, corrected the inaccurate Dockerfile comment, and tightened the DCGM_IMAGE ABI guidance to require a load-smoke for future tag overrides.

6. 2567d2c6 ΓÇö fix(power-agent): close DCGM zero-topology gaps and address sttts review ΓÇö re-probes device_count() when the DCGM topology reads empty and restores the outcome-semantics / dedup-by-pod-UID fixes; moves pending_acquisition into managed_state.pending_acquisition so the actuator's import power_agent copy and the __main__ reconcile flush share one retry queue (with shared-object identity tests); rejects DCGM numeric blank power-limit sentinels (>= 0x7FFFFFF0) without importing dcgmvalue; removes the client-side K8S_LIST_TIMEOUT_S/_request_timeout on pod LIST while keeping resource_version="0" watch-cache reads; and adds deploy/helm/charts/power-agent/Makefile, wired into PR and post-merge CI via make -C deploy/helm/charts/power-agent lint test.

7. bd3da174a — fix(power-agent): bound pod listing during shutdown — bounds pod LIST calls with a 20s apiserver timeout and 25s best-effort client timeout, disables transport retries on the Power Agent's dedicated Kubernetes client, skips new reconcile work once SIGTERM is requested, and makes the Helm termination grace period configurable with a validated 60s minimum.

The --dcgm-enforce flag, dcgm_enforce_failures_total metric, agent.dcgm.enforce value, and the validate_enforce helm suite that appeared in earlier revisions of this branch have been removed; the shipped surface is the three CLI flags and six metrics listed below.

Changes:

1. Power Agent implementation (deploy/power-agent/):

   • actuator.py: Actuator protocol, NvmlActuator, and DcgmActuator. UUID-keyed identity mapping, _with_reconnect for DCGM_ST_CONNECTION_NOT_VALID, _GpuIdentityMismatch apply-time re-verification, and scan_uuid_index_map() returning a single {uuid: index} snapshot plus a conclusive flag (DCGM reports an empty discovered topology as INCONCLUSIVE so a dropped hostengine connection cannot masquerade as a GPU-less node and prune every persisted UUID).

   • power_agent.py: reconcile, SIGTERM, and orphan recovery dispatch through the active actuator; CLI exposes --actuator {nvml,dcgm}, --dcgm-host, --dcgm-port. Orphan recovery restores only persisted UUIDs that are currently visible and idle, and prunes state-only "absent" UUIDs only when the identity scan is conclusive. _read_managed_gpus_state() returns (uuids, conclusive) so a transient read error cannot rewrite the state file empty. Acquisition and retirement failures queue to _pending_acquisition / _pending_retirement and are retried (persist-only, never a repeated un-cap) at the top of each reconcile cycle. The SIGTERM path runs a UUID-complete sweep so a re-enumerated index cannot strand a cap.

   • managed_state.py: single source of truth for the mutable managed-GPU sets (managed_gpu_indices, previously_managed, pending_acquisition) so the __main__ and imported power_agent module copies do not split state.

   • Dockerfile: vendors pydcgm bindings and libdcgm.so from the DCGM image for the opt-in path; the default NVML build is unaffected. DCGM_IMAGE defaults to 4.5.3-1-ubuntu24.04; version guidance is otherwise neutral (pin the operator's nvidia-dcgm tag; client >= hostengine), with a load-smoke required for tag overrides.

   • tests/: power-agent unit suites ΓÇö actuator-protocol satisfaction, CLI/actuator selection, DCGM actuator behavior, orphan recovery, pending-persistence queues, reconcile wiring, reconcile fail-safe, SIGTERM dispatch, shared-state identity, managed-state parsing, multi-DGD topology, and GPU-less parity-harness logic. e2e_actuator_parity.py is real-GPU parity scaffolding (joins NVML vs DCGM by UUID, not by index), excluded from CI by default.

2. Helm chart (deploy/helm/charts/power-agent/):

   • Chart.yaml: chart/appVersion bump.

   • Makefile: lint (helm lint + render prod/dev paths) and test (helm-unittest) targets, invoked by PR and post-merge CI.

   • values.yaml: adds agent.actuator and agent.dcgm.{host,port}; the default keeps actuator=nvml. The DCGM-version note references the operator's nvidia-dcgm tag.

   • templates/daemonset.yaml, templates/dev-pod.yaml, templates/role.yaml, templates/NOTES.txt: render actuator/DCGM flags and dev-mode mounts; NOTES lists the six exposed metrics and the effective RBAC scope.

   • templates/_helpers.tpl: validates actuator and image pinning (:tag vs @sha256: digest) at template time.

   • tests/validate_{actuator,image_tag,dev_image_tag}_test.yaml: helm-unittest coverage.

3. CI (.github/workflows/pr.yaml, .github/workflows/post-merge-ci.yml): the Power Agent job builds the runtime image, runs the in-image test stage, and runs make -C deploy/helm/charts/power-agent lint test.

Exposed Prometheus metrics (six): applied_limit_watts, multi_pod_gpu_total, safe_default_applied_total, apply_failures_total, cap_clamped_total, k8s_list_failures_total.

Reviewer start points:

• deploy/power-agent/actuator.py ΓÇö actuator protocol, DCGM UUID-identity handling, scan_uuid_index_map().

• deploy/power-agent/power_agent.py ΓÇö reconcile dispatch, orphan recovery, metric semantics, SIGTERM routing (incl. the UUID sweep), and pending-persistence retries.

• deploy/helm/charts/power-agent/ ΓÇö values, helpers, templates, Makefile, and tests.

Validation

Validated at bd3da174a25f71b4ea0721cff4100825f9d05782:

• python3.10 -m pytest deploy/power-agent/tests/ -q: 319 passed on the host.
• python3.10 -m pytest deploy/power-agent/tests/test_reconcile_failsafe.py -q: 16 passed (focused coverage for LIST timeouts, transport retries, and shutdown fast-path).
• helm lint deploy/helm/charts/power-agent plus prod/dev helm template render checks: passed.
• helm unittest deploy/helm/charts/power-agent: 48 passed across 4 suites (actuator, image_tag, dev_image_tag, terminationGracePeriodSeconds).
• git diff --check on the intended patch files: clean.
• Pre-commit on the touched files: isort, black, flake8, codespell, ruff, check-yaml, and the rest pass. pytest-marker-report fails only on the Windows workstation due to the known POSIX-only fcntl import in the repo's global test harness (unrelated to this diff); confirmed the focused suite and fcntl import pass on Linux (WSL).
• Inside the shipped image at the previous head 2567d2c66c5c7d17425b09ccc838874690c2d25a (docker build --target test, python:3.12-slim base + baked deps): 314 passed. The latest bd3da174a2 commit only adds bounded pod LIST/shutdown handling and Helm grace-period validation; full PR CI is being refreshed for this head.
• DCO: all seven commits are signed off.
• Rebase: replayed the stack onto current main (was 120 commits behind at rebase time). The only conflict was in .lycheeignore (main added a bitnami ignore entry; this branch adds then removes three deployment-guide entries across two commits) — resolved so the net tree keeps main's bitnami entry and drops the three deployment-guide entries.

The heavy GitLab-mirror CI (Power Agent image build, tests-in-image, Helm, deploy tests) needs to run on bd3da174a2; the local artifact/source checks above were reproduced locally.

Merge Strategy

Seven commits rebased onto current main; conflict-free. Squash- or rebase-merge are both fine.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

Walkthrough

This pull request introduces a complete Power Agent DaemonSet for enforcing per-GPU power caps on Dynamo worker nodes, featuring a pluggable dual-actuator design (NVML default, DCGM opt-in), comprehensive test coverage, and production-ready Helm chart deployment with template-time validation.

Changes

Power Agent Dual-Actuator System

Layer / File(s)	Summary
Actuator Protocol and Implementations components/power_agent/actuator.py, components/power_agent/logger.py, components/power_agent/tests/test_actuator_protocol.py, components/power_agent/tests/test_actuator_selection.py, components/power_agent/tests/test_dcgm_actuator.py, components/power_agent/tests/e2e_actuator_parity.py	Defines Actuator Protocol with lifecycle and cap control APIs. Implements NvmlActuator with lazy NVML imports and DcgmActuator with TCP hostengine connection, stale-handle recovery, and cross-library UUID mapping. Includes protocol conformance tests, factory selection tests, comprehensive DcgmActuator unit tests, and e2e parity harness comparing both actuators against nvidia-smi ground truth.
Power Agent Core and Reconciliation components/power_agent/power_agent.py, components/power_agent/tests/test_reconcile_wiring.py, components/power_agent/tests/test_shutdown.py, components/power_agent/tests/test_orphan_recovery.py, components/power_agent/tests/test_managed_state_parser.py, components/power_agent/tests/test_multi_pod_policy.py	Implements PowerAgent with periodic GPU reconciliation (pod UID mapping, multi-pod cap policy, actuator-driven application), optional Prometheus metrics, SIGTERM-driven graceful shutdown routing through active actuator, and cold-start orphan recovery using UUID-gated managed GPU state. Tests validate actuator routing contract, shutdown behavior with failure isolation, orphan recovery logic, managed state persistence robustness, and policy fallback metrics semantics.
Container Image and Component Documentation components/power_agent/Dockerfile, components/power_agent/README.md	Multi-stage Dockerfile supporting both NVML and DCGM with vendored DCGM Python bindings and libraries. Component README documents dual-actuator modes, Helm chart compatibility requirements, and troubleshooting including Prometheus metrics semantics.
Helm Chart Values and Configuration deploy/helm/charts/power-agent/Chart.yaml, deploy/helm/charts/power-agent/values.yaml, deploy/helm/charts/power-agent/README.md	Chart metadata (v1.2.0, appVersion v1.1.0) and comprehensive values.yaml with image pinning (tag/digest), actuator selection, DCGM connection options, and dev-mode ConfigMap requirements. Chart README provides production and dev install guidance, image pinning rules, troubleshooting, and cross-writer flapping warnings.
Helm Template Validation and Deployment deploy/helm/charts/power-agent/templates/_helpers.tpl, deploy/helm/charts/power-agent/templates/daemonset.yaml, deploy/helm/charts/power-agent/templates/dev-pod.yaml, deploy/helm/charts/power-agent/templates/NOTES.txt	Strict image validation helpers (tag/digest mutual exclusivity, :latest rejection, whitespace rejection, SHA-256 format enforcement) and actuator/enforce validators. imageRef helper renders canonical OCI references. DaemonSet and dev-Pod templates conditionally inject actuator and DCGM flags. Post-install notes guide mode-specific verification and iteration.
Helm Unittest Test Suites deploy/helm/charts/power-agent/tests/validate_image_tag_test.yaml, deploy/helm/charts/power-agent/tests/validate_actuator_test.yaml, deploy/helm/charts/power-agent/tests/validate_enforce_test.yaml	46 unit test cases validating template-time behavior: image tag/digest mutual exclusivity, :latest rejection, whitespace/format validation; actuator selection (nvml default, dcgm opt-in, invalid rejection); dcgm-enforce boolean allowlist (yes/no/on/off/true/false with case normalization, rejection of invalid values, cross-validator behavior for unused modes).
Design Documentation docs/design-docs/power-agent-helm-chart-plan.md	Comprehensive plan document updated from draft to implementation record, including v1.1.0 dual-actuator expansion and v1.2.0 image-pinning hardening. Detailed sections on chart schema, validation strategy, template-time validators, installation flows, commit gates, risk analysis, and pre-flight/post-push checklists.

---

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 49.79% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The description is detailed and aligned with the PR, but it omits the template's required Related Issues section.	Add the Related Issues section and either link the issue(s) with Closes/Relates to #XXXX or check "Confirmed — no related issue".

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately summarizes the main change: adding an opt-in DCGM dual actuator while keeping NVML as the default.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kaim-eng]

Rebased onto the refreshed pr1a/power-agent (tip 29f0831) and main (~316 commits forward from the prior base). Force-pushed pr1b/power-agent-dcgm-actuator to abcc922.

What moved:

• The four PR feat(power-agent): per-node power-cap enforcement DaemonSet #9682 runtime-hardening fixes (state-file parser, NVML shutdown logging, pod-UID dedup, argparse import scope) were backported to pr1a/power-agent so each fix lives with the code it changes. Replies posted on every PR feat(power-agent): per-node power-cap enforcement DaemonSet #9682 review thread.
• The advisory-mode gate on _apply_power_annotations landed on pr1b/planner-infra (feat(planner): power infrastructure — pod annotation, RBAC, config, Prometheus #9683, commit 8f47413) — addresses dynamo-ops's core/base.py:976 finding.
• On this PR, commit 36b9350 keeps the actuator-aware refinements: the per-entry isinstance(str) filter on the state parser, the actuator-aware shutdown logging that names DCGM vs NVML in the error message, and the +29 new test cases under components/power_agent/tests/test_managed_state_parser.py / test_reconcile_wiring.py / test_shutdown.py / test_dcgm_actuator.py.

CI rerunning now.

@coderabbitai review

[coderabbitai]

@kaim-eng Thanks for the detailed rebase summary! The backport strategy—keeping each hardening fix with the code it touches on pr1a while retaining the actuator-aware refinements and the 29 new test cases here on pr1b—looks like a clean separation. I'll kick off a fresh review of the force-pushed tip now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

components/power_agent/tests/test_managed_state_parser.py (1)

85-275: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add required pytest markers (scheduling + GPU + type) to this test module.

The newly added tests are unmarked and don’t satisfy the required test marker policy.

As per coding guidelines: “Every test must have at least one scheduling marker… at least one GPU marker… and at least one type marker…”.

🤖 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 `@components/power_agent/tests/test_managed_state_parser.py` around lines 85 -
275, Import pytest at the top of the test module and add a module-level
pytestmark list that applies the required markers (scheduling + GPU + type) to
all tests, e.g. place `import pytest` and `pytestmark = [pytest.mark.scheduling,
pytest.mark.gpu, pytest.mark.unit]` (or whichever type marker is appropriate)
near the top so TestHappyPath, TestMissingFile, TestOsErrorSiblings,
TestMalformedJson, TestNonObjectRoot, TestManagedUuidsNotAList, and
TestEntryTypeValidation all inherit the markers.

components/power_agent/tests/test_orphan_recovery.py (1)

85-286: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add required pytest markers (scheduling + GPU + type) to this test module.

This suite needs the mandatory test markers before merge.

As per coding guidelines: “Every test must have at least one scheduling marker… at least one GPU marker… and at least one type marker…”.

🤖 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 `@components/power_agent/tests/test_orphan_recovery.py` around lines 85 - 286,
The module lacks required pytest markers (scheduling + GPU + type); add a
module-level pytestmark so every test in this file gets those markers: import
pytest at top of the file and define pytestmark =
[pytest.mark.<scheduling_marker>, pytest.mark.<gpu_marker>,
pytest.mark.<type_marker>] (replace placeholders with the project's chosen
markers), which will apply to the test classes such as TestUuidGating,
TestWorkloadBusySkip, TestCurrentVsDefaultGuard, TestPerGpuExceptionIsolation,
and TestManagedSetPruning.

components/power_agent/tests/test_shutdown.py (1)

46-229: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add required pytest markers (scheduling + GPU + type) to this test module.

This file currently lacks the mandatory test classification markers.

As per coding guidelines: “Every test must have at least one scheduling marker… at least one GPU marker… and at least one type marker…”.

🤖 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 `@components/power_agent/tests/test_shutdown.py` around lines 46 - 229, Add the
required pytest classification markers by defining a module-level pytestmark
list (e.g. pytestmark = [pytest.mark.scheduling, pytest.mark.gpu,
pytest.mark.type]) so every test in this module (including
TestSigtermViaActuator and TestSigtermFallback and their methods like
_handle_sigterm interactions) carries scheduling, GPU and type markers; place
the pytestmark definition near the top of the file (above the test classes) and
import pytest if not already imported.

components/power_agent/tests/test_reconcile_wiring.py (1)

46-326: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add required pytest markers (scheduling + GPU + type) for this module.

These tests currently have none of the required markers.

Suggested patch

+import pytest
+
+pytestmark = [
+    pytest.mark.post_merge,  # or pre_merge/nightly/etc. per intended cadence
+    pytest.mark.gpu_0,
+    pytest.mark.unit,
+]

As per coding guidelines: “Every test must have at least one scheduling marker… at least one GPU marker… and at least one type marker…”.

🤖 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 `@components/power_agent/tests/test_reconcile_wiring.py` around lines 46 - 326,
Add the required pytest markers by importing pytest and applying scheduling,
GPU, and type markers to this test module; either set a module-level pytestmark
list or add decorators to each test class (e.g.
TestReconcileGpuRoutesViaActuator, TestReconcileGpuDedupesByPodUid,
TestReconcileGpuPolicyResolution) to include at least one scheduling marker, one
GPU marker, and one type marker per the guidelines so all tests in this file are
properly marked.

🧹 Nitpick comments (4)

deploy/helm/charts/power-agent/tests/validate_enforce_test.yaml (1)

54-95: ⚡ Quick win

Strengthen enforce-flag assertion coverage for accepted inputs and nvml dead-config checks.

Line 54 onward accepts multiple string forms with only notFailedTemplate, and Line 136 claims “ANY allowlisted value” absence while checking only a subset. Add exact contains/notContains assertions for the remaining allowlisted renderable forms to catch silent drift.

Proposed test hardening pattern

  - it: should accept enforce=1 (string)
    set:
      agent.dcgm.enforce: "1"
    asserts:
      - notFailedTemplate: {}
+      - contains:
+          path: spec.template.spec.containers[0].command
+          content: --dcgm-enforce=1

  - it: should accept enforce=0 (string)
    set:
      agent.dcgm.enforce: "0"
    asserts:
      - notFailedTemplate: {}
+      - contains:
+          path: spec.template.spec.containers[0].command
+          content: --dcgm-enforce=0

  - it: should accept enforce=yes
    set:
      agent.dcgm.enforce: "yes"
    asserts:
      - notFailedTemplate: {}
+      - contains:
+          path: spec.template.spec.containers[0].command
+          content: --dcgm-enforce=yes

      - notContains:
          path: spec.template.spec.containers[0].command
          content: --dcgm-enforce=false
+      - notContains:
+          path: spec.template.spec.containers[0].command
+          content: --dcgm-enforce=1
+      - notContains:
+          path: spec.template.spec.containers[0].command
+          content: --dcgm-enforce=0
+      - notContains:
+          path: spec.template.spec.containers[0].command
+          content: --dcgm-enforce=yes
+      - notContains:
+          path: spec.template.spec.containers[0].command
+          content: --dcgm-enforce=no
+      - notContains:
+          path: spec.template.spec.containers[0].command
+          content: --dcgm-enforce=on
+      - notContains:
+          path: spec.template.spec.containers[0].command
+          content: --dcgm-enforce=off

Also applies to: 136-147

🤖 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/charts/power-agent/tests/validate_enforce_test.yaml` around lines
54 - 95, The tests under validate_enforce_test.yaml currently only use
notFailedTemplate for many agent.dcgm.enforce string variants; update each case
(e.g., the tests setting agent.dcgm.enforce to
"1","0","yes","no","on","off","TRUE") to also assert the rendered templates
explicitly include or exclude the expected configuration snippets by adding
contains/notContains assertions for the corresponding normalized values (e.g.,
numeric 1/0, true/false, on/off forms) so silent drift is caught, and apply the
same hardening to the other block referenced (the ANY allowlisted values section
around lines 136-147) to cover all allowed renderable forms and the nvml
dead-config checks rather than relying solely on notFailedTemplate.

components/power_agent/tests/test_reconcile_wiring.py (1)

218-219: ⚡ Quick win

Move function-scope imports to module scope.

from tests.test_multi_pod_policy import _FakeMetrics is imported inside test methods; this should be hoisted to top-level imports.

Suggested patch

 import unittest
 from unittest.mock import MagicMock, patch
 
 import power_agent
 from power_agent import PowerAgent
+from tests.test_multi_pod_policy import _FakeMetrics
@@
-        from tests.test_multi_pod_policy import _FakeMetrics
-
         agent.metrics = _FakeMetrics()
@@
-        from tests.test_multi_pod_policy import _FakeMetrics
-
         agent.metrics = _FakeMetrics()
@@
-        from tests.test_multi_pod_policy import _FakeMetrics
-
         agent.metrics = _FakeMetrics()

As per coding guidelines: “ensure imports are only at module scope (no imports inside functions/classes)”.

Also applies to: 243-244, 269-270

🤖 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 `@components/power_agent/tests/test_reconcile_wiring.py` around lines 218 -
219, The test module imports `_FakeMetrics` inside test functions; hoist `from
tests.test_multi_pod_policy import _FakeMetrics` to the top-level imports in the
module (module scope) and remove the function-scope imports in the test methods
(including the other occurrences where `_FakeMetrics` is imported inside tests).
Ensure the module-level import is added near the other test imports so all tests
reference the same top-level `_FakeMetrics`.

components/power_agent/tests/test_managed_state_parser.py (1)

60-67: ⚡ Quick win

Hoist logging imports to module scope.

import logging is currently inside methods; move it to top-level imports for consistency with Python guidelines.

As per coding guidelines: “ensure imports are only at module scope (no imports inside functions/classes)”.

Also applies to: 114-121

🤖 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 `@components/power_agent/tests/test_managed_state_parser.py` around lines 60 -
67, Move the "import logging" statements out of the test functions and into
module-level imports in test_managed_state_parser.py; remove the in-function
imports at the locations that set up captured, handler, and logger (the blocks
that create captured: list[logging.LogRecord], handler = logging.Handler(),
handler.emit = captured.append, handler.setLevel(logging.WARNING), and logger =
logging.getLogger("power_agent")), so the tests use the top-level logging import
consistently (also apply the same change to the other block referenced in the
file).

components/power_agent/tests/e2e_actuator_parity.py (1)

497-500: ⚡ Quick win

Avoid silently swallowing NVML shutdown errors.

This except Exception: pass hides teardown faults and makes parity-run failures hard to diagnose.

Suggested patch

     finally:
         try:
             pynvml.nvmlShutdown()
-        except Exception:
-            pass
+        except Exception as e:
+            print(f"WARNING: pynvml.nvmlShutdown() failed: {e}", file=sys.stderr)

As per coding guidelines: “fail fast (don’t swallow exceptions, catch specific exceptions only, and if catching Exception then log and re-raise)”.

🤖 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 `@components/power_agent/tests/e2e_actuator_parity.py` around lines 497 - 500,
The try/except that currently calls pynvml.nvmlShutdown() and swallows all
exceptions should be changed to catch the specific NVML exception type
(pynvml.NVMLError) instead of Exception, log the error with traceback, and
re-raise so teardown failures are visible; update the block around
pynvml.nvmlShutdown() (replace the bare except Exception: pass) to except
pynvml.NVMLError as e: logger.exception("pynvml.nvmlShutdown failed: %s", e) and
raise, and if a module-level logger isn’t present add logger =
logging.getLogger(__name__) and import logging.

🤖 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 `@components/power_agent/actuator.py`:
- Around line 775-785: The except-block around the call to self._apply_cap_inner
should only catch DCGM write-related exceptions instead of all Exception;
replace "except Exception as e:" with a narrow catch such as "except DcgmError
as e" (or the specific DCGM client exception types your project uses, e.g.,
DcgmApiError / DcgmWriteError) and ensure those exception classes are imported,
keep the existing logger.error and metrics.inc for that case, and re-raise any
other unexpected exceptions so programming/runtime bugs are not swallowed by
apply_cap/_apply_cap_inner.
- Around line 539-540: The except block that swallows all exceptions after
calling _handle.Shutdown() must be changed to avoid hiding failures: catch only
expected exceptions or catch Exception but log the exception context before
continuing. Locate the try/except surrounding _handle.Shutdown() in actuator.py
(the block that currently reads "except Exception: pass") and replace the silent
swallow with a logged error using the module logger (include the exception
instance and stacktrace) or re-raise critical exceptions after logging; ensure
the log message mentions "_handle.Shutdown" so it’s easy to find in logs.
- Around line 618-622: The RuntimeError raised in DcgmActuator.list_running_pids
when a GPU UUID is no longer visible should preserve the original KeyError as
the root cause; change the raise so it uses exception chaining (raise
RuntimeError(...) from err) where err is the caught KeyError from the UUID-map
lookup, keeping the message unchanged but appending "from err" to preserve the
original traceback and context for uuid and gpu_idx.

In `@components/power_agent/power_agent.py`:
- Around line 169-177: The current check uses valid = {u for u in uuids if
isinstance(u, str)} which turns uuids into a set and conflates duplicates with
non-string entries; replace this by collecting only string entries without
deduping (e.g., valid_list = [u for u in uuids if isinstance(u, str)]) and
compute non_string_count = sum(1 for u in uuids if not isinstance(u, str)); then
change the logger.warning invocation (the one referencing _MANAGED_STATE_PATH)
to report non_string_count and the number of kept UUIDs (use len(valid_list) or
len(set(valid_list)) depending on whether you intend to report raw entries or
unique UUIDs) instead of comparing len(valid) to len(uuids).

In `@deploy/helm/charts/power-agent/README.md`:
- Around line 49-53: The README currently references a specific chart version
("chart v1.1.0 requires the v1.1.0 image"); update the wording to tie the
requirement to the chart's appVersion instead of a hard-coded chart version:
state that the image tag MUST match the chart's `appVersion`, and explain that
this is necessary because the chart renders the `--actuator` / `--dcgm-*` CLI
flags and `power_agent.py` imports the appVersion-specific `actuator` module;
replace the version-specific sentence with this appVersion-based guidance so
future chart bumps don't require text edits.

In `@deploy/helm/charts/power-agent/templates/_helpers.tpl`:
- Around line 117-119: The helper currently only checks for leading/trailing
whitespace on $tag but allows internal whitespace (e.g., "v1 .1.0") which
produces invalid OCI image references; update the validation in the template
that handles image.tag (the block using $tag and the fail message) to reject any
whitespace characters anywhere in $tag (not just trim differences) by testing
for matches against a whitespace pattern and calling fail with the existing
informative message when any whitespace is found; keep the same fail text and
hint but trigger it when $tag contains internal spaces or other whitespace.

In `@docs/design-docs/power-agent-helm-chart-plan.md`:
- Line 828: The Markdown has unlabeled fenced code blocks that trigger MD040;
update each triple-backtick block in power-agent-helm-chart-plan.md to include a
language identifier (e.g., change ``` to ```text or to a specific language such
as ```yaml, ```bash, or ```json as appropriate) so the linter recognizes the
block language; apply the same change to the other unlabeled fences mentioned in
the comment to resolve MD040.
- Line 29: The heading "### Revision history" is skipping a level; change that
heading to "## Revision history" (or add a parent "##" section before it) so the
document preserves correct heading hierarchy and satisfies MD001; update the
line containing "### Revision history" in
docs/design-docs/power-agent-helm-chart-plan.md accordingly.
- Around line 826-846: Update the stale v1.1.0 summary counts in sections §5.2
and §8 to match the v1.2.0 source-of-truth numbers used elsewhere in this
document: replace "13 files / 2 helm-unittests / 24 passed" with "14 files / 3
helm-unittests / 46 passed" (and any adjacent totals like "Chart-only total" or
LOC if they differ), and make the identical corrections where the same summary
appears around lines 1151-1160 so all references are consistent.

---

Outside diff comments:
In `@components/power_agent/tests/test_managed_state_parser.py`:
- Around line 85-275: Import pytest at the top of the test module and add a
module-level pytestmark list that applies the required markers (scheduling + GPU
+ type) to all tests, e.g. place `import pytest` and `pytestmark =
[pytest.mark.scheduling, pytest.mark.gpu, pytest.mark.unit]` (or whichever type
marker is appropriate) near the top so TestHappyPath, TestMissingFile,
TestOsErrorSiblings, TestMalformedJson, TestNonObjectRoot,
TestManagedUuidsNotAList, and TestEntryTypeValidation all inherit the markers.

In `@components/power_agent/tests/test_orphan_recovery.py`:
- Around line 85-286: The module lacks required pytest markers (scheduling + GPU
+ type); add a module-level pytestmark so every test in this file gets those
markers: import pytest at top of the file and define pytestmark =
[pytest.mark.<scheduling_marker>, pytest.mark.<gpu_marker>,
pytest.mark.<type_marker>] (replace placeholders with the project's chosen
markers), which will apply to the test classes such as TestUuidGating,
TestWorkloadBusySkip, TestCurrentVsDefaultGuard, TestPerGpuExceptionIsolation,
and TestManagedSetPruning.

In `@components/power_agent/tests/test_reconcile_wiring.py`:
- Around line 46-326: Add the required pytest markers by importing pytest and
applying scheduling, GPU, and type markers to this test module; either set a
module-level pytestmark list or add decorators to each test class (e.g.
TestReconcileGpuRoutesViaActuator, TestReconcileGpuDedupesByPodUid,
TestReconcileGpuPolicyResolution) to include at least one scheduling marker, one
GPU marker, and one type marker per the guidelines so all tests in this file are
properly marked.

In `@components/power_agent/tests/test_shutdown.py`:
- Around line 46-229: Add the required pytest classification markers by defining
a module-level pytestmark list (e.g. pytestmark = [pytest.mark.scheduling,
pytest.mark.gpu, pytest.mark.type]) so every test in this module (including
TestSigtermViaActuator and TestSigtermFallback and their methods like
_handle_sigterm interactions) carries scheduling, GPU and type markers; place
the pytestmark definition near the top of the file (above the test classes) and
import pytest if not already imported.

---

Nitpick comments:
In `@components/power_agent/tests/e2e_actuator_parity.py`:
- Around line 497-500: The try/except that currently calls pynvml.nvmlShutdown()
and swallows all exceptions should be changed to catch the specific NVML
exception type (pynvml.NVMLError) instead of Exception, log the error with
traceback, and re-raise so teardown failures are visible; update the block
around pynvml.nvmlShutdown() (replace the bare except Exception: pass) to except
pynvml.NVMLError as e: logger.exception("pynvml.nvmlShutdown failed: %s", e) and
raise, and if a module-level logger isn’t present add logger =
logging.getLogger(__name__) and import logging.

In `@components/power_agent/tests/test_managed_state_parser.py`:
- Around line 60-67: Move the "import logging" statements out of the test
functions and into module-level imports in test_managed_state_parser.py; remove
the in-function imports at the locations that set up captured, handler, and
logger (the blocks that create captured: list[logging.LogRecord], handler =
logging.Handler(), handler.emit = captured.append,
handler.setLevel(logging.WARNING), and logger =
logging.getLogger("power_agent")), so the tests use the top-level logging import
consistently (also apply the same change to the other block referenced in the
file).

In `@components/power_agent/tests/test_reconcile_wiring.py`:
- Around line 218-219: The test module imports `_FakeMetrics` inside test
functions; hoist `from tests.test_multi_pod_policy import _FakeMetrics` to the
top-level imports in the module (module scope) and remove the function-scope
imports in the test methods (including the other occurrences where
`_FakeMetrics` is imported inside tests). Ensure the module-level import is
added near the other test imports so all tests reference the same top-level
`_FakeMetrics`.

In `@deploy/helm/charts/power-agent/tests/validate_enforce_test.yaml`:
- Around line 54-95: The tests under validate_enforce_test.yaml currently only
use notFailedTemplate for many agent.dcgm.enforce string variants; update each
case (e.g., the tests setting agent.dcgm.enforce to
"1","0","yes","no","on","off","TRUE") to also assert the rendered templates
explicitly include or exclude the expected configuration snippets by adding
contains/notContains assertions for the corresponding normalized values (e.g.,
numeric 1/0, true/false, on/off forms) so silent drift is caught, and apply the
same hardening to the other block referenced (the ANY allowlisted values section
around lines 136-147) to cover all allowed renderable forms and the nvml
dead-config checks rather than relying solely on notFailedTemplate.

🪄 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: Pro

Run ID: 4b586c90-dca2-4d5f-aaae-4f35d04c75ef

📥 Commits

Reviewing files that changed from the base of the PR and between 29f0831 and abcc922.

📒 Files selected for processing (26)

• components/power_agent/Dockerfile
• components/power_agent/README.md
• components/power_agent/actuator.py
• components/power_agent/logger.py
• components/power_agent/power_agent.py
• components/power_agent/tests/e2e_actuator_parity.py
• components/power_agent/tests/test_actuator_protocol.py
• components/power_agent/tests/test_actuator_selection.py
• components/power_agent/tests/test_dcgm_actuator.py
• components/power_agent/tests/test_managed_state_parser.py
• components/power_agent/tests/test_multi_pod_policy.py
• components/power_agent/tests/test_orphan_recovery.py
• components/power_agent/tests/test_reconcile_wiring.py
• components/power_agent/tests/test_shutdown.py
• deploy/helm/charts/power-agent/Chart.yaml
• deploy/helm/charts/power-agent/README.md
• deploy/helm/charts/power-agent/templates/NOTES.txt
• deploy/helm/charts/power-agent/templates/_helpers.tpl
• deploy/helm/charts/power-agent/templates/daemonset.yaml
• deploy/helm/charts/power-agent/templates/dev-pod.yaml
• deploy/helm/charts/power-agent/tests/validate_actuator_test.yaml
• deploy/helm/charts/power-agent/tests/validate_enforce_test.yaml
• deploy/helm/charts/power-agent/tests/validate_image_tag_test.yaml
• deploy/helm/charts/power-agent/values.yaml
• docs/design-docs/power-agent-dual-actuator.md
• docs/design-docs/power-agent-helm-chart-plan.md

[kaim-eng]

/ok to test 7226298

[kaim-eng]

/ok to test 3b84f22

[kaim-eng]

/ok to test 85df904

[kaim-eng]

@nv-anants @sttts (cc @dynamo-docs) — re-review request.

Since the last review pass I pushed one commit, 85df9043. Unlike the earlier refreshes this is not a rebase — it adds small, focused changes from a final best-effort review:

• fix (correctness): DcgmActuator.device_count() now re-probes hostengine discovery when the cached GPU topology is empty, so a transiently-empty startup (agent connects before nvidia-dcgm finishes enumerating) self-heals on the next reconcile instead of silently enforcing no caps until a restart. Regression tests added.
• fix (observability): a DCGMError from the pre-write constraints read in apply_cap() is now absorbed into apply_failures_total (it previously escaped the metric); a malformed power-limit value is deliberately left to surface.
• ci: added a helm unittest step to the power-agent job — the 42 chart tests existed but were never executed. Pinned to plugin v1.0.3 because v1.1.0's platformHooks manifest breaks the pinned Helm 3.17.3 (helm-unittest#856); verified locally against that exact Helm version.
• docs/logs: corrected the values.yaml DCGM version (4.5.3), the actuator lazy-import list, the previously_managed comment, and the restore-outcome docstring/log wording.

A real-DCGM ABI/load smoke test is tracked as a follow-up in #11213.

All 20 review threads are resolved, and full CI is re-running on 85df9043 (/ok to test posted). Local gates: power-agent suite 311 passed, helm unittest 42 passed. Could you take another look when you have a moment? Thanks!

[kaim-eng]

@nv-anants @sttts - one small clarification to keep the review record precise, then the merge ask. Two older resolved-thread replies were a little too broad compared with the current code. Device-count refresh: the frozen startup count bug is fixed because reconcile_once refreshes self._actuator.device_count() each cycle, so topology growth is picked up and shrink is not carried forward from the stale startup count. My earlier 'a shrink no longer raises per-index errors' sentence was too strong: if DCGM reconnects/re-enumerates during a GPU loop after range(self.device_count) has already been captured, that cycle can still see transient per-index errors; the next cycle refreshes and recovers. Restore outcome / ownership retirement: current contract is True = restored a live below-default cap, None = no write needed / cleanly absent / already at default, False = inconclusive/retain ownership. Current callers retire/prune on True or None, and retain only on False. Some older wording said only True prunes and already-at-default remains tracked; that was stale. The enum cleanup remains tracked separately in #11193 as agreed. I did another pass over the code/comments after the final review round. The code changes look consistent with the replies, CI is green on head 85df904, and the PR is only blocked on review approval. Could you approve when you get a chance if this looks good now? Thanks.

[sttts]

Codex xhigh review:

Findings

[P1] _pending_acquisition still has the same __main__ / import power_agent split bug

The PR correctly moves the main ownership sets into managed_state.py, but the acquisition retry queue remains module-local in power_agent.py.

Refs:

• _pending_acquisition is local: deploy/power-agent/power_agent.py:267
• failed durable ADD queues there: deploy/power-agent/power_agent.py:294
• reconcile flushes its own module copy: deploy/power-agent/power_agent.py:1371
• DCGM records via import power_agent: deploy/power-agent/actuator.py:1568, deploy/power-agent/actuator.py:1576
• entrypoint comment says all shared mutable state lives in managed_state, but this queue does not: deploy/power-agent/power_agent.py:1703

Production launches power_agent.py as __main__, while the actuators import canonical power_agent. If a cap write succeeds but persisting managed_gpus.json fails, the UUID can be queued in the canonical module’s _pending_acquisition, while the running daemon flushes the __main__ queue. That retry never happens.

This affects DCGM directly and NVML too, because NvmlActuator.apply_cap also imports canonical power_agent before delegating: deploy/power-agent/actuator.py:425, deploy/power-agent/actuator.py:431.

Fix: move _pending_acquisition into managed_state.py, or make the entrypoint/import module canonical.

[P2] DCGM blank-sentinel fallback can silently accept blank power values

_coerce_power_limit_watts builds the blank sentinel set from dcgmvalue, but if dcgmvalue is missing or one constant is renamed, it catches ImportError / AttributeError and continues with an empty blank_values set: deploy/power-agent/actuator.py:1181, deploy/power-agent/actuator.py:1192.

The comment says finite/positive checks still reject dangerous values, but DCGM blank values are finite positive numbers. The file itself calls out DCGM_FP64_BLANK = 140737488355328.0: deploy/power-agent/actuator.py:1062. So in that fallback path, a blank current/default/min/max value can be treated as a real watt value.

Fix: include known numeric blank constants as fallback values, or fail closed if the blank constants cannot be loaded.

[P2] Helm unit tests run on PR but not post-merge

The PR adds helm unittest to .github/workflows/pr.yaml: .github/workflows/pr.yaml:274.

The post-merge Power Agent job still only does Helm lint/template: .github/workflows/post-merge-ci.yml:166.

That leaves the new chart test suite as PR-only coverage. I would mirror the same helm plugin install + helm unittest deploy/helm/charts/power-agent step in post-merge CI.

Old threads

The old SIGTERM / re-enumeration GPU-A leak looks fixed now:

• append-only _capped_uuids: deploy/power-agent/actuator.py:622
• UUID sweep after index restore: deploy/power-agent/power_agent.py:895
• UUID-addressed restore: deploy/power-agent/actuator.py:1692

The other old findings I rechecked also look addressed:

• device count refresh
• no post-Set UUID reread
• narrowed DCGM write exception handling
• reconnect shutdown logging
• logger shim removal

Architecture concern

Separate from code correctness: the repo-boundary concern still stands.

The hard Dynamo interface appears to be only dynamo.nvidia.com/gpu-power-limit: deploy/power-agent/power_agent.py:78.

The agent is already a standalone DaemonSet/Helm chart, and repo-wide search did not show Dynamo code outside the agent/chart producing or consuming that annotation. So the “why is this in Dynamo instead of a standalone GPU power-agent consumed as a subchart/dependency?” question remains valid.

[sttts]

And a Codex smell check:

Code smell ranking

• **** Split module state: __main__ vs import power_agent

  • Where: managed_state.py:4, power_agent.py:267, power_agent.py:1703, actuator.py:1568
  • Why it smells: the code already had to create managed_state.py because Python creates two module objects. But not all shared mutable state was moved there.
  • Fix difficulty: medium
  • Fix: move _pending_acquisition into managed_state.py, or better, stop importing power_agent from actuator.py and pass an explicit state object into the actuator.

• **** Global mutable state + persistence queues

  • Where: power_agent.py:140, power_agent.py:267, power_agent.py:459, power_agent.py:469
  • Why it smells: ownership state, durable state, and retry queues are spread across module globals. Correctness depends on mutating the right global in the right module copy.
  • Fix difficulty: medium/hard
  • Fix: introduce an AgentState object that owns managed_gpu_indices, previously_managed, pending acquisitions, pending retirements, and persistence.

• *** Tri-state Optional[bool] restore contract

  • Where: actuator.py:177, actuator.py:192, power_agent.py:840, power_agent.py:910, power_agent.py:1041
  • Why it smells: True, None, and False encode three semantic outcomes, and call sites must remember which values mean prune vs retain.
  • Fix difficulty: medium
  • Fix: replace with an enum like RESTORED, NOT_NEEDED, INCONCLUSIVE.

• *** DCGM identity/re-enumeration state machine is spread across too many paths

  • Where: power_agent.py:581, power_agent.py:953, actuator.py:1221, actuator.py:1692, actuator.py:1927
  • Why it smells: the same invariant appears in reconcile, release, shutdown, orphan recovery, and DCGM internals: resolve UUID, verify identity, write or skip, retain or prune.
  • Fix difficulty: hard
  • Fix: centralize the UUID-bound restore/write logic into named helpers or a transaction-style object.

• *** Huge comments are carrying correctness

  • Where: managed_state.py:4, power_agent.py:581, power_agent.py:953, actuator.py:1221, actuator.py:1692
  • Why it smells: many comments are explaining how not to break subtle invariants. That usually means the code shape does not encode the invariant strongly enough.
  • Fix difficulty: medium/hard
  • Fix: move invariants into types, enums, smaller state-transition functions, and explicit state objects.

• *** PR mixes refactor, feature, and hardening

  • Where: whole PR
  • Why it smells: reviewers must validate the Actuator refactor, the new DCGM backend, and generic ownership/recovery bugfixes at the same time.
  • Fix difficulty: hard now, easy if split earlier
  • Fix: ideal split would be: Actuator refactor first, NVML-only ownership/recovery hardening second, DCGM backend third.

• *** Agent/repo boundary is unclear

  • Where: power_agent.py:78, deploy/helm/charts/power-agent
  • Why it smells: the hard Dynamo interface appears to be only dynamo.nvidia.com/gpu-power-limit, while the agent is otherwise standalone.
  • Fix difficulty: hard/org-level
  • Fix: decide whether this is Dynamo-owned or a general NVIDIA GPU power agent. If general, move it to its own repo with a neutral annotation/API and consume it from Dynamo as a Helm subchart or external dependency.

• ** Chart test coverage not mirrored post-merge

  • Where: .github/workflows/pr.yaml:274, .github/workflows/post-merge-ci.yml:166
  • Why it smells: new chart tests run on PR but not after merge.
  • Fix difficulty: easy
  • Fix: copy the helm unittest step into post-merge CI.

• ** DCGM blank sentinel handling has fragile fallback

  • Where: actuator.py:1181, actuator.py:1192
  • Why it smells: if dcgmvalue constants are unavailable, finite positive DCGM blank values may pass as real watt values.
  • Fix difficulty: easy/medium
  • Fix: add known fallback blank numeric constants or fail closed when blank constants cannot be loaded.

[kaim-eng]

@sttts — thanks for the Codex xhigh pass and the two inline threads. Fixes are in amended commit 551d4f7789 (local; force-push pending).

P1 — _pending_acquisition __main__ / import power_agent split

Agreed. _managed_gpu_indices and _previously_managed were already on managed_state, but _pending_acquisition was still module-local, so DcgmActuator writes via import power_agent queued on the canonical module while reconcile_once in __main__ flushed an empty copy.

Fix: managed_state.pending_acquisition is now the single queue; power_agent.py aliases _pending_acquisition to it. Regression tests in test_managed_state_sharing.py assert shared-object identity across both import paths and that a flush in __main__ drains writes from the canonical module. (_pending_retirement was already safe — writers and flusher both live in __main__.)

P2 — DCGM blank-sentinel fallback

Agreed. When dcgmvalue import fails we used to leave blank_values empty, and DCGM_FP64_BLANK is finite/positive so it could slip through.

Fix: reject any numeric power limit >= 0x7FFFFFF0 (_DCGM_NUMERIC_BLANK_MIN) without importing dcgmvalue. Regression test explicitly sets dcgmvalue=None under patch.dict(..., clear=False) so the already-loaded module path is exercised.

P2 — Helm unittest only in PR CI

Fix: mirrored into post-merge-ci.yml, and both workflows now run make -C deploy/helm/charts/power-agent lint test (see inline reply on pr.yaml).

Inline — K8s list client timeout (power_agent.py)

Accepted. Removed K8S_LIST_TIMEOUT_S / _request_timeout on pod LIST calls. We still pass resource_version="0" for watch-cache reads; under apiserver pressure we now rely on server-side backpressure instead of a fixed client timeout that could amplify retry storms. Tests updated accordingly.

Inline — Helm Makefile pattern (pr.yaml)

Accepted. Added deploy/helm/charts/power-agent/Makefile with lint + test targets; PR and post-merge CI call make -C deploy/helm/charts/power-agent lint test, same shape as the platform chart. We pin HELM_UNITTEST_VERSION ?= v1.0.3 here (platform still uses 0.7.2) because this chart’s CI image is on Helm 3.17.3.

Architecture / smell notes (repo boundary, enum, AgentState, comment volume)

Fair points — I don’t think this PR is the right place to relocate power-agent or collapse the DCGM state machine. The ActuatorKind enum cleanup is tracked in #11193. Happy to discuss repo-boundary separately; for this stack I’m trying to keep the diff reviewable against #9682’s landing shape.

Validation

• python3.10 -m pytest deploy/power-agent/tests -q → 314 passed
• make -C deploy/helm/charts/power-agent lint test (WSL) → 42 helm unittests passed

Let me know if you want any of the smell/architecture items split into follow-up issues before merge.

[kaim-eng]

@nv-anants @sttts — re-review request.

Rebased the six commits onto current main (branch was 120 behind); new head 2567d2c6. The only conflict was .lycheeignore — resolved to keep main's new bitnami ignore entry and drop the three deployment-guide entries this branch already removes, so deploy/power-agent/** and deploy/helm/charts/power-agent/** are byte-identical to the pre-rebase tip.

Local re-validation at the new head:

• python3.10 -m pytest deploy/power-agent/tests: 314 passed
• in-image docker build --target test: 314 passed
• helm unittest deploy/helm/charts/power-agent: 42 passed (3 suites)
• pre-commit clean (the pytest-marker-report red is the known Windows-only fcntl artifact; exit 0 on Linux)
• DCO: 6/6 signed

@sttts — the two July 6 threads (pod-LIST client timeout, and the power-agent Helm Makefile/CI wiring) are addressed and verified live at the new head; inline replies point to the exact lines. Would appreciate a resolve/sign-off when you have a moment.

[kaim-eng]

/ok to test 2567d2c

[sttts]

Another pass with Codex xhigh:

[P1] SIGTERM cleanup can be missed while blocked in pod LIST

The PR moves restore work out of the signal handler into run()'s finally, but reconcile_once() can still block before it reaches any shutdown check.

Refs:

• deploy/power-agent/power_agent.py:799: _handle_sigterm() only sets _shutdown
• deploy/power-agent/power_agent.py:1262: _list_pods_on_node()
• deploy/power-agent/power_agent.py:1303: list_namespaced_pod(...) has no _request_timeout
• deploy/power-agent/power_agent.py:1309: list_pod_for_all_namespaces(...) has no _request_timeout
• deploy/power-agent/power_agent.py:1369: LIST happens before the GPU loop
• deploy/power-agent/power_agent.py:1405: shutdown is checked only after LIST returns
• deploy/power-agent/power_agent.py:1591: _shutdown_cleanup() only runs from finally
• deploy/helm/charts/power-agent/templates/daemonset.yaml:42: pod only has 30s grace

If SIGTERM lands while the Kubernetes client is stuck/throttled in the pod LIST, the handler only flips the event. Cleanup cannot run until the LIST returns. If kubelet reaches the 30s grace limit first, the process is SIGKILLed and managed caps are not restored.

Suggested fix: keep heavy restore work out of the handler, but make pre-cleanup blocking calls bounded/cancellable. At minimum, add a client-side LIST timeout shorter than terminationGracePeriodSeconds and treat timeout as the existing None failure path. Also add a shutdown guard before starting the LIST.

[kaim-eng]

/ok to test bd3da17

[kaim-eng]

@sttts thanks, agreed — fixed in bd3da174a.

The SIGTERM path now has three protections:

1. reconcile_once() checks _shutdown before persistence flushes, pod LIST, or GPU work, so if SIGTERM has already landed we immediately return to run()'s finally cleanup.
2. Pod LIST calls are bounded on both sides: timeout_seconds=20 for the apiserver and _request_timeout=25 as a best-effort client bound. Transport retries are disabled on this agent's dedicated Kubernetes client, so the timeout is not silently multiplied into retry load.
3. Helm now defaults terminationGracePeriodSeconds to 60 and rejects values below 60, giving cleanup headroom after a worst-case in-flight LIST.

Regression coverage:

• test_reconcile_failsafe.py asserts both LIST variants carry the dual bounds, the client timeout is >= the server timeout, transport retries are disabled, and the early shutdown guard skips persistence flushes and pod LIST.
• Helm unittest now covers default/override/rejection for terminationGracePeriodSeconds in both DaemonSet and dev-pod modes.

Validation at bd3da174a:

• python3.10 -m pytest deploy/power-agent/tests/ -q -> 319 passed
• helm lint + prod/dev render + helm unittest -> 48 passed
• git diff --check clean

Would appreciate your approval/sign-off if this addresses the P1.

[kaim-eng]

@nv-anants gentle approval nudge for #9790. The Power Agent shutdown fix is now on head bd3da174a2, the PR body is refreshed, and all Power Agent / Helm / Operator / Snapshot gates are green on this SHA.

The remaining full-PR CI reds are unrelated to this diff:

• trtllm-runtime / Test cuda13.1, amd64
• trtllm-runtime / Test cuda13.1, arm64
• backend-status-check (aggregate failure because trtllm-test failed)

Root cause: components/src/dynamo/trtllm/tests/test_trtllm_multimodal_encoder.py builds a synthetic transformers ModuleType fixture and only defines AutoConfig on it, then imports dynamo.trtllm.engine, whose line 13 imports PretrainedConfig. That fails against the incomplete stub (ImportError: cannot import name 'PretrainedConfig' from 'transformers' (unknown location)). This fixture/test path comes from upstream TRTLLM changes already in the merge base; PR #9790 touches no components/, TRTLLM, transformers, or container files.

Relevant #9790 checks are green:

• Power Agent -> success
• Helm Chart Tests -> success
• Operator / deploy-operator -> success
• Snapshot Agent -> success
• pre-commit / DCO / docs / Fern / PR-XPU guard -> success

@sttts also got the SIGTERM/pod-LIST fix explanation at bd3da174a2; waiting on reviewer sign-off now. Would appreciate your approval if the Power Agent changes look good from your side.

[nv-anants]

workflow and dockerfile changes look

[kaim-eng]

/ok to test b416308

[kaim-eng]

@sttts @nv-anants — heads-up on a rebase, no content change.

I rebased this branch onto current main (new head b416308d6f). The rebase was purely to pick up the multimodal-encoder test-fixture fix from #11383 (2c7ce4a8a3), which was the only thing turning full CI red — that failure was a pre-existing defect on main, not from this PR (this PR touches no components/, TRT-LLM, Transformers, or container files).

• The 7 Power Agent / Helm commits are unchanged in content; only their parent moved.
• Re-ran focused gates on the new head: 319 pytest + 48 Helm unittest all pass; every commit is DCO-signed.
• Full CI re-triggered on b416308d6f.

Your approvals carried over onto the new head, so nothing is required from you — this is just a courtesy notice. If you'd like to re-confirm given the rebase, that's welcome, but I don't want to reset anything unnecessarily. Thanks again for the reviews.