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

Url and Address Url set in TrainedModel Status using InferenceService Status #1319

Merged
merged 11 commits into from
Jan 29, 2021
2 changes: 2 additions & 0 deletions config/crd/serving.kubeflow.org_trainedmodels.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ spec:
observedGeneration:
format: int64
type: integer
url:
type: string
type: object
type: object
version: v1alpha1
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/serving/v1alpha1/trained_model_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,18 @@ limitations under the License.
package v1alpha1

import (
"knative.dev/pkg/apis"
duckv1 "knative.dev/pkg/apis/duck/v1"
)

// TrainedModelStatus defines the observed state of TrainedModel
type TrainedModelStatus struct {
// Conditions for trained model
duckv1.Status `json:",inline"`
// URL holds the url that will distribute traffic over the provided traffic targets.
// For v1: http[s]://{route-name}.{route-namespace}.{cluster-level-suffix}/v1/models/<trainedmodel>:predict
// For v2: http[s]://{route-name}.{route-namespace}.{cluster-level-suffix}/v2/models/<trainedmodel>/infer
URL *apis.URL `json:"url,omitempty"`
// Addressable endpoint for the deployed trained model
// http://<inferenceservice.metadata.name>/v1/models/<trainedmodel>.metadata.name
Address *duckv1.Addressable `json:"address,omitempty"`
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

63 changes: 61 additions & 2 deletions pkg/controller/v1alpha1/trainedmodel/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,14 @@ import (
"github.com/kubeflow/kfserving/pkg/constants"
"github.com/kubeflow/kfserving/pkg/controller/v1alpha1/trainedmodel/reconcilers/modelconfig"
"github.com/kubeflow/kfserving/pkg/utils"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/record"
"knative.dev/pkg/apis"
duckv1 "knative.dev/pkg/apis/duck/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand Down Expand Up @@ -118,15 +122,70 @@ func (r *TrainedModelReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error
return ctrl.Result{}, nil
}

// update URL and Address fo TrainedModel
if err := r.updateStatus(req, tm); err != nil {
return ctrl.Result{}, err
}

// Reconcile modelconfig to add this TrainedModel to its parent InferenceService's configmap
if err := r.ModelConfigReconciler.Reconcile(req, tm); err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}

