Skip to content

Commit 45273ba

Browse files
authored
enh: (helm) - add annotation to wait for all resources to be deleted before removing CR finalizer (#4487)
Signed-off-by: Mike Ng <[email protected]>
1 parent 2406b8d commit 45273ba

File tree

7 files changed

+394
-25
lines changed

7 files changed

+394
-25
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
entries:
2+
- description: >
3+
For Helm based-operators, added annotation `helm.sdk.operatorframework.io/uninstall-wait: "true"`
4+
to allow all resources to be deleted before removing the custom resource's finalizer.
5+
kind: "addition"
6+
breaking: false

internal/helm/controller/reconcile.go

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ const (
6060
uninstallFinalizer = "helm.sdk.operatorframework.io/uninstall-release"
6161
// Deprecated: use uninstallFinalizer. This will be removed in operator-sdk v2.0.0.
6262
uninstallFinalizerLegacy = "uninstall-helm-release"
63+
64+
helmUpgradeForceAnnotation = "helm.sdk.operatorframework.io/upgrade-force"
65+
helmUninstallWaitAnnotation = "helm.sdk.operatorframework.io/uninstall-wait"
6366
)
6467

6568
// Reconcile reconciles the requested resource by installing, updating, or
@@ -122,25 +125,58 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
122125
}
123126
status.RemoveCondition(types.ConditionReleaseFailed)
124127

128+
wait := hasAnnotation(helmUninstallWaitAnnotation, o)
125129
if errors.Is(err, driver.ErrReleaseNotFound) {
126-
log.Info("Release not found, removing finalizer")
130+
log.Info("Release not found")
127131
} else {
128132
log.Info("Uninstalled release")
129133
if log.V(0).Enabled() && uninstalledRelease != nil {
130134
fmt.Println(diff.Generate(uninstalledRelease.Manifest, ""))
131135
}
136+
if !wait {
137+
status.SetCondition(types.HelmAppCondition{
138+
Type: types.ConditionDeployed,
139+
Status: types.StatusFalse,
140+
Reason: types.ReasonUninstallSuccessful,
141+
})
142+
status.DeployedRelease = nil
143+
}
144+
}
145+
if wait {
132146
status.SetCondition(types.HelmAppCondition{
133-
Type: types.ConditionDeployed,
134-
Status: types.StatusFalse,
135-
Reason: types.ReasonUninstallSuccessful,
147+
Type: types.ConditionDeployed,
148+
Status: types.StatusFalse,
149+
Reason: types.ReasonUninstallSuccessful,
150+
Message: "Waiting until all resources are deleted.",
136151
})
137-
status.DeployedRelease = nil
138152
}
139153
if err := r.updateResourceStatus(ctx, o, status); err != nil {
140154
log.Info("Failed to update CR status")
141155
return reconcile.Result{}, err
142156
}
143157

