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

Update tests to use new Caikit + TGIS images #1009

Merged
merged 15 commits into from
Nov 14, 2023
Merged

Update tests to use new Caikit + TGIS images #1009

merged 15 commits into from
Nov 14, 2023

Conversation

bdattoma
Copy link
Contributor

@bdattoma bdattoma commented Nov 7, 2023

Updating the tests to reflect the new architecture of Caikit runtime with TGIS, as implemented in opendatahub-io/caikit-tgis-serving#107: new runtime is composed by 2 separate containers, one for TGIS and one for Caikit, in the same pod.

@bdattoma bdattoma self-assigned this Nov 7, 2023
@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 7, 2023
@bdattoma
Copy link
Contributor Author

bdattoma commented Nov 8, 2023

tested on rhods-ci-pr-test/2111/ . 2 failures not related to this PR: one GPU test which wasn't excluded (no gpu on the cluster) and one failed for timing issue, the pod wasn't ready by the timeout.

@bdattoma bdattoma added verified This PR has been tested with Jenkins and removed needs testing Needs to be tested in Jenkins labels Nov 8, 2023
@bdattoma bdattoma changed the title Update tests to use new Caikit and TGIS images Update tests to use new Caikit + TGIS images Nov 8, 2023
@bdattoma bdattoma marked this pull request as ready for review November 8, 2023 16:35
@bdattoma bdattoma added the do not merge Do not merge this yet please label Nov 9, 2023
@bdattoma
Copy link
Contributor Author

bdattoma commented Nov 9, 2023

do not merge for now, there is a new change coming into the product image and the test files need to be accordingly changed

github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 13, 2023
View reviewed changes
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.

github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 13, 2023
View reviewed changes
ods_ci/tests/Tests/400__ods_dashboard/420__model_serving/422__model_serving_llm.robot
@@ -528,13 +532,39 @@
[Teardown] Clean Up Test Project test_ns=${test_namespace}
... isvc_names=${models_names}

Verify User Can Query A Model Using HTTP Calls

Check warning

Code scanning / Robocop

Test case '{{ test_name }}' is too long ({{ test_length }}/{{ allowed_length }})

Test case 'Verify User Can Query A Model Using HTTP Calls' is too long (24/20)
ods_ci/tests/Tests/400__ods_dashboard/420__model_serving/422__model_serving_llm.robot
[Arguments] ${namespace}
${rc} ${out}= Run And Return Rc And Output oc get ServingRuntime caikit-runtime -n ${namespace}
[Arguments] ${namespace} ${protocol}
${rc} ${out}= Run And Return Rc And Output oc get ServingRuntime caikit-tgis-runtime -n ${namespace}

Check warning

Code scanning / Robocop

Local variable '{{ name }}' is overwritten before usage

Local variable '${out}' is overwritten before usage
ods_ci/tests/Tests/400__ods_dashboard/420__model_serving/422__model_serving_llm.robot Outdated
# temporarily disabling these lines - will be finalized in later stage due to a different format
# of streamed reponse when using http protocol instead of grpc
# ${cleaned_response_text}= Replace String Using Regexp ${model_response} data:(\\s+)?" "
# ${cleaned_response_text}= Replace String Using Regexp ${cleaned_response_text} data:(\\s+)?{ {

Check warning

Code scanning / Robocop

Trailing whitespace at the end of line

Trailing whitespace at the end of line
ods_ci/tests/Tests/400__ods_dashboard/420__model_serving/422__model_serving_llm.robot
IF ${validate_response} == ${FALSE}
${skip_json_load_response}= Set Variable ${TRUE}
ELSE
${skip_json_load_response}= Set Variable ${streamed_response} # always skip if using streaming endpoint
END
${host}= Get KServe Inference Host Via CLI isvc_name=${isvc_name} namespace=${namespace}
${body}= Set Variable '{"text": "${EXP_RESPONSES}[queries][${query_idx}][query_text]"}'
${header}= Set Variable 'mm-model-id: ${model_name}'
IF "${protocol}" == "grpc"

Check notice

Code scanning / Robocop

Variable '{{ name }}' in '{{ block_name }}' condition has unnecessary string conversion

Variable '${protocol}' in 'IF' condition has unnecessary string conversion
ods_ci/tests/Tests/400__ods_dashboard/420__model_serving/422__model_serving_llm.robot
IF "${protocol}" == "grpc"
${body}= Set Variable '{"text": "${EXP_RESPONSES}[queries][${query_idx}][query_text]"}'
${header}= Set Variable 'mm-model-id: ${model_name}'
ELSE IF "${protocol}" == "http"

