Migrate upgrade tests to the new framework (knative#10216)

* Migrate upgrade tests to using pkg/test/upgrade framework

* Convert AssertAutoscaleUpToNumPods and inner functions to helper
functions that
return error instead of calling t.Fatal to fail the current test.
Avoid using *testing.T
so that these functions can be reused outside of tests or span
multiple tests.
* The autoscaler helper functions are used in upgrade tests where
"setup" and
"verify" phases run within different tests. Pull test.EnsureTearDown
from SetupSvc to ensure
that a kservice is not destroyed at the end of the first phase
("setup") but
remains active until "verify" phase. This is ensured by calling
EnsureTearDown later in the
  "verify" phase.
* Adjust Bash scripts to avoid unbound variable errors during upgrade
tests exexution (due
  to using -u flag by the upgrade framework).

* Use umbrella functions for individual groups of tests

That's for easy reuse in other repos such as knative/operator

* Define curPods and targetPods constants in upgrade tests

* Fix imports and licences

* Fix lint - comments on new exported functions

* Split AssertAutoscaleUpToNumPods into setup and wait parts

* Pass errgroup by reference

* Fix lint error

* Move logf into TestContext

* Remaining fixes for moving logf to TestContext

* Call logf on context directly

* Update readme

* Remove the comment and fix imports

* Update modules after running update-codegen

* Do not upgrade Ingress when upgrading Knative

* autoscale and prober tests will fail if ingress is replaced during
upgrade

* Pass around func() error instead of errGroup

* Mark a few functions as t.Helper()

* Fix codegen

hack/generate-yamls.sh

@@ -66,6 +66,7 @@ readonly CONSOLIDATED_ARTIFACTS
 
 # Flags for all ko commands
 KO_YAML_FLAGS="-P"
+KO_FLAGS="${KO_FLAGS:-}"
 [[ "${KO_DOCKER_REPO}" != gcr.io/* ]] && KO_YAML_FLAGS=""
 
 if [[ "${KO_FLAGS}" != *"--platform"* ]]; then
@@ -74,7 +75,7 @@ fi
 
 readonly KO_YAML_FLAGS="${KO_YAML_FLAGS} ${KO_FLAGS}"
 
-if [[ -n "${TAG}" ]]; then
+if [[ -n "${TAG:-}" ]]; then
   LABEL_YAML_CMD=(sed -e "s|serving.knative.dev/release: devel|serving.knative.dev/release: \"${TAG}\"|")
 else
   LABEL_YAML_CMD=(cat)

test/e2e-common.sh

@@ -15,8 +15,9 @@
 # limitations under the License.
 
 # This script provides helper methods to perform cluster actions.
-source $(dirname $0)/../vendor/knative.dev/hack/e2e-tests.sh
-source $(dirname $0)/e2e-networking-library.sh
+# shellcheck disable=SC1090
+source "$(dirname "${BASH_SOURCE[0]}")/../vendor/knative.dev/hack/e2e-tests.sh"
+source "$(dirname "${BASH_SOURCE[0]}")/e2e-networking-library.sh"
 
 CERT_MANAGER_VERSION="latest"
 # Since default is istio, make default ingress as istio
@@ -39,19 +40,27 @@ MESH=0
 INSTALL_CUSTOM_YAMLS=""
 
 UNINSTALL_LIST=()
-TMP_DIR=$(mktemp -d -t ci-$(date +%Y-%m-%d-%H-%M-%S)-XXXXXXXXXX)
-readonly TMP_DIR
+export TMP_DIR
+TMP_DIR="${TMP_DIR:-$(mktemp -d -t ci-$(date +%Y-%m-%d-%H-%M-%S)-XXXXXXXXXX)}"
 readonly KNATIVE_DEFAULT_NAMESPACE="knative-serving"
 # This the namespace used to install Knative Serving. Use generated UUID as namespace.
 export SYSTEM_NAMESPACE
-SYSTEM_NAMESPACE=$(uuidgen | tr 'A-Z' 'a-z')
+SYSTEM_NAMESPACE="${SYSTEM_NAMESPACE:-$(uuidgen | tr 'A-Z' 'a-z')}"
 
 
 # Keep this in sync with test/ha/ha.go
 readonly REPLICAS=3
 readonly BUCKETS=10
 HA_COMPONENTS=()
 
+# Latest serving release. If user does not supply this as a flag, the latest
+# tagged release on the current branch will be used.
+LATEST_SERVING_RELEASE_VERSION=$(latest_version)
+
+# Latest net-istio release.
+LATEST_NET_ISTIO_RELEASE_VERSION=$(
+  curl -L --silent "https://api.github.com/repos/knative/net-istio/releases" | grep '"tag_name"' \
+    | cut -f2 -d: | sed "s/[^v0-9.]//g" | sort | tail -n1)
 
 # Parse our custom flags.
 function parse_flags() {
@@ -141,14 +150,15 @@ function parse_flags() {
 # All generated YAMLs will be available and pointed by the corresponding
 # environment variables as set in /hack/generate-yamls.sh.
 function build_knative_from_source() {
-  local YAML_LIST="$(mktemp)"
+  local FULL_OUTPUT YAML_LIST LOG_OUTPUT ENV_OUTPUT
+  YAML_LIST="$(mktemp)"
 
   # Generate manifests, capture environment variables pointing to the YAML files.
-  local FULL_OUTPUT="$( \
-      source $(dirname $0)/../hack/generate-yamls.sh ${REPO_ROOT_DIR} ${YAML_LIST} ; \
+  FULL_OUTPUT="$( \
+      source "$(dirname "${BASH_SOURCE[0]}")/../hack/generate-yamls.sh" "${REPO_ROOT_DIR}" "${YAML_LIST}" ; \
       set | grep _YAML=/)"
-  local LOG_OUTPUT="$(echo "${FULL_OUTPUT}" | grep -v _YAML=/)"
-  local ENV_OUTPUT="$(echo "${FULL_OUTPUT}" | grep '^[_0-9A-Z]\+_YAML=/')"
+  LOG_OUTPUT="$(echo "${FULL_OUTPUT}" | grep -v _YAML=/)"
+  ENV_OUTPUT="$(echo "${FULL_OUTPUT}" | grep '^[_0-9A-Z]\+_YAML=/')"
   [[ -z "${LOG_OUTPUT}" || -z "${ENV_OUTPUT}" ]] && fail_test "Error generating manifests"
   # Only import the environment variables pointing to the YAML files.
   echo "${LOG_OUTPUT}"
@@ -164,7 +174,7 @@ function build_knative_from_source() {
 function install_knative_serving() {
   local version=${1:-"HEAD"}
   if [[ -z "${INSTALL_CUSTOM_YAMLS}" ]]; then
-    install_knative_serving_standard "$version" "$2"
+    install_knative_serving_standard "$version" "${2:-}"
     return
   fi
   echo ">> Installing Knative serving from custom YAMLs"
@@ -227,29 +237,31 @@ function install_knative_serving_standard() {
     ko apply -f "${SERVING_RELEASE_YAML}" --selector=knative.dev/crd-install=true || return 1
   fi
 
-  echo ">> Installing Ingress"
-  if [[ -n "${GLOO_VERSION}" ]]; then
-    install_gloo || return 1
-  elif [[ -n "${KOURIER_VERSION}" ]]; then
-    install_kourier || return 1
-  elif [[ -n "${AMBASSADOR_VERSION}" ]]; then
-    install_ambassador || return 1
-  elif [[ -n "${CONTOUR_VERSION}" ]]; then
-    install_contour || return 1
-  elif [[ -n "${KONG_VERSION}" ]]; then
-    install_kong || return 1
-  else
-    if [[ "$1" == "HEAD" ]]; then
-      install_istio "./third_party/istio-latest/net-istio.yaml" || return 1
+  if [[ -z "${REUSE_INGRESS:-}" ]]; then
+    echo ">> Installing Ingress"
+    if [[ -n "${GLOO_VERSION:-}" ]]; then
+      install_gloo || return 1
+    elif [[ -n "${KOURIER_VERSION:-}" ]]; then
+      install_kourier || return 1
+    elif [[ -n "${AMBASSADOR_VERSION:-}" ]]; then
+      install_ambassador || return 1
+    elif [[ -n "${CONTOUR_VERSION:-}" ]]; then
+      install_contour || return 1
+    elif [[ -n "${KONG_VERSION:-}" ]]; then
+      install_kong || return 1
     else
-      # Download the latest release of net-istio.
-      local url="https://github.com/knative/net-istio/releases/download/${LATEST_NET_ISTIO_RELEASE_VERSION}"
-      local yaml="net-istio.yaml"
-      local YAML_NAME=${TMP_DIR}/"net-istio-${LATEST_NET_ISTIO_RELEASE_VERSION}.yaml"
-      wget "${url}/${yaml}" -O "${YAML_NAME}" \
-        || fail_test "Unable to download latest knative/net-istio release."
-      echo "net-istio YAML: ${YAML_NAME}"
-      install_istio $YAML_NAME || return 1
+      if [[ "$1" == "HEAD" ]]; then
+        install_istio "./third_party/istio-latest/net-istio.yaml" || return 1
+      else
+        # Download the latest release of net-istio.
+        local url="https://github.com/knative/net-istio/releases/download/${LATEST_NET_ISTIO_RELEASE_VERSION}"
+        local yaml="net-istio.yaml"
+        local YAML_NAME=${TMP_DIR}/"net-istio-${LATEST_NET_ISTIO_RELEASE_VERSION}.yaml"
+        wget "${url}/${yaml}" -O "${YAML_NAME}" \
+          || fail_test "Unable to download latest knative/net-istio release."
+        echo "net-istio YAML: ${YAML_NAME}"
+        install_istio $YAML_NAME || return 1
+      fi
     fi
   fi
 
@@ -511,3 +523,27 @@ function disable_chaosduck() {
 function enable_chaosduck() {
   kubectl -n "${SYSTEM_NAMESPACE}" scale deployment "chaosduck" --replicas=1 || fail_test
 }
+
+function install_latest_release() {
+  header "Installing Knative latest public release"
+
+  install_knative_serving latest-release \
+      || fail_test "Knative latest release installation failed"
+  test_logging_config_setup
+  wait_until_pods_running ${SYSTEM_NAMESPACE}
+  wait_until_batch_job_complete ${SYSTEM_NAMESPACE}
+}
+
+function install_head_reuse_ingress() {
+  header "Installing Knative head release and reusing ingress"
+  # Keep the existing ingress and do not upgrade it. The ingress upgrade
+  # makes ongoing requests fail.
+  REUSE_INGRESS=true install_knative_serving || fail_test "Knative head release installation failed"
+  test_logging_config_setup
+  wait_until_pods_running ${SYSTEM_NAMESPACE}
+  wait_until_batch_job_complete ${SYSTEM_NAMESPACE}
+}
+
+function knative_setup() {
+  install_latest_release
+}

test/e2e-networking-library.sh

@@ -15,17 +15,17 @@
 # limitations under the License.
 
 function install_istio() {
-  if [[ -z "${ISTIO_VERSION}" ]]; then
+  if [[ -z "${ISTIO_VERSION:-}" ]]; then
     readonly ISTIO_VERSION="stable"
   fi
 
-  if [[ -z "${NET_ISTIO_COMMIT}" ]]; then
+  if [[ -z "${NET_ISTIO_COMMIT:-}" ]]; then
     NET_ISTIO_COMMIT=$(head -n 1 ${1} | grep "# Generated when HEAD was" | sed 's/^.* //')
     echo "Got NET_ISTIO_COMMIT from ${1}: ${NET_ISTIO_COMMIT}"
   fi
 
   # TODO: remove this when all the net-istio.yaml in use contain a commit ID
-  if [[ -z "${NET_ISTIO_COMMIT}" ]]; then
+  if [[ -z "${NET_ISTIO_COMMIT:-}" ]]; then
     NET_ISTIO_COMMIT="8102cd3d32f05be1c58260a9717d532a4a6d2f60"
     echo "Hard coded NET_ISTIO_COMMIT: ${NET_ISTIO_COMMIT}"
   fi
@@ -41,7 +41,7 @@ function install_istio() {
   )
 
   ISTIO_PROFILE="istio"
-  if [[ -n "$KIND" ]]; then
+  if [[ -n "${KIND:-}" ]]; then
     ISTIO_PROFILE+="-kind"
   else
     ISTIO_PROFILE+="-ci"
@@ -52,7 +52,7 @@ function install_istio() {
   ISTIO_PROFILE+="-mesh"
   ISTIO_PROFILE+=".yaml"
 
-  if [[ -n "$CLUSTER_DOMAIN" ]]; then
+  if [[ -n "${CLUSTER_DOMAIN:-}" ]]; then
     sed -ie "s/cluster\.local/${CLUSTER_DOMAIN}/g" ${NET_ISTIO_DIR}/third_party/istio-${ISTIO_VERSION}/${ISTIO_PROFILE}
   fi
 
@@ -61,7 +61,7 @@ function install_istio() {
   echo "Istio profile: ${ISTIO_PROFILE}"
   ${NET_ISTIO_DIR}/third_party/istio-${ISTIO_VERSION}/install-istio.sh ${ISTIO_PROFILE}
 
-  if [[ -n "$1" ]]; then
+  if [[ -n "${1:-}" ]]; then
     echo ">> Installing net-istio"
     echo "net-istio original YAML: ${1}"
     # Create temp copy in which we replace knative-serving by the test's system namespace.
@@ -161,35 +161,35 @@ function install_contour() {
 }
 
 function wait_until_ingress_running() {
-  if [[ -n "${ISTIO_VERSION}" ]]; then
+  if [[ -n "${ISTIO_VERSION:-}" ]]; then
     wait_until_pods_running istio-system || return 1
     wait_until_service_has_external_http_address istio-system istio-ingressgateway || return 1
   fi
-  if [[ -n "${GLOO_VERSION}" ]]; then
+  if [[ -n "${GLOO_VERSION:-}" ]]; then
     # we must set these override values to allow the test spoofing client to work with Gloo
     # see https://github.com/knative/pkg/blob/release-0.7/test/ingress/ingress.go#L37
     export GATEWAY_OVERRIDE=knative-external-proxy
     export GATEWAY_NAMESPACE_OVERRIDE=gloo-system
     wait_until_pods_running gloo-system || return 1
     wait_until_service_has_external_ip gloo-system knative-external-proxy
   fi
-  if [[ -n "${KOURIER_VERSION}" ]]; then
+  if [[ -n "${KOURIER_VERSION:-}" ]]; then
     # we must set these override values to allow the test spoofing client to work with Kourier
     # see https://github.com/knative/pkg/blob/release-0.7/test/ingress/ingress.go#L37
     export GATEWAY_OVERRIDE=kourier
     export GATEWAY_NAMESPACE_OVERRIDE=kourier-system
     wait_until_pods_running kourier-system || return 1
     wait_until_service_has_external_http_address kourier-system kourier
   fi
-  if [[ -n "${AMBASSADOR_VERSION}" ]]; then
+  if [[ -n "${AMBASSADOR_VERSION:-}" ]]; then
     # we must set these override values to allow the test spoofing client to work with Ambassador
     # see https://github.com/knative/pkg/blob/release-0.7/test/ingress/ingress.go#L37
     export GATEWAY_OVERRIDE=ambassador
     export GATEWAY_NAMESPACE_OVERRIDE=ambassador
     wait_until_pods_running ambassador || return 1
     wait_until_service_has_external_http_address ambassador ambassador
   fi
-  if [[ -n "${CONTOUR_VERSION}" ]]; then
+  if [[ -n "${CONTOUR_VERSION:-}" ]]; then
     # we must set these override values to allow the test spoofing client to work with Contour
     # see https://github.com/knative/pkg/blob/release-0.7/test/ingress/ingress.go#L37
     export GATEWAY_OVERRIDE=envoy
@@ -198,7 +198,7 @@ function wait_until_ingress_running() {
     wait_until_pods_running contour-internal || return 1
     wait_until_service_has_external_ip "${GATEWAY_NAMESPACE_OVERRIDE}" "${GATEWAY_OVERRIDE}"
   fi
-  if [[ -n "${KONG_VERSION}" ]]; then
+  if [[ -n "${KONG_VERSION:-}" ]]; then
     # we must set these override values to allow the test spoofing client to work with Kong
     # see https://github.com/knative/pkg/blob/release-0.7/test/ingress/ingress.go#L37
     export GATEWAY_OVERRIDE=kong-proxy

test/e2e-upgrade-tests.sh

@@ -28,39 +28,8 @@
 # You can specify the version to run against with the --version argument
 # (e.g. --version v0.7.0). If this argument is not specified, the script will
 # run against the latest tagged version on the current branch.
-
-source $(dirname $0)/e2e-common.sh
-
-# Latest serving release. If user does not supply this as a flag, the latest
-# tagged release on the current branch will be used.
-LATEST_SERVING_RELEASE_VERSION=$(latest_version)
-
-# Latest net-istio release.
-LATEST_NET_ISTIO_RELEASE_VERSION=$(
-  curl -L --silent "https://api.github.com/repos/knative/net-istio/releases" | grep '"tag_name"' \
-    | cut -f2 -d: | sed "s/[^v0-9.]//g" | sort | tail -n1)
-
-function install_latest_release() {
-  header "Installing Knative latest public release"
-
-  install_knative_serving latest-release \
-      || fail_test "Knative latest release installation failed"
-  test_logging_config_setup
-  wait_until_pods_running ${SYSTEM_NAMESPACE}
-  wait_until_batch_job_complete ${SYSTEM_NAMESPACE}
-}
-
-function install_head() {
-  header "Installing Knative head release"
-  install_knative_serving || fail_test "Knative head release installation failed"
-  test_logging_config_setup
-  wait_until_pods_running ${SYSTEM_NAMESPACE}
-  wait_until_batch_job_complete ${SYSTEM_NAMESPACE}
-}
-
-function knative_setup() {
-  install_latest_release
-}
+# shellcheck disable=SC1090
+source "$(dirname "${BASH_SOURCE[0]}")/e2e-common.sh"
 
 # Script entry point.
 
@@ -75,51 +44,14 @@ initialize "$@" --skip-istio-addon --min-nodes=4 --max-nodes=4
 disable_chaosduck
 
 # TODO(#2656): Reduce the timeout after we get this test to consistently passing.
-TIMEOUT=10m
-# Probe tests starts before postupgrade tests and ends after postdowngrade tests.
-# The timeout should be at least 10m + 10m + installation time
-PROBE_TIMEOUT=20m
+TIMEOUT=30m
 
-header "Running preupgrade tests"
+header "Running upgrade tests"
 
-go_test_e2e -tags=preupgrade -timeout=${TIMEOUT} ./test/upgrade \
+go_test_e2e -tags=upgrade -timeout=${TIMEOUT} \
+  ./test/upgrade \
   --resolvabledomain=$(use_resolvable_domain) || fail_test
 
-header "Starting prober test"
-
-# Remove the following files in case we failed to clean them up in an earlier test.
-rm -f /tmp/prober-signal
-rm -f /tmp/autoscaling-signal
-rm -f /tmp/autoscaling-tbc-signal
-
-go_test_e2e -tags=probe -timeout=${PROBE_TIMEOUT} ./test/upgrade \
-  --resolvabledomain=$(use_resolvable_domain) &
-PROBER_PID=$!
-echo "Prober PID is ${PROBER_PID}"
-
-install_head
-
-header "Running postupgrade tests"
-go_test_e2e -tags=postupgrade -timeout=${TIMEOUT} ./test/upgrade \
-  --resolvabledomain=$(use_resolvable_domain) || fail_test
-
-install_latest_release
-
-header "Running postdowngrade tests"
-go_test_e2e -tags=postdowngrade -timeout=${TIMEOUT} ./test/upgrade \
-  --resolvabledomain=$(use_resolvable_domain) || fail_test
-
-# The probe tests are blocking on the following files to know when it should exit.
-#
-# This is kind of gross. First attempt was to just send a signal to the go test,
-# but "go test" intercepts the signal and always exits with a non-zero code.
-echo "done" > /tmp/prober-signal
-echo "done" > /tmp/autoscaling-signal
-echo "done" > /tmp/autoscaling-tbc-signal
-
-header "Waiting for prober test"
-wait ${PROBER_PID} || fail_test "Prober failed"
-
 # Remove the kail log file if the test flow passes.
 # This is for preventing too many large log files to be uploaded to GCS in CI.
 rm "${KAIL_LOG_FILE}"