Update installation files for cluster-wide #685

andraxylia · 2017-09-06T20:23:56Z

Update installation files for cluster-wide

Istio control plane is now in a dedicated "istio-system" namespace. Applications can now run on multiple namespaces.

Unified auth and non-auth templates, and got rid of the istio-auth directory.

For backward compatibility, the files are still the same, except all istio*.yaml have rbac beta. Rbac is tricky around namespaces. I the next PR, istio-cluster-wide will become istio-yaml, istio-auth.yaml will disappear, and istio-no-auth.yaml will be created. This will require minimal changes.

Generated files go by default in istio-system. By running updateVersio,sh -n , you can generate same files for any namespace.

istio-merge-robot · 2017-09-06T20:25:49Z

@andraxylia PR needs rebase

istio-merge-robot · 2017-09-06T20:25:50Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
We suggest the following additional approver: rshriram

Assign the PR to them by writing /assign @rshriram in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

OWNERS

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

douglas-reid

Only one question. Doesn't need to block PR progress.

douglas-reid · 2017-09-06T20:29:48Z

install/kubernetes/addons/grafana.yaml

@@ -3,6 +3,7 @@ apiVersion: v1
 kind: Service
 metadata:
  name: grafana
+  namespace: istio-system


i don't know that it really matters, but should we (if it is possible) have the addons be in a separate namespace?

it is possible, but it is an enhancement.

frankbu · 2017-09-06T21:18:05Z

install/README.md

@@ -15,6 +15,7 @@ The [updateVersion.sh](updateVersion.sh) script is used to update image versions
 * `-c <hub>,<tag>` new ca image
 * `-i <url>` new `istioctl` download URL
 * `-g` create a `git commit` titled "Updating istio version" for the changes
+* `-n` <namespace> namespace in which to install Istio control plane components (defaults to "istio-system")


Shouldn't the default be the value that was last set in istio.VERSION (i.e., the current value), same as is done with other fields?

frankbu · 2017-09-06T21:20:54Z

install/updateVersion.sh

@@ -134,30 +142,46 @@ export MIXER_TAG="${MIXER_TAG}"
 export ISTIOCTL_URL="${ISTIOCTL_URL}"
 export PILOT_HUB="${PILOT_HUB}"
 export PILOT_TAG="${PILOT_TAG}"
+export ISTIO_NAMESPACE="istio-system"


Value should be ${ISTIO_NAMESPACE}

done, PTAL.

andraxylia · 2017-09-06T22:47:09Z

This should not be merged until cluster-pool is ready and can be tested in isolation.

rshriram · 2017-09-06T23:06:15Z

Please update..

…e2e tests

istio-merge-robot · 2017-09-07T01:34:52Z

@andraxylia PR needs rebase

ldemailly

so IMO there are only 3 "must change" left to get this in:

revert the change to mixer_test or do it without loosing the feature
the ISTIO_NAMESPACE="$(echo ${ISTIO_NAMESPACE}|cut -f1 -d,)"
header in generated files

and ideally but I'm ok without it:

.md update or remove mention of updateVersion
getting rid of useless -alpha files
follow up on double rbac apply

ldemailly · 2017-09-10T19:25:37Z

install/updateVersion.sh

@@ -134,30 +151,42 @@ export MIXER_TAG="${MIXER_TAG}"
 export ISTIOCTL_URL="${ISTIOCTL_URL}"
 export PILOT_HUB="${PILOT_HUB}"
 export PILOT_TAG="${PILOT_TAG}"
+export ISTIO_NAMESPACE=${ISTIO_NAMESPACE}
 EOF
 }

 function update_istio_install() {
  pushd $TEMP_DIR/templates


oh I missed that initially, everything is copied in /tmp/templates and modified there
it probably should be a unique temp dir instead but I guess that's not a new problem created by this pr

hopefully we can agree this script can be improved and replaced by something less convoluted (not in this PR clearly, but for 0.3)

Yes, sorry about the existence of this script :) The problem with these bash scripts is that they tend to grow beyond their original purpose. We should try to address the core problems (auth vs no auth) and obviate the need to customize installations.

+1, it's a pain to write bash

ldemailly · 2017-09-10T22:12:43Z

