[Test Framework] Fixes to support multicluster (#8253)

* [Test Framework] Fixes to support multicluster

* use current context from each kubeconfig file.

* addressing comments

content/en/docs/tasks/observability/distributed-tracing/zipkin/multicluster_test.sh

@@ -1,5 +1,5 @@
 #!/usr/bin/env bash
-# shellcheck disable=SC1090,SC2154,SC2155,SC2034
+# shellcheck disable=SC1090,SC2154,SC2155,SC2031,SC2034
 
 # Copyright Istio Authors
 #
@@ -21,14 +21,16 @@ set -o pipefail
 
 # @setup multicluster
 
+# Initialize KUBECONFIG_FILES
+_set_kube_vars
+
 # TODO: Remove this once we have a real multicluster test
 
 echo "start multicluster test with KUBECONFIG=${KUBECONFIG}"
 echo "DOCTEST_KUBECONFIG=${DOCTEST_KUBECONFIG}"
 echo "DOCTEST_NETWORK_TOPOLOGY=${DOCTEST_NETWORK_TOPOLOGY}"
 
-IFS=',' read -r -a CONFIGS <<< "${KUBECONFIG}"
-for kcfg in "${CONFIGS[@]}"; do
+for kcfg in "${KUBECONFIG_FILES[@]}"; do
   kubectl --kubeconfig="$kcfg" get pods -A
 done
 

pkg/test/istioio/framework.go

@@ -55,8 +55,8 @@ var (
 		source "tests/util/helpers.sh"
 	`
 
-	clusterSnapshot = `
-		__cluster_snapshot
+	createClusterSnapshots = `
+		__create_cluster_snapshots
 	`
 
 	clusterCleanupCheck = `
@@ -155,7 +155,7 @@ func checkFile(path string) (*TestCase, error) {
 	config := setups[0][1]
 
 	// Check for proper test cleanup
-	testScript = clusterSnapshot + testScript
+	testScript = createClusterSnapshots + testScript
 	cleanupScript += clusterCleanupCheck
 
 	testCase = &TestCase{

pkg/test/istioio/script.go

@@ -15,7 +15,6 @@
 package istioio
 
 import (
-	"bytes"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -35,6 +34,11 @@ const (
 	kubeConfigEnvVar    = "KUBECONFIG"
 )
 
+var (
+	// Logging scope for the script output.
+	scriptLog = log.RegisterScope("script", "output of test scripts", 0)
+)
+
 var _ Step = Script{}
 
 // Script is a test Step that executes a shell script.
@@ -73,6 +77,7 @@ func (s Script) run(ctx Context) {
 	}
 
 	// Now run the command...
+	defer scopes.Framework.Infof("Finished running command script %s", input.Name())
 	scopes.Framework.Infof("Running command script %s", input.Name())
 
 	// Copy the command to workDir.
@@ -93,27 +98,23 @@ func (s Script) run(ctx Context) {
 	cmd.Env = s.getEnv(ctx, fileName)
 	cmd.Stdin = strings.NewReader(command)
 
-	// Output will be streamed to logs as well as to the output buffer (to be written to disk)
-	var output bytes.Buffer
-	cmd.Stdout = io.MultiWriter(&LogWriter{}, &output)
-	cmd.Stderr = io.MultiWriter(&LogWriter{}, &output)
-
-	// Run the command and get the output.
-	cmdErr := cmd.Run()
-
-	// Copy the command output from the script to workDir
-	outputFileName := fileName + "_output.txt"
-	if werr := ioutil.WriteFile(filepath.Join(ctx.WorkDir(), outputFileName), bytes.TrimSpace(output.Bytes()), 0644); werr != nil {
-		ctx.Fatalf("failed copying output for command %s: %v", input.Name(), werr)
+	// Output will be streamed to logs as well as to the output file (to be written to disk)
+	outputFileName := filepath.Join(ctx.WorkDir(), fileName+"_output.txt")
+	outputFile, err := os.Create(outputFileName)
+	if err != nil {
+		ctx.Fatalf("failed creating output file for command %s: %v", input.Name(), err)
 	}
+	defer func() { _ = outputFile.Close() }()
+	cmd.Stdout = io.MultiWriter(&LogWriter{}, outputFile)
+	cmd.Stderr = io.MultiWriter(&LogWriter{}, outputFile)
 
-	if cmdErr != nil {
-		ctx.Fatalf("script %s returned an error: %v. Output:\n%s", input.Name(), cmdErr, output.String())
+	// Run the command.
+	if err := cmd.Run(); err != nil {
+		ctx.Fatalf("error running script %s: %v. Check output file for details: %s",
+			input.Name(), err, outputFileName)
 	}
 }
 
-var scriptLog = log.RegisterScope("script", "output of test scripts", 0)
-
 type LogWriter struct{}
 
 func (l LogWriter) Write(p []byte) (n int, err error) {
@@ -143,7 +144,7 @@ func (s Script) getEnv(ctx Context, fileName string) []string {
 	customVars[testDebugFile] = fileName + "_debug.txt"
 
 	if ctx.TestContext.Clusters().IsMulticluster() {
-		customVars[kubeConfigEnvVar] = strings.Join(ctx.KubeEnv().Settings().KubeConfig, ",")
+		customVars[kubeConfigEnvVar] = strings.Join(ctx.KubeEnv().Settings().KubeConfig, ":")
 	} else {
 		customVars[kubeConfigEnvVar] = ctx.KubeEnv().Settings().KubeConfig[0]
 	}

tests/README.md

@@ -292,3 +292,7 @@ error as the Istio control plane is being started. Adding a config when creating
 
 1. Set the HUB and TAG environment variables to use a particular Istio build when running tests.
    If unset, their default values will match those used by the prow tests.
+
+1. For help debugging, you can enable script output to the `stdout` with the command-line flag
+   `--log_output_level=script:debug`. This is useful when you're running in an IDE and don't
+   want to find and tail the test output files.

tests/setup/multicluster/doc_test.go

@@ -31,7 +31,7 @@ func TestMain(m *testing.M) {
 	}
 	framework.
 		NewSuite(m).
-		RequireMinClusters(2).
+		RequireMinClusters(3).
 		Run()
 }
 

tests/util/helpers.sh

@@ -1,5 +1,5 @@
 #!/bin/bash
-# shellcheck disable=SC2155
+# shellcheck disable=SC2155,SC2030,SC2031
 
 # Copyright Istio Authors
 #
@@ -15,6 +15,37 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# _set_kube_vars initializes the following variables based on the value of KUBECONFIG:
+#
+# KUBECONFIG_FILES: an array containing the individual files in the order specified.
+# KUBE_CONTEXTS: the names of the kube contexts, in the order of the KUBECONFIG files specified.
+function _set_kube_vars()
+{
+  # Split out the kube config files and then get the current context in
+  # each. We do this because the contexts are stored in a map, which
+  # means that order of the context returned by
+  # `kubectl config get-contexts` is not guaranteed. By pulling out
+  # the context on a per-file basis, we maintain the order of the
+  # files in the KUBECONFIG variable.
+  KUBE_CONTEXTS=()
+  IFS=':' read -r -a KUBECONFIG_FILES <<< "${KUBECONFIG}"
+  for KUBECONFIG_FILE in "${KUBECONFIG_FILES[@]}"; do
+    CTX="$(export KUBECONFIG=$KUBECONFIG_FILE; kubectl config current-context)"
+    if [[ -z "${CTX}" ]]; then
+      echo "${KUBECONFIG_FILE} contains no current context"
+      exit 1
+    fi
+    KUBE_CONTEXTS+=("${CTX}")
+  done
+
+  export KUBECONFIG_FILES
+  export KUBE_CONTEXTS
+
+  echo "KUBECONFIG=${KUBECONFIG}"
+  echo "KUBECONFIG_FILES=${KUBECONFIG_FILES[*]}"
+  echo "KUBE_CONTEXTS=${KUBE_CONTEXTS[*]}"
+}
+
 # Set the INGRESS_HOST, INGRESS_PORT, SECURE_INGRESS_PORT, and TCP_INGRESS_PORT environment variables
 _set_ingress_environment_variables() {
     # check for external load balancer

tests/util/verify.sh

@@ -1,4 +1,5 @@
 #!/bin/bash
+# shellcheck disable=SC2030,SC2031
 
 # Copyright Istio Authors
 #
@@ -264,7 +265,7 @@ __verify_with_retry() {
 # Get the resource state of the cluster. Used by the test framework to compare the
 # cluster state before and after running each test:
 #
-# __cluster_snapshot
+# __cluster_cluster_snapshots
 #
 # ... test commands
 # ... cleanup commands
@@ -280,30 +281,59 @@ __cluster_state() {
     # TEMP WORKAROUND, don't check istio-system
     kubectl get ns -o name | sed '/istio-system/d'
     kubectl get all --ignore-not-found -n default
-    kubectl get istiooperators --ignore-not-found -n default 2>&1 | grep -v "error: the server doesn't have a resource type"
-    kubectl get destinationrules --ignore-not-found -n default -n istio-system 2>&1 | grep -v "error: the server doesn't have a resource type"
-    kubectl get envoyfilters --ignore-not-found -n default -n istio-system 2>&1 | grep -v "error: the server doesn't have a resource type"
-    kubectl get gateways --ignore-not-found -n default -n istio-system  2>&1| grep -v "error: the server doesn't have a resource type"
-    kubectl get serviceentries --ignore-not-found -n default -n istio-system 2>&1 | grep -v "error: the server doesn't have a resource type"
-    kubectl get sidecars --ignore-not-found -n default -n istio-system 2>&1 | grep -v "error: the server doesn't have a resource type"
-    kubectl get virtualservices --ignore-not-found -n default -n istio-system 2>&1 | grep -v "error: the server doesn't have a resource type"
-    kubectl get workloadentries --ignore-not-found -n default -n istio-system 2>&1 | grep -v "error: the server doesn't have a resource type"
-    kubectl get authorizationpolicies --ignore-not-found -n default -n istio-system 2>&1 | grep -v "error: the server doesn't have a resource type"
-    kubectl get peerauthentications --ignore-not-found -n default -n istio-system 2>&1 | grep -v "error: the server doesn't have a resource type"
-    kubectl get requestauthentications --ignore-not-found -n default -n istio-system 2>&1 | grep -v "error: the server doesn't have a resource type"
+    kubectl get istiooperators --ignore-not-found -n default 2>&1
+    kubectl get destinationrules --ignore-not-found -n default -n istio-system 2>&1
+    kubectl get envoyfilters --ignore-not-found -n default -n istio-system 2>&1
+    kubectl get gateways --ignore-not-found -n default -n istio-system  2>&1
+    kubectl get serviceentries --ignore-not-found -n default -n istio-system 2>&1
+    kubectl get sidecars --ignore-not-found -n default -n istio-system 2>&1
+    kubectl get virtualservices --ignore-not-found -n default -n istio-system 2>&1
+    kubectl get workloadentries --ignore-not-found -n default -n istio-system 2>&1
+    kubectl get authorizationpolicies --ignore-not-found -n default -n istio-system 2>&1
+    kubectl get peerauthentications --ignore-not-found -n default -n istio-system 2>&1
+    kubectl get requestauthentications --ignore-not-found -n default -n istio-system 2>&1
 }
 
-__cluster_snapshot() {
-    __cluster_state > __cluster_snapshot.txt 2>&1
+__create_cluster_snapshots() {
+    # Get the list of KUBECONFIG files as an array.
+    IFS=':' read -r -a KFILES <<< "${KUBECONFIG}"
+    for KFILE in "${KFILES[@]}"; do
+        # Get the contexts in this KUBECONFIG file as an array.
+        CTX="$(export KUBECONFIG=${KFILE}; kubectl config current-context)"
+        if [[ -z "${CTX}" ]]; then
+          echo "${KFILE} contains no current context."
+          exit 1
+        fi
+
+        # Dump the state of this cluster to a snapshot file.
+        SNAPSHOT_FILE="__cluster_snapshot_${CTX}.txt"
+        echo "Creating snapshot ${SNAPSHOT_FILE}"
+        (KUBECONFIG="${KFILE}"; __cluster_state > "${SNAPSHOT_FILE}" 2>&1)
+    done
 }
 
 __cluster_cleanup_check() {
-    # shellcheck disable=SC2034
-    snapshot=$(<__cluster_snapshot.txt)
-    rm __cluster_snapshot.txt
+    # Get the list of KUBECONFIG files as an array.
+    IFS=':' read -r -a KFILES <<< "${KUBECONFIG}"
+    for KFILE in "${KFILES[@]}"; do
+        # Get the contexts in this KUBECONFIG file as an array.
+        CTX="$(export KUBECONFIG=${KFILE}; kubectl config current-context)"
+        if [[ -z "${CTX}" ]]; then
+          echo "${KFILE} contains no current context."
+          exit 1
+        fi
 
-    VERIFY_RETRIES=9
-    _verify_like __cluster_state "$snapshot"
+        # Read the snapshot file for this cluster.
+        SNAPSHOT_FILE="__cluster_snapshot_${CTX}.txt"
+        echo "Performing cleanup check against snapshot ${SNAPSHOT_FILE}"
+        SNAPSHOT=$(<"${SNAPSHOT_FILE}")
+        rm "${SNAPSHOT_FILE}"
+
+        # Verify that we've restored the original cluster state.
+        VERIFY_RETRIES=9
+        (KUBECONFIG="${KFILE}"; _verify_like __cluster_state "${SNAPSHOT}")
+        echo "Finished cleanup check against snapshot ${SNAPSHOT_FILE}"
+    done
 }