-
Notifications
You must be signed in to change notification settings - Fork 10
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
add Shell script openshift-ci make some REST call to MR #295
Conversation
"mr-registered-model-id": "$rm_id" | ||
"mr-model-version-id": "$mv_id" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the K8s ISVC is created with the IDs from the MR REST calls
# TODO this will continue once we have MC PR merged from: https://github.com/opendatahub-io/odh-model-controller/pull/135 | ||
iss_mr=$(curl -s -X 'GET' "$MR_HOSTNAME/api/model_registry/v1alpha1/inference_services" \ | ||
-H 'accept: application/json') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As confirmed to offline review, this will eventually transform into a loop-while, waiting for the condition than some InferenceService entity on MR is valorized with the registeredModelId, modelVersionId as needed.
(once opendatahub-io/odh-model-controller#135 merged)
This described change, I suggest to be done in a subsequent PR, as this covers already for basic REST functionality.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #295 +/- ##
=======================================
Coverage 76.42% 76.42%
=======================================
Files 17 17
Lines 1909 1909
Branches 73 73
=======================================
Hits 1459 1459
Misses 276 276
Partials 174 174 ☔ View full report in Codecov by Sentry. |
In the scope of testing of Model Registry in openshift-ci: - make a Shell script which invokes some REST calls to MR, - so to make sure the REST endpoint is responsive, - then create a K8s ISVC on the cluster, - and display the MR InferenceService entities. Later, in a subsequent issue/PR, once: opendatahub-io/odh-model-controller#135 is merged, the last bulletpoint can be automated and placed under test in the final part of this script so to make sure the K8s ISVC on the cluster reflected as a precise MR InferenceService entity.
fa2b5a4
to
187cbb4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good starting point, I think this could be refined in subsequent PRs if needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is great for a starting point and /lgtm. Only thing I might add is to remove the entity using a rest api call once the reconciliation step is completed succesfully. That can be done later though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, just a nit on the shebang highlighted below :) still, I expect that we're not going to use this script a lot outside of the CI setup, so feel free to merge whatever works.
there is no concept of deletion in MLMD and consequently no concept of deletion also on MR. |
In the scope of testing of Model Registry in openshift-ci:
Later, in a subsequent issue/PR, once:
is merged, the last bulletpoint can be automated and placed under test in the final part of this script so to make sure the K8s ISVC on the cluster reflected as a precise MR InferenceService entity.
Description
Resolves #294
How Has This Been Tested?
Tested with:
OCP_CLUSTER_NAME="rosa.mmortari-rosa-h.w0x4.p3.openshiftapps.com"
Result:
Merge criteria: