diff --git a/docs/operator-manual/application.yaml b/docs/operator-manual/application.yaml index c693a581a45ec..fbe943b9baa84 100644 --- a/docs/operator-manual/application.yaml +++ b/docs/operator-manual/application.yaml @@ -155,6 +155,7 @@ spec: syncOptions: # Sync options which modifies sync behavior - Validate=false # disables resource validation (equivalent to 'kubectl apply --validate=false') ( true by default ). - CreateNamespace=true # Namespace Auto-Creation ensures that namespace specified as the application destination exists in the destination cluster. + - DeleteNamespace=true # Delete Auto-Created namespace. Ignored when CreateNamespace=true is not set. - PrunePropagationPolicy=foreground # Supported policies are background, foreground and orphan. - PruneLast=true # Allow the ability for resource pruning to happen as a final, implicit wave of a sync operation managedNamespaceMetadata: # Sets the metadata for the application namespace. Only valid if CreateNamespace=true (see above), otherwise it's a no-op. diff --git a/docs/user-guide/sync-options.md b/docs/user-guide/sync-options.md index 098291e80d28b..64015ee560da4 100644 --- a/docs/user-guide/sync-options.md +++ b/docs/user-guide/sync-options.md @@ -280,6 +280,13 @@ The example above shows how an Argo CD Application can be configured so it will Note that the namespace to be created must be informed in the `spec.destination.namespace` field of the Application resource. The `metadata.namespace` field in the Application's child manifests must match this value, or can be omitted, so resources are created in the proper destination. +### Delete Namespace + +If `DeleteNamespace=true` is set, we can delete an auto-created namespace when an application is deleted. +Note that the namespace is deleted only when both `CreateNamespace=true` and `DeleteNamespace=true` are set. +In addition, the namespace must have a label or an annotation to indicate the application tracks the namespace [see [resource tracking](resource_tracking.md)]. +Otherwise, the application is deleted, but the namespace remains throwing error. + ### Namespace Metadata We can also add labels and annotations to the namespace through `managedNamespaceMetadata`. If we extend the example above diff --git a/server/application/application.go b/server/application/application.go index 090ce5f8219b1..1af01d20c1863 100644 --- a/server/application/application.go +++ b/server/application/application.go @@ -1000,6 +1000,19 @@ func (s *Server) Delete(ctx context.Context, q *application.ApplicationDeleteReq return nil, fmt.Errorf("error deleting application: %w", err) } s.logAppEvent(a, ctx, argo.EventReasonResourceDeleted, "deleted application") + + // Delete a namespace when both CreateNamespace and DeleteNamespace are set to true. + if a.Spec.SyncPolicy != nil { + syncOptions := a.Spec.SyncPolicy.SyncOptions + if syncOptions.HasOption("CreateNamespace=true") && syncOptions.HasOption("DeleteNamespace=true") { + err := s.deleteNamespace(ctx, a.Name, a.Spec.Destination.Namespace, metav1.DeleteOptions{}) + if err != nil { + return nil, fmt.Errorf("error deleting namespace: %w", err) + } + s.logAppEvent(a, ctx, argo.EventReasonResourceDeleted, fmt.Sprintf("deleted namespace: %s", a.Spec.Destination.Namespace)) + } + } + return &application.ApplicationResponse{}, nil } @@ -2353,3 +2366,41 @@ func (s *Server) appNamespaceOrDefault(appNs string) string { func (s *Server) isNamespaceEnabled(namespace string) bool { return security.IsNamespaceEnabled(namespace, s.ns, s.enabledNamespaces) } + +// The namespace can only be deleted when the namespace has an appropriate label or annotation which shows the namespace is tracked by the application. +func (s *Server) deleteNamespace(ctx context.Context, appName, namespace string, opts metav1.DeleteOptions) error { + canDelete, err := s.isNamespaceTrackedByApplication(ctx, appName, namespace, metav1.GetOptions{}) + if err != nil { + return err + } + + if canDelete { + err := s.kubeclientset.CoreV1().Namespaces().Delete(ctx, namespace, metav1.DeleteOptions{}) + return err + } else { + return fmt.Errorf("error cannot delete namespace %s which is not managed by %s", namespace, appName) + } +} + +func (s *Server) isNamespaceTrackedByApplication(ctx context.Context, appName, namespace string, opts metav1.GetOptions) (bool, error) { + appLabelKey, err := s.settingsMgr.GetAppInstanceLabelKey() + if err != nil { + log.Errorf("Could not get appInstanceLabelKey: %v", err) + return false, err + } + + ns, err := s.kubeclientset.CoreV1().Namespaces().Get(ctx, namespace, opts) + if err != nil { + return false, err + } + + unstructedNs, err := kube.ToUnstructured(ns) + if err != nil { + return false, err + } + + trackingMethod := argo.GetTrackingMethod(s.settingsMgr) + nsOwner := argo.NewResourceTracking().GetAppName(unstructedNs, appLabelKey, trackingMethod) + + return nsOwner == appName, nil +} diff --git a/server/application/application_test.go b/server/application/application_test.go index 7569734d33b42..0f85b9085f0b3 100644 --- a/server/application/application_test.go +++ b/server/application/application_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes/fake" + k8sfake "k8s.io/client-go/kubernetes/fake" kubetesting "k8s.io/client-go/testing" k8scache "k8s.io/client-go/tools/cache" "k8s.io/utils/pointer" @@ -277,6 +278,32 @@ spec: server: https://cluster-api.com ` +const fakeAppWithDeleteNamespace = ` +apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + name: test-app + namespace: default +spec: + source: + path: some/path + repoURL: https://github.com/argoproj/argocd-example-apps.git + targetRevision: HEAD + ksonnet: + environment: default + destination: + namespace: ` + test.FakeDestNamespace + ` + name: fake-cluster + syncPolicy: + syncOptions: # Sync options which modifies sync behavior + - CreateNamespace=true + - DeleteNamespace=true +` + +func newTestAppWithDeleteNamespace(opts ...func(app *appsv1.Application)) *appsv1.Application { + return createTestApp(fakeAppWithDeleteNamespace, opts...) +} + func newTestAppWithDestName(opts ...func(app *appsv1.Application)) *appsv1.Application { return createTestApp(fakeAppWithDestName, opts...) } @@ -615,6 +642,87 @@ func TestDeleteApp_InvalidName(t *testing.T) { assert.True(t, apierrors.IsNotFound(err)) } +func TestDeleteApp_DeleteNamespace(t *testing.T) { + ctx := context.Background() + appServer := newTestAppServer() + createReq := application.ApplicationCreateRequest{ + Application: newTestAppWithDeleteNamespace(), + } + appLabelKey, err := appServer.settingsMgr.GetAppInstanceLabelKey() + assert.Nil(t, err) + + app, err := appServer.Create(ctx, &createReq) + assert.Nil(t, err) + ns := v1.Namespace{} + + // Flag whether Delete is called + appDeleted := false + nsDeleted := false + revertValues := func() { + appDeleted = false + nsDeleted = false + } + + // Fake Application response + fakeAppCs := appServer.appclientset.(*apps.Clientset) + // this removes the default */* reactor so we can set our own patch/delete reactor + fakeAppCs.ReactionChain = nil + fakeAppCs.AddReactor("delete", "applications", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { + appDeleted = true + return true, nil, nil + }) + fakeAppCs.AddReactor("get", "applications", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { + return true, app, nil + }) + appServer.appclientset = fakeAppCs + + // Fake Namespace response + fakeKubeCs := appServer.kubeclientset.(*k8sfake.Clientset) + // this removes the default */* reactor so we can set our own patch/delete reactor + fakeKubeCs.ReactionChain = nil + fakeKubeCs.AddReactor("get", "namespaces", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { + return true, &ns, nil + }) + fakeKubeCs.AddReactor("delete", "namespaces", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { + nsDeleted = true + return true, nil, nil + }) + appServer.kubeclientset = fakeKubeCs + + t.Run("Success to delete namespace", func(t *testing.T) { + ns = v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: test.FakeDestNamespace, + Labels: map[string]string{ + appLabelKey: app.Name, + }, + }, + } + _, err = appServer.Delete(ctx, &application.ApplicationDeleteRequest{Name: &app.Name}) + assert.Nil(t, err) + assert.True(t, appDeleted) + assert.True(t, nsDeleted) + t.Cleanup(revertValues) + }) + + t.Run("Fail to delete namespace because of invalid label", func(t *testing.T) { + ns = v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: test.FakeDestNamespace, + Labels: map[string]string{ + appLabelKey: "this-is-invalid", + }, + }, + } + _, err = appServer.Delete(ctx, &application.ApplicationDeleteRequest{Name: &app.Name}) + assert.NotNil(t, err) + assert.Equal(t, fmt.Sprintf("error deleting namespace: error cannot delete namespace %s which is not managed by test-app", test.FakeDestNamespace), err.Error()) + assert.True(t, appDeleted) + assert.False(t, nsDeleted) + t.Cleanup(revertValues) + }) +} + func TestSyncAndTerminate(t *testing.T) { ctx := context.Background() appServer := newTestAppServer() diff --git a/test/e2e/app_management_test.go b/test/e2e/app_management_test.go index 338a6ce0bc590..6cdb4faae5a35 100644 --- a/test/e2e/app_management_test.go +++ b/test/e2e/app_management_test.go @@ -2459,3 +2459,122 @@ func TestAnnotationTrackingExtraResources(t *testing.T) { Expect(HealthIs(health.HealthStatusHealthy)) } + +// Given +// - application is set with --sync-option CreateNamespace=true --sync-option DeleteNamespace=true +// - application --dest-namespace does not exist +// +// Verity +// - application --dest-namespace is created +// - application sync successful +// - when application is deleted, --dest-namespace is deleted +func TestAppDeletionDeleteNamespace(t *testing.T) { + appName := "test-app-deletion-delete-namespace" + SkipOnEnv(t, "OPENSHIFT") + createdNamespace := getNewNamespace(t) + Given(t). + Timeout(30). + Path("guestbook"). + When(). + CreateFromFile(func(app *Application) { + app.Spec.SyncPolicy = &SyncPolicy{ + SyncOptions: SyncOptions{"CreateNamespace=true", "DeleteNamespace=true", "PruneLast=true"}, + ManagedNamespaceMetadata: &ManagedNamespaceMetadata{ + Labels: map[string]string{"app.kubernetes.io/instance": appName}, + }} + }). + Then(). + Expect(NoNamespace(createdNamespace)). + When(). + AppSet("--dest-namespace", createdNamespace). + Sync(). + Then(). + Expect(Success("")). + Expect(OperationPhaseIs(OperationSucceeded)). + Expect(ResourceHealthWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, health.HealthStatusHealthy)). + Expect(ResourceHealthWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, health.HealthStatusHealthy)). + Expect(ResourceSyncStatusWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, SyncStatusCodeSynced)). + Expect(ResourceSyncStatusWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, SyncStatusCodeSynced)). + And(func(app *Application) { + // Verify ns is created + output, err := Run("", "kubectl", "get", "namespace", createdNamespace) + assert.NoError(t, err) + assert.Contains(t, output, createdNamespace) + }). + When(). + Delete(true). + Then(). + Expect(Success("")). + And(func(app *Application) { + //Verify app delete the namespace auto created + _, err := Run("", "kubectl", "wait", "--for=delete", "namespace", createdNamespace, "--timeout=60s") + assert.NoError(t, err) + }). + Expect(NoNamespace(createdNamespace)) +} + +// Given +// - application is set with --sync-option CreateNamespace=true --sync-option DeleteNamespace=true +// - namespace does not have an appropriate label ("app.kubernetes.io/instance": application-name) +// - application --dest-namespace does not exist +// +// Verity +// - application --dest-namespace is created +// - application sync successful +// - when application is deleted, --dest-namespace is not deleted +// - deletion process shows error +func TestAppDeletionDoesNotDeleteNamespace(t *testing.T) { + appName := "test-app-deletion-does-not-delete-namespace" + SkipOnEnv(t, "OPENSHIFT") + createdNamespace := getNewNamespace(t) + defer func() { + if !t.Skipped() { + _, err := Run("", "kubectl", "delete", "namespace", createdNamespace) + assert.NoError(t, err) + } + }() + Given(t). + Timeout(30). + Path("guestbook"). + When(). + CreateFromFile(func(app *Application) { + app.Spec.SyncPolicy = &SyncPolicy{ + SyncOptions: SyncOptions{"CreateNamespace=true", "DeleteNamespace=true", "PruneLast=true"}, + ManagedNamespaceMetadata: &ManagedNamespaceMetadata{ + Labels: map[string]string{"app.kubernetes.io/instance": appName}, + }} + }). + Then(). + Expect(NoNamespace(createdNamespace)). + When(). + AppSet("--dest-namespace", createdNamespace). + Sync(). + Then(). + Expect(Success("")). + Expect(OperationPhaseIs(OperationSucceeded)). + Expect(ResourceHealthWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, health.HealthStatusHealthy)). + Expect(ResourceHealthWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, health.HealthStatusHealthy)). + Expect(ResourceSyncStatusWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, SyncStatusCodeSynced)). + Expect(ResourceSyncStatusWithNamespaceIs("Deployment", "guestbook-ui", createdNamespace, SyncStatusCodeSynced)). + When(). + And(func() { + _, err := Run("", "kubectl", "label", "--overwrite", "namespace", createdNamespace, "app.kubernetes.io/instance=wrong-owner") + assert.NoError(t, err) + }). + Then(). + Expect(Namespace(createdNamespace, func(app *Application, ns *v1.Namespace) { + delete(ns.Labels, "kubernetes.io/metadata.name") + assert.Equal(t, map[string]string{"app.kubernetes.io/instance": "wrong-owner"}, ns.Labels) + })). + When(). + IgnoreErrors(). + Delete(true). + Then(). + Expect(Error("", "which is not managed by "+appName)). + And(func(app *Application) { + //Verify delete app does not delete the namespace auto created + output, err := Run("", "kubectl", "get", "namespace", createdNamespace) + assert.NoError(t, err) + assert.Contains(t, output, createdNamespace) + }) +} diff --git a/util/argo/resource_tracking.go b/util/argo/resource_tracking.go index 53659115e8b10..ac67e711a5999 100644 --- a/util/argo/resource_tracking.go +++ b/util/argo/resource_tracking.go @@ -31,7 +31,7 @@ type ResourceTracking interface { Normalize(config, live *unstructured.Unstructured, labelKey, trackingMethod string) error } -//AppInstanceValue store information about resource tracking info +// AppInstanceValue store information about resource tracking info type AppInstanceValue struct { ApplicationName string Group string @@ -146,12 +146,12 @@ func (rt *resourceTracking) SetAppInstance(un *unstructured.Unstructured, key, v } } -//BuildAppInstanceValue build resource tracking id in format ;/// +// BuildAppInstanceValue build resource tracking id in format ;/// func (rt *resourceTracking) BuildAppInstanceValue(value AppInstanceValue) string { return fmt.Sprintf("%s:%s/%s:%s/%s", value.ApplicationName, value.Group, value.Kind, value.Namespace, value.Name) } -//ParseAppInstanceValue parse resource tracking id from format :/:/ to struct +// ParseAppInstanceValue parse resource tracking id from format :/:/ to struct func (rt *resourceTracking) ParseAppInstanceValue(value string) (*AppInstanceValue, error) { var appInstanceValue AppInstanceValue parts := strings.Split(value, ":")