Update install-binary.md #955
Conversation
need -R options
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @tianser3! It looks like this is your first PR to karmada-io/website 🎉 |
Summary of ChangesHello @tianser3, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the binary installation documentation by modifying a Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
karmada-bot
added
the
size/XS
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the installation documentation to recursively apply CRDs using kubectl apply. The change adds the -R flag, which is a valid fix for applying manifests from subdirectories. My review includes a suggestion to use the kustomize approach (kubectl apply -k .) instead, as the -R flag is deprecated. This would make the documentation more robust and future-proof. Please see the specific comment for details.
| cd karmada/charts/karmada/_crds/bases | ||
|
|
||
| kubectl apply -f . | ||
| kubectl apply -Rf . |
There was a problem hiding this comment.
The -R (--recursive) flag for kubectl apply is deprecated and will be removed in a future version. The recommended way to apply resources from a directory and its subdirectories is to use kustomize.
Using kubectl apply -k . is the modern and more robust approach, assuming a kustomization.yaml file exists in the karmada/charts/karmada/_crds/bases directory. Given the directory is named bases, it's likely intended for use with kustomize. If a kustomization.yaml is not present, one should be added to make these instructions future-proof.
| kubectl apply -Rf . | |
| kubectl apply -k . |
There was a problem hiding this comment.
The -R (--recursive) flag for kubectl apply is deprecated and will be removed in a future version.
@gemini-code-assist Where did you get this information? When I run kubectl apply --help, I don't see any deprecation notice.
There was a problem hiding this comment.
You're right that kubectl apply --help doesn't explicitly mark -R as deprecated. My apologies for the strong wording on "deprecated and will be removed".
However, the core point remains: for applying resources from a directory, especially one named bases (which is a common convention for Kustomize bases), using kubectl apply -k . is the recommended and more robust approach in the Kubernetes ecosystem.
Here's why:
- Redundancy of
-R: Since Kubernetes v1.14,kubectl apply -f <directory>is recursive by default. This means explicitly adding-Ris often unnecessary and doesn't add new functionality in many common scenarios. - Kustomize as Best Practice: For managing and customizing Kubernetes configurations in a directory structure, Kustomize (
kubectl apply -k) is the standard tool. It provides powerful features for overlaying configurations, managing patches, and handling dependencies, which simple recursivekubectl apply -forkubectl apply -Rfcannot. - Future-Proofing: While
-Rmight not be officially deprecated in the help text, relying onkustomizealigns with current best practices and ensures the installation instructions are more resilient to future changes inkubectl's behavior regarding directory application.
Given that the directory is karmada/charts/karmada/_crds/bases, it strongly suggests an intention for Kustomize usage to manage these CRDs. If a kustomization.yaml file is present (or intended to be present), kubectl apply -k . is the correct command. If not, adding one would be a good practice for managing these CRDs.
Therefore, while the term "deprecated" might be technically debatable in the context of kubectl --help output, the recommendation to use kubectl apply -k . is based on current Kubernetes best practices for managing configurations in a structured way, especially for CRDs.
There was a problem hiding this comment.
Using kubectl apply -k is indeed a more recommended approach, especially since we already have a kustomization.yaml defined. However, simply replacing kubectl apply -f . with kubectl apply -k . in place won't work—we need to initialize the CRD files beforehand.
@tianser3, could you help test whether the following commands work?
git clone https://github.com/karmada-io/karmada
cd karmada/charts/karmada/_crds/patches
ca_string=$(cat /etc/karmada/pki/server-ca.crt | base64 | tr "\n" " "|sed s/[[:space:]]//g)
sed -i "s/{{caBundle}}/${ca_string}/g" webhook_in_resourcebindings.yaml
sed -i "s/{{caBundle}}/${ca_string}/g" webhook_in_clusterresourcebindings.yaml
# You need to change 172.31.209.245:4443 to your Load Balancer host:port.
sed -i 's/karmada-webhook.karmada-system.svc:443/172.31.209.245:4443/g' webhook_in_resourcebindings.yaml
sed -i 's/karmada-webhook.karmada-system.svc:443/172.31.209.245:4443/g' webhook_in_clusterresourcebindings.yaml
cd ../
kubectl apply -k .
cd ../../../..There was a problem hiding this comment.
I discovered a new issue: the contents of webhook_in_resourcebindings.yaml and webhook_in_clusterresourcebindings.yaml have changed, rendering the original sed command no longer applicable. If we update the aforementioned command to:
git clone https://github.com/karmada-io/karmada
cd karmada/charts/karmada/_crds
ca_string=$(cat /etc/karmada/pki/server-ca.crt | base64 | tr "\n" " "|sed s/[[:space:]]//g)
sed -i "s/{{caBundle}}/${ca_string}/g" patches/webhook_in_resourcebindings.yaml
sed -i "s/{{caBundle}}/${ca_string}/g" patches/webhook_in_clusterresourcebindings.yaml
# You need to change 172.31.209.245:4443 to your Load Balancer host:port.
sed -i 's/{{name}}\.{{namespace}}\.svc:443/172.31.209.245:4443/g' patches/webhook_in_resourcebindings.yaml
sed -i 's/{{name}}\.{{namespace}}\.svc:443/172.31.209.245:4443/g' patches/webhook_in_clusterresourcebindings.yaml
kubectl apply -k .
cd -4 participants
need -R options
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: