Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enh: (helm) - add annotation to wait for all resources to be deleted before removing CR finalizer #4487

Merged
merged 3 commits into from
Apr 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog/fragments/helm-uninstall-wait-anno.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
entries:
- description: >
For Helm based-operators, added annotation `helm.sdk.operatorframework.io/uninstall-wait: "true"`
to allow all resources to be deleted before removing the custom resource's finalizer.
kind: "addition"
breaking: false
59 changes: 47 additions & 12 deletions internal/helm/controller/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ const (
uninstallFinalizer = "helm.sdk.operatorframework.io/uninstall-release"
// Deprecated: use uninstallFinalizer. This will be removed in operator-sdk v2.0.0.
uninstallFinalizerLegacy = "uninstall-helm-release"

helmUpgradeForceAnnotation = "helm.sdk.operatorframework.io/upgrade-force"
helmUninstallWaitAnnotation = "helm.sdk.operatorframework.io/uninstall-wait"
)

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

wait := hasAnnotation(helmUninstallWaitAnnotation, o)
if errors.Is(err, driver.ErrReleaseNotFound) {
log.Info("Release not found, removing finalizer")
log.Info("Release not found")
} else {
log.Info("Uninstalled release")
if log.V(0).Enabled() && uninstalledRelease != nil {
fmt.Println(diff.Generate(uninstalledRelease.Manifest, ""))
}
if !wait {
status.SetCondition(types.HelmAppCondition{
Type: types.ConditionDeployed,
Status: types.StatusFalse,
Reason: types.ReasonUninstallSuccessful,
})
status.DeployedRelease = nil
}
}
if wait {
status.SetCondition(types.HelmAppCondition{
Type: types.ConditionDeployed,
Status: types.StatusFalse,
Reason: types.ReasonUninstallSuccessful,
Type: types.ConditionDeployed,
Status: types.StatusFalse,
Reason: types.ReasonUninstallSuccessful,
Message: "Waiting until all resources are deleted.",
})
status.DeployedRelease = nil
}
if err := r.updateResourceStatus(ctx, o, status); err != nil {
log.Info("Failed to update CR status")
return reconcile.Result{}, err
}

if wait && status.DeployedRelease != nil && status.DeployedRelease.Manifest != "" {
log.Info("Uninstall wait")
isAllResourcesDeleted, err := manager.CleanupRelease(ctx, status.DeployedRelease.Manifest)
if err != nil {
log.Error(err, "Failed to cleanup release")
status.SetCondition(types.HelmAppCondition{
Type: types.ConditionReleaseFailed,
Status: types.StatusTrue,
Reason: types.ReasonUninstallError,
Message: err.Error(),
})
_ = r.updateResourceStatus(ctx, o, status)
return reconcile.Result{}, err
}
if !isAllResourcesDeleted {
log.Info("Waiting until all resources are deleted")
return reconcile.Result{RequeueAfter: r.ReconcilePeriod}, nil
}
status.RemoveCondition(types.ConditionReleaseFailed)
}

log.Info("Removing finalizer")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

