Implement garbage collection action (#1374)

Signed-off-by: Luca Burgazzoli <[email protected]>

.golangci.yml

@@ -59,6 +59,8 @@ linters-settings:
       # also allow generics
       - generic
       - EventHandler # for ToOwner
+      - discovery.DiscoveryInterface
+      - dynamic.Interface
       - predicate.Predicate
       - client.Object
   revive:

controllers/components/dashboard/dashboard_controller.go

@@ -31,7 +31,7 @@ import (
 	componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
 	"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/render"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/security"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
@@ -90,7 +90,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(configureDependencies).
 		WithAction(security.NewUpdatePodSecurityRoleBindingAction(serviceAccounts)).
 		WithAction(kustomize.NewAction(
-			kustomize.WithCache(render.DefaultCachingKeyFn),
+			kustomize.WithCache(),
 			// Those are the default labels added by the legacy deploy method
 			// and should be preserved as the original plugin were affecting
 			// deployment selectors that are immutable once created, so it won't
@@ -112,6 +112,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 			updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.DashboardInstanceName),
 		)).
 		WithAction(updateStatus).
+		// must be the final action
+		WithAction(gc.NewAction(
+			gc.WithLabel(labels.ComponentPartOf, componentsv1.DashboardInstanceName),
+			gc.WithUnremovables(gvk.OdhDashboardConfig),
+		)).
 		Build(ctx)
 
 	if err != nil {

controllers/components/datasciencepipelines/datasciencepipelines_controller.go

@@ -29,7 +29,7 @@ import (
 
 	componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources"
@@ -71,7 +71,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(initialize).
 		WithAction(devFlags).
 		WithAction(kustomize.NewAction(
-			kustomize.WithCache(render.DefaultCachingKeyFn),
+			kustomize.WithCache(),
 			kustomize.WithLabel(labels.ODH.Component(componentsv1.DataSciencePipelinesComponentName), "true"),
 			kustomize.WithLabel(labels.K8SCommon.PartOf, componentsv1.DataSciencePipelinesComponentName),
 		)).
@@ -83,6 +83,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(updatestatus.NewAction(
 			updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.DataSciencePipelinesInstanceName),
 		)).
+		// must be the final action
+		WithAction(gc.NewAction(
+			gc.WithLabel(labels.ComponentPartOf, componentsv1.DataSciencePipelinesInstanceName),
+		)).
 		Build(ctx)
 
 	if err != nil {

controllers/components/kueue/kueue_controller.go

@@ -30,7 +30,7 @@ import (
 
 	componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources"
@@ -64,7 +64,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(initialize).
 		WithAction(devFlags).
 		WithAction(kustomize.NewAction(
-			kustomize.WithCache(render.DefaultCachingKeyFn),
+			kustomize.WithCache(),
 			kustomize.WithLabel(labels.ODH.Component(ComponentName), "true"),
 			kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName),
 		)).
@@ -76,6 +76,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(updatestatus.NewAction(
 			updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.KueueInstanceName),
 		)).
+		// must be the final action
+		WithAction(gc.NewAction(
+			gc.WithLabel(labels.ComponentPartOf, componentsv1.KueueInstanceName),
+		)).
 		Build(ctx)
 
 	if err != nil {

controllers/components/modelregistry/modelregistry_controller.go

@@ -28,12 +28,15 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
+	dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1"
 	"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/render"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/template"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/generation"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/reconciler"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/metadata/labels"
@@ -56,6 +59,13 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		Owns(&appsv1.Deployment{}, reconciler.WithPredicates(resources.NewDeploymentPredicate())).
 		Owns(&admissionregistrationv1.MutatingWebhookConfiguration{}).
 		Owns(&admissionregistrationv1.ValidatingWebhookConfiguration{}).
+		// MR also depends on DSCInitialization to properly configure the SMM
+		// resource
+		Watches(
+			&dsciv1.DSCInitialization{},
+			reconciler.WithEventHandler(handlers.ToNamed(componentsv1.ModelRegistryInstanceName)),
+			reconciler.WithPredicates(generation.New()),
+		).
 		Watches(&corev1.Namespace{}).
 		Watches(&extv1.CustomResourceDefinition{}).
 		// Some ClusterRoles are part of the component deployment, but not owned by
@@ -71,10 +81,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(initialize).
 		WithAction(configureDependencies).
 		WithAction(template.NewAction(
-			template.WithCache(render.DefaultCachingKeyFn),
+			template.WithCache(),
 		)).
 		WithAction(kustomize.NewAction(
-			kustomize.WithCache(render.DefaultCachingKeyFn),
+			kustomize.WithCache(),
 			kustomize.WithLabel(labels.ODH.Component(ComponentName), "true"),
 			kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName),
 		)).
@@ -88,6 +98,11 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 			updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.ModelRegistryInstanceName),
 		)).
 		WithAction(updateStatus).