ps: that /tmp/templates hardcoded non unique path - any chance it bites us on the cont build
(or is every build run 100% completely isolated / have their own filesystem for exclusive use ? @sebastienvas )

andraxylia · 2017-09-11T00:53:37Z

Will address the minor comments in a separate PR, so we can mitigate the risks introduced by istio/old_pilot_repo#1223.

ldemailly · 2017-09-11T01:10:58Z

please revert the mixer_test change at minimum
and fix the cut, it takes 3 sec to edit and retest locally
for the headers that are missing (and will cause dev errors) I suppose we can also postpone that but I mentioned it days ago, not sure why you didn't make the change

ldemailly · 2017-09-11T01:22:20Z

I'm making the changes I'm requesting, should be in shortly.

Minimum set to get this PR in - the rest will wait

ldemailly

there are many small things needed in follow up but this will do for now as it is a major feature we need to get started testing

ldemailly · 2017-09-11T01:27:49Z

let's merge in 16 minutes once prow/e2e-suite-rbac-auth.sh passes

ldemailly · 2017-09-11T01:51:51Z

I'm ok to merge the PR as is but not (without review) if it further changes

ldemailly · 2017-09-11T02:01:27Z

istio.VERSION

@@ -7,3 +7,4 @@ export MIXER_TAG="1bc30a23190aa58635d02ff7fd31bf74de0d011e"
 export ISTIOCTL_URL="https://storage.googleapis.com/istio-artifacts/pilot/330dd286541d1b84c5ac1f4fc504556796c070af/artifacts/istioctl"
 export PILOT_HUB="gcr.io/istio-testing"
 export PILOT_TAG="330dd286541d1b84c5ac1f4fc504556796c070af"
+export ISTIO_NAMESPACE=istio-system


@kyessenov this PR isn't changing pilot so let's fix the auth in a simple follow up which should have been done whenever that authPolicy change happened

* Update installation files for cluster-wide * Add zipkin-to-stackdriver template * Address code review comments * Put back rbac files where they were for e2e tests * Create a new file istio-cluster-wide.yaml to keep compatibility with e2e tests * Fix bad merge * Update README.md * Unify auth and non-auth templates * Fix mixer template * Changes tests to replace istio-system with the current namespace * Fix bazel * Bazel fix * Bazel fix * Revert wrong line * Remove logs creating error * Updates README * Fix kube-inject to point to the same namespace * Code review comments Minimum set to get this PR in - the rest will wait Former-commit-id: feeb301

* added redirect routing rule to e2e * review comments * bug fix * code review comments

* Update installation files for cluster-wide * Add zipkin-to-stackdriver template * Address code review comments * Put back rbac files where they were for e2e tests * Create a new file istio-cluster-wide.yaml to keep compatibility with e2e tests * Fix bad merge * Update README.md * Unify auth and non-auth templates * Fix mixer template * Changes tests to replace istio-system with the current namespace * Fix bazel * Bazel fix * Bazel fix * Revert wrong line * Remove logs creating error * Updates README * Fix kube-inject to point to the same namespace * Code review comments Minimum set to get this PR in - the rest will wait Former-commit-id: feeb301

…tio#685)

Update installation files for cluster-wide

1c3cbac

googlebot added the cla: yes label Sep 6, 2017

andraxylia requested review from kyessenov, douglas-reid, wattli and frankbu September 6, 2017 20:24

Merge remote-tracking branch 'origin' into clusterwide

63d675e

istio-merge-robot added needs-rebase Indicates a PR needs to be rebased before being merged release-note-label-needed labels Sep 6, 2017

istio-merge-robot removed the needs-rebase Indicates a PR needs to be rebased before being merged label Sep 6, 2017

douglas-reid reviewed Sep 6, 2017

View reviewed changes

frankbu reviewed Sep 6, 2017

View reviewed changes

andraxylia added 4 commits September 6, 2017 14:47

Merge remote-tracking branch 'origin' into clusterwide

d0667ea

Add zipkin-to-stackdriver template

7a7ed70

Address code review comments

8ef70d9

Put back rbac files where they were for e2e tests

12cd964

andraxylia added the do-not-merge Block automatic merging of a PR. label Sep 6, 2017

Merge branch 'master' into clusterwide

