-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Remove templates #5091
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
Remove templates #5091
Conversation
The env wg is working to converge towards templating. If there is a file in here you need, please weigh in on the PR.
Modify updateVersion.sh not to call updateVersion_orig.sh
[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. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/assign @linsun |
@sdake: The following tests 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. |
Pilot tests seem to be failing due to '--kubeconfig=' ( no value ). Not sure how it relates to the change. |
Added '0.8' label for tracking - I don't think we should ship 0.8 with the deprecated/unused templates, we must at least remove them from the release, but if possible from the code as well, to finish up the merging and shift to single templates. |
Note that there is a reference to |
@@ -22,11 +22,6 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/.." | |||
|
|||
cd ${ROOT} | |||
|
|||
# just run the old version | |||
${ROOT}/install/updateVersion_orig.sh "$@" |
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.
@mandarjog I think had left this for one namespace or some other less major yaml ? have we migrated all tests to only use istio-auth.yaml / istio.yaml ?
as long as we still produce |
where would I make the following fix, in the new world: https://github.com/istio/istio/pull/5028/files#diff-17e443480eea200223e3547be4382604 |
In the PR description, we mention keeping the addons, but I think we actually want to remove some of them (and then transition the other ones). In particular, the duplicative |
Yes, for 0.8 we need to cleanup both the tree and release. Zipkin
deprecated addon was used in tests.
…On Fri, Apr 20, 2018, 20:30 Douglas Reid ***@***.***> wrote:
In the PR description, we mention keeping the addons, but I think we
actually want to remove some of them (and then transition the other ones).
In particular, the duplicative grafana, prometheus, and zipkin templates
should disappear (as they are outdated).
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#5091 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAFI6n9Ub6HKrOBK03zQX0WFqs-gwN4Bks5tqqe_gaJpZM4Tct5V>
.
|
@sdake: PR needs rebase. 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. |
@linsun I'm not sure how to produce a binary release. After having a good look at the codebase, this change may be dangerous as I don't entirely understand all of the variants in the templates directory and who needs what. @douglas-reid i'll remove the addons as well, although I am not sure we have sorted out a solution for generating addons from via @ldemailly agree that is the goal of this PR, however, it feels risky given that convergence on the helm charts has not completely finished as evidenced by the changes going into this directory. |
I don't think we need to generate the same yaml - except for istio.yaml and
istio-auth.yaml maybe.
We agreed that supporting/testing all combinations is not needed for 1.0,
and the -namespace
variants can be removed.
The 'official' install options are real helm+tiller or
helm-template/istioctl. For 0.8 to ease migration
we may still generate the old istio.yaml - with clear warnings it is
deprecated and not meant for
production.
The goal is to get to 1.0 - and we agreed on how the install will look
like. We have 1-2 weeks to
polish the 0.8 release - and making sure we're not testing deprecated
configs that will not be used
by our users seems pretty important given how much we rely on the automated
tests. They
can't test configs we won't ship - if removing the old templates breaks the
test it means the tests
were already broken.
…On Sat, Apr 21, 2018 at 8:54 PM, Steven Dake ***@***.***> wrote:
@linsun <https://github.com/linsun> I'm not sure how to produce a binary
release. After having a good look at the codebase, this change may be
dangerous as I don't entirely understand all of the variants in the
templates directory and who needs what.
@douglas-reid <https://github.com/douglas-reid> i'll remove the addons as
well, although I am not sure we have sorted out a solution for generating
addons from via make generate_yaml.
@ldemailly <https://github.com/ldemailly> agree that is the goal of this
PR, however, it feels risky given that convergence on the helm charts has
not completely finished as evidenced by the changes going into this
directory.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#5091 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAFI6gS8wN5IFRwfO8Is6RtZcvH-7oRmks5tq_7xgaJpZM4Tct5V>
.
|
@costinm sounds good. One tactical problem - I am loaded on documenting multicluster for the next 4 days and then i am traveling to Copenhagen returning May 5th. I can work on improving this PR after the docs are resolved, and even during travel but I'm not certain I have enough time to finish the job if the job involves convergence into the main helm chart (to enable a functional automated testing for example). |
Here is citadel standalone - #5292. Convergence of each of these templates into the main chart will take some time - I think not viable for 0.8. |
abandoning this PR as most of the work has been done by other folks. I will continue forward to make citadel standalone work properly from the main helm chart. |
The environment workgroup wants to deprecate the install/kubernetes/templates directory.
A few addons are left in the repository so they may be diffed against the Helm chart.
If there is any file you want to remain in repo, please speak up now.