feat(inferenceservice): added integration tests to inferenceservice reconcile function #574
Conversation
google-oss-prow
bot
requested review from
andreyvelich,
rareddy and
tarilabs
| @@ -0,0 +1,16330 @@ | |||
| --- | |||
There was a problem hiding this comment.
question: instead of having this file locally, can we use from kserve repo like:
https://github.com/kserve/kserve/blob/release-0.13/charts/kserve-crd/templates/serving.kserve.io_inferenceservices.yaml
by indicating a link of sort to envtest?
| return fmt.Sprintf("%s.%s.svc.cluster.local:%d", name, namespace, *restApiPort), nil | ||
| return fmt.Sprintf("http://%s.%s.svc.cluster.local:%d", name, namespace, *restApiPort), nil |
There was a problem hiding this comment.
wouldn't hardcoding http:// be a problem midstream/downstream of Kubeflow where Distributions may default to mTLS? 🤔
There was a problem hiding this comment.
I would leave it like that for a default configuration and let the user decide by using the label modelregistry.kubeflow.org/url for every other scenario mLTS/external route/etc
Signed-off-by: Alessio Pragliola <seth.pro@gmail.com>
Al-Pragliola
force-pushed
the
al-pragliola-inferenceservice-tests
branch
from
7e39107 to
bee3508
Compare
Signed-off-by: Alessio Pragliola <seth.pro@gmail.com>
There was a problem hiding this comment.
thank you for following up on the comments @Al-Pragliola
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tarilabs 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 |
Description
This pull request introduces integration tests for the new inferenceservice reconcile feature, and I chose to use
envtestto test the functionality, as it is the de facto standard for testing controller-like behavior. Since envtest is not a real Kubernetes cluster, I had to mock responses from the model registry, as you can see in the suite_test.go file, and check that the inference-service-id label is added to the kserve inferenceservice CR. I also found some bugs in the reconciler thanks to the tests, and fixed them in this PR.NOTE: The large chunk of code in this PR comes from the CRD that needs to be added to the envtest cluster to make it handle inference services.
How Has This Been Tested?
make testMerge criteria:
DCOcheck)