158+
if wait && status.DeployedRelease != nil && status.DeployedRelease.Manifest != "" {
159+
log.Info("Uninstall wait")
160+
isAllResourcesDeleted, err := manager.CleanupRelease(ctx, status.DeployedRelease.Manifest)
161+
if err != nil {
162+
log.Error(err, "Failed to cleanup release")
163+
status.SetCondition(types.HelmAppCondition{
164+
Type: types.ConditionReleaseFailed,
165+
Status: types.StatusTrue,
166+
Reason: types.ReasonUninstallError,
167+
Message: err.Error(),
168+
})
169+
_ = r.updateResourceStatus(ctx, o, status)
170+
return reconcile.Result{}, err
171+
}
172+
if !isAllResourcesDeleted {
173+
log.Info("Waiting until all resources are deleted")
174+
return reconcile.Result{RequeueAfter: r.ReconcilePeriod}, nil
175+
}
176+
status.RemoveCondition(types.ConditionReleaseFailed)
177+
}
178+
179+
log.Info("Removing finalizer")
144180
controllerutil.RemoveFinalizer(o, uninstallFinalizer)
145181
controllerutil.RemoveFinalizer(o, uninstallFinalizerLegacy)
146182
if err := r.updateResource(ctx, o); err != nil {
@@ -254,7 +290,7 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
254290
r.EventRecorder.Eventf(o, "Warning", "OverrideValuesInUse",
255291
"Chart value %q overridden to %q by operator's watches.yaml", k, v)
256292
}
257-
force := hasHelmUpgradeForceAnnotation(o)
293+
force := hasAnnotation(helmUpgradeForceAnnotation, o)
258294
previousRelease, upgradedRelease, err := manager.UpgradeRelease(ctx, release.ForceUpgrade(force))
259295
if err != nil {
260296
log.Error(err, "Release failed")
@@ -357,16 +393,15 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
357393

358394
// returns the boolean representation of the annotation string
359395
// will return false if annotation is not set
360-
func hasHelmUpgradeForceAnnotation(o *unstructured.Unstructured) bool {
361-
const helmUpgradeForceAnnotation = "helm.sdk.operatorframework.io/upgrade-force"
362-
force := o.GetAnnotations()[helmUpgradeForceAnnotation]
363-
if force == "" {
396+
func hasAnnotation(anno string, o *unstructured.Unstructured) bool {
397+
boolStr := o.GetAnnotations()[anno]
398+
if boolStr == "" {
364399
return false
365400
}
366401
value := false
367-
if i, err := strconv.ParseBool(force); err != nil {
402+
if i, err := strconv.ParseBool(boolStr); err != nil {
368403
log.Info("Could not parse annotation as a boolean",
369-
"annotation", helmUpgradeForceAnnotation, "value informed", force)
404+
"annotation", anno, "value informed", boolStr)
370405
} else {
371406
value = i
372407
}

internal/helm/controller/reconcile_test.go

Lines changed: 64 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ import (
2121
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2222
)
2323

24-
func TestHasHelmUpgradeForceAnnotation(t *testing.T) {
25-
tests := []struct {
24+
func TestHasAnnotation(t *testing.T) {
25+
upgradeForceTests := []struct {
2626
input map[string]interface{}
2727
expectedVal bool
2828
expectedOut string
@@ -33,47 +33,101 @@ func TestHasHelmUpgradeForceAnnotation(t *testing.T) {
3333
"helm.sdk.operatorframework.io/upgrade-force": "True",
3434
},
3535
expectedVal: true,
36-
name: "base case true",
36+
name: "upgrade force base case true",
3737
},
3838
{
3939
input: map[string]interface{}{
4040
"helm.sdk.operatorframework.io/upgrade-force": "False",
4141
},
4242
expectedVal: false,
43-
name: "base case false",
43+
name: "upgrade force base case false",
4444
},
4545
{
4646
input: map[string]interface{}{
4747
"helm.sdk.operatorframework.io/upgrade-force": "1",
4848
},
4949
expectedVal: true,
50-
name: "true as int",
50+
name: "upgrade force true as int",
5151
},
5252
{
5353
input: map[string]interface{}{
5454
"helm.sdk.operatorframework.io/upgrade-force": "0",
5555
},
5656
expectedVal: false,
57-
name: "false as int",
57+
name: "upgrade force false as int",
5858
},
5959
{
6060
input: map[string]interface{}{
6161
"helm.sdk.operatorframework.io/wrong-annotation": "true",
6262
},
6363
expectedVal: false,
64-
name: "annotation not set",
64+
name: "upgrade force annotation not set",
6565
},
6666
{
6767
input: map[string]interface{}{
6868
"helm.sdk.operatorframework.io/upgrade-force": "invalid",
6969
},
7070
expectedVal: false,
71-
name: "invalid value",
71+
name: "upgrade force invalid value",
7272
},
7373
}
7474

75-
for _, test := range tests {
76-
assert.Equal(t, test.expectedVal, hasHelmUpgradeForceAnnotation(annotations(test.input)), test.name)
75+
for _, test := range upgradeForceTests {
76+
assert.Equal(t, test.expectedVal, hasAnnotation(helmUpgradeForceAnnotation, annotations(test.input)), test.name)
77+
}
78+
79+
uninstallWaitTests := []struct {
80+
input map[string]interface{}
81+
expectedVal bool
82+
expectedOut string
83+
name string
84+
}{
85+
{
86+
input: map[string]interface{}{
87+
"helm.sdk.operatorframework.io/uninstall-wait": "True",
88+
},
89+
expectedVal: true,
90+
name: "uninstall wait base case true",
91+
},
92+
{
93+
input: map[string]interface{}{
94+
"helm.sdk.operatorframework.io/uninstall-wait": "False",
95+
},
96+
expectedVal: false,
97+
name: "uninstall wait base case false",
98+
},
99+
{
100+
input: map[string]interface{}{
101+
"helm.sdk.operatorframework.io/uninstall-wait": "1",
102+
},
103+
expectedVal: true,
104+
name: "uninstall wait true as int",
105+
},
106+
{
107+
input: map[string]interface{}{
108+
"helm.sdk.operatorframework.io/uninstall-wait": "0",
109+
},
110+
expectedVal: false,
111+
name: "uninstall wait false as int",
112+
},
113+
{
114+
input: map[string]interface{}{
115+
"helm.sdk.operatorframework.io/wrong-annotation": "true",
116+
},
117+
expectedVal: false,
118+
name: "uninstall wait annotation not set",
119+
},
120+
{
121+
input: map[string]interface{}{
122+
"helm.sdk.operatorframework.io/uninstall-wait": "invalid",
123+
},
124+
expectedVal: false,
125+
name: "uninstall wait invalid value",
126+
},
127+
}
128+
129+
for _, test := range uninstallWaitTests {
130+
assert.Equal(t, test.expectedVal, hasAnnotation(helmUninstallWaitAnnotation, annotations(test.input)), test.name)
77131
}
78132
}
79133

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
Copyright 2021 The Operator-SDK Authors.
3+
Licensed under the Apache License, Version 2.0 (the "License");
4+
you may not use this file except in compliance with the License.
5+
You may obtain a copy of the License at
6+
http://www.apache.org/licenses/LICENSE-2.0
7+
Unless required by applicable law or agreed to in writing, software
8+
distributed under the License is distributed on an "AS IS" BASIS,
9+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
See the License for the specific language governing permissions and
11+
limitations under the License.
12+
*/
13+
14+
package manifestutil
15+
16+
import (
17+
"strings"
18+
19+
"helm.sh/helm/v3/pkg/kube"
20+
"helm.sh/helm/v3/pkg/releaseutil"
21+
)
22+
23+
// Source from https://github.com/helm/helm/blob/v3.4.2/pkg/action/resource_policy.go
24+
func FilterManifestsToKeep(manifests []releaseutil.Manifest) (keep, remaining []releaseutil.Manifest) {
25+
for _, m := range manifests {
26+
if m.Head.Metadata == nil || m.Head.Metadata.Annotations == nil || len(m.Head.Metadata.Annotations) == 0 {
27+
remaining = append(remaining, m)
28+
continue
29+
}
30+
31+
resourcePolicyType, ok := m.Head.Metadata.Annotations[kube.ResourcePolicyAnno]
32+
if !ok {
33+
remaining = append(remaining, m)
34+
continue
35+
}
36+
37+
resourcePolicyType = strings.ToLower(strings.TrimSpace(resourcePolicyType))
38+
if resourcePolicyType == kube.KeepPolicy {
39+
keep = append(keep, m)
40+
}
41+
42+
}
43+
return keep, remaining
44+
}

internal/helm/release/manager.go

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ import (
2626
"helm.sh/helm/v3/pkg/action"
2727
cpb "helm.sh/helm/v3/pkg/chart"
2828
"helm.sh/helm/v3/pkg/kube"
29-
helmkube "helm.sh/helm/v3/pkg/kube"
3029
rpb "helm.sh/helm/v3/pkg/release"
30+
"helm.sh/helm/v3/pkg/releaseutil"
3131
"helm.sh/helm/v3/pkg/storage"
3232
"helm.sh/helm/v3/pkg/storage/driver"
3333
apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -36,10 +36,13 @@ import (
3636
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3737
"k8s.io/apimachinery/pkg/runtime"
3838
apitypes "k8s.io/apimachinery/pkg/types"
39+
apiutilerrors "k8s.io/apimachinery/pkg/util/errors"
3940
"k8s.io/apimachinery/pkg/util/strategicpatch"
4041
"k8s.io/cli-runtime/pkg/resource"
42+
"k8s.io/client-go/discovery"
4143

4244
"github.com/operator-framework/operator-sdk/internal/helm/internal/types"
45+
"github.com/operator-framework/operator-sdk/internal/helm/manifestutil"
4346
)
4447

4548
// Manager manages a Helm release. It can install, upgrade, reconcile,
@@ -53,6 +56,7 @@ type Manager interface {
5356
UpgradeRelease(context.Context, ...UpgradeOption) (*rpb.Release, *rpb.Release, error)
5457
ReconcileRelease(context.Context) (*rpb.Release, error)
5558
UninstallRelease(context.Context, ...UninstallOption) (*rpb.Release, error)
59+
CleanupRelease(context.Context, string) (bool, error)
5660
}
5761

5862
type manager struct {
@@ -293,7 +297,7 @@ func createPatch(existing runtime.Object, expected *resource.Info) ([]byte, apit
293297
}
294298

295299
// Get a versioned object
296-
versionedObject := helmkube.AsVersioned(expected)
300+
versionedObject := kube.AsVersioned(expected)
297301

298302
// Unstructured objects, such as CRDs, may not have an not registered error
299303
// returned from ConvertToVersion. Anything that's unstructured should
@@ -363,3 +367,50 @@ func (m manager) UninstallRelease(ctx context.Context, opts ...UninstallOption)
363367
}
364368
return uninstallResponse.Release, err
365369
}
370+
371+
// CleanupRelease deletes resources if they are not deleted already.
372+
// Return true if all the resources are deleted, false otherwise.
373+
func (m manager) CleanupRelease(ctx context.Context, manifest string) (bool, error) {
374+
dc, err := m.actionConfig.RESTClientGetter.ToDiscoveryClient()
375+
if err != nil {
376+
return false, fmt.Errorf("failed to get Kubernetes discovery client: %w", err)
377+
}
378+
apiVersions, err := action.GetVersionSet(dc)
379+
if err != nil && !discovery.IsGroupDiscoveryFailedError(err) {
380+
return false, fmt.Errorf("failed to get apiVersions from Kubernetes: %w", err)
381+
}
382+
manifests := releaseutil.SplitManifests(manifest)
383+
_, files, err := releaseutil.SortManifests(manifests, apiVersions, releaseutil.UninstallOrder)
384+
if err != nil {
385+
return false, fmt.Errorf("failed to sort manifests: %w", err)
386+
}
387+
// do not delete resources that are annotated with the Helm resource policy 'keep'
388+
_, filesToDelete := manifestutil.FilterManifestsToKeep(files)
389+
var builder strings.Builder
390+
for _, file := range filesToDelete {
391+
builder.WriteString("\n---\n" + file.Content)
392+
}
393+
resources, err := m.kubeClient.Build(strings.NewReader(builder.String()), false)
394+
if err != nil {
395+
return false, fmt.Errorf("failed to build resources from manifests: %w", err)
396+
}
397+
if resources == nil || len(resources) <= 0 {
398+
return true, nil
399+
}
400+
for _, resource := range resources {
401+
err = resource.Get()
402+
if err != nil {
403+
if apierrors.IsNotFound(err) {
404+
continue // resource is already delete, check the next one.
405+
}
406+
return false, fmt.Errorf("failed to get resource: %w", err)
407+
}
408+
// found at least one resource that is not deleted so just delete everything again.
409+
_, errs := m.kubeClient.Delete(resources)
410+
if len(errs) > 0 {
411+
return false, fmt.Errorf("failed to delete resources: %v", apiutilerrors.NewAggregate(errs))
412+
}
413+
return false, nil
414+
}
415+
return true, nil
416+
}

0 commit comments

Comments
 (0)