From 6128753bbe701cf4c56a24fb8edc515beb1801be Mon Sep 17 00:00:00 2001
From: Dyanngg <dingyang@vmware.com>
Date: Mon, 22 Jan 2024 01:02:38 -0800
Subject: [PATCH] Fix redundant reconciles in FQDN controller (#5893)

Signed-off-by: Dyanngg <dingyang@vmware.com>
---
 pkg/agent/controller/networkpolicy/fqdn.go    | 20 ++++++++++---------
 .../controller/networkpolicy/fqdn_test.go     | 18 +++++++++++++++++
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/pkg/agent/controller/networkpolicy/fqdn.go b/pkg/agent/controller/networkpolicy/fqdn.go
index 882eb2fa1c1..84d58d9eec6 100644
--- a/pkg/agent/controller/networkpolicy/fqdn.go
+++ b/pkg/agent/controller/networkpolicy/fqdn.go
@@ -482,20 +482,22 @@ func (f *fqdnController) onDNSResponseMsg(dnsMsg *dns.Msg, lookupTime time.Time,
 // wait for another attempt of realization of these rules, before forwarding the response to the
 // original client.
 func (f *fqdnController) syncDirtyRules(fqdn string, waitCh chan error, addressUpdate bool) {
+	if waitCh == nil && !addressUpdate {
+		// No dirty rules to sync
+		return
+	}
+	dirtyRules := sets.New[string]()
+	for selectorItem := range f.fqdnToSelectorItem[fqdn] {
+		utilsets.MergeString(dirtyRules, f.selectorItemToRuleIDs[selectorItem])
+	}
 	if waitCh == nil {
 		if addressUpdate {
-			for selectorItem := range f.fqdnToSelectorItem[fqdn] {
-				for ruleID := range f.selectorItemToRuleIDs[selectorItem] {
-					klog.V(4).InfoS("Reconciling dirty rule", "ruleID", ruleID)
-					f.dirtyRuleHandler(ruleID)
-				}
+			for ruleID := range dirtyRules {
+				klog.V(4).InfoS("Reconciling dirty rule for FQDN address updates", "ruleID", ruleID)
+				f.dirtyRuleHandler(ruleID)
 			}
 		}
 	} else {
-		dirtyRules := sets.New[string]()
-		for selectorItem := range f.fqdnToSelectorItem[fqdn] {
-			utilsets.MergeString(dirtyRules, f.selectorItemToRuleIDs[selectorItem])
-		}
 		if !addressUpdate {
 			// If there is no address update for this FQDN, and rules selecting this FQDN
 			// were all previously realized successfully, then there will be no dirty rules
diff --git a/pkg/agent/controller/networkpolicy/fqdn_test.go b/pkg/agent/controller/networkpolicy/fqdn_test.go
index 4a2e762f286..d2ffeaa74b6 100644
--- a/pkg/agent/controller/networkpolicy/fqdn_test.go
+++ b/pkg/agent/controller/networkpolicy/fqdn_test.go
@@ -395,6 +395,10 @@ func TestSyncDirtyRules(t *testing.T) {
 	selectorItem2 := fqdnSelectorItem{
 		matchName: testFQDN2,
 	}
+	testFQDN3 := "*antrea.io"
+	selectorItem3 := fqdnSelectorItem{
+		matchRegex: testFQDN3,
+	}
 	tests := []struct {
 		name                        string
 		fqdnsToSync                 []string
@@ -406,6 +410,17 @@ func TestSyncDirtyRules(t *testing.T) {
 		expectedDirtyRulesRemaining sets.Set[string]
 		expectErr                   bool
 	}{
+		{
+			name:                        "test non-blocking dirty rule sync without address update",
+			fqdnsToSync:                 []string{testFQDN},
+			prevDirtyRules:              sets.New[string](),
+			addressUpdates:              []bool{false},
+			waitChs:                     []chan error{nil},
+			notifications:               []ruleRealizationUpdate{},
+			expectedDirtyRuleSyncCalls:  []string{},
+			expectedDirtyRulesRemaining: sets.New[string](),
+			expectErr:                   false,
+		},
 		{
 			name:                        "test non-blocking dirty rule sync with address update",
 			fqdnsToSync:                 []string{testFQDN},
@@ -482,10 +497,13 @@ func TestSyncDirtyRules(t *testing.T) {
 				dirtyRuleSyncCalls = append(dirtyRuleSyncCalls, s)
 			}
 			f.addFQDNSelector("1", []string{testFQDN})
+			f.addFQDNSelector("1", []string{testFQDN3})
 			f.addFQDNSelector("2", []string{testFQDN})
 			f.addFQDNSelector("2", []string{testFQDN2})
 			f.setFQDNMatchSelector(testFQDN, selectorItem)
 			f.setFQDNMatchSelector(testFQDN2, selectorItem2)
+			f.setFQDNMatchSelector(testFQDN, selectorItem3)
+			f.setFQDNMatchSelector(testFQDN2, selectorItem3)
 			// This simulates failed rule syncs in previous syncDirtyRules() calls
 			if len(tc.prevDirtyRules) > 0 {
 				f.ruleSyncTracker.dirtyRules = tc.prevDirtyRules