docs: flesh out vSphere provider overview, API reference, and CRDs#51
docs: flesh out vSphere provider overview, API reference, and CRDs#51jiazhiguang wants to merge 3 commits intomasterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds extensive VMware vSphere documentation and six new vSphere CRD manifests (VSphereCluster, VSphereMachine, VSphereMachineTemplate, VSphereMachineConfigPool, VSphereFailureDomain, VSphereDeploymentZone); marks provider Available; tightens a create-cluster parameter namespace constraint. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/en/overview/providers/vmware-vsphere.mdx (1)
45-49: Nit: link style is inconsistent within the same list.Lines 45, 47, and 48 use explicit
.mdxfile references, while lines 46 and 49 use trailing-slash directory links. For consistency and easier maintenance, consider normalizing to explicitindex.mdxreferences.♻️ Suggested normalization
- [Installation Guide](../../install/vmware-vsphere.mdx) -- [Creating Clusters](../../create-cluster/vmware-vsphere/) +- [Creating Clusters](../../create-cluster/vmware-vsphere/index.mdx) - [Managing Nodes](../../manage-nodes/vmware-vsphere.mdx) - [Upgrading Clusters](../../upgrade-cluster/vmware-vsphere.mdx) -- [Provider APIs](../../apis/providers/vmware-vsphere/) +- [Provider APIs](../../apis/providers/vmware-vsphere/index.mdx)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/overview/providers/vmware-vsphere.mdx` around lines 45 - 49, Normalize the inconsistent link styles in the VMwarе vSphere overview list by replacing directory-style links that end with a trailing slash (the entries pointing to ../../create-cluster/vmware-vsphere/ and ../../apis/providers/vmware-vsphere/) with explicit index.mdx file references (e.g., ../../create-cluster/vmware-vsphere/index.mdx and ../../apis/providers/vmware-vsphere/index.mdx) so all list entries consistently use explicit .mdx paths like the other items (Installation Guide, Managing Nodes, Upgrading Clusters).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@docs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspheremachineconfigpools.yaml`:
- Line 115: The regex for hostname validation in the CRD's pattern field
currently contains a double-backslash sequence
"(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*" which will be interpreted as a literal
backslash in YAML rather than an escaped dot; get explicit user approval before
changing generated code, then verify the intended upstream CAPV pattern and, if
upstream is wrong here, update the generated YAML to use a single-escaped dot
(e.g. "\.[a-z0-9]...") or properly quote/escape the pattern so the regex matches
DNS subdomains, and if this is an upstream generator bug open an issue/PR in the
CAPV repo instead of only patching locally.
---
Nitpick comments:
In `@docs/en/overview/providers/vmware-vsphere.mdx`:
- Around line 45-49: Normalize the inconsistent link styles in the VMwarе
vSphere overview list by replacing directory-style links that end with a
trailing slash (the entries pointing to ../../create-cluster/vmware-vsphere/ and
../../apis/providers/vmware-vsphere/) with explicit index.mdx file references
(e.g., ../../create-cluster/vmware-vsphere/index.mdx and
../../apis/providers/vmware-vsphere/index.mdx) so all list entries consistently
use explicit .mdx paths like the other items (Installation Guide, Managing
Nodes, Upgrading Clusters).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 759ad669-b8c3-4051-bfe2-f86c58cc0e1d
📒 Files selected for processing (17)
docs/en/apis/providers/vmware-vsphere/index.mdxdocs/en/apis/providers/vmware-vsphere/vspherecluster.mdxdocs/en/apis/providers/vmware-vsphere/vspheredeploymentzone.mdxdocs/en/apis/providers/vmware-vsphere/vspherefailuredomain.mdxdocs/en/apis/providers/vmware-vsphere/vspheremachine.mdxdocs/en/apis/providers/vmware-vsphere/vspheremachineconfigpool.mdxdocs/en/apis/providers/vmware-vsphere/vspheremachinetemplate.mdxdocs/en/create-cluster/vmware-vsphere/parameter-checklist.mdxdocs/en/overview/feature.mdxdocs/en/overview/providers/index.mdxdocs/en/overview/providers/vmware-vsphere.mdxdocs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vsphereclusters.yamldocs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspheredeploymentzones.yamldocs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yamldocs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspheremachineconfigpools.yamldocs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspheremachines.yamldocs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml
Deploying alauda-immutable-infra with
|
| Latest commit: |
a3b2c86
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://41aaf04a.alauda-immutable-infra.pages.dev |
| Branch Preview URL: | https://docs-flesh-out-vsphere-provi.alauda-immutable-infra.pages.dev |
jiazhiguang
force-pushed
the
docs/flesh-out-vsphere-provider
branch
from
7d11b5b to
2d7546b
Compare
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Currently only cpaas-system is supported for storing vSphere workload cluster objects; using another namespace may cause cluster creation to fail. The <namespace> placeholder is retained in the manifests. Also sync VSphereMachineConfigPool CRD with the upstream hostname regex escape fix from cluster-api-provider-vsphere commit 012a6e775. The previous pattern used `\\.` which YAML's plain scalar preserved as two literal backslashes, making the regex reject any multi-label hostname such as node1.example.com. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jiazhiguang
force-pushed
the
docs/flesh-out-vsphere-provider
branch
from
2d7546b to
e94b0da
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yaml (1)
366-367: Minor: description says “list” but schema is a keyed map.
networkConfigurationsis described as “a list of network configurations within this failure domain,” but the schema declaresx-kubernetes-list-type: mapwithx-kubernetes-list-map-keys: [networkName](Line 546-548), i.e. a keyed map-list. This wording comes from upstream, so only worth fixing if you’re happy to diverge from the generated source; otherwise ignore. No functional impact.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yaml` around lines 366 - 367, The description for networkConfigurations incorrectly calls it a "list" while the schema uses a keyed map; update the description for networkConfigurations to reflect that it's a map keyed by networkName (or "a keyed map of network configurations keyed by networkName") to match the schema that uses x-kubernetes-list-type: map and x-kubernetes-list-map-keys: [networkName].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@docs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspheredeploymentzones.yaml`:
- Around line 80-81: The CRD description for the conditions field is incorrect:
update the description string under the VSphereDeploymentZone schema (the
conditions property) so it references VSphereDeploymentZone instead of
VSphereMachine; search for the three occurrences of the conditions description
(the entries adjacent to the VSphereDeploymentZone schema/conditions property)
and replace "VSphereMachine" with "VSphereDeploymentZone" in each occurrence
(the same replacement should be applied at the two other matching descriptions
noted in the comment).
- Around line 323-337: Update the descriptions for the schema fields message and
reason to match the validation (minLength: 1) by clarifying that while the
fields may be omitted, they must not be empty strings when provided (e.g., "This
field may be omitted; if present it MUST be a non-empty string."). Locate the
message and reason entries in the VsphereDeploymentZones CRD and replace the
current "may be empty" wording with text that enforces the non-empty requirement
to align documentation with the minLength constraint.
In
`@docs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yaml`:
- Around line 456-460: Update the CRD definitions for useRoutes in both
dhcp4Overrides and dhcp6Overrides: change the schema type from "string" to
"boolean" for the useRoutes field (retain the existing description), ensuring
its type matches the sibling use* boolean flags (e.g., useDNS, useHostname) and
the intended true/false semantics; locate the useRoutes entries inside the
dhcp4Overrides and dhcp6Overrides object schemas and replace type: string with
type: boolean.
---
Nitpick comments:
In
`@docs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yaml`:
- Around line 366-367: The description for networkConfigurations incorrectly
calls it a "list" while the schema uses a keyed map; update the description for
networkConfigurations to reflect that it's a map keyed by networkName (or "a
keyed map of network configurations keyed by networkName") to match the schema
that uses x-kubernetes-list-type: map and x-kubernetes-list-map-keys:
[networkName].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fa99d4b3-11db-4775-bf64-f0ec7ea9c51c
📒 Files selected for processing (17)
docs/en/apis/providers/vmware-vsphere/index.mdxdocs/en/apis/providers/vmware-vsphere/vspherecluster.mdxdocs/en/apis/providers/vmware-vsphere/vspheredeploymentzone.mdxdocs/en/apis/providers/vmware-vsphere/vspherefailuredomain.mdxdocs/en/apis/providers/vmware-vsphere/vspheremachine.mdxdocs/en/apis/providers/vmware-vsphere/vspheremachineconfigpool.mdxdocs/en/apis/providers/vmware-vsphere/vspheremachinetemplate.mdxdocs/en/create-cluster/vmware-vsphere/parameter-checklist.mdxdocs/en/overview/feature.mdxdocs/en/overview/providers/index.mdxdocs/en/overview/providers/vmware-vsphere.mdxdocs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vsphereclusters.yamldocs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspheredeploymentzones.yamldocs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yamldocs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspheremachineconfigpools.yamldocs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspheremachines.yamldocs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml
✅ Files skipped from review due to trivial changes (11)
- docs/en/apis/providers/vmware-vsphere/vspheredeploymentzone.mdx
- docs/en/overview/feature.mdx
- docs/en/apis/providers/vmware-vsphere/vspheremachineconfigpool.mdx
- docs/en/apis/providers/vmware-vsphere/vspherecluster.mdx
- docs/en/overview/providers/index.mdx
- docs/en/apis/providers/vmware-vsphere/vspheremachinetemplate.mdx
- docs/en/apis/providers/vmware-vsphere/vspherefailuredomain.mdx
- docs/en/create-cluster/vmware-vsphere/parameter-checklist.mdx
- docs/en/apis/providers/vmware-vsphere/index.mdx
- docs/en/apis/providers/vmware-vsphere/vspheremachine.mdx
- docs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vspheremachineconfigpools.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/shared/crds/providers/vmware-vsphere/infrastructure.cluster.x-k8s.io_vsphereclusters.yaml
Sync CRD description fixes from cluster-api-provider-vsphere commit 7a5fa5119: - VSphereDeploymentZone status.conditions description erroneously referenced VSphereMachine across all three schema versions; now points to VSphereDeploymentZone. - v1beta1 Condition message/reason said "This field may be empty" while the kubebuilder markers enforce minLength: 1, so a literal "" is rejected. Reworded to "optional, but if set it must not be empty." Origin of the contradiction is upstream CAPI. - DHCPOverrides.UseRoutes is typed as *string and accepts true, false, or route (tests pass "route"), but its description implied a bool. Aligned with UseDomains wording across vspherefailuredomains, vspheremachines, and vspheremachinetemplates CRDs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Review Findings Report Scope
Repository Evidence Used
Read-only Result Latest After comparing PR #51 against the provider source, I still see four issues worth fixing before I would call the VMware docs source-faithful and HCS-level AI-usable. Findings
CodeRabbit Assessment
Non-Issues Or Confirmed Good Patterns
AI Usability Delta vs HCS
|
2 participants
Summary by CodeRabbit
cpaas-system.