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

Jump to bottom

Fix test loophole for loading samples during KFP startup #1807

Merged
merged 1 commit into from
Aug 12, 2019
Merged

Fix test loophole for loading samples during KFP startup #1807

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 9 additions & 3 deletions test/deploy-pipeline.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,18 @@ cd ${DIR}/${KFAPP}

## Update pipeline component image
pushd ks_app
# Delete pipeline component first before applying so we guarantee the pipeline component is new.
ks delete default -c pipeline
sleep 60s
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why add the sleep here? Is the 'ks delete' nonblocking? If so, we might need to wait until the deletion completes here.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes it's async delete. ks apply wont fail fast if it's applying to a terminating resource.
sleep 60s would ensure things are cleaned up.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the longer plan should be building the pipeline images before deploying the kubeflow and have the kubeflow deployment include the new images.
On the other hand, I thought the delete is a blocking call. If it is async, depending on a hardcoded timing is not good practice. If the deletion takes less than 60 seconds, we are wasting time in the tests; If the deletion takes more than 60 seconds, it is not working. Above all, I think we need a more deterministic way to wait for the pipeline deletion.
WDYT?


ks param set pipeline apiImage ${GCR_IMAGE_BASE_DIR}/api-server:${GCR_IMAGE_TAG}
ks param set pipeline persistenceAgentImage ${GCR_IMAGE_BASE_DIR}/persistenceagent:${GCR_IMAGE_TAG}
ks param set pipeline scheduledWorkflowImage ${GCR_IMAGE_BASE_DIR}/scheduledworkflow:${GCR_IMAGE_TAG}
ks param set pipeline uiImage ${GCR_IMAGE_BASE_DIR}/frontend:${GCR_IMAGE_TAG}
# Delete pipeline component first before applying so we guarantee the pipeline component is new.
ks delete default -c pipeline
sleep 60s
# Swap the metadata/artifact storage PD to avoid reusing the old data.
# We should remove this hack when we deprecate ksonnet.
# See https://github.com/kubeflow/pipelines/pull/1805#issuecomment-520204987 for context
ks param set pipeline minioPd ${KFAPP}-storage-metadata-store
ks param set pipeline mysqlPd ${KFAPP}-storage-artifact-store
ks apply default -c pipeline
popd