Skip to content

refactor(operator): move power admission validation to Go - #12481

Closed
sttts wants to merge 47 commits into
mainfrom
sttts-admission-power-validation
Closed

refactor(operator): move power admission validation to Go#12481
sttts wants to merge 47 commits into
mainfrom
sttts-admission-power-validation

Conversation

@sttts

@sttts sttts commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • move DGD GPU power-input immutability from generated CEL in deploy/operator/Makefile into the structural Go update validators
  • preserve v1beta1 and v1alpha1 source-version behavior, including limits-over-requests and legacy custom-resource precedence
  • report precise invalid field paths and keep standalone DynamoComponentDeployment updates unaffected
  • remove the DGD container maxItems bound that existed only to satisfy CEL cost estimation

This PR is stacked on #12012 and should be reviewed as its follow-up diff.

Validation

  • make manifests
  • KUBEBUILDER_ASSETS=... go test ./internal/webhook/validation -run '^TestDynamoGraphDeploymentValidator_Validate$' -count=1
  • KUBEBUILDER_ASSETS=... go test ./internal/webhook/validation -count=1
  • make vet
  • make lint
  • repository pre-commit hooks

kaim-eng and others added 30 commits July 30, 2026 09:21
Summary
-------
Rework the power feature so the Planner READS per-GPU caps from DGD worker
podTemplate annotations (dynamo.nvidia.com/gpu-power-limit) and clamps replica
scaling against a total power budget, instead of owning/PATCHing per-Pod caps.
Supersedes the Planner-owned Pod-PATCH stack (#9683-#9685).

- Caps are authored on worker podTemplate annotations, applied to Pods by the
  operator, and enforced by the Power Agent; the Planner only reads them.
- DGD parsing: Service.get_gpu_power_limit_watts, ComponentPowerConfig,
  resolve_component_power_configs (disagg + agg via the generic type:worker
  fallback), positive-int GPU/nodeCount validation.
- Config: remove Planner-owned per-GPU write fields (rejected via a migration
  validator); require environment=kubernetes when awareness is on.
- Deployment state/capabilities carry resolved per-replica watts and a
  deployment-scoped scale-up-blocked flag. Init is strict/fail-closed; runtime
  is conservative (keep last-good, hold the per-role max, block scale-up on a
  changed/malformed cap or a transient apiserver error).
- Projection gauges read cached state; final budget boundary applies GPU budget
  then power budget (ceiling-only clamp, decode-no-upscale, partial-proposal
  masks; power wins over the GPU floor), plus a startup min-footprint gate.
  While ANY power-relevant role is mid-rollout the Kubernetes environment marks
  BOTH roles' settled target unknown (one deployment-wide stability flag), so
  every role's scale-up is held at its ready count -- keyed off rollout state,
  not None targets, since the merge fills every role. The projection then masks
  any target equal to its ready count back to None, so a held scale-up emits
  nothing and a scale-down of one role does not drag the other role's echoed
  ready count along (DisaggPlanner applies every non-None target as the new DGD
  desired); the in-flight rollout's desired is left untouched and a proposal
  cannot admit an over-budget settled state.
- Remove the Pod sweep/PATCH machinery, pods/patch RBAC, and superseded frontend
  scaffolding (get_component_pods / list_frontend_pods / post_busy_threshold);
  the baseline get_frontend_metrics_url is retained. Correct Power Agent
  ownership docstrings. Add example + docs.

The budget is a projected ceiling over the requested caps, not a proven hardware
limit; effective-cap feedback is deferred to a dynamic-control design.

Validation
----------
- 555 planner tests pass (unit suite + mocked no-mutation integration gate);
  black/isort/ruff/flake8 clean. (The broader plugins/offline test trees need
  grpc/aiconfigurator, absent in this env, and are unaffected by this change.)
- Cut fresh from origin/main; power-only diff -- no aggregate name-plumbing (the
  generic type:worker fallback and the power resolver are kept).

Refs #9683, #9684, #9685

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Kai Ma <kaim@nvidia.com>
Own the new examples/power-aware-budget/ under planner and ignore the
main-branch GitHub URL until the example lands, so short CI can pass.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Empty commit to re-fire pull_request checks after synchronize
did not schedule Pre Merge / codeowners / lychee on 98dd214.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Prevent baseline-echoed roles from being adjusted by the final power budget, remove confirmed dead helpers, and surface blocked-scale diagnostics so the reviewed edge cases remain observable.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Clamp partial GPU proposals against fixed peer usage and conservatively block scale-up when deployment inventory refreshes fail.

Signed-off-by: Kai Ma <kaim@nvidia.com>
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>
Remove unpublished power-write migration surface, gate ready-equal
projection masks behind enable_power_awareness, and restore origin/main
disagg GPU-budget clamping (drop partial residual policy).

Signed-off-by: Kai Ma <kaim@nvidia.com>
Chain invalid power annotation errors and restore coverage ensuring GPU clamps preserve single-component proposal masks.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Make the non-optional proposal invariant explicit so the single-pool clamp cannot imply a dead None path.

Signed-off-by: Kai Ma <kaim@nvidia.com>
power_watts_per_replica is cached at startup while num_gpus refreshes each
tick, so a worker rollout that changes GPUs-per-replica left the power budget
projected against stale wattage and could admit an over-budget scale-up. Snapshot
the startup per-pod GPU count and fail closed on divergence, requiring a Planner
restart to re-read the DGD annotation. Also narrow the example README budget claim
to the scale-up-admission guarantee (no automatic over-budget remediation).

Signed-off-by: Kai Ma <kaim@nvidia.com>
Re-resolve runtime GPU topology through the power-config path so aggregate and named generic workers cannot retain stale per-replica wattage after a rollout. Keep startup caps static and fail closed until Planner restart when replica-wide GPU topology changes or cannot be verified.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Include the per-GPU cap in the startup power fingerprint so a cap-only DGD rollout cannot resume scaling against stale per-replica watts. Fail closed until Planner restart when either cap or GPU topology changes.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Reuse one Kubernetes DGD GET for GPU counts and the power fingerprint guard when awareness is on, fail closed if the startup fingerprint is missing, and log when power undoes a GPU-floor raise.

Signed-off-by: Kai Ma <kaim@nvidia.com>
… peaks

Remove the per-tick DGD power fingerprint guard (_assert_power_config_static and runtime drift tests). Caps are read once at startup; a change requires worker rollout plus Planner restart.

After proportional clamping, re-run peak/staging against the final target so a clamp-synthesized opposing rebalance (e.g. current (4,1), proposal (5,5) -> clamped (2,3) peaking at (4,3)) is staged instead of emitted in one tick.

Signed-off-by: Kai Ma <kaim@nvidia.com>
When exactly one role is proposed, charge the peer at its ready count and size the adjustable role against the residual GPU ceiling/floor. Joint-then-discard assumed the peer also shrank and could emit an over-ceiling applied state (e.g. current (7,1), decode-only 7, ceiling 8 -> applied (7,4)=11 GPUs). Both-role proposals keep the joint proportional clamp.

Signed-off-by: Kai Ma <kaim@nvidia.com>
… holds

Inspect method signatures before forwarding deployment= so real TypeErrors inside connectors are not silently retried. Emit the mid-rollout scale-up hold warning once per continuous rolling stretch (reset when stable). Document why initialize() refreshes again after FPM async_init.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Limit single-role residual GPU sizing to enable_power_awareness so power-off disagg keeps the historical joint-then-discard clamp (Ted P2). Fix mypy: avoid empty-dict assignment in signature inspect fallback, and assert ready counts before residual math.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Remove stale runtime drift and scale-up-blocked wording so documentation matches the init-only static-cap contract.

Signed-off-by: Kai Ma <kaim@nvidia.com>
…U ceiling; fix None-current hold

Two correctness fixes from post-rebase review:

1. Residual GPU clamp (engine_adapter.py): when the fixed role ready count alone
   meets or exceeds max_gpu_budget, residual_max drops to 0 and
   proportional_clamp_single would return 0 replicas -- a spurious scale-to-zero.
   Guard before the clamp: if residual_max < min_endpoint * gpu, hold the proposed
   role at min(desired, ready). Symmetric guard for both single-role branches.

2. _hold_at_current (budget.py): current=None was treated as no baseline so a
   create-from-nothing proposal passed through when the fixed peer alone already
   exhausted total_gpu_power_limit. Treat current=None as baseline 0.

Tests: pin num_decode==3 in the existing mask test (was a weak range); add
test_residual_gpu_clamp_holds_when_fixed_peer_already_over_ceiling for the
over-ceiling case (ready_prefill=9, ceiling=8, decode proposes 4 held at 1);
add test_partial_proposal_suppressed_when_current_missing_and_peer_over_budget
for the None-current power budget path.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Kai Ma <kaim@nvidia.com>
Cache startup power caps only from the DGD snapshot whose backing DCD or Grove resources have adopted the desired worker revision, preventing stale Pods from running above a newly lowered cap.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Treat a budget hold at an unobserved zero baseline as no change, preserve startup wait ordering for duck-typed connectors, and remove runtime change checks for startup-static power fields.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Signed-off-by: Kai Ma <kaim@nvidia.com>
Keep legacy power-disabled readiness compatible while ensuring all power-resolved workers, including untyped named components, finish their backing rollout before caps are cached.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Signed-off-by: Kai Ma <kaim@nvidia.com>
The Kubernetes validate_deployment and GPU-count paths use the typed
decode role resolver (get_component_from_type_or_name), which does not
follow the generic type:worker fallback added by the power parser in
_resolve_one_power_service(). For a standard vLLM agg DGD (VllmWorker,
type: worker), validate_deployment would fail before power caps are ever
read, making power awareness unreachable for agg.

Add explicit PlannerConfig validation rejecting the combination, one
config test, and comments on the parser-level agg coverage tests
clarifying they do not imply end-to-end agg+power support.

Signed-off-by: Kai Ma <kaim@nvidia.com>
enable_power_awareness is not supported with mode='agg'. Add this
restriction to the field description in PlannerConfig, the planner
examples doc, and the power-aware-budget example README.

Signed-off-by: Kai Ma <kaim@nvidia.com>
…matting

Signed-off-by: Kai Ma <kaim@nvidia.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Deployment/LWS status.componentNames are underlying workload names
(e.g. <dcd>-deployment), not DCD names. Looking them up as DCDs
404s in production and can stall power-aware Planner startup.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Mirror operator bridge precedence so a stale-but-ready v2 DCD cannot
short-circuit settlement while the active v1 DCD still lags.

Signed-off-by: Kai Ma <kaim@nvidia.com>
kaim-eng and others added 5 commits July 30, 2026 09:22
P2-1: move RolloutFailedError check before the stability gate on the
require_backing_settled path so unstable-replica Failed rollouts raise
immediately instead of burning the full 30-minute timeout. The check
is placed before the include_planner branch so it fires on both paths;
the now-dead block below is removed and its comment relocated.

P2-2: extend runtime get_actual_worker_counts to treat
rollingUpdate.phase=InProgress/Pending and phase=Failed as unstable
when check_terminating_pods=True. Failed was excluded from
ROLLING_UPDATE_BLOCKING_PHASES because startup raises immediately;
at runtime there is no raise so we fail-closed instead.

P3-2: flip wait_for_settled_graph_deployment default include_planner
to False in both KubernetesConnector and PowerAwareConnector Protocol;
production callers already pass False explicitly.

Tests: add test_wait_failed_rollout_raises_immediately_while_replicas_unstable,
is_rolling_update_blocking_settlement default to mock_kube_api fixture,
connector tests for InProgress and Failed runtime instability (power-on)
and stable legacy path (power-off).

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Kai Ma <kaim@nvidia.com>
M1 — verbatim annotation propagation: add Service.get_gpu_power_limit_annotation()
returning the raw DGD string and switch wait_for_graph_deployment_ready to use it
for expected_power; the operator copies podTemplate annotations verbatim onto Pods,
so settlement must compare the same raw value, not the canonical str(int). Update
test to assert DGD ' 350 ' + Pod ' 350 ' settles and DGD ' 350 ' + Pod '350' does
not; rename test to reflect verbatim-propagation semantics.

M2 — conditional pods/list RBAC: gate both ClusterRole and namespace-restricted Role
pods/list rules behind planner.powerAwareness.enabled (default false) in the Helm
chart; add planner_rbac_test.yaml with four helm-unittest cases covering enabled/
disabled in both RBAC modes; document helm dependency build + the new Helm flag in
the power-aware example README and planner-examples.md.

m1 — protocol guard: replace isinstance(..., PowerAwareConnector) with
is_power_aware_connector() using inspect.getattr_static + callable() so the check
is correct on Python < 3.12 and rejects non-callable sentinels; return
TypeGuard[PowerAwareConnector]; add four unit tests (bare MagicMock, valid
connector, missing method, non-callable attribute).

m2 — stale docstring: fix get_graph_deployment docstring (was "duck-typed via
getattr"; now references is_power_aware_connector); fix check_terminating_pods
docstring RBAC comment (permission is installation-time, not per-instance).

m3 — _shrink_pair no-growth: cap new_d at num_d with min(num_d, max(min_endpoint,
floor(remaining / d_watts))); fix direction comment (p_watts >> d_watts, not <<);
add regression test for the asymmetric-wattage case.

Signed-off-by: Kai Ma <kaim@nvidia.com>
…_pods on PowerAwareConnector

External PlannerConnector implementations using the pre-PR signature
crashed with TypeError on every planning cycle because the call site
unconditionally passed check_terminating_pods regardless of whether
power awareness was enabled.

- Restore PlannerConnector.get_actual_worker_counts to the main-branch
  signature (no check_terminating_pods) so external connectors are not broken
- Add get_actual_worker_counts with check_terminating_pods to PowerAwareConnector
  so the type-narrowed power-aware branch is fully typed
- Environment now branches explicitly: power-disabled path calls the old
  signature; power-enabled path checks is_power_aware_connector and raises
  DeploymentValidationError on mismatch before passing check_terminating_pods=True
- Remove check_terminating_pods from VirtualConnector and GlobalPlannerConnector
  (neither environment supports power awareness)
- Add three regression tests: legacy-signature connector with power off,
  power-aware connector receives True, non-power-aware + power-on raises

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Kai Ma <kaim@nvidia.com>
…-budget

- Add v1beta1/v1alpha1 CRD CEL transition rules immutabilising the
  power annotation, effective main-container nvidia.com/gpu count, and
  multinode.nodeCount for annotated DGD components; regenerate CRD.
  spec.components is a list-map keyed by name so oldSelf correlates by
  name across reordering.
- Consolidate runtime Pod inspection into one DGD-scoped LIST inside
  asyncio.to_thread; partition locally by component label, eliminating
  two synchronous per-component LISTs from the Planner event loop.
- Add get_power_aware_worker_counts to PowerAwareConnector protocol and
  _POWER_AWARE_REQUIRED; retire check_terminating_pods kwarg that left
  the TypeGuard incomplete.
- Carry PROPOSE-stage component provenance through PipelineOutcome as a
  frozenset; replace equality-based mask inference in _project_scale_to
  with explicit proposed_components; held scale-ups emit None directly.
- Update Planner config, docstrings, docs, and power-aware example to
  state CRD immutability contract and DGD-recreation/Planner-restart
  requirements honestly.

Signed-off-by: Kai Ma <kaim@nvidia.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Kai Ma <kaim@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
@github-actions github-actions Bot added deployment::k8s Relates to dynamo deployment in kubernetes refactor labels Jul 31, 2026
Comment thread deploy/operator/internal/webhook/validation/dynamographdeployment_helpers.go Outdated
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Comment thread deploy/operator/internal/webhook/validation/dynamographdeployment.go Outdated
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Comment thread deploy/operator/internal/webhook/validation/dynamographdeployment.go Outdated
Comment thread deploy/operator/internal/webhook/validation/dynamographdeployment_helpers.go Outdated
Comment thread deploy/operator/internal/webhook/validation/dynamographdeployment_helpers.go Outdated
Comment thread deploy/operator/internal/webhook/validation/dynamographdeployment.go Outdated
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Comment thread deploy/operator/internal/webhook/validation/dynamographdeployment_helpers.go Outdated
sttts added 2 commits July 31, 2026 13:45
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Comment thread deploy/operator/internal/webhook/validation/shared_v1beta1.go Outdated
Signed-off-by: Dr. Stefan Schimanski <sschimanski@nvidia.com>
Base automatically changed from feat/dgd-owned-power-budget to main August 4, 2026 19:56
@sttts sttts closed this Aug 10, 2026
@sttts
sttts deleted the sttts-admission-power-validation branch August 25, 2026 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deployment::k8s Relates to dynamo deployment in kubernetes refactor size/XXL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants