-
Notifications
You must be signed in to change notification settings - Fork 1.6k
docs: update GKE ambient guide to remove manual ResourceQuota step (Fix istio/istio#56376) #16660
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
base: master
Are you sure you want to change the base?
docs: update GKE ambient guide to remove manual ResourceQuota step (Fix istio/istio#56376) #16660
Conversation
istio/istio#56376) Signed-off-by: Parv Agarwal <[email protected]>
😊 Welcome! This is either your first contribution to the Istio documentation repo, or
Thanks for contributing! Courtesy of your friendly welcome wagon. |
Hi @parv18050212. Thanks for your PR. I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
⚠️ **For Istio versions earlier than 1.26:** | ||
You must manually create a ResourceQuota in the namespace (such as `istio-system`), for example: |
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.
Please use a shortcode for this kind of content.
(You won't be needing this after the changes so it's just a FYI)
content/en/docs/ambient/install/platform-prerequisites/index.md
Outdated
Show resolved
Hide resolved
content/en/docs/ambient/install/platform-prerequisites/index.md
Outdated
Show resolved
Hide resolved
content/en/docs/ambient/install/platform-prerequisites/index.md
Outdated
Show resolved
Hide resolved
Because this is now more an FYI for people who don't want to install in |
/ok-to-test |
Thanks for the suggestion, @craigbox! That makes sense. I’ll update the PR accordingly. |
Co-authored-by: Craig Box <[email protected]>
Co-authored-by: Craig Box <[email protected]>
Fixed the error - please re-order the sections and I can merge. |
I’ve now reordered the sections as suggested. The updated file is ready for review and merge. |
You seem to have lost all the edits that were made before you rearranged the content? |
Thanks for pointing that out! I've restored the earlier changes that were lost during the rearrangement. Let me know if anything else needs adjustment. |
@craigbox This is my first time contributing, and I’ve tried to follow all the guidelines, but I’m still getting a linting error I can’t figure out. Could you please help me understand what might be causing it? I'd really appreciate any guidance. Thanks! |
Means there are headers (which are #s) that share the same name. Headers have anchors applied to them, so they must be unique.
Ordered lists (numbered) must all be prefixed with 1. In markdown, these will be numbered correctly... ie: Bad
Good
|
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.
I think you have a formatter or something that is replacing a lot in this file that it shouldn't be.
|
||
## CNI plugins | ||
|
||
The following configurations apply to all platforms, when certain {{< gloss "CNI" >}}CNI plugins{{< /gloss >}} are used: | ||
The following configurations apply to all platforms, when certain CNI plugins are used: |
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.
Why is this being changed?
2. Cilium's BPF masquerading is currently disabled by default, and has issues with Istio's use of link-local IPs for Kubernetes health checking. Enabling BPF masquerading via `bpf.masquerade=true` is not currently supported, and results in non-functional pod health checks in Istio ambient. Cilium's default iptables masquerading implementation should continue to function correctly. | ||
3. Due to how Cilium manages node identity and internally allow-lists node-level health probes to pods, | ||
applying any default-DENY `NetworkPolicy` in a Cilium CNI install underlying Istio in ambient mode will cause `kubelet` health probes (which are by-default silently exempted from all policy enforcement by Cilium) to be blocked. This is because Istio uses a link-local SNAT address for kubelet health probes, which Cilium is not aware of, and Cilium does not have an option to exempt link-local addresses from policy enforcement. |
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.
This is where you're getting your ordered number linting failure. Not sure why these are being changed, however?
{{< text syntax=yaml >}} | ||
```yaml |
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.
Why?
Revert 7cb276f and I can tell you what is wrong with it at that stage. (If all you are doing is swapping two blocks, then there shouldn't be any net-new headings) |
Hi all, I’ve updated the Platform Prerequisites and Namespace Restrictions sections to improve clarity and correctness, particularly for the GKE part. Everything else is copied directly from the original file. |
This PR updates the Istio ambient mesh installation guide for GKE:
Fixes istio/istio#56376
Signed-off-by: Parv Agarwal [email protected]