From b955951e59a8e509ae832b5e3f2bd3f1cf6ec450 Mon Sep 17 00:00:00 2001 From: JoaoBraveCoding Date: Tue, 6 Apr 2021 21:41:59 +0200 Subject: [PATCH] feat: Added annotation to skip resource deletion when app is deleted #5945 Signed-off-by: JoaoBraveCoding --- common/common.go | 2 + controller/appcontroller.go | 3 +- controller/appcontroller_test.go | 79 ++++++++++++++++++++++++++++++++ docs/user-guide/app_deletion.md | 18 ++++++-- test/e2e/app_management_test.go | 28 +++++++++++ 5 files changed, 126 insertions(+), 4 deletions(-) diff --git a/common/common.go b/common/common.go index 260a18f6cbe3d..d3f5cfa4b5708 100644 --- a/common/common.go +++ b/common/common.go @@ -119,6 +119,8 @@ const ( // AnnotationCompareOptions is a comma-separated list of options for comparison AnnotationCompareOptions = "argocd.argoproj.io/compare-options" + // AnnotationDeleteOptions is a comma-separated list of options for deletion of resources + AnnotationDeleteOptions = "argocd.argoproj.io/delete-options" // AnnotationKeyRefresh is the annotation key which indicates that app needs to be refreshed. Removed by application controller after app is refreshed. // Might take values 'normal'/'hard'. Value 'hard' means manifest cache and target cluster state cache should be invalidated before refresh. diff --git a/controller/appcontroller.go b/controller/appcontroller.go index 91823a78dde55..e1e57be54d4cc 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -18,6 +18,7 @@ import ( "github.com/argoproj/gitops-engine/pkg/diff" "github.com/argoproj/gitops-engine/pkg/health" synccommon "github.com/argoproj/gitops-engine/pkg/sync/common" + resourceutil "github.com/argoproj/gitops-engine/pkg/sync/resource" "github.com/argoproj/gitops-engine/pkg/utils/kube" jsonpatch "github.com/evanphx/json-patch" log "github.com/sirupsen/logrus" @@ -777,7 +778,7 @@ func (ctrl *ApplicationController) removeProjectFinalizer(proj *appv1.AppProject // shouldBeDeleted returns whether a given resource obj should be deleted on cascade delete of application app func (ctrl *ApplicationController) shouldBeDeleted(app *appv1.Application, obj *unstructured.Unstructured) bool { - return !kube.IsCRD(obj) && !isSelfReferencedApp(app, kube.GetObjectRef(obj)) + return !kube.IsCRD(obj) && !isSelfReferencedApp(app, kube.GetObjectRef(obj)) && !resourceutil.HasAnnotationOption(obj, common.AnnotationDeleteOptions, "Skip") } func (ctrl *ApplicationController) getPermittedAppLiveObjects(app *appv1.Application, proj *appv1.AppProject) (map[kube.ResourceKey]*unstructured.Unstructured, error) { diff --git a/controller/appcontroller_test.go b/controller/appcontroller_test.go index d7787879201a3..524bacd4a7eac 100644 --- a/controller/appcontroller_test.go +++ b/controller/appcontroller_test.go @@ -252,6 +252,19 @@ metadata: data: ` +var fakeResourceWithSkipAnnotation = ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-cm + namespace: ` + test.FakeArgoCDNamespace + ` + annotations: + argocd.argoproj.io/delete-options: Skip + labels: + app.kubernetes.io/instance: my-app +data: +` + func newFakeApp() *argoappv1.Application { return createFakeApp(fakeApp) } @@ -282,6 +295,15 @@ func newFakeCM() map[string]interface{} { return cm } +func newFakeCMWithSkipAnnotation() map[string]interface{} { + var cm map[string]interface{} + err := yaml.Unmarshal([]byte(fakeResourceWithSkipAnnotation), &cm) + if err != nil { + panic(err) + } + return cm +} + func TestAutoSync(t *testing.T) { app := newFakeApp() ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}) @@ -610,6 +632,63 @@ func TestFinalizeAppDeletion(t *testing.T) { } }) + // Ensure that any project with Skip deletion annotation option are skiped during app deletion + t.Run("ResourcesWithSkipAnnotationAreSkiped", func(*testing.T) { + restrictedProj := argoappv1.AppProject{ + ObjectMeta: metav1.ObjectMeta{ + Name: "restricted", + Namespace: test.FakeArgoCDNamespace, + }, + Spec: argoappv1.AppProjectSpec{ + SourceRepos: []string{"*"}, + Destinations: []argoappv1.ApplicationDestination{ + { + Server: "*", + Namespace: "my-app", + }, + }, + }, + } + app := newFakeApp() + app.Spec.Destination.Namespace = test.FakeArgoCDNamespace + app.Spec.Project = "restricted" + appObj := kube.MustToUnstructured(&app) + cm := newFakeCMWithSkipAnnotation() + objToBeSkipped := kube.MustToUnstructured(&cm) + ctrl := newFakeController(&fakeData{ + apps: []runtime.Object{app, &defaultProj, &restrictedProj}, + managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{ + kube.GetResourceKey(appObj): appObj, + kube.GetResourceKey(objToBeSkipped): objToBeSkipped, + }, + }) + + patched := false + fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) + defaultReactor := fakeAppCs.ReactionChain[0] + fakeAppCs.ReactionChain = nil + fakeAppCs.AddReactor("get", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { + return defaultReactor.React(action) + }) + fakeAppCs.AddReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { + patched = true + return true, nil, nil + }) + objs, err := ctrl.finalizeApplicationDeletion(app) + assert.NoError(t, err) + assert.True(t, patched) + objsMap, err := ctrl.stateCache.GetManagedLiveObjs(app, []*unstructured.Unstructured{}) + if err != nil { + assert.NoError(t, err) + } + // Managed objects must be empty + assert.Empty(t, objsMap) + // Loop through all deleted objects, ensure that test-cm is none of them + for _, o := range objs { + assert.NotEqual(t, "test-cm", o.GetName()) + } + }) + t.Run("DeleteWithDestinationClusterName", func(t *testing.T) { app := newFakeAppWithDestName() appObj := kube.MustToUnstructured(&app) diff --git a/docs/user-guide/app_deletion.md b/docs/user-guide/app_deletion.md index d50853e2ed4ad..b0f753b482a2c 100644 --- a/docs/user-guide/app_deletion.md +++ b/docs/user-guide/app_deletion.md @@ -1,6 +1,6 @@ # App Deletion -Apps can be deleted with or without a cascade option. A **cascade delete**, deletes both the app and its resources, rather than only the app. +Apps can be deleted with or without a cascade option. A **cascade delete**, deletes both the app and its resources, rather than only the app. ## Deletion Using `argocd` @@ -39,7 +39,7 @@ kubectl delete app APPNAME # About The Deletion Finalizer -For the technical amongst you, the Argo CD application controller watches for this finalizer: +For the technical amongst you, the Argo CD application controller watches for this finalizer: ```yaml metadata: @@ -49,4 +49,16 @@ metadata: Argo CD's app controller watches for this and will then delete both the app and its resources. -When you invoke `argocd app delete` with `--cascade`, the finalizer is added automatically. +When you invoke `argocd app delete` with `--cascade`, the finalizer is added automatically. + +# Skiping resources during deletion + +By adding the following annotation to resources owned by an application: + +```yaml +metadata: + annotations: + argocd.argoproj.io/delete-options: Skip +``` + +It's possible to skip the deletion of such resources even when cascade deletion is enabled. \ No newline at end of file diff --git a/test/e2e/app_management_test.go b/test/e2e/app_management_test.go index 1aaa39f97e054..7c2d97a74242e 100644 --- a/test/e2e/app_management_test.go +++ b/test/e2e/app_management_test.go @@ -218,6 +218,34 @@ func TestAppDeletion(t *testing.T) { assert.NotContains(t, output, Name()) } +// TestDeleteOptionSkip test that resources with the annotation argocd.argoproj.io/delete-options: Skip +// are not deleted when the parrent application is deleted and cascade is true +func TestDeleteOptionSkip(t *testing.T) { + Given(t). + Path("two-nice-pods"). + When(). + PatchFile("pod-1.yaml", `[{"op": "add", "path": "/metadata/annotations", "value": {"argocd.argoproj.io/delete-options": "Skip"}}]`). + Create(). + Sync(). + Then(). + Expect(OperationPhaseIs(OperationSucceeded)). + Expect(SyncStatusIs(SyncStatusCodeSynced)). + When(). + Delete(true). + Then(). + Expect(DoesNotExist()). + Expect(Event(EventReasonResourceDeleted, "delete")). + When(). + And(func() { + _, err := KubeClientset.CoreV1().Pods(DeploymentNamespace()).Get(context.Background(), "pod-1", metav1.GetOptions{}) + assert.NoError(t, err) + }) + + output, err := RunCli("app", "list") + assert.NoError(t, err) + assert.NotContains(t, output, Name()) +} + func TestAppLabels(t *testing.T) { Given(t). Path("config-map").