Skip to content

Commit e909e73

Browse files
committed
fix: use RevertUserInitiatedDeletion for application recreation
Assisted-by: Cursor Signed-off-by: Rizwana777 <[email protected]>
1 parent 9fc1c8a commit e909e73

File tree

5 files changed

+80
-110
lines changed

5 files changed

+80
-110
lines changed

agent/outbound.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,18 @@ func (a *Agent) addAppDeletionToQueue(app *v1alpha1.Application) {
118118
logCtx := log().WithField(logfields.Event, "DeleteApp").WithField(logfields.Application, app.QualifiedName())
119119
logCtx.Debugf("Delete app event")
120120

121+
if isResourceFromPrincipal(app) {
122+
reverted, err := manager.RevertUserInitiatedDeletion(a.context, app, a.deletions, a.appManager, logCtx)
123+
if err != nil {
124+
logCtx.WithError(err).Error("failed to revert invalid deletion of application")
125+
return
126+
}
127+
if reverted {
128+
logCtx.Trace("Deleted application is recreated")
129+
return
130+
}
131+
}
132+
121133
a.resources.Remove(resources.NewResourceKeyFromApp(app))
122134

123135
if !a.appManager.IsManaged(app.QualifiedName()) {

agent/outbound_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/alicebob/miniredis/v2"
88
"github.com/argoproj-labs/argocd-agent/internal/argocd/cluster"
99
"github.com/argoproj-labs/argocd-agent/internal/event"
10+
"github.com/argoproj-labs/argocd-agent/internal/manager"
1011
"github.com/argoproj-labs/argocd-agent/internal/resources"
1112
"github.com/argoproj-labs/argocd-agent/pkg/client"
1213
"github.com/argoproj-labs/argocd-agent/pkg/types"
@@ -17,6 +18,7 @@ import (
1718
"github.com/stretchr/testify/require"
1819
corev1 "k8s.io/api/core/v1"
1920
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
21+
k8stypes "k8s.io/apimachinery/pkg/types"
2022
)
2123

2224
func Test_addAppCreationToQueue(t *testing.T) {
@@ -164,6 +166,69 @@ func Test_addAppDeletionToQueue(t *testing.T) {
164166
assert.Equal(t, 0, items.Len())
165167
require.False(t, a.appManager.IsManaged("agent/guestbook"))
166168
})
169+
t.Run("Recreation of application from principal when deleted", func(t *testing.T) {
170+
// Create a fresh agent for this test
171+
testAgent, _ := newAgent(t)
172+
testAgent.remote.SetClientID("agent")
173+
testAgent.emitter = event.NewEventSource("principal")
174+
testAgent.mode = types.AgentModeManaged
175+
176+
// Create an app with source UID annotation (from principal)
177+
app := &v1alpha1.Application{
178+
ObjectMeta: v1.ObjectMeta{
179+
Name: "guestbook",
180+
Namespace: "agent",
181+
Annotations: map[string]string{
182+
manager.SourceUIDAnnotation: "uid-123",
183+
},
184+
},
185+
}
186+
187+
// App must be already managed for event to be generated
188+
_ = testAgent.appManager.Manage("agent/guestbook")
189+
defer testAgent.appManager.ClearManaged()
190+
191+
// Don't mark the deletion as expected, so it should be recreated
192+
testAgent.addAppDeletionToQueue(app)
193+
194+
// Should have recreated the app (no event in queue because recreation happened)
195+
assert.Equal(t, 0, testAgent.queues.SendQ(defaultQueueName).Len())
196+
197+
// Verify the app is still managed (recreated)
198+
assert.True(t, testAgent.appManager.IsManaged("agent/guestbook"))
199+
})
200+
t.Run("No recreation when deletion is expected from principal", func(t *testing.T) {
201+
// Create a fresh agent for this test
202+
testAgent, _ := newAgent(t)
203+
testAgent.remote.SetClientID("agent")
204+
testAgent.emitter = event.NewEventSource("principal")
205+
testAgent.mode = types.AgentModeManaged
206+
207+
// Create an app with source UID annotation (from principal)
208+
app := &v1alpha1.Application{
209+
ObjectMeta: v1.ObjectMeta{
210+
Name: "guestbook",
211+
Namespace: "agent",
212+
Annotations: map[string]string{
213+
manager.SourceUIDAnnotation: "uid-123",
214+
},
215+
},
216+
}
217+
218+
// App must be already managed for event to be generated
219+
_ = testAgent.appManager.Manage("agent/guestbook")
220+
defer testAgent.appManager.ClearManaged()
221+
222+
// Mark the deletion as expected
223+
testAgent.deletions.MarkExpected(k8stypes.UID("uid-123"))
224+
225+
testAgent.addAppDeletionToQueue(app)
226+
227+
// Should have sent delete event (not recreated)
228+
assert.Equal(t, 1, testAgent.queues.SendQ(defaultQueueName).Len())
229+
ev, _ := testAgent.queues.SendQ(defaultQueueName).Get()
230+
assert.Equal(t, event.Delete.String(), ev.Type())
231+
})
167232
}
168233

169234
func Test_deleteNamespaceCallback(t *testing.T) {

principal/event.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -255,21 +255,6 @@ func (s *Server) processApplicationEvent(ctx context.Context, agentName string,
255255
// 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)
256256
if app.DeletionTimestamp == nil {
257257
logCtx.Warn("application was detected as deleted from managed agent, even though principal application is not in deletion state")
258-
// In managed mode, if the application was deleted from the agent but the principal
259-
// application is not in deletion state, we should recreate it by sending the
260-
// application spec back to the managed agent
261-
logCtx.Info("Recreating application in managed agent to maintain desired state")
262-
263-
// Send the application spec to the managed agent to recreate it
264-
ev := s.events.ApplicationEvent(event.SpecUpdate, app)
265-
q := s.queues.SendQ(agentName)
266-
if q == nil {
267-
logCtx.Error("Send queue not found for agent, cannot recreate application")
268-
return fmt.Errorf("send queue not found for agent %s", agentName)
269-
}
270-
q.Add(ev)
271-
logCtx.Infof("Sent application spec to managed agent %s to recreate application %s", agentName, app.Name)
272-
273258
return nil
274259
}
275260

principal/event_test.go

Lines changed: 0 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1172,95 +1172,3 @@ func Test_processClusterCacheInfoUpdateEvent(t *testing.T) {
11721172
assert.ErrorContains(t, err, "not mapped to any cluster")
11731173
})
11741174
}
1175-
1176-
func Test_ManagedAppRecreationLogic(t *testing.T) {
1177-
1178-
type test struct {
1179-
name string
1180-
deletionTimestampSetOnPrincipal bool
1181-
expectRecreation bool
1182-
}
1183-
1184-
tests := []test{
1185-
{
1186-
name: "Delete event from managed agent should recreate app when principal is not in deletion state",
1187-
deletionTimestampSetOnPrincipal: false,
1188-
expectRecreation: true,
1189-
},
1190-
{
1191-
name: "Delete event from managed agent should not recreate app when principal is in deletion state",
1192-
deletionTimestampSetOnPrincipal: true,
1193-
expectRecreation: false,
1194-
},
1195-
}
1196-
1197-
for _, test := range tests {
1198-
1199-
t.Run(test.name, func(t *testing.T) {
1200-
delApp := &v1alpha1.Application{
1201-
ObjectMeta: v1.ObjectMeta{
1202-
Name: "test-app",
1203-
Namespace: "agent-managed",
1204-
Finalizers: []string{"test-finalizer"},
1205-
},
1206-
Spec: v1alpha1.ApplicationSpec{
1207-
Project: "default",
1208-
Source: &v1alpha1.ApplicationSource{
1209-
RepoURL: "https://github.com/argoproj/argocd-example-apps",
1210-
TargetRevision: "HEAD",
1211-
Path: "kustomize-guestbook",
1212-
},
1213-
Destination: v1alpha1.ApplicationDestination{
1214-
Server: "https://kubernetes.default.svc",
1215-
Namespace: "guestbook",
1216-
},
1217-
},
1218-
Status: v1alpha1.ApplicationStatus{
1219-
Sync: v1alpha1.SyncStatus{Status: v1alpha1.SyncStatusCodeSynced},
1220-
},
1221-
}
1222-
1223-
if test.deletionTimestampSetOnPrincipal {
1224-
delApp.DeletionTimestamp = ptr.To(v1.Time{Time: time.Now()})
1225-
}
1226-
1227-
fac := kube.NewKubernetesFakeClientWithApps("argocd")
1228-
ev := cloudevents.NewEvent()
1229-
ev.SetDataSchema("application")
1230-
ev.SetType(event.Delete.String())
1231-
ev.SetData(cloudevents.ApplicationJSON, delApp)
1232-
wq := wqmock.NewTypedRateLimitingInterface[*cloudevents.Event](t)
1233-
wq.On("Get").Return(&ev, false)
1234-
wq.On("Done", &ev)
1235-
s, err := NewServer(context.Background(), fac, "argocd", WithGeneratedTokenSigningKey())
1236-
require.NoError(t, err)
1237-
s.setAgentMode("agent-managed", types.AgentModeManaged)
1238-
s.events = event.NewEventSource("test")
1239-
1240-
// Set up the send queue for normal operation
1241-
err = s.queues.Create("agent-managed")
1242-
require.NoError(t, err)
1243-
1244-
// Create the application in the principal
1245-
_, err = fac.ApplicationsClientset.ArgoprojV1alpha1().Applications(delApp.Namespace).Create(context.Background(), delApp, v1.CreateOptions{})
1246-
require.NoError(t, err)
1247-
1248-
got, err := s.processRecvQueue(context.Background(), "agent-managed", wq)
1249-
require.NoError(t, err)
1250-
require.Equal(t, ev, *got)
1251-
1252-
if test.expectRecreation {
1253-
// When recreation is expected, the application should still exist in the principal
1254-
// because the deletion was prevented and a recreation event was sent
1255-
_, err = fac.ApplicationsClientset.ArgoprojV1alpha1().Applications(delApp.Namespace).Get(context.Background(), delApp.Name, v1.GetOptions{})
1256-
require.NoError(t, err, "Application should still exist in principal when recreation is expected")
1257-
} else {
1258-
// When recreation is not expected, the app should be deleted
1259-
// This happens when the principal app is already in deletion state
1260-
_, err = fac.ApplicationsClientset.ArgoprojV1alpha1().Applications(delApp.Namespace).Get(context.Background(), delApp.Name, v1.GetOptions{})
1261-
require.True(t, apierrors.IsNotFound(err), "Application should be deleted when recreation is not expected")
1262-
}
1263-
})
1264-
}
1265-
1266-
}

test/e2e/resync_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,9 +1019,9 @@ func sampleRepository() *corev1.Secret {
10191019
}
10201020

10211021
// Test_ManagedAppRecreationOnDirectDeletion tests the scenario where:
1022-
// 1. A managed application exists on both principal and managed agent
1023-
// 2. The application is directly deleted from the managed agent (while agent is running)
1024-
// 3. The principal should detect this and recreate the application automatically
1022+
// 1. A managed application exists on both principal and managed agent
1023+
// 2. The application is directly deleted from the managed agent (while agent is running)
1024+
// 3. The agent should detect this unauthorized deletion and recreate the application automatically
10251025
func (suite *ResyncTestSuite) Test_ManagedAppRecreationOnDirectDeletion() {
10261026
requires := suite.Require()
10271027
t := suite.T()

0 commit comments

Comments
 (0)