Fix multicluster e2e test setup to use helm chart template (istio#6679)

* Fix istio-remote and multicluster e2e test setup to use helm chart template

- Add istio-ingressgateway service and endpoints option to istio-remote charts
- Make the multicluster tests always setup istio-remote via helm template with
  passed args.
- Fix crash in getEndpointIPForService method

* e2e test install addons prior to remote cluster install

* Update e2e framework to work with istio-remote helm chart created configmap

- istio-remote changes remove selectorless service/endpoint setup so DNS
  for istio control plane services won't resolve on remote.  Sidecar
  injection configmap setup on remote uses the IP addresses of services
  directly.
  - change istioctl in e2e framework to allow use of sidecar inject
    configmap on remote rather than default settings.
- enable/disable sidecar injection in remote based on test flag
- misc. fix zipkin address in istio-remote configmap

* Multicluster pilot e2e: fix remote cleanup and ingress subtests

tests/e2e/framework/app_manager.go

@@ -90,7 +90,7 @@ func (am *AppManager) deploy(a *App) error {
 			log.Errorf("CreateTempfile failed %v", err)
 			return err
 		}
-		if err = am.istioctl.KubeInject(a.AppYaml, finalYaml); err != nil {
+		if err = am.istioctl.KubeInject(a.AppYaml, finalYaml, am.Kubeconfig); err != nil {
 			log.Errorf("KubeInject failed for yaml %s: %v", a.AppYaml, err)
 			return err
 		}

tests/e2e/framework/framework.go

@@ -107,6 +107,7 @@ func NewCommonConfigWithVersion(testID, version string) (*CommonConfig, error) {
 	c.Cleanup.RegisterCleanable(c.Kube.Istioctl)
 	c.Cleanup.RegisterCleanable(c.Kube.AppManager)
 	if c.Kube.RemoteKubeConfig != "" {
+		c.Cleanup.RegisterCleanable(c.Kube.RemoteAppManager.istioctl)
 		c.Cleanup.RegisterCleanable(c.Kube.RemoteAppManager)
 	}
 

tests/e2e/framework/istioctl.go

@@ -50,10 +50,12 @@ type Istioctl struct {
 	yamlDir         string
 	// If true, will ignore proxyHub and proxyTag but use the default one.
 	defaultProxy bool
+	// if non-null, used for sidecar inject (note: proxyHub and proxyTag overridden)
+	injectConfigMap string
 }
 
 // NewIstioctl create a new istioctl by given temp dir.
-func NewIstioctl(yamlDir, namespace, istioNamespace, proxyHub, proxyTag string, imagePullPolicy string) (*Istioctl, error) {
+func NewIstioctl(yamlDir, namespace, istioNamespace, proxyHub, proxyTag, imagePullPolicy, injectConfigMap string) (*Istioctl, error) {
 	tmpDir, err := ioutil.TempDir(os.TempDir(), tmpPrefix)
 	if err != nil {
 		return nil, err
@@ -76,6 +78,7 @@ func NewIstioctl(yamlDir, namespace, istioNamespace, proxyHub, proxyTag string,
 		imagePullPolicy: imagePullPolicy,
 		yamlDir:         filepath.Join(yamlDir, "istioctl"),
 		defaultProxy:    *defaultProxy,
+		injectConfigMap: injectConfigMap,
 	}, nil
 }
 
@@ -145,18 +148,30 @@ func (i *Istioctl) run(format string, args ...interface{}) error {
 // TODO The commands below could be generalized so that istioctl doesn't default to
 // using the in cluster kubeconfig this is useful in multicluster cases to perform
 // injection on remote clusters.
-func (i *Istioctl) KubeInject(src, dest string) error {
+func (i *Istioctl) KubeInject(src, dest, kubeconfig string) error {
+	injectCfgMapStr := ""
+	if i.injectConfigMap != "" {
+		injectCfgMapStr = fmt.Sprintf("--injectConfigMapName %s", i.injectConfigMap)
+	}
+	kubeconfigStr := ""
+	if kubeconfig != "" {
+		kubeconfigStr = " --kubeconfig " + kubeconfig
+	}
 	if i.defaultProxy {
-		return i.run(`kube-inject -f %s -o %s -n %s -i %s --meshConfigMapName=istio`,
-			src, dest, i.namespace, i.namespace)
+		return i.run(`kube-inject -f %s -o %s -n %s -i %s --meshConfigMapName=istio %s %s`,
+			src, dest, i.namespace, i.namespace, injectCfgMapStr, kubeconfigStr)
 	}
 
 	imagePullPolicyStr := ""
 	if i.imagePullPolicy != "" {
 		imagePullPolicyStr = fmt.Sprintf("--imagePullPolicy %s", i.imagePullPolicy)
 	}
-	return i.run(`kube-inject -f %s -o %s --hub %s --tag %s %s -n %s -i %s --meshConfigMapName=istio`,
-		src, dest, i.proxyHub, i.proxyTag, imagePullPolicyStr, i.namespace, i.namespace)
+	hubAndTagStr := ""
+	if i.injectConfigMap == "" {
+		hubAndTagStr = fmt.Sprintf("--hub %s --tag %s", i.proxyHub, i.proxyTag)
+	}
+	return i.run(`kube-inject -f %s -o %s %s %s -n %s -i %s --meshConfigMapName=istio %s %s`,
+		src, dest, hubAndTagStr, imagePullPolicyStr, i.namespace, i.namespace, injectCfgMapStr, kubeconfigStr)
 }
 
 // CreateRule create new rule(s)

tests/e2e/framework/kubernetes.go

@@ -183,7 +183,7 @@ func newKubeInfo(tmpDir, runID, baseVersion string) (*KubeInfo, error) {
 		}
 	}
 	yamlDir := filepath.Join(tmpDir, "yaml")
-	i, err := NewIstioctl(yamlDir, *namespace, *namespace, *proxyHub, *proxyTag, *imagePullPolicy)
+	i, err := NewIstioctl(yamlDir, *namespace, *namespace, *proxyHub, *proxyTag, *imagePullPolicy, "")
 	if err != nil {
 		return nil, err
 	}
@@ -231,8 +231,16 @@ func newKubeInfo(tmpDir, runID, baseVersion string) (*KubeInfo, error) {
 		if remoteKubeClient, err = kube.CreateInterface(remoteKubeConfig); err != nil {
 			return nil, err
 		}
-
-		aRemote = NewAppManager(tmpDir, *namespace, i, remoteKubeConfig)
+		// Create Istioctl for remote using injectConfigMap on remote (not the same as master cluster's)
+		remoteI, err := NewIstioctl(yamlDir, *namespace, *namespace, *proxyHub, *proxyTag, *imagePullPolicy, "istio-sidecar-injector")
+		if err != nil {
+			return nil, err
+		}
+		if baseVersion != "" {
+			remoteI.localPath = filepath.Join(releaseDir, "/bin/istioctl")
+			remoteI.defaultProxy = true
+		}
+		aRemote = NewAppManager(tmpDir, *namespace, remoteI, remoteKubeConfig)
 	}
 
 	a := NewAppManager(tmpDir, *namespace, i, kubeConfig)
@@ -325,13 +333,6 @@ func (k *KubeInfo) Setup() error {
 				log.Error("Failed to deploy Istio.")
 				return err
 			}
-
-			if k.InstallAddons {
-				if err = k.deployAddons(); err != nil {
-					log.Error("Failed to deploy istio addons")
-					return err
-				}
-			}
 		}
 		// Create the ingress secret.
 		certDir := util.GetResourcePath("./tests/testdata/certs")
@@ -464,12 +465,6 @@ func (k *KubeInfo) Teardown() error {
 				log.Errorf("Failed to delete namespace %s", k.Namespace)
 				return err
 			}
-			if *multiClusterDir != "" {
-				if err := util.DeleteNamespace(k.Namespace, k.RemoteKubeConfig); err != nil {
-					log.Errorf("Failed to delete namespace %s on remote cluster", k.Namespace)
-					return err
-				}
-			}
 
 			// ClusterRoleBindings are not namespaced and need to be deleted separately
 			if _, err := util.Shell("kubectl get --kubeconfig=%s clusterrolebinding -o jsonpath={.items[*].metadata.name}"+
@@ -488,6 +483,11 @@ func (k *KubeInfo) Teardown() error {
 			}
 		}
 	}
+	if *multiClusterDir != "" {
+		if err := util.DeleteNamespace(k.Namespace, k.RemoteKubeConfig); err != nil {
+			log.Errorf("Failed to delete namespace %s on remote cluster", k.Namespace)
+		}
+	}
 
 	// confirm the namespace is deleted as it will cause future creation to fail
 	maxAttempts := 600
@@ -646,6 +646,13 @@ func (k *KubeInfo) deployIstio() error {
 		return err
 	}
 
+	if k.InstallAddons {
+		if err := k.deployAddons(); err != nil {
+			log.Error("Failed to deploy istio addons")
+			return err
+		}
+	}
+
 	if *multiClusterDir != "" {
 		// Create namespace on any remote clusters
 		if err := util.CreateNamespace(k.Namespace, k.RemoteKubeConfig); err != nil {
@@ -662,10 +669,8 @@ func (k *KubeInfo) deployIstio() error {
 			log.Infof("Failed to create Cacerts with namespace %s in remote cluster", k.Namespace)
 		}
 
-		yamlDir := filepath.Join(istioInstallDir, mcRemoteInstallFile)
-		baseIstioYaml := filepath.Join(k.ReleaseDir, yamlDir)
 		testIstioYaml := filepath.Join(k.TmpDir, "yaml", mcRemoteInstallFile)
-		if err := k.generateRemoteIstio(baseIstioYaml, testIstioYaml); err != nil {
+		if err := k.generateRemoteIstio(testIstioYaml, *useAutomaticInjection, *proxyHub, *proxyTag); err != nil {
 			log.Errorf("Generating Remote yaml %s failed", testIstioYaml)
 			return err
 		}

tests/e2e/framework/multicluster.go

@@ -67,40 +67,66 @@ func (k *KubeInfo) getEndpointIPForService(svc string) (ip string, err error) {
 	}
 
 	if err == nil && eps != nil {
-		if len(eps.Subsets[0].Addresses) != 0 {
-			ip = eps.Subsets[0].Addresses[0].IP
-		} else if len(eps.Subsets[0].NotReadyAddresses) != 0 {
-			ip = eps.Subsets[0].NotReadyAddresses[0].IP
-		} else {
+		if len(eps.Subsets) > 0 {
+			if len(eps.Subsets[0].Addresses) != 0 {
+				ip = eps.Subsets[0].Addresses[0].IP
+			} else if len(eps.Subsets[0].NotReadyAddresses) != 0 {
+				ip = eps.Subsets[0].NotReadyAddresses[0].IP
+			}
+		}
+		if ip == "" {
 			err = fmt.Errorf("could not get endpoint addresses for service %s", svc)
 			return "", err
 		}
 	}
 	return
 }
 
-func (k *KubeInfo) generateRemoteIstio(src, dst string) error {
-	content, err := ioutil.ReadFile(src)
-	if err != nil {
-		return fmt.Errorf("cannot read remote yaml file %s", src)
+func (k *KubeInfo) generateRemoteIstio(dst string, useAutoInject bool, proxyHub, proxyTag string) (err error) {
+	svcToHelmVal := map[string]string{
+		"istio-pilot":              "remotePilotAddress",
+		"istio-policy":             "remotePolicyAddress",
+		"istio-statsd-prom-bridge": "proxy.envoyStatsd.host",
+		"istio-ingressgateway":     "ingressGatewayEndpoint",
+		"istio-telemetry":          "remoteTelemetryAddress",
+		"zipkin":                   "remoteZipkinAddress",
 	}
-
-	svcs := []string{"istio-pilot", "istio-policy", "istio-statsd-prom-bridge", "istio-ingress", "istio-telemetry"}
-	for _, svc := range svcs {
+	var helmSetContent string
+	var ingressGatewayAddr string
+	for svc, helmVal := range svcToHelmVal {
 		var ip string
 		ip, err = k.getEndpointIPForService(svc)
 		if err == nil {
-			replaceStr := fmt.Sprintf("%s.istio-system", svc)
-			content = replacePattern(content, replaceStr, ip)
+			helmSetContent += " --set global." + helmVal + "=" + ip
 			log.Infof("Service %s has an endpoint IP %s", svc, ip)
+			if svc == "istio-statsd-prom-bridge" {
+				helmSetContent += " --set global.proxy.envoyStatsd.enabled=true"
+			}
+			if svc == "istio-ingressgateway" {
+				ingressGatewayAddr = ip
+			}
 		} else {
-			return err
+			log.Infof("Endpoint for service %s not found", svc)
 		}
 	}
-	content = replacePattern(content, istioSystem, k.Namespace)
-	err = ioutil.WriteFile(dst, content, 0600)
+	if !useAutoInject {
+		helmSetContent += " --set sidecarInjectorWebhook.enabled=false"
+		log.Infof("Remote cluster auto-sidecar injection disabled")
+	} else {
+		helmSetContent += " --set sidecarInjectorWebhook.enabled=true"
+		log.Infof("Remote cluster auto-sidecar injection enabled")
+	}
+	if proxyHub != "" && proxyTag != "" {
+		helmSetContent += " --set global.hub=" + proxyHub + " --set global.tag=" + proxyTag
+	}
+	chartDir := filepath.Join(k.ReleaseDir, "install/kubernetes/helm/istio-remote")
+	util.HelmTemplate(chartDir, "istio-remote", k.Namespace, helmSetContent, dst)
 	if err != nil {
-		return fmt.Errorf("cannot write remote into generated yaml file %s", dst)
+		log.Errorf("cannot write remote into generated yaml file %s: %v", dst, err)
+		return err
+	}
+	if ingressGatewayAddr != "" {
+		k.appendIngressGateway(dst, ingressGatewayAddr)
 	}
 	return nil
 }
@@ -123,3 +149,57 @@ func (k *KubeInfo) createCacerts(remoteCluster bool) (err error) {
 	}
 	return err
 }
+
+func (k *KubeInfo) appendIngressGateway(dst, ingressAddr string) (err error) {
+	var ingressGwSvc = `
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: istio-ingressgateway
+  namespace: %s
+spec:
+  type: ClusterIP
+  clusterIP: None
+  ports:
+    -
+      name: http2
+      port: 80
+    -
+      name: https
+      port: 443
+    -
+      name: tcp
+      port: 31400
+---
+apiVersion: v1
+kind: Endpoints
+metadata:
+  name: istio-ingressgateway
+  namespace: istio-system
+
+subsets:
+- addresses:
+  - ip: %s
+  ports:
+    -
+      name: http2
+      port: 80
+    -
+      name: https
+      port: 443
+    -
+      name: tcp
+      port: 31400
+`
+	f, err := os.OpenFile(dst, os.O_APPEND|os.O_WRONLY, 0644)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+
+	if _, err = f.WriteString(fmt.Sprintf(ingressGwSvc, k.Namespace, ingressAddr)); err != nil {
+		return err
+	}
+	return nil
+}

tests/e2e/tests/pilot/ingressgateway_test.go

@@ -68,7 +68,7 @@ func TestGateway_HTTPIngress(t *testing.T) {
 			for _, elt := range resp.Version {
 				count[elt] = count[elt] + 1
 			}
-			log.Infof("request counts %v", count)
+			log.Infof("request counts %+v", count)
 			if count["v2"] >= 95 {
 				return nil
 			}
@@ -103,7 +103,7 @@ func TestGateway_HTTPSIngress(t *testing.T) {
 			for _, elt := range resp.Version {
 				count[elt] = count[elt] + 1
 			}
-			log.Infof("request counts %v", count)
+			log.Infof("request counts %+v", count)
 			if count["v2"] >= 95 {
 				return nil
 			}
@@ -138,7 +138,7 @@ func TestGateway_TCPIngress(t *testing.T) {
 			for _, elt := range resp.Version {
 				count[elt] = count[elt] + 1
 			}
-			log.Infof("request counts %v", count)
+			log.Infof("request counts %+v", count)
 			if count["v1"] >= 95 {
 				return nil
 			}
@@ -254,7 +254,7 @@ cleanup:
 			}
 			if count["200"] != len(resp.Code) {
 				// have entries other than 200
-				t.Errorf("Got non 200 status code while changing rules: %v", count)
+				t.Errorf("Got non 200 status code while changing rules: %+v", count)
 			} else {
 				log.Infof("No 503s were encountered while changing rules (total %d requests)", len(resp.Code))
 			}

tests/util/kube_utils.go

@@ -192,6 +192,14 @@ func HelmInstall(chartDir, chartName, namespace, setValue string) error {
 	return err
 }
 
+// HelmTemplate helm template from a chart for a given namespace
+//      --set stringArray        set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)
+func HelmTemplate(chartDir, chartName, namespace, setValue, outfile string) error {
+	_, err := Shell("helm template %s --name %s --namespace %s %s > %s", chartDir,
+		chartName, namespace, setValue, outfile)
+	return err
+}
+
 // HelmDelete helm del --purge a chart
 func HelmDelete(chartName string) error {
 	_, err := Shell("helm del --purge %s", chartName)