From f5014439fc2b977a357ff3b49fd6aab052302f7c Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Fri, 5 Apr 2019 08:44:53 +0200 Subject: [PATCH 1/7] prow.sh: AllAlpha=true for unknown Kubernetes versions This ensures that also new, currently unknown alpha gates are enabled when testing against a future Kubernetes versions. For all currently known Kubernetes versions we just use the minimal set of alpha gates, which ensures that we don't miss any of them in our documentation. --- prow.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/prow.sh b/prow.sh index 490fe99f13..5a636bc37b 100755 --- a/prow.sh +++ b/prow.sh @@ -246,8 +246,9 @@ configvar CSI_PROW_E2E_ALPHA "$(get_versioned_variable CSI_PROW_E2E_ALPHA "${csi # the failing test for "latest" or by updating the test and not running # it anymore for older releases. configvar CSI_PROW_E2E_ALPHA_GATES_1_13 'VolumeSnapshotDataSource=true,BlockVolume=true,CSIBlockVolume=true' "alpha feature gates for Kubernetes 1.13" +configvar CSI_PROW_E2E_ALPHA_GATES_1_14 'VolumeSnapshotDataSource=true' "alpha feature gates for Kubernetes 1.14" # TODO: add new CSI_PROW_ALPHA_GATES entry for future Kubernetes releases -configvar CSI_PROW_E2E_ALPHA_GATES_LATEST 'VolumeSnapshotDataSource=true' "alpha feature gates for latest Kubernetes" +configvar CSI_PROW_E2E_ALPHA_GATES_LATEST 'AllAlpha=true' "alpha feature gates for latest Kubernetes" configvar CSI_PROW_E2E_ALPHA_GATES "$(get_versioned_variable CSI_PROW_E2E_ALPHA_GATES "${csi_prow_kubernetes_version_suffix}")" "alpha E2E feature gates" # Some tests are known to be unusable in a KinD cluster. For example, From 31dfaf31dc6fb132255f9696a4562ab3e8a741ae Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 8 Apr 2019 08:44:10 +0200 Subject: [PATCH 2/7] prow.sh: fix running of just "alpha" tests "grep -w" treated "serial-alpha" as two words and therefore CSI_PROW_TESTS sometimes ran too many tests. --- prow.sh | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/prow.sh b/prow.sh index 5a636bc37b..f39b339409 100755 --- a/prow.sh +++ b/prow.sh @@ -203,7 +203,16 @@ configvar CSI_PROW_SANITY_CONTAINER "hostpath" "Kubernetes container with CSI dr # by setting CSI_PROW_TESTS_SANITY. configvar CSI_PROW_TESTS "unit parallel serial parallel-alpha serial-alpha ${CSI_PROW_TESTS_SANITY}" "tests to run" test_enabled () { - echo "${CSI_PROW_TESTS}" | grep -q -w -e "$1" + local test="$1" + # We want word-splitting here, so ignore: Double quote to prevent globbing and word splitting. + # shellcheck disable=SC2086 + set ${CSI_PROW_TESTS} + for t in "$@"; do + if [ "$t" = "$test" ]; then + return + fi + done + return 1 } # Serial vs. parallel is always determined by these regular expressions. From f3d1d2df5c85dede9cf25201c69634551a538b92 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 8 Apr 2019 08:47:15 +0200 Subject: [PATCH 3/7] prow.sh: fix hostpath driver version check The previous logic failed for canary jobs, those also deploy a recent driver. Instead of guessing what driver gets installed based on job parameters, check what really runs in the cluster and base the decision on that. We only need to maintain this blacklist for 1.0.x until we replace it with 1.1.0, then this entire hostpath_supports_block can be removed. --- prow.sh | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/prow.sh b/prow.sh index f39b339409..cd04131954 100755 --- a/prow.sh +++ b/prow.sh @@ -619,18 +619,18 @@ install_sanity () ( # Whether the hostpath driver supports raw block devices depends on which version # we are testing. It would be much nicer if we could determine that by querying the -# installed driver. +# installed driver's capabilities instead of having to do a version check. hostpath_supports_block () { - if [ -e "cmd/hostpathplugin" ] && ${CSI_PROW_BUILD_JOB}; then - # The assumption is that if we build the hostpath driver, then it is - # a current version with support. - echo true - return - fi - - case "${CSI_PROW_DEPLOYMENT}" in kubernetes-1.13) echo false;; # wasn't supported and probably won't be backported - *) echo true;; # probably all other deployments have a recent driver - esac + local result + result="$(docker exec csi-prow-control-plane docker image ls --format='{{.Repository}} {{.Tag}} {{.ID}}' | grep hostpath | while read -r repo tag id; do + if [ "$tag" == "v1.0.1" ]; then + # Old version because the revision label is missing: didn't have support yet. + echo "false" + return + fi + done)" + # If not set, then it must be a newer driver with support. + echo "${result:-true}" } # Captures pod output while running some other command. From aa45a1cd9b9fafde60f7d8a8e314287e3b8a7794 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 8 Apr 2019 08:51:11 +0200 Subject: [PATCH 4/7] prow.sh: more efficient execution of individual tests When running only some tests, sometimes extra, unnecessarily work was done, like bringing up the cluster without alpha gates. --- prow.sh | 214 ++++++++++++++++++++++++++++++-------------------------- 1 file changed, 116 insertions(+), 98 deletions(-) diff --git a/prow.sh b/prow.sh index cd04131954..72be16f5a4 100755 --- a/prow.sh +++ b/prow.sh @@ -202,18 +202,30 @@ configvar CSI_PROW_SANITY_CONTAINER "hostpath" "Kubernetes container with CSI dr # is off by default. A CSI driver can change that default in its .prow.sh # by setting CSI_PROW_TESTS_SANITY. configvar CSI_PROW_TESTS "unit parallel serial parallel-alpha serial-alpha ${CSI_PROW_TESTS_SANITY}" "tests to run" -test_enabled () { - local test="$1" - # We want word-splitting here, so ignore: Double quote to prevent globbing and word splitting. - # shellcheck disable=SC2086 - set ${CSI_PROW_TESTS} - for t in "$@"; do - if [ "$t" = "$test" ]; then - return - fi +tests_enabled () { + local t1 t2 + # We want word-splitting here, so ignore: Quote to prevent word splitting, or split robustly with mapfile or read -a. + # shellcheck disable=SC2206 + local tests=(${CSI_PROW_TESTS}) + for t1 in "$@"; do + for t2 in "${tests[@]}"; do + if [ "$t1" = "$t2" ]; then + return + fi + done done return 1 } +tests_need_kind () { + tests_enabled "sanity" "parallel" "serial" "serial-alpha" "parallel-alpha" +} +tests_need_non_alpha_cluster () { + tests_enabled "sanity" "parallel" "serial" +} +tests_need_alpha_cluster () { + tests_enabled "parallel-alpha" "serial-alpha" +} + # Serial vs. parallel is always determined by these regular expressions. # Individual regular expressions are seperated by spaces for readability @@ -521,6 +533,15 @@ install_hostpath () { return 1 fi + if ${CSI_PROW_BUILD_JOB}; then + # Ignore: Double quote to prevent globbing and word splitting. + # Ignore: To read lines rather than words, pipe/redirect to a 'while read' loop. + # shellcheck disable=SC2086 disable=SC2013 + for i in $(grep '^\s*CMDS\s*=' Makefile | sed -e 's/\s*CMDS\s*=//'); do + kind load docker-image --name csi-prow $i:csiprow || die "could not load the $i:latest image into the kind cluster" + done + fi + if deploy_hostpath="$(find_deployment "$(pwd)/deploy")"; then : elif [ "${CSI_PROW_HOSTPATH_REPO}" = "none" ]; then @@ -836,7 +857,7 @@ main () { # might have been minor or unavoidable, for example when experimenting with # changes in "release-tools" in a PR (that fails the "is release-tools unmodified" # test). - if test_enabled "unit"; then + if tests_enabled "unit"; then if ! run_with_go "${CSI_PROW_GO_VERSION_BUILD}" make -k test 2>&1 | make_test_to_junit; then warn "'make test' failed, proceeding anyway" ret=1 @@ -846,102 +867,99 @@ main () { run_with_go "${CSI_PROW_GO_VERSION_BUILD}" make container || die "'make container' failed" fi - install_kind || die "installing kind failed" - start_cluster || die "starting the cluster failed" + if tests_need_kind; then + install_kind || die "installing kind failed" - if ${CSI_PROW_BUILD_JOB}; then - cmds="$(grep '^\s*CMDS\s*=' Makefile | sed -e 's/\s*CMDS\s*=//')" - # Get the image that was just built (if any) from the - # top-level Makefile CMDS variable and set the - # deploy-hostpath.sh env variables for it. We also need to - # side-load those images into the cluster. - for i in $cmds; do - e=$(echo "$i" | tr '[:lower:]' '[:upper:]' | tr - _) - images="$images ${e}_REGISTRY=none ${e}_TAG=csiprow" - - # We must avoid the tag "latest" because that implies - # always pulling the image - # (https://github.com/kubernetes-sigs/kind/issues/328). - docker tag "$i:latest" "$i:csiprow" || die "tagging the locally built container image for $i failed" - kind load docker-image --name csi-prow "$i:csiprow" || die "could not load the $i:latest image into the kind cluster" - done + if ${CSI_PROW_BUILD_JOB}; then + cmds="$(grep '^\s*CMDS\s*=' Makefile | sed -e 's/\s*CMDS\s*=//')" + # Get the image that was just built (if any) from the + # top-level Makefile CMDS variable and set the + # deploy-hostpath.sh env variables for it. We also need to + # side-load those images into the cluster. + for i in $cmds; do + e=$(echo "$i" | tr '[:lower:]' '[:upper:]' | tr - _) + images="$images ${e}_REGISTRY=none ${e}_TAG=csiprow" + + # We must avoid the tag "latest" because that implies + # always pulling the image + # (https://github.com/kubernetes-sigs/kind/issues/328). + docker tag "$i:latest" "$i:csiprow" || die "tagging the locally built container image for $i failed" + done - if [ -e deploy/kubernetes/rbac.yaml ]; then - # This is one of those components which has its own RBAC rules (like external-provisioner). - # We are testing a locally built image and also want to test with the the current, - # potentially modified RBAC rules. - if [ "$(echo "$cmds" | wc -w)" != 1 ]; then - die "ambiguous deploy/kubernetes/rbac.yaml: need exactly one command, got: $cmds" + if [ -e deploy/kubernetes/rbac.yaml ]; then + # This is one of those components which has its own RBAC rules (like external-provisioner). + # We are testing a locally built image and also want to test with the the current, + # potentially modified RBAC rules. + if [ "$(echo "$cmds" | wc -w)" != 1 ]; then + die "ambiguous deploy/kubernetes/rbac.yaml: need exactly one command, got: $cmds" + fi + e=$(echo "$cmds" | tr '[:lower:]' '[:upper:]' | tr - _) + images="$images ${e}_RBAC=$(pwd)/deploy/kubernetes/rbac.yaml" fi - e=$(echo "$cmds" | tr '[:lower:]' '[:upper:]' | tr - _) - images="$images ${e}_RBAC=$(pwd)/deploy/kubernetes/rbac.yaml" - fi - fi - - # Installing the driver might be disabled, in which case we bail out early. - if ! install_hostpath "$images"; then - info "hostpath driver installation disabled, skipping E2E testing" - return "$ret" - fi - - collect_cluster_info - - if test_enabled "sanity"; then - if ! run_sanity; then - ret=1 - fi - fi - - if test_enabled "parallel"; then - # Ignore: Double quote to prevent globbing and word splitting. - # shellcheck disable=SC2086 - if ! run_e2e parallel ${CSI_PROW_GINKO_PARALLEL} \ - -focus="External.Storage" \ - -skip="$(regex_join "${CSI_PROW_E2E_SERIAL}" "${CSI_PROW_E2E_ALPHA}" "${CSI_PROW_E2E_SKIP}")"; then - warn "E2E parallel failed" - ret=1 fi - fi - - if test_enabled "serial"; then - if ! run_e2e serial \ - -focus="External.Storage.*($(regex_join "${CSI_PROW_E2E_SERIAL}"))" \ - -skip="$(regex_join "${CSI_PROW_E2E_ALPHA}" "${CSI_PROW_E2E_SKIP}")"; then - warn "E2E serial failed" - ret=1 - fi - fi - if (test_enabled "parallel-alpha" || test_enabled "serial-alpha") && [ "${CSI_PROW_E2E_ALPHA_GATES}" ]; then - # Need to (re)create the cluster. - start_cluster "${CSI_PROW_E2E_ALPHA_GATES}" || die "starting alpha cluster failed" - if ${CSI_PROW_BUILD_JOB}; then - # Ignore: Double quote to prevent globbing and word splitting. - # Ignore: To read lines rather than words, pipe/redirect to a 'while read' loop. - # shellcheck disable=SC2086 disable=SC2013 - for i in $(grep '^\s*CMDS\s*=' Makefile | sed -e 's/\s*CMDS\s*=//'); do - kind load docker-image --name csi-prow $i:csiprow || die "could not load the $i:latest image into the kind cluster" - done - fi - install_hostpath "$images" || die "hostpath driver installation failed unexpectedly on alpha cluster" - - if test_enabled "parallel-alpha"; then - # Ignore: Double quote to prevent globbing and word splitting. - # shellcheck disable=SC2086 - if ! run_e2e parallel-alpha ${CSI_PROW_GINKO_PARALLEL} \ - -focus="External.Storage.*($(regex_join "${CSI_PROW_E2E_ALPHA}"))" \ - -skip="$(regex_join "${CSI_PROW_E2E_SERIAL}" "${CSI_PROW_E2E_SKIP}")"; then - warn "E2E parallel alpha failed" - ret=1 + if tests_need_non_alpha_cluster; then + start_cluster || die "starting the non-alpha cluster failed" + + # Installing the driver might be disabled. + if install_hostpath "$images"; then + collect_cluster_info + + if tests_enabled "sanity"; then + if ! run_sanity; then + ret=1 + fi + fi + + if tests_enabled "parallel"; then + # Ignore: Double quote to prevent globbing and word splitting. + # shellcheck disable=SC2086 + if ! run_e2e parallel ${CSI_PROW_GINKO_PARALLEL} \ + -focus="External.Storage" \ + -skip="$(regex_join "${CSI_PROW_E2E_SERIAL}" "${CSI_PROW_E2E_ALPHA}" "${CSI_PROW_E2E_SKIP}")"; then + warn "E2E parallel failed" + ret=1 + fi + fi + + if tests_enabled "serial"; then + if ! run_e2e serial \ + -focus="External.Storage.*($(regex_join "${CSI_PROW_E2E_SERIAL}"))" \ + -skip="$(regex_join "${CSI_PROW_E2E_ALPHA}" "${CSI_PROW_E2E_SKIP}")"; then + warn "E2E serial failed" + ret=1 + fi + fi fi fi - if test_enabled "serial-alpha"; then - if ! run_e2e serial-alpha \ - -focus="External.Storage.*(($(regex_join "${CSI_PROW_E2E_SERIAL}")).*($(regex_join "${CSI_PROW_E2E_ALPHA}"))|($(regex_join "${CSI_PROW_E2E_ALPHA}")).*($(regex_join "${CSI_PROW_E2E_SERIAL}")))" \ - -skip="$(regex_join "${CSI_PROW_E2E_SKIP}")"; then - warn "E2E serial alpha failed" - ret=1 + if tests_need_alpha_cluster && [ "${CSI_PROW_E2E_ALPHA_GATES}" ]; then + # Need to (re)create the cluster. + start_cluster "${CSI_PROW_E2E_ALPHA_GATES}" || die "starting alpha cluster failed" + + # Installing the driver might be disabled. + if install_hostpath "$images"; then + collect_cluster_info + + if tests_enabled "parallel-alpha"; then + # Ignore: Double quote to prevent globbing and word splitting. + # shellcheck disable=SC2086 + if ! run_e2e parallel-alpha ${CSI_PROW_GINKO_PARALLEL} \ + -focus="External.Storage.*($(regex_join "${CSI_PROW_E2E_ALPHA}"))" \ + -skip="$(regex_join "${CSI_PROW_E2E_SERIAL}" "${CSI_PROW_E2E_SKIP}")"; then + warn "E2E parallel alpha failed" + ret=1 + fi + fi + + if tests_enabled "serial-alpha"; then + if ! run_e2e serial-alpha \ + -focus="External.Storage.*(($(regex_join "${CSI_PROW_E2E_SERIAL}")).*($(regex_join "${CSI_PROW_E2E_ALPHA}"))|($(regex_join "${CSI_PROW_E2E_ALPHA}")).*($(regex_join "${CSI_PROW_E2E_SERIAL}")))" \ + -skip="$(regex_join "${CSI_PROW_E2E_SKIP}")"; then + warn "E2E serial alpha failed" + ret=1 + fi + fi fi fi fi From 9b0d9cd74370bff4dfaf86c4ab75973ec9aa56a2 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 8 Apr 2019 11:37:01 +0200 Subject: [PATCH 5/7] build.make: skip shellcheck if Docker is not available Not all environments have Docker. The simplifying assumption here is that if the Docker command is available, it's also usable. --- build.make | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/build.make b/build.make index 078100c9f6..e5769c5865 100644 --- a/build.make +++ b/build.make @@ -141,6 +141,10 @@ test: test-shellcheck test-shellcheck: @ echo; echo "### $@:" @ ret=0; \ + if ! command -v docker; then \ + echo "skipped, no Docker"; \ + return 0; \ + fi; \ for dir in $(abspath $(TEST_SHELLCHECK_DIRS)); do \ echo; \ echo "$$dir:"; \ From 546d5504a1902276561ee1bfc3811afb61346855 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 8 Apr 2019 19:42:38 +0200 Subject: [PATCH 6/7] prow.sh: debug failing KinD cluster creation When KinD fails in a Prow job, we need additional information to understand why it failed. --- prow.sh | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/prow.sh b/prow.sh index 72be16f5a4..c2075685b1 100755 --- a/prow.sh +++ b/prow.sh @@ -488,7 +488,17 @@ $(list_gates "$gates") featureGates: $(list_gates "$gates") EOF - run kind create cluster --name csi-prow --config "${CSI_PROW_WORK}/kind-config.yaml" --wait 5m --image "$image" || die "'kind create cluster' failed" + info "kind-config.yaml:" + cat "${CSI_PROW_WORK}/kind-config.yaml" + if ! run kind create cluster --name csi-prow --config "${CSI_PROW_WORK}/kind-config.yaml" --wait 5m --image "$image"; then + warn "Cluster creation failed. Will try again with higher verbosity." + info "Available Docker images:" + docker image ls + if ! run kind --loglevel debug create cluster --retain --name csi-prow --config "${CSI_PROW_WORK}/kind-config.yaml" --wait 5m --image "$image"; then + run kind export logs --name csi-prow "$ARTIFACTS/kind-cluster" + die "Cluster creation failed again, giving up. See the 'kind-cluster' artifact directory for additional logs." + fi + fi KUBECONFIG="$(kind get kubeconfig-path --name=csi-prow)" export KUBECONFIG } From cda2fc5874b42ed68b68cbb60e92fff6ab7ba4c0 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 8 Apr 2019 19:45:44 +0200 Subject: [PATCH 7/7] prow.sh: avoid AllAlpha=true It turned out to not work. Instead of reverting the commit which introduced this, let's better document this explicitly. --- prow.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prow.sh b/prow.sh index c2075685b1..0a2b37d716 100755 --- a/prow.sh +++ b/prow.sh @@ -267,9 +267,9 @@ configvar CSI_PROW_E2E_ALPHA "$(get_versioned_variable CSI_PROW_E2E_ALPHA "${csi # the failing test for "latest" or by updating the test and not running # it anymore for older releases. configvar CSI_PROW_E2E_ALPHA_GATES_1_13 'VolumeSnapshotDataSource=true,BlockVolume=true,CSIBlockVolume=true' "alpha feature gates for Kubernetes 1.13" -configvar CSI_PROW_E2E_ALPHA_GATES_1_14 'VolumeSnapshotDataSource=true' "alpha feature gates for Kubernetes 1.14" +configvar CSI_PROW_E2E_ALPHA_GATES_1_14 'VolumeSnapshotDataSource=true,ExpandCSIVolumes=true' "alpha feature gates for Kubernetes 1.14" # TODO: add new CSI_PROW_ALPHA_GATES entry for future Kubernetes releases -configvar CSI_PROW_E2E_ALPHA_GATES_LATEST 'AllAlpha=true' "alpha feature gates for latest Kubernetes" +configvar CSI_PROW_E2E_ALPHA_GATES_LATEST 'AllAlpha=true,ExpandCSIVolumes=true' "alpha feature gates for latest Kubernetes" configvar CSI_PROW_E2E_ALPHA_GATES "$(get_versioned_variable CSI_PROW_E2E_ALPHA_GATES "${csi_prow_kubernetes_version_suffix}")" "alpha E2E feature gates" # Some tests are known to be unusable in a KinD cluster. For example,