Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix obsolete image cache when the same PR commit is tested with a new master #2738

Merged
merged 2 commits into from
Dec 17, 2019
Merged

Fix obsolete image cache when the same PR commit is tested with a new master #2738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f4d009
Fix obsolete image cache when the same PR commit is tested with a new…
Bobgy Dec 16, 2019
94a9ca6
Merge remote-tracking branch 'official/master' into testing_obsolete_…
Bobgy Dec 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions test/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/build-images.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion test/postsubmit-tests-with-pipeline-deployment.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
20 changes: 12 additions & 8 deletions test/presubmit-tests-with-pipeline-deployment.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PULL_PULL_SHA is expected to be exactly that - the SHA of a merge commit between the current master tip and the tip of the branch.

# 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"

Expand All @@ -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"
Expand All @@ -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} \
Expand Down