Skip to content

Commit 97cf331

Browse files
RafalKoreptaRafalKorepta
committed
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
1 parent 5dd27cc commit 97cf331

File tree

5 files changed

+177
-12
lines changed

5 files changed

+177
-12
lines changed

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,18 @@ rules:
188188
- patch
189189
- update
190190
- watch
191+
- apiGroups:
192+
- coordination.k8s.io
193+
resources:
194+
- leases
195+
verbs:
196+
- create
197+
- delete
198+
- get
199+
- list
200+
- patch
201+
- update
202+
- watch
191203
- apiGroups:
192204
- helm.toolkit.fluxcd.io
193205
resources:

‎operator/internal/controller/redpanda/decommission-role.yaml‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
---
2+
apiVersion: rbac.authorization.k8s.io/v1
3+
kind: ClusterRole
4+
metadata:
5+
name: decommissioner-role
6+
rules:
7+
- apiGroups:
8+
- ""
9+
resources:
10+
- events
11+
verbs:
12+
- create
13+
- patch
14+
- apiGroups:
15+
- ""
16+
resources:
17+
- persistentvolumeclaims
18+
verbs:
19+
- delete
20+
- get
21+
- list
22+
- watch
23+
- apiGroups:
24+
- ""
25+
resources:
26+
- persistentvolumes
27+
verbs:
28+
- patch
29+
- apiGroups:
30+
- ""
31+
resources:
32+
- pods
33+
- secrets
34+
verbs:
35+
- get
36+
- list
37+
- watch
38+
- apiGroups:
39+
- apps
40+
resources:
41+
- statefulsets
42+
verbs:
43+
- get
44+
- list
45+
- watch
46+
- apiGroups:
47+
- coordination.k8s.io
48+
resources:
49+
- leases
50+
verbs:
51+
- create
52+
- delete
53+
- get
54+
- list
55+
- patch
56+
- update
57+
- watch
58+
---
59+
apiVersion: rbac.authorization.k8s.io/v1
60+
kind: Role
61+
metadata:
62+
name: decommissioner-role
63+
namespace: default
64+
rules:
65+
- apiGroups:
66+
- ""
67+
resources:
68+
- events
69+
verbs:
70+
- create
71+
- patch
72+
- apiGroups:
73+
- ""
74+
resources:
75+
- persistentvolumeclaims
76+
verbs:
77+
- delete
78+
- get
79+
- list
80+
- watch
81+
- apiGroups:
82+
- ""
83+
resources:
84+
- pods
85+
- secrets
86+
verbs:
87+
- get
88+
- list
89+
- watch
90+
- apiGroups:
91+
- apps
92+
resources:
93+
- statefulsets
94+
verbs:
95+
- get
96+
- list
97+
- watch
98+
- apiGroups:
99+
- coordination.k8s.io
100+
resources:
101+
- leases
102+
verbs:
103+
- create
104+
- delete
105+
- get
106+
- list
107+
- patch
108+
- update
109+
- watch

‎operator/internal/controller/redpanda/redpanda_controller.go‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ import (
5454
)
5555

