-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
update istio-ingress installation name in helm install instructions #13516
Conversation
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.
lgtm for the zh part if the en change is correct.
FYI: #11762 (review) |
His comment is directed at the namespace rather than installation name. The installation name is inconsistent in this doc vs another doc with helm installation (ie, https://istio.io/latest/docs/setup/additional-setup/gateway/#deploying-a-gateway). This change aligns the general install doc with your gateway doc change PR here: #11734. |
@howardjohn ptal |
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 was intentional. Using the same names across samples, IMO, is actively harmful. users should understand what fields link things up and set them to match when appropriate.
We should not rely on a demo world where we happened to name everything the same. It ends with users thinking istio: ingressgateway
is some magic value, when really its a simple label selector. I have seen this countless times. I have never seen the same with Kubernetes Service -- but probably we would, if their docs always used hello: world
in every sample and treated it as a bug if it wasn't that.
@howardjohn However the current documentation varies from the default value we specify in the meshConfig for ingressService, which is I guess an alternative could be to update the documentation to clarify that the name in this step differs from meshConfig's default value and additional steps are required. I'm trying to approach this as someone who knows Kubernetes and Helm, but doesn't know Istio. I would follow the guide to get a learning environment setup and play in, and nothing seems to indicate that this wouldn't work "out of the box" until they go to test the environment and see the ingress not working. |
Please provide a description for what this PR is for.
Mesh Config has a default of looking for
istio-ingressgateway
, which is the name elsewhere and by other methods for installation. This PR changes the installation instructions to match the name used elsewhere. For example: https://istio.io/latest/docs/setup/additional-setup/gateway/#deploying-a-gateway, https://istio.io/latest/docs/setup/install/istioctl/#check-whats-installed.Fixes #13497
And to help us figure out who should review this PR, please
put an X in all the areas that this PR affects.