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 (2)
✅ Files skipped from review due to trivial changes (1)
WalkthroughDocumentation and CRD schema updates for the Huawei Cloud Stack provider: tightened validation (required identity name, hostname pattern, non-empty configs/networks), clarified subnet fields and manifest guidance (including deprecated misspelling), prohibited runtime identity fields in templates, and expanded kubeadm/kubelet/bootstrap examples and notes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/en/create-cluster/huawei-cloud-stack.mdx (1)
321-335: Minor inconsistency:protect-kernel-defaultsin initConfiguration but not joinConfiguration.The
initConfiguration.nodeRegistration.kubeletExtraArgsincludesprotect-kernel-defaults: "true"(line 327), butjoinConfiguration.nodeRegistration.kubeletExtraArgs(lines 333-335) does not include this flag.While the kubelet patch file at line 257 sets
protectKernelDefaults: trueviaKubeletConfiguration(which takes precedence), having the CLI arg in only one place may cause confusion.Consider adding it to
joinConfigurationfor consistency, or remove it frominitConfigurationsince the patch file handles it:Option: Add to joinConfiguration for consistency
nodeRegistration: kubeletExtraArgs: node-labels: "kube-ovn/role=master" + protect-kernel-defaults: "true" volume-plugin-dir: "/opt/libexec/kubernetes/kubelet-plugins/volume/exec/"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/create-cluster/huawei-cloud-stack.mdx` around lines 321 - 335, The init/join kubelet CLI args are inconsistent: initConfiguration.nodeRegistration.kubeletExtraArgs includes protect-kernel-defaults but joinConfiguration.nodeRegistration.kubeletExtraArgs does not; update the manifest so both places are consistent by either adding protect-kernel-defaults: "true" to joinConfiguration.nodeRegistration.kubeletExtraArgs or removing it from initConfiguration.nodeRegistration.kubeletExtraArgs (since the KubeletConfiguration patch uses protectKernelDefaults: true). Locate the keys initConfiguration, joinConfiguration, nodeRegistration.kubeletExtraArgs and the protect-kernel-defaults entry and make the chosen change so both initConfiguration and joinConfiguration reflect the same kubelet CLI args.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/en/create-cluster/huawei-cloud-stack.mdx`:
- Around line 321-335: The init/join kubelet CLI args are inconsistent:
initConfiguration.nodeRegistration.kubeletExtraArgs includes
protect-kernel-defaults but joinConfiguration.nodeRegistration.kubeletExtraArgs
does not; update the manifest so both places are consistent by either adding
protect-kernel-defaults: "true" to
joinConfiguration.nodeRegistration.kubeletExtraArgs or removing it from
initConfiguration.nodeRegistration.kubeletExtraArgs (since the
KubeletConfiguration patch uses protectKernelDefaults: true). Locate the keys
initConfiguration, joinConfiguration, nodeRegistration.kubeletExtraArgs and the
protect-kernel-defaults entry and make the chosen change so both
initConfiguration and joinConfiguration reflect the same kubelet CLI args.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19835da1-4f11-42cf-b3ca-0839089eacf4
📒 Files selected for processing (6)
docs/en/create-cluster/huawei-cloud-stack.mdxdocs/en/manage-nodes/huawei-cloud-stack.mdxdocs/en/overview/providers/huawei-cloud-stack.mdxdocs/en/upgrade-cluster/huawei-cloud-stack.mdxdocs/shared/crds/providers/huawei-cloud-stack/infrastructure.cluster.x-k8s.io_hcsclusters.yamldocs/shared/crds/providers/huawei-cloud-stack/infrastructure.cluster.x-k8s.io_hcsmachineconfigpools.yaml
Deploying alauda-immutable-infra with
|
| Latest commit: |
2e5dc2a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://58ec499f.alauda-immutable-infra.pages.dev |
| Branch Preview URL: | https://ait-68208.alauda-immutable-infra.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/en/manage-nodes/huawei-cloud-stack.mdx`:
- Line 47: Update the guidance to consistently describe the three subnet fields
(deprecated `subenetName`, current `subnetName`, and `subnetId`) and the allowed
combinations: state that for existing manifests that use the deprecated
`subenetName` you may optionally include `subnetName` only if its value exactly
matches `subenetName`; for new manifests use either `subnetName` or `subnetId`
but not both; and warn not to supply conflicting values across these fields.
Also add a short note that the mutual-exclusivity is enforced by the
provider/webhook and not by the CRD schema (the CRD lists all three as
optional), so users should follow the provider-level constraint.
🪄 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: 5df3f749-01e2-41ee-8b4b-7bc1966ebe56
📒 Files selected for processing (3)
docs/en/manage-nodes/huawei-cloud-stack.mdxdocs/en/overview/providers/huawei-cloud-stack.mdxdocs/en/upgrade-cluster/huawei-cloud-stack.mdx
✅ Files skipped from review due to trivial changes (2)
- docs/en/overview/providers/huawei-cloud-stack.mdx
- docs/en/upgrade-cluster/huawei-cloud-stack.mdx
| The pool must include at least as many entries as the number of worker nodes you plan to deploy. Insufficient entries will prevent node deployment. | ||
| ::: | ||
|
|
||
| Use `subnetName` in new manifests. The provider still accepts the deprecated `subenetName` field for existing manifests, but do not use both fields with different values. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify subnet guidance in docs vs CRD schema constraints (read-only).
rg -n -C2 'subenetName|subnetName|subnetId|Set either|required:|oneOf|allOf' \
docs/en/manage-nodes/huawei-cloud-stack.mdx \
docs/shared/crds/providers/huawei-cloud-stack/infrastructure.cluster.x-k8s.io_hcsmachineconfigpools.yamlRepository: alauda/immutable-infra-docs
Length of output: 7293
Clarify subnet field guidance to address conflicting expectations.
The documentation presents conflicting guidance on subnet field usage: Line 47 allows both subenetName and subnetName when values match, while Line 80 states "Set either subnetName or subnetId." Additionally, Line 47 doesn't address subnetId, and Line 80 omits the deprecated field entirely. Clarify that users choosing between three fields (deprecated subenetName, subnetName, and subnetId) must:
- For existing manifests with
subenetName: optionally pair withsubnetNameonly if values match - For new manifests: use
subnetNameorsubnetId, but not both
Also specify that this "either/or" constraint is enforced at the provider/webhook level (the CRD schema shows all three fields as optional with no schema-level mutual-exclusivity constraint).
🧰 Tools
🪛 LanguageTool
[grammar] ~47-~47: Ensure spelling is correct
Context: ...e provider still accepts the deprecated subenetName field for existing manifests, but do no...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/en/manage-nodes/huawei-cloud-stack.mdx` at line 47, Update the guidance
to consistently describe the three subnet fields (deprecated `subenetName`,
current `subnetName`, and `subnetId`) and the allowed combinations: state that
for existing manifests that use the deprecated `subenetName` you may optionally
include `subnetName` only if its value exactly matches `subenetName`; for new
manifests use either `subnetName` or `subnetId` but not both; and warn not to
supply conflicting values across these fields. Also add a short note that the
mutual-exclusivity is enforced by the provider/webhook and not by the CRD schema
(the CRD lists all three as optional), so users should follow the provider-level
constraint.
What changed
fix/subnetname-compatibilityprovider branch forHCSClusterandHCSMachineConfigPoolproviderIDandserverId/etc/kubernetes/admission/psa-config.yamlis a completePodSecurityConfigurationWhy
The product docs were lagging behind the provider behavior in
fix/subnetname-compatibility, especially aroundHCSMachineConfigPoolvalidation and the control plane bootstrap configuration. The PSA example was also incomplete, which made the published YAML unsafe to copy directly.Impact
Users and AI coding agents can rely on the HCS docs to generate cluster and worker manifests that match the intended provider behavior. The API reference now exposes the current CRD constraints for the affected HCS resources.
Root cause
The docs and shared CRD assets had not been fully synchronized with the provider branch that introduced subnet field compatibility, stricter validation, and additional bootstrap security configuration.
Validation
/Users/changjia/alauda/cluster-api-provider-hcsonorigin/fix/subnetname-compatibilityyarn build; the user will validate withyarn devSummary by CodeRabbit