Fix status report when no-op changes are applied to Antrea-native pol…

…icies (#5096)

When no-op changes are applied to Antrea-native policies, such as adding
or removing a non-existing group, there will be no datapath change but
the generation has changed. The agent must report it has realized the
latest generation even if it does nothing, otherwise the policy would
stay realizing.

Signed-off-by: Quan Tian <qtian@vmware.com>

pkg/agent/controller/networkpolicy/cache.go

@@ -758,16 +758,18 @@ 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()
 	defer c.policyMapLock.Unlock()
 	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.

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

test/e2e/antreapolicy_test.go

@@ -4560,13 +4560,39 @@ 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.
+	// Although nothing will be changed in datapath, the policy's status should be realized with the latest generation.
+	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)
+	anp = checkANPStatus(t, data, anp, crdv1alpha1.NetworkPolicyStatus{
+		Phase:                crdv1alpha1.NetworkPolicyRealized,
+		ObservedGeneration:   3,
+		CurrentNodesRealized: 1,
+		DesiredNodesRealized: 1,
+		Conditions:           networkpolicy.GenerateNetworkPolicyCondition(nil),
+	})
+
+	// Delete the non-existing group.
+	// Although nothing will be changed in datapath, the policy's status should be realized with the latest generation.
+	anp.Spec.Ingress[0].AppliedTo = anp.Spec.Ingress[0].AppliedTo[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{
+		Phase:                crdv1alpha1.NetworkPolicyRealized,
+		ObservedGeneration:   4,
+		CurrentNodesRealized: 1,
+		DesiredNodesRealized: 1,
+		Conditions:           networkpolicy.GenerateNetworkPolicyCondition(nil),
+	})
 }
 
 func TestAntreaPolicyStatusWithAppliedToUnsupportedGroup(t *testing.T) {