Initial version of the VM helpers, extracted from the test script. by costinm · Pull Request #737 · istio/istio

costinm · 2017-09-11T22:01:28Z

We'll need to update them with the commands to copy files to other clusters - right now
I only have gcloud. Also need more polish - and some instructions or helper to download all
needed artifacts.

Added scripts for VM setup (cluster extension)

rshriram

Copyright missing.. some naming consistency. Otherwise looks ok

rshriram · 2017-09-11T22:15:50Z

install/tools/istio_vm_common.sh

@@ -0,0 +1,223 @@
+#!/usr/bin/env bash


Copyright missing

rshriram · 2017-09-11T22:17:36Z

install/tools/istio_vm_common.sh

+#!/usr/bin/env bash
+
+# Helper functions for extending the cluster with external VMs.
+


Cluster -> mesh
Helper functions for extending istio mesh outside kubernetes

rshriram · 2017-09-11T22:18:03Z

install/tools/istio_vm_common.sh

+# ISTIO_NAMESPACE - if not set the default in .kube/config is used.
+# ISTIO_FILES - directory where istio artifacts are downloaded, default to current dir
+
+# Initialize internal load balancers to access K8S DNS and Istio Pilot, Mixer, CA.


K8s to kubernetes

rshriram · 2017-09-11T22:20:28Z

install/tools/istio_vm_common.sh

+kind: Service
+metadata:
+  name: istio-pilot-ilb
+  annotations:


Istio-pilot-ilb-gcp

There are a bunch of other clusters using ILB, I didn't add the other annotations since I
didn't have time to test them. I think in the case of bluemix ( and possibly
others ) this creates a regular internal service (annotation ignored), but still routable from the VMs.

Maybe I should call it istio-pilot-ext ? ( since in some cases an ILB is not needed )

