update: logic for manage resource

- rename updateResoruce() to CreateOrUpdateResource()
- use annoataion RHOAIApplicationNamespace and ODHApplicationNamespace than hardcode
- add predicate.Funcs for deployments to reduce reconciles on all components
- use switch for better readiness in CreateOrUpdateResource for annoataion handling

Signed-off-by: Wen Zhou <[email protected]>

controllers/datasciencecluster/datasciencecluster_controller.go

@@ -56,6 +56,7 @@ import (
 	"github.com/opendatahub-io/opendatahub-operator/v2/components/datasciencepipelines"
 	"github.com/opendatahub-io/opendatahub-operator/v2/controllers/status"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
+	annotations "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade"
 )
@@ -367,7 +368,25 @@ var configMapPredicates = predicate.Funcs{
 			return false
 		}
 		// Do not reconcile on kserver's inferenceservice-config CM updates, for rawdeployment
-		if e.ObjectNew.GetName() == "inferenceservice-config" && (e.ObjectNew.GetNamespace() == "redhat-ods-applications" || e.ObjectNew.GetNamespace() == "opendatahub") {
+		namespace := e.ObjectNew.GetNamespace()
+		if e.ObjectNew.GetName() == "inferenceservice-config" && (namespace == cluster.RHOAIApplicationNamespace || namespace == cluster.ODHApplicationNamespace) {
+			return false
+		}
+		return true
+	},
+}
+
+// reduce unnecessary reconcile triggered by odh component's deployment change due to ManagedByODHOperator annotation.
+var componentDeploymentPredicates = predicate.Funcs{
+	UpdateFunc: func(e event.UpdateEvent) bool {
+		namespace := e.ObjectNew.GetNamespace()
+		if namespace == cluster.ODHApplicationNamespace || namespace == cluster.RHOAIApplicationNamespace {
+			oldManaged, oldExists := e.ObjectOld.GetAnnotations()[annotations.ManagedByODHOperator]
+			newManaged := e.ObjectNew.GetAnnotations()[annotations.ManagedByODHOperator]
+			// only reoncile if annotation not exist to set to true, or from value non-true to true
+			if newManaged == "true" && (!oldExists || oldManaged != "true") {
+				return true
+			}
 			return false
 		}
 		return true
@@ -377,7 +396,8 @@ var configMapPredicates = predicate.Funcs{
 // a workaround for 2.5 due to odh-model-controller serivceaccount keeps updates with label.
 var saPredicates = predicate.Funcs{
 	UpdateFunc: func(e event.UpdateEvent) bool {
-		if e.ObjectNew.GetName() == "odh-model-controller" && (e.ObjectNew.GetNamespace() == "redhat-ods-applications" || e.ObjectNew.GetNamespace() == "opendatahub") {
+		namespace := e.ObjectNew.GetNamespace()
+		if e.ObjectNew.GetName() == "odh-model-controller" && (namespace == cluster.RHOAIApplicationNamespace || namespace == cluster.ODHApplicationNamespace) {
 			return false
 		}
 		return true
@@ -444,7 +464,7 @@ func (r *DataScienceClusterReconciler) SetupWithManager(ctx context.Context, mgr
 		Owns(&rbacv1.RoleBinding{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRBPredicates))).
 		Owns(&rbacv1.ClusterRole{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRolePredicates))).
 		Owns(&rbacv1.ClusterRoleBinding{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshRBPredicates))).
-		Owns(&appsv1.Deployment{}).
+		Owns(&appsv1.Deployment{}, builder.WithPredicates(componentDeploymentPredicates)).
 		Owns(&corev1.PersistentVolumeClaim{}).
 		Owns(&corev1.Service{}, builder.WithPredicates(predicate.Or(predicate.GenerationChangedPredicate{}, modelMeshGeneralPredicates))).
 		Owns(&appsv1.StatefulSet{}).

controllers/dscinitialization/dscinitialization_controller.go

@@ -341,7 +341,6 @@ var CMContentChangedPredicate = predicate.Funcs{
 
 var DSCDeletionPredicate = predicate.Funcs{
 	DeleteFunc: func(e event.DeleteEvent) bool {
-
 		return true
 	},
 }

pkg/cluster/const.go

@@ -9,4 +9,11 @@ const (
 	OpenDataHub Platform = "Open Data Hub"
 	// Unknown indicates that operator is not deployed using OLM.
 	Unknown Platform = ""
+
+	// Opendatahub default applicatiaon namespace.
+	// Even we have the capability to config to use different namespace, but for now, it is hardcoded to only use this namespace.
+	ODHApplicationNamespace = "opendatahub"
+	// RHOAI default applicatiaon namespace.
+	// RHOAI does not officially support using a different application namespace, as not being verified.
+	RHOAIApplicationNamespace = "redhat-ods-applications"
 )

pkg/deploy/deploy.go

@@ -207,26 +207,18 @@ func manageResource(ctx context.Context, cli client.Client, res *resource.Resour
 		if !k8serr.IsNotFound(err) {
 			return err
 		}
-		// Create resource if it doesn't exist and component enabled
-		if enabled {
-			return createResource(ctx, cli, res, owner)
-		}
-		// Skip if resource doesn't exist and component is disabled
-		return nil
 	}
 
-	// when resource is found
+	// Component is enabled
 	if enabled {
-		// Exception to not update kserve with managed annotation
-		// do not reconcile kserve resource with annotation "opendatahub.io/managed: false"
-		// TODO: remove this exception when we define managed annotation across odh
-		if found.GetAnnotations()[annotations.ManagedByODHOperator] == "false" && componentName == "kserve" {
-			return nil
-		}
-		return updateResource(ctx, cli, res, found, owner)
+		// Create resource if it doesn't exist OR reconcile component if exists
+		return CreateOrUpdateResource(ctx, cli, res, found, owner)
+	}
+	// Component id removed: delete resource if it exists or do nothing if not found
+	if found != nil {
+		return handleDisabledComponent(ctx, cli, found, componentName)
 	}
-	// Delete resource if it exists and component is disabled
-	return handleDisabledComponent(ctx, cli, found, componentName)
+	return nil
 }
 
 func getResource(ctx context.Context, cli client.Client, obj *resource.Resource) (*unstructured.Unstructured, error) {
@@ -273,36 +265,33 @@ func deleteResource(ctx context.Context, cli client.Client, found *unstructured.
 	return nil
 }
 
-func createResource(ctx context.Context, cli client.Client, res *resource.Resource, owner metav1.Object) error {
+func CreateOrUpdateResource(ctx context.Context, cli client.Client, res *resource.Resource, found *unstructured.Unstructured, owner metav1.Object) error {
 	obj, err := conversion.ResourceToUnstructured(res)
 	if err != nil {
 		return err
 	}
-	if obj.GetKind() != "CustomResourceDefinition" && obj.GetKind() != "OdhDashboardConfig" {
-		if err := ctrl.SetControllerReference(owner, metav1.Object(obj), cli.Scheme()); err != nil {
-			return err
-		}
+
+	// Create case
+	if found == nil {
+		return createResource(ctx, cli, obj, owner)
 	}
-	return cli.Create(ctx, obj)
-}
 
-func updateResource(ctx context.Context, cli client.Client, res *resource.Resource, found *unstructured.Unstructured, owner metav1.Object) error {
-	// Skip ODHDashboardConfig Update
+	// Exception to skip ODHDashboardConfig CR reconcile
 	if found.GetKind() == "OdhDashboardConfig" {
 		return nil
 	}
-
-	// only reconcile whiltelistedFields if the existing resource has annoation set to "true"
-	// all other cases, whiltelistedfields will be skipped by ODH operator
-	if managed, exists := found.GetAnnotations()[annotations.ManagedByODHOperator]; !exists || managed != "true" {
+	// handl allowlisted fields with annoation
+	switch managed, exists := found.GetAnnotations()[annotations.ManagedByODHOperator]; {
+	case !exists: // found did not have annoataion
 		if err := skipUpdateOnAllowlistedFields(res); err != nil {
 			return err
 		}
-	}
-
-	obj, err := conversion.ResourceToUnstructured(res)
-	if err != nil {
-		return err
+	case managed != "true":
+		if err := skipUpdateOnAllowlistedFields(res); err != nil {
+			return err
+		}
+	default: // case managed == "true":
+		// noop
 	}
 
 	// Retain existing labels on update
@@ -319,7 +308,6 @@ func skipUpdateOnAllowlistedFields(res *resource.Resource) error {
 			return err
 		}
 	}
-
 	return nil
 }
 
@@ -335,11 +323,24 @@ func updateLabels(found, obj *unstructured.Unstructured) {
 	obj.SetLabels(foundLabels)
 }
 
+// createResource works for create cases.
+func createResource(ctx context.Context, cli client.Client, obj *unstructured.Unstructured, owner metav1.Object) error {
+	if obj.GetKind() != "CustomResourceDefinition" && obj.GetKind() != "OdhDashboardConfig" {
+		// force controller
+		if err := ctrl.SetControllerReference(owner, metav1.Object(obj), cli.Scheme()); err != nil {
+			return err
+		}
+	}
+	return cli.Create(ctx, obj)
+}
+
+// preformPatch works for update cases.
 func performPatch(ctx context.Context, cli client.Client, obj, found *unstructured.Unstructured, owner metav1.Object) error {
 	data, err := json.Marshal(obj)
 	if err != nil {
 		return err
 	}
+	// force owner to be default-dsc
 	return cli.Patch(ctx, found, client.RawPatch(types.ApplyPatchType, data), client.ForceOwnership, client.FieldOwner(owner.GetName()))
 }
 

pkg/upgrade/upgrade.go

@@ -242,7 +242,7 @@ func CleanupExistingResource(ctx context.Context, cli client.Client, platform cl
 	multiErr = multierror.Append(multiErr, deleteDeprecatedServiceMonitors(ctx, cli, dscMonitoringNamespace, deprecatedOperatorSM))
 
 	// Remove deprecated opendatahub namespace(previously owned by kuberay and Kueue)
-	multiErr = multierror.Append(multiErr, deleteDeprecatedNamespace(ctx, cli, "opendatahub"))
+	multiErr = multierror.Append(multiErr, deleteDeprecatedNamespace(ctx, cli, cluster.ODHApplicationNamespace))
 
 	// Handling for dashboard OdhApplication Jupyterhub CR, see jira #443
 	multiErr = multierror.Append(multiErr, removOdhApplicationsCR(ctx, cli, gvk.OdhApplication, "jupyterhub", dscApplicationsNamespace))