-
Notifications
You must be signed in to change notification settings - Fork 692
fix: fix broken link in docs #2870
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: Julien Mancuso <jmancuso@nvidia.com>
WalkthroughAdds a new FluxCD GitOps guide, links it from the Dynamo deploy README, and streamlines the operator doc by updating links and removing in-doc GitOps installation details, pointing to centralized guides and API reference. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Git as Git Repository
participant Flux as FluxCD Controller
participant K8s as Kubernetes API
participant Oper as Dynamo Operator
participant Cluster as Pods/Services
Dev->>Git: Commit DynamoGraphDeployment CR
Note right of Git: Versioned CR manifests
Flux->>Git: Poll/Watch for changes
Flux->>K8s: Apply CR (reconcile)
K8s-->>Flux: CR accepted
K8s->>Oper: Emit reconcile event
Oper->>K8s: Create PVCs, Deployments, Services
Oper->>Cluster: Orchestrate build/deploy steps
Note over Oper,Cluster: Operator drives graph deployment
Dev->>K8s: kubectl get DynamoGraphDeployment status
K8s-->>Dev: Status: Ready/Progressing/Error
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, 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: 2
🧹 Nitpick comments (3)
docs/guides/dynamo_deploy/dynamo_operator.md (1)
26-31: LGTM: fixed links to API reference and installation guide.Both absolute paths look correct and improve portability. Minor nit: consider removing the bold emoji to keep heading style consistent across docs.
-**📖 [Dynamo CRD API Reference](/docs/guides/dynamo_deploy/api_reference.md)** +**[Dynamo CRD API Reference](/docs/guides/dynamo_deploy/api_reference.md)**docs/guides/dynamo_deploy/fluxcd.md (2)
21-22: Tighten Step 1 wording.Grammar/clarity issue and the heading says “Build and Push,” but the text points to installation docs.
-First, follow to [See Install Dynamo Cloud](/docs/guides/dynamo_deploy/installation_guide.md). +First, install Dynamo Cloud by following the guide: [Installation Guide](/docs/guides/dynamo_deploy/installation_guide.md).If building and pushing the operator image is required, add explicit commands or link to the operator build section.
63-66: Use “CR” instead of “CRD.”We update the Custom Resource, not the definition.
-To update your pipeline, just update the associated DynamoGraphDeployment CRD +To update your pipeline, update the associated DynamoGraphDeployment CR (custom resource)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
docs/guides/dynamo_deploy/README.md(1 hunks)docs/guides/dynamo_deploy/dynamo_operator.md(1 hunks)docs/guides/dynamo_deploy/fluxcd.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: julienmancuso
PR: ai-dynamo/dynamo#1474
File: deploy/cloud/operator/internal/controller/dynamocomponent_controller.go:1308-1312
Timestamp: 2025-06-11T21:29:28.650Z
Learning: User julienmancuso expects replies in English; avoid switching languages unless explicitly requested.
🪛 LanguageTool
docs/guides/dynamo_deploy/fluxcd.md
[grammar] ~7-~7: There might be a mistake here.
Context: ..._deploy/installation_guide.md) installed - [FluxCD](https://fluxcd.io/flux/installat...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...installation/) installed in your cluster - A Git repository to store your deploymen...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...ment in your cluster. The operator will: - Create the specified PVCs - Build contai...
(QB_NEW_EN)
docs/guides/dynamo_deploy/dynamo_operator.md
[grammar] ~26-~26: There might be a mistake here.
Context: ...ns, see: 📖 Dynamo CRD API Reference ## Installation [See installation steps](/...
(QB_NEW_EN)
⏰ 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 (3)
docs/guides/dynamo_deploy/README.md (1)
170-171: LGTM: new FluxCD guide link resolves the broken reference.Absolute path is correct and matches the new file location.
docs/guides/dynamo_deploy/fluxcd.md (2)
28-54: Validate CR schema fields (envs,pvc).Example uses
envsand a top-levelpvcblock under a service. Other docs in this repo showenvFromSecretandextraPodSpecpatterns. Risk of users copying an invalid spec.Would you confirm these keys against the API reference and adjust the example accordingly? If needed, I can rewrite this snippet to match the published CRD.
73-77: No change needed forkubectl get dynamographdeployment: singular form is registered and valid. The CRD definessingular: dynamographdeployment, so the existing lowercase singular command works. You may optionally addkubectl describe dynamographdeployment llm-agg -n "$NAMESPACE"for more detailed status.
Likely an incorrect or invalid review comment.
biswapanda
left a comment
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.
Lgtm.
can you take a look at code rabbit's comments -
qualify the operator for less confusion FluxCD Operator or Dynamo Operator
Build container images for all components
Signed-off-by: Julien Mancuso <jmancuso@nvidia.com>
Signed-off-by: Julien Mancuso <jmancuso@nvidia.com> Co-authored-by: Dmitry Tokarev <dtokarev@nvidia.com>
Signed-off-by: Julien Mancuso <jmancuso@nvidia.com> Co-authored-by: Dmitry Tokarev <dtokarev@nvidia.com> Signed-off-by: nnshah1 <neelays@nvidia.com>
Signed-off-by: Julien Mancuso <jmancuso@nvidia.com> Co-authored-by: Dmitry Tokarev <dtokarev@nvidia.com>
Overview:
fix broken link in docs
Summary by CodeRabbit