Skip to content

Commit

Permalink
Implementation of ODH defaulters for InferenceGraph and InferenceService
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
israel-hdez committed Dec 16, 2024
1 parent 3a45bac commit 2412463
Show file tree
Hide file tree
Showing 17 changed files with 601 additions and 22 deletions.
1 change: 1 addition & 0 deletions Dockerfile
10 changes: 10 additions & 0 deletions PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ resources:
version: v1beta1
webhooks:
validation: true
defaulting: true
webhookVersion: v1
- controller: true
core: true
Expand Down Expand Up @@ -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"
8 changes: 8 additions & 0 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down Expand Up @@ -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 {
Expand Down
44 changes: 44 additions & 0 deletions config/webhook/manifests.yaml
Original file line number Diff line number Diff line change
@@ -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.kb.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.kb.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
Expand Down
8 changes: 4 additions & 4 deletions internal/controller/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ limitations under the License.

package constants

type IsvcDeploymentMode string
type KServeDeploymentMode string

const (
InferenceServiceKind = "InferenceService"
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/serving/inferenceservice_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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.")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
16 changes: 8 additions & 8 deletions internal/controller/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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{
Expand Down
51 changes: 51 additions & 0 deletions internal/webhook/serving/defaulters.go
Original file line number Diff line number Diff line change
@@ -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
}
73 changes: 73 additions & 0 deletions internal/webhook/serving/v1alpha1/inferencegraph_webhook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
Copyright 2024.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1

import (
"context"
"fmt"

servingv1alpha1 "github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"

"github.com/opendatahub-io/odh-model-controller/internal/webhook/serving"
)

// nolint:unused
// log is for logging in this package.
var inferencegraphlog = logf.Log.WithName("inferencegraph-resource")

// SetupInferenceGraphWebhookWithManager registers the webhook for InferenceGraph in the manager.
func SetupInferenceGraphWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).For(&servingv1alpha1.InferenceGraph{}).
WithDefaulter(&InferenceGraphCustomDefaulter{client: mgr.GetClient()}).
Complete()
}

// +kubebuilder:webhook:path=/mutate-serving-kserve-io-v1alpha1-inferencegraph,mutating=true,failurePolicy=fail,sideEffects=None,groups=serving.kserve.io,resources=inferencegraphs,verbs=create,versions=v1alpha1,name=minferencegraph-v1alpha1.kb.io,admissionReviewVersions=v1

// InferenceGraphCustomDefaulter struct is responsible for setting default values on the custom resource of the
// Kind InferenceGraph when those are created or updated.
//
// NOTE: The +kubebuilder:object:generate=false marker prevents controller-gen from generating DeepCopy methods,
// as it is used only for temporary operations and does not need to be deeply copied.
type InferenceGraphCustomDefaulter struct {
client client.Client
}

var _ webhook.CustomDefaulter = &InferenceGraphCustomDefaulter{}

// Default implements webhook.CustomDefaulter so a webhook will be registered for the Kind InferenceGraph.
func (d *InferenceGraphCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error {
inferencegraph, ok := obj.(*servingv1alpha1.InferenceGraph)

if !ok {
return fmt.Errorf("expected an InferenceGraph object but got %T", obj)
}
logger := inferencegraphlog.WithValues("name", inferencegraph.GetName())
logger.Info("Defaulting for InferenceGraph")

err := serving.ApplyDefaultServerlessAnnotations(ctx, d.client, inferencegraph.GetName(), &inferencegraph.ObjectMeta, logger)
if err != nil {
return err
}

return nil
}
Loading

0 comments on commit 2412463

Please sign in to comment.