Updated traffic management concept guide by LisaFC · Pull Request #5014 · istio/istio.io

LisaFC · 2019-09-18T15:25:00Z

Updated traffic management concept guide to make it more feature-focused, per my proposal:
https://docs.google.com/document/d/1PnT1P6dHjYqkZv-cx1qg2JH4x8dVdPsRv1IvYx6wFlw/edit

[ ] Configuration Infrastructure
[X] Docs
[ ] Installation
[X] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

istio-testing · 2019-09-18T15:25:15Z

Hi @LisaFC. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

geeknoid · 2019-09-18T15:35:43Z

/ok-to-test

content/en/docs/concepts/traffic-management/index.md

howardjohn

Looks great, left a few nitpick comments

content/en/docs/concepts/traffic-management/index.md

Co-Authored-By: John Howard <howardjohn@google.com>

content/en/docs/concepts/traffic-management/index.md

Co-Authored-By: Frank Budinsky <frankb@ca.ibm.com>

LisaFC · 2019-09-19T14:39:56Z

/retest

Implementing Frank's suggestions

Co-Authored-By: Frank Budinsky <frankb@ca.ibm.com>

sdake · 2019-09-20T15:16:43Z

@LisaFC - linting errors:

Total in 13807 ms
    ./en/docs/concepts/traffic-management/index.md
      326 | [HTTPMatchRequest reference](/docs/reference/co 
      356 | [HTTPRoute reference](/docs/reference/co 

>> 2 spelling errors found in 466 files
To learn how to address spelling errors, please see https://istio.io/about/contribute/creating-and-editing-pages/#linting
Running ["OpenGraphCheck", "ScriptCheck", "HtmlCheck", "LinkCheck", "ImageCheck"] on ["./public"] on *.html...

To fix these, place HTTPMatchRequest and HTTPRoute in backticks. Its annoying that our infrastructure requires backticks for links.

content/en/docs/concepts/traffic-management/index.md

sdake

@LisaFC - thanks for the stellar contribution! I left a bunch of semi-nitpicky comments. I am not finished with the review. I have a meeting now and will conclude my review once my meetings clear up for the day.

sdake · 2019-09-20T15:17:54Z

content/en/blog/2019/evolving-istios-apis/index.md

 All service meshes and, by extension, Istio seeks to automate complex infrastructure operations, like networking and security. That means there will always be complexity in its APIs, but Istio will always aim to solve the needs of operators, while continuing to evolve the API to provide robust building blocks and prioritize flexibility through role-centric abstractions.

-We can't wait for you to join our [community](/about/community/join/) to see what you build with Istio next!
+We can't wait for you to join our [community](/about/community/join/) to see what you build with Istio next!


Please revert this change. At some point we may make the blogs autodate based upon git timestamps - and the timestamp would be incorrect.

Ah, sorry, I think this got sucked in from an old copy. Will fix.

content/en/docs/concepts/traffic-management/index.md

sdake · 2019-09-20T15:24:30Z

content/en/docs/concepts/traffic-management/index.md

+[observability](/docs/concepts/observability/) features, helps make your application
+more robust against failures of dependent services or the network.
+
+Istio’s traffic management model relies on the {{< gloss >}}Envoy{{</ gloss >}}


along with your services is a little vague, but I'm not sure how to add enough detail without blowing people up with too much detail.

"as sidecars to your services"?

sdake · 2019-09-20T15:30:41Z

content/en/docs/concepts/traffic-management/index.md

+
+Istio’s traffic management model relies on the {{< gloss >}}Envoy{{</ gloss >}}
+proxies that are deployed along with your services. All traffic that your mesh
+services send and receive ({{< gloss >}}data plane{{</ gloss >}} traffic) is proxied through Envoy, making


parenthesis in the docs is generally discouraged. Perhaps a better approach would be to rework this sentence a bit such as All data-plane traffic within your service mesh is proxied through Envoy. This proxy model makes it easy for you to direct and control traffic within your mesh without making any changes to your services.

this simplifies the sentence a bit and makes it less complex to understand.

Will what "data plane traffic" means be obvious to all readers? The main reason for the parenthesized bit is to convey "this traffic is also referred to as data plane traffic" (though I have just added data plane to the glossary, so that might mitigate)

sdake · 2019-09-20T15:33:04Z

content/en/docs/concepts/traffic-management/index.md

+
+If you’re interested in the details of how the features described in this guide
+work, you can find out more about Istio’s traffic management architecture in the
+[Architecture](#architecture) section at the end of this document. The rest of


This wording is a little clunky for a link. How about something like "If you are interested in the details of traffic management, please reference the architecture(link).

I think that loses a little of the meaning - it's less the details of traffic management, more the details of how traffic management works (ie the implementation part). How about:

If you're interested in the details of how Istio's traffic management works, you can find out more in the Architecture section.

Don't lose the emphasis in the sentence that this guide is focused on "features", not architecture.

How about:

If you're interested in the details of how the features described in this guide work, you can find out more in....

Yes, remove the "about Istio’s traffic management architecture" part. That works.

content/en/docs/concepts/traffic-management/index.md

sdake · 2019-09-20T15:57:02Z

content/en/docs/concepts/traffic-management/index.md

+Istio service registry, they are simply virtual destinations. This lets you model
+traffic for virtual hosts that don't have routable entries inside the mesh.
+
+#### Routing rules {#routing-rules}


note to self - review from here forward.

frankbu · 2019-09-20T17:06:26Z

@LisaFC, @sdake: I think we're getting far too picky with grammatical details now. Unless somebody spots any more technical inaccuracies, I would like to merge this now. We can always improve it further in future iterations.

I'll approve/merge this today if there are no serious objections.

sdake · 2019-09-20T17:17:24Z

@frankbu understood. I didn't intend to be difficult, I just wanted the best content possible.

@LisaFC apologies if I came across too strong :)

Cheers
-steve

frankbu

This looks good to merge now. Further improvements can be made in followup PRs.

frankbu · 2019-09-20T20:58:19Z

/cherrypick release-1.3

istio-testing · 2019-09-20T20:58:28Z

@frankbu: #5014 failed to apply on top of branch "release-1.3":

Applying: Updated traffic management guide
error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	content/en/blog/2019/evolving-istios-apis/index.md
Falling back to patching base and 3-way merge...
Auto-merging content/en/blog/2019/evolving-istios-apis/index.md
CONFLICT (content): Merge conflict in content/en/blog/2019/evolving-istios-apis/index.md
Patch failed at 0002 Updated traffic management guide

Details

In response to this:

/cherrypick release-1.3

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.

LisaFC · 2019-09-23T14:13:54Z

Is there anything I can do to fix the not-mergeable-with-1.3 thing?

frankbu · 2019-09-23T15:25:54Z

@LisaFC It looks like the change to content/en/blog/2019/evolving-istios-apis/index.md (which ironically isn't really a change at all) breaks the automatic cherrypick to the release-1.3 branch. If you send another PR to branch release-1.3 manually (with just the 6 changed files), that would be great.

LisaFC · 2019-09-23T15:41:43Z

Will do.

LisaFC · 2019-09-23T16:10:05Z

#5042

LisaFC added 3 commits September 16, 2019 16:47

New traffic management rewrite

2913f7e

Updated traffic management guide

a97c62e

merge fix

181a7b4

LisaFC requested a review from a team as a code owner September 18, 2019 15:25

istio-policy-bot added area/networking kind/docs labels Sep 18, 2019

googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Sep 18, 2019

istio-testing added the needs-ok-to-test label Sep 18, 2019

istio-testing added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Sep 18, 2019

istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Sep 18, 2019

geeknoid reviewed Sep 18, 2019

View reviewed changes

content/en/docs/concepts/traffic-management/index.md Outdated Show resolved Hide resolved

geeknoid requested review from a team September 18, 2019 15:39

Fix typo

51b93cb

geeknoid added the cherrypick/release-1.3 label Sep 18, 2019

howardjohn reviewed Sep 18, 2019

View reviewed changes

frankbu reviewed Sep 18, 2019

View reviewed changes

content/en/docs/concepts/traffic-management/index.md Outdated Show resolved Hide resolved

LisaFC and others added 2 commits September 18, 2019 20:12

Update content/en/docs/concepts/traffic-management/index.md

3764f71

Co-Authored-By: John Howard <howardjohn@google.com>

Update content/en/docs/concepts/traffic-management/index.md

86c3c96

Co-Authored-By: John Howard <howardjohn@google.com>

frankbu reviewed Sep 18, 2019

View reviewed changes

content/en/docs/concepts/traffic-management/index.md Outdated Show resolved Hide resolved

frankbu reviewed Sep 18, 2019

View reviewed changes

content/en/docs/concepts/traffic-management/index.md Outdated Show resolved Hide resolved

frankbu reviewed Sep 18, 2019

View reviewed changes

content/en/docs/concepts/traffic-management/index.md Outdated Show resolved Hide resolved

frankbu reviewed Sep 18, 2019

View reviewed changes

content/en/docs/concepts/traffic-management/index.md Outdated Show resolved Hide resolved

LisaFC and others added 3 commits September 19, 2019 14:27

Implementing Frank and John's edits

47be0cb

Update content/en/docs/concepts/traffic-management/index.md

fdc6f5d

Co-Authored-By: Frank Budinsky <frankb@ca.ibm.com>

Merge branch 'master' of https://github.com/istio/istio.io

2c004bf

LisaFC and others added 13 commits September 20, 2019 15:14

Implementing Frank's suggestions

af06740

Merge pull request #1 from LisaFC/LisaFC-patch-1

07cf82c

Implementing Frank's suggestions

More edits

3639d8e

Update content/en/docs/concepts/traffic-management/index.md

d5c53c8

Co-Authored-By: Frank Budinsky <frankb@ca.ibm.com>

Update content/en/docs/concepts/traffic-management/index.md

500aba4

Co-Authored-By: Frank Budinsky <frankb@ca.ibm.com>

Update content/en/docs/concepts/traffic-management/index.md

0d9824b

Co-Authored-By: Frank Budinsky <frankb@ca.ibm.com>

More edits from Frank and Srinath

e2b5822

Update content/en/docs/concepts/traffic-management/index.md

4bf6c18

Co-Authored-By: Frank Budinsky <frankb@ca.ibm.com>

More edits

6f5c61d

Update index.md

fb3467d

yet more edits

abd09f7

Merge branch 'master' of https://github.com/LisaFC/istio.io

84d56aa

Merge branch 'master' of https://github.com/istio/istio.io

4168266

LisaFC added 2 commits September 20, 2019 16:24

Lint fixes and CRD glossary entry

da6526f

more links

101221a

frankbu reviewed Sep 20, 2019

View reviewed changes

content/en/docs/concepts/traffic-management/index.md Outdated Show resolved Hide resolved

sdake reviewed Sep 20, 2019

View reviewed changes

sdake's edits

7130972

frankbu approved these changes Sep 20, 2019

View reviewed changes

istio-testing merged commit 1ecc6cf into istio:master Sep 20, 2019

LisaFC mentioned this pull request Sep 27, 2019

Revisit any unresolved comments on #5014 (Traffic Management concept guide update) #5069

Closed

Conversation

LisaFC commented Sep 18, 2019

Uh oh!

istio-testing commented Sep 18, 2019

Uh oh!

geeknoid commented Sep 18, 2019

Uh oh!

Uh oh!

howardjohn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LisaFC commented Sep 19, 2019

Uh oh!

sdake commented Sep 20, 2019

Uh oh!

Uh oh!

sdake left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frankbu commented Sep 20, 2019

Uh oh!

sdake commented Sep 20, 2019

Uh oh!

frankbu left a comment

Choose a reason for hiding this comment

Uh oh!

frankbu commented Sep 20, 2019

Uh oh!

istio-testing commented Sep 20, 2019

Uh oh!

LisaFC commented Sep 23, 2019

Uh oh!

frankbu commented Sep 23, 2019

Uh oh!

LisaFC commented Sep 23, 2019

Uh oh!