feat(planner): DGD-owned GPU power caps and budget-aware scaling - #12012
Conversation
WalkthroughThe planner adds Kubernetes-only power-aware scaling. It reads DGD-owned GPU power annotations, validates and tracks per-role power data, clamps projected replica counts against a total budget, publishes power metrics, and adds tests, documentation, and an example deployment. ChangesPower-aware scaling
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/src/dynamo/planner/monitoring/dgd_services.py`:
- Around line 267-270: Update the exception handling in the watts parsing block
to explicitly chain PowerAnnotationInvalidError from the caught ValueError or
TypeError using from; preserve the existing validation error and message while
retaining the original parse failure as its cause.
In `@components/src/dynamo/planner/tests/integration/test_power_no_mutation.py`:
- Around line 139-141: Update the Step 3 test setup around WorkerCounts and
_apply_final_budget so expected_num_prefill and expected_num_decode are
explicitly set to stable target counts matching the proposed (4, 4) scale-up.
This must prevent _hold_scale_up_during_rollout from reducing the proposal
before apply_power_budget runs, allowing the test to exercise proportional
clamping against the 5000 W budget.
In `@examples/power-aware-budget/README.md`:
- Around line 1-6: Update the documentation header in the example README to
follow the repository’s Fern convention: replace the HTML SPDX comment with the
required YAML frontmatter, and change the body-level “DGD-owned power caps and
budget-aware scaling” H1 to an `##` heading. Follow the metadata structure
specified by docs/AGENTS.md.
- Line 18: Fix the blank line between the two blockquote callouts in the README
by either replacing it with a `>` separator line or removing it, while
preserving both callouts and making the content MD028-compliant.
🪄 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: 6db45d29-47a9-425d-a4d4-de48c37bb54c
📒 Files selected for processing (32)
components/src/dynamo/planner/config/defaults.pycomponents/src/dynamo/planner/config/planner_config.pycomponents/src/dynamo/planner/connectors/base.pycomponents/src/dynamo/planner/connectors/kubernetes.pycomponents/src/dynamo/planner/connectors/virtual.pycomponents/src/dynamo/planner/core/base.pycomponents/src/dynamo/planner/core/budget.pycomponents/src/dynamo/planner/core/types.pycomponents/src/dynamo/planner/core/util.pycomponents/src/dynamo/planner/environment/base.pycomponents/src/dynamo/planner/environment/state.pycomponents/src/dynamo/planner/errors.pycomponents/src/dynamo/planner/monitoring/dgd_services.pycomponents/src/dynamo/planner/monitoring/planner_metrics.pycomponents/src/dynamo/planner/plugins/orchestrator/engine_adapter.pycomponents/src/dynamo/planner/tests/integration/test_power_no_mutation.pycomponents/src/dynamo/planner/tests/unit/test_actuation_knobs.pycomponents/src/dynamo/planner/tests/unit/test_dgd_power_annotation.pycomponents/src/dynamo/planner/tests/unit/test_metric_publication.pycomponents/src/dynamo/planner/tests/unit/test_planner_config.pycomponents/src/dynamo/planner/tests/unit/test_power_annotation_contract.pycomponents/src/dynamo/planner/tests/unit/test_power_budget.pycomponents/src/dynamo/planner/tests/unit/test_power_environment.pydeploy/helm/charts/platform/components/operator/templates/planner.yamldeploy/helm/charts/power-agent/values.yamldeploy/power-agent/power_agent.pydeploy/power-agent/tests/test_annotation_scope.pydeploy/power-agent/tests/test_multi_dgd_topology.pydocs/components/planner/planner-examples.mdexamples/power-aware-budget/README.mdexamples/power-aware-budget/dgd.yamlexamples/power-aware-budget/planner_config.json
|
/ok to test 52363b6 |
|
🎯 Code Coverage (details) 🔗 Commit SHA: deb67c3 | Docs | Datadog PR Page | Give us feedback! |
|
/ok to test 57d2efd |
|
Review nudge for CODEOWNER stamps on tip What this PR does: planner stops PATCHing Pod power caps; it reads Current state: short CI green / near-green; full CI re-triggered via Please stamp your area when you have a chance:
Happy to walk any of the above live if useful. |
tedzhouhk
left a comment
There was a problem hiding this comment.
One blocking correctness issue in the power-budget actuation path.
tedzhouhk
left a comment
There was a problem hiding this comment.
Additional planner findings: three blocking scope/correctness issues and four non-blocking cross-cutting behavior changes.
When settled (4P,1D) fits the budget but parallel rollouts would peak above the ceiling (e.g. (1P,4D)->(4P,1D)), defer scale-up legs via peak_parallel_watts and emit scale-downs first. Also implement GlobalPlannerConnector.get_component_power_configs for mypy. Fixes Ted P1 on #12012 and the dynamo-runtime / mypy CI failure. Signed-off-by: Kai Ma <kaim@nvidia.com>
…refresh Address Ted review on #12012: read DGD power annotations once at startup after worker readiness, remove runtime re-read/sticky scale-up blocking, restore origin/main refresh fail-stop when power awareness is off, drop get_component_power_configs from PlannerConnector, and localize agg type:worker resolution to the power parser. Signed-off-by: Kai Ma <kaim@nvidia.com>
|
All 16 review threads on tip
Short CI is green; merge is still blocked only on @tedzhouhk — when you have a chance, could you re-review the tip and stamp if the earlier findings look closed? |
|
/ok to test 5fe990d |
|
/ok to test 84a8316 |
|
C:/Program Files/Git/ok to test 3983b3d |
|
/ok to test 3983b3d |
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Signed-off-by: Kai Ma <kaim@nvidia.com> # Conflicts: # .lycheeignore
Signed-off-by: Kai Ma <kaim@nvidia.com>
|
C:/Program Files/Git/ok to test 354a024 |
|
/ok to test 354a024 |
…gained get_power_aware_worker_counts All eight test_power_environment failures on amd64/arm64 stem from _POWER_AWARE_REQUIRED expanding to four methods when get_power_aware_worker_counts was added, while the test mocks only supplied three. - _power_controller(): add get_power_aware_worker_counts = AsyncMock(return_value=(1, 1, True)); update docstring "three"→"four" - Positive-case and three negative-case protocol tests: add the fourth method so each test exercises exactly the defect its name claims - test_init_requires_power_capable_connector: widen match to "PowerAwareConnector" to track the updated error message - test_initialize_caches_caps_from_settled_snapshot_not_lagging_get: add get_power_aware_worker_counts for both _refresh_deployment_state calls inside initialize() Production: align _resolve_power_configs error message with the other two guards (list all four methods); update stale "three methods" comment in kubernetes.py. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Kai Ma <kaim@nvidia.com>
|
/ok to test a7000d8 |
…ate time - Add validateDGDPowerLimitValue helper: rejects non-integer and non-positive values at admission so a bad annotation surfaces as a rejected DGD rather than a failing Planner Pod against an immutable annotation. - Extend dgdDRAPath to scan InitContainers in addition to Containers, closing a theoretical bypass of the DRA + power incompatibility guard. - Add four envtest cases covering zero, negative, non-integer, and init-container DRA scenarios. - Fix stale docstring on _resolve_one_power_service in dgd_services.py. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Kai Ma <kaim@nvidia.com>
|
C:/Program Files/Git/ok to test 6dc517e |
validateDynamoGraphDeploymentSpec hit cyclomatic complexity 31 (limit 30) after the inline power-value and DRA checks were added. Extract both checks into validateDGDComponentPowerAnnotation, reducing the parent function complexity by 3. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Kai Ma <kaim@nvidia.com>
|
C:/Program Files/Git/ok to test deb67c3 |
|
/ok to test deb67c3 |
1 similar comment
|
/ok to test deb67c3 |
jthomson04
left a comment
There was a problem hiding this comment.
approving planner_metrics.py observability changes.
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Only approving process changes from DevOps side.
areas.yaml auto-merged; only the generated CODEOWNERS conflicted. Resolved by regenerating from the merged areas.yaml rather than hand-merging, so the committed artifact is reproducible and the workflow's regenerate-and-diff step passes. Main gained ownership entries from #12488, #12012, #11874 and #11923 while this branch was in review, plus #12361, which co-owns the docs publish workflow and the link-checker config with docs. All survive: fern-docs.yml and .lycheeignore still resolve to ops and docs. This branch's own additions survive too: CODEOWNERS carries all 23 areas and areas.yaml carries ops and process. Validation: strict full-tree gate exits 0 at 5200/5200 owned with no stale globs; 150 tests pass. Signed-off-by: Dan Gil <dagil@nvidia.com>
Summary
Add Kubernetes-only, power-aware scaling to the Planner. Per-GPU caps are owned by DynamoGraphDeployment (DGD) worker
podTemplateannotations, propagated to Pods by the operator, and enforced by the Power Agent. The Planner is read-only with respect to those caps: it resolves each role's per-replica power cost at startup and clamps scaling proposals againsttotal_gpu_power_limit.This replaces the Planner-owned Pod PATCH design from #9683, #9684, and #9685. The power path has no Pod write surface and requires no
pods/patchRBAC.Details
Power ownership and admission contract
dynamo.nvidia.com/gpu-power-limiton their DGDpodTemplate.nvidia.com/gpucount andmultinode.nodeCount, and caches watts per logical replica.failurePolicy: Failby default. Phase-1 cached accounting assumes this admission contract is enforced.Power awareness is supported for Kubernetes disaggregated Planner deployments. Configuration validation requires
environment=kubernetes,mode=disagg, and a positivetotal_gpu_power_limit.Budget and rollout behavior
min_endpoint.The total budget is a projection over requested caps, not measured hardware draw. Effective-cap feedback and dynamic cap retargeting are outside this phase.
Additional cleanup
This PR also removes unused diagnostic-projection helpers from
engine_adapter.pyand its module-local__all__. Repository-wide search found no call sites.Where should reviewers start?
deploy/operator/internal/webhook/validation/dynamographdeployment.godeploy/operator/internal/webhook/validation/dynamographdeployment_helpers.godeploy/operator/internal/webhook/validation/dynamographdeployment_validation_envtest_test.gocomponents/src/dynamo/planner/monitoring/dgd_services.pycomponents/src/dynamo/planner/environment/base.pycomponents/src/dynamo/planner/connectors/kubernetes.pycomponents/src/dynamo/planner/core/budget.pycomponents/src/dynamo/planner/plugins/orchestrator/engine_adapter.pycomponents/src/dynamo/planner/plugins/orchestrator/pipeline.pycomponents/src/dynamo/planner/tests/unit/test_power_budget.pycomponents/src/dynamo/planner/tests/unit/test_power_environment.pycomponents/src/dynamo/planner/tests/unit/test_kubernetes_connector.pydeploy/operator/internal/webhook/validation/dynamographdeployment_validation_envtest_test.goRelated issues
Supersedes the unmerged Planner-owned Pod-PATCH stack: #9683, #9684, and #9685.
Validation
Current tip:
354a0240c9491d7953b97f59a11b20c1eb3377fb.Passing on the current tip:
The full CI run is still in progress. The Planner CPU-test jobs on amd64 and arm64 are currently failing in the parallelized CPU-test step; detailed logs are pending workflow completion and must be investigated or retried before merge.