operator: Delete HelmChart custom resource when HelmRelease is suspended

When Redpanda resource is migrated from using flux to defluxed mode
the HelmRelease custom resource is put into suspend mode. That suspend mode
can affect deletion process as HelmChart (a child resource that doesn't have
ownerReference) is not fully reconciled by helmrelease_controller. Redpanda
operator now try to delete HelmChart.

Reference
https://github.com/fluxcd/helm-controller/blob/2d335f2aa0e2e0df2a631ebf19394aed07c556f3/internal/reconcile/helmchart_template.go#L163-L166
https://github.com/fluxcd/helm-controller/blob/2d335f2aa0e2e0df2a631ebf19394aed07c556f3/internal/controller/helmrelease_controller.go#L375-L388

Author: RafalKorepta

operator/config/rbac/v2-manager-role/role.yaml

@@ -188,6 +188,18 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - coordination.k8s.io
+  resources:
+  - leases
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
 - apiGroups:
   - helm.toolkit.fluxcd.io
   resources:

operator/internal/controller/redpanda/redpanda_controller.go

@@ -54,7 +54,8 @@ import (
 )
 
 const (
-	FinalizerKey = "operator.redpanda.com/finalizer"
+	FinalizerKey     = "operator.redpanda.com/finalizer"
+	FluxFinalizerKey = "finalizers.fluxcd.io"
 
 	NotManaged = "false"
 
@@ -135,6 +136,10 @@ type RedpandaReconciler struct {
 // +kubebuilder:rbac:groups=cluster.redpanda.com,resources=redpandas/finalizers,verbs=update
 // +kubebuilder:rbac:groups=core,namespace=default,resources=events,verbs=create;patch
 
+// sidecar resources
+// The leases is used by controller-runtime in sidecar. Operator main reconciliation needs to have leases permissions in order to create role that have the same permissions.
+// +kubebuilder:rbac:groups=coordination.k8s.io,namespace=default,resources=leases,verbs=get;list;watch;create;update;patch;delete
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *RedpandaReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
 	if err := registerHelmReferencedIndex(ctx, mgr, "statefulset", &appsv1.StatefulSet{}); err != nil {
@@ -861,6 +866,10 @@ func (r *RedpandaReconciler) reconcileHelmRepository(ctx context.Context, rp *re
 }
 
 func (r *RedpandaReconciler) reconcileDelete(ctx context.Context, rp *redpandav1alpha2.Redpanda) error {
+	if err := r.deleteHelmChart(ctx, rp); err != nil {
+		return err
+	}
+
 	if controllerutil.ContainsFinalizer(rp, FinalizerKey) {
 		controllerutil.RemoveFinalizer(rp, FinalizerKey)
 		if err := r.Client.Update(ctx, rp); err != nil {
@@ -870,6 +879,35 @@ func (r *RedpandaReconciler) reconcileDelete(ctx context.Context, rp *redpandav1
 	return nil
 }
 
+func (r *RedpandaReconciler) deleteHelmChart(ctx context.Context, rp *redpandav1alpha2.Redpanda) error {
+	// When HelmRelease is suspended it will not delete child resource HelmChart. In this function there is attempt
+	// to delete HelmChart custom resource.
+	// Reference:
+	// https://github.com/fluxcd/helm-controller/blob/2d335f2aa0e2e0df2a631ebf19394aed07c556f3/internal/reconcile/helmchart_template.go#L163-L166
+	// https://github.com/fluxcd/helm-controller/blob/2d335f2aa0e2e0df2a631ebf19394aed07c556f3/internal/controller/helmrelease_controller.go#L375-L388
+	namespacedName := client.ObjectKey{Namespace: rp.Namespace, Name: rp.Namespace + "-" + rp.Name}
+	var chart sourcev1.HelmChart
+	err := r.Client.Get(ctx, namespacedName, &chart)
+	if err != nil && !apierrors.IsNotFound(err) {
+		// Return error to retry until we succeed.
+		return fmt.Errorf("failed to get flux HelmChart '%s': %w", namespacedName.Name, err)
+	} else if err == nil {
+		if controllerutil.ContainsFinalizer(&chart, FluxFinalizerKey) {
+			controllerutil.RemoveFinalizer(&chart, FluxFinalizerKey)
+			if err := r.Client.Update(ctx, &chart); err != nil {
+				return fmt.Errorf("failed to update flux HelmChart '%s': %w", namespacedName.Name, err)
+			}
+		}
+
+		// Delete the HelmChart.
+		if err = r.Client.Delete(ctx, &chart); err != nil {
+			return fmt.Errorf("failed to delete flux HelmChart '%s': %w", namespacedName.Name, err)
+		}
+	}
+
+	return nil
+}
+
 func (r *RedpandaReconciler) createHelmReleaseFromTemplate(ctx context.Context, rp *redpandav1alpha2.Redpanda) (*helmv2beta2.HelmRelease, error) {
 	log := ctrl.LoggerFrom(ctx).WithName("RedpandaReconciler.createHelmReleaseFromTemplate")
 

operator/internal/controller/redpanda/redpanda_controller_test.go

@@ -739,6 +739,10 @@ func (s *RedpandaControllerSuite) TestStableUIDAndGeneration() {
 		s.compareSnapshot(flipped, flippedBack, isStable)
 
 		s.deleteAndWait(rp)
+
+		var hr v2beta2.HelmRelease
+		err := s.client.Get(s.ctx, types.NamespacedName{Name: rp.Namespace + "-" + rp.Name, Namespace: rp.Namespace}, &hr)
+		assert.True(s.T(), apierrors.IsNotFound(err))
 	}
 }
 
@@ -831,15 +835,6 @@ Starting helm repository that serves %q as the development version of the redpan
 		for _, rp := range redpandas.Items {
 			s.deleteAndWait(&rp)
 		}
-
-		// For some reason, it seems that HelmCharts can get abandoned. Clean
-		// them up to prevent hanging the NS deletion.
-		var helmCharts sourcecontrollerv1beta2.HelmChartList
-		s.NoError(s.env.Client().List(s.ctx, &helmCharts))
-
-		for _, chart := range helmCharts.Items {
-			s.deleteAndWait(&chart)
-		}
 	})
 }
 
@@ -956,7 +951,7 @@ func (s *RedpandaControllerSuite) minimalRP(useFlux bool) *redpandav1alpha2.Redp
 					},
 				},
 				RBAC: &redpandav1alpha2.RBAC{
-					Enabled: ptr.To(true),
+					Enabled: ptr.To(false),
 				},
 			},
 		},

operator/internal/controller/redpanda/role.yaml

@@ -188,6 +188,18 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - coordination.k8s.io
+  resources:
+  - leases
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
 - apiGroups:
   - helm.toolkit.fluxcd.io
   resources: