Add annotation to prevent uninstalling helm chart of Job/App (#231)

Sometimes we want to remove App/Job CR but keep their helm chart
running, it is a kind of soft self-uninstalling)

  To handle this use-case, ketch-controller looks for
theketch.io/dont-uninstall-helm-chart=true annotation.
If it is present on an App/Job CR and kuberentes has marked the CR for deletion,
ketch controller doesn't uninstall its helm chart.

internal/api/v1beta1/annotations.go

@@ -0,0 +1,9 @@
+package v1beta1
+
+import "fmt"
+
+// DontUninstallHelmChartAnnotation returns an annotation that prevents
+// ketch-controller from uninstalling helm chart of Application or Job.
+func DontUninstallHelmChartAnnotation(group string) string {
+	return fmt.Sprintf("%s/dont-uninstall-helm-chart", group)
+}

internal/controllers/app_controller.go

@@ -681,14 +681,16 @@ func (r *AppReconciler) deleteChart(ctx context.Context, app *ketchv1.App) error
 			continue
 		}
 
-		helmClient, err := r.HelmFactoryFn(framework.Spec.NamespaceName)
-		if err != nil {
-			return err
-		}
-		err = helmClient.DeleteChart(app.Name)
-		if err != nil {
-			return err
+		if uninstallHelmChart(r.Group, app.Annotations) {
+			helmClient, err := r.HelmFactoryFn(framework.Spec.NamespaceName)
+			if err != nil {
+				return err
+			}
+			if err = helmClient.DeleteChart(app.Name); err != nil {
+				return err
+			}
 		}
+
 		patchedFramework := framework
 
 		patchedFramework.Status.Apps = make([]string, 0, len(patchedFramework.Status.Apps))

internal/controllers/app_controller_test.go

@@ -144,6 +144,22 @@ func TestAppReconciler_Reconcile(t *testing.T) {
 			},
 			wantConditionStatus: v1.ConditionTrue,
 		},
+		{
+			name: "running application, delete it but keep its helm chart",
+			app: ketchv1.App{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "app-running-with-dont-uninstall-annotation",
+					Annotations: map[string]string{
+						"theketch.io/dont-uninstall-helm-chart": "true",
+					},
+				},
+				Spec: ketchv1.AppSpec{
+					Deployments: []ketchv1.AppDeploymentSpec{},
+					Framework:   "working-framework",
+				},
+			},
+			wantConditionStatus: v1.ConditionTrue,
+		},
 		{
 			name: "create an app linked to a framework without available slots to run the app",
 			app: ketchv1.App{

internal/controllers/job_controller.go

@@ -196,13 +196,15 @@ func (r *JobReconciler) deleteChart(ctx context.Context, job *ketchv1.Job) error
 			continue
 		}
 
-		helmClient, err := r.HelmFactoryFn(framework.Spec.NamespaceName)
-		if err != nil {
-			return err
-		}
-		err = helmClient.DeleteChart(job.Name)
-		if err != nil {
-			return err
+		if uninstallHelmChart(ketchv1.Group, job.Annotations) {
+			helmClient, err := r.HelmFactoryFn(framework.Spec.NamespaceName)
+			if err != nil {
+				return err
+			}
+			err = helmClient.DeleteChart(job.Name)
+			if err != nil {
+				return err
+			}
 		}
 		patchedFramework := framework
 

internal/controllers/job_controller_test.go

@@ -56,32 +56,48 @@ func TestJobReconciler_Reconcile(t *testing.T) {
 		wantConditionMessage string
 	}{
 		{
-			name: "job linked to nonexisting framework",
+			name: "running job",
 			job: ketchv1.Job{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "framework-missing-job",
+					Name:      "test-job",
 					Namespace: "default",
 				},
 				Spec: ketchv1.JobSpec{
-					Framework: "non-existent-framework",
+					Framework: "working-framework",
 				},
 			},
-			wantConditionStatus:  v1.ConditionFalse,
-			wantConditionMessage: `framework "non-existent-framework" is not found`,
+			wantConditionStatus: v1.ConditionTrue,
 		},
 		{
-			name: "running job",
+			name: "running job, delete it but keep its helm chart",
 			job: ketchv1.Job{
 				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-job",
+					Name:      "test-job-2",
 					Namespace: "default",
+					Annotations: map[string]string{
+						"theketch.io/dont-uninstall-helm-chart": "true",
+					},
 				},
 				Spec: ketchv1.JobSpec{
 					Framework: "working-framework",
 				},
 			},
 			wantConditionStatus: v1.ConditionTrue,
 		},
+		{
+			name: "job linked to nonexisting framework",
+			job: ketchv1.Job{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "framework-missing-job",
+					Namespace: "default",
+				},
+				Spec: ketchv1.JobSpec{
+					Framework: "non-existent-framework",
+				},
+			},
+			wantConditionStatus:  v1.ConditionFalse,
+			wantConditionMessage: `framework "non-existent-framework" is not found`,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -100,6 +116,11 @@ func TestJobReconciler_Reconcile(t *testing.T) {
 			require.Equal(t, tt.wantConditionStatus, condition.Status)
 			require.Equal(t, tt.wantConditionMessage, condition.Message)
 			require.True(t, controllerutil.ContainsFinalizer(&resultJob, ketchv1.KetchFinalizer))
+			if condition.Status == v1.ConditionTrue {
+				err = ctx.k8sClient.Delete(context.Background(), &resultJob)
+				require.Nil(t, err)
+			}
 		})
 	}
