Skip to content

refactor(operator): extract reusable DCD workload renderer - #12115

Closed
julienmancuso wants to merge 4 commits into
mainfrom
jsm/12035-1
Closed

refactor(operator): extract reusable DCD workload renderer#12115
julienmancuso wants to merge 4 commits into
mainfrom
jsm/12035-1

Conversation

@julienmancuso

@julienmancuso julienmancuso commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Extract DCD workload rendering from DynamoComponentDeploymentReconciler into a concrete, reusable dcdWorkloadRenderer.

Overview

Rendering pod templates and Services is not itself a controller state machine, but it was previously implemented as methods on the DCD reconciler. That forced another workload implementation to either duplicate DCD rendering logic or construct a reconciler solely to call its helpers.

This change introduces a narrow composition boundary:

DCD or composite workload
    -> DCD workload renderer
        -> pod templates and Services

The controller remains responsible for reconciliation, status, watches, finalizers, readiness, and composing the rendered templates into a concrete workload resource.

Details

The package-private dcdWorkloadRenderer receives its dependencies explicitly:

  • Kubernetes client.
  • Operator configuration.
  • Runtime feature configuration.
  • Docker secret retriever.

It centralizes the existing rendering behavior for:

  • Base pod templates.
  • Leader and worker pod templates.
  • Multinode template pairs used by LeaderWorkerSet and future composite workloads.
  • Component Services.
  • Checkpoint-aware pod shaping.
  • ServiceAccount selection.
  • Legacy worker-selector compatibility.

DynamoComponentDeploymentReconciler delegates workload rendering directly to this renderer and composes the returned templates into its existing Deployment or LeaderWorkerSet resources. No public provider interface is introduced.

The renderer deliberately preserves the existing checkpoint preparation and compatibility lookups. Separating stable-resource preparation and per-reconcile resolved state is deferred to later composition-first work.

Where should the reviewer start?

  1. deploy/operator/internal/controller/dynamocomponentdeployment_renderer.go defines the extracted rendering boundary.
  2. deploy/operator/internal/controller/dynamocomponentdeployment_controller.go shows the controller delegating rendering and retaining reconciliation ownership.
  3. deploy/operator/internal/controller/dynamocomponentdeployment_controller_test.go checks renderer/controller resource equivalence and existing compatibility behavior.

Non-goals

This MR does not:

  • Introduce a generic workload-provider interface.
  • Change workload-pathway selection.
  • Change DGD or DCD status behavior.
  • Change rollout behavior.
  • Change resource ownership or finalizers.
  • Change controller watches.
  • Move code into new provider packages.
  • Introduce public APIs.
  • Make rendering completely side-effect-free.

Compatibility

The refactor preserves:

  • Generated pod specifications.
  • Leader and worker role labels.
  • Component and subcomponent labels.
  • Service selectors and names.
  • LeaderWorkerSet names and specifications.
  • Checkpoint metadata and injection behavior.
  • ServiceAccount selection.
  • Legacy worker-selector compatibility.
  • Existing create, update, delete, status, and readiness behavior.

Validation

docker buildx build --platform linux/arm64 --target linter --progress=plain --build-context snapshot=../snapshot .

docker buildx build --platform linux/arm64 --target tester --progress=plain \
  --build-context snapshot=../snapshot \
  --build-context operator=../operator \
  --build-context operator-chart=../helm/charts/platform/components/operator \
  .

Both Docker targets pass.

Related issues

Summary by CodeRabbit

  • Refactor

    • Centralized workload resource rendering for consistent leader, worker, service, and multinode deployment generation.
    • Improved compatibility with existing worker labels and selectors during deployment updates.
  • Bug Fixes

    • Ensured generated resources remain consistent across controller and renderer pathways.
    • Preserved checkpoint, discovery, metrics, labeling, and service-account behavior during resource generation.

@julienmancuso
julienmancuso requested a review from a team as a code owner July 24, 2026 20:01
@github-actions github-actions Bot added refactor deployment::k8s Relates to dynamo deployment in kubernetes labels Jul 24, 2026
@datadog-official

datadog-official Bot commented Jul 24, 2026

Copy link
Copy Markdown

Pipelines  Tests

⚠️ Warnings

🚦 5 Pipeline jobs failed

PR | TRTLLM DynamoCheckpoint Deploy Test   View in Datadog   GitHub Actions

🔄 Retry job. This looks flaky and may succeed on retry. v1alpha1 admission smoke test failed to become reachable: conversion webhook for nvidia.com/v1alpha1, Kind=DynamoGraphDeployment failed: connection refused to the webhook service.

PR | SGLang DynamoCheckpoint Operator Setup   View in Datadog   GitHub Actions

See error Failed to create DynamoGraphDeployment due to connection refused on webhook service for v1alpha1 API version.

PR | deploy-operator   View in Datadog   GitHub Actions

See error Error syncing: patch host object: update object: Operation cannot be fulfilled on pods 'grove-operator-5c999446c5-fd4k4-x-default-x-ci-30649458250': the object has been modified; please apply your changes to the latest version and try again

View all 5 failed jobs.

ℹ️ Info

🔄 Datadog auto-retried 1 job - 1 passed on retry View in Datadog

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 8a684fb | Docs | Datadog PR Page | Give us feedback!

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

Workload rendering centralization

Layer / File(s) Summary
Reusable workload renderer
deploy/operator/internal/controller/dynamocomponentdeployment_renderer.go
Adds shared rendering for multinode pod templates, services, workload labels, checkpoint configuration, service accounts, and legacy worker selector compatibility.
Controller delegation and LWS composition
deploy/operator/internal/controller/dynamocomponentdeployment_controller.go
Routes pod, service, label, component-type, and selector generation through the renderer and uses its templates for LeaderWorkerSet resources.
Renderer/controller equivalence test
deploy/operator/internal/controller/dynamocomponentdeployment_controller_test.go
Compares directly rendered services and pod templates with resources composed by the controller.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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: extracting a reusable DCD workload renderer.
Description check ✅ Passed The description covers the required sections and adds useful details, review guidance, validation, and a related issue.

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

coderabbitai[bot]

This comment was marked as resolved.

Signed-off-by: Julien Mancuso <jmancuso@nvidia.com>
Signed-off-by: Julien Mancuso <jmancuso@nvidia.com>
@julienmancuso
julienmancuso requested a review from a team as a code owner July 31, 2026 17:00

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

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.

1 participant