Fix obsolete image cache when the same PR commit is tested with a new…

… master (#2738)
kubeflow · Dec 17, 2019 · 3d008f9 · 3d008f9
1 parent c85226b
commit 3d008f9
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 12 deletions.
diff --git a/test/README.md b/test/README.md
@@ -69,8 +69,7 @@ argo submit integration_test_gke.yaml -p branch="my-branch" -p cleanup="false"
 
 Run the following commands from root of kubeflow/pipelines repo.
 ```
-#$PULL_PULL_SHA and $WORKSPACE are env variables set by Prow
-export PULL_PULL_SHA=pull-sha-placeholder
+# $WORKSPACE are env variables set by Prow
 export WORKSPACE=$(pwd) # root of kubeflow/pipelines git repo
 export SA_KEY_FILE=PATH/TO/YOUR/GCP/PROJECT/SERVICE/ACCOUNT/KEY
 # (optional) uncomment the following to keep reusing the same cluster

diff --git a/test/build-images.sh b/test/build-images.sh
@@ -36,7 +36,7 @@ then
     persistenceagent, viewer-crd-controller, inverse-proxy-agent, and visualization-server \
     are already built in ${GCR_IMAGE_BASE_DIR}."
 else
-  echo "submitting cloud build to build docker images for commit ${PULL_PULL_SHA}..."
+  echo "submitting cloud build to build docker images for commit ${COMMIT_SHA}..."
   IMAGES_BUILDING=true
   CLOUD_BUILD_COMMON_ARGS=(. --async --format='value(id)' --substitutions=_GCR_BASE=${GCR_IMAGE_BASE_DIR})
   # Split into two tasks because api_server builds slowly, use a separate task

diff --git a/test/postsubmit-tests-with-pipeline-deployment.sh b/test/postsubmit-tests-with-pipeline-deployment.sh
@@ -35,6 +35,7 @@ GCR_IMAGE_BASE_DIR=gcr.io/ml-pipeline-test
 TARGET_IMAGE_BASE_DIR=gcr.io/ml-pipeline-test/${PULL_BASE_SHA}
 TIMEOUT_SECONDS=1800
 NAMESPACE=kubeflow
+COMMIT_SHA="$PULL_BASE_SHA"
 
 while [ "$1" != "" ]; do
     case $1 in
@@ -114,7 +115,7 @@ elif [[ ${CLOUDBUILD_FINISHED} == TIMEOUT ]];then
   exit 1
 fi
 
-COMMIT_SHA=$PULL_BASE_SHA source "${DIR}/deploy-cluster.sh"
+source "${DIR}/deploy-cluster.sh"
 echo "cluster deployed"
 
 # Install Argo

diff --git a/test/presubmit-tests-with-pipeline-deployment.sh b/test/presubmit-tests-with-pipeline-deployment.sh
@@ -63,19 +63,23 @@ while [ "$1" != "" ]; do
     shift
 done
 
-# PULL_PULL_SHA is empty whne Pros/Tide tests the batches.
-# PULL_BASE_SHA cannot be used here as it still points to master tip in that case.
-PULL_PULL_SHA="${PULL_PULL_SHA:-$(git rev-parse HEAD)}"
+# This is merged commit's SHA.
+COMMIT_SHA="$(git rev-parse HEAD)"
 
-# Variables
-GCR_IMAGE_BASE_DIR=gcr.io/${PROJECT}/${PULL_PULL_SHA}
-TEST_RESULTS_GCS_DIR=gs://${TEST_RESULT_BUCKET}/${PULL_PULL_SHA}/${TEST_RESULT_FOLDER}
+# Paths are using commit sha, instead of pull sha, because tests may be rerun with the same PR
+# commit, but merged on a different master version. When this happens, we cannot reuse cached
+# results on the previous test run.
+GCR_IMAGE_BASE_DIR=gcr.io/${PROJECT}/${COMMIT_SHA}
+TEST_RESULTS_GCS_DIR=gs://${TEST_RESULT_BUCKET}/${COMMIT_SHA}/${TEST_RESULT_FOLDER}
 DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" > /dev/null && pwd)"
 
 # Configure `time` command output format.
 TIMEFORMAT="[test-timing] It took %lR."
 
 echo "presubmit test starts"
+if [ -n "$PULL_PULL_SHA" ]; then
+    echo "PR commit is ${PULL_PULL_SHA}"
+fi
 time source "${DIR}/test-prep.sh"
 echo "test env prepared"
 
@@ -84,7 +88,7 @@ echo "test env prepared"
 time source "${DIR}/build-images.sh"
 echo "KFP images cloudbuild jobs submitted"
 
-time COMMIT_SHA=$PULL_PULL_SHA source "${DIR}/deploy-cluster.sh"
+time source "${DIR}/deploy-cluster.sh"
 echo "cluster deployed"
 
 time source "${DIR}/check-build-image-status.sh"
@@ -97,7 +101,7 @@ echo "argo installed"
 time source "${DIR}/deploy-pipeline-lite.sh"
 echo "KFP lite deployed"
 
-echo "submitting argo workflow to run tests for commit ${PULL_PULL_SHA}..."
+echo "submitting argo workflow to run tests for commit ${COMMIT_SHA}..."
 ARGO_WORKFLOW=`argo submit ${DIR}/${WORKFLOW_FILE} \
 -p image-build-context-gcs-uri="$remote_code_archive_uri" \
 ${IMAGE_BUILDER_ARG} \