+	require.Equal(t, []string{"test-job"}, helmMock.deleteChartCalled)
 }

internal/controllers/suite_test.go

@@ -97,6 +97,7 @@ func setup(reader templates.Reader, helm Helm, objects []client.Object) (*testin
 			return helm, nil
 		},
 		Recorder: k8sManager.GetEventRecorderFor("App"),
+		Group:    "theketch.io",
 	}).SetupWithManager(k8sManager)
 	if err != nil {
 		return nil, err

internal/controllers/utils.go

@@ -0,0 +1,24 @@
+package controllers
+
+import (
+	"strconv"
+
+	ketchv1 "github.com/theketchio/ketch/internal/api/v1beta1"
+)
+
+// uninstallHelmChart checks if there is a special annotation that
+// prevents ketch-controller from uninstalling helm chart of App/Job.
+func uninstallHelmChart(group string, annotations map[string]string) bool {
+	if len(annotations) == 0 {
+		return true
+	}
+	value, ok := annotations[ketchv1.DontUninstallHelmChartAnnotation(group)]
+	if !ok {
+		return true
+	}
+	keepChart, err := strconv.ParseBool(value)
+	if err != nil {
+		return true
+	}
+	return !keepChart
+}

internal/controllers/utils_test.go

@@ -0,0 +1,45 @@
+package controllers
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func Test_uninstallHelmChart(t *testing.T) {
+	tests := []struct {
+		name          string
+		group         string
+		annotations   map[string]string
+		wantUninstall bool
+	}{
+		{
+			name:  "dont uninstall",
+			group: "theketch.io",
+			annotations: map[string]string{
+				"theketch.io/dont-uninstall-helm-chart": "true",
+			},
+			wantUninstall: false,
+		},
+		{
+			name:  "uninstall",
+			group: "theketch.io",
+			annotations: map[string]string{
+				"theketch.io/dont-uninstall-helm-chart": "some-value",
+			},
+			wantUninstall: true,
+		},
+		{
+			name:          "no annotation - uninstall",
+			group:         "theketch.io",
+			annotations:   map[string]string{},
+			wantUninstall: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			uninstall := uninstallHelmChart(tt.group, tt.annotations)
+			require.Equal(t, tt.wantUninstall, uninstall)
+		})
+	}
+}