From 8b82a9d7f252e462f52a79153471ba5010724999 Mon Sep 17 00:00:00 2001 From: Spolti Date: Wed, 6 Mar 2024 15:42:18 -0300 Subject: [PATCH 1/3] [RHOAIENG-4191] - odh-model-controller should tolerate missing Authorino This commit allows to run ODH without Authorino when KServe is Removed. Signed-off-by: Spolti --- config/rbac/role.yaml | 8 +++++++ controllers/inferenceservice_controller.go | 19 ++++++++++++--- controllers/utils/utils.go | 28 ++++++++++++++++++++++ main.go | 1 + 4 files changed, 53 insertions(+), 3 deletions(-) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 5d514c83..a62ef6b9 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -45,6 +45,14 @@ rules: - patch - update - watch +- apiGroups: + - datasciencecluster.opendatahub.io + resources: + - datascienceclusters + verbs: + - get + - list + - watch - apiGroups: - extensions resources: diff --git a/controllers/inferenceservice_controller.go b/controllers/inferenceservice_controller.go index 31cd8fc6..6f1e54e8 100644 --- a/controllers/inferenceservice_controller.go +++ b/controllers/inferenceservice_controller.go @@ -17,7 +17,6 @@ package controllers import ( "context" - "github.com/go-logr/logr" kservev1alpha1 "github.com/kserve/kserve/pkg/apis/serving/v1alpha1" kservev1beta1 "github.com/kserve/kserve/pkg/apis/serving/v1beta1" @@ -122,7 +121,6 @@ func (r *OpenshiftInferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager) Owns(&networkingv1.NetworkPolicy{}). Owns(&monitoringv1.ServiceMonitor{}). Owns(&monitoringv1.PodMonitor{}). - Owns(&authorinov1beta2.AuthConfig{}). Watches(&source.Kind{Type: &kservev1alpha1.ServingRuntime{}}, handler.EnqueueRequestsFromMapFunc(func(o client.Object) []reconcile.Request { r.log.Info("Reconcile event triggered by serving runtime: " + o.GetName()) @@ -152,7 +150,22 @@ func (r *OpenshiftInferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager) } return reconcileRequests })) - err := builder.Complete(r) + + // check if kserve is enabled, otherwise don't require Authorino. + enabled, err := utils.VerifyIfComponentIsEnabled(context.TODO(), r.client, "kserve") + if err != nil { + r.log.V(1).Error(err, "could not determine if kserve is enabled, default is enabled") + enabled = true + } + + if enabled { + builder.Owns(&authorinov1beta2.AuthConfig{}) + r.log.Info("kserve component is enabled, Authorino is required") + } else { + r.log.Info("kserve component is disabled, ignoring Authorino requirement") + } + + err = builder.Complete(r) if err != nil { return err } diff --git a/controllers/utils/utils.go b/controllers/utils/utils.go index d13fa1ed..8ee7e810 100644 --- a/controllers/utils/utils.go +++ b/controllers/utils/utils.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "os" "reflect" @@ -23,6 +24,8 @@ var ( const ( inferenceServiceDeploymentModeAnnotation = "serving.kserve.io/deploymentMode" KserveConfigMapName = "inferenceservice-config" + dataScienceClusterKind = "DataScienceCluster" + dataScienceClusterApiVersion = "datasciencecluster.opendatahub.io/v1" ) func GetDeploymentModeForIsvc(ctx context.Context, cli client.Client, isvc *kservev1beta1.InferenceService) (IsvcDeploymentMode, error) { @@ -69,6 +72,31 @@ func GetDeploymentModeForIsvc(ctx context.Context, cli client.Client, isvc *kser } } +// VerifyIfComponentIsEnabled will query the DCS in the cluster and see if the desired componentName is enabled +func VerifyIfComponentIsEnabled(ctx context.Context, cli client.Client, componentName string) (bool, error) { + // Query the custom object + objectList := &unstructured.UnstructuredList{} + objectList.SetAPIVersion(dataScienceClusterApiVersion) + objectList.SetKind(dataScienceClusterKind) + + if err := cli.List(ctx, objectList); err != nil { + return false, fmt.Errorf("not able to read %s: %w", objectList, err) + } + + // there must be only one dsc + if len(objectList.Items) == 1 { + fields := []string{"spec", "components", componentName, "managementState"} + val, _, err := unstructured.NestedString(objectList.Items[0].Object, fields...) + if err != nil { + return false, fmt.Errorf("failed to retrieve the component [%s] status from %+v", + componentName, objectList.Items[0]) + } + return val == "Managed", nil + } else { + return false, fmt.Errorf("there is no %s available in the cluster", dataScienceClusterKind) + } +} + func IsNil(i any) bool { return reflect.ValueOf(i).IsNil() } diff --git a/main.go b/main.go index a82c9073..929edc3b 100644 --- a/main.go +++ b/main.go @@ -68,6 +68,7 @@ func init() { //nolint:gochecknoinits //reason this way we ensure schemes are al // +kubebuilder:rbac:groups="",resources=namespaces;pods;services;endpoints,verbs=get;list;watch;create;update;patch // +kubebuilder:rbac:groups="",resources=secrets;configmaps;serviceaccounts,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=authorino.kuadrant.io,resources=authconfigs,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=datasciencecluster.opendatahub.io,resources=datascienceclusters,verbs=get;list;watch func getEnvAsBool(name string, defaultValue bool) bool { valStr := os.Getenv(name) From 9748ecb6213c50aef5758926531d6470c905e1b3 Mon Sep 17 00:00:00 2001 From: Spolti Date: Thu, 7 Mar 2024 12:26:18 -0300 Subject: [PATCH 2/3] applying review suggestion check if the serving is enabled as service mesh is tied with Authorino. this will allow rawDeployments to also work without Authorin. Signed-off-by: Spolti --- controllers/inferenceservice_controller.go | 10 ++++++---- controllers/utils/utils.go | 8 ++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/controllers/inferenceservice_controller.go b/controllers/inferenceservice_controller.go index 6f1e54e8..f2d46027 100644 --- a/controllers/inferenceservice_controller.go +++ b/controllers/inferenceservice_controller.go @@ -152,17 +152,19 @@ func (r *OpenshiftInferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager) })) // check if kserve is enabled, otherwise don't require Authorino. - enabled, err := utils.VerifyIfComponentIsEnabled(context.TODO(), r.client, "kserve") + enabled, err := utils.VerifyIfComponentIsEnabled(context.TODO(), r.client, utils.KserveAuthorinoComponent) + logMessage := "kserve service mesh component is enabled, Authorino is required" if err != nil { - r.log.V(1).Error(err, "could not determine if kserve is enabled, default is enabled") + r.log.V(1).Error(err, "could not determine if kserve have service mesh enabled") enabled = true + logMessage = "could not determine if kserve have service mesh enabled, Authorino integration will be enabled" } if enabled { builder.Owns(&authorinov1beta2.AuthConfig{}) - r.log.Info("kserve component is enabled, Authorino is required") + r.log.Info(logMessage) } else { - r.log.Info("kserve component is disabled, ignoring Authorino requirement") + r.log.Info("kserve serving component is disabled, ignoring Authorino requirement") } err = builder.Complete(r) diff --git a/controllers/utils/utils.go b/controllers/utils/utils.go index 8ee7e810..6cb345a2 100644 --- a/controllers/utils/utils.go +++ b/controllers/utils/utils.go @@ -26,6 +26,7 @@ const ( KserveConfigMapName = "inferenceservice-config" dataScienceClusterKind = "DataScienceCluster" dataScienceClusterApiVersion = "datasciencecluster.opendatahub.io/v1" + KserveAuthorinoComponent = "kserve-authorino" ) func GetDeploymentModeForIsvc(ctx context.Context, cli client.Client, isvc *kservev1beta1.InferenceService) (IsvcDeploymentMode, error) { @@ -86,6 +87,13 @@ func VerifyIfComponentIsEnabled(ctx context.Context, cli client.Client, componen // there must be only one dsc if len(objectList.Items) == 1 { fields := []string{"spec", "components", componentName, "managementState"} + if componentName == KserveAuthorinoComponent { + // For KServe, Authorino is required when ServiceMesh is enabled + // By Disabling ServiceMesh for RawDeployment, it should reflect on disabling + // the Authorino integration as well. + fields = []string{"spec", "components", "kserve", "serving", "managementState"} + } + val, _, err := unstructured.NestedString(objectList.Items[0].Object, fields...) if err != nil { return false, fmt.Errorf("failed to retrieve the component [%s] status from %+v", From a45a02f6ab6dc0641c665ece698307c04d59e1a2 Mon Sep 17 00:00:00 2001 From: Spolti Date: Fri, 8 Mar 2024 18:04:26 -0300 Subject: [PATCH 3/3] serving also needs to consider unmanaged state Signed-off-by: Spolti --- controllers/inferenceservice_controller.go | 12 +++++------- controllers/utils/utils.go | 9 +++++++++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/controllers/inferenceservice_controller.go b/controllers/inferenceservice_controller.go index f2d46027..52c37715 100644 --- a/controllers/inferenceservice_controller.go +++ b/controllers/inferenceservice_controller.go @@ -152,19 +152,17 @@ func (r *OpenshiftInferenceServiceReconciler) SetupWithManager(mgr ctrl.Manager) })) // check if kserve is enabled, otherwise don't require Authorino. - enabled, err := utils.VerifyIfComponentIsEnabled(context.TODO(), r.client, utils.KserveAuthorinoComponent) - logMessage := "kserve service mesh component is enabled, Authorino is required" + + isAuthorinoRequired, err := utils.VerifyIfComponentIsEnabled(context.TODO(), r.client, utils.KserveAuthorinoComponent) if err != nil { r.log.V(1).Error(err, "could not determine if kserve have service mesh enabled") - enabled = true - logMessage = "could not determine if kserve have service mesh enabled, Authorino integration will be enabled" } - if enabled { + if isAuthorinoRequired { builder.Owns(&authorinov1beta2.AuthConfig{}) - r.log.Info(logMessage) + r.log.Info("kserve is enabled with Service Mesh, Authorino is a requirement") } else { - r.log.Info("kserve serving component is disabled, ignoring Authorino requirement") + r.log.Info("didn't find kserve with service mesh, Authorino is not a requirement") } err = builder.Complete(r) diff --git a/controllers/utils/utils.go b/controllers/utils/utils.go index 6cb345a2..7ce45b08 100644 --- a/controllers/utils/utils.go +++ b/controllers/utils/utils.go @@ -92,6 +92,15 @@ func VerifyIfComponentIsEnabled(ctx context.Context, cli client.Client, componen // By Disabling ServiceMesh for RawDeployment, it should reflect on disabling // the Authorino integration as well. fields = []string{"spec", "components", "kserve", "serving", "managementState"} + kserveFields := []string{"spec", "components", "kserve", "managementState"} + serving, _, err := unstructured.NestedString(objectList.Items[0].Object, fields...) + kserve, _, err := unstructured.NestedString(objectList.Items[0].Object, kserveFields...) + if err != nil { + return false, fmt.Errorf("failed to retrieve the component [%s] status from %+v", + componentName, objectList.Items[0]) + } + + return (serving == "Managed" || serving == "Unmanaged") && kserve == "Managed", nil } val, _, err := unstructured.NestedString(objectList.Items[0].Object, fields...)