add istio.io tutorial yamls #3940
Conversation
for the Istio tutorial to be published - istio/istio.io#1035
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/test istio-presubmit |
|
Looks good. Mind flipping the copyright date to 2018? Thanks. |
|
@geeknoid yep, will sed the date to 2018. |
|
@geeknoid I changed the year to |
|
/hold |
|
@ldemailly The tutorial uses the existing bookinfo yamls where possible, and also the yamls of this PR. |
|
@ldemailly I have deleted bookinfo-reviews-v2-with-app-label.yaml, will I can't see any other yamls that can be replaced by the existing ones. |
|
the problem with those is they are easy to add but add technical debt and maintenance cost for when something change and we need to update them there should be a way to combine the 2; there are already 36 yamls in samples/bookinfo/kube we should not add 13 more maybe in https://github.com/istio/contrib but even there; it's easy to duplicate/create; hard to maintain/keep working |
|
@ldemailly I propose that I will write e2e tests to test the ones I add for the tutorial. Tomorrow starts the tests week, so it is a good opportunity actually. I do not see any way other than to have multiple yamls. I think we need to write more blogs, tutorials, documentation about Istio. If we use inline yamls in the docs, the docs themselves will rot. If we put the yamls in https://github.com/istio/contrib, without writing e2e tests, the yamls will rot in https://github.com/istio/contrib. In my opinion - we should have as many yamls as required, with as many e2e tests as required, the more tests, the better. We should process all the yamls by scripts, for example updating all the bookinfo images in all the yamls is performed by the following script https://github.com/istio/istio/blob/master/samples/bookinfo/build_push_update_images.sh . Once we have to update the yamls, for example when changing the route rules format, we should update all of them by a script in one stroke, using sed, etc., or writing a converter tool. |
|
sounds good to me if you add tests can you at high level say which features are covered in the blog/tutorial that aren't already covered by the other 36 yamls / require 13 new ones? |
|
@ldemailly I explained it in the comment of the PR: istio/istio.io#1035 (comment)
The non-covered feature: incremental enablement of Istio. Though it should be covered in a task as well. Also not covered by the tasks - running a single microservice on a local computer, running the single microservice in a Docker container, running Bookinfo on Kubernetes without Istio. |
|
I had read the description in the other PR, I still don't get what specifically prevents 95% of the yamls to be the exact same (whether rules or bookinfo itself etc...) and just the ordering and/or change of cleanup Also personally I'd rather have 1 unified better content than 2 similar yet subtly different set of tasks and stuff to maintain for the same app - why not fix whatever is wrong with the tasks? and add local computer case ? Looking forward to the tests and minimization of new files |
Let's come to terms with the fact that we will need multiple types of documentation, tailored to the different audiences/different goals. There will be always overlap between these types, the same information is presented differently. In the same way there will be external resources about Istio, videos, presentations etc. It is not that some type is better or worse, it is the same content for different goals. Consider learning some technology. Some people prefer a book, some a video course, some face-to-face course, some prefer tutorials or blogs or video talks or live talks. We have the following types of content on istio.io:
This is, of course, my opinion and I may be wrong. This is how I propose to treat different types of documentation. |
There was a problem hiding this comment.
I agree with Laurent that bookinfo config is a huge mess, and adding more configs without deleting old ones will make it even harder to understand.
For both bookinfo and e2e tests we should at least separate the 'install app' templates from the 'dynamically change things for a demo'. If we can keep at least the first part organized it'll be easier to deploy all test apps
on a cluster and have a stable test env.
Since next week we'll focus on tests - maybe this time
we'll have the new clean config replacing the old ones,
so we can add it to the circleci presubmit.
I would be glad to participate in cleaning the mess during this week of tests. However, I do not understand why should this particular PR and the tutorial I wrote be dependent on the cleanup? I wrote a tutorial and got approval of several people to proceed with it, see the PR for the tutorial istio/istio.io#1035. |
|
@ldemailly I documented per each yaml why it is needed. Please advise me which one I can remove.
|
TODO until this PR is merged istio/istio#3940
|
closing this PR, I will work on restructuring samples/bookinfo first |
|
Reopening following #4042 (comment). |
|
/test istio-pilot-e2e |
|
/test istio-unit-tests |
…rule-ratings-test-abort.yaml
Codecov Report
@@ Coverage Diff @@
## master #3940 +/- ##
=======================================
- Coverage 73% 73% -<1%
=======================================
Files 313 313
Lines 28135 28759 +624
=======================================
+ Hits 20520 20937 +417
- Misses 6349 6500 +151
- Partials 1266 1322 +56
Continue to review full report at Codecov.
|
|
@vadimeisenbergibm: The following test failed, say
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. I understand the commands that are listed here. |
|
closing to submit file by file, with matching tests |
for the Istio tutorial to be published - istio/istio.io#1035