From 1790b63f771cd5b855b36232a77d5985dcc154b7 Mon Sep 17 00:00:00 2001 From: khewonc <39867936+khewonc@users.noreply.github.com> Date: Tue, 7 Jan 2025 15:30:08 -0500 Subject: [PATCH 1/2] Do not error if pod needing deletion is not found --- .../controller_reconcile_agent.go | 5 +- .../controller_reconcile_agent_test.go | 223 ++++++++++++++++++ 2 files changed, 227 insertions(+), 1 deletion(-) diff --git a/internal/controller/datadogagent/controller_reconcile_agent.go b/internal/controller/datadogagent/controller_reconcile_agent.go index 54fcdb42d..4f6a9530a 100644 --- a/internal/controller/datadogagent/controller_reconcile_agent.go +++ b/internal/controller/datadogagent/controller_reconcile_agent.go @@ -26,6 +26,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -379,7 +380,9 @@ func (r *Reconciler) cleanupPodsForProfilesThatNoLongerApply(ctx context.Context }, } if err = r.client.Delete(ctx, &toDelete); err != nil { - return err + if !apierrors.IsNotFound(err) { + return err + } } } } diff --git a/internal/controller/datadogagent/controller_reconcile_agent_test.go b/internal/controller/datadogagent/controller_reconcile_agent_test.go index 4cc45bdd4..307aaebc9 100644 --- a/internal/controller/datadogagent/controller_reconcile_agent_test.go +++ b/internal/controller/datadogagent/controller_reconcile_agent_test.go @@ -1841,3 +1841,226 @@ func Test_labelNodesWithProfiles(t *testing.T) { }) } } + +func Test_cleanupPodsForProfilesThatNoLongerApply(t *testing.T) { + sch := runtime.NewScheme() + _ = scheme.AddToScheme(sch) + ctx := context.Background() + + testCases := []struct { + name string + description string + profilesByNode map[string]types.NamespacedName + ddaNamespace string + existingPods []client.Object + wantPods []corev1.Pod + }{ + { + name: "delete agent pod that shouldn't be running", + description: "pod-2 should be deleted", + profilesByNode: map[string]types.NamespacedName{ + "node-1": { + Namespace: "foo", + Name: "profile-1", + }, + "node-2": { + Namespace: "foo", + Name: "profile-2", + }, + "node-default": { + Namespace: "", + Name: "default", + }, + }, + ddaNamespace: "foo", + existingPods: []client.Object{ + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "foo", + Labels: map[string]string{ + apicommon.AgentDeploymentComponentLabelKey: constants.DefaultAgentResourceSuffix, + agentprofile.ProfileLabelKey: "profile-1", + }, + }, + Spec: corev1.PodSpec{ + NodeName: "node-1", + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-2", + Namespace: "foo", + Labels: map[string]string{ + apicommon.AgentDeploymentComponentLabelKey: constants.DefaultAgentResourceSuffix, + agentprofile.ProfileLabelKey: "profile-1", + }, + }, + Spec: corev1.PodSpec{ + NodeName: "node-2", + }, + }, + }, + wantPods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "foo", + Labels: map[string]string{ + apicommon.AgentDeploymentComponentLabelKey: constants.DefaultAgentResourceSuffix, + agentprofile.ProfileLabelKey: "profile-1", + }, + ResourceVersion: "999", + }, + Spec: corev1.PodSpec{ + NodeName: "node-1", + }, + }, + }, + }, + { + name: "delete default agent on profile node", + description: "pod-2 should be deleted", + profilesByNode: map[string]types.NamespacedName{ + "node-1": { + Namespace: "foo", + Name: "profile-1", + }, + "node-2": { + Namespace: "foo", + Name: "profile-2", + }, + "node-default": { + Namespace: "", + Name: "default", + }, + }, + ddaNamespace: "foo", + existingPods: []client.Object{ + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "foo", + Labels: map[string]string{ + apicommon.AgentDeploymentComponentLabelKey: constants.DefaultAgentResourceSuffix, + agentprofile.ProfileLabelKey: "profile-1", + }, + }, + Spec: corev1.PodSpec{ + NodeName: "node-1", + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-default", + Namespace: "foo", + Labels: map[string]string{ + apicommon.AgentDeploymentComponentLabelKey: constants.DefaultAgentResourceSuffix, + }, + }, + Spec: corev1.PodSpec{ + NodeName: "node-2", + }, + }, + }, + wantPods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "foo", + Labels: map[string]string{ + apicommon.AgentDeploymentComponentLabelKey: constants.DefaultAgentResourceSuffix, + agentprofile.ProfileLabelKey: "profile-1", + }, + ResourceVersion: "999", + }, + Spec: corev1.PodSpec{ + NodeName: "node-1", + }, + }, + }, + }, + { + name: "delete profile agent on default node", + description: "pod-2 should be deleted", + profilesByNode: map[string]types.NamespacedName{ + "node-1": { + Namespace: "foo", + Name: "profile-1", + }, + "node-2": { + Namespace: "foo", + Name: "profile-2", + }, + "node-default": { + Namespace: "", + Name: "default", + }, + }, + ddaNamespace: "foo", + existingPods: []client.Object{ + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "foo", + Labels: map[string]string{ + apicommon.AgentDeploymentComponentLabelKey: constants.DefaultAgentResourceSuffix, + agentprofile.ProfileLabelKey: "profile-1", + }, + }, + Spec: corev1.PodSpec{ + NodeName: "node-1", + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-2", + Namespace: "foo", + Labels: map[string]string{ + apicommon.AgentDeploymentComponentLabelKey: constants.DefaultAgentResourceSuffix, + agentprofile.ProfileLabelKey: "profile-2", + }, + }, + Spec: corev1.PodSpec{ + NodeName: "node-default", + }, + }, + }, + wantPods: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: "foo", + Labels: map[string]string{ + apicommon.AgentDeploymentComponentLabelKey: constants.DefaultAgentResourceSuffix, + agentprofile.ProfileLabelKey: "profile-1", + }, + ResourceVersion: "999", + }, + Spec: corev1.PodSpec{ + NodeName: "node-1", + }, + }, + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithScheme(sch).WithObjects(tt.existingPods...).Build() + + r := &Reconciler{ + client: fakeClient, + } + + err := r.cleanupPodsForProfilesThatNoLongerApply(ctx, tt.profilesByNode, tt.ddaNamespace) + assert.NoError(t, err) + + podList := &corev1.PodList{} + err = fakeClient.List(ctx, podList) + assert.NoError(t, err) + assert.Len(t, podList.Items, len(tt.wantPods)) + assert.Equal(t, tt.wantPods, podList.Items) + }) + } +} From c9656fc25a281480abb1e15046bb688b8d78c791 Mon Sep 17 00:00:00 2001 From: khewonc <39867936+khewonc@users.noreply.github.com> Date: Tue, 7 Jan 2025 15:51:51 -0500 Subject: [PATCH 2/2] Remove duplicate import --- .../controller/datadogagent/controller_reconcile_agent.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/controller/datadogagent/controller_reconcile_agent.go b/internal/controller/datadogagent/controller_reconcile_agent.go index 4f6a9530a..c2bd34fcd 100644 --- a/internal/controller/datadogagent/controller_reconcile_agent.go +++ b/internal/controller/datadogagent/controller_reconcile_agent.go @@ -26,7 +26,6 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -379,10 +378,8 @@ func (r *Reconciler) cleanupPodsForProfilesThatNoLongerApply(ctx context.Context Name: agentPod.Name, }, } - if err = r.client.Delete(ctx, &toDelete); err != nil { - if !apierrors.IsNotFound(err) { - return err - } + if err = r.client.Delete(ctx, &toDelete); err != nil && !errors.IsNotFound(err) { + return err } } }