[RHOAIENG-12726] - odh-model-controller crashes with empty isvc.model…

… field (#264) chore: fix nil pointer if the custom isvc does not have the model field. Signed-off-by: Spolti <[email protected]>
opendatahub-io · Sep 13, 2024 · 479815e · 479815e
1 parent 8794179
commit 479815e
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 6 deletions.
diff --git a/controllers/kserve_inferenceservice_controller_metrics_test.go b/controllers/kserve_inferenceservice_controller_metrics_test.go
@@ -20,9 +20,11 @@ const (
 	KserveOvmsInferenceServiceName         = "example-onnx-mnist"
 	UnsupportedMetricsInferenceServiceName = "sklearn-v2-iris"
 	NilRuntimeInferenceServiceName         = "sklearn-v2-iris-no-runtime"
+	NilModelInferenceServiceName           = "custom-runtime"
 	UnsupportedMetricsInferenceServicePath = "./testdata/deploy/kserve-unsupported-metrics-inference-service.yaml"
 	UnsupprtedMetricsServingRuntimePath    = "./testdata/deploy/kserve-unsupported-metrics-serving-runtime.yaml"
 	NilRuntimeInferenceServicePath         = "./testdata/deploy/kserve-nil-runtime-inference-service.yaml"
+	NilModelInferenceServicePath           = "./testdata/deploy/kserve-nil-model-inference-service.yaml"
 )
 
 var _ = Describe("The KServe Dashboard reconciler", func() {
@@ -126,6 +128,25 @@ var _ = Describe("The KServe Dashboard reconciler", func() {
 			}
 			Expect(compareConfigMap(metricsConfigMap, expectedmetricsConfigMap)).Should(BeTrue())
 		})
+
+		It("if the isvc does not have the model field specified, an unsupported metrics configmap should be created", func() {
+			_ = createInferenceService(testNs, NilModelInferenceServiceName, NilModelInferenceServicePath)
+
+			metricsConfigMap, err := waitForConfigMap(cli, testNs, NilModelInferenceServiceName+constants.KserveMetricsConfigMapNameSuffix, 30, 1*time.Second)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(metricsConfigMap).NotTo(BeNil())
+
+			expectedCM := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      NilModelInferenceServiceName + constants.KserveMetricsConfigMapNameSuffix,
+					Namespace: testNs,
+				},
+				Data: map[string]string{
+					"supported": "false",
+				},
+			}
+			Expect(compareConfigMap(metricsConfigMap, expectedCM)).Should(BeTrue())
+		})
 	})
 
 	When("deleting the deployed models", func() {
@@ -147,7 +168,7 @@ var _ = Describe("The KServe Dashboard reconciler", func() {
 			Expect(cli.Delete(ctx, SklearnInferenceService)).Should(Succeed())
 			Eventually(func() error {
 				configmap := &corev1.ConfigMap{}
-				key := types.NamespacedName{Name: KserveOvmsInferenceServiceName + constants.KserveMetricsConfigMapNameSuffix, Namespace: SklearnInferenceService.Namespace}
+				key := types.NamespacedName{Name: UnsupportedMetricsInferenceServiceName + constants.KserveMetricsConfigMapNameSuffix, Namespace: SklearnInferenceService.Namespace}
 				err := cli.Get(ctx, key, configmap)
 				return err
 			}, timeout, interval).ShouldNot(Succeed())

diff --git a/controllers/reconcilers/kserve_metrics_dashboard_reconciler.go b/controllers/reconcilers/kserve_metrics_dashboard_reconciler.go
@@ -95,17 +95,24 @@ func (r *KserveMetricsDashboardReconciler) createDesiredResource(ctx context.Con
 	var servingRuntime string
 	runtime := &kservev1alpha1.ServingRuntime{}
 	supported := false
+
+	// there is the possibility to also have and nil model field, e.g:
+	//predictor:
+	//	containers:
+	//		- name: kserve-container
+	//		image: user/custom-model:v1
+	if nil == isvc.Spec.Predictor.Model {
+		log.V(1).Info("no `predictor.model` field found in InferenceService, no metrics will be available")
+		return r.createConfigMap(isvc, false, log)
+	}
+
 	// resolve SR
 	isvcRuntime := isvc.Spec.Predictor.Model.Runtime
 	if isvcRuntime == nil {
 		runtime, err = utils.FindSupportingRuntimeForISvc(ctx, r.client, log, isvc)
 		if err != nil {
 			if errwrap.Contains(err, constants.NoSuitableRuntimeError) {
-				configmap, err := r.createConfigMap(isvc, false, log)
-				if err != nil {
-					return nil, err
-				}
-				return configmap, nil
+				return r.createConfigMap(isvc, false, log)
 			}
 			return nil, err
 		}

diff --git a/controllers/testdata/deploy/kserve-nil-model-inference-service.yaml b/controllers/testdata/deploy/kserve-nil-model-inference-service.yaml
@@ -0,0 +1,10 @@
+apiVersion: serving.kserve.io/v1beta1
+kind: InferenceService
+metadata:
+  name: custom-model
+  namespace: default
+spec:
+  predictor:
+    containers:
+      - name: kserve-container
+        image: user/custom-model:v1