diff --git a/changelog/fragments/helm-uninstall-wait-anno.yml b/changelog/fragments/helm-uninstall-wait-anno.yml new file mode 100644 index 00000000000..19e9b8358a4 --- /dev/null +++ b/changelog/fragments/helm-uninstall-wait-anno.yml @@ -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 diff --git a/internal/helm/controller/reconcile.go b/internal/helm/controller/reconcile.go index 644b6f89e54..3840d1e0aa2 100644 --- a/internal/helm/controller/reconcile.go +++ b/internal/helm/controller/reconcile.go @@ -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 @@ -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") controllerutil.RemoveFinalizer(o, uninstallFinalizer) controllerutil.RemoveFinalizer(o, uninstallFinalizerLegacy) if err := r.updateResource(ctx, o); err != nil { @@ -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") @@ -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 } diff --git a/internal/helm/controller/reconcile_test.go b/internal/helm/controller/reconcile_test.go index 130b1d5f25b..188e936f406 100644 --- a/internal/helm/controller/reconcile_test.go +++ b/internal/helm/controller/reconcile_test.go @@ -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 @@ -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) } } diff --git a/internal/helm/manifestutil/resource_policy.go b/internal/helm/manifestutil/resource_policy.go new file mode 100644 index 00000000000..320d9f4d3a2 --- /dev/null +++ b/internal/helm/manifestutil/resource_policy.go @@ -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 +} diff --git a/internal/helm/release/manager.go b/internal/helm/release/manager.go index c2968e4ddf8..2d494f42c29 100644 --- a/internal/helm/release/manager.go +++ b/internal/helm/release/manager.go @@ -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" @@ -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, @@ -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 { @@ -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 @@ -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) { + 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 + } + return true, nil +} diff --git a/test/e2e/helm/cluster_test.go b/test/e2e/helm/cluster_test.go index 05f600875d6..96f61059629 100644 --- a/test/e2e/helm/cluster_test.go +++ b/test/e2e/helm/cluster_test.go @@ -277,17 +277,84 @@ var _ = Describe("Running Helm projects", func() { tc.Version) Eventually(getCurlLogs, time.Minute, time.Second).Should(ContainSubstring(metricExportedCR)) + By("annotate CR with uninstall-wait") + cmdOpts = []string{ + "annotate", tc.Kind, fmt.Sprintf("%s-sample", strings.ToLower(tc.Kind)), + "helm.sdk.operatorframework.io/uninstall-wait=true", + } + _, err = tc.Kubectl.Command(cmdOpts...) + Expect(err).NotTo(HaveOccurred()) + + By("adding a finalizer to statefulset") + cmdOpts = []string{ + "patch", "statefulset", releaseName, "-p", + "{\"metadata\":{\"finalizers\":[\"helm.sdk.operatorframework.io/fake-finalizer\"]}}", + "--type=merge", + } + _, err = tc.Kubectl.Command(cmdOpts...) + Expect(err).NotTo(HaveOccurred()) + By("deleting CR manifest") + _, err = tc.Kubectl.Delete(false, "-f", sampleFile, "--wait=false") + Expect(err).NotTo(HaveOccurred()) + + By("ensuring the CR gets reconciled and uninstall-wait is enabled") + managerContainerLogsAfterDeleteCR := func() string { + logOutput, err := tc.Kubectl.Logs(controllerPodName, "-c", "manager") + Expect(err).NotTo(HaveOccurred()) + return logOutput + } + Eventually(managerContainerLogsAfterDeleteCR, time.Minute, time.Second).Should(ContainSubstring("Uninstall wait")) + Eventually(managerContainerLogsAfterDeleteCR, time.Minute, time.Second).Should(ContainSubstring("Waiting until all resources are deleted")) + + By("removing the finalizer from statefulset") + cmdOpts = []string{ + "patch", "statefulset", releaseName, "-p", + "{\"metadata\":{\"finalizers\":[]}}", + "--type=merge", + } + _, err = tc.Kubectl.Command(cmdOpts...) + Expect(err).NotTo(HaveOccurred()) + + By("ensuring the CR gets reconciled and CR finalizer is removed") + Eventually(managerContainerLogsAfterDeleteCR, 2*time.Minute, time.Second).Should(ContainSubstring("Removing finalizer")) + + By("ensuring the CR is deleted") + verifyDeletedCR := func() error { + _, err := tc.Kubectl.Get( + true, + tc.Kind, fmt.Sprintf("%s-sample", strings.ToLower(tc.Kind))) + if err == nil { + return fmt.Errorf("the %s CR is not deleted", tc.Kind) + } + return nil + } + Eventually(verifyDeletedCR, time.Minute, time.Second).Should(Succeed()) + + By("creating an instance of release(CR) again after a delete with uninstall-wait") + _, err = tc.Kubectl.Apply(false, "-f", sampleFile) + Expect(err).NotTo(HaveOccurred()) + + By("ensuring the CR gets reconciled and the release was Installed again") + managerContainerLogs = func() string { + logOutput, err := tc.Kubectl.Logs(controllerPodName, "-c", "manager") + Expect(err).NotTo(HaveOccurred()) + return logOutput + } + Eventually(managerContainerLogs, time.Minute, time.Second).Should(ContainSubstring("Installed release")) + + By("deleting CR manifest again without uninstall-wait") _, err = tc.Kubectl.Delete(false, "-f", sampleFile) Expect(err).NotTo(HaveOccurred()) By("ensuring the CR gets reconciled and the release was Uninstalled") - managerContainerLogsAfterDeleteCR := func() string { + managerContainerLogsAfterDeleteCR = func() string { logOutput, err := tc.Kubectl.Logs(controllerPodName, "-c", "manager") Expect(err).NotTo(HaveOccurred()) return logOutput } Eventually(managerContainerLogsAfterDeleteCR, time.Minute, time.Second).Should(ContainSubstring("Uninstalled release")) + Eventually(managerContainerLogsAfterDeleteCR, time.Minute, time.Second).Should(ContainSubstring("Removing finalizer")) }) }) }) diff --git a/website/content/en/docs/building-operators/helm/reference/advanced_features/annotations.md b/website/content/en/docs/building-operators/helm/reference/advanced_features/annotations.md index 09445546b14..604bd059c1b 100644 --- a/website/content/en/docs/building-operators/helm/reference/advanced_features/annotations.md +++ b/website/content/en/docs/building-operators/helm/reference/advanced_features/annotations.md @@ -33,3 +33,115 @@ log message when an upgrade succeeds: ``` {"level":"info","ts":1591198931.1703992,"logger":"helm.controller","msg":"Upgraded release","namespace":"helm-nginx","name":"example-nginx","apiVersion":"cache.example.com/v1alpha1","kind":"Nginx","release":"example-nginx","force":true} ``` + +## `helm.sdk.operatorframework.io/uninstall-wait` + +This annotation can be set to `"true"` on custom resources to enable the deletion to wait until all the resources in the +`status.deployedRelease.manifest` are deleted. + +**Example** + +```yaml +apiVersion: example.com/v1alpha1 +kind: Nginx +metadata: + name: nginx-sample + annotations: + helm.sdk.operatorframework.io/uninstall-wait: "true" +spec: +... +status: + ... + deployedRelease: + manifest: | + --- + # Source: nginx/templates/serviceaccount.yaml + apiVersion: v1 + kind: ServiceAccount + metadata: + name: nginx-sample + labels: + helm.sh/chart: nginx-0.1.0 + app.kubernetes.io/name: nginx + app.kubernetes.io/instance: nginx-sample + app.kubernetes.io/version: "1.16.0" + app.kubernetes.io/managed-by: Helm + --- + # Source: nginx/templates/service.yaml + apiVersion: v1 + kind: Service + metadata: + name: nginx-sample + labels: + helm.sh/chart: nginx-0.1.0 + app.kubernetes.io/name: nginx + app.kubernetes.io/instance: nginx-sample + app.kubernetes.io/version: "1.16.0" + app.kubernetes.io/managed-by: Helm + spec: + type: ClusterIP + ports: + - port: 80 + targetPort: http + protocol: TCP + name: http + selector: + app.kubernetes.io/name: nginx + app.kubernetes.io/instance: nginx-sample + --- + # Source: nginx/templates/deployment.yaml + apiVersion: apps/v1 + kind: Deployment + metadata: + name: nginx-sample + labels: + helm.sh/chart: nginx-0.1.0 + app.kubernetes.io/name: nginx + app.kubernetes.io/instance: nginx-sample + app.kubernetes.io/version: "1.16.0" + app.kubernetes.io/managed-by: Helm + spec: + replicas: 1 + selector: + matchLabels: + app.kubernetes.io/name: nginx + app.kubernetes.io/instance: nginx-sample + template: + metadata: + labels: + app.kubernetes.io/name: nginx + app.kubernetes.io/instance: nginx-sample + spec: + serviceAccountName: nginx-sample + securityContext: + {} + containers: + - name: nginx + securityContext: + {} + image: "nginx:1.16.0" + imagePullPolicy: IfNotPresent + ports: + - name: http + containerPort: 80 + protocol: TCP + livenessProbe: + httpGet: + path: / + port: http + readinessProbe: + httpGet: + path: / + port: http + resources: + {} +``` + +Setting this annotation to `true` and deleting the custom resource will cause the custom resource to be reconciled +continuously until all the resources in `status.deployedRelease.manifest` are deleted. This can be verified in the +log message when a delete has been triggered: + +``` +{"level":"info","ts":1612294054.5845876,"logger":"helm.controller","msg":"Uninstall wait","namespace":"default","name":"nginx-sample","apiVersion":"example.com/v1alpha1","kind":"Nginx","release":"nginx-sample"} + +```