func (r *TrainedModelReconciler) updateStatus(desiredService *v1alpha1api.TrainedModel) error {
//TODO update TrainedModel status object, this will be done in a separate PR
func (r *TrainedModelReconciler) updateStatus(req ctrl.Request, desiredModel *v1alpha1api.TrainedModel) error {
// Get the parent inference service
isvc := &v1beta1api.InferenceService{}
if err := r.Get(context.TODO(), types.NamespacedName{Namespace: req.Namespace, Name: desiredModel.Spec.InferenceService}, isvc); err != nil {
return err
}

// Check if parent inference service has the status URL
if isvc.Status.URL != nil {
// Update status to contain the isvc URL with /v1/models/trained-model-name:predict appened
url := isvc.Status.URL.String() + constants.PredictPath(desiredModel.Name, isvc.Spec.Predictor.GetImplementation().GetProtocol())
externURL, err := apis.ParseURL(url)
if err != nil {
return err
}
desiredModel.Status.URL = externURL
}

// Check if parent inference service has the address URL
if isvc.Status.Address != nil {
if isvc.Status.Address.URL != nil {
////Update status to contain the isvc address with /v1/models/trained-model-name:predict appened
url := isvc.Status.Address.URL.String() + constants.PredictPath(desiredModel.Name, isvc.Spec.Predictor.GetImplementation().GetProtocol())
clusterURL, err := apis.ParseURL(url)
if err != nil {
return err
}
desiredModel.Status.Address = &duckv1.Addressable{
URL: clusterURL,
}
}
}

// Get the current model
existingModel := &v1alpha1api.TrainedModel{}
if err := r.Get(context.TODO(), req.NamespacedName, existingModel); err != nil {
if errors.IsNotFound(err) {
return nil
}
return err
}

if equality.Semantic.DeepEqual(existingModel.Status, desiredModel.Status) {
// We did not update anything
} else {
// Try to update model
if err := r.Status().Update(context.TODO(), desiredModel); err != nil {
r.Recorder.Eventf(desiredModel, v1.EventTypeWarning, "UpdateFailed",
"Failed to update status for TrainedModel %q: %v", desiredModel.Name, err)
}
}

return nil
}

Expand Down
41 changes: 35 additions & 6 deletions pkg/controller/v1alpha1/trainedmodel/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package trainedmodel
import (
"context"
"github.com/golang/protobuf/proto"
"github.com/google/go-cmp/cmp"
v1alpha1api "github.com/kubeflow/kfserving/pkg/apis/serving/v1alpha1"
"github.com/kubeflow/kfserving/pkg/apis/serving/v1beta1"
"github.com/kubeflow/kfserving/pkg/constants"
Expand All @@ -28,16 +29,20 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"knative.dev/pkg/apis"
duckv1 "knative.dev/pkg/apis/duck/v1"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"time"
)

var _ = Describe("v1beta1 TrainedModel controller", func() {
// Define utility constants for object names and testing timeouts/durations and intervals.
const (
timeout = time.Second * 20
duration = time.Second * 20
interval = time.Millisecond * 250
timeout = time.Second * 20
duration = time.Second * 20
interval = time.Millisecond * 250
domain = "example.com"
clusterIp = "example.svc.local.cluster"
)

var (
Expand Down Expand Up @@ -227,6 +232,24 @@ var _ = Describe("v1beta1 TrainedModel controller", func() {
}
Expect(k8sClient.Create(ctx, isvc)).Should(Succeed())

inferenceService := &v1beta1.InferenceService{}
Eventually(func() bool {
err := k8sClient.Get(ctx, serviceKey, inferenceService)
if err != nil {
return false
}
return true
}, timeout, interval).Should(BeTrue())

// Updates the url and address of inference service status
predictorUrl, _ := apis.ParseURL("http://" + constants.InferenceServiceHostName(constants.DefaultPredictorServiceName(serviceKey.Name), serviceKey.Namespace, domain))
clusterURL, _ := apis.ParseURL("http://" + constants.InferenceServiceHostName(constants.DefaultPredictorServiceName(serviceKey.Name), serviceKey.Namespace, clusterIp))
inferenceService.Status.URL = predictorUrl
inferenceService.Status.Address = &duckv1.Addressable{
URL: clusterURL,
}
Expect(k8sClient.Status().Update(context.TODO(), inferenceService)).To(BeNil())

tmInstance := &v1alpha1api.TrainedModel{
ObjectMeta: metav1.ObjectMeta{
Name: modelName,
Expand Down Expand Up @@ -260,12 +283,18 @@ var _ = Describe("v1beta1 TrainedModel controller", func() {
return false
}
if len(tmInstanceUpdate.Finalizers) > 0 {
return true
} else {
return false
if tmInstanceUpdate.Status.Address != nil {
return tmInstanceUpdate.Status.Address.URL != nil && tmInstanceUpdate.Status.URL != nil
}
}
return false
}, timeout).Should(BeTrue())

tmExpectedURL := predictorUrl.String() + constants.PredictPath(tmInstanceUpdate.Name, isvc.Spec.Predictor.GetImplementation().GetProtocol())
tmExpectedAddressURL := clusterURL.String() + constants.PredictPath(tmInstanceUpdate.Name, isvc.Spec.Predictor.GetImplementation().GetProtocol())
Expect(cmp.Diff(tmInstanceUpdate.Status.URL.String(), tmExpectedURL)).To(Equal(""))
Expect(cmp.Diff(tmInstanceUpdate.Status.Address.URL.String(), tmExpectedAddressURL)).To(Equal(""))

updatedModelUri := "s3//model2"
tmInstanceUpdate.Spec.Model.StorageURI = updatedModelUri
Expect(k8sClient.Update(context.TODO(), tmInstanceUpdate)).NotTo(HaveOccurred())
Expand Down