Added logic to update ModelStatus #2088

Suresh-Nakkeran · 2022-03-14T17:30:42Z

Signed-off-by: Suresh Nakkeran suresh.n@ideas2it.com

As part of this PR, ModelStatus field is added in InferenceServiceStatus struct which will be used for updating single model deployment status and ModelMesh predictor status.

Suresh-Nakkeran · 2022-03-16T12:56:24Z

/retest

pkg/controller/v1beta1/inferenceservice/components/predictor.go

yuzisun · 2022-03-26T03:03:29Z

@Suresh-Nakkeran Please help rebase paul's status API PR

pkg/apis/serving/v1beta1/inference_service_status.go

pkg/controller/v1beta1/inferenceservice/components/predictor.go

pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go

njhill · 2022-04-04T21:50:03Z

pkg/apis/serving/v1beta1/inference_service_status.go

+	ss.ModelStatus.TransitionStatus = status
+	// Update model state to 'FailedToLoad' in case of invalid spec provided
+	if ss.ModelStatus.TransitionStatus == InvalidSpec {
+		ss.UpdateModelRevisionStates(FailedToLoad, info)


This will overwrite TransitionStatus to be BlockedByFailedLoad which I don't think is what we want here?

I'm not sure it makes sense to update the model state here at all actually since an invalid spec doesn't affect the active model state, just means we can't fully reconcile.

It does not affect the active model, but invalid spec would result in model deployment failure and we should surface this error back to the user right?

Yes it will still be reflected in the transitionStatus of InvalidSpec and the failureInfo. I thought the distinction made sense since it isn't a model loading failure per se, the load isn't even attempted because the spec is invalid.

@Suresh-Nakkeran I agree here that we can remove the code updating to FailedToLoad, the model loading has not started yet.

okay @yuzisun

njhill · 2022-04-04T21:59:54Z

pkg/controller/v1beta1/inferenceservice/components/predictor.go

@@ -91,15 +94,27 @@ func (p *Predictor) Reconcile(isvc *v1beta1.InferenceService) error {
 		if isvc.Spec.Predictor.Model.Runtime != nil {
 			r, err := isvcutils.GetServingRuntime(p.client, *isvc.Spec.Predictor.Model.Runtime, isvc.Namespace)
 			if err != nil {
+				isvc.Status.UpdateModelTransitionStatus(v1beta1.InvalidSpec, &v1beta1.FailureInfo{


Better for InvalidSpec here to be BlockedByFailedLoad?

@njhill I think here it should be InvalidSpec as it has not got to the point to load the model yet?

njhill · 2022-04-04T22:00:17Z

pkg/controller/v1beta1/inferenceservice/components/predictor.go

 				return err
 			}

 			if r.IsDisabled() {
+				isvc.Status.UpdateModelTransitionStatus(v1beta1.InvalidSpec, &v1beta1.FailureInfo{


Better for InvalidSpec here to be BlockedByFailedLoad?

njhill · 2022-04-04T22:00:22Z

pkg/controller/v1beta1/inferenceservice/components/predictor.go

 				return fmt.Errorf("specified runtime %s is disabled", *isvc.Spec.Predictor.Model.Runtime)
 			}

 			// Verify that the selected runtime supports the specified framework.
 			if !isvc.Spec.Predictor.Model.RuntimeSupportsModel(r) {
+				isvc.Status.UpdateModelTransitionStatus(v1beta1.InvalidSpec, &v1beta1.FailureInfo{


Better for InvalidSpec here to be BlockedByFailedLoad?

njhill · 2022-04-04T22:00:34Z

pkg/controller/v1beta1/inferenceservice/components/predictor.go

@@ -110,22 +125,38 @@ func (p *Predictor) Reconcile(isvc *v1beta1.InferenceService) error {
 				return err
 			}
 			if len(runtimes) == 0 {
+				isvc.Status.UpdateModelTransitionStatus(v1beta1.InvalidSpec, &v1beta1.FailureInfo{


Better for InvalidSpec here to be BlockedByFailedLoad?

pkg/controller/v1beta1/inferenceservice/components/predictor.go

pkg/apis/serving/v1beta1/inference_service_status.go

Suresh-Nakkeran · 2022-04-27T04:52:31Z

/retest

pkg/controller/v1beta1/inferenceservice/components/predictor.go

pkg/apis/serving/v1beta1/inference_service_status.go

Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com>

andyi2it · 2022-05-26T11:54:22Z

/retest

yuzisun · 2022-05-28T15:00:13Z

/lgtm
/approve

kserve-oss-bot · 2022-05-28T15:00:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Suresh-Nakkeran, yuzisun

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~~ [yuzisun]

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

…252) * Update end to end test to cover RAW_DEPLOYMENT for model with logger * Add rbac definition from kserve/kserve#2088

* added logic to update model status Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com> * added tests for modelstatus support Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com> * model status changes - incorporated review commands Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com> * adding more tests for modelstatus changes Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com> Signed-off-by: alexagriffith <agriffith96@gmail.com>

…araml-dev#252) * Update end to end test to cover RAW_DEPLOYMENT for model with logger * Add rbac definition from kserve/kserve#2088

kserve-oss-bot requested review from Iamlovingit and Tomcli March 14, 2022 17:30

pvaneck mentioned this pull request Mar 15, 2022

feat: Add ModelMesh related status fields to isvc #2084

Merged

Suresh-Nakkeran force-pushed the add-modelstatus-support branch 7 times, most recently from 26993a4 to 058d946 Compare March 16, 2022 11:48

yuzisun reviewed Mar 22, 2022

View reviewed changes

pkg/controller/v1beta1/inferenceservice/components/predictor.go Outdated Show resolved Hide resolved

yuzisun reviewed Mar 22, 2022

View reviewed changes

pkg/controller/v1beta1/inferenceservice/components/predictor.go Outdated Show resolved Hide resolved

Suresh-Nakkeran force-pushed the add-modelstatus-support branch from 058d946 to 66ad604 Compare March 28, 2022 12:31

Suresh-Nakkeran changed the title ~~Added ModelStatus field to InferenceServiceStatus~~ Added logic to update ModelStatus Mar 28, 2022

alexagriffith reviewed Mar 30, 2022

View reviewed changes

pkg/apis/serving/v1beta1/inference_service_status.go Outdated Show resolved Hide resolved

yuzisun reviewed Apr 1, 2022

View reviewed changes

pkg/apis/serving/v1beta1/inference_service_status.go Outdated Show resolved Hide resolved

pkg/apis/serving/v1beta1/inference_service_status.go Outdated Show resolved Hide resolved

pkg/controller/v1beta1/inferenceservice/components/predictor.go Outdated Show resolved Hide resolved

yuzisun reviewed Apr 1, 2022

View reviewed changes

pkg/controller/v1beta1/inferenceservice/components/predictor.go Outdated Show resolved Hide resolved

Suresh-Nakkeran force-pushed the add-modelstatus-support branch 2 times, most recently from 7d05a72 to 358941b Compare April 1, 2022 10:43

yuzisun reviewed Apr 3, 2022

View reviewed changes

pkg/controller/v1beta1/inferenceservice/rawkube_controller_test.go Outdated Show resolved Hide resolved

Suresh-Nakkeran force-pushed the add-modelstatus-support branch 4 times, most recently from b6b282f to c5a8529 Compare April 4, 2022 12:31

njhill reviewed Apr 5, 2022

View reviewed changes

Suresh-Nakkeran force-pushed the add-modelstatus-support branch 3 times, most recently from b9d90b1 to ffb904f Compare April 7, 2022 15:59

Suresh-Nakkeran force-pushed the add-modelstatus-support branch 2 times, most recently from 88cb9c4 to ee92445 Compare April 27, 2022 02:06

yuzisun reviewed Apr 28, 2022

View reviewed changes

pkg/controller/v1beta1/inferenceservice/components/predictor.go Outdated Show resolved Hide resolved

Suresh-Nakkeran force-pushed the add-modelstatus-support branch from b437494 to f706f71 Compare April 28, 2022 16:11

yuzisun mentioned this pull request May 1, 2022

KServe 0.9 release tracking #2168

Closed

15 tasks

Suresh-Nakkeran force-pushed the add-modelstatus-support branch 3 times, most recently from c95d413 to abd1300 Compare May 16, 2022 08:00

yuzisun reviewed May 21, 2022

View reviewed changes

pkg/apis/serving/v1beta1/inference_service_status.go Outdated Show resolved Hide resolved

Suresh-Nakkeran added 3 commits May 23, 2022 07:35

added logic to update model status

c3b0318

Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com>

added tests for modelstatus support

6af6eed

Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com>

model status changes - incorporated review commands

74d2d3c

Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com>

Suresh-Nakkeran force-pushed the add-modelstatus-support branch 2 times, most recently from ed0c687 to 682eb95 Compare May 23, 2022 02:44

adding more tests for modelstatus changes

c1b4b7d

Signed-off-by: Suresh Nakkeran <suresh.n@ideas2it.com>

Suresh-Nakkeran force-pushed the add-modelstatus-support branch from 682eb95 to c1b4b7d Compare May 24, 2022 09:31

yuzisun mentioned this pull request May 25, 2022

Validate ServingRuntime when deploying InferenceService #2196

Open

kserve-oss-bot assigned yuzisun May 28, 2022

kserve-oss-bot added the lgtm label May 28, 2022

kserve-oss-bot added the approved label May 28, 2022

kserve-oss-bot merged commit 1a55119 into kserve:master May 28, 2022

pradithya pushed a commit to caraml-dev/merlin that referenced this pull request Jun 2, 2022

Add rbac definition from kserve/kserve#2088

0a40b07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added logic to update ModelStatus #2088

Added logic to update ModelStatus #2088

Suresh-Nakkeran commented Mar 14, 2022

Suresh-Nakkeran commented Mar 16, 2022

yuzisun commented Mar 26, 2022

njhill Apr 4, 2022

yuzisun Apr 5, 2022

njhill Apr 5, 2022

yuzisun May 24, 2022

Suresh-Nakkeran May 25, 2022

njhill Apr 4, 2022

yuzisun May 24, 2022

njhill Apr 4, 2022

njhill Apr 4, 2022

njhill Apr 4, 2022

Suresh-Nakkeran commented Apr 27, 2022

andyi2it commented May 26, 2022

yuzisun commented May 28, 2022

kserve-oss-bot commented May 28, 2022

Added logic to update ModelStatus #2088

Added logic to update ModelStatus #2088

Conversation

Suresh-Nakkeran commented Mar 14, 2022

Suresh-Nakkeran commented Mar 16, 2022

yuzisun commented Mar 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Suresh-Nakkeran commented Apr 27, 2022

andyi2it commented May 26, 2022

yuzisun commented May 28, 2022

kserve-oss-bot commented May 28, 2022