From b6c0aa69e573e4353b7312996423a8cddc335339 Mon Sep 17 00:00:00 2001
From: Quan Tian <qtian@vmware.com>
Date: Wed, 7 Jun 2023 20:16:22 +0800
Subject: [PATCH] Fix status report when Antrea-native policies are made no-op
 change

Signed-off-by: Quan Tian <qtian@vmware.com>
---
 pkg/agent/controller/networkpolicy/cache.go        |  8 +++++---
 .../networkpolicy/networkpolicy_controller.go      |  9 ++++-----
 test/e2e/antreapolicy_test.go                      | 14 +++++++++++++-
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/pkg/agent/controller/networkpolicy/cache.go b/pkg/agent/controller/networkpolicy/cache.go
index 7665e88462d..f61fcd227f8 100644
--- a/pkg/agent/controller/networkpolicy/cache.go
+++ b/pkg/agent/controller/networkpolicy/cache.go
@@ -758,7 +758,7 @@ func (c *ruleCache) AddNetworkPolicy(policy *v1beta.NetworkPolicy) {
 	c.updateNetworkPolicyLocked(policy)
 }
 
-// UpdateNetworkPolicy updates a cached *v1beta.NetworkPolicy and returns whether there is any rule change.
+// UpdateNetworkPolicy updates a cached *v1beta.NetworkPolicy and returns whether any rule or the generation changes.
 // The added rules and removed rules will be regarded as dirty.
 func (c *ruleCache) UpdateNetworkPolicy(policy *v1beta.NetworkPolicy) bool {
 	c.policyMapLock.Lock()
@@ -766,8 +766,10 @@ func (c *ruleCache) UpdateNetworkPolicy(policy *v1beta.NetworkPolicy) bool {
 	return c.updateNetworkPolicyLocked(policy)
 }
 
-// updateNetworkPolicyLocked returns whether there is any rule change.
+// updateNetworkPolicyLocked returns whether any rule or the generation changes.
 func (c *ruleCache) updateNetworkPolicyLocked(policy *v1beta.NetworkPolicy) bool {
+	oldPolicy, exists := c.policyMap[string(policy.UID)]
+	generationUpdated := !exists || oldPolicy.Generation != policy.Generation
 	c.policyMap[string(policy.UID)] = policy
 	existingRules, _ := c.rules.ByIndex(policyIndex, string(policy.UID))
 	ruleByID := map[string]interface{}{}
@@ -810,7 +812,7 @@ func (c *ruleCache) updateNetworkPolicyLocked(policy *v1beta.NetworkPolicy) bool
 		c.dirtyRuleHandler(ruleID)
 		anyRuleUpdate = true
 	}
-	return anyRuleUpdate
+	return anyRuleUpdate || generationUpdated
 }
 
 // DeleteNetworkPolicy deletes a cached *v1beta.NetworkPolicy.
diff --git a/pkg/agent/controller/networkpolicy/networkpolicy_controller.go b/pkg/agent/controller/networkpolicy/networkpolicy_controller.go
index 8cad7f77d7c..c65f69b8c55 100644
--- a/pkg/agent/controller/networkpolicy/networkpolicy_controller.go
+++ b/pkg/agent/controller/networkpolicy/networkpolicy_controller.go
@@ -252,11 +252,10 @@ func NewNetworkPolicyController(antreaClientGetter agent.AntreaClientProvider,
 					policy.SourceRef.ToString())
 				return nil
 			}
-			anyRuleUpdate := c.ruleCache.UpdateNetworkPolicy(policy)
-			// If there is any rule update, we ensure statusManager will resync the policy's status once, in case that
-			// the added/deleted/updated rule is not effective, in which case the rule's realization status is not
-			// changed but the whole policy's generation is changed.
-			if c.statusManagerEnabled && anyRuleUpdate && policy.SourceRef.Type != v1beta2.K8sNetworkPolicy {
+			updated := c.ruleCache.UpdateNetworkPolicy(policy)
+			// If any rule or the generation changes, we ensure statusManager will resync the policy's status once, in
+			// case the changes don't cause any actual rule update but the whole policy's generation is changed.
+			if c.statusManagerEnabled && updated && policy.SourceRef.Type != v1beta2.K8sNetworkPolicy {
 				c.statusManager.Resync(policy.UID)
 			}
 			return nil
diff --git a/test/e2e/antreapolicy_test.go b/test/e2e/antreapolicy_test.go
index 53e551c57bc..ed76db89ee8 100644
--- a/test/e2e/antreapolicy_test.go
+++ b/test/e2e/antreapolicy_test.go
@@ -4560,13 +4560,25 @@ func TestAntreaPolicyStatusWithAppliedToPerRule(t *testing.T) {
 	anp.Spec.Ingress = anp.Spec.Ingress[0:1]
 	_, err = data.crdClient.CrdV1alpha1().NetworkPolicies(anp.Namespace).Update(context.TODO(), anp, metav1.UpdateOptions{})
 	assert.NoError(t, err)
-	checkANPStatus(t, data, anp, crdv1alpha1.NetworkPolicyStatus{
+	anp = checkANPStatus(t, data, anp, crdv1alpha1.NetworkPolicyStatus{
 		Phase:                crdv1alpha1.NetworkPolicyRealized,
 		ObservedGeneration:   2,
 		CurrentNodesRealized: 1,
 		DesiredNodesRealized: 1,
 		Conditions:           networkpolicy.GenerateNetworkPolicyCondition(nil),
 	})
+
+	// Add a non-existing group.
+	anp.Spec.Ingress[0].AppliedTo = append(anp.Spec.Ingress[0].AppliedTo, crdv1alpha1.AppliedTo{Group: "foo"})
+	_, err = data.crdClient.CrdV1alpha1().NetworkPolicies(anp.Namespace).Update(context.TODO(), anp, metav1.UpdateOptions{})
+	assert.NoError(t, err)
+	checkANPStatus(t, data, anp, crdv1alpha1.NetworkPolicyStatus{
+		Phase:                crdv1alpha1.NetworkPolicyRealized,
+		ObservedGeneration:   3,
+		CurrentNodesRealized: 1,
+		DesiredNodesRealized: 1,
+		Conditions:           networkpolicy.GenerateNetworkPolicyCondition(nil),
+	})
 }
 
 func TestAntreaPolicyStatusWithAppliedToUnsupportedGroup(t *testing.T) {