feat: add crds for vllm and llm examples #1766
Conversation
Signed-off-by: mohammedabdulwahhab <furkhan324@berkeley.edu>
…m/ai-dynamo/dynamo into mabdulwahhab/add-crds-to-examples
…add-crds-to-examples
…m/ai-dynamo/dynamo into mabdulwahhab/add-crds-to-examples
hhzhang16
requested review from
GuanLuo,
PeaBrane,
alec-flowers,
grahamking and
tedzhouhk
as code owners
hhzhang16
requested review from
ptarasiewiczNV,
rmccorm4 and
tanmayv25
as code owners
hhzhang16
changed the title
WalkthroughSeveral new Kubernetes deployment YAML files are introduced, each defining a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Processor
participant Router
participant VllmWorker
participant PrefillWorker
participant Planner
User->>Frontend: Send LLM inference request
Frontend->>Processor: Forward request (if present)
Processor->>Router: Route request (if present)
Router->>VllmWorker: Dispatch for LLM computation
VllmWorker->>PrefillWorker: Delegate prefill (if present)
VllmWorker->>Frontend: Return result
Frontend->>User: Respond with LLM output
Frontend->>Planner: (Optional) Send metrics/feedback
Planner->>Frontend: (Optional) Send new plan/instructions
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (11)
examples/llm/deploy/disagg_router.yaml (2)
6-9: JSON-in-string is hard to maintain – move to a ConfigMap or|-blockThe colossal, doubly-escaped JSON makes diffs & reviews painful and is error-prone (a stray quote will break the pod). Consider:
- Storing the JSON in a ConfigMap and
valueFrom.configMapKeyRef, or- Using a YAML multiline scalar (
|-) to avoid the back-slash soup.
100-111: Pin container image by digest for immutability
nvcr.io/nvidia/ai-dynamo/vllm-runtime:0.3.1can be retagged upstream, causing silent drift. Pin to a digest (@sha256:…) to guarantee bit-for-bit reproducibility.examples/llm/deploy/agg_router.yaml (2)
6-9: Escape-heavy JSON string – favour ConfigMap or multiline scalarAll earlier rationale applies.
100-111: Pin image by digestexamples/vllm_v0/deploy/disagg.yaml (2)
6-9: Consider ConfigMap / multiline scalar forDYN_DEPLOYMENT_CONFIG
50-60: Pin runtime image by digestAlso applies to: 76-88
examples/llm/deploy/disagg.yaml (2)
6-9: Large escaped JSON – move to ConfigMap / multiline scalar
75-86: Pin image digestAlso applies to: 101-113
examples/vllm_v0/deploy/agg.yaml (2)
6-9: Prefer ConfigMap / multiline scalar for deployment config JSON
50-61: Pin container image digestexamples/llm/deploy/agg.yaml (1)
60-87: Consider declaringcomponentType: workerfor VllmWorker
Processoris explicitly marked as a worker, butVllmWorkeris not. For clarity and label consistency add it unless there is a deliberate reason to rely on defaults.VllmWorker: envFromSecret: hf-token-secret dynamoNamespace: llm-agg + componentType: worker
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/llm/deploy/agg.yaml(1 hunks)examples/llm/deploy/agg_router.yaml(1 hunks)examples/llm/deploy/disagg.yaml(1 hunks)examples/llm/deploy/disagg_router.yaml(1 hunks)examples/vllm_v0/deploy/agg.yaml(1 hunks)examples/vllm_v0/deploy/disagg.yaml(1 hunks)examples/vllm_v0/deploy/disagg_planner.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
examples/vllm_v0/deploy/disagg_planner.yaml (1)
Learnt from: julienmancuso
PR: ai-dynamo/dynamo#1365
File: deploy/cloud/operator/api/v1alpha1/dynamocomponentdeployment_types.go:171-178
Timestamp: 2025-06-04T13:09:53.416Z
Learning: The `DYN_DEPLOYMENT_CONFIG` environment variable (commonconsts.DynamoDeploymentConfigEnvVar) in the Dynamo operator will never be set via ValueFrom (secrets/config maps), only via direct Value assignment. The GetDynamoDeploymentConfig method correctly only checks env.Value for this specific environment variable.
examples/vllm_v0/deploy/disagg.yaml (1)
Learnt from: julienmancuso
PR: ai-dynamo/dynamo#1365
File: deploy/cloud/operator/api/v1alpha1/dynamocomponentdeployment_types.go:171-178
Timestamp: 2025-06-04T13:09:53.416Z
Learning: The `DYN_DEPLOYMENT_CONFIG` environment variable (commonconsts.DynamoDeploymentConfigEnvVar) in the Dynamo operator will never be set via ValueFrom (secrets/config maps), only via direct Value assignment. The GetDynamoDeploymentConfig method correctly only checks env.Value for this specific environment variable.
examples/llm/deploy/agg.yaml (3)
Learnt from: nnshah1
PR: ai-dynamo/dynamo#1444
File: tests/fault_tolerance/scenarios.py:57-57
Timestamp: 2025-07-01T15:39:56.789Z
Learning: The fault tolerance tests in tests/fault_tolerance/ are designed to run only in the mounted container environment, so hardcoded absolute paths with `/workspace/` prefix are intentional and should not be changed to relative paths.
Learnt from: julienmancuso
PR: ai-dynamo/dynamo#1365
File: deploy/cloud/operator/api/v1alpha1/dynamocomponentdeployment_types.go:171-178
Timestamp: 2025-06-04T13:09:53.416Z
Learning: The `DYN_DEPLOYMENT_CONFIG` environment variable (commonconsts.DynamoDeploymentConfigEnvVar) in the Dynamo operator will never be set via ValueFrom (secrets/config maps), only via direct Value assignment. The GetDynamoDeploymentConfig method correctly only checks env.Value for this specific environment variable.
Learnt from: biswapanda
PR: ai-dynamo/dynamo#1266
File: deploy/cloud/operator/internal/controller/dynamocomponentdeployment_controller.go:85-85
Timestamp: 2025-05-29T16:29:45.152Z
Learning: In the Dynamo codebase, ComponentTypePlanner constants with different cases ("Planner" vs "planner") are intentional and serve different purposes: component type in config vs component label. These should not be made consistent as they handle different contexts.
🪛 GitHub Actions: Copyright Checks
examples/vllm_v0/deploy/agg.yaml
[error] 1-1: Invalid or missing copyright header detected. Please add the required SPDX copyright and license header.
examples/llm/deploy/disagg_router.yaml
[error] 1-1: Invalid or missing copyright header detected. Please add the required SPDX copyright and license header.
examples/vllm_v0/deploy/disagg_planner.yaml
[error] 1-1: Invalid or missing copyright header detected. Please add the required SPDX copyright and license header.
examples/llm/deploy/agg_router.yaml
[error] 1-1: Invalid or missing copyright header detected. Please add the required SPDX copyright and license header.
examples/vllm_v0/deploy/disagg.yaml
[error] 1-1: Invalid or missing copyright header detected. Please add the required SPDX copyright and license header.
examples/llm/deploy/disagg.yaml
[error] 1-1: Invalid or missing copyright header detected. Please add the required SPDX copyright and license header.
examples/llm/deploy/agg.yaml
[error] 1-1: Invalid or missing copyright header detected. Please add the required SPDX copyright and license header.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Test - vllm
🔇 Additional comments (6)
examples/llm/deploy/disagg_router.yaml (1)
86-88: Verify presence ofhf-token-secretin target namespaceThe pods will crash-loop if the secret is missing or mis-named. Ensure the secret exists in
llm-disagg-router.Also applies to: 113-115
examples/llm/deploy/agg_router.yaml (1)
86-88: Ensurehf-token-secretexists inllm-agg-routernamespaceexamples/vllm_v0/deploy/disagg.yaml (1)
37-38: Verifyhf-token-secretavailability invllm-v0-disaggAlso applies to: 64-65
examples/llm/deploy/disagg.yaml (1)
61-62: Confirmhf-token-secretexists inllm-disaggnamespaceAlso applies to: 88-89
examples/vllm_v0/deploy/agg.yaml (1)
36-37: Check presence ofhf-token-secretinvllm-v0-aggnamespaceexamples/vllm_v0/deploy/disagg_planner.yaml (1)
132-142: Prometheus pod is launching LLM code, not PrometheusRunning
graphs.disagg_planner:Frontendinside the Prometheus pod is almost certainly unintentional.
Verify whether you really want Prometheus, and if so switch to the proper image/args (prom/prometheusetc.) or a dedicated Dynamo graph component if one exists.
hhzhang16
changed the title
Signed-off-by: mohammedabdulwahhab <furkhan324@berkeley.edu> Co-authored-by: Hannah Zhang <hannahz@nvidia.com> Co-authored-by: hhzhang16 <54051230+hhzhang16@users.noreply.github.com>
Overview:
Add deployment CRDs for LLM and vllm_v0 examples, as those are supported in nvcr.io/nvidia/ai-dynamo/vllm-runtime:0.3.1. Also added CRDS for vllm_v1 to prepare for 0.3.2 runtime release
Details:
Where should the reviewer start?
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit