From 9cc05a31e8c0b3794cab8ab37dc9d9178cdd7c72 Mon Sep 17 00:00:00 2001 From: Mahamed Ali Date: Fri, 18 Aug 2023 18:51:17 +0300 Subject: [PATCH] Deprecate calling kubetest2 via kntest (#230) * deprecate calling kubetest2 via kntest * fix e2e test parameters * accept some shellcheck suggestions and fix golang typo * fix performance tests script too * move addons to infra library and put flags on a separate line * cleanup unused test --------- Co-authored-by: upodroid --- e2e-tests.sh | 9 ++------ infra-library.sh | 41 ++++++++++++++++++++++++++++--------- performance-tests.sh | 16 +++++++++------ presubmit-tests.sh | 2 +- test/e2e-tests.sh | 4 ++-- test/unit/presubmit_test.go | 21 ++----------------- test/unit/sharedlib_test.go | 1 + 7 files changed, 49 insertions(+), 45 deletions(-) diff --git a/e2e-tests.sh b/e2e-tests.sh index 96290f9f..331511cc 100755 --- a/e2e-tests.sh +++ b/e2e-tests.sh @@ -19,8 +19,6 @@ source "$(dirname "${BASH_SOURCE[0]:-$0}")/infra-library.sh" -readonly TEST_RESULT_FILE=/tmp/${REPO_NAME}-e2e-result - # Tear down the test resources. function teardown_test_resources() { header "Tearing down test environment" @@ -133,6 +131,7 @@ E2E_SCRIPT="" function initialize() { local run_tests=0 local custom_flags=() + local extra_gcloud_flags=() local parse_script_flags=0 E2E_SCRIPT="$(get_canonical_path "$0")" local e2e_script_command=( "${E2E_SCRIPT}" "--run-tests" ) @@ -177,16 +176,12 @@ function initialize() { shift done - if [[ "${CLOUD_PROVIDER}" == "gke" ]]; then - custom_flags+=("--addons=NodeLocalDNS") - fi - readonly SKIP_DUMP_ON_FAILURE readonly TEARDOWN readonly CLOUD_PROVIDER if (( ! run_tests )); then - create_test_cluster "${CLOUD_PROVIDER}" custom_flags e2e_script_command + create_test_cluster "${CLOUD_PROVIDER}" custom_flags extra_gcloud_flags e2e_script_command else setup_test_cluster fi diff --git a/infra-library.sh b/infra-library.sh index 84a87e2a..4782d09a 100644 --- a/infra-library.sh +++ b/infra-library.sh @@ -91,7 +91,7 @@ function create_test_cluster() { fi case "$1" in - gke) create_gke_test_cluster "$2" "$3" "${4:-}" ;; + gke) create_gke_test_cluster "$2" "$3" "$4" "${5:-}" ;; kind) create_kind_test_cluster "$2" "$3" "${4:-}" ;; *) echo "unsupported provider: $1"; exit 1 ;; esac @@ -117,29 +117,50 @@ function create_kind_test_cluster() { } # Create a GKE test cluster with kubetest2 and run the test command. -# Parameters: $1 - custom flags defined in kntest -# $2 - test command to run after the cluster is created (optional) +# Parameters: $1 - custom flags defined in kubetest2 +# $2 - custom flags to pass directly to gcloud +# $3 - test command to run after the cluster is created (optional) function create_gke_test_cluster() { local -n _custom_flags=$1 - local -n _test_command=$2 + local -n _extra_gcloud_flags=$2 + local -n _test_command=$3 # We are disabling logs and metrics on Boskos Clusters by default as they are not used. Manually set ENABLE_GKE_TELEMETRY to true to enable telemetry # and ENABLE_PREEMPTIBLE_NODES to true to create preemptible/spot VMs. VM Preemption is a rare event and shouldn't be distruptive given the fault tolerant nature of our tests. - local extra_gcloud_flags="" if [[ "${ENABLE_GKE_TELEMETRY:-}" != "true" ]]; then - extra_gcloud_flags="${extra_gcloud_flags} --logging=NONE --monitoring=NONE" + _extra_gcloud_flags+=("--logging=NONE --monitoring=NONE") + fi + + if [[ "${CLOUD_PROVIDER}" == "gke" ]]; then + extra_gcloud_flags+=("--addons=NodeLocalDNS") fi if [[ "${ENABLE_PREEMPTIBLE_NODES:-}" == "true" ]]; then - extra_gcloud_flags="${extra_gcloud_flags} --preemptible" + _extra_gcloud_flags+=("--preemptible") fi + + _extra_gcloud_flags+=("--quiet") if ! command -v kubetest2 >/dev/null; then tmpbin="$(mktemp -d)" echo "kubetest2 not found, installing in temp path: ${tmpbin}" GOBIN="$tmpbin" go install sigs.k8s.io/kubetest2/...@latest export PATH="${tmpbin}:${PATH}" fi - run_kntest kubetest2 gke "${_custom_flags[@]}" \ - --test-command="${_test_command[*]}" \ - --extra-gcloud-flags="${extra_gcloud_flags}" + if [[ ! " ${_custom_flags[*]} " =~ "--machine-type=" ]]; then + _custom_flags+=("--machine-type=e2-standard-4") + fi + kubetest2 gke "${_custom_flags[@]}" \ + --rundir-in-artifacts \ + --up \ + --down \ + --boskos-heartbeat-interval-seconds=20 \ + --v=1 \ + --network=e2e-network \ + --boskos-acquire-timeout-seconds=1200 \ + --region="${E2E_CLUSTER_REGION},us-east1,us-west1" \ + --gcloud-extra-flags="${_extra_gcloud_flags[*]}" \ + --retryable-error-patterns='.*does not have enough resources available to fulfill.*,.*only \\d+ nodes out of \\d+ have registered; this is likely due to Nodes failing to start correctly.*,.*All cluster resources were brought up.+ but: component .+ from endpoint .+ is unhealthy.*' \ + --test=exec \ + -- \ + "${_test_command[@]}" } diff --git a/performance-tests.sh b/performance-tests.sh index 440668de..23e17e5b 100755 --- a/performance-tests.sh +++ b/performance-tests.sh @@ -17,7 +17,7 @@ # This is a helper script for Knative performance test scripts. # See README.md for instructions on how to use it. -source $(dirname "${BASH_SOURCE[0]}")/library.sh +source "$(dirname "${BASH_SOURCE[0]}")"/library.sh # Configurable parameters. # If not provided, they will fall back to the default values. @@ -76,7 +76,8 @@ EOF update_knative || abort "failed to update knative" fi # get benchmark name from the cluster name - local benchmark_name=$(get_benchmark_name "$1") + local benchmark_name + benchmark_name=$(get_benchmark_name "$1") if function_exists update_benchmark; then update_benchmark "${benchmark_name}" || abort "failed to update benchmark" fi @@ -92,13 +93,16 @@ function get_benchmark_name() { # Update the clusters related to the current repo. function update_clusters() { header "Updating all clusters for ${REPO_NAME}" - local all_clusters=$(gcloud container clusters list --project="${PROJECT_NAME}" --format="csv[no-heading](name,zone)") + local all_clusters + all_clusters=$(gcloud container clusters list --project="${PROJECT_NAME}" --format="csv[no-heading](name,zone)") echo ">> Project contains clusters:" "${all_clusters}" for cluster in ${all_clusters}; do - local name=$(echo "${cluster}" | cut -f1 -d",") + local name + name=$(echo "${cluster}" | cut -f1 -d",") # the cluster name is prefixed with "${REPO_NAME}--", here we should only handle clusters belonged to the current repo [[ ! ${name} =~ ^${REPO_NAME}-- ]] && continue - local zone=$(echo "${cluster}" | cut -f2 -d",") + local zone + zone=$(echo "${cluster}" | cut -f2 -d",") # Update all resources installed on the cluster update_cluster "${name}" "${zone}" @@ -109,7 +113,7 @@ function update_clusters() { # Run the perf-tests tool # Parameters: $1..$n - parameters passed to the tool function run_perf_cluster_tool() { - perf-tests $@ + perf-tests "$@" } # Delete the old clusters belonged to the current repo, and recreate them with the same configuration. diff --git a/presubmit-tests.sh b/presubmit-tests.sh index 1c6e5a8d..d210dc70 100755 --- a/presubmit-tests.sh +++ b/presubmit-tests.sh @@ -17,7 +17,7 @@ # This is a helper script for Knative presubmit test scripts. # See README.md for instructions on how to use it. -source $(dirname "${BASH_SOURCE[0]}")/library.sh +source "$(dirname "${BASH_SOURCE[0]}")"/library.sh # Custom configuration of presubmit tests readonly PRESUBMIT_TEST_FAIL_FAST=${PRESUBMIT_TEST_FAIL_FAST:-0} diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index f10eb6f6..fb216d07 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -42,9 +42,9 @@ function dump_metrics() { } # Script entry point. -initialize "$@" --max-nodes=1 --machine=e2-standard-2 \ +initialize "$@" --num-nodes=1 --machine-type=e2-standard-4 \ --enable-workload-identity --cluster-version=latest \ - --extra-gcloud-flags "--enable-stackdriver-kubernetes --no-enable-ip-alias --no-enable-autoupgrade" + --gcloud-extra-flags "--logging=NONE --monitoring=NONE" [[ ${KNATIVE_SETUP_DONE:-0} == 1 ]] || fail_test 'Knative setup not persisted' [[ ${TEST_SETUP_DONE:-0} == 1 ]] || fail_test 'Test setup not persisted' diff --git a/test/unit/presubmit_test.go b/test/unit/presubmit_test.go index 5d26d23a..2fb1bd75 100644 --- a/test/unit/presubmit_test.go +++ b/test/unit/presubmit_test.go @@ -2,14 +2,12 @@ package unit_test import ( "fmt" - "path" "strings" "testing" ) func TestMainFunc(t *testing.T) { t.Parallel() - rootDir := path.Dir(path.Dir(currentDir())) sc := newShellScript( fakeProwJob(), loadFile("source-presubmit-tests.bash"), @@ -43,23 +41,8 @@ func TestMainFunc(t *testing.T) { contains("-- -short -race -count 1 ./..."), contains("UNIT TESTS PASSED"), }, - }, { - name: `main --integration-tests`, - stdout: []check{ - contains("RUNNING INTEGRATION TESTS"), - contains("Running integration test test/e2e-tests.sh"), - contains(fmt.Sprintf("go run knative.dev/test-infra/tools/kntest/cmd/kntest@latest"+ - " kubetest2 gke --max-nodes=1 --machine=e2-standard-2 "+ - "--enable-workload-identity --cluster-version=latest "+ - "--extra-gcloud-flags --enable-stackdriver-kubernetes "+ - "--no-enable-ip-alias --no-enable-autoupgrade "+ - "--addons=NodeLocalDNS "+ - "--test-command=%s/test/e2e-tests.sh "+ - "--run-tests --extra-gcloud-flags= --logging=NONE "+ - "--monitoring=NONE", rootDir)), - contains("INTEGRATION TESTS PASSED"), - }, - }} + }, + } for _, tc := range tcs { tc := tc t.Run(tc.name, tc.test(sc)) diff --git a/test/unit/sharedlib_test.go b/test/unit/sharedlib_test.go index 5b19dbe9..71fd92c1 100644 --- a/test/unit/sharedlib_test.go +++ b/test/unit/sharedlib_test.go @@ -305,6 +305,7 @@ func fakeProwJob() scriptlet { mockBinary("cosign"), mockBinary("rcodesign"), mockBinary("gsutil"), + mockBinary("kubetest2"), ) }