Fix mr reconciler

opendatahub-io · Nov 27, 2023 · 8e5397e · 8e5397e
1 parent 861d086
commit 8e5397e
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 18 deletions.
diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml
@@ -168,6 +168,7 @@ rules:
   - inferenceservices
   verbs:
   - create
+  - delete
   - get
   - list
   - update

diff --git a/controllers/reconcilers/mr_inferenceservice_reconciler.go b/controllers/reconcilers/mr_inferenceservice_reconciler.go
@@ -66,14 +66,25 @@ func (r *ModelRegistryInferenceServiceReconciler) Reconcile(ctx context.Context,
 
 	// For each inference, check if the corresponding one exist in the cluster
 	for _, is := range isListByRuntime {
+		if is.Id == nil {
+			log.Error(fmt.Errorf("missing id for InferenceService"), "Retrieved InferenceService from model registry is missing its id, skipping")
+			continue
+		}
+
+		// initialize with the id and override with the name if provided
+		isName := *is.Id
+		if is.Name != nil {
+			isName = *is.Name
+		}
+
 		isvc, err := r.findInferenceService(ctx, log, &is, namespace)
 		if err != nil {
 			log.Error(err, "Unable to find inference service")
 		} else if isvc == nil && is.State != nil && *is.State == openapi.INFERENCESERVICESTATE_DEPLOYED {
 			// ISVC not existing in the namespace and InferenceService is in DEPLOYED state in model registry
 			// Create new ISVC in the provided namespace for the corresponding InferenceService in model registry
-			log.Info("ISVC not found for InferenceService: " + *is.Id)
-			isvc, err := r.createDesiredInferenceService(ctx, log, namespace, mrClient, &is, servingRuntimeName)
+			log.Info("ISVC not found for InferenceService " + isName + " and InferenceService state is DEPLOYED. Creating new ISVC.")
+			isvc, err := r.createDesiredInferenceService(ctx, log, namespace, isName, mrClient, &is, servingRuntimeName)
 			if err != nil {
 				log.Error(err, "Something went wrong creating desired InferenceService CR")
 				continue
@@ -86,38 +97,41 @@ func (r *ModelRegistryInferenceServiceReconciler) Reconcile(ctx context.Context,
 					log.Error(err, "Something went wrong updating ServeModel state to FAILED in model registry")
 				}
 			} else {
-				log.Info("Created desired ISVC: " + isvc.Name)
+				log.Info("Created desired ISVC: " + isName)
 				if _, err = r.logServeModel(mrClient, &is); err != nil {
 					log.Error(err, "Something went wrong logging ServeModel in model registry")
 				}
 			}
 		} else if isvc != nil && (is.State == nil || *is.State == openapi.INFERENCESERVICESTATE_UNDEPLOYED) {
 			// ISVC already existing in the namespace and InferenceService is not in DEPLOYED state in model registry
 			// Delete existing ISCV from the provided namespace
-			log.Info("ISVC already existing, removing inference: " + isvc.Name)
+			log.Info("ISVC already existing, removing inference: " + isName)
 
 			// Remove existing ISVC
 			if err = r.client.Delete(ctx, isvc); err != nil {
-				log.Error(err, "Something went wrong deleting InferenceService CR: "+*is.Name)
+				log.Error(err, "Something went wrong deleting InferenceService CR: "+isName)
 				_, err := r.updateServeModel(mrClient, &is, openapi.EXECUTIONSTATE_UNKNOWN)
 				if err != nil {
 					log.Error(err, "Something went wrong updating ServeModel state to UNKNOWN in model registry")
 				}
 			} else {
-				log.Info("Deleted ISVC: " + isvc.Name)
+				log.Info("Deleted ISVC: " + isName)
 				if _, err = r.updateServeModel(mrClient, &is, openapi.EXECUTIONSTATE_COMPLETE); err != nil {
 					log.Error(err, "Something went wrong updating ServeModel state to COMPLETE in model registry")
 				}
 			}
-		} else {
+		} else if isvc != nil {
 			// ISVC already existing in the namespace and InferenceService is in DEPLOYED state in model registry
 			// Update the ServeModel to RUNNING state, no changes in the cluster
-			log.Info("ISVC already existing: " + isvc.Name)
+			log.Info("ISVC already existing: " + isName)
 
 			// TODO: we could add some checks in the ISVC status to assure is really running
 			if _, err = r.updateServeModel(mrClient, &is, openapi.EXECUTIONSTATE_RUNNING); err != nil {
 				log.Error(err, "Something went wrong updating ServeModel state to RUNNING in model registry")
 			}
+		} else {
+			// ISVC not existing but InferenceService in model registry is not in DEPLOYED state
+			log.Info("ISVC not existing for " + isName + " but InferenceService state is not DEPLOYED. Do nothing.")
 		}
 	}
 
@@ -149,7 +163,7 @@ func (r *ModelRegistryInferenceServiceReconciler) findInferenceService(ctx conte
 }
 
 // createDesiredInferenceService fetch all required data from model registry and create a new kservev1beta1.InferenceService CR object based on that data
-func (r *ModelRegistryInferenceServiceReconciler) createDesiredInferenceService(ctx context.Context, log logr.Logger, namespace string, mrClient mrapi.ModelRegistryApi, is *openapi.InferenceService, servingEnvironmentName string) (*kservev1beta1.InferenceService, error) {
+func (r *ModelRegistryInferenceServiceReconciler) createDesiredInferenceService(ctx context.Context, log logr.Logger, namespace string, isName string, mrClient mrapi.ModelRegistryApi, is *openapi.InferenceService, servingEnvironmentName string) (*kservev1beta1.InferenceService, error) {
 	// Fetch the required information from model registry
 	modelVersion, err := mrClient.GetModelVersionByInferenceService(*is.Id)
 	if err != nil {
@@ -169,23 +183,23 @@ func (r *ModelRegistryInferenceServiceReconciler) createDesiredInferenceService(
 	modelArtifact := &modelArtifactList.Items[0]
 
 	// Assemble the desired InferenceService CR
-	desiredInferenceService := r.assembleInferenceService(namespace, *is, *modelArtifact, true)
+	desiredInferenceService := r.assembleInferenceService(namespace, isName, *is, *modelArtifact, true)
 
 	return desiredInferenceService, nil
 }
 
 // assembleInferenceService create a new kservev1beta1.InferenceService CR object based on the provided data
-func (r *ModelRegistryInferenceServiceReconciler) assembleInferenceService(namespace string, is openapi.InferenceService, modelArtifact openapi.ModelArtifact, isModelMesh bool) *kservev1beta1.InferenceService {
+func (r *ModelRegistryInferenceServiceReconciler) assembleInferenceService(namespace string, isName string, is openapi.InferenceService, modelArtifact openapi.ModelArtifact, isModelMesh bool) *kservev1beta1.InferenceService {
 	desiredInferenceService := &kservev1beta1.InferenceService{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      *is.Name,
+			Name:      isName,
 			Namespace: namespace,
 			Annotations: map[string]string{
-				"openshift.io/display-name": *is.Name,
+				"openshift.io/display-name": isName,
 			},
 			Labels: map[string]string{
 				constants.ModelRegistryInferenceServiceLabel: *is.Id,
-				"name":                     *is.Name,
+				"name":                     isName,
 				"opendatahub.io/dashboard": "true",
 			},
 		},

diff --git a/go.mod b/go.mod
@@ -7,7 +7,7 @@ require (
 	github.com/kserve/kserve v0.11.0
 	github.com/onsi/ginkgo v1.16.4
 	github.com/onsi/gomega v1.27.6
-	github.com/opendatahub-io/model-registry v0.0.0-20231123100425-e0f29251df1f
+	github.com/opendatahub-io/model-registry v0.0.0-20231127083511-2fac15092d90
 	github.com/openshift/api v3.9.0+incompatible
 	github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.64.1
 	go.uber.org/zap v1.24.0

diff --git a/go.sum b/go.sum
@@ -221,8 +221,8 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8
 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
 github.com/opencontainers/image-spec v1.1.0-rc5 h1:Ygwkfw9bpDvs+c9E34SdgGOj41dX/cbdlwvlWt0pnFI=
 github.com/opencontainers/runc v1.1.5 h1:L44KXEpKmfWDcS02aeGm8QNTFXTo2D+8MYGDIJ/GDEs=
-github.com/opendatahub-io/model-registry v0.0.0-20231123100425-e0f29251df1f h1:ROffEYkqL8yI1RON8V5jhXoB8UL+aXwqIqY8bXme7o0=
-github.com/opendatahub-io/model-registry v0.0.0-20231123100425-e0f29251df1f/go.mod h1:H/WFSBsgo0Fd9n2tl5vQ9FHQHWCt3pLqd8pHfh5GMHc=
+github.com/opendatahub-io/model-registry v0.0.0-20231127083511-2fac15092d90 h1:Q/di435Z4oqHeMxFossgPLFhXMFjEjnWONyhuj+I2sE=
+github.com/opendatahub-io/model-registry v0.0.0-20231127083511-2fac15092d90/go.mod h1:H/WFSBsgo0Fd9n2tl5vQ9FHQHWCt3pLqd8pHfh5GMHc=
 github.com/openshift/api v3.9.0+incompatible h1:fJ/KsefYuZAjmrr3+5U9yZIZbTOpVkDDLDLFresAeYs=
 github.com/openshift/api v3.9.0+incompatible/go.mod h1:dh9o4Fs58gpFXGSYfnVxGR9PnV53I8TW84pQaJDdGiY=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=

diff --git a/main.go b/main.go
@@ -53,7 +53,7 @@ var (
 
 // ClusterRole permissions
 
-// +kubebuilder:rbac:groups=serving.kserve.io,resources=inferenceservices,verbs=get;list;watch;create;update
+// +kubebuilder:rbac:groups=serving.kserve.io,resources=inferenceservices,verbs=get;list;watch;create;update;delete
 // +kubebuilder:rbac:groups=serving.kserve.io,resources=inferenceservices/finalizers,verbs=get;list;watch;update
 // +kubebuilder:rbac:groups=serving.kserve.io,resources=servingruntimes,verbs=get;list;watch;create;update
 // +kubebuilder:rbac:groups=serving.kserve.io,resources=servingruntimes/finalizers,verbs=get;list;watch;create;update;patch;delete
-Original file line number
+Diff line change
@@ Expand Up / @@ -168,6 +168,7 @@ rules: @@
       - inferenceservices
       verbs:
       - create
+      - delete
       - get
       - list
       - update
@@ Expand Down @@