Improve gc service/action

pkg/controller/actions/gc/action_gc.go

@@ -17,15 +17,17 @@ import (
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/services/gc"
 )
 
-type PredicateFn func(*odhTypes.ReconciliationRequest, unstructured.Unstructured) (bool, error)
+type ObjectPredicateFn func(*odhTypes.ReconciliationRequest, unstructured.Unstructured) (bool, error)
+type TypePredicateFn func(*odhTypes.ReconciliationRequest, schema.GroupVersionKind) (bool, error)
 type ActionOpts func(*Action)
 
 type Action struct {
-	labels       map[string]string
-	selector     labels.Selector
-	unremovables []schema.GroupVersionKind
-	gc           *gc.GC
-	predicateFn  PredicateFn
+	labels            map[string]string
+	selector          labels.Selector
+	unremovables      []schema.GroupVersionKind
+	gc                *gc.GC
+	objectPredicateFn ObjectPredicateFn
+	typePredicateFn   TypePredicateFn
 }
 
 func WithLabel(name string, value string) ActionOpts {
@@ -56,13 +58,22 @@ func WithUnremovables(items ...schema.GroupVersionKind) ActionOpts {
 	}
 }
 
-func WithPredicate(value PredicateFn) ActionOpts {
+func WithObjectPredicate(value ObjectPredicateFn) ActionOpts {
 	return func(action *Action) {
 		if value == nil {
 			return
 		}
 
-		action.predicateFn = value
+		action.objectPredicateFn = value
+	}
+}
+func WithTypePredicate(value TypePredicateFn) ActionOpts {
+	return func(action *Action) {
+		if value == nil {
+			return
+		}
+
+		action.typePredicateFn = value
 	}
 }
 
@@ -102,13 +113,20 @@ func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) er
 	deleted, err := a.gc.Run(
 		ctx,
 		selector,
-		func(ctx context.Context, obj unstructured.Unstructured) (bool, error) {
+		gc.WithTypeFilter(func(ctx context.Context, kind schema.GroupVersionKind) (bool, error) {
+			if slices.Contains(a.unremovables, kind) {
+				return false, nil
+			}
+
+			return a.typePredicateFn(rr, kind)
+		}),
+		gc.WithObjectFilter(func(ctx context.Context, obj unstructured.Unstructured) (bool, error) {
 			if slices.Contains(a.unremovables, obj.GroupVersionKind()) {
 				return false, nil
 			}
 
-			return a.predicateFn(rr, obj)
-		},
+			return a.objectPredicateFn(rr, obj)
+		}),
 	)
 
 	if err != nil {
@@ -124,7 +142,8 @@ func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) er
 
 func NewAction(opts ...ActionOpts) actions.Fn {
 	action := Action{}
-	action.predicateFn = DefaultPredicate
+	action.objectPredicateFn = DefaultObjectPredicate
+	action.typePredicateFn = DefaultTypePredicate
 	action.unremovables = make([]schema.GroupVersionKind, 0)
 
 	for _, opt := range opts {

pkg/controller/actions/gc/action_gc_support.go

@@ -5,13 +5,14 @@ import (
 	"strconv"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	odhTypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
 	odhAnnotations "github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/annotations"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
 )
 
-func DefaultPredicate(rr *odhTypes.ReconciliationRequest, obj unstructured.Unstructured) (bool, error) {
+func DefaultObjectPredicate(rr *odhTypes.ReconciliationRequest, obj unstructured.Unstructured) (bool, error) {
 	if obj.GetAnnotations() == nil {
 		return false, nil
 	}
@@ -44,3 +45,7 @@ func DefaultPredicate(rr *odhTypes.ReconciliationRequest, obj unstructured.Unstr
 
 	return rr.Instance.GetGeneration() != int64(g), nil
 }
+
+func DefaultTypePredicate(_ *odhTypes.ReconciliationRequest, _ schema.GroupVersionKind) (bool, error) {
+	return true, nil
+}

pkg/controller/actions/gc/action_gc_test.go

@@ -13,6 +13,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	authorizationv1 "k8s.io/api/authorization/v1"
 	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	k8serr "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -129,7 +130,7 @@ func TestGcAction(t *testing.T) {
 			generated:      true,
 			matcher:        Not(HaveOccurred()),
 			metricsMatcher: BeNumerically("==", 1),
-			options: []gc.ActionOpts{gc.WithPredicate(
+			options: []gc.ActionOpts{gc.WithObjectPredicate(
 				func(request *types.ReconciliationRequest, unstructured unstructured.Unstructured) (bool, error) {
 					return unstructured.GroupVersionKind() != gvk.ConfigMap, nil
 				},
@@ -254,3 +255,148 @@ func TestGcAction(t *testing.T) {
 		})
 	}
 }
+
+func TestGcActionCluster(t *testing.T) {
+	g := NewWithT(t)
+
+	s := runtime.NewScheme()
+	ctx := context.Background()
+	id := xid.New().String()
+	nsn := xid.New().String()
+
+	utilruntime.Must(corev1.AddToScheme(s))
+	utilruntime.Must(appsv1.AddToScheme(s))
+	utilruntime.Must(apiextensionsv1.AddToScheme(s))
+	utilruntime.Must(authorizationv1.AddToScheme(s))
+	utilruntime.Must(rbacv1.AddToScheme(s))
+
+	envTest := &envtest.Environment{
+		CRDInstallOptions: envtest.CRDInstallOptions{
+			Scheme:          s,
+			CleanUpAfterUse: true,
+		},
+	}
+
+	t.Cleanup(func() {
+		_ = envTest.Stop()
+	})
+
+	cfg, err := envTest.Start()
+	g.Expect(err).NotTo(HaveOccurred())
+
+	envTestClient, err := ctrlCli.New(cfg, ctrlCli.Options{Scheme: s})
+	g.Expect(err).NotTo(HaveOccurred())
+
+	cli, err := client.NewFromConfig(cfg, envTestClient)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(cli).NotTo(BeNil())
+
+	gci := gcSvc.New(
+		cli,
+		nsn,
+		// This is required as there are no kubernetes controller running
+		// with the envtest, hence we can't use the foreground deletion
+		// policy (default)
+		gcSvc.WithPropagationPolicy(metav1.DeletePropagationBackground),
+	)
+
+	ns := corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: nsn,
+		},
+	}
+
+	g.Expect(cli.Create(ctx, &ns)).
+		NotTo(HaveOccurred())
+	g.Expect(gci.Start(ctx)).
+		NotTo(HaveOccurred())
+
+	rr := types.ReconciliationRequest{
+		Client: cli,
+		DSCI: &dsciv1.DSCInitialization{
+			ObjectMeta: metav1.ObjectMeta{
+				Generation: 1,
+			},
+		},
+		Instance: &componentApi.Dashboard{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: componentApi.GroupVersion.String(),
+				Kind:       componentApi.DashboardKind,
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Generation: 1,
+				UID:        apytypes.UID(id),
+			},
+		},
+		Release: common.Release{
+			Name: cluster.OpenDataHub,
+			Version: version.OperatorVersion{
+				Version: semver.Version{Major: 0, Minor: 2, Patch: 0},
+			},
+		},
+		Generated: true,
+	}
+
+	om := metav1.ObjectMeta{
+		Namespace: nsn,
+		Annotations: map[string]string{
+			annotations.InstanceGeneration: "1",
+			annotations.InstanceUID:        id,
+			annotations.PlatformType:       string(cluster.OpenDataHub),
+		},
+		Labels: map[string]string{
+			labels.PlatformPartOf: strings.ToLower(componentApi.DashboardKind),
+		},
+	}
+
+	cm1 := corev1.ConfigMap{ObjectMeta: *om.DeepCopy()}
+	cm1.Name = xid.New().String()
+	cm1.Annotations[annotations.PlatformVersion] = "0.1.0"
+
+	cm2 := corev1.ConfigMap{ObjectMeta: *om.DeepCopy()}
+	cm2.Name = xid.New().String()
+	cm2.Annotations[annotations.PlatformVersion] = rr.Release.Version.String()
+
+	cr1 := rbacv1.ClusterRole{ObjectMeta: *om.DeepCopy()}
+	cr1.Name = xid.New().String()
+	cr1.Annotations[annotations.PlatformVersion] = "0.1.0"
+
+	cr2 := rbacv1.ClusterRole{ObjectMeta: *om.DeepCopy()}
+	cr2.Name = xid.New().String()
+	cr2.Annotations[annotations.PlatformVersion] = rr.Release.Version.String()
+
+	g.Expect(cli.Create(ctx, &cm1)).
+		NotTo(HaveOccurred())
+
+	g.Expect(cli.Create(ctx, &cm2)).
+		NotTo(HaveOccurred())
+
+	g.Expect(cli.Create(ctx, &cr1)).
+		NotTo(HaveOccurred())
+
+	g.Expect(cli.Create(ctx, &cr2)).
+		NotTo(HaveOccurred())
+
+	a := gc.NewAction(gc.WithGC(gci))
+
+	gc.DeletedTotal.Reset()
+	gc.DeletedTotal.WithLabelValues("dashboard").Add(0)
+
+	err = a(ctx, &rr)
+	g.Expect(err).NotTo(HaveOccurred())
+
+	err = cli.Get(ctx, ctrlCli.ObjectKeyFromObject(&cm1), &corev1.ConfigMap{})
+	g.Expect(err).To(MatchError(k8serr.IsNotFound, "IsNotFound"))
+
+	err = cli.Get(ctx, ctrlCli.ObjectKeyFromObject(&cm2), &corev1.ConfigMap{})
+	g.Expect(err).ToNot(HaveOccurred())
+
+	err = cli.Get(ctx, ctrlCli.ObjectKeyFromObject(&cr1), &rbacv1.ClusterRole{})
+	g.Expect(err).To(MatchError(k8serr.IsNotFound, "IsNotFound"))
+
+	err = cli.Get(ctx, ctrlCli.ObjectKeyFromObject(&cr2), &rbacv1.ClusterRole{})
+	g.Expect(err).ToNot(HaveOccurred())
+
+	ct := testutil.ToFloat64(gc.DeletedTotal)
+	g.Expect(ct).Should(BeNumerically("==", 2))
+}