fix: add clusterole with aggregationrules in to predicate to skip rec…

…oncile - change in DSC to enabled/disable kserve and modelmesh is enough to trigger reconcile on modelcontroller by DSC CR - it directly reflect into modelcontroller .spec.kserve / .spec.modelmeshserving fileds - keep DSC CR ad the controller for modelcontroller CR, but add modelmesh and kserve as ownerreference for now - no need extra watches from ModelController directly to Kserve or ModelMeshServing Signed-off-by: Wen Zhou <wenzhou@redhat.com>
opendatahub-io · Nov 25, 2024 · 9e72341 · 9e72341
1 parent cc76ed8
commit 9e72341
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 8 deletions.
diff --git a/controllers/components/modelcontroller/modelcontroller_actions.go b/controllers/components/modelcontroller/modelcontroller_actions.go
@@ -22,8 +22,13 @@ import (
 	"strings"
 
 	operatorv1 "github.com/openshift/api/operator/v1"
+	k8serr "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
+	odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors"
 	odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
 	odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
 )
@@ -75,11 +80,11 @@ func devFlags(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
 
 	// for _, subcomponent := range rr.DSC.Spec.Components.Kserve.DevFlags.Manifests {
 	// 	if strings.Contains(subcomponent.URI, ComponentName) {
-	// 		// Download odh-model-controller
+	// 		Download odh-model-controller
 	// 		if err := odhdeploy.DownloadManifests(ctx, ComponentName, subcomponent); err != nil {
 	// 			return err
 	// 		}
-	// 		// If overlay is defined, update paths
+	// 		If overlay is defined, update paths
 	// 		if subcomponent.SourcePath != "" {
 	// 			rr.Manifests[0].SourcePath = subcomponent.SourcePath
 	// 		}
@@ -89,3 +94,64 @@ func devFlags(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
 	// TODO: Implement devflags logmode logic
 	return nil
 }
+
+func patchOwnerReference(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
+	l := log.FromContext(ctx)
+	mc, ok := rr.Instance.(*componentsv1.ModelController)
+	if !ok {
+		return fmt.Errorf("resource instance %v is not a componentsv1.ModelController)", rr.Instance)
+	}
+
+	owners := []metav1.OwnerReference{}
+	if mc.Spec.ModelMeshServing == operatorv1.Managed {
+		mm := &componentsv1.ModelMeshServing{}
+		if err := rr.Client.Get(ctx, client.ObjectKey{Name: "default-modelmesh"}, mm); err != nil {
+			if k8serr.IsNotFound(err) {
+				return odherrors.NewStopError("ModelMesh CR not exist: %v", err)
+			}
+			return odherrors.NewStopError("failed to get ModelMesh CR: %v", err)
+		}
+		owners = append(owners, metav1.OwnerReference{
+			Kind:               componentsv1.ModelMeshServingKind,
+			APIVersion:         componentsv1.GroupVersion.String(),
+			Name:               componentsv1.ModelMeshServingInstanceName,
+			UID:                mm.GetUID(),
+			BlockOwnerDeletion: func(b bool) *bool { return &b }(false),
+			Controller:         func(b bool) *bool { return &b }(false),
+		},
+		)
+		l.Info("Update Ownerreference on modelmesh change")
+	}
+	if mc.Spec.Kserve == operatorv1.Managed {
+		k := &componentsv1.ModelMeshServing{}
+		if err := rr.Client.Get(ctx, client.ObjectKey{Name: "default-kserve"}, k); err != nil {
+			if k8serr.IsNotFound(err) {
+				return odherrors.NewStopError("Kserve CR not exist: %v", err)
+			}
+			return odherrors.NewStopError("failed to get Kserve CR: %v", err)
+		}
+		owners = append(owners, metav1.OwnerReference{
+			Kind:               "Kserve", // TODO : use componentsv1.KserveKind
+			APIVersion:         componentsv1.GroupVersion.String(),
+			Name:               "default-kserve", // TODO: componentsv1.KserveInstanceName
+			UID:                k.GetUID(),
+			BlockOwnerDeletion: func(b bool) *bool { return &b }(false),
+			Controller:         func(b bool) *bool { return &b }(false),
+		},
+		)
+		l.Info("Update Ownerreference on kserve change")
+	}
+
+	mc.SetOwnerReferences(owners)
+	mc.SetManagedFields(nil) // remove managed fields to avoid conflicts when SSA apply
+	force := true
+	opt := &client.PatchOptions{
+		Force:        &force,
+		FieldManager: componentsv1.ModelControllerInstanceName,
+	}
+	if err := rr.Client.Patch(ctx, mc, client.Apply, opt); err != nil {
+		return fmt.Errorf("error update ownerreference for CR %s : %w", mc.GetName(), err)
+	}
+
+	return nil
+}
diff --git a/controllers/components/modelcontroller/modelcontroller_controller.go b/controllers/components/modelcontroller/modelcontroller_controller.go
@@ -31,7 +31,6 @@ import (
 
 	componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
@@ -61,17 +60,26 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		Owns(&promv1.ServiceMonitor{}).
 		Owns(&networkingv1.NetworkPolicy{}).
 		Owns(&rbacv1.Role{}).
-		Owns(&rbacv1.ClusterRole{}).
+		Owns(&rbacv1.ClusterRole{}, reconciler.WithPredicates(resources.ClusterRolePredicate())). // model-serving-admin has aggregationrules
 		Owns(&rbacv1.RoleBinding{}).
 		Owns(&rbacv1.ClusterRoleBinding{}).
 		Owns(&corev1.Service{}).
 		Owns(&admissionregistrationv1.ValidatingWebhookConfiguration{}).
 		Owns(&templatev1.Template{}).
 		Owns(&appsv1.Deployment{}, reconciler.WithPredicates(resources.NewDeploymentPredicate())).
-		WatchesGVK(gvk.ModelMeshServing).           // watch ModelMeshServing type with  default create, update, delete and generic
-		WatchesGVK(gvk.Kserve).                     // watch Kserve type with default create, update, delete and generic
+		// WatchesGVK(
+		// 	gvk.ModelMeshServing,
+		// 	reconciler.Dynamic(),
+		// 	reconciler.WithPredicates(resources.ServingComponent()),
+		// ). // watch ModelMeshServing when CR is create or delete
+		// WatchesGVK(
+		// 	gvk.Kserve,
+		// 	reconciler.Dynamic(),
+		// 	reconciler.WithPredicates(resources.ServingComponent()),
+		// ).                                          // watch Kserve CR when create or delete
 		Watches(&extv1.CustomResourceDefinition{}). // call ForLabel() + new predicates
 		// Add ModelController specific actions
+		WithAction(patchOwnerReference).
 		WithAction(initialize).
 		WithAction(devFlags).
 		WithAction(security.NewUpdatePodSecurityRoleBindingAction(serviceAccounts)).

diff --git a/controllers/components/modelmeshserving/modelmeshserving_controller.go b/controllers/components/modelmeshserving/modelmeshserving_controller.go
@@ -62,7 +62,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		Owns(&corev1.Service{}).
 		Owns(&rbacv1.Role{}).
 		Owns(&rbacv1.ClusterRole{}).
-		Owns(&rbacv1.RoleBinding{}).
+		Owns(&rbacv1.ClusterRole{}, reconciler.WithPredicates(resources.ClusterRolePredicate())). // model-serving-admin has aggregationrules
 		Owns(&rbacv1.ClusterRoleBinding{}).
 		Owns(&appsv1.Deployment{}, reconciler.WithPredicates(resources.NewDeploymentPredicate())).
 		Watches(&extv1.CustomResourceDefinition{}). // call ForLabel() + new predicates

diff --git a/controllers/datasciencecluster/datasciencecluster_controller.go b/controllers/datasciencecluster/datasciencecluster_controller.go
@@ -406,6 +406,7 @@ func (r *DataScienceClusterReconciler) SetupWithManager(ctx context.Context, mgr
 		Owns(&componentsv1.TrainingOperator{}).
 		Owns(&componentsv1.DataSciencePipelines{}).
 		Owns(&componentsv1.ModelMeshServing{}).
+		// TODO: Owns(&componentsv1.Kserve{}).
 		Owns(&componentsv1.ModelController{}).
 		Watches(
 			&dsciv1.DSCInitialization{},
@@ -541,4 +542,4 @@ var defaultIngressCertSecretPredicates = predicate.Funcs{
 	DeleteFunc: func(e event.DeleteEvent) bool {
 		return true
 	},
-}
+}
diff --git a/pkg/controller/predicates/resources/resources.go b/pkg/controller/predicates/resources/resources.go
@@ -12,6 +12,14 @@ type DeploymentPredicate struct {
 	predicate.Funcs
 }
 
+func ClusterRolePredicate() predicate.Funcs {
+	return predicate.Funcs{
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			return e.ObjectNew.GetName() != "model-serving-admin"
+		},
+	}
+}
+
 // Update implements default UpdateEvent filter for validating generation change.
 func (DeploymentPredicate) Update(e event.UpdateEvent) bool {
 	if e.ObjectOld == nil || e.ObjectNew == nil {
@@ -53,3 +61,20 @@ func Deleted() predicate.Funcs {
 		},
 	}
 }
+
+func ServingComponent() predicate.Funcs {
+	return predicate.Funcs{
+		CreateFunc: func(e event.CreateEvent) bool {
+			return true
+		},
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			return false
+		},
+		DeleteFunc: func(e event.DeleteEvent) bool {
+			return true
+		},
+		GenericFunc: func(e event.GenericEvent) bool {
+			return false
+		},
+	}
+}