fix: cascaded delete on Application should clean up all managed-agent app resources

Signed-off-by: Jonathan West <jonwest@redhat.com>

Author: jgwest

agent/outbound.go

@@ -126,11 +126,6 @@ func (a *Agent) addAppDeletionToQueue(app *v1alpha1.Application) {
 		return
 	}
 
-	// Only send the deletion event when we're in autonomous mode
-	if !a.mode.IsAutonomous() {
-		return
-	}
-
 	q.Add(a.emitter.ApplicationEvent(event.Delete, app))
 	logCtx.WithField("sendq_len", q.Len()).Debugf("Added app delete event to send queue")
 }

agent/outbound_test.go

@@ -138,10 +138,11 @@ func Test_addAppDeletionToQueue(t *testing.T) {
 		defer func() {
 			a.mode = types.AgentModeAutonomous
 		}()
-		// App must be already managed for event to be generated
+		// Delete events will be sent for managed applications on managed agent, whether or not the application is managed.
 		_ = a.appManager.Manage("agent/guestbook")
-		a.addAppDeletionToQueue(app)
 		require.Equal(t, 0, a.queues.SendQ(defaultQueueName).Len())
+		a.addAppDeletionToQueue(app)
+		require.Equal(t, 1, a.queues.SendQ(defaultQueueName).Len())
 		require.False(t, a.appManager.IsManaged("agent/guestbook"))
 	})
 	t.Run("Deletion event for unmanaged application", func(t *testing.T) {

internal/manager/application/application.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/util/retry"
+	"k8s.io/utils/ptr"
 
 	"github.com/argoproj-labs/argocd-agent/internal/backend"
 	appCache "github.com/argoproj-labs/argocd-agent/internal/cache"
@@ -185,6 +186,8 @@ func (m *ApplicationManager) UpdateManagedApp(ctx context.Context, incoming *v1a
 		return nil, fmt.Errorf("updatedManagedApp should be called on a managed agent, only")
 	}
 
+	deletionTimestampChanged := false
+
 	updated, err = m.update(ctx, m.allowUpsert, incoming, func(existing, incoming *v1alpha1.Application) {
 		if v, ok := existing.Annotations[manager.SourceUIDAnnotation]; ok {
 			if incoming.Annotations == nil {
@@ -198,6 +201,11 @@ func (m *ApplicationManager) UpdateManagedApp(ctx context.Context, incoming *v1a
 		existing.Spec = *incoming.Spec.DeepCopy()
 		existing.Operation = incoming.Operation.DeepCopy()
 		existing.Status = *incoming.Status.DeepCopy()
+
+		if incoming.DeletionTimestamp != nil && existing.DeletionTimestamp == nil {
+			deletionTimestampChanged = true
+		}
+
 	}, func(existing, incoming *v1alpha1.Application) (jsondiff.Patch, error) {
 		// We need to keep the refresh label if it is set on the existing app
 		if v, ok := existing.Annotations["argocd.argoproj.io/refresh"]; ok {
@@ -214,6 +222,10 @@ func (m *ApplicationManager) UpdateManagedApp(ctx context.Context, incoming *v1a
 			incoming.Annotations[manager.SourceUIDAnnotation] = v
 		}
 
+		if incoming.DeletionTimestamp != nil && existing.DeletionTimestamp == nil {
+			deletionTimestampChanged = true
+		}
+
 		target := &v1alpha1.Application{
 			ObjectMeta: v1.ObjectMeta{
 				Annotations: incoming.Annotations,
@@ -247,6 +259,14 @@ func (m *ApplicationManager) UpdateManagedApp(ctx context.Context, incoming *v1a
 			logCtx.Warnf("Couldn't unignore change %s for app %s: %v", updated.ResourceVersion, updated.QualifiedName(), err)
 		}
 	}
+
+	if deletionTimestampChanged {
+		logCtx.Infof("deletionTimestamp of managed agent changed from nil to non-nil, so deleting Application")
+		if err := m.applicationBackend.Delete(ctx, incoming.Name, incoming.Namespace, ptr.To(backend.DeletePropagationForeground)); err != nil {
+			return nil, err
+		}
+	}
+
 	return updated, err
 }
 

internal/manager/application/application_test.go

@@ -143,6 +143,7 @@ func Test_ManagerUpdateManaged(t *testing.T) {
 					"bar":                       "foo",
 					manager.SourceUIDAnnotation: "random_uid",
 				},
+				Finalizers: []string{"test-finalizer"},
 			},
 			Spec: v1alpha1.ApplicationSpec{
 				Source: &v1alpha1.ApplicationSource{
@@ -223,6 +224,10 @@ func Test_ManagerUpdateManaged(t *testing.T) {
 
 		// Labels and annotations must be in sync with incoming
 		require.Equal(t, incoming.Labels, updated.Labels)
+
+		// Finalizers must be in sync
+		require.Equal(t, incoming.Finalizers, updated.Finalizers)
+
 		// Non-refresh annotations must be in sync with incoming
 		require.Equal(t, incoming.Annotations["bar"], updated.Annotations["bar"])
 		// Refresh annotation must not be removed
@@ -232,6 +237,7 @@ func Test_ManagerUpdateManaged(t *testing.T) {
 		// Spec must be in sync with incoming
 		require.Equal(t, incoming.Spec, updated.Spec)
 	})
+
 }
 
 func Test_ManagerUpdateStatus(t *testing.T) {

principal/callbacks.go

@@ -69,15 +69,6 @@ func (s *Server) updateAppCallback(old *v1alpha1.Application, new *v1alpha1.Appl
 		"event":            "application_update",
 		"application_name": old.Name,
 	})
-	if len(new.Finalizers) > 0 && len(new.Finalizers) != len(old.Finalizers) {
-		tmp, err := s.appManager.RemoveFinalizers(s.ctx, new)
-		if err != nil {
-			logCtx.WithError(err).Warnf("Could not remove finalizer")
-		} else {
-			logCtx.Debug("Removed finalizer")
-			new = tmp
-		}
-	}
 	if s.appManager.IsChangeIgnored(new.QualifiedName(), new.ResourceVersion) {
 		logCtx.WithField("resource_version", new.ResourceVersion).Debugf("Resource version has already been seen")
 		return

principal/event.go

@@ -33,6 +33,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sync/semaphore"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/validation"
@@ -213,20 +214,59 @@ func (s *Server) processApplicationEvent(ctx context.Context, agentName string,
 		logCtx.Infof("Updated application spec %s", incoming.QualifiedName())
 	// App deletion
 	case event.Delete.String():
-		if !agentMode.IsAutonomous() {
-			logCtx.Debug("Discarding event, because agent is not in autonomous mode")
-			return event.NewEventNotAllowedErr("event type not allowed when mode is not autonomous")
-		}
+		if agentMode.IsAutonomous() {
 
-		deletionPropagation := backend.DeletePropagationForeground
-		err := s.appManager.Delete(ctx, agentName, incoming, &deletionPropagation)
-		if err != nil {
-			if kerrors.IsNotFound(err) {
+			deletionPropagation := backend.DeletePropagationForeground
+			err = s.appManager.Delete(ctx, agentName, incoming, &deletionPropagation)
+			if err != nil {
+				if kerrors.IsNotFound(err) {
+					return nil
+				}
+				return fmt.Errorf("could not delete application %s: %w", incoming.QualifiedName(), err)
+			}
+			logCtx.Infof("Deleted application %s", incoming.QualifiedName())
+
+		} else if agentMode.IsManaged() {
+
+			app, err := s.appManager.Get(ctx, incoming.Name, incoming.Namespace)
+			if err != nil {
+				if errors.IsNotFound(err) {
+					return nil // ignore not found error: no need to delete app if it doesn't exist
+				}
+				logCtx.WithError(err).Error("unexpected error when attempting to retrieve deleted Application")
+				return err
+			}
+
+			// When we receive an 'application deleted' event from a managed agent, it should only cause the principal application to be deleted IF the principal Application is ALSO in the deletion state (deletionTimestamp is non-nil)
+			if app.DeletionTimestamp == nil {
+				logCtx.Warn("application was detected as deleted from managed agent, even though principal application is not in deletion state")
 				return nil
 			}
-			return fmt.Errorf("could not delete application %s: %w", incoming.QualifiedName(), err)
+
+			// Remove the finalizers from the Application when it has been deleted from managed-agent
+			if len(app.Finalizers) > 0 {
+				_, err := s.appManager.RemoveFinalizers(ctx, app)
+				// app.Finalizers = nil
+				// _, err := s.appManager.UpdateManagedApp(ctx, app)
+				if err != nil {
+					logCtx.WithError(err).Error("unexpected error when attempting to update finalizers of deleted Application")
+					return err
+				}
+			}
+
+			deletionPropagation := backend.DeletePropagationForeground
+			err = s.appManager.Delete(ctx, agentName, incoming, &deletionPropagation)
+			if err != nil {
+				if kerrors.IsNotFound(err) {
+					return nil
+				}
+				return fmt.Errorf("could not delete application %s: %w", incoming.QualifiedName(), err)
+			}
+			logCtx.Infof("Deleted application %s", incoming.QualifiedName())
+
+		} else {
+			return fmt.Errorf("unexpected agent mode")
 		}
-		logCtx.Infof("Deleted application %s", incoming.QualifiedName())
 	default:
 		return fmt.Errorf("unable to process event of type %s", ev.Type())
 	}

test/e2e2/delete_test.go

@@ -0,0 +1,140 @@
+// Copyright 2025 The argocd-agent 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 e2e2
+
+import (
+	"testing"
+	"time"
+
+	"github.com/argoproj-labs/argocd-agent/test/e2e2/fixture"
+
+	"github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
+	argoapp "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
+	"github.com/stretchr/testify/suite"
+	appsv1 "k8s.io/api/apps/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+type DeleteTestSuite struct {
+	fixture.BaseSuite
+}
+
+func (suite *DeleteTestSuite) Test_CascadeDeleteOnManagedAgent() {
+	requires := suite.Require()
+
+	t := suite.T()
+
+	t.Log("Create a managed application in the principal's cluster")
+	appOnPrincipal := v1alpha1.Application{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "my-app",
+			Namespace: "agent-managed",
+		},
+		Spec: v1alpha1.ApplicationSpec{
+			Project: "default",
+			Source: &v1alpha1.ApplicationSource{
+				RepoURL:        "https://github.com/argoproj/argocd-example-apps",
+				TargetRevision: "HEAD",
+				Path:           "kustomize-guestbook",
+			},
+			Destination: v1alpha1.ApplicationDestination{
+				Name:      "agent-managed",
+				Namespace: "guestbook",
+			},
+			SyncPolicy: &v1alpha1.SyncPolicy{
+				Automated: &v1alpha1.SyncPolicyAutomated{},
+				SyncOptions: v1alpha1.SyncOptions{
+					"CreateNamespace=true",
+				},
+			},
+		},
+	}
+
+	err := ensureAppExistsAndIsSyncedAndHealthy(&appOnPrincipal, suite.PrincipalClient, &suite.BaseSuite)
+	requires.NoError(err)
+
+	t.Log("'kustomize-guestbook-ui' Deployment should exist on managed-agent")
+	depl := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "kustomize-guestbook-ui", Namespace: "guestbook"}}
+	requires.Eventually(func() bool {
+		err := suite.ManagedAgentClient.Get(suite.Ctx, fixture.ToNamespacedName(depl), depl, metav1.GetOptions{})
+		return err == nil
+	}, 60*time.Second, 1*time.Second)
+
+	t.Log("Add a finalizer to the Deployment to keep it from being deleted")
+	depl.Finalizers = append(depl.Finalizers, "test-e2e/my-test-finalizer")
+	err = suite.ManagedAgentClient.Update(suite.Ctx, depl, metav1.UpdateOptions{})
+	requires.NoError(err)
+
+	// Ensure that finalizer is removed from Deployment if the test ends prematurely
+	defer func() {
+		err = suite.ManagedAgentClient.Get(suite.Ctx, fixture.ToNamespacedName(depl), depl, metav1.GetOptions{})
+		if err == nil { // If the Deployment still exists
+			depl.Finalizers = nil // Remove the finalizer
+			err = suite.ManagedAgentClient.Update(suite.Ctx, depl, metav1.UpdateOptions{})
+			requires.NoError(err)
+		}
+	}()
+
+	t.Log("Simulate cascade deletion from Argo CD: Add a resources-finalizer to principal, then delete it (which sets deletion timestamp)")
+	err = suite.PrincipalClient.EnsureApplicationUpdate(suite.Ctx, fixture.ToNamespacedName(&appOnPrincipal), func(a *v1alpha1.Application) error {
+		a.Finalizers = append(appOnPrincipal.Finalizers, "resources-finalizer.argocd.argoproj.io")
+		return nil
+	}, metav1.UpdateOptions{})
+	requires.NoError(err)
+
+	err = suite.PrincipalClient.Delete(suite.Ctx, &appOnPrincipal, metav1.DeleteOptions{})
+	requires.NoError(err)
+
+	t.Log("Ensure that finalizers and deletion timestamp are synced to managed-agent")
+	requires.Eventually(func() bool {
+		app := argoapp.Application{}
+		err := suite.ManagedAgentClient.Get(suite.Ctx, fixture.ToNamespacedName(&appOnPrincipal), &app, metav1.GetOptions{})
+		return err == nil && app.DeletionTimestamp != nil && len(app.Finalizers) > 0
+	}, 60*time.Second, 1*time.Second)
+
+	t.Log("Remove finalizer from the Deployment, so that it may proceed with being deleted")
+	err = suite.ManagedAgentClient.Get(suite.Ctx, fixture.ToNamespacedName(depl), depl, metav1.GetOptions{})
+	requires.NoError(err)
+
+	depl.Finalizers = nil
+	err = suite.ManagedAgentClient.Update(suite.Ctx, depl, metav1.UpdateOptions{})
+	requires.NoError(err)
+
+	t.Log("Verify managed agent application is eventually deleted")
+	requires.Eventually(func() bool {
+		app := argoapp.Application{}
+		err := suite.ManagedAgentClient.Get(suite.Ctx, fixture.ToNamespacedName(&appOnPrincipal), &app, metav1.GetOptions{})
+		return err != nil && errors.IsNotFound(err)
+	}, 60*time.Second, 1*time.Second)
+
+	t.Log("Verify guestbook Deployment is deleted from managed-agent cluster, which proves it is a cascaded delete")
+	requires.Eventually(func() bool {
+		err := suite.ManagedAgentClient.Get(suite.Ctx, fixture.ToNamespacedName(depl), depl, metav1.GetOptions{})
+		return err != nil && errors.IsNotFound(err)
+	}, 60*time.Second, 1*time.Second)
+
+	t.Log("Verify principal application is deleted")
+	requires.Eventually(func() bool {
+		app := argoapp.Application{}
+		err := suite.PrincipalClient.Get(suite.Ctx, fixture.ToNamespacedName(&appOnPrincipal), &app, metav1.GetOptions{})
+		return err != nil && errors.IsNotFound(err)
+	}, 60*time.Second, 1*time.Second)
+
+}
+
+func TestDeleteTestSuite(t *testing.T) {
+	suite.Run(t, new(DeleteTestSuite))
+}

test/e2e2/fixture/fixture.go

@@ -85,8 +85,31 @@ func EnsureDeletion(ctx context.Context, kclient KubeClient, obj KubeObject) err
 		return err
 	}
 
+	// Wait for the object to be deleted  for 60 seconds
+	// - Primarily this will be waiting for the finalizer to be removed, so that the object is deleted
 	key := types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()}
-	for count := 0; count < 120; count++ {
+	for count := 0; count < 60; count++ {
+		err := kclient.Get(ctx, key, obj, metav1.GetOptions{})
+		if errors.IsNotFound(err) {
+			return nil
+		} else if err == nil {
+			time.Sleep(1 * time.Second)
+		} else {
+			return err
+		}
+	}
+
+	// After X seconds, give up waiting for the child objects to be deleted, and remove any finalizers on the object
+	if len(obj.GetFinalizers()) > 0 {
+		obj.SetFinalizers(nil)
+		if err := kclient.Update(ctx, obj, metav1.UpdateOptions{}); err != nil {
+			return err
+		}
+	}
+
+	// Continue waiting for object to be deleted, now that finalizers have been removed.
+	key = types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()}
+	for count := 0; count < 60; count++ {
 		err := kclient.Get(ctx, key, obj, metav1.GetOptions{})
 		if errors.IsNotFound(err) {
 			return nil