From cebd287ac78b979f6c1fa4bd5f2af378d61befe6 Mon Sep 17 00:00:00 2001 From: Vedant Mahabaleshwarkar Date: Tue, 27 Feb 2024 12:42:22 -0500 Subject: [PATCH] allow setting default deployment mode for Kserve in DSC (#864) * allow setting default deployment mode for Kserve in DSC Signed-off-by: Vedant Mahabaleshwarkar * move kserve config logic to separate file + enhancements Signed-off-by: Vedant Mahabaleshwarkar * revert dev image set in operator CSV Signed-off-by: Vedant Mahabaleshwarkar * only setup kserve config if component is enabled Signed-off-by: Vedant Mahabaleshwarkar * bug fix Signed-off-by: Vedant Mahabaleshwarkar * address PR feedback Signed-off-by: Vedant Mahabaleshwarkar * cleanup Signed-off-by: Vedant Mahabaleshwarkar * fix lint error Signed-off-by: Vedant Mahabaleshwarkar * set default value for Kserve defaultDeploymentMode to be empty Signed-off-by: Vedant Mahabaleshwarkar * more pr feedback Signed-off-by: Vedant Mahabaleshwarkar * update bundle Signed-off-by: Vedant Mahabaleshwarkar * enhance documentation Signed-off-by: Vedant Mahabaleshwarkar * add readme for dev preview Signed-off-by: Vedant Mahabaleshwarkar --------- Signed-off-by: Vedant Mahabaleshwarkar --- ...er.opendatahub.io_datascienceclusters.yaml | 11 ++ components/kserve/kserve.go | 91 +++------ components/kserve/kserve_config_handler.go | 175 ++++++++++++++++++ ...er.opendatahub.io_datascienceclusters.yaml | 11 ++ .../datasciencecluster/kubebuilder_rbac.go | 2 +- docs/Dev-Preview.md | 25 +++ 6 files changed, 249 insertions(+), 66 deletions(-) create mode 100644 components/kserve/kserve_config_handler.go diff --git a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 700e41d60b1..04ea8247be7 100644 --- a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -178,6 +178,17 @@ spec: before enable component Does not support enabled ModelMeshServing at the same time properties: + defaultDeploymentMode: + description: Configures the default deployment mode for Kserve. + This can be set to 'Serverless' or 'RawDeployment'. The + value specified in this field will be used to set the default + deployment mode in the 'inferenceservice-config' configmap + for Kserve + enum: + - Serverless + - RawDeployment + pattern: ^(Serverless|RawDeployment)$ + type: string devFlags: description: Add developer fields properties: diff --git a/components/kserve/kserve.go b/components/kserve/kserve.go index b5e1de72ffb..87fc406d28c 100644 --- a/components/kserve/kserve.go +++ b/components/kserve/kserve.go @@ -7,7 +7,6 @@ import ( "path/filepath" "strings" - "github.com/hashicorp/go-multierror" operatorv1 "github.com/openshift/api/operator/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/rest" @@ -18,7 +17,6 @@ import ( infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/infrastructure/v1" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" - "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/monitoring" ) @@ -34,6 +32,16 @@ var ( // Verifies that Kserve implements ComponentInterface. var _ components.ComponentInterface = (*Kserve)(nil) +// +kubebuilder:validation:Pattern=`^(Serverless|RawDeployment)$` +type DefaultDeploymentMode string + +var ( + // Serverless will be used as the default deployment mode for Kserve. This requires Serverless and ServiceMesh operators configured as dependencies. + Serverless DefaultDeploymentMode = "Serverless" + // RawDeployment will be used as the default deployment mode for Kserve. + RawDeployment DefaultDeploymentMode = "RawDeployment" +) + // Kserve struct holds the configuration for the Kserve component. // +kubebuilder:object:generate=true type Kserve struct { @@ -41,6 +49,11 @@ type Kserve struct { // Serving configures the KNative-Serving stack used for model serving. A Service // Mesh (Istio) is prerequisite, since it is used as networking layer. Serving infrav1.ServingSpec `json:"serving,omitempty"` + // Configures the default deployment mode for Kserve. This can be set to 'Serverless' or 'RawDeployment'. + // The value specified in this field will be used to set the default deployment mode in the 'inferenceservice-config' configmap for Kserve + // If no default deployment mode is specified, Kserve will use Serverless mode + // +kubebuilder:validation:Enum=Serverless;RawDeployment + DefaultDeploymentMode DefaultDeploymentMode `json:"defaultDeploymentMode,omitempty"` } func (k *Kserve) OverrideManifests(_ string) error { @@ -102,6 +115,10 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client, resC return err } } else { + // Configure dependencies + if err := k.configureServerless(cli, dscispec); err != nil { + return err + } if k.DevFlags != nil { // Download manifests and update paths if err = k.OverrideManifests(string(platform)); err != nil { @@ -109,10 +126,6 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client, resC } } - if err := k.configureServerless(cli, dscispec); err != nil { - return err - } - // Update image parameters only when we do not have customized manifests set if (dscispec.DevFlags == nil || dscispec.DevFlags.ManifestsUri == "") && (k.DevFlags == nil || len(k.DevFlags.Manifests) == 0) { if err := deploy.ApplyParams(Path, k.SetImageParamsMap(imageParamMap), false); err != nil { @@ -125,6 +138,12 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client, resC return err } + if enabled { + if err := k.setupKserveConfig(ctx, cli, dscispec); err != nil { + return err + } + } + // For odh-model-controller if enabled { if err := cluster.UpdatePodSecurityRolebinding(cli, dscispec.ApplicationsNamespace, "odh-model-controller"); err != nil { @@ -144,6 +163,7 @@ func (k *Kserve) ReconcileComponent(ctx context.Context, cli client.Client, resC return err } } + // CloudService Monitoring handling if platform == deploy.ManagedRhods { if enabled { @@ -169,62 +189,3 @@ func (k *Kserve) Cleanup(_ client.Client, instance *dsciv1.DSCInitializationSpec return k.removeServiceMeshConfigurations(instance) } - -func (k *Kserve) configureServerless(cli client.Client, instance *dsciv1.DSCInitializationSpec) error { - switch k.Serving.ManagementState { - case operatorv1.Unmanaged: // Bring your own CR - fmt.Println("Serverless CR is not configured by the operator, we won't do anything") - - case operatorv1.Removed: // we remove serving CR - fmt.Println("existing Serverless CR (owned by operator) will be removed") - if err := k.removeServerlessFeatures(instance); err != nil { - return err - } - - case operatorv1.Managed: // standard workflow to create CR - switch instance.ServiceMesh.ManagementState { - case operatorv1.Unmanaged, operatorv1.Removed: - return fmt.Errorf("ServiceMesh is need to set to 'Managed' in DSCI CR, it is required by KServe serving field") - } - - // check on dependent operators if all installed in cluster - dependOpsErrors := checkDependentOperators(cli).ErrorOrNil() - if dependOpsErrors != nil { - return dependOpsErrors - } - - serverlessFeatures := feature.ComponentFeaturesHandler(k, instance, k.configureServerlessFeatures()) - - if err := serverlessFeatures.Apply(); err != nil { - return err - } - } - return nil -} - -func (k *Kserve) removeServerlessFeatures(instance *dsciv1.DSCInitializationSpec) error { - serverlessFeatures := feature.ComponentFeaturesHandler(k, instance, k.configureServerlessFeatures()) - - return serverlessFeatures.Delete() -} - -func checkDependentOperators(cli client.Client) *multierror.Error { - var multiErr *multierror.Error - - if found, err := deploy.OperatorExists(cli, ServiceMeshOperator); err != nil { - multiErr = multierror.Append(multiErr, err) - } else if !found { - err = fmt.Errorf("operator %s not found. Please install the operator before enabling %s component", - ServiceMeshOperator, ComponentName) - multiErr = multierror.Append(multiErr, err) - } - - if found, err := deploy.OperatorExists(cli, ServerlessOperator); err != nil { - multiErr = multierror.Append(multiErr, err) - } else if !found { - err = fmt.Errorf("operator %s not found. Please install the operator before enabling %s component", - ServerlessOperator, ComponentName) - multiErr = multierror.Append(multiErr, err) - } - return multiErr -} diff --git a/components/kserve/kserve_config_handler.go b/components/kserve/kserve_config_handler.go new file mode 100644 index 00000000000..39fdd62f8f9 --- /dev/null +++ b/components/kserve/kserve_config_handler.go @@ -0,0 +1,175 @@ +package kserve + +import ( + "context" + "encoding/json" + "fmt" + + "github.com/hashicorp/go-multierror" + operatorv1 "github.com/openshift/api/operator/v1" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature" +) + +const ( + KserveConfigMapName string = "inferenceservice-config" +) + +func (k *Kserve) setupKserveConfig(ctx context.Context, cli client.Client, dscispec *dsciv1.DSCInitializationSpec) error { + // as long as Kserve.Serving is not 'Removed', we will setup the dependencies + + switch k.Serving.ManagementState { + case operatorv1.Managed, operatorv1.Unmanaged: + if k.DefaultDeploymentMode == "" { + // if the default mode is empty in the DSC, assume mode is "Serverless" since k.Serving is Managed + if err := k.setDefaultDeploymentMode(ctx, cli, dscispec, Serverless); err != nil { + return err + } + } else { + // if the default mode is explicitly specified, respect that + if err := k.setDefaultDeploymentMode(ctx, cli, dscispec, k.DefaultDeploymentMode); err != nil { + return err + } + } + case operatorv1.Removed: + if k.DefaultDeploymentMode == Serverless { + return fmt.Errorf("setting defaultdeployment mode as Serverless is incompatible with having Serving 'Removed'") + } + if k.DefaultDeploymentMode == "" { + fmt.Println("Serving is removed, Kserve will default to rawdeployment") + } + if err := k.setDefaultDeploymentMode(ctx, cli, dscispec, RawDeployment); err != nil { + return err + } + } + return nil +} + +func (k *Kserve) setDefaultDeploymentMode(ctx context.Context, cli client.Client, dscispec *dsciv1.DSCInitializationSpec, defaultmode DefaultDeploymentMode) error { + inferenceServiceConfigMap := &corev1.ConfigMap{} + err := cli.Get(ctx, client.ObjectKey{ + Namespace: dscispec.ApplicationsNamespace, + Name: KserveConfigMapName, + }, inferenceServiceConfigMap) + if err != nil { + return fmt.Errorf("error getting configmap 'inferenceservice-config'. %w", err) + } + + // set data.deploy.defaultDeploymentMode to the model specified in the Kserve spec + var deployData map[string]interface{} + if err = json.Unmarshal([]byte(inferenceServiceConfigMap.Data["deploy"]), &deployData); err != nil { + return fmt.Errorf("error retrieving value for key 'deploy' from configmap %s. %w", KserveConfigMapName, err) + } + modeFound := deployData["defaultDeploymentMode"] + if modeFound != string(defaultmode) { + deployData["defaultDeploymentMode"] = defaultmode + deployDataBytes, err := json.MarshalIndent(deployData, "", " ") + if err != nil { + return fmt.Errorf("could not set values in configmap %s. %w", KserveConfigMapName, err) + } + inferenceServiceConfigMap.Data["deploy"] = string(deployDataBytes) + + var ingressData map[string]interface{} + if err = json.Unmarshal([]byte(inferenceServiceConfigMap.Data["ingress"]), &ingressData); err != nil { + return fmt.Errorf("error retrieving value for key 'ingress' from configmap %s. %w", KserveConfigMapName, err) + } + if defaultmode == RawDeployment { + ingressData["disableIngressCreation"] = true + } else { + ingressData["disableIngressCreation"] = false + } + ingressDataBytes, err := json.MarshalIndent(ingressData, "", " ") + if err != nil { + return fmt.Errorf("could not set values in configmap %s. %w", KserveConfigMapName, err) + } + inferenceServiceConfigMap.Data["ingress"] = string(ingressDataBytes) + + if err = cli.Update(ctx, inferenceServiceConfigMap); err != nil { + return fmt.Errorf("could not set default deployment mode for Kserve. %w", err) + } + + // Restart the pod if configmap is updated so that kserve boots with the correct value + podList := &corev1.PodList{} + listOpts := []client.ListOption{ + client.InNamespace(dscispec.ApplicationsNamespace), + client.MatchingLabels{ + "app.opendatahub.io/kserve": "true", + "control-plane": "kserve-controller-manager", + }, + } + if err := cli.List(ctx, podList, listOpts...); err != nil { + return fmt.Errorf("failed to list pods: %w", err) + } + for _, pod := range podList.Items { + pod := pod + if err := cli.Delete(ctx, &pod); err != nil { + return fmt.Errorf("failed to delete pod %s: %w", pod.Name, err) + } + } + } + + return nil +} + +func (k *Kserve) configureServerless(cli client.Client, instance *dsciv1.DSCInitializationSpec) error { + switch k.Serving.ManagementState { + case operatorv1.Unmanaged: // Bring your own CR + fmt.Println("Serverless CR is not configured by the operator, we won't do anything") + + case operatorv1.Removed: // we remove serving CR + fmt.Println("existing Serverless CR (owned by operator) will be removed") + if err := k.removeServerlessFeatures(instance); err != nil { + return err + } + + case operatorv1.Managed: // standard workflow to create CR + switch instance.ServiceMesh.ManagementState { + case operatorv1.Unmanaged, operatorv1.Removed: + return fmt.Errorf("ServiceMesh is need to set to 'Managed' in DSCI CR, it is required by KServe serving field") + } + + // check on dependent operators if all installed in cluster + dependOpsErrors := checkDependentOperators(cli).ErrorOrNil() + if dependOpsErrors != nil { + return dependOpsErrors + } + + serverlessFeatures := feature.ComponentFeaturesHandler(k, instance, k.configureServerlessFeatures()) + + if err := serverlessFeatures.Apply(); err != nil { + return err + } + } + return nil +} + +func (k *Kserve) removeServerlessFeatures(instance *dsciv1.DSCInitializationSpec) error { + serverlessFeatures := feature.ComponentFeaturesHandler(k, instance, k.configureServerlessFeatures()) + + return serverlessFeatures.Delete() +} + +func checkDependentOperators(cli client.Client) *multierror.Error { + var multiErr *multierror.Error + + if found, err := deploy.OperatorExists(cli, ServiceMeshOperator); err != nil { + multiErr = multierror.Append(multiErr, err) + } else if !found { + err = fmt.Errorf("operator %s not found. Please install the operator before enabling %s component", + ServiceMeshOperator, ComponentName) + multiErr = multierror.Append(multiErr, err) + } + + if found, err := deploy.OperatorExists(cli, ServerlessOperator); err != nil { + multiErr = multierror.Append(multiErr, err) + } else if !found { + err = fmt.Errorf("operator %s not found. Please install the operator before enabling %s component", + ServerlessOperator, ComponentName) + multiErr = multierror.Append(multiErr, err) + } + return multiErr +} diff --git a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 01e02e4cbbf..32a784a4796 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -179,6 +179,17 @@ spec: before enable component Does not support enabled ModelMeshServing at the same time properties: + defaultDeploymentMode: + description: Configures the default deployment mode for Kserve. + This can be set to 'Serverless' or 'RawDeployment'. The + value specified in this field will be used to set the default + deployment mode in the 'inferenceservice-config' configmap + for Kserve + enum: + - Serverless + - RawDeployment + pattern: ^(Serverless|RawDeployment)$ + type: string devFlags: description: Add developer fields properties: diff --git a/controllers/datasciencecluster/kubebuilder_rbac.go b/controllers/datasciencecluster/kubebuilder_rbac.go index 5494834da87..c4dfad27adb 100644 --- a/controllers/datasciencecluster/kubebuilder_rbac.go +++ b/controllers/datasciencecluster/kubebuilder_rbac.go @@ -196,7 +196,7 @@ package datasciencecluster // +kubebuilder:rbac:groups="core",resources=endpoints,verbs=watch;list;get;create;update;delete // +kubebuilder:rbac:groups="core",resources=configmaps/status,verbs=get;update;patch;delete -// +kubebuilder:rbac:groups="core",resources=configmaps,verbs=get;create;update;watch;patch;delete;list +// +kubebuilder:rbac:groups="core",resources=configmaps,verbs=get;create;watch;patch;delete;list;update // +kubebuilder:rbac:groups="core",resources=clusterversions,verbs=watch;list // +kubebuilder:rbac:groups="config.openshift.io",resources=clusterversions,verbs=watch;list diff --git a/docs/Dev-Preview.md b/docs/Dev-Preview.md index c0586dfc3cd..b817b5546f8 100644 --- a/docs/Dev-Preview.md +++ b/docs/Dev-Preview.md @@ -108,3 +108,28 @@ EOF - Currently on integration of ODH [core components](https://opendatahub.io/docs/tiered-components/) are available with the Operator. - Tier 1 and Tier 2 components can be deployed manually using [kustomize build](https://kubectl.docs.kubernetes.io/references/kustomize/cmd/build/) + + +## Preview Features + +### Kserve + +#### Changing the Default Deployment Mode for Kserve + +Kserve can now support `Serverless` and `RawDeployment` as the default deployment modes. The default deployment mode for Kserve can be set in the DSC as follows : +``` +kserve: + defaultDeploymentMode: RawDeployment + managementState: Managed + serving: + ingressGateway: + certificate: + type: SelfSigned + managementState: Removed + name: knative-serving +``` + +Notes : +- If a value for defaultDeploymentMode is not provided, it is assumed to be `Serverless` as long as kserve.serving.managementState is not `Removed` +- If kserve.serving.managementState is `Removed`, the default deployment mode is assumed to be `RawDeployment` if no value is provided. +- Explicitly setting defaultDeploymentMode to `Serverless` with kserve.serving.managementState set to `Removed` will result in an expected error due to incompatible options.