It's not gcp-specific, the service will be used for other providers as well.
(and we'll add annotations as needed by the other providers )

istio-pilot-alt?

or leave it as is

I'll leave it as is if you don't mind, we can probably merge it into the main configs ( just add annotations to the 'real service' ) after we get more testing.

rshriram · 2017-09-11T22:23:35Z

install/tools/istio_vm_common.sh

+apiVersion: v1
+kind: Service
+metadata:
+  name: mixer-ilb


Consistent naming. Istio-mixer, kube-dns, etc

rshriram · 2017-09-11T22:25:54Z

install/tools/istio_vm_common.sh

+# - name of the VM - used to copy files over.
+# - service account to be provisioned (defaults to istio.default)
+function istioProvisionVM() {
+ NAME=${1}


istioBootstrapServiceInstance

how about istioBootstrapVM (or Node?) ? Service Instance is super confusing (with the actual service ), we're provisioning a machine.

Node is good!

rshriram · 2017-09-11T22:26:22Z

install/tools/istio_vm_common.sh

+ NAME=${1}
+
+ local SA=${2:-istio.default}
+


Need comments

Done ( and will keep adding as we refine it ).

linsun · 2017-09-12T03:08:37Z

install/tools/istio_vm_common.sh

+  (cd $ISTIO_IO/pilot; bazel build tools/deb/... )
+  (cd $ISTIO_IO/auth; bazel build tools/deb/... )
+
+  cp $ISTIO_IO/proxy/bazel-bin/tools/deb/istio-proxy-envoy.deb \


Are these .deb files only working on certain platforms?

I tested on current debian and xenial. Should work on other similar platforms.

Only tested on current deb and xenial.
For Trusty would be tricky ( envoy c++ deps ), but it is possible with a special build.

linsun · 2017-09-12T03:09:12Z

install/tools/istio_vm_common.sh

+    if [[ ${ISTIO_ZONE:-} != "" ]]; then
+       OPTS="$OPTS --zone $ISTIO_ZONE"
+    fi
+    echo $OPTS


Is it possible to have cloud platform dependent methods in one file vs spread out? That seems easier to maintain.

I can try to group them, technically I can get rid of this function and rely on defaults.
I suppose I can also use env variables to inject 'gcloud' if it is a major concern - we can
refine it as we test on more platforms.

Maybe in a separate PR ?

linsun · 2017-09-12T03:11:23Z

install/tools/istio_vm_common.sh

+# ISTIO_FILES - directory where istio artifacts are downloaded, default to current dir
+
+# Initialize internal load balancers to access K8S DNS and Istio Pilot, Mixer, CA.
+# Must be run once per cluster.


could we document dependencies and params for this helper script?

linsun · 2017-09-12T03:12:20Z

install/tools/istio_vm_common.sh

+metadata:
+  name: istio-ca-ilb
+  annotations:
+    cloud.google.com/load-balancer-type: "internal"


could the annotation/name be kept generic, e.g. not platform specific?

Unfortunately that's the magic syntax that is required. We can add multiple annotations ( to cover the other platforms that need magic strings ).

See above - hopefully a 'standard' k8s emerges, but until then we'll need one for each platform.

costinm · 2017-09-13T00:53:56Z

/test

costinm · 2017-09-13T00:55:14Z

/retest

ldemailly

minor nit/confusion

ldemailly · 2017-09-13T08:04:52Z

install/tools/istio_vm_common.sh

+# by istioProvisionVM
+function istioCopyBuildFiles() {
+  local ISTIO_IO=${ISTIO_BASE:-${GOPATH:-$HOME/go}}/src/istio.io
+  local ISTIO_FILES=${ISTIO_FILES:-.}


it is not super clear to me what that variable is/does exactly in this function, is it a destination directory, a source dir, a set of files...

Added more docs.

ldemailly · 2017-09-13T08:05:57Z

install/tools/istio_vm_setup.sh

+
+# Script to install istio components for the raw VM.
+
+# Environment variable pointing to the generated Istio configs and binaries.


directory ? can it really be files ? for example ?

For example "." - we'll need to adjust this based on how we distribute, probably a bunch of curl to
fetch the .deb files, or a tar (as you suggested earlier).
Added a TODO, since I don't know yet.

ldemailly · 2017-09-13T08:06:27Z

install/tools/istio_vm_setup.sh

+
+  # Copy config files for DNS
+  chmod go+r ${ISTIO_FILES}/kubedns
+  cp ${ISTIO_FILES}/kubedns /etc/dnsmasq.d


k rename to ISTIO_STAGE_DIR or some such, it's not FILES

ldemailly · 2017-09-13T22:09:18Z

install/tools/istio_vm_setup.sh

+  istioNetworkInit
+  istioInstall
+  istioRestart
+fi


missing new line

ldemailly · 2017-09-13T22:09:35Z

/lgtm

ldemailly · 2017-09-13T22:19:17Z

please put release notes also

istio-merge-robot · 2017-09-13T22:22:47Z

/lgtm cancel //PR changed after LGTM, removing LGTM. @costinm @ldemailly

ldemailly · 2017-09-13T22:32:44Z

/lgtm

istio-merge-robot · 2017-09-13T22:35:48Z

/lgtm cancel //PR changed after LGTM, removing LGTM. @costinm @ldemailly

ldemailly · 2017-09-13T22:44:21Z

/lgtm

istio-merge-robot · 2017-09-13T22:44:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ldemailly

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

Details

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

~~OWNERS~~ [ldemailly]

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

istio-merge-robot · 2017-09-13T22:58:52Z

/test all [submit-queue is verifying that this PR is safe to merge]

istio-merge-robot · 2017-09-13T23:21:25Z

Automatic merge from submit-queue

istio-testing · 2017-09-13T23:50:37Z

@costinm: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
prow/new-e2e-rbac_no_auth.sh	`758d4a6`	link	`/test new-e2e-rbac_no_auth`
prow/e2e-suite-rbac-no_auth.sh	`59cd27c`	link	`/test e2e-suite-rbac-no_auth`

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. I understand the commands that are listed here.

) Automatic merge from submit-queue Initial version of the VM helpers, extracted from the test script. We'll need to update them with the commands to copy files to other clusters - right now I only have gcloud. Also need more polish - and some instructions or helper to download all needed artifacts. ```release-note Added scripts for VM setup (cluster extension) ``` Former-commit-id: a45134d

…stio#737) Automatic merge from submit-queue Initial version of the VM helpers, extracted from the test script. We'll need to update them with the commands to copy files to other clusters - right now I only have gcloud. Also need more polish - and some instructions or helper to download all needed artifacts. ```release-note Added scripts for VM setup (cluster extension) ``` Former-commit-id: a45134d

) Automatic merge from submit-queue Initial version of the VM helpers, extracted from the test script. We'll need to update them with the commands to copy files to other clusters - right now I only have gcloud. Also need more polish - and some instructions or helper to download all needed artifacts. ```release-note Added scripts for VM setup (cluster extension) ``` Former-commit-id: a45134d

Automatic merge from submit-queue. [DO NOT MERGE] Auto PR to update dependencies of proxy This PR will be merged automatically once checks are successful. ```release-note none ```

costinm added 2 commits September 11, 2017 14:52

Initial version of VM tools

2042053

Merge branch 'master' of github.com:istio/istio into costin-vm

c64d4ff

costinm requested review from ldemailly and rshriram September 11, 2017 22:01

googlebot added the cla: yes label Sep 11, 2017

istio-merge-robot added the release-note-none label Sep 11, 2017

Minor fixes

ab5197d

rshriram suggested changes Sep 11, 2017

View reviewed changes

Default to istio-system, for consistency

11fa5ab

linsun reviewed Sep 12, 2017

View reviewed changes

Fixes based on review and testing

6db44aa

Merge branch 'master' into costin-vm

21ba9cd

rshriram approved these changes Sep 13, 2017

View reviewed changes

ldemailly reviewed Sep 13, 2017

View reviewed changes

costinm added 3 commits September 13, 2017 14:07

Merge branch 'master' into costin-vm

5e3d48e

Merge branch 'master' into costin-vm

758d4a6

Update based on PR feedback

ce3c149

ldemailly reviewed Sep 13, 2017

View reviewed changes

istio-testing assigned ldemailly Sep 13, 2017

istio-testing added the lgtm label Sep 13, 2017

istio-merge-robot added the approved label Sep 13, 2017

costinm and others added 3 commits September 13, 2017 15:21

CRLF

7859614

Merge branch 'costin-vm' of github.com:istio/istio into HEAD

2c6cdc3

Merge branch 'master' into costin-vm

8e99cfb

istio-merge-robot removed the lgtm label Sep 13, 2017

istio-testing added the lgtm label Sep 13, 2017

Merge branch 'master' into costin-vm

59cd27c

istio-merge-robot removed the lgtm label Sep 13, 2017

istio-testing added the lgtm label Sep 13, 2017

istio-merge-robot merged commit a45134d into master Sep 13, 2017

costinm deleted the costin-vm branch September 14, 2017 20:43

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

remove file watcher for template renderer (istio#737)

ce0beea

		#!/usr/bin/env bash

		# Helper functions for extending the cluster with external VMs.


		# Script to install istio components for the raw VM.

		# Environment variable pointing to the generated Istio configs and binaries.

Conversation

costinm commented Sep 11, 2017 • edited by ldemailly Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rshriram 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!

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!

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!

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!

costinm commented Sep 13, 2017

Uh oh!

costinm commented Sep 13, 2017

Uh oh!

ldemailly 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!

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!

costinm commented Sep 11, 2017 •

edited by ldemailly

Loading