+		// must be the final action
+		WithAction(gc.NewAction(
+			gc.WithLabel(labels.ComponentPartOf, componentsv1.ModelRegistryInstanceName),
+			gc.WithUnremovables(gvk.ServiceMeshMember),
+		)).
 		Build(ctx)
 
 	if err != nil {

controllers/components/modelregistry/modelregistry_controller_actions.go

@@ -57,6 +57,7 @@ func initialize(ctx context.Context, rr *odhtypes.ReconciliationRequest) error {
 		baseManifestInfo(BaseManifestsSourcePath),
 		extraManifestInfo(BaseManifestsSourcePath),
 	}
+
 	rr.Templates = []odhtypes.TemplateInfo{{
 		FS:   resourcesFS,
 		Path: ServiceMeshMemberTemplate,

controllers/components/ray/ray_controller.go

@@ -28,7 +28,7 @@ import (
 
 	componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources"
@@ -57,7 +57,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(initialize).
 		WithAction(devFlags).
 		WithAction(kustomize.NewAction(
-			kustomize.WithCache(render.DefaultCachingKeyFn),
+			kustomize.WithCache(),
 			kustomize.WithLabel(labels.ODH.Component(ComponentName), "true"),
 			kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName),
 		)).
@@ -69,6 +69,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(updatestatus.NewAction(
 			updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.RayInstanceName),
 		)).
+		// must be the final action
+		WithAction(gc.NewAction(
+			gc.WithLabel(labels.ComponentPartOf, componentsv1.RayInstanceName),
+		)).
 		Build(ctx)
 
 	if err != nil {

controllers/components/trainingoperator/trainingoperator_controller.go

@@ -28,7 +28,7 @@ import (
 
 	componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources"
@@ -54,7 +54,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(initialize).
 		WithAction(devFlags).
 		WithAction(kustomize.NewAction(
-			kustomize.WithCache(render.DefaultCachingKeyFn),
+			kustomize.WithCache(),
 			kustomize.WithLabel(labels.ODH.Component(ComponentName), "true"),
 			kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName),
 		)).
@@ -66,6 +66,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(updatestatus.NewAction(
 			updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.TrainingOperatorInstanceName),
 		)).
+		// must be the final action
+		WithAction(gc.NewAction(
+			gc.WithLabel(labels.ComponentPartOf, componentsv1.TrainingOperatorInstanceName),
+		)).
 		Build(ctx)
 
 	if err != nil {

controllers/components/trustyai/trustyai_controller.go

@@ -27,7 +27,7 @@ import (
 
 	componentsv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
-	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/resources"
@@ -55,7 +55,7 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(initialize).
 		WithAction(devFlags).
 		WithAction(kustomize.NewAction(
-			kustomize.WithCache(render.DefaultCachingKeyFn),
+			kustomize.WithCache(),
 			kustomize.WithLabel(labels.ODH.Component(ComponentName), "true"),
 			kustomize.WithLabel(labels.K8SCommon.PartOf, ComponentName),
 		)).
@@ -67,6 +67,10 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
 		WithAction(updatestatus.NewAction(
 			updatestatus.WithSelectorLabel(labels.ComponentPartOf, componentsv1.TrustyAIInstanceName),
 		)).
+		// must be the final action
+		WithAction(gc.NewAction(
+			gc.WithLabel(labels.ComponentPartOf, componentsv1.TrustyAIInstanceName),
+		)).
 		Build(ctx)
 
 	if err != nil {

controllers/dscinitialization/suite_test.go

@@ -123,7 +123,7 @@ var _ = BeforeSuite(func() {
 	Expect(err).NotTo(HaveOccurred())
 	Expect(k8sClient).NotTo(BeNil())
 
-	odhClient, err := odhClient.New(gCtx, cfg, k8sClient)
+	odhClient, err := odhClient.NewFromConfig(cfg, k8sClient)
 	Expect(err).NotTo(HaveOccurred())
 	Expect(odhClient).NotTo(BeNil())
 

main.go

@@ -36,6 +36,7 @@ import (
 	promv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	appsv1 "k8s.io/api/apps/v1"
+	authorizationv1 "k8s.io/api/authorization/v1"
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -73,6 +74,7 @@ import (
 	odhClient "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/logger"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources"
+	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/services/gc"
 	"github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade"
 
 	_ "github.com/opendatahub-io/opendatahub-operator/v2/controllers/components/dashboard"
@@ -245,6 +247,7 @@ func main() { //nolint:funlen,maintidx
 			Cache: &client.CacheOptions{
 				DisableFor: []client.Object{
 					resources.GvkToUnstructured(gvk.OpenshiftIngress),
+					&authorizationv1.SelfSubjectRulesReview{},
 				},
 				// Set it to true so the cache-backed client reads unstructured objects
 				// or lists from the cache instead of a live lookup.
@@ -259,7 +262,7 @@ func main() { //nolint:funlen,maintidx
 
 	webhook.Init(mgr)
 
-	oc, err := odhClient.NewFromManager(ctx, mgr)
+	oc, err := odhClient.NewFromManager(mgr)
 	if err != nil {
 		setupLog.Error(err, "unable to create client")
 		os.Exit(1)
@@ -309,6 +312,24 @@ func main() { //nolint:funlen,maintidx
 		os.Exit(1)
 	}
 
+	ons, err := cluster.GetOperatorNamespace()
+	if err != nil {
+		setupLog.Error(err, "unable to determine Operator Namespace")
+		os.Exit(1)
+	}
+
+	gc.Instance = gc.New(
+		oc,
+		ons,
+		gc.WithUnremovables(gvk.CustomResourceDefinition, gvk.Lease),
+	)
+
+	err = mgr.Add(gc.Instance)
+	if err != nil {
+		setupLog.Error(err, "unable to register GC service")
+		os.Exit(1)
+	}
+
 	// Initialize component reconcilers
 	if err = CreateComponentReconcilers(ctx, mgr); err != nil {
 		os.Exit(1)

pkg/cluster/gvk/gvk.go

@@ -2,6 +2,7 @@ package gvk
 
 import (
 	appsv1 "k8s.io/api/apps/v1"
+	coordinationv1 "k8s.io/api/coordination/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -174,4 +175,10 @@ var (
 		Version: "v1",
 		Kind:    "ServiceMeshMember",
 	}
+
+	Lease = schema.GroupVersionKind{
+		Group:   coordinationv1.SchemeGroupVersion.Group,
+		Version: coordinationv1.SchemeGroupVersion.Version,
+		Kind:    "Lease",
+	}
 )

pkg/controller/actions/deleteresource/action_delete_resources_test.go

@@ -28,7 +28,6 @@ func TestDeleteResourcesAction(t *testing.T) {
 	ns := xid.New().String()
 
 	cl, err := fakeclient.New(
-		ctx,
 		&appsv1.Deployment{
 			TypeMeta: metav1.TypeMeta{
 				APIVersion: gvk.Deployment.GroupVersion().String(),

pkg/controller/actions/deploy/action_deploy.go

@@ -113,8 +113,20 @@ func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) er
 
 	controllerName := strings.ToLower(rr.Instance.GetObjectKind().GroupVersionKind().Kind)
 
+	deployHash, err := odhTypes.HashStr(rr)
+	if err != nil {
+		return fmt.Errorf("unable to compute reauest hash: %w", err)
+	}
+
+	deployAnnotations := map[string]string{
+		annotations.ComponentGeneration: strconv.FormatInt(rr.Instance.GetGeneration(), 10),
+		annotations.ComponentHash:       deployHash,
+		annotations.PlatformType:        string(rr.Release.Name),
+		annotations.PlatformVersion:     rr.Release.Version.String(),
+	}
+
 	for i := range rr.Resources {
-		ok, err := a.deploy(ctx, rr, rr.Resources[i])
+		ok, err := a.deploy(ctx, rr, rr.Resources[i], deployAnnotations)
 		if err != nil {
 			return fmt.Errorf("failure deploying %s: %w", rr.Resources[i], err)
 		}
@@ -131,6 +143,7 @@ func (a *Action) deploy(
 	ctx context.Context,
 	rr *odhTypes.ReconciliationRequest,
 	obj unstructured.Unstructured,
+	deployAnnotations map[string]string,
 ) (bool, error) {
 	current, lookupErr := a.lookup(ctx, rr.Client, obj)
 	if lookupErr != nil {
@@ -145,10 +158,7 @@ func (a *Action) deploy(
 
 	resources.SetLabels(&obj, a.labels)
 	resources.SetAnnotations(&obj, a.annotations)
-
-	resources.SetAnnotation(&obj, annotations.ComponentGeneration, strconv.FormatInt(rr.Instance.GetGeneration(), 10))
-	resources.SetAnnotation(&obj, annotations.PlatformType, string(rr.Release.Name))
-	resources.SetAnnotation(&obj, annotations.PlatformVersion, rr.Release.Version.String())
+	resources.SetAnnotations(&obj, deployAnnotations)
 
 	// backup copy for caching
 	origObj := obj.DeepCopy()