controllerutil.RemoveFinalizer(o, uninstallFinalizer)
controllerutil.RemoveFinalizer(o, uninstallFinalizerLegacy)
if err := r.updateResource(ctx, o); err != nil {
Expand Down Expand Up @@ -254,7 +290,7 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
r.EventRecorder.Eventf(o, "Warning", "OverrideValuesInUse",
"Chart value %q overridden to %q by operator's watches.yaml", k, v)
}
force := hasHelmUpgradeForceAnnotation(o)
force := hasAnnotation(helmUpgradeForceAnnotation, o)
previousRelease, upgradedRelease, err := manager.UpgradeRelease(ctx, release.ForceUpgrade(force))
if err != nil {
log.Error(err, "Release failed")
Expand Down Expand Up @@ -357,16 +393,15 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile

// returns the boolean representation of the annotation string
// will return false if annotation is not set
func hasHelmUpgradeForceAnnotation(o *unstructured.Unstructured) bool {
const helmUpgradeForceAnnotation = "helm.sdk.operatorframework.io/upgrade-force"
force := o.GetAnnotations()[helmUpgradeForceAnnotation]
if force == "" {
func hasAnnotation(anno string, o *unstructured.Unstructured) bool {
boolStr := o.GetAnnotations()[anno]
if boolStr == "" {
return false
}
value := false
if i, err := strconv.ParseBool(force); err != nil {
if i, err := strconv.ParseBool(boolStr); err != nil {
log.Info("Could not parse annotation as a boolean",
"annotation", helmUpgradeForceAnnotation, "value informed", force)
"annotation", anno, "value informed", boolStr)
} else {
value = i
}
Expand Down
74 changes: 64 additions & 10 deletions internal/helm/controller/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

func TestHasHelmUpgradeForceAnnotation(t *testing.T) {
tests := []struct {
func TestHasAnnotation(t *testing.T) {
upgradeForceTests := []struct {
input map[string]interface{}
expectedVal bool
expectedOut string
Expand All @@ -33,47 +33,101 @@ func TestHasHelmUpgradeForceAnnotation(t *testing.T) {
"helm.sdk.operatorframework.io/upgrade-force": "True",
},
expectedVal: true,
name: "base case true",
name: "upgrade force base case true",
},
{
input: map[string]interface{}{
"helm.sdk.operatorframework.io/upgrade-force": "False",
},
expectedVal: false,
name: "base case false",
name: "upgrade force base case false",
},
{
input: map[string]interface{}{
"helm.sdk.operatorframework.io/upgrade-force": "1",
},
expectedVal: true,
name: "true as int",
name: "upgrade force true as int",
},
{
input: map[string]interface{}{
"helm.sdk.operatorframework.io/upgrade-force": "0",
},
expectedVal: false,
name: "false as int",
name: "upgrade force false as int",
},
{
input: map[string]interface{}{
"helm.sdk.operatorframework.io/wrong-annotation": "true",
},
expectedVal: false,
name: "annotation not set",
name: "upgrade force annotation not set",
},
{
input: map[string]interface{}{
"helm.sdk.operatorframework.io/upgrade-force": "invalid",
},
expectedVal: false,
name: "invalid value",
name: "upgrade force invalid value",
},
}

for _, test := range tests {
assert.Equal(t, test.expectedVal, hasHelmUpgradeForceAnnotation(annotations(test.input)), test.name)
for _, test := range upgradeForceTests {
assert.Equal(t, test.expectedVal, hasAnnotation(helmUpgradeForceAnnotation, annotations(test.input)), test.name)
}

uninstallWaitTests := []struct {
input map[string]interface{}
expectedVal bool
expectedOut string
name string
}{
{
input: map[string]interface{}{
"helm.sdk.operatorframework.io/uninstall-wait": "True",
},
expectedVal: true,
name: "uninstall wait base case true",
},
{
input: map[string]interface{}{
"helm.sdk.operatorframework.io/uninstall-wait": "False",
},
expectedVal: false,
name: "uninstall wait base case false",
},
{
input: map[string]interface{}{
"helm.sdk.operatorframework.io/uninstall-wait": "1",
},
expectedVal: true,
name: "uninstall wait true as int",
},
{
input: map[string]interface{}{
"helm.sdk.operatorframework.io/uninstall-wait": "0",
},
expectedVal: false,
name: "uninstall wait false as int",
},
{
input: map[string]interface{}{
"helm.sdk.operatorframework.io/wrong-annotation": "true",
},
expectedVal: false,
name: "uninstall wait annotation not set",
},
{
input: map[string]interface{}{
"helm.sdk.operatorframework.io/uninstall-wait": "invalid",
},
expectedVal: false,
name: "uninstall wait invalid value",
},
}

for _, test := range uninstallWaitTests {
assert.Equal(t, test.expectedVal, hasAnnotation(helmUninstallWaitAnnotation, annotations(test.input)), test.name)
}
}

Expand Down
44 changes: 44 additions & 0 deletions internal/helm/manifestutil/resource_policy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
Copyright 2021 The Operator-SDK Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package manifestutil

import (
"strings"

"helm.sh/helm/v3/pkg/kube"
"helm.sh/helm/v3/pkg/releaseutil"
)

// Source from https://github.com/helm/helm/blob/v3.4.2/pkg/action/resource_policy.go
func FilterManifestsToKeep(manifests []releaseutil.Manifest) (keep, remaining []releaseutil.Manifest) {
for _, m := range manifests {
if m.Head.Metadata == nil || m.Head.Metadata.Annotations == nil || len(m.Head.Metadata.Annotations) == 0 {
remaining = append(remaining, m)
continue
}

resourcePolicyType, ok := m.Head.Metadata.Annotations[kube.ResourcePolicyAnno]
if !ok {
remaining = append(remaining, m)
continue
}

resourcePolicyType = strings.ToLower(strings.TrimSpace(resourcePolicyType))
if resourcePolicyType == kube.KeepPolicy {
keep = append(keep, m)
}

}
return keep, remaining
}
55 changes: 53 additions & 2 deletions internal/helm/release/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import (
"helm.sh/helm/v3/pkg/action"
cpb "helm.sh/helm/v3/pkg/chart"
"helm.sh/helm/v3/pkg/kube"
helmkube "helm.sh/helm/v3/pkg/kube"
rpb "helm.sh/helm/v3/pkg/release"
"helm.sh/helm/v3/pkg/releaseutil"
"helm.sh/helm/v3/pkg/storage"
"helm.sh/helm/v3/pkg/storage/driver"
apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
Expand All @@ -36,10 +36,13 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
apitypes "k8s.io/apimachinery/pkg/types"
apiutilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/strategicpatch"
"k8s.io/cli-runtime/pkg/resource"
"k8s.io/client-go/discovery"

"github.com/operator-framework/operator-sdk/internal/helm/internal/types"
"github.com/operator-framework/operator-sdk/internal/helm/manifestutil"
)

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

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

// Get a versioned object
versionedObject := helmkube.AsVersioned(expected)
versionedObject := kube.AsVersioned(expected)

// Unstructured objects, such as CRDs, may not have an not registered error
// returned from ConvertToVersion. Anything that's unstructured should
Expand Down Expand Up @@ -363,3 +367,50 @@ func (m manager) UninstallRelease(ctx context.Context, opts ...UninstallOption)
}
return uninstallResponse.Release, err
}

// CleanupRelease deletes resources if they are not deleted already.
// Return true if all the resources are deleted, false otherwise.
func (m manager) CleanupRelease(ctx context.Context, manifest string) (bool, error) {
dc, err := m.actionConfig.RESTClientGetter.ToDiscoveryClient()
if err != nil {
return false, fmt.Errorf("failed to get Kubernetes discovery client: %w", err)
}
apiVersions, err := action.GetVersionSet(dc)
if err != nil && !discovery.IsGroupDiscoveryFailedError(err) {
estroz marked this conversation as resolved.
Show resolved Hide resolved
return false, fmt.Errorf("failed to get apiVersions from Kubernetes: %w", err)
}
manifests := releaseutil.SplitManifests(manifest)
_, files, err := releaseutil.SortManifests(manifests, apiVersions, releaseutil.UninstallOrder)
if err != nil {
return false, fmt.Errorf("failed to sort manifests: %w", err)
}
// do not delete resources that are annotated with the Helm resource policy 'keep'
_, filesToDelete := manifestutil.FilterManifestsToKeep(files)
var builder strings.Builder
for _, file := range filesToDelete {
builder.WriteString("\n---\n" + file.Content)
}
resources, err := m.kubeClient.Build(strings.NewReader(builder.String()), false)
if err != nil {
return false, fmt.Errorf("failed to build resources from manifests: %w", err)
}
if resources == nil || len(resources) <= 0 {
return true, nil
}
for _, resource := range resources {
err = resource.Get()
if err != nil {
if apierrors.IsNotFound(err) {
continue // resource is already delete, check the next one.
}
return false, fmt.Errorf("failed to get resource: %w", err)
}
// found at least one resource that is not deleted so just delete everything again.
_, errs := m.kubeClient.Delete(resources)
if len(errs) > 0 {
return false, fmt.Errorf("failed to delete resources: %v", apiutilerrors.NewAggregate(errs))
}
return false, nil
estroz marked this conversation as resolved.
Show resolved Hide resolved
}
return true, nil
}
Loading