Improve Kserve's FeatureTraker handing

- Make it possible to set FT's OwnerReference as Controller reference so
  the kubernetes garbage collector can block owner deletion till the FT
  has been deleted
- Make it possible to set FT's generated resources OwnerReference as
  Controller reference so the kubernetes garbage collector can block
  FT deletion till the resources have been deleted
- Add an Kserver reconciler action to remove legacy ownership on
  DSCI/DSC is any and related e2e tests

controllers/components/kserve/kserve_controller.go

@@ -29,7 +29,6 @@ import (
 	extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/event"
-	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
@@ -67,17 +66,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		// changes. The compareHashPredicate ensures that we don't needlessly enqueue
 		// requests if there are no changes that we don't care about.
 		Owns(&templatev1.Template{}, reconciler.WithPredicates(hash.Updated())).
-		// The FeatureTrackers are created slightly differently, and have
-		// ownerRefs set by controllerutil.SetOwnerReference() rather than
-		// controllerutil.SetControllerReference(), which means that the default
-		// eventHandler for Owns won't work, so a slightly modified variant is
-		// added here
-		Owns(&featuresv1.FeatureTracker{}, reconciler.WithEventHandler(
-			handler.EnqueueRequestForOwner(
-				mgr.GetScheme(),
-				mgr.GetRESTMapper(),
-				&componentApi.Kserve{},
-			))).
+		Owns(&featuresv1.FeatureTracker{}).
 		Owns(&networkingv1.NetworkPolicy{}).
 		Owns(&monitoringv1.ServiceMonitor{}).
 		Owns(&admissionregistrationv1.MutatingWebhookConfiguration{}).
@@ -155,6 +144,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(checkPreConditions).
 		WithAction(initialize).
 		WithAction(devFlags).
+		WithAction(removeLegacyFeatureTrackerOwnerRef).
 		WithAction(configureServerless).
 		WithAction(configureServiceMesh).
 		WithAction(kustomize.NewAction(

controllers/components/kserve/kserve_controller_actions.go

@@ -9,11 +9,14 @@ import (
 	operatorv1 "github.com/openshift/api/operator/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	k8serr "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
+	featuresv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/controllers/status"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
@@ -22,6 +25,7 @@ import (
 	"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/metadata/labels"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
 )
 
 func checkPreConditions(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
@@ -137,6 +141,32 @@ func devFlags(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
 	return nil
 }
 
+func removeLegacyFeatureTrackerOwnerRef(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
+	ftNames := []string{
+		rr.DSCI.Spec.ApplicationsNamespace + "-serverless-serving-deployment",
+		rr.DSCI.Spec.ApplicationsNamespace + "-serverless-net-istio-secret-filtering",
+		rr.DSCI.Spec.ApplicationsNamespace + "-serverless-serving-gateways",
+		rr.DSCI.Spec.ApplicationsNamespace + "-kserve-external-authz",
+	}
+
+	for _, ftName := range ftNames {
+		obj := &featuresv1.FeatureTracker{}
+		err := rr.Client.Get(ctx, client.ObjectKey{Name: ftName}, obj)
+		switch {
+		case k8serr.IsNotFound(err):
+			continue
+		case err != nil:
+			return fmt.Errorf("error while retrieving FeatureTracker %s: %w", ftName, err)
+		}
+
+		if err := resources.RemoveOwnerReferences(ctx, rr.Client, obj, isLegacyOwnerRef); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func configureServerless(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
 	k, ok := rr.Instance.(*componentApi.Kserve)
 	if !ok {

controllers/components/kserve/kserve_support.go

@@ -22,6 +22,7 @@ import (
 	dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	featuresv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
 	odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/feature"
@@ -266,3 +267,14 @@ func ownedViaFT(cli client.Client) handler.MapFunc {
 		return []reconcile.Request{}
 	}
 }
+
+func isLegacyOwnerRef(or metav1.OwnerReference) bool {
+	switch {
+	case or.APIVersion == gvk.DataScienceCluster.GroupVersion().String() && or.Kind == gvk.DataScienceCluster.Kind:
+		return true
+	case or.APIVersion == gvk.DSCInitialization.GroupVersion().String() && or.Kind == gvk.DSCInitialization.Kind:
+		return true
+	default:
+		return false
+	}
+}

pkg/cluster/gvk/gvk.go

@@ -8,6 +8,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
+	featuresv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/features/v1"
 	serviceApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/services/v1alpha1"
 )
 
@@ -28,6 +29,11 @@ var (
 		Version: "v1",
 		Kind:    "DSCInitialization",
 	}
+	FeatureTracker = schema.GroupVersionKind{
+		Group:   featuresv1.GroupVersion.Group,
+		Version: featuresv1.GroupVersion.Version,
+		Kind:    "FeatureTracker",
+	}
 
 	Deployment = schema.GroupVersionKind{
 		Group:   appsv1.SchemeGroupVersion.Group,

pkg/cluster/meta.go

@@ -37,6 +37,12 @@ func OwnedBy(owner metav1.Object, scheme *runtime.Scheme) MetaOptions {
 	}
 }
 
+func ControllerBy(owner metav1.Object, scheme *runtime.Scheme) MetaOptions {
+	return func(obj metav1.Object) error {
+		return controllerutil.SetControllerReference(owner, obj, scheme)
+	}
+}
+
 func WithLabels(labels ...string) MetaOptions {
 	return func(obj metav1.Object) error {
 		labelsMap, err := extractKeyValues(labels)

pkg/controller/actions/deploy/action_deploy.go

@@ -134,7 +134,7 @@ func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) er
 		default:
 			// Remove the DSC and DSCI owner reference if set, This is required during the
 			// transition from the old to the new operator.
-			if err := removeOwnerReferences(ctx, rr.Client, current, isLegacyOwnerRef); err != nil {
+			if err := resources.RemoveOwnerReferences(ctx, rr.Client, current, isLegacyOwnerRef); err != nil {
 				return err
 			}
 

pkg/controller/actions/deploy/action_deploy_support.go

@@ -1,11 +1,7 @@
 package deploy
 
 import (
-	"context"
-	"fmt"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
 )
@@ -20,54 +16,3 @@ func isLegacyOwnerRef(or metav1.OwnerReference) bool {
 		return false
 	}
 }
-
-// removeOwnerReferences removes all owner references from a Kubernetes object that match the provided predicate.
-//
-// This function iterates through the OwnerReferences of the given object, filters out those that satisfy
-// the predicate, and updates the object in the cluster using the provided client.
-//
-// Parameters:
-//   - ctx: The context for the request, which can carry deadlines, cancellation signals, and other request-scoped values.
-//   - cli: A controller-runtime client used to update the Kubernetes object.
-//   - obj: The Kubernetes object whose OwnerReferences are to be filtered. It must implement client.Object.
-//   - predicate: A function that takes an OwnerReference and returns true if the reference should be removed.
-//
-// Returns:
-//   - An error if the update operation fails, otherwise nil.
-func removeOwnerReferences(
-	ctx context.Context,
-	cli client.Client,
-	obj client.Object,
-	predicate func(reference metav1.OwnerReference) bool,
-) error {
-	oldRefs := obj.GetOwnerReferences()
-	if len(oldRefs) == 0 {
-		return nil
-	}
-
-	newRefs := oldRefs[:0]
-	for _, ref := range oldRefs {
-		if !predicate(ref) {
-			newRefs = append(newRefs, ref)
-		}
-	}
-
-	if len(newRefs) == len(oldRefs) {
-		return nil
-	}
-
-	obj.SetOwnerReferences(newRefs)
-
-	// Update the object in the cluster
-	if err := cli.Update(ctx, obj); err != nil {
-		return fmt.Errorf(
-			"failed to remove owner references from object %s/%s with gvk %s: %w",
-			obj.GetNamespace(),
-			obj.GetName(),
-			obj.GetObjectKind().GroupVersionKind(),
-			err,
-		)
-	}
-
-	return nil
-}

pkg/controller/actions/deploy/action_deploy_support_test.go

@@ -2,30 +2,12 @@
 package deploy
 
 import (
-	"context"
-	"path/filepath"
 	"testing"
 
-	"github.com/onsi/gomega/gstruct"
 	"github.com/onsi/gomega/types"
-	"github.com/rs/xid"
-	appsv1 "k8s.io/api/apps/v1"
-	corev1 "k8s.io/api/core/v1"
-	rbacv1 "k8s.io/api/rbac/v1"
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
-	"sigs.k8s.io/controller-runtime/pkg/envtest"
 
-	componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
-	dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1"
-	dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk"
-	odhCli "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client"
-	"github.com/opendatahub-io/opendatahub-operator/v2/tests/envtestutil"
 
 	. "github.com/onsi/gomega"
 )
@@ -97,93 +79,3 @@ func TestIsLegacyOwnerRef(t *testing.T) {
 		})
 	}
 }
-
-func TestRemoveOwnerRef(t *testing.T) {
-	g := NewWithT(t)
-	s := runtime.NewScheme()
-
-	ctx := context.Background()
-	ns := xid.New().String()
-
-	utilruntime.Must(corev1.AddToScheme(s))
-	utilruntime.Must(appsv1.AddToScheme(s))
-	utilruntime.Must(apiextensionsv1.AddToScheme(s))
-	utilruntime.Must(componentApi.AddToScheme(s))
-	utilruntime.Must(dsciv1.AddToScheme(s))
-	utilruntime.Must(dscv1.AddToScheme(s))
-	utilruntime.Must(rbacv1.AddToScheme(s))
-
-	projectDir, err := envtestutil.FindProjectRoot()
-	g.Expect(err).NotTo(HaveOccurred())
-
-	envTest := &envtest.Environment{
-		CRDInstallOptions: envtest.CRDInstallOptions{
-			Scheme: s,
-			Paths: []string{
-				filepath.Join(projectDir, "config", "crd", "bases"),
-			},
-			ErrorIfPathMissing: true,
-			CleanUpAfterUse:    false,
-		},
-	}
-
-	t.Cleanup(func() {
-		_ = envTest.Stop()
-	})
-
-	cfg, err := envTest.Start()
-	g.Expect(err).NotTo(HaveOccurred())
-
-	envTestClient, err := client.New(cfg, client.Options{Scheme: s})
-	g.Expect(err).NotTo(HaveOccurred())
-
-	cli, err := odhCli.NewFromConfig(cfg, envTestClient)
-	g.Expect(err).NotTo(HaveOccurred())
-
-	err = cli.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}})
-	g.Expect(err).ToNot(HaveOccurred())
-
-	cm1 := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm1", Namespace: ns}}
-	cm1.SetGroupVersionKind(gvk.ConfigMap)
-
-	err = cli.Create(ctx, cm1)
-	g.Expect(err).ToNot(HaveOccurred())
-
-	cm2 := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "cm2", Namespace: ns}}
-	cm2.SetGroupVersionKind(gvk.ConfigMap)
-
-	err = cli.Create(ctx, cm2)
-	g.Expect(err).ToNot(HaveOccurred())
-
-	// Create a ConfigMap with OwnerReferences
-	configMap := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "test-configmap", Namespace: ns}}
-
-	err = controllerutil.SetOwnerReference(cm1, configMap, s)
-	g.Expect(err).ToNot(HaveOccurred())
-	err = controllerutil.SetOwnerReference(cm2, configMap, s)
-	g.Expect(err).ToNot(HaveOccurred())
-
-	err = cli.Create(ctx, configMap)
-	g.Expect(err).ToNot(HaveOccurred())
-
-	predicate := func(ref metav1.OwnerReference) bool {
-		return ref.Name == cm1.Name
-	}
-
-	err = removeOwnerReferences(ctx, cli, configMap, predicate)
-	g.Expect(err).ToNot(HaveOccurred())
-
-	updatedConfigMap := &corev1.ConfigMap{}
-	err = cli.Get(ctx, client.ObjectKeyFromObject(configMap), updatedConfigMap)
-	g.Expect(err).ToNot(HaveOccurred())
-
-	g.Expect(updatedConfigMap.GetOwnerReferences()).Should(And(
-		HaveLen(1),
-		HaveEach(gstruct.MatchFields(gstruct.IgnoreExtras, gstruct.Fields{
-			"Name":       Equal(cm2.Name),
-			"APIVersion": Equal(gvk.ConfigMap.GroupVersion().String()),
-			"Kind":       Equal(gvk.ConfigMap.Kind),
-			"UID":        Equal(cm2.UID),
-		})),
-	))
-}