-
Notifications
You must be signed in to change notification settings - Fork 55
fix (platform): add registration of host clusters #1340
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
✅ Deploy Preview for vcluster-docs-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Piotr1215
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.
@guowenatk this fits better in the Administer section:
Piotr1215
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.
Some more comments/suggestions.
|
This page https://deploy-preview-1340--vcluster-docs-site.netlify.app/docs/platform/next/administer/clusters/connect-cluster already contains plenty info about how to register the host cluster to the platform. I will merge the changes of this PR into that page. |
Piotr1215
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.
@guowenatk
Consolidating things into existing pages, could work, but we need to address the review comments I left on the original file. They still apply.
Quick things to fix:
-
Scope issue: This PR is mixing "connecting clusters" with "upgrading agents". DOC-429 is about connecting/adding clusters with Helm, not upgrading existing ones. The upgrade stuff already lives in
cluster-upgrades.mdx. -
Review comments: The inline comments I left aren't optional - we need them for consistency:
<Flow>and<Step>components (see other pages for examples)- Explain what a host cluster is upfront
- Call out required permissions + link to permissions page
- Proper
:::infoadmonitions - Link "vCluster Platform agent" to agent docs
-
Structure: Consolidating into
connect-cluster.mdxis fine, but formatting standards still apply.
Please go through my comments and implement them.
4dc388f to
2b3870c
Compare
|
@pascalbreuninger thank you for detailed review. @guowenatk let's resolve merge conflicts and push the version as is with Pascal's comments, we will iterate if needed. |
df14aec to
651901f
Compare
Content Description
Add documentation about registration of host clusters
Preview Link
https://deploy-preview-1340--vcluster-docs-site.netlify.app/docs/platform/next/administer/clusters/connect-cluster
Internal Reference
Closes DOC-429
@netlify /docs