ed96480

frankbu approved these changes Sep 6, 2017

View reviewed changes

Create a new file istio-cluster-wide.yaml to keep compatibility with …

96ed447

…e2e tests

andraxylia removed the do-not-merge Block automatic merging of a PR. label Sep 7, 2017

istio-merge-robot added the needs-rebase Indicates a PR needs to be rebased before being merged label Sep 7, 2017

Merge remote-tracking branch 'origin' into clusterwide

91032ff

istio-merge-robot removed the needs-rebase Indicates a PR needs to be rebased before being merged label Sep 7, 2017

ldemailly reviewed Sep 10, 2017

View reviewed changes

ldemailly added cla: yes and removed cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. labels Sep 10, 2017

ldemailly mentioned this pull request Sep 11, 2017

Dynamic mesh config istio/old_pilot_repo#1223

Closed

Code review comments

b10fb83

Minimum set to get this PR in - the rest will wait

googlebot added cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. and removed cla: yes labels Sep 11, 2017

ldemailly approved these changes Sep 11, 2017

View reviewed changes

ldemailly added cla: human-approved and removed cla: no Set by the Google CLA bot to indicate the author of a PR has not signed the Google CLA. labels Sep 11, 2017

andraxylia added the do-not-merge Block automatic merging of a PR. label Sep 11, 2017

ldemailly reviewed Sep 11, 2017

View reviewed changes

andraxylia merged commit feeb301 into master Sep 11, 2017

andraxylia removed the do-not-merge Block automatic merging of a PR. label Sep 11, 2017

ldemailly deleted the clusterwide branch September 15, 2017 21:29

mandarjog pushed a commit that referenced this pull request Oct 31, 2017

redirect routing rule for e2e (#685)

e3469b7

* added redirect routing rule to e2e * review comments * bug fix * code review comments

howardjohn pushed a commit to howardjohn/istio that referenced this pull request Jan 12, 2020

update(installer): use latest sha for telemetry v2 and owners fix (is…

118c499

…tio#685)

0x01001011 pushed a commit to thedemodrive/istio that referenced this pull request Jul 16, 2020

Add prototool for linting, precommit, Circle, Prow. (istio#685)

d2dcf00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update installation files for cluster-wide #685

Update installation files for cluster-wide #685

andraxylia commented Sep 6, 2017 •

edited by ldemailly

Loading

istio-merge-robot commented Sep 6, 2017

istio-merge-robot commented Sep 6, 2017

douglas-reid left a comment

douglas-reid Sep 6, 2017

andraxylia Sep 6, 2017

frankbu Sep 6, 2017

andraxylia Sep 6, 2017

frankbu Sep 6, 2017

andraxylia Sep 6, 2017

andraxylia commented Sep 6, 2017

rshriram commented Sep 6, 2017

istio-merge-robot commented Sep 7, 2017

ldemailly left a comment •

edited

Loading

ldemailly Sep 10, 2017

kyessenov Sep 10, 2017

andraxylia Sep 11, 2017

ldemailly commented Sep 10, 2017

andraxylia commented Sep 11, 2017

ldemailly commented Sep 11, 2017

ldemailly commented Sep 11, 2017

ldemailly left a comment

ldemailly commented Sep 11, 2017

ldemailly commented Sep 11, 2017

ldemailly Sep 11, 2017

Update installation files for cluster-wide #685

Update installation files for cluster-wide #685

Conversation

andraxylia commented Sep 6, 2017 • edited by ldemailly Loading

istio-merge-robot commented Sep 6, 2017

istio-merge-robot commented Sep 6, 2017

douglas-reid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andraxylia commented Sep 6, 2017

rshriram commented Sep 6, 2017

istio-merge-robot commented Sep 7, 2017

ldemailly left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ldemailly commented Sep 10, 2017

andraxylia commented Sep 11, 2017

ldemailly commented Sep 11, 2017

ldemailly commented Sep 11, 2017

ldemailly left a comment

Choose a reason for hiding this comment

ldemailly commented Sep 11, 2017

ldemailly commented Sep 11, 2017

Choose a reason for hiding this comment

andraxylia commented Sep 6, 2017 •

edited by ldemailly

Loading

ldemailly left a comment •

edited

Loading