Check notice

Code scanning / Robocop

Variable '{{ name }}' in '{{ block_name }}' condition has unnecessary string conversion

Variable '${protocol}' in 'ELSE IF' condition has unnecessary string conversion
ods_ci/tests/Tests/400__ods_dashboard/420__model_serving/422__model_serving_llm.robot
... json_body=${body} json_header=${header}
... insecure=${TRUE} skip_res_json=${skip_json_load_response}
... &{args}
IF "${protocol}" == "grpc"

Check notice

Code scanning / Robocop

Variable '{{ name }}' in '{{ block_name }}' condition has unnecessary string conversion

Variable '${protocol}' in 'IF' condition has unnecessary string conversion
ods_ci/tests/Tests/400__ods_dashboard/420__model_serving/422__model_serving_llm.robot
... json_body=${body} json_header=${header}
... insecure=${TRUE} skip_res_json=${skip_json_load_response}
... &{args}
ELSE IF "${protocol}" == "http"

Check notice

Code scanning / Robocop

Variable '{{ name }}' in '{{ block_name }}' condition has unnecessary string conversion

Variable '${protocol}' in 'ELSE IF' condition has unnecessary string conversion
ods_ci/tests/Tests/400__ods_dashboard/420__model_serving/422__model_serving_llm.robot Fixed Show fixed Hide fixed
ods_ci/tests/Resources/Page/ODH/ODHDashboard/ODHDashboardAPI.resource
@@ -29,7 +29,11 @@
Set To Dictionary ${LOG_RESP_DICT} url=${response.url} headers=${response.headers} body=${response.text}
... status_code=${response.status_code} reason=${response.reason}
Log ${request_type} Response: ${LOG_RESP_DICT}
RETURN ${response.json()}
IF ${skip_res_json} == ${TRUE}

Check notice

Code scanning / Robocop

'{{ block_name }}' condition can be simplified

'IF' condition can be simplified
ods_ci/tests/Tests/400__ods_dashboard/420__model_serving/422__model_serving_llm.robot Fixed Show fixed Hide fixed
@bdattoma
Copy link
Contributor Author

bdattoma commented Nov 13, 2023 •
edited
Loading

PR validation:

  1. Entire WatsonX/Kserve suiterhods-smoke/3880 . 3 failures related to the PR changes.
  2. Re-run base deployment test + failed test cases after fixes. rhods-smoke/3881 1 failure.
  3. FINAL: re-run entire suite after fixes: rhods-smoke/3885/ 100% PASS

@sonarqubecloud SonarQubeCloud
Copy link

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 removed the do not merge Do not merge this yet please label Nov 14, 2023
@bdattoma bdattoma merged commit 5b47b89 into red-hat-data-services:master Nov 14, 2023
ChughShilpa pushed a commit to ChughShilpa/ods-ci that referenced this pull request Nov 28, 2023
@bdattoma
Update tests to use new Caikit + TGIS images (red-hat-data-services#1009
b689337 
)

* update runtime and model path

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

* enable script-based install as default

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

* use new runtime without configs

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

* restore grpc port

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

* restore grpc port

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

* add test for http protocol

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

* change tag for runtime upgrade test

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

* delete unused file

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

* fix all tokens http response validation

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

* add missing test teardown

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

* fix post deployment test

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

* fix autoscale test + restore suite setup/teardown

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

* fix robocop alerts

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

* fix post install test - change label

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

---------

Signed-off-by: bdattoma <[email protected]>
ChughShilpa pushed a commit to ChughShilpa/ods-ci that referenced this pull request Jan 2, 2024
@bdattoma
Update tests to use new Caikit + TGIS images (red-hat-data-services#1009
d2eba5e 
)

* update runtime and model path

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

* enable script-based install as default

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

* use new runtime without configs

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

* restore grpc port

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

* restore grpc port

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

* add test for http protocol

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

* change tag for runtime upgrade test

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

* delete unused file

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

* fix all tokens http response validation

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

* add missing test teardown

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

* fix post deployment test

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

* fix autoscale test + restore suite setup/teardown

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

* fix robocop alerts

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

* fix post install test - change label

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
Reviewers

@lugi0 lugi0 lugi0 approved these changes

@jstourac jstourac jstourac approved these changes

@jgarciao jgarciao Awaiting requested review from jgarciao

@FedeAlonso FedeAlonso Awaiting requested review from FedeAlonso

@tarukumar tarukumar Awaiting requested review from tarukumar

Assignees

@bdattoma bdattoma

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bdattoma @jstourac @lugi0