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

Update Model Serving tests for 2.5 #1044

Merged
merged 19 commits into from
Dec 5, 2023

Conversation

bdattoma
Copy link
Contributor

@bdattoma bdattoma commented Nov 27, 2023

Tests for CustomServingRuntime feature must be updated to account for serving platform selection, introduced with 2.5.

This PR includes:

  1. fix broken CustomServingRuntime tests
  2. add one new CustomServingRuntime test (ODS-2542)
  3. fix keywords for model mesh serving UI
  4. fix test teardown for KServe tests
  5. fix Resource-GPU tag for KServe test (it should be Resources-GPU)
  6. (tentative) try to mitigate timing issue in KServe metrics test

@bdattoma bdattoma added needs testing Needs to be tested in Jenkins enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) labels Nov 27, 2023
Copy link
Contributor

github-actions bot commented Nov 27, 2023

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass %
373 0 0 373 100

@bdattoma bdattoma marked this pull request as ready for review December 4, 2023 16:55
Signed-off-by: bdattoma <[email protected]>
Signed-off-by: bdattoma <[email protected]>
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

Robocop found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

Signed-off-by: bdattoma <[email protected]>
Signed-off-by: bdattoma <[email protected]>
@bdattoma bdattoma changed the title Update Custom Serving Runtime tests for 2.5 Update Custom Serving Runtime + Model Mesh tests for 2.5 Dec 5, 2023
@bdattoma
Copy link
Contributor Author

bdattoma commented Dec 5, 2023

PR Validation

  1. CustomServingRuntime suite: rhods-ci-pr-test/2165 PASS - re-run after new changes: rhods-ci-pr-test/2171 PASS
  2. Model (mesh) Serving Smoke suite: rhods-ci-pr-test/2172 PASS
  3. KServe metrics tests: rhods-ci-pr-test/2174 PASS
  4. Model (mesh) Serving Sanity suite: rhods-ci-pr-test/2177 PASS

@bdattoma bdattoma changed the title Update Custom Serving Runtime + Model Mesh tests for 2.5 Update Model Serving tests for 2.5 Dec 5, 2023
@bdattoma bdattoma added verified This PR has been tested with Jenkins needs testing Needs to be tested in Jenkins and removed needs testing Needs to be tested in Jenkins verified This PR has been tested with Jenkins labels Dec 5, 2023
@bdattoma bdattoma added verified This PR has been tested with Jenkins and removed needs testing Needs to be tested in Jenkins labels Dec 5, 2023
lugi0
lugi0 previously approved these changes Dec 5, 2023
Copy link
Member

@jstourac jstourac left a comment

Choose a reason for hiding this comment

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

Thank you Berto.

I put some comments but overall LGTM.

Note: we should mark the new test-case as automated in Polarion once this is merged, correct?

Verify Model Status ${model_name} success
Verify Model Inference ${model_name} ${inference_input} ${exp_inference_output} token_auth=${TRUE}
... project_title=${PRJ_TITLE}

Copy link
Member

Choose a reason for hiding this comment

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

please remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why? according to the linter, there must be 2 lines btw sections

Copy link
Member

Choose a reason for hiding this comment

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

I meant to remove the trailing spaces 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, sorry. I'll remove it in some next PRs (I have in my backlog new tests in this suite after 2.5 release)

... supported serving platform (single model, multi model, both)
Set Suite Variable ${RUNTIME_BOTH_FILEPATH} ${RESOURCES_DIRPATH}/csr_both_model.yaml
Set Suite Variable ${RUNTIME_SINGLE_FILEPATH} ${RESOURCES_DIRPATH}/csr_single_model.yaml
Set Suite Variable ${RUNTIME_MULTI_FILEPATH} ${RESOURCES_DIRPATH}/csr_multi_model.yaml
Copy link
Member

Choose a reason for hiding this comment

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

These three ${RESOURCES_DIRPATH}/csr_* are used on two places. Maybe it would make sense to create a global variables in this file for them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these will be used only by one test, hence I made these suite variable only when running that particular test

Copy link
Member

Choose a reason for hiding this comment

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

yeah, I didn't mean to make it global as that much global, just to define it only once in the file, but I don't insist, that's a small thing 🙂

jgarciao
jgarciao previously approved these changes Dec 5, 2023
@bdattoma bdattoma dismissed stale reviews from jgarciao and lugi0 via a7972c9 December 5, 2023 16:15
Signed-off-by: bdattoma <[email protected]>
Copy link

sonarqubecloud bot commented Dec 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@bdattoma bdattoma merged commit e2c9711 into red-hat-data-services:master Dec 5, 2023
8 checks passed
ChughShilpa pushed a commit to ChughShilpa/ods-ci that referenced this pull request Jan 2, 2024
* add platform selection and check

Signed-off-by: bdattoma <[email protected]>

* replace rhods ns with global variable

Signed-off-by: bdattoma <[email protected]>

* fix ods-2276

Signed-off-by: bdattoma <[email protected]>

* fix runtime creation steps

Signed-off-by: bdattoma <[email protected]>

* fix model deployment step

Signed-off-by: bdattoma <[email protected]>

* increase model ready timeout in the test

Signed-off-by: bdattoma <[email protected]>

* fix alerts

Signed-off-by: bdattoma <[email protected]>

* add ODS-2542

Signed-off-by: bdattoma <[email protected]>

* fix one alert

Signed-off-by: bdattoma <[email protected]>

* remove unused line

Signed-off-by: bdattoma <[email protected]>

* fix teardown for kserve tests

Signed-off-by: bdattoma <[email protected]>

* fix modelmesh teardown

Signed-off-by: bdattoma <[email protected]>

* adapt serve model kw for 2.5

Signed-off-by: bdattoma <[email protected]>

* fix gpu tag for kserve test

Signed-off-by: bdattoma <[email protected]>

* invert order uwm conf with enablement

Signed-off-by: bdattoma <[email protected]>

* change modelmesh token enablement logic for 2.5

Signed-off-by: bdattoma <[email protected]>

* minor change in kserve clean up

Signed-off-by: bdattoma <[email protected]>

* fix auto fetch of project in mm sanity test

Signed-off-by: bdattoma <[email protected]>

* remove sleep

Signed-off-by: bdattoma <[email protected]>

---------

Signed-off-by: bdattoma <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancements Bugfixes, enhancements, refactoring, ... in tests or libraries (PR will be listed in release-notes) verified This PR has been tested with Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants