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
46223 Add missing step to apply destination rules after running virtual services #13655
base: master
Are you sure you want to change the base?
Conversation
😊 Welcome! This is either your first contribution to the Istio documentation repo, or
Thanks for contributing! Courtesy of your friendly welcome wagon. |
|
Hi @bedla. 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/test-infra repository. |
@@ -59,6 +59,12 @@ Run the following command to apply virtual services that will route all traffic | |||
$ kubectl apply -f @samples/bookinfo/networking/virtual-service-all-v1.yaml@ | |||
{{< /text >}} | |||
|
|||
And to apply `DestinationRule` items run following command. |
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 isn't needed because the warning on line 46 tells the user to make sure it's already done.
If you haven't already, follow the instructions in [define the service versions](/docs/examples/bookinfo/#define-the-service-versions).
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.
ahh, I see. It is kind of hidden.
I executed command from section https://istio.io/latest/docs/setup/getting-started/#bookinfo
And did not clicked to link (because it looks like that it is not necessary) that goes to https://istio.io/latest/docs/examples/bookinfo/ ... let me update the text, to make it more explicit.
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 have force pushed changed, what do you think?
59a01ba
to
00e7d9c
Compare
@@ -110,7 +110,7 @@ Follow these steps to get started with Istio: | |||
|
|||
## Deploy the sample application {#bookinfo} | |||
|
|||
1. Deploy the [`Bookinfo` sample application](/docs/examples/bookinfo/): | |||
1. Follow steps in the page [`Bookinfo` sample application](/docs/examples/bookinfo/) to deploy sample application: |
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 don't think we want to change the getting started page to tell users to go to another document to do things. The getting started doc is supposed to be a simple, self contained, set of instructions to make it easy for users to start looking at Istio.
I see why the current request-routing instructions are a little confusing (hidden). My suggestion would be to remove the warning on line 46 and put the warning under each tab (Istio APIs
and Gateway API
) instead.
Then you can change the wording under the Istio APIs
tab to make it clear that it's asking you to create the destination rules:
If you haven’t already applied destination rules, follow the instructions in [define the service versions](/docs/examples/bookinfo/#define-the-service-versions).
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.
/ok-to-test
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 update the PR with Frank's suggestions.
(I definitely think the problem is on the request-routing
page, not the getting started page.)
Hi,
This is PR for #13664 ... see detials there.
Thx
Ivos