fix: remove ui plugin finalizers in favor of k8s garbage collection

Signed-off-by: Gabriel Bernal <gbernal@redhat.com>

Author: jgbernalp

hack/dev-deploy.sh

@@ -46,10 +46,7 @@ CAT_NAME=$(oc get catalogsource | grep 'observability-operator' | awk '{print $1
 SUB_NAME=$(oc get subscriptions | grep 'observability-operator' | awk '{print $1}') && oc delete subscriptions "${SUB_NAME}"
 CSV_NAME=$(oc get clusterserviceversion | grep 'observability-operator' | awk '{print $1}') && oc delete clusterserviceversion "${CSV_NAME}"
 
-# delete uiplugin if hanging by unblock finalizer
-kubectl patch uiplugin monitoring --type='merge' -p='{"metadata":{"finalizers":null}}'
-
-# OR Delete the whole operator 
+# OR Delete the whole operator
 operator-sdk cleanup observability-operator -n openshift-operators
 
 # Run the bundle using the fully qualified image tag.

pkg/controllers/uiplugin/controller.go

@@ -224,35 +224,19 @@ func (rm resourceManager) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 	if !plugin.ObjectMeta.DeletionTimestamp.IsZero() {
 		logger.V(6).Info("deregistering plugin from the console")
 		if err := rm.deregisterPluginFromConsole(ctx, pluginTypeToConsoleName[plugin.Spec.Type]); err != nil {
-			return ctrl.Result{}, err
+			logger.V(3).Info("best-effort console deregistration failed during deletion", "error", err)
 		}
 
-		// Remove finalizer if present
-		if controllerutil.ContainsFinalizer(plugin, finalizerName) {
-			patch := client.MergeFrom(plugin.DeepCopy())
-			controllerutil.RemoveFinalizer(plugin, finalizerName)
-			if err := rm.k8sClient.Patch(ctx, plugin, patch); err != nil {
-				if apierrors.IsNotFound(err) {
-					return ctrl.Result{}, nil
-				}
-				return ctrl.Result{}, err
-			}
+		if err := rm.removeLegacyFinalizer(ctx, plugin); err != nil {
+			return ctrl.Result{}, err
 		}
 
 		logger.V(6).Info("skipping reconcile since object is already scheduled for deletion")
 		return ctrl.Result{}, nil
 	}
 
-	// Add finalizer if not present
-	if !controllerutil.ContainsFinalizer(plugin, finalizerName) {
-		patch := client.MergeFrom(plugin.DeepCopy())
-		controllerutil.AddFinalizer(plugin, finalizerName)
-		if err := rm.k8sClient.Patch(ctx, plugin, patch); err != nil {
-			if apierrors.IsNotFound(err) {
-				return ctrl.Result{}, nil
-			}
-			return ctrl.Result{}, err
-		}
+	if err := rm.removeLegacyFinalizer(ctx, plugin); err != nil {
+		return ctrl.Result{}, err
 	}
 
 	compatibilityInfo, err := lookupImageAndFeatures(plugin.Spec.Type, rm.clusterVersion)
@@ -424,6 +408,21 @@ func (rm resourceManager) deregisterPluginFromConsole(ctx context.Context, plugi
 	return nil
 }
 
+func (rm resourceManager) removeLegacyFinalizer(ctx context.Context, plugin *uiv1alpha1.UIPlugin) error {
+	if !controllerutil.ContainsFinalizer(plugin, finalizerName) {
+		return nil
+	}
+	patch := client.MergeFrom(plugin.DeepCopy())
+	controllerutil.RemoveFinalizer(plugin, finalizerName)
+	if err := rm.k8sClient.Patch(ctx, plugin, patch); err != nil {
+		if apierrors.IsNotFound(err) {
+			return nil
+		}
+		return err
+	}
+	return nil
+}
+
 func (rm resourceManager) getUIPlugin(ctx context.Context, req ctrl.Request) (*uiv1alpha1.UIPlugin, error) {
 	logger := rm.logger.WithValues("plugin", req.NamespacedName)
 

pkg/controllers/uiplugin/plugin_info_builder.go

@@ -48,6 +48,10 @@ var pluginTypeToConsoleName = map[uiv1alpha1.UIPluginType]string{
 	uiv1alpha1.TypeMonitoring:           "monitoring-console-plugin",
 }
 
+func ConsoleNameForType(pluginType uiv1alpha1.UIPluginType) string {
+	return pluginTypeToConsoleName[pluginType]
+}
+
 func PluginInfoBuilder(ctx context.Context, k client.Client, dk dynamic.Interface, plugin *uiv1alpha1.UIPlugin, pluginConf UIPluginsConfiguration, compatibilityInfo CompatibilityEntry, clusterVersion string, logger logr.Logger) (*UIPluginInfo, error) {
 	image := pluginConf.Images[compatibilityInfo.ImageKey]
 	if image == "" {

pkg/operator/operator.go

@@ -6,16 +6,19 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"slices"
 	"time"
 
 	configv1 "github.com/openshift/api/config/v1"
+	operatorv1 "github.com/openshift/api/operator/v1"
 	openshifttls "github.com/openshift/controller-runtime-common/pkg/tls"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/server/dynamiccertificates"
 	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
@@ -45,6 +48,7 @@ const (
 // OpenShift installations).
 type Operator struct {
 	manager               manager.Manager
+	restConfig            *rest.Config
 	servingCertController *dynamiccertificates.DynamicServingCertificateController
 	clientCAController    *dynamiccertificates.ConfigMapCAController
 }
@@ -382,11 +386,20 @@ func New(ctx context.Context, cfg *OperatorConfiguration) (*Operator, error) {
 		return nil, fmt.Errorf("unable to add health probe: %w", err)
 	}
 
-	return &Operator{
+	op := &Operator{
 		manager:               mgr,
+		restConfig:            restConfig,
 		servingCertController: servingCertController,
 		clientCAController:    clientCAController,
-	}, nil
+	}
+
+	if cfg.FeatureGates.OpenShift.Enabled {
+		if err := mgr.Add(op.newShutdownCleanupRunnable()); err != nil {
+			return nil, fmt.Errorf("unable to add shutdown cleanup runnable: %w", err)
+		}
+	}
+
+	return op, nil
 }
 
 func (o *Operator) Start(ctx context.Context) error {
@@ -405,6 +418,75 @@ func (o *Operator) Start(ctx context.Context) error {
 	return nil
 }
 
+func (o *Operator) newShutdownCleanupRunnable() manager.Runnable {
+	return manager.RunnableFunc(func(ctx context.Context) error {
+		// Block until the manager's context is cancelled (shutdown signal).
+		<-ctx.Done()
+		o.cleanupUIPluginsFromConsole()
+		return nil
+	})
+}
+
+func (o *Operator) cleanupUIPluginsFromConsole() {
+	logger := ctrl.Log.WithName("shutdown-cleanup")
+	logger.Info("attempting best-effort UIPlugin console deregistration")
+
+	cleanupCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	directClient, err := client.New(o.restConfig, client.Options{
+		Scheme: o.manager.GetScheme(),
+	})
+	if err != nil {
+		logger.Error(err, "failed to create client for shutdown cleanup")
+		return
+	}
+
+	pluginList := &uiv1alpha1.UIPluginList{}
+	if err := directClient.List(cleanupCtx, pluginList); err != nil {
+		logger.Error(err, "failed to list UIPlugins during shutdown cleanup")
+		return
+	}
+
+	if len(pluginList.Items) == 0 {
+		return
+	}
+
+	toRemove := make(map[string]struct{}, len(pluginList.Items))
+	for _, plugin := range pluginList.Items {
+		if name := uictrl.ConsoleNameForType(plugin.Spec.Type); name != "" {
+			toRemove[name] = struct{}{}
+		}
+	}
+
+	if len(toRemove) == 0 {
+		return
+	}
+
+	cluster := &operatorv1.Console{}
+	if err := directClient.Get(cleanupCtx, client.ObjectKey{Name: "cluster"}, cluster); err != nil {
+		logger.Error(err, "failed to get Console CR during shutdown cleanup")
+		return
+	}
+
+	original := cluster.DeepCopy()
+	cluster.Spec.Plugins = slices.DeleteFunc(cluster.Spec.Plugins, func(name string) bool {
+		_, ok := toRemove[name]
+		return ok
+	})
+
+	if slices.Equal(cluster.Spec.Plugins, original.Spec.Plugins) {
+		return
+	}
+
+	patch := client.MergeFrom(original)
+	if err := directClient.Patch(cleanupCtx, cluster, patch); err != nil {
+		logger.Error(err, "failed to patch Console CR during shutdown cleanup")
+		return
+	}
+	logger.Info("successfully cleaned up Console CR during shutdown")
+}
+
 func (o *Operator) GetClient() client.Client {
 	return o.manager.GetClient()
 }