Skip to content

[RHOAIENG-9004] Adjust e2e tests to use GPU #583

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
Merged
merged 2 commits into from
Jul 11, 2024
Merged

[RHOAIENG-9004] Adjust e2e tests to use GPU #583

merged 2 commits into from
Jul 11, 2024

Conversation

sutaakar
Copy link
Contributor

@sutaakar sutaakar commented Jul 9, 2024 •
edited
Loading

Issue link

RHOAIENG-9004

What changes have been made

Adding additional test function to run tests using NVidia GPU, altering test implementation to specify GPU requirements.
In a separate commit altering CFO deployment to avoid building operator, but rather using existing latest image.

Verification steps

Tested by PR check.
Can be tested manually by running poetry run pytest -v -s ./tests/e2e -m 'nvidia_gpu' against configured cluster with NVidia GPU.

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • Testing is not required for this change

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 9, 2024
@sutaakar sutaakar force-pushed the gpu-tests branch 21 times, most recently from c53bc8d to 38f3daa Compare July 10, 2024 12:57
@sutaakar sutaakar marked this pull request as ready for review July 10, 2024 13:42
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 10, 2024
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2024
@Bobbins228 Bobbins228 removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 11, 2024
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 11, 2024
@sutaakar
Copy link
Contributor Author

@ChristianZaccaria Adjusted readme for GPU instructions.

ChristianZaccaria
ChristianZaccaria approved these changes Jul 11, 2024
View reviewed changes
Copy link
Collaborator

@ChristianZaccaria ChristianZaccaria left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2024
@ChughShilpa
Copy link
Contributor

Just a nitpick - e2e.md doc is missing instructions for openshift clusters

@ChughShilpa
Copy link
Contributor

Just a nitpick - e2e.md doc is missing instructions for openshift clusters

My bad, I overlooked you added Gpu support for kind only

@ChughShilpa ChughShilpa reopened this Jul 11, 2024
@sutaakar
Copy link
Contributor Author

Well, the tests can be executed also on OpenShift, not just KinD.
I will update the doc

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2024
@sutaakar
Copy link
Contributor Author

@ChughShilpa doc updated

@ChughShilpa
Copy link
Contributor

For openshift cluster, we can mention a workaround is needed as ClusterConfiguration cannot configure tolerations yet.
May be provide with an example of workaround till we don't come up with a solution for it

@sutaakar
Copy link
Contributor Author

@ChughShilpa Added a note that SDK tests can't be executed on nodes with taint.

@ChughShilpa
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2024
Bobbins228
Bobbins228 approved these changes Jul 11, 2024
View reviewed changes
Copy link
Contributor

@Bobbins228 Bobbins228 left a comment

Choose a reason for hiding this comment

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

/approve

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Jul 11, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhijeet-dhumal, Bobbins228, ChristianZaccaria, Fiona-Waters, Srihari1192, Ygnas

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:
  • OWNERS [Bobbins228,Fiona-Waters]

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

@openshift-merge-bot openshift-merge-bot bot merged commit c308c0f into project-codeflare:main Jul 11, 2024
4 checks passed
@sutaakar sutaakar deleted the gpu-tests branch July 12, 2024 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChughShilpa ChughShilpa ChughShilpa left review comments

@Ygnas Ygnas Ygnas approved these changes

@Srihari1192 Srihari1192 Srihari1192 approved these changes

@Bobbins228 Bobbins228 Bobbins228 approved these changes

@ChristianZaccaria ChristianZaccaria ChristianZaccaria approved these changes

@abhijeet-dhumal abhijeet-dhumal abhijeet-dhumal approved these changes

@jiripetrlik jiripetrlik Awaiting requested review from jiripetrlik

@Fiona-Waters Fiona-Waters Awaiting requested review from Fiona-Waters

Assignees

@Ygnas Ygnas

@Srihari1192 Srihari1192

@Bobbins228 Bobbins228

@ChristianZaccaria ChristianZaccaria

@abhijeet-dhumal abhijeet-dhumal

@ChughShilpa ChughShilpa

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@sutaakar @Fiona-Waters @ChughShilpa @Ygnas @Srihari1192 @Bobbins228 @ChristianZaccaria @abhijeet-dhumal