Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Unmanage app on agent on deletion event #301

Merged
merged 3 commits into from
Feb 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion agent/outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ func (a *Agent) addAppDeletionToQueue(app *v1alpha1.Application) {
logCtx.Debugf("Delete app event")

if !a.appManager.IsManaged(app.QualifiedName()) {
logCtx.Tracef("App is not managed")
logCtx.Tracef("App is not managed, proceeding anyways")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: logCtx.Trace can be used as there are no format specifiers.

} else {
_ = a.appManager.Unmanage(app.QualifiedName())
}

q := a.queues.SendQ(a.remote.ClientID())
Expand Down
150 changes: 150 additions & 0 deletions agent/outbound_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
package agent

import (
"testing"

"github.com/argoproj-labs/argocd-agent/internal/event"
"github.com/argoproj-labs/argocd-agent/pkg/types"
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func Test_addAppCreationToQueue(t *testing.T) {
a := newAgent(t)
a.remote.SetClientID("agent")
a.emitter = event.NewEventSource("principal")
err := a.queues.Create("agent")
require.NoError(t, err)

t.Run("Add some application", func(t *testing.T) {
app := &v1alpha1.Application{ObjectMeta: v1.ObjectMeta{Name: "guestbook", Namespace: "agent"}}
a.addAppCreationToQueue(app)

// Should have an event in queue
ev, _ := a.queues.SendQ("agent").Get()
assert.NotNil(t, ev)
// Queue should be empty after get
assert.Equal(t, 0, a.queues.SendQ("agent").Len())

// App should be managed by now
assert.True(t, a.appManager.IsManaged("agent/guestbook"))
})

t.Run("Add some application that is already managed", func(t *testing.T) {
a.appManager.Manage("agent/guestbook")
app := &v1alpha1.Application{ObjectMeta: v1.ObjectMeta{Name: "guestbook", Namespace: "agent"}}
a.addAppCreationToQueue(app)

// Should not have an event in queue
items := a.queues.SendQ("agent").Len()
assert.Equal(t, 0, items)

// App should be managed by now
assert.True(t, a.appManager.IsManaged("agent/guestbook"))
})

t.Run("Missing queue", func(t *testing.T) {
a.remote.SetClientID("notexisting")
app := &v1alpha1.Application{ObjectMeta: v1.ObjectMeta{Name: "testapp", Namespace: "agent"}}
a.addAppCreationToQueue(app)

// Should not have an event in queue
items := a.queues.SendQ("agent").Len()
assert.Equal(t, 0, items)

// App should be managed by now
assert.True(t, a.appManager.IsManaged("agent/testapp"))
})
}

func Test_addAppUpdateToQueue(t *testing.T) {
a := newAgent(t)
a.remote.SetClientID("agent")
a.emitter = event.NewEventSource("principal")
err := a.queues.Create("agent")
require.NoError(t, err)

t.Run("Update event for autonomous agent", func(t *testing.T) {
app := &v1alpha1.Application{ObjectMeta: v1.ObjectMeta{Name: "guestbook", Namespace: "agent"}}
// App must be already managed for event to be generated
_ = a.appManager.Manage("agent/guestbook")
a.mode = types.AgentModeAutonomous
a.addAppUpdateToQueue(app, app)
defer a.appManager.Unmanage("agent/guestbook")

ev, _ := a.queues.SendQ("agent").Get()
require.NotNil(t, ev)
assert.Equal(t, event.SpecUpdate.String(), ev.Type())
})

t.Run("Update event for managed agent", func(t *testing.T) {
app := &v1alpha1.Application{ObjectMeta: v1.ObjectMeta{Name: "guestbook", Namespace: "agent"}}
// App must be already managed for event to be generated
_ = a.appManager.Manage("agent/guestbook")
a.mode = types.AgentModeManaged
a.addAppUpdateToQueue(app, app)
defer a.appManager.Unmanage("agent/guestbook")

ev, _ := a.queues.SendQ("agent").Get()
require.NotNil(t, ev)
assert.Equal(t, event.StatusUpdate.String(), ev.Type())
})

t.Run("Update event for unmanaged application", func(t *testing.T) {
app := &v1alpha1.Application{ObjectMeta: v1.ObjectMeta{Name: "guestbook", Namespace: "agent"}}
a.addAppUpdateToQueue(app, app)
defer a.appManager.Unmanage("agent/guestbook")
require.Equal(t, 0, a.queues.SendQ("agent").Len())
})

}

func Test_addAppDeletionToQueue(t *testing.T) {
a := newAgent(t)
a.remote.SetClientID("agent")
a.emitter = event.NewEventSource("principal")
err := a.queues.Create("agent")
require.NoError(t, err)

t.Run("Deletion event for managed application", func(t *testing.T) {
app := &v1alpha1.Application{ObjectMeta: v1.ObjectMeta{Name: "guestbook", Namespace: "agent"}}
// App must be already managed for event to be generated
_ = a.appManager.Manage("agent/guestbook")
a.addAppDeletionToQueue(app)
ev, _ := a.queues.SendQ("agent").Get()
assert.Equal(t, event.Delete.String(), ev.Type())
require.False(t, a.appManager.IsManaged("agent/guestbook"))
})
t.Run("Deletion event for unmanaged application", func(t *testing.T) {
app := &v1alpha1.Application{ObjectMeta: v1.ObjectMeta{Name: "guestbook", Namespace: "agent"}}
// App must be already managed for event to be generated
a.addAppDeletionToQueue(app)
ev, _ := a.queues.SendQ("agent").Get()
assert.Equal(t, event.Delete.String(), ev.Type())
require.False(t, a.appManager.IsManaged("agent/guestbook"))
})
}

func Test_deleteNamespaceCallback(t *testing.T) {
a := newAgent(t)
a.remote.SetClientID("agent")
a.emitter = event.NewEventSource("principal")
err := a.queues.Create("agent")
require.NoError(t, err)

t.Run("Delete queue with same name as namespace", func(t *testing.T) {
ns := &corev1.Namespace{ObjectMeta: v1.ObjectMeta{Name: "agent"}}
a.deleteNamespaceCallback(ns)
require.False(t, a.queues.HasQueuePair("agent"))
// Recreate queue so that following tests will work
a.queues.Create("agent")
})
t.Run("Unrelated namespace deletion", func(t *testing.T) {
ns := &corev1.Namespace{ObjectMeta: v1.ObjectMeta{Name: "argocd"}}
a.deleteNamespaceCallback(ns)
require.True(t, a.queues.HasQueuePair("agent"))
})
}
6 changes: 6 additions & 0 deletions pkg/client/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,12 @@ func (r *Remote) SetClientMode(mode types.AgentMode) {
r.clientMode = mode
}

// SetClientID sets the client ID for this remote
// The only use case for this is to be used in unit testing.
func (r *Remote) SetClientID(id string) {
r.clientID = id
}

func log() *logrus.Entry {
return logrus.WithField("module", "Connector")
}
Loading