Skip to content

Conversation

@yankay
Copy link

@yankay yankay commented Sep 23, 2025

Overview:

This PR addresses an issue where two different namespace names were used in the deployment documentation, causing confusion and potential execution errors during Kubernetes deployments.

Details:

The documentation contained inconsistent namespace references: dynamo-cloud and dynamo-kubernetes
Using the incorrect namespace (dynamo-cloud) would lead to execution failures when following the deployment guides

Summary by CodeRabbit

  • Documentation
    • Updated deployment and installation guides to use the Kubernetes namespace dynamo-kubernetes instead of dynamo-cloud.
    • Adjusted example commands (including kubectl apply paths) and environment setup to reflect the new NAMESPACE.
    • Aligned Path C (Custom Development) with Path A for consistent guidance.
    • Clarified steps for deploying an aggregated vLLM backend using the updated namespace, reducing setup confusion.

@copy-pr-bot
Copy link

copy-pr-bot bot commented Sep 23, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions
Copy link

👋 Hi yankay! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

@github-actions github-actions bot added docs external-contribution Pull request is from an external contributor labels Sep 23, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 23, 2025

Walkthrough

Documentation updates change the Kubernetes namespace in deployment examples from dynamo-cloud to dynamo-kubernetes across two guides, adjusting environment variable setup and kubectl command paths.

Changes

Cohort / File(s) Summary of Changes
Kubernetes namespace doc updates
docs/guides/dynamo_deploy/README.md, docs/guides/dynamo_deploy/installation_guide.md
Replaced namespace references from dynamo-cloud to dynamo-kubernetes in examples and environment setup; updated kubectl apply paths and ${NAMESPACE} usage accordingly.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Poem

In clusters where pods quietly brood,
I twitch my whiskers, set namespace to good.
From cloud to kubernetes, hop I go—
A tiny switch, commands now flow.
Ears up, tails high, configs align,
apply, get, logs—everything’s fine. 🐇✨

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "docs: Fix inconsistent namespace naming in deployment documentation" is concise, directly reflects the primary change in the changeset (normalizing namespace references from "dynamo-cloud" to "dynamo-kubernetes"), and matches the modified files in the summary. It is specific, relevant to reviewers, and free of noise or ambiguous wording. This makes it appropriate for commit history and quick scanning by teammates.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed The PR description includes clear Overview and Details that state the inconsistent namespace usage and the intended fix, so the core motivation and change are well explained. However, it omits the "Where should the reviewer start?" section and does not include a filled-in "Related Issues" entry (the template's placeholder "closes GitHub issue: #xxx" is not replaced), which the repository template expects. Because the essential information is present but template fields are missing, the description is mostly complete but not fully aligned with the repository template.

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.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/guides/dynamo_deploy/installation_guide.md (1)

101-125: Update chart paths in installation guide (docs/guides/dynamo_deploy/installation_guide.md:101-125)
The helm commands reference ./crds/ and ./platform/, but the charts are at deploy/cloud/helm/crds and deploy/cloud/helm/platform — update the docs to use ./deploy/cloud/helm/crds and ./deploy/cloud/helm/platform or add cd deploy/cloud/helm before running the helm commands.

🧹 Nitpick comments (2)
docs/guides/dynamo_deploy/installation_guide.md (2)

106-113: Optional: normalize namespace creation approach.
Path A uses --create-namespace; Path C uses kubectl create namespace. Either is fine—consider standardizing to one for consistency.

Also applies to: 118-125


93-101: Small UX nit: ensure RELEASE_VERSION is set in this block.
Path C relies on RELEASE_VERSION (used for IMAGE_TAG) set earlier. Add a reminder/export here to help users who jump directly to Path C.

 # 1. Set environment
+export RELEASE_VERSION=0.x.x  # any Dynamo 0.3.2+; if already set, skip
 export NAMESPACE=dynamo-kubernetes
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7b43b3a and e0fea57.

📒 Files selected for processing (2)
  • docs/guides/dynamo_deploy/README.md (1 hunks)
  • docs/guides/dynamo_deploy/installation_guide.md (1 hunks)
⏰ 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 - dynamo
🔇 Additional comments (2)
docs/guides/dynamo_deploy/installation_guide.md (1)

95-95: Namespace fix aligns Path C with Path A — good catch.
This removes a footgun where following Path C would deploy into a different namespace.

docs/guides/dynamo_deploy/README.md (1)

55-55: Namespace value standardized — looks good.
Keeps deployment examples consistent with the installation guide.

Signed-off-by: Kay Yan <kay.yan@daocloud.io>
@PeaBrane
Copy link
Contributor

@hhzhang16 could you please take a look at this when you get the chance?

@hhzhang16
Copy link
Contributor

@yankay please resolve the conflicts then it lgtm! (keep the optional line kubectl create namespace ${NAMESPACE} if the user doesn't yet have that namespace)

@yankay
Copy link
Author

yankay commented Sep 26, 2025

This issue has already been resolved by #3199.
So close this PR.

@yankay yankay closed this Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs external-contribution Pull request is from an external contributor size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants