Repository navigation
feat(operator): add LPX integration - #15062
Conversation
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Syncs the complete lpu-dynamo delta between the recorded fork checkpoints. Fork-Source-Base: 8f44ed118d7f82289096bebb57bd215da7db9af4 Fork-Source-Head: bedd20140be7a5ed325b99a1ec4285420569c161 Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit f5da9fc2663bc7946d720006a42c7d3344cac1ca)
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit dbdd330feec73058704974646544d995134864f8)
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 6f4276c447a5f0244b46f82d251387c3b4a95cae) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 2d69c3fcd50d24d0cec9fd7b123d7ae0b4b1dec6) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 9f054ffd4087fcec23affa44ebf628f401b3c955) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 52b050a592234fb774ed7ea4468a91b4354a3ab3) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 7d7c2e66c4994b81a56fc0889af289fcb72eb274) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit ee4c42ac02da237e9f991743a1da9f2b33bc96ae) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Replace --start-port with --controlplane-start-port in the generated LPX conductor command and update the rendered YAML fixtures. --start-port was an alias for --controlplane-start-port, so using the canonical name should not change runtime behavior. The port remains 12345. Recent lpu-conductor builds removed the alias, causing startup to fail with an unexpected-argument error. Validation: go test ./internal/dynamo/lpx ./internal/dynamo; go test ./internal/controller/lpx -run ^TestGenerateGrovePodCliqueSet_FromDGDYaml$ (from deploy/operator, with KUBECONFIG=/dev/null). (cherry picked from commit e80a65494d3194bf0e43191db18fd375256810a6) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit fdc2a19c2d79f08dfbe74a1ee45c968fc5bab59f) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 84d151f25d46ce4b96488e00b61c4217e0c562e7) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 195b1cb4016664716b80177b5b468e41e57e1960) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 7b484829a90e322acceea3788d7ac9a148d4abd0) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit f10a6a1160ac8b4ddf83c802f92d483d62fab190) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit ff3e0d7b0d1f71cdc891ba328c17fef79c99914a) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit a54ce59f447d55d7c9b67d43bc0794e8acb255cb) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit a6498294dccd9a88d86578e262ef4ad0322ef222) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 1d182279fb9364f856d61888f1315c323d9ed59f) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 7b90a943bbfe15546c829438064cc3fe5b28d063) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 646bcee4ad121fbe5a3940c9dfcc41d9ee29d20d) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit ac5b298d9a91b6f99a2e9ef5000c81456704ed93) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit c29f1bdbb34e1e9d4364f9383ed46afcddc63ece) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 9497019a4b338e2beedf187cb181b15c847ab41a) Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit db335a18b4bbdd8a18c18b8bb3c4f318f1e408df)
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com> (cherry picked from commit 21548b8a755c6e200b922fe1d05e4dcfc48d6839)
|
@sttts Addressed all five points in 4ad02ea4e: documented experimental stability, gated admission with unchanged-component allowances, conditional CRD installation and controller/watch registration, and early disabled status before workload writes. Existing resources and deletion remain supported. Added regression coverage; operator/envtest and Helm tests pass. |
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
|
/ok to test f90624b |
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
|
/ok to test 3104d46 |
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Approving at 3104d46d29. The changes since my approval at cbe156469b meet the bar, and the four earlier findings are fixed. I found no new defect in the parts I read.
Open items: none.
What I ran again at 3104d46d29, and the result of each.
- Round 1, absolute
buildIdwith a configured registry,internal/dynamo/lpx/model_registry.go:129.TestRegistryBuildURLpasses 16 of 16. Without that rejection, exactly the 8 rows for absolute references fail. - Round 1, the LPX checkpoint stanza. Admission now refuses it. My reply on that thread has the measurement.
- Round 1, the
podcliquesets/finalizersgrant. It is atdeploy/helm/charts/platform/components/operator/files/role.yaml:202, with its marker atinternal/controller/lpx/controller.go:83. The Grove owner atinternal/controller/lpx/pipeline_request.go:179is still the only owner outsidenvidia.com. - Round 2,
CheckPCSGReadycoverage. It is back to 100 percent, and the mutant that always reports ready fails 15 subtests. My reply on that thread has the numbers. CODEOWNERS. Thebuild_codeowners.py --strictgate passes, andemit_codeowners.pywrites a byte-identical file.
I ran the Go probes in the golang:1.26.6 image with envtest assets for Kubernetes 1.30.0.
The DGD CRD: the operator sends 45 percent of the 3 MiB request limit.
crd-apply sends json.Marshal(crd) as a server-side apply patch, at cmd/crd-apply/main.go:109. I repeated its unmarshal and marshal on each file.
| tree | file | request body |
|---|---|---|
main at 59ac36782c |
3,019,275 bytes | 3,058,534 bytes of YAML, 97.23% |
3104d46d29 |
3,025,860 bytes | 1,415,460 bytes of JSON, 45.00% |
main still sends yaml.Marshal(crd), so this PR lowers the risk. TestCRDApplyInstallsGeneratedSchemas builds the production installer and applies these files to a local API server with its default limits. It passed at this head.
What I read, and what I did not read.
Since cbe156469b the branch has 86 commits of its own. They change 169 files, +12253/-12309. The rest came from 5 merges of main.
I read the full change in the 26 files that ordinary graphs without LPX run through. These files hold the Grove program, grove.go, graph.go, the DGD controller, and the watch predicates. They also hold the pod cache, admission, conversion, the API types, crd-apply, and the operator Helm template. I also read the new LPX gate, the restart rule in CEL and its tests, and Reconcile in internal/controller/lpx/controller.go.
I did not read line by line the rest of the LPX source, about 45 files with 976 new lines. I also did not read the 54 changed test files, the 23 fixtures, or the new internal/controller/lpx/AGENTS.md. LPX stays off by default, and TestSetupDynamoGraphDeploymentWithoutLPXCRDs starts the DGD controller without the LPX CRDs.
CI at this head: Pre Merge passed, and the PR workflow is still running.
Pre Merge run 36475332052 passed. Its operator job ran envtest for internal/controller, internal/webhook/validation, and cmd/crd-apply, and make check finished clean.
The PR workflow, run 36475337054, is still running. The PR diff at f90624ba40 is byte-identical to this head. There, Helm Chart Tests, Operator, Operator Integration, and the vLLM and SGLang snapshot deploy tests passed. The red jobs there do not come from this diff. TRTLLM Snapshot Deploy Test also fails in all 10 post-merge runs on main since 2026-09-24 that ran it. The three GPU test jobs stopped mid-test, two of them at the same second, and this PR changes no backend code.
8c74b28 to
f6d6c3e
Compare
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Approving at f6d6c3e745: the PR still meets the bar. After my approval at 3104d46d29, the only change is this commit, which touches the TRT-LLM checkpoint test and its two CI jobs.
Open items:
TRTLLM Snapshot Deploy Testin run 36585260018 has not finished. It is the evidence that the agent preload fixes the restore.- P3: the test value replaces the image's own
LD_PRELOAD.tests/deploy/test_dynamocheckpoint.py:186. - P3: no comment gives the reason that the worker and the agent need the host driver, or the removal condition for the step.
.github/workflows/pr.yaml:2057.
Why the agent step matters: at 8c74b2842f, the worker preload alone left the restore hanging.
| head and run | worker LD_PRELOAD |
snapshot agent | checkpoint | restored worker |
|---|---|---|---|---|
3104d46d29, run 36557305687 |
image default | chart default | did not finish in 12 minutes | not reached |
8c74b2842f, run 36571980780, attempt 1 |
/usr/$LIB/libcuda.so.1 |
chart default | finished in 70 s | not ready at the 900 s deployment timeout |
8c74b2842f, run 36571980780, attempt 2 |
/usr/$LIB/libcuda.so.1 |
chart default | finished in 68 s | not ready at the 900 s deployment timeout |
f0f8b69, run 36546302715 |
host libcuda.so.595.58.03 and the two image entries |
LD_PRELOAD of the host libcuda.so.595.58.03 |
finished | ready, and the test passed in 456 s |
At 8c74b2842f, the agent log ends at CRIU post-restore: process restored in both attempts. Your driver probe in run 36534810617 shows the same pattern. The helper615_target595_restore case timed out at restore, and matched595 passed all four steps.
This head adds the agent preload. f0f8b69 also put the host library directory ahead of the compat directory in the worker LD_LIBRARY_PATH, and this head does not. The open CI job shows whether the restore needs that too.
What else I checked in f6d6c3e745.
-
The commit changes a test and two CI jobs. It changes no operator code, Helm chart, or image, so it cannot hide a missing library in a shipped image.
-
The new step runs only in the
TRTLLM Snapshot Deploy Testjob ofpr.yamlandpost-merge-ci.yml. No other workflow runs this test. Both files parse, and the step is the same in both. -
In the rc27 image,
/usr/$LIB/libcuda.so.1resolves to/usr/lib/x86_64-linux-gnu/libcuda.so.1. The NVIDIA container toolkit mounts that file from the host driver. -
If that file is absent, the loader ignores the entry and finds
libcudaas it does without the preload. It prints this line:ERROR: ld.so: object '/usr/$LIB/libdoesnotexist.so.1' from LD_PRELOAD cannot be preloaded (cannot open shared object file): ignored. -
The host driver is the right one. The GPU nodes run driver 595.58.03, and the rc27 image carries the CUDA 13.4 compat driver
libcuda.so.615.65.02. Your probe shows that the helper and the process need the same driver. -
I did not run the deploy test, because it needs the GPU cluster. I read the job logs and the pod-log artifacts of the CI runs.
-
The rest of the PR did not change. My approval at
3104d46d29lists what I read and what I did not read, and that list still holds.
CI at this head.
Pre Merge run 36585253526 passed. The PR workflow, run 36585260018, has not finished.
f6d6c3e to
4b37faf
Compare
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Approving at 4b37faf948: the PR still meets the bar. The only change since my approval at f6d6c3e745 is this commit. It fixes both of my P3 items from that review, so I resolved those two threads.
Open item:
- P3: with this preload, a PTX load segfaults in a worker process that starts after
import tensorrt_llm.tests/deploy/test_dynamocheckpoint.py:192.
What changed, and CI at this head: the TRT-LLM checkpoint and the restore both finished.
The commit keeps the two image preloads in the worker LD_PRELOAD and adds the same comment at the three places. The rest of the PR is the same as at f6d6c3e745. My approval at 3104d46d29 lists what I read and what I did not read, and that list still holds.
In run 36596785970, TRTLLM Snapshot Deploy Test passed. The checkpoint finished, the restored worker pod has the nvidia.com/Restored condition with reason RestoreSucceeded, and inference on the restored worker passed. The same job passed at f6d6c3e745 in run 36585260018. Neither run changes the worker LD_LIBRARY_PATH, so the restore does not need the order that my last review left open. The new P3 asks for that order for another reason, the PTX JIT compiler. Other jobs in run 36596785970 have not finished.
Preload the host libcuda alias for the TRTLLM checkpoint test without pinning a driver version. Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
4b37faf to
3546a43
Compare
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Approving at 3546a4316e: the PR still meets the bar, and none of my findings is open. The new worker LD_LIBRARY_PATH fixes my P3 about the PTX JIT compiler, so I resolved that thread.
TRTLLM Snapshot Deploy Test did not run at this head. It is the CI job that checks checkpoint and restore with the new library path.
What changed, what I measured, and why the snapshot job did not run.
The only change since my approval at 4b37faf948 is the worker LD_LIBRARY_PATH in tests/deploy/test_dynamocheckpoint.py. The new commit replaces the old one on the same parent, 3104d46d29. The two heads differ only by the 11 lines added at tests/deploy/test_dynamocheckpoint.py:197-207. The change follows the ask in my P3, so this part of the review checks my own advice.
The new value is the image LD_LIBRARY_PATH with /usr/$LIB added after /opt/dynamo/mpi/lib. On a GPU host with the rc27 runtime image, a child process that loads PTX after import tensorrt_llm passes with the new value. It segfaults with the old value. The new order changes only which driver files load. My reply on the thread has the measurements.
In run 36610307797, three image builds stopped at the 1-hour job limit: frontend / Build multi-arch cpu, planner / Build multi-arch cpu and dynamo-runtime / image / Build multi-arch cuda13.0. The three Snapshot Deploy Tests need the frontend image, so GitHub skipped them. At 4b37faf948, the same three builds took 4 to 7 minutes, and TRTLLM Snapshot Deploy Test passed in run 36596785970.
My approval at 3104d46d29 lists what I read and what I did not read, and that list still holds.
Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
There was a problem hiding this comment.
Approving at cf24cb07cb: the PR still meets the bar, and none of my findings is open.
The new commit merges main. It differs from the automatic merge only in the two conflicts, .github/codeowners/areas.yaml and CODEOWNERS. Both take the main side from #15244, and the generated CODEOWNERS matches its source.
TRTLLM Snapshot Deploy Test has not finished at this head yet. It is part of PR run 36630453955, and it is the CI job that tests checkpoint and restore with the new library path.
The merge adds no hand edit outside the two conflicts. Apart from the - lpx line and its generated row, the PR lines did not change.
The merge drops the - lpx line that the PR added to the owner list of the CODEOWNERS rule. The union rule for aggregate lists does not apply to it. #15244 removed that list of every area, so the line has no list to join. The lpx area and its three shared rows are unchanged. On the merge, emit_codeowners.py writes a byte-identical CODEOWNERS, and build_codeowners.py --strict covers 6077 of 6077 files. Each gate fails on a planted edit.
Against the new base, the added and removed lines of the PR are the same as before in every file except these two. Three files merged without a conflict and have changes from both sides: shared_v1beta1.go and its envtest file from #14758, and .github/workflows/post-merge-ci.yml. The #14758 replica rule is inside the ComponentTypeEPP branch, and IsLPX() is true only for ComponentTypeLPX, so the two changes do not meet. The merged post-merge-ci.yml parses, and the host CUDA library step is still in the TRTLLM Snapshot Deploy Test job.
The DGD CRD is the same file as at 3546a4316e. It is 3,025,860 bytes, 96.19% of the 3 MiB request limit. As compact JSON, it is 1,413,575 bytes, 44.94%.
Pre Merge ran on a merge ref with the same tree as this head. There, make check found no uncommitted changes in the generated operator files. The internal/webhook/validation envtests ran with KUBEBUILDER_ASSETS set. The three image builds that stopped at the 1-hour limit at 3546a4316e passed at this head.
My approval at 3104d46d29 lists what I read and what I did not read, and that list still holds.
This reverts commit 3546a43. Restore default worker and Snapshot-agent CUDA library loading after the CI host driver upgrade. Signed-off-by: Andrew Paprotsky <apaprotskyi@nvidia.com>
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Approving at 5143224563: the PR still meets the bar, and none of my findings is open. The new commit is an exact revert of 3546a4316e, so the PR no longer changes the three files of that commit.
The commit message says:
Restore default worker and Snapshot-agent CUDA library loading after the CI host driver upgrade.
The GPU nodes of the CI snapshot cluster still report driver 595.58.03. For this reason, expect TRTLLM Snapshot Deploy Test to fail at this head, as it does on main. The test and the two workflows now match main at the merge base, so that failure is not a defect of this PR.
The revert is exact, and the GPU nodes of the snapshot cluster still report driver 595.58.03.
I applied the reverse of 3546a4316e to cf24cb07cb. The result is tree 5fac1f2723, which is the tree of this head. At this head, pr.yaml, post-merge-ci.yml and test_dynamocheckpoint.py are the same blobs as on main at the merge base 0ce693b3b0.
Nothing in the PR still uses what the revert removed. A search of the whole tree for the removed strings finds the same lines at this head as at the merge base. The strings include LD_PRELOAD, libcuda.so.1, /usr/$LIB, snapshot/issues/412 and the name of the removed step.
This head merges with main at 8c226c99bb without a conflict. The codeowners job ran on that merge and passed, with 6080 of 6080 files owned.
The job logs do not print the driver version. A failed checkpoint job saves the node list in its pod-logs-* artifact, as cluster-info/nodes.json. In both failed jobs below, all six GPU nodes have the label nvidia.com/cuda.driver-version.full=595.58.03. Five nodes have nvidia.com/gpu-driver-upgrade-state=upgrade-required, and one has upgrade-failed. In both jobs, the snapshot agent starts the checkpoint, finds the GPU, and then logs nothing until the test times out after 900 s.
| job | commit | contains 3546a4316e |
result |
|---|---|---|---|
| 109617704854 | main at 0ce693b3b0 |
no | failed at 2026-09-29 21:18Z |
| 109630183407 | PR at cf24cb07cb |
yes | passed at 2026-09-29 21:44Z, in 476.52 s |
| 109756396986 | main at 3f46e3d3c5 |
no | failed at 2026-09-30 05:57Z |
The last job started at 05:40Z, after the push of this commit at 05:31Z. TRTLLM Snapshot Deploy Test has not finished at this head yet. When that job fails, the required deploy-status-check also fails.
My approval at 3104d46d29 lists what I read and what I did not read, and that list still holds.
Summary
@ai-dynamo/dynamo-lpx-codeownersas a co-owner of the LPX controller and rendering directories, retaining Operator ownership. The LPX team must exist and have repository write access before GitHub can use it for reviews.Where should the reviewer start?
Related Issues
Validation
The following checks were performed during development:
git diff --checkKnown follow-ups
replicasis omitted, a whole-PCS replacement currently loses the externally managed engine count and falls back to one. Preserving that count is deferred to a follow-up.Summary by CodeRabbit
New Features
LPXGraphDeploymentKubernetes resource and LPX component support with agent and conductor roles.Documentation
Tests