Codecov Report

Merging #4623 into master will decrease coverage by 2%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           master   #4623     +/-   ##
========================================
- Coverage      73%     72%     -1%     
========================================
  Files         322     295     -27     
  Lines       28085   25247   -2838     
========================================
- Hits        20464   18006   -2458     
+ Misses       6815    6488    -327     
+ Partials      806     753     -53

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6fa713...81065d6. Read the comment docs.

@sdake You were right, I had an old master :(. I rebased the PR on to the latest master. Can you please check?

I would recommend just integrating here: https://github.com/istio/istio/blob/master/release/create_release_archives.sh#L113

add *.cfg, *.yml, and *.j2. That should get things going.

@sebastienvas may want a different approach, but I think this makes the most sense.

@sdake I was pondering that, but I was afraid it might add files we don't want.

Do you think it's safe? Isn't it better to explicitly state that we want everything in the ansible directory?

@geoand I am not well-versed in the release process, however, reading that find command, it finds all files in istio/install that should go in the release tarball. I believe is it safe to say .yml files, .cfg files, and .j2 files are always related to install and should be specified individually rather than calling out a specific directory.

Also as a further followup work, it may make sense to move the ansible dir into the install/kube directory (since the Ansible code is kubernetes specific and doesn't handle other aspects of installation on different platforms.

Cheers
-steve

Thanks for the comments @sdake!

I updated the script to not use the install/ansible directory and instead use the specific file types.
Would you like me to also move the content of the ansible directory into install/kubernetes like you mentioned?

@geoand yes I was thinking about the fact that the Ansible directory is in the wrong parent and why that was the case this weekend as I was reworking your Ansible documentation. I hadn't picked up the incorrect parenting for your Istio specific deployment playbook.

@andraxylia had requested in the past an Ansible implementation that automates mesh expansion. After seeing mesh expansion executed, I agree that needs to be done.

I'll leave the question to her to answer as the choice impacts mesh expansion.

Cheers
-steve

/assign @andraxylia

@andraxylia please weigh in on comment #4623 (comment).

Cheers
-steve

@geoand please ping on this PR Monday if no action has been taken. I would like to see this merge prior to 0.8 (~5-10 days before branching) so the Ansible playbooks are not DOA.

I will let @sebastienvas review, he has knowledge of the release script.

@sdake Do we have any update on this?

Thank you very much

/assign @sebastienvas

@geoand I don't have an update - I don't have ACLs to merge files in this directory. (Check OWNERS file).

Thanks for the update @sdake !

/lgtm

/test istio-pilot-e2e

/test e2e-bookInfo-envoyv2-v1alpha3

@sdake @geoand I would really like to see ansible for mesh expansion, and I bet I am not the only one.

I agree the current /install directory where ansible for k8s is not under kubernetes does not make sense, and I forgot the reason why it was added there.
We still have to decide where ansible for mesh expansion will leave, right now mesh expansion is also under kubernetes. We will need to separate it and possible re-organize the directory layout, but it does not have to be part of this PR.

I am not familiar with mesh expansion, but I'll have a look soon and see if I can use Ansible for it

@andraxylia @sdake Do this PR needs to be updated or can we merge it so the contents of the ansible playbook will be available in the next release?

/retest

I asked the question here: https://groups.google.com/forum/#!topic/istio-dev/grGYrymC-xc

Thanks @sdake

pending required test ci/circleci: e2e-pilot-auth-v1alpha3-v2

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

/retest

@geoand: The following test failed, say /retest to rerun them all:

This pull request has been automatically marked as stale because it has not had activity in the last 2 weeks. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

/retest

I have tested this PR and it includes the additional 25 files from the install/ansible directory. It would be great to have the Ansible installer in the 1.0 release.

Closing in favor of #7194