diff --git a/controllers/datadogagent/controller_reconcile_v2.go b/controllers/datadogagent/controller_reconcile_v2.go index ab9496d3c6..c69ece5d77 100644 --- a/controllers/datadogagent/controller_reconcile_v2.go +++ b/controllers/datadogagent/controller_reconcile_v2.go @@ -137,10 +137,11 @@ func (r *Reconciler) reconcileInstanceV2(ctx context.Context, logger logr.Logger // ----------------------------- var err error + now := metav1.NewTime(time.Now()) result, err = r.reconcileV2ClusterAgent(logger, requiredComponents, features, instance, resourceManagers, newStatus) if utils.ShouldReturn(result, err) { - return r.updateStatusIfNeededV2(logger, instance, newStatus, result, err) + return r.updateStatusIfNeededV2(logger, instance, newStatus, result, err, now) } // Start with an "empty" profile and provider @@ -161,12 +162,11 @@ func (r *Reconciler) reconcileInstanceV2(ctx context.Context, logger logr.Logger } if r.options.DatadogAgentProfileEnabled { - now := metav1.NewTime(time.Now()) - profileList, profilesByNode, e := r.profilesToApply(ctx, logger, nodeList, now) + var profilesByNode map[string]types.NamespacedName + profiles, profilesByNode, e = r.profilesToApply(ctx, logger, nodeList, now) if err != nil { return reconcile.Result{}, e } - profiles = profileList if err = r.handleProfiles(ctx, profilesByNode, instance.Namespace); err != nil { return reconcile.Result{}, err @@ -178,7 +178,7 @@ func (r *Reconciler) reconcileInstanceV2(ctx context.Context, logger logr.Logger for provider := range providerList { result, err = r.reconcileV2Agent(logger, requiredComponents, features, instance, resourceManagers, newStatus, provider, providerList, &profile) if utils.ShouldReturn(result, err) { - return r.updateStatusIfNeededV2(logger, instance, newStatus, result, err) + return r.updateStatusIfNeededV2(logger, instance, newStatus, result, err, now) } } } @@ -189,7 +189,7 @@ func (r *Reconciler) reconcileInstanceV2(ctx context.Context, logger logr.Logger result, err = r.reconcileV2ClusterChecksRunner(logger, requiredComponents, features, instance, resourceManagers, newStatus) if utils.ShouldReturn(result, err) { - return r.updateStatusIfNeededV2(logger, instance, newStatus, result, err) + return r.updateStatusIfNeededV2(logger, instance, newStatus, result, err, now) } // ------------------------------ @@ -213,11 +213,10 @@ func (r *Reconciler) reconcileInstanceV2(ctx context.Context, logger logr.Logger if !result.Requeue && result.RequeueAfter == 0 { result.RequeueAfter = defaultRequeuePeriod } - return r.updateStatusIfNeededV2(logger, instance, newStatus, result, err) + return r.updateStatusIfNeededV2(logger, instance, newStatus, result, err, now) } -func (r *Reconciler) updateStatusIfNeededV2(logger logr.Logger, agentdeployment *datadoghqv2alpha1.DatadogAgent, newStatus *datadoghqv2alpha1.DatadogAgentStatus, result reconcile.Result, currentError error) (reconcile.Result, error) { - now := metav1.NewTime(time.Now()) +func (r *Reconciler) updateStatusIfNeededV2(logger logr.Logger, agentdeployment *datadoghqv2alpha1.DatadogAgent, newStatus *datadoghqv2alpha1.DatadogAgentStatus, result reconcile.Result, currentError error, now metav1.Time) (reconcile.Result, error) { if currentError == nil { datadoghqv2alpha1.UpdateDatadogAgentStatusConditions(newStatus, now, datadoghqv2alpha1.DatadogAgentReconcileErrorConditionType, metav1.ConditionFalse, "DatadogAgent_reconcile_ok", "DatadogAgent reconcile ok", false) } else { @@ -303,7 +302,7 @@ func (r *Reconciler) updateMetricsForwardersFeatures(dda *datadoghqv2alpha1.Data // } } -// ProfilesToApply gets a list of profiles and returns the ones that should be +// profilesToApply gets a list of profiles and returns the ones that should be // applied in the cluster. // - If there are no profiles, it returns the default profile. // - If there are no conflicting profiles, it returns all the profiles plus the default one. @@ -321,26 +320,26 @@ func (r *Reconciler) profilesToApply(ctx context.Context, logger logr.Logger, no return nil, nil, err } - var profilesToApply []datadoghqv1alpha1.DatadogAgentProfile - profileAppliedPerNode := make(map[string]types.NamespacedName, len(nodeList)) + var profileListToApply []datadoghqv1alpha1.DatadogAgentProfile + profileAppliedByNode := make(map[string]types.NamespacedName, len(nodeList)) sortedProfiles := agentprofile.SortProfiles(profilesList.Items) for _, profile := range sortedProfiles { - profileAppliedPerNode, err = agentprofile.ProfileToApply(logger, &profile, nodeList, profileAppliedPerNode, now) + profileAppliedByNode, err = agentprofile.ProfileToApply(logger, &profile, nodeList, profileAppliedByNode, now) r.updateDAPStatus(logger, &profile) if err != nil { // profile is invalid or conflicts logger.Error(err, "profile cannot be applied", "name", profile.Name, "namespace", profile.Namespace) continue } - profilesToApply = append(profilesToApply, profile) + profileListToApply = append(profileListToApply, profile) } // add default profile - profilesToApply = agentprofile.ApplyDefaultProfile(profilesToApply, profileAppliedPerNode, nodeList) + profileListToApply = agentprofile.ApplyDefaultProfile(profileListToApply, profileAppliedByNode, nodeList) - return profilesToApply, profileAppliedPerNode, nil + return profileListToApply, profileAppliedByNode, nil } func (r *Reconciler) getNodeList(ctx context.Context) ([]corev1.Node, error) { diff --git a/controllers/datadogagent/controller_reconcile_v2_test.go b/controllers/datadogagent/controller_reconcile_v2_test.go index 58586780ad..e2b0629bcc 100644 --- a/controllers/datadogagent/controller_reconcile_v2_test.go +++ b/controllers/datadogagent/controller_reconcile_v2_test.go @@ -38,12 +38,12 @@ func Test_profilesToApply(t *testing.T) { ctx := context.Background() testCases := []struct { - name string - nodeList []corev1.Node - profileList []client.Object - wantProfilesToApply func() []v1alpha1.DatadogAgentProfile - wantProfileAppliedPerNode map[string]types.NamespacedName - wantError error + name string + nodeList []corev1.Node + profileList []client.Object + wantProfilesToApply func() []v1alpha1.DatadogAgentProfile + wantProfileAppliedByNode map[string]types.NamespacedName + wantError error }{ { name: "no user-created profiles to apply", @@ -69,7 +69,7 @@ func Test_profilesToApply(t *testing.T) { wantProfilesToApply: func() []v1alpha1.DatadogAgentProfile { return []v1alpha1.DatadogAgentProfile{defaultProfile()} }, - wantProfileAppliedPerNode: map[string]types.NamespacedName{ + wantProfileAppliedByNode: map[string]types.NamespacedName{ "node1": { Namespace: "", Name: "default", @@ -86,7 +86,7 @@ func Test_profilesToApply(t *testing.T) { profileList: []client.Object{}, wantProfilesToApply: func() []v1alpha1.DatadogAgentProfile { return []v1alpha1.DatadogAgentProfile{defaultProfile()} - }, wantProfileAppliedPerNode: map[string]types.NamespacedName{}, + }, wantProfileAppliedByNode: map[string]types.NamespacedName{}, }, { name: "no nodes", @@ -112,7 +112,7 @@ func Test_profilesToApply(t *testing.T) { profileList[0].ResourceVersion = "1000" return profileList }, - wantProfileAppliedPerNode: map[string]types.NamespacedName{}, + wantProfileAppliedByNode: map[string]types.NamespacedName{}, }, { name: "one profile", @@ -162,7 +162,7 @@ func Test_profilesToApply(t *testing.T) { profileList[0].ResourceVersion = "1000" return profileList }, - wantProfileAppliedPerNode: map[string]types.NamespacedName{ + wantProfileAppliedByNode: map[string]types.NamespacedName{ "node1": { Namespace: testNamespace, Name: "1", @@ -244,7 +244,7 @@ func Test_profilesToApply(t *testing.T) { profileList[1].ResourceVersion = "1000" return profileList }, - wantProfileAppliedPerNode: map[string]types.NamespacedName{ + wantProfileAppliedByNode: map[string]types.NamespacedName{ "node1": { Namespace: testNamespace, Name: "1", @@ -348,7 +348,7 @@ func Test_profilesToApply(t *testing.T) { return profileList }, - wantProfileAppliedPerNode: map[string]types.NamespacedName{ + wantProfileAppliedByNode: map[string]types.NamespacedName{ "node1": { Namespace: testNamespace, Name: "3", @@ -411,7 +411,7 @@ func Test_profilesToApply(t *testing.T) { profileList[0].ResourceVersion = "1000" return profileList }, - wantProfileAppliedPerNode: map[string]types.NamespacedName{ + wantProfileAppliedByNode: map[string]types.NamespacedName{ "node1": { Namespace: testNamespace, Name: "1", @@ -442,7 +442,7 @@ func Test_profilesToApply(t *testing.T) { wantProfilesToApply: func() []v1alpha1.DatadogAgentProfile { return generateProfileList([]string{}, []time.Time{}) }, - wantProfileAppliedPerNode: map[string]types.NamespacedName{ + wantProfileAppliedByNode: map[string]types.NamespacedName{ "node1": { Namespace: "", Name: "default", @@ -549,7 +549,7 @@ func Test_profilesToApply(t *testing.T) { profileList[1].ResourceVersion = "1000" return profileList }, - wantProfileAppliedPerNode: map[string]types.NamespacedName{ + wantProfileAppliedByNode: map[string]types.NamespacedName{ "node1": { Namespace: testNamespace, Name: "1", @@ -574,13 +574,13 @@ func Test_profilesToApply(t *testing.T) { }, } - profilesToApply, profileAppliedPerNode, err := r.profilesToApply(ctx, logger, tt.nodeList, metav1.NewTime(t1)) + profilesToApply, profileAppliedByNode, err := r.profilesToApply(ctx, logger, tt.nodeList, metav1.NewTime(t1)) require.NoError(t, err) wantProfilesToApply := tt.wantProfilesToApply() assert.Equal(t, wantProfilesToApply, profilesToApply) // assert.ElementsMatch(t, wantProfilesToApply, profilesToApply) - assert.Equal(t, tt.wantProfileAppliedPerNode, profileAppliedPerNode) + assert.Equal(t, tt.wantProfileAppliedByNode, profileAppliedByNode) }) } } diff --git a/pkg/agentprofile/agent_profile.go b/pkg/agentprofile/agent_profile.go index b337e6f702..78f96bade5 100644 --- a/pkg/agentprofile/agent_profile.go +++ b/pkg/agentprofile/agent_profile.go @@ -31,9 +31,8 @@ const ( // ProfileToApply validates a profile spec and returns a map that maps each // node name to the profile that should be applied to it. -func ProfileToApply(logger logr.Logger, profile *datadoghqv1alpha1.DatadogAgentProfile, nodes []v1.Node, profileAppliedPerNode map[string]types.NamespacedName, +func ProfileToApply(logger logr.Logger, profile *datadoghqv1alpha1.DatadogAgentProfile, nodes []v1.Node, profileAppliedByNode map[string]types.NamespacedName, now metav1.Time) (map[string]types.NamespacedName, error) { - conflicts := false nodesThatMatchProfile := map[string]bool{} profileStatus := datadoghqv1alpha1.DatadogAgentProfileStatus{} @@ -48,24 +47,29 @@ func ProfileToApply(logger logr.Logger, profile *datadoghqv1alpha1.DatadogAgentP profileStatus.Conditions = SetDatadogAgentProfileCondition(profileStatus.Conditions, NewDatadogAgentProfileCondition(ValidConditionType, metav1.ConditionFalse, now, InvalidConditionReason, err.Error())) profileStatus.Valid = metav1.ConditionFalse UpdateProfileStatus(profile, profileStatus, now) - return profileAppliedPerNode, err + return profileAppliedByNode, err } for _, node := range nodes { matchesNode, err := profileMatchesNode(profile, node.Labels) if err != nil { + logger.Error(err, "profile selector is invalid, skipping", "name", profile.Name, "namespace", profile.Namespace) profileStatus.Conditions = SetDatadogAgentProfileCondition(profileStatus.Conditions, NewDatadogAgentProfileCondition(ValidConditionType, metav1.ConditionFalse, now, InvalidConditionReason, err.Error())) profileStatus.Valid = metav1.ConditionFalse UpdateProfileStatus(profile, profileStatus, now) - return profileAppliedPerNode, err + return profileAppliedByNode, err } + profileStatus.Valid = metav1.ConditionTrue + profileStatus.Conditions = SetDatadogAgentProfileCondition(profileStatus.Conditions, NewDatadogAgentProfileCondition(ValidConditionType, metav1.ConditionTrue, now, ValidConditionReason, "Valid manifest")) if matchesNode { - if existingProfile, found := profileAppliedPerNode[node.Name]; found { + if existingProfile, found := profileAppliedByNode[node.Name]; found { // Conflict. This profile should not be applied. - conflicts = true logger.Info("conflict with existing profile, skipping", "conflicting profile", profile.Namespace+"/"+profile.Name, "existing profile", existingProfile.String()) - break + profileStatus.Conditions = SetDatadogAgentProfileCondition(profileStatus.Conditions, NewDatadogAgentProfileCondition(AppliedConditionType, metav1.ConditionFalse, now, ConflictConditionReason, "Conflict with existing profile")) + profileStatus.Applied = metav1.ConditionFalse + UpdateProfileStatus(profile, profileStatus, now) + return profileAppliedByNode, fmt.Errorf("conflict with existing profile") } else { nodesThatMatchProfile[node.Name] = true profileStatus.Conditions = SetDatadogAgentProfileCondition(profileStatus.Conditions, NewDatadogAgentProfileCondition(AppliedConditionType, metav1.ConditionTrue, now, AppliedConditionReason, "Profile applied")) @@ -74,18 +78,8 @@ func ProfileToApply(logger logr.Logger, profile *datadoghqv1alpha1.DatadogAgentP } } - profileStatus.Valid = metav1.ConditionTrue - profileStatus.Conditions = SetDatadogAgentProfileCondition(profileStatus.Conditions, NewDatadogAgentProfileCondition(ValidConditionType, metav1.ConditionTrue, now, ValidConditionReason, "Valid manifest")) - - if conflicts { - profileStatus.Conditions = SetDatadogAgentProfileCondition(profileStatus.Conditions, NewDatadogAgentProfileCondition(AppliedConditionType, metav1.ConditionFalse, now, ConflictConditionReason, "Conflict with existing profile")) - profileStatus.Applied = metav1.ConditionFalse - UpdateProfileStatus(profile, profileStatus, now) - return profileAppliedPerNode, fmt.Errorf("conflict with existing profile") - } - for node := range nodesThatMatchProfile { - profileAppliedPerNode[node] = types.NamespacedName{ + profileAppliedByNode[node] = types.NamespacedName{ Namespace: profile.Namespace, Name: profile.Name, } @@ -93,16 +87,16 @@ func ProfileToApply(logger logr.Logger, profile *datadoghqv1alpha1.DatadogAgentP UpdateProfileStatus(profile, profileStatus, now) - return profileAppliedPerNode, nil + return profileAppliedByNode, nil } -func ApplyDefaultProfile(profilesToApply []datadoghqv1alpha1.DatadogAgentProfile, profileAppliedPerNode map[string]types.NamespacedName, nodes []v1.Node) []datadoghqv1alpha1.DatadogAgentProfile { +func ApplyDefaultProfile(profilesToApply []datadoghqv1alpha1.DatadogAgentProfile, profileAppliedByNode map[string]types.NamespacedName, nodes []v1.Node) []datadoghqv1alpha1.DatadogAgentProfile { profilesToApply = append(profilesToApply, defaultProfile()) // Apply the default profile to all nodes that don't have a profile applied for _, node := range nodes { - if _, found := profileAppliedPerNode[node.Name]; !found { - profileAppliedPerNode[node.Name] = types.NamespacedName{ + if _, found := profileAppliedByNode[node.Name]; !found { + profileAppliedByNode[node.Name] = types.NamespacedName{ Name: defaultProfileName, } } diff --git a/pkg/agentprofile/agent_profile_test.go b/pkg/agentprofile/agent_profile_test.go index 9b3efe764c..a569a7038b 100644 --- a/pkg/agentprofile/agent_profile_test.go +++ b/pkg/agentprofile/agent_profile_test.go @@ -31,12 +31,12 @@ func TestProfileToApply(t *testing.T) { name string profile v1alpha1.DatadogAgentProfile nodes []v1.Node - profileAppliedPerNode map[string]types.NamespacedName + profileAppliedByNode map[string]types.NamespacedName expectedProfilesAppliedPerNode map[string]types.NamespacedName expectedErr error }{ { - name: "empty profile, empty profileAppliedPerNode", + name: "empty profile, empty profileAppliedByNode", profile: v1alpha1.DatadogAgentProfile{}, nodes: []v1.Node{ { @@ -48,12 +48,12 @@ func TestProfileToApply(t *testing.T) { }, }, }, - profileAppliedPerNode: map[string]types.NamespacedName{}, + profileAppliedByNode: map[string]types.NamespacedName{}, expectedProfilesAppliedPerNode: map[string]types.NamespacedName{}, expectedErr: fmt.Errorf("profileAffinity must be defined"), }, { - name: "empty profile, non-empty profileAppliedPerNode", + name: "empty profile, non-empty profileAppliedByNode", profile: v1alpha1.DatadogAgentProfile{}, nodes: []v1.Node{ { @@ -65,7 +65,7 @@ func TestProfileToApply(t *testing.T) { }, }, }, - profileAppliedPerNode: map[string]types.NamespacedName{ + profileAppliedByNode: map[string]types.NamespacedName{ "node1": { Namespace: testNamespace, Name: "linux", @@ -80,10 +80,10 @@ func TestProfileToApply(t *testing.T) { expectedErr: fmt.Errorf("profileAffinity must be defined"), }, { - name: "empty profile, , non-empty profileAppliedPerNode, no nodes", + name: "empty profile, , non-empty profileAppliedByNode, no nodes", profile: v1alpha1.DatadogAgentProfile{}, nodes: []v1.Node{}, - profileAppliedPerNode: map[string]types.NamespacedName{ + profileAppliedByNode: map[string]types.NamespacedName{ "node1": { Namespace: testNamespace, Name: "linux", @@ -98,7 +98,7 @@ func TestProfileToApply(t *testing.T) { expectedErr: fmt.Errorf("profileAffinity must be defined"), }, { - name: "non-conflicting profile, empty profileAppliedPerNode", + name: "non-conflicting profile, empty profileAppliedByNode", profile: exampleProfileForLinux(), nodes: []v1.Node{ { @@ -110,7 +110,7 @@ func TestProfileToApply(t *testing.T) { }, }, }, - profileAppliedPerNode: map[string]types.NamespacedName{}, + profileAppliedByNode: map[string]types.NamespacedName{}, expectedProfilesAppliedPerNode: map[string]types.NamespacedName{ "node1": { Namespace: testNamespace, @@ -120,7 +120,7 @@ func TestProfileToApply(t *testing.T) { expectedErr: nil, }, { - name: "non-conflicting profile, non-empty profileAppliedPerNode", + name: "non-conflicting profile, non-empty profileAppliedByNode", profile: exampleProfileForLinux(), nodes: []v1.Node{ { @@ -140,7 +140,7 @@ func TestProfileToApply(t *testing.T) { }, }, }, - profileAppliedPerNode: map[string]types.NamespacedName{ + profileAppliedByNode: map[string]types.NamespacedName{ "node2": { Namespace: testNamespace, Name: "windows", @@ -159,10 +159,10 @@ func TestProfileToApply(t *testing.T) { expectedErr: nil, }, { - name: "non-conflicting profile, non-empty profileAppliedPerNode, no nodes", + name: "non-conflicting profile, non-empty profileAppliedByNode, no nodes", profile: exampleProfileForLinux(), nodes: []v1.Node{}, - profileAppliedPerNode: map[string]types.NamespacedName{ + profileAppliedByNode: map[string]types.NamespacedName{ "node2": { Namespace: testNamespace, Name: "windows", @@ -197,7 +197,7 @@ func TestProfileToApply(t *testing.T) { }, }, }, - profileAppliedPerNode: map[string]types.NamespacedName{ + profileAppliedByNode: map[string]types.NamespacedName{ "node1": { Namespace: testNamespace, Name: "linux", @@ -231,7 +231,7 @@ func TestProfileToApply(t *testing.T) { }, }, }, - profileAppliedPerNode: map[string]types.NamespacedName{ + profileAppliedByNode: map[string]types.NamespacedName{ "node1": { Namespace: testNamespace, Name: "linux", @@ -250,9 +250,9 @@ func TestProfileToApply(t *testing.T) { t.Run(test.name, func(t *testing.T) { testLogger := zap.New(zap.UseDevMode(true)) now := metav1.NewTime(time.Now()) - profileAppliedPerNode, err := ProfileToApply(testLogger, &test.profile, test.nodes, test.profileAppliedPerNode, now) + profileAppliedByNode, err := ProfileToApply(testLogger, &test.profile, test.nodes, test.profileAppliedByNode, now) assert.Equal(t, test.expectedErr, err) - assert.Equal(t, test.expectedProfilesAppliedPerNode, profileAppliedPerNode) + assert.Equal(t, test.expectedProfilesAppliedPerNode, profileAppliedByNode) }) } }