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

Add clean up code in e2e to enable multiple runs on the same cluster #449

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add clean up code in e2e to enable multiple runs on the same cluster #449

wants to merge 1 commit into from

Conversation

harche
Copy link
Contributor

@harche harche commented Feb 20, 2025

This PR adds a clean up code to run e2e on the same node multiple times.

Fixes : #448

@openshift-ci openshift-ci bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 20, 2025
@harche
Copy link
Contributor Author

harche commented Feb 20, 2025

Although the fix is only for emulator mode, it would be still a good idea to verify it on GPUs to make sure we aren't accidentally breaking e2e there.

/hold

@openshift-ci openshift-ci bot requested review from mamy-CS and mrunalp February 20, 2025 16:29
@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 20, 2025
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Feb 20, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: harche

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 20, 2025
@harche
Copy link
Contributor Author

harche commented Feb 20, 2025

Although the fix is only for emulator mode, it would be still a good idea to verify it on GPUs to make sure we aren't accidentally breaking e2e there.

@asm582 our MoC cluster is having some hardware issue CCI-MOC/esi#702. Do you have any cluster with GPUs to test the e2e with this PR?

@harche
Copy link
Contributor Author

harche commented Feb 20, 2025

Although the fix is only for emulator mode, it would be still a good idea to verify it on GPUs to make sure we aren't accidentally breaking e2e there.

@asm582 our MoC cluster is having some hardware issue CCI-MOC/esi#702. Do you have any cluster with GPUs to test the e2e with this PR?

Running into capacity issues with GCP too :(

@harche
Copy link
Contributor Author

harche commented Feb 20, 2025

Although the fix is only for emulator mode, it would be still a good idea to verify it on GPUs to make sure we aren't accidentally breaking e2e there.

@asm582 our MoC cluster is having some hardware issue CCI-MOC/esi#702. Do you have any cluster with GPUs to test the e2e with this PR?

Running into capacity issues with GCP too :(

Nevermind, I got some allocated to me from us-central zone.

@asm582
Copy link
Contributor

asm582 commented Feb 20, 2025

Although the fix is only for emulator mode, it would be still a good idea to verify it on GPUs to make sure we aren't accidentally breaking e2e there.

@asm582 our MoC cluster is having some hardware issue CCI-MOC/esi#702. Do you have any cluster with GPUs to test the e2e with this PR?

Running into capacity issues with GCP too :(

Nevermind, I got some allocated to me from us-central zone.

Sorry, we have a busy cluster today, let me see if I can get to this today

@harche
Copy link
Contributor Author

harche commented Feb 20, 2025

Sorry, we have a busy cluster today, let me see if I can get to this today

No worries, right after I wrote that, I got the GPUs allocated in GCP.

@harche
Copy link
Contributor Author

harche commented Feb 20, 2025 •
edited
Loading

Alright the test pass on GPUs, but @cpmeadors suggested on slack that we can add a check in BeforeSuite as well to make sure we only have 1 instaslice object before starting the tests in emulator mode. I will update the PR accordingly.

@openshift-ci openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 20, 2025
@openshift-ci openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 20, 2025
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Feb 20, 2025

@harche: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/lint eec6be6 link true /test lint

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mamy-CS mamy-CS Awaiting requested review from mamy-CS

@mrunalp mrunalp Awaiting requested review from mrunalp

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

e2e tests needs to remove labels from node when done
2 participants
@harche @asm582