5656
const (
57-
FinalizerKey = "operator.redpanda.com/finalizer"
57+
FinalizerKey = "operator.redpanda.com/finalizer"
58+
FluxFinalizerKey = "finalizers.fluxcd.io"
5859

5960
NotManaged = "false"
6061

@@ -135,6 +136,10 @@ type RedpandaReconciler struct {
135136
// +kubebuilder:rbac:groups=cluster.redpanda.com,resources=redpandas/finalizers,verbs=update
136137
// +kubebuilder:rbac:groups=core,namespace=default,resources=events,verbs=create;patch
137138

139+
// sidecar resources
140+
// 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.
141+
// +kubebuilder:rbac:groups=coordination.k8s.io,namespace=default,resources=leases,verbs=get;list;watch;create;update;patch;delete
142+
138143
// SetupWithManager sets up the controller with the Manager.
139144
func (r *RedpandaReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
140145
if err := registerHelmReferencedIndex(ctx, mgr, "statefulset", &appsv1.StatefulSet{}); err != nil {
@@ -861,6 +866,10 @@ func (r *RedpandaReconciler) reconcileHelmRepository(ctx context.Context, rp *re
861866
}
862867

863868
func (r *RedpandaReconciler) reconcileDelete(ctx context.Context, rp *redpandav1alpha2.Redpanda) error {
869+
if err := r.deleteHelmChart(ctx, rp); err != nil {
870+
return err
871+
}
872+
864873
if controllerutil.ContainsFinalizer(rp, FinalizerKey) {
865874
controllerutil.RemoveFinalizer(rp, FinalizerKey)
866875
if err := r.Client.Update(ctx, rp); err != nil {
@@ -870,6 +879,35 @@ func (r *RedpandaReconciler) reconcileDelete(ctx context.Context, rp *redpandav1
870879
return nil
871880
}
872881

882+
func (r *RedpandaReconciler) deleteHelmChart(ctx context.Context, rp *redpandav1alpha2.Redpanda) error {
883+
// When HelmRelease is suspended it will not delete child resource HelmChart. In this function there is attempt
884+
// to delete HelmChart custom resource.
885+
// Reference:
886+
// https://github.com/fluxcd/helm-controller/blob/2d335f2aa0e2e0df2a631ebf19394aed07c556f3/internal/reconcile/helmchart_template.go#L163-L166
887+
// https://github.com/fluxcd/helm-controller/blob/2d335f2aa0e2e0df2a631ebf19394aed07c556f3/internal/controller/helmrelease_controller.go#L375-L388
888+
namespacedName := client.ObjectKey{Namespace: rp.Namespace, Name: rp.Namespace + "-" + rp.Name}
889+
var chart sourcev1.HelmChart
890+
err := r.Client.Get(ctx, namespacedName, &chart)
891+
if err != nil && !apierrors.IsNotFound(err) {
892+
// Return error to retry until we succeed.
893+
return fmt.Errorf("failed to get flux HelmChart '%s': %w", namespacedName.Name, err)
894+
} else if err == nil {
895+
if controllerutil.ContainsFinalizer(&chart, FluxFinalizerKey) {
896+
controllerutil.RemoveFinalizer(&chart, FluxFinalizerKey)
897+
if err := r.Client.Update(ctx, &chart); err != nil {
898+
return fmt.Errorf("failed to update flux HelmChart '%s': %w", namespacedName.Name, err)
899+
}
900+
}
901+
902+
// Delete the HelmChart.
903+
if err = r.Client.Delete(ctx, &chart); err != nil {
904+
return fmt.Errorf("failed to delete flux HelmChart '%s': %w", namespacedName.Name, err)
905+
}
906+
}
907+
908+
return nil
909+
}
910+
873911
func (r *RedpandaReconciler) createHelmReleaseFromTemplate(ctx context.Context, rp *redpandav1alpha2.Redpanda) (*helmv2beta2.HelmRelease, error) {
874912
log := ctrl.LoggerFrom(ctx).WithName("RedpandaReconciler.createHelmReleaseFromTemplate")
875913

‎operator/internal/controller/redpanda/redpanda_controller_test.go‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626

2727
"github.com/fluxcd/helm-controller/api/v2beta2"
2828
fluxclient "github.com/fluxcd/pkg/runtime/client"
29-
sourcecontrollerv1beta2 "github.com/fluxcd/source-controller/api/v1beta2"
3029
"github.com/go-logr/logr/testr"
3130
"github.com/stretchr/testify/assert"
3231
"github.com/stretchr/testify/require"
@@ -739,6 +738,10 @@ func (s *RedpandaControllerSuite) TestStableUIDAndGeneration() {
739738
s.compareSnapshot(flipped, flippedBack, isStable)
740739

741740
s.deleteAndWait(rp)
741+
742+
var hr v2beta2.HelmRelease
743+
err := s.client.Get(s.ctx, types.NamespacedName{Name: rp.Namespace + "-" + rp.Name, Namespace: rp.Namespace}, &hr)
744+
assert.True(s.T(), apierrors.IsNotFound(err))
742745
}
743746
}
744747

@@ -831,15 +834,6 @@ Starting helm repository that serves %q as the development version of the redpan
831834
for _, rp := range redpandas.Items {
832835
s.deleteAndWait(&rp)
833836
}
834-
835-
// For some reason, it seems that HelmCharts can get abandoned. Clean
836-
// them up to prevent hanging the NS deletion.
837-
var helmCharts sourcecontrollerv1beta2.HelmChartList
838-
s.NoError(s.env.Client().List(s.ctx, &helmCharts))
839-
840-
for _, chart := range helmCharts.Items {
841-
s.deleteAndWait(&chart)
842-
}
843837
})
844838
}
845839

@@ -956,7 +950,7 @@ func (s *RedpandaControllerSuite) minimalRP(useFlux bool) *redpandav1alpha2.Redp
956950
},
957951
},
958952
RBAC: &redpandav1alpha2.RBAC{
959-
Enabled: ptr.To(true),
953+
Enabled: ptr.To(false),
960954
},
961955
},
962956
},

‎operator/internal/controller/redpanda/role.yaml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,18 @@ rules:
188188
- patch
189189
- update
190190
- watch
191+
- apiGroups:
192+
- coordination.k8s.io
193+
resources:
194+
- leases
195+
verbs:
196+
- create
197+
- delete
198+
- get
199+
- list
200+
- patch
201+
- update
202+
- watch
191203
- apiGroups:
192204
- helm.toolkit.fluxcd.io
193205
resources:

0 commit comments

Comments
 (0)