-
Notifications
You must be signed in to change notification settings - Fork 748
docs: fix helm chart urls #2033
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Neal Vaidya <[email protected]>
WalkthroughThe documentation for installing Dynamo Cloud was updated to use new Helm chart repository paths and filenames, reflecting a change in the storage location and naming convention for the CRDs and platform charts. No installation logic or parameters were altered. Changes
Estimated code review effort1 (~2 minutes) Possibly related PRs
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/guides/dynamo_deploy/quickstart.md (3)
27-31: Usehelm pull, not the deprecatedhelm fetch, and quote the entire URL.
helm fetchwas deprecated in Helm v3.7 and removed in v3.12. Replacing it withhelm pullavoids future breakage.
Additionally, wrapping the full URL in quotes protects the$RELEASE_VERSIONexpansion from word-splitting and makes copy-pasting safer.-# Fetch the CRDs helm chart -helm fetch https://helm.ngc.nvidia.com/nvidia/ai-dynamo/charts/dynamo-crds-${RELEASE_VERSION}.tgz +# Fetch the CRDs helm chart +helm pull "https://helm.ngc.nvidia.com/nvidia/ai-dynamo/charts/dynamo-crds-${RELEASE_VERSION}.tgz" - -# Fetch the platform helm chart -helm fetch https://helm.ngc.nvidia.com/nvidia/ai-dynamo/charts/dynamo-platform-${RELEASE_VERSION}.tgz +# Fetch the platform helm chart +helm pull "https://helm.ngc.nvidia.com/nvidia/ai-dynamo/charts/dynamo-platform-${RELEASE_VERSION}.tgz"
39-43: Add an integrity check to fail fast if the chart download is corrupted.Immediately verifying the chart’s SHA256 after download (or using
--verifywith a provenance file, if published) prevents cryptic Helm errors later in the install step.Example:
echo "EXPECTED_SHA256 dynamo-crds-${RELEASE_VERSION}.tgz" | sha256sum -c -
50-51: Consider re-using the Helm repository instead of local.tgzfiles.Since you already added the
nvidiarepo, installing straight from the repo keeps commands shorter and avoids keeping the tarballs around:helm install dynamo-platform nvidia/ai-dynamo/dynamo-platform \ --version ${RELEASE_VERSION} \ --namespace ${NAMESPACE}Not mandatory, but it reduces manual steps and potential version drift between download and install.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/guides/dynamo_deploy/quickstart.md(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: julienmancuso
PR: ai-dynamo/dynamo#2012
File: deploy/cloud/helm/crds/templates/nvidia.com_dynamocomponentdeployments.yaml:92-98
Timestamp: 2025-07-18T16:04:31.743Z
Learning: CRD schemas in files like deploy/cloud/helm/crds/templates/*.yaml are auto-generated from Kubernetes library upgrades and should not be manually modified as changes would be overwritten during regeneration.
docs/guides/dynamo_deploy/quickstart.md (2)
Learnt from: julienmancuso
PR: #2012
File: deploy/cloud/helm/crds/templates/nvidia.com_dynamocomponentdeployments.yaml:92-98
Timestamp: 2025-07-18T16:04:31.743Z
Learning: CRD schemas in files like deploy/cloud/helm/crds/templates/*.yaml are auto-generated from Kubernetes library upgrades and should not be manually modified as changes would be overwritten during regeneration.
Learnt from: julienmancuso
PR: #2012
File: deploy/cloud/helm/crds/templates/nvidia.com_dynamocomponentdeployments.yaml:1178-1180
Timestamp: 2025-07-18T16:05:05.482Z
Learning: The stopSignal field under lifecycle in DynamoComponentDeployment CRDs is autogenerated due to Kubernetes library upgrades (k8s.io/api and k8s.io/apimachinery from v0.32.3 to v0.33.1), not a manual design decision by the user.
🪛 markdownlint-cli2 (0.17.2)
docs/guides/dynamo_deploy/quickstart.md
36-36: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
Summary by CodeRabbit