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.

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

test(components): Reduce sagemaker component test flakiness #10225

Merged
merged 1 commit into from Feb 14, 2024

Conversation

ananth102
Copy link
Contributor

Description of your changes:

  • Do not use a fixture for the kfp client
  • Upgrade ACK version
  • Uninstall standalone kfp to cleanup resources.

Checklist:

@@ -280,6 +280,8 @@ function cleanup_kfp() {
# If this fails, deleting the nodegroup later will clean it up anyway
kill -9 $MINIO_PID || true
fi
kubectl delete -k "github.com/kubeflow/pipelines/manifests/kustomize/env/cert-manager/dev?ref=$KFP_VERSION&timeout=90s"
Copy link
Contributor

Choose a reason for hiding this comment

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

How long does this line take? Do we need to add wait time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

About 2 minutes at the maximum since this is a lightweight distribution, wait time is unnecessary here. My concern was more about the installation, it may take longer because of needing to create the persistent volume, which depends on EBS. I think the best solution might just be to uninstall on test failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in new revision, uninstallation only occurs on failure which mitigates the risk of a long uninstallation/installation.

@google-oss-prow google-oss-prow bot added size/S and removed size/XS labels Feb 14, 2024
@ananth102 ananth102 force-pushed the simplekfpfixes branch 2 times, most recently from 3cda2a6 to 2fd35b8 Compare February 14, 2024 02:26
Signed-off-by: ananth102 <abashyam@amazon.com>
@ryansteakley
Copy link
Contributor

/lgtm

Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rd-pong, ryansteakley

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 664deaf into kubeflow:master Feb 14, 2024
3 checks passed
KevinGrantLee pushed a commit to KevinGrantLee/pipelines that referenced this pull request Feb 15, 2024
petethegreat pushed a commit to petethegreat/pipelines that referenced this pull request Mar 27, 2024
petethegreat pushed a commit to petethegreat/pipelines that referenced this pull request Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants