Implementation of ODH defaulters for InferenceGraph and InferenceService

On creation of InferenceGraph or InferenceService resources, the following default annotations will be added: * `serving.knative.openshift.io/enablePassthrough: true` * `sidecar.istio.io/inject: true` * `sidecar.istio.io/rewriteAppHTTPProbers: true` The annotations are added only for Serverless mode, and only if they are missing. Signed-off-by: Edgar Hernández <[email protected]>
israel-hdez · Dec 18, 2024 · 35a5bea · 35a5bea
1 parent 3a45bac
commit 35a5bea
Show file tree

Hide file tree

Showing 18 changed files with 643 additions and 38 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -0,0 +1 @@
+Containerfile
diff --git a/PROJECT b/PROJECT
@@ -30,6 +30,7 @@ resources:
   version: v1beta1
   webhooks:
     validation: true
+    defaulting: true
     webhookVersion: v1
 - controller: true
   core: true
@@ -59,4 +60,13 @@ resources:
   webhooks:
     validation: true
     webhookVersion: v1
+- domain: kserve.io
+  external: true
+  group: serving
+  kind: InferenceGraph
+  path: github.com/kserve/kserve/pkg/apis/serving/v1alpha1
+  version: v1alpha1
+  webhooks:
+    defaulting: true
+    webhookVersion: v1
 version: "3"
diff --git a/cmd/main.go b/cmd/main.go
@@ -48,6 +48,7 @@ import (
 	"github.com/opendatahub-io/odh-model-controller/internal/controller/utils"
 	webhooknimv1 "github.com/opendatahub-io/odh-model-controller/internal/webhook/nim/v1"
 	webhookservingv1 "github.com/opendatahub-io/odh-model-controller/internal/webhook/serving/v1"
+	webhookservingv1alpha1 "github.com/opendatahub-io/odh-model-controller/internal/webhook/serving/v1alpha1"
 	webhookservingv1beta1 "github.com/opendatahub-io/odh-model-controller/internal/webhook/serving/v1beta1"
 	// +kubebuilder:scaffold:imports
 )
@@ -271,6 +272,13 @@ func main() {
 			os.Exit(1)
 		}
 	}
+	// nolint:goconst
+	if os.Getenv("ENABLE_WEBHOOKS") != "false" {
+		if err = webhookservingv1alpha1.SetupInferenceGraphWebhookWithManager(mgr); err != nil {
+			setupLog.Error(err, "unable to create webhook", "webhook", "InferenceGraph")
+			os.Exit(1)
+		}
+	}
 	// +kubebuilder:scaffold:builder
 
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {

diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml
@@ -11,27 +11,44 @@ patches:
       annotations:
         service.beta.openshift.io/inject-cabundle: true
     webhooks:
-      - name: validating.ksvc.odh-model-controller.opendatahub.io
-        clientConfig:
-          service:
-            name: odh-model-controller-webhook-service
-        objectSelector:
-          matchExpressions:
-            - key: serving.kserve.io/inferenceservice
-              operator: Exists
-      - name: validating.nim.account.odh-model-controller.opendatahub.io
-        clientConfig:
-          service:
-            name: odh-model-controller-webhook-service
-      - name: validating.isvc.odh-model-controller.opendatahub.io
-        clientConfig:
-          service:
-            name: odh-model-controller-webhook-service
+    - name: validating.ksvc.odh-model-controller.opendatahub.io
+      clientConfig:
+        service:
+          name: odh-model-controller-webhook-service
+      objectSelector:
+        matchExpressions:
+        - key: serving.kserve.io/inferenceservice
+          operator: Exists
+    - name: validating.nim.account.odh-model-controller.opendatahub.io
+      clientConfig:
+        service:
+          name: odh-model-controller-webhook-service
+    - name: validating.isvc.odh-model-controller.opendatahub.io
+      clientConfig:
+        service:
+          name: odh-model-controller-webhook-service
   target:
     group: admissionregistration.k8s.io
     kind: ValidatingWebhookConfiguration
     name: validating-webhook-configuration
     version: v1
+- patch: |-
+    apiVersion: admissionregistration.k8s.io/v1
+    kind: MutatingWebhookConfiguration
+    metadata:
+      name: mutating-webhook-configuration
+      annotations:
+        service.beta.openshift.io/inject-cabundle: true
+    webhooks:
+    - name: minferencegraph-v1alpha1.odh-model-controller.opendatahub.io
+      clientConfig:
+        service:
+          name: odh-model-controller-webhook-service
+    - name: minferenceservice-v1beta1.odh-model-controller.opendatahub.io
+      clientConfig:
+        service:
+          name: odh-model-controller-webhook-service
+
 - patch: |-
     - op: replace
       path: /metadata/name
@@ -41,6 +58,15 @@ patches:
     kind: ValidatingWebhookConfiguration
     name: validating-webhook-configuration
     version: v1
+- patch: |-
+    - op: replace
+      path: /metadata/name
+      value: mutating.odh-model-controller.opendatahub.io
+  target:
+    group: admissionregistration.k8s.io
+    kind: MutatingWebhookConfiguration
+    name: mutating-webhook-configuration
+    version: v1
 
 configurations:
 - kustomizeconfig.yaml
diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml
@@ -1,5 +1,49 @@
 ---
 apiVersion: admissionregistration.k8s.io/v1
+kind: MutatingWebhookConfiguration
+metadata:
+  name: mutating-webhook-configuration
+webhooks:
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-serving-kserve-io-v1alpha1-inferencegraph
+  failurePolicy: Fail
+  name: minferencegraph-v1alpha1.odh-model-controller.opendatahub.io
+  rules:
+  - apiGroups:
+    - serving.kserve.io
+    apiVersions:
+    - v1alpha1
+    operations:
+    - CREATE
+    resources:
+    - inferencegraphs
+  sideEffects: None
+- admissionReviewVersions:
+  - v1
+  clientConfig:
+    service:
+      name: webhook-service
+      namespace: system
+      path: /mutate-serving-kserve-io-v1beta1-inferenceservice
+  failurePolicy: Fail
+  name: minferenceservice-v1beta1.odh-model-controller.opendatahub.io
+  rules:
+  - apiGroups:
+    - serving.kserve.io
+    apiVersions:
+    - v1beta1
+    operations:
+    - CREATE
+    resources:
+    - inferenceservices
+  sideEffects: None
+---
+apiVersion: admissionregistration.k8s.io/v1
 kind: ValidatingWebhookConfiguration
 metadata:
   name: validating-webhook-configuration

diff --git a/internal/controller/constants/constants.go b/internal/controller/constants/constants.go
@@ -15,7 +15,7 @@ limitations under the License.
 
 package constants
 
-type IsvcDeploymentMode string
+type KServeDeploymentMode string
 
 const (
 	InferenceServiceKind = "InferenceService"
@@ -43,9 +43,9 @@ const (
 
 // isvc modes
 var (
-	Serverless    IsvcDeploymentMode = "Serverless"
-	RawDeployment IsvcDeploymentMode = "RawDeployment"
-	ModelMesh     IsvcDeploymentMode = "ModelMesh"
+	Serverless    KServeDeploymentMode = "Serverless"
+	RawDeployment KServeDeploymentMode = "RawDeployment"
+	ModelMesh     KServeDeploymentMode = "ModelMesh"
 )
 
 // model registry

diff --git a/internal/controller/serving/inferenceservice_controller.go b/internal/controller/serving/inferenceservice_controller.go
@@ -138,7 +138,7 @@ func (r *InferenceServiceReconciler) ReconcileServing(ctx context.Context, req c
 	}
 
 	// Check what deployment mode is used by the InferenceService. We have differing reconciliation logic for Kserve and ModelMesh
-	IsvcDeploymentMode, err := utils.GetDeploymentModeForIsvc(ctx, r.Client, isvc)
+	IsvcDeploymentMode, err := utils.GetDeploymentModeForKServeResource(ctx, r.Client, isvc.GetAnnotations())
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -259,7 +259,7 @@ func (r *InferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager) error {
 func (r *InferenceServiceReconciler) onDeletion(ctx context.Context, log logr.Logger, inferenceService *kservev1beta1.InferenceService) error {
 	log.V(1).Info("Running cleanup logic")
 
-	IsvcDeploymentMode, err := utils.GetDeploymentModeForIsvc(ctx, r.Client, inferenceService)
+	IsvcDeploymentMode, err := utils.GetDeploymentModeForKServeResource(ctx, r.Client, inferenceService.GetAnnotations())
 	if err != nil {
 		log.V(1).Error(err, "Could not determine deployment mode for ISVC. Some resources related to the inferenceservice might not be deleted.")
 	}

diff --git a/internal/controller/serving/reconcilers/kserve_raw_inferenceservice_reconciler.go b/internal/controller/serving/reconcilers/kserve_raw_inferenceservice_reconciler.go
@@ -73,7 +73,7 @@ func (r *KserveRawInferenceServiceReconciler) CleanupNamespaceIfNoKserveIsvcExis
 
 	for i := len(inferenceServiceList.Items) - 1; i >= 0; i-- {
 		inferenceService := inferenceServiceList.Items[i]
-		isvcDeploymentMode, err := utils.GetDeploymentModeForIsvc(ctx, r.client, &inferenceService)
+		isvcDeploymentMode, err := utils.GetDeploymentModeForKServeResource(ctx, r.client, inferenceService.GetAnnotations())
 		if err != nil {
 			return err
 		}

diff --git a/internal/controller/serving/reconcilers/kserve_serverless_inferenceservice_reconciler.go b/internal/controller/serving/reconcilers/kserve_serverless_inferenceservice_reconciler.go
@@ -86,7 +86,7 @@ func (r *KserveServerlessInferenceServiceReconciler) CleanupNamespaceIfNoKserveI
 
 	for i := len(inferenceServiceList.Items) - 1; i >= 0; i-- {
 		inferenceService := inferenceServiceList.Items[i]
-		isvcDeploymentMode, err := utils.GetDeploymentModeForIsvc(ctx, r.client, &inferenceService)
+		isvcDeploymentMode, err := utils.GetDeploymentModeForKServeResource(ctx, r.client, inferenceService.GetAnnotations())
 		if err != nil {
 			return err
 		}

diff --git a/internal/controller/serving/reconcilers/mm_inferenceservice_reconciler.go b/internal/controller/serving/reconcilers/mm_inferenceservice_reconciler.go
@@ -62,7 +62,7 @@ func (r *ModelMeshInferenceServiceReconciler) DeleteModelMeshResourcesIfNoMMIsvc
 
 	for i := len(inferenceServiceList.Items) - 1; i >= 0; i-- {
 		inferenceService := inferenceServiceList.Items[i]
-		isvcDeploymentMode, err := utils.GetDeploymentModeForIsvc(ctx, r.client, &inferenceService)
+		isvcDeploymentMode, err := utils.GetDeploymentModeForKServeResource(ctx, r.client, inferenceService.GetAnnotations())
 		if err != nil {
 			return err
 		}

diff --git a/internal/controller/utils/utils.go b/internal/controller/utils/utils.go
@@ -38,15 +38,15 @@ var (
 )
 
 const (
-	inferenceServiceDeploymentModeAnnotation = "serving.kserve.io/deploymentMode"
-	KserveConfigMapName                      = "inferenceservice-config"
-	KServeWithServiceMeshComponent           = "kserve-service-mesh"
+	KServeDeploymentModeAnnotation = "serving.kserve.io/deploymentMode"
+	KserveConfigMapName            = "inferenceservice-config"
+	KServeWithServiceMeshComponent = "kserve-service-mesh"
 )
 
-func GetDeploymentModeForIsvc(ctx context.Context, cli client.Client, isvc *kservev1beta1.InferenceService) (constants.IsvcDeploymentMode, error) {
+func GetDeploymentModeForKServeResource(ctx context.Context, cli client.Client, annotations map[string]string) (constants.KServeDeploymentMode, error) {
 
-	// If ISVC specifically sets deployment mode using an annotation, return bool depending on value
-	value, exists := isvc.Annotations[inferenceServiceDeploymentModeAnnotation]
+	// If explicitly sets deployment mode using an annotation, return bool depending on value
+	value, exists := annotations[KServeDeploymentModeAnnotation]
 	if exists {
 		switch value {
 		case string(constants.ModelMesh):
@@ -56,10 +56,10 @@ func GetDeploymentModeForIsvc(ctx context.Context, cli client.Client, isvc *kser
 		case string(constants.RawDeployment):
 			return constants.RawDeployment, nil
 		default:
-			return "", fmt.Errorf("the deployment mode '%s' of the Inference Service is invalid", value)
+			return "", fmt.Errorf("the deployment mode '%s' of the KServe resource is invalid", value)
 		}
 	} else {
-		// ISVC does not specifically set deployment mode using an annotation, determine the default from configmap
+		// There is no explicit deployment mode using an annotation, determine the default from configmap
 		controllerNs := os.Getenv("POD_NAMESPACE")
 		inferenceServiceConfigMap := &corev1.ConfigMap{}
 		err := cli.Get(ctx, client.ObjectKey{

diff --git a/internal/webhook/serving/defaulters.go b/internal/webhook/serving/defaulters.go
@@ -0,0 +1,51 @@
+package serving
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/go-logr/logr"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/opendatahub-io/odh-model-controller/internal/controller/constants"
+	"github.com/opendatahub-io/odh-model-controller/internal/controller/utils"
+)
+
+func ApplyDefaultServerlessAnnotations(ctx context.Context, client client.Client, resourceName string, resourceMetadata *v1.ObjectMeta, logger logr.Logger) error {
+	deploymentMode, err := utils.GetDeploymentModeForKServeResource(ctx, client, resourceMetadata.GetAnnotations())
+	if err != nil {
+		return fmt.Errorf("error resolving deployment mode for resource %s: %w", resourceName, err)
+	}
+
+	if deploymentMode == constants.Serverless {
+		logAnnotationsAdded := make([]string, 0, 3)
+		resourceAnnotations := resourceMetadata.GetAnnotations()
+		if resourceAnnotations == nil {
+			resourceAnnotations = make(map[string]string)
+		}
+
+		if _, exists := resourceAnnotations["serving.knative.openshift.io/enablePassthrough"]; !exists {
+			resourceAnnotations["serving.knative.openshift.io/enablePassthrough"] = "true"
+			logAnnotationsAdded = append(logAnnotationsAdded, "serving.knative.openshift.io/enablePassthrough")
+		}
+
+		if _, exists := resourceAnnotations["sidecar.istio.io/inject"]; !exists {
+			resourceAnnotations["sidecar.istio.io/inject"] = "true"
+			logAnnotationsAdded = append(logAnnotationsAdded, "sidecar.istio.io/inject")
+		}
+
+		if _, exists := resourceAnnotations["sidecar.istio.io/rewriteAppHTTPProbers"]; !exists {
+			resourceAnnotations["sidecar.istio.io/rewriteAppHTTPProbers"] = "true"
+			logAnnotationsAdded = append(logAnnotationsAdded, "sidecar.istio.io/rewriteAppHTTPProbers")
+		}
+
+		if len(logAnnotationsAdded) > 0 {
+			logger.V(1).Info("Annotations added", "annotations", strings.Join(logAnnotationsAdded, ","))
+		}
+
+		resourceMetadata.SetAnnotations(resourceAnnotations)
+	}
+	return nil
+}