From 349374363e26af2b52c20ad877639e52de2145ea Mon Sep 17 00:00:00 2001
From: Qiyue Yao <yaoq@vmware.com>
Date: Mon, 8 Aug 2022 15:33:42 -0700
Subject: [PATCH] address comments UPDATE change

Signed-off-by: Qiyue Yao <yaoq@vmware.com>
---
 docs/antrea-network-policy.md                 |  2 --
 pkg/agent/openflow/client_test.go             |  2 +-
 pkg/agent/openflow/network_policy.go          | 25 ++++++++++++++-----
 .../networkpolicy/clusternetworkpolicy.go     |  7 +++++-
 4 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/docs/antrea-network-policy.md b/docs/antrea-network-policy.md
index cd6845f3308..9efb0e885b9 100644
--- a/docs/antrea-network-policy.md
+++ b/docs/antrea-network-policy.md
@@ -659,8 +659,6 @@ for all NetworkPolicies in the Namespace. Packets of any connection that match
 a NetworkPolicy rule will be logged with a reference to the NetworkPolicy name,
 but packets dropped by the implicit "default drop" (not allowed by any NetworkPolicy)
 will only be logged with consistent name `K8sNetworkPolicy` for reference.
-Note that currently, Antrea only retrieves the logging Annotation once when adding
-NetworkPolicies and in case of agent restart, it does not monitor Annotation updates.
 The rules are logged in the following format:
 
 ```text
diff --git a/pkg/agent/openflow/client_test.go b/pkg/agent/openflow/client_test.go
index d698ab6fe61..b3043a52b77 100644
--- a/pkg/agent/openflow/client_test.go
+++ b/pkg/agent/openflow/client_test.go
@@ -437,7 +437,7 @@ func prepareTraceflowFlow(ctrl *gomock.Controller) *client {
 	c.bridge = ovsoftest.NewMockBridge(ctrl)
 
 	mFlow := ovsoftest.NewMockFlow(ctrl)
-	ctx := &conjMatchFlowContext{dropFlow: mFlow}
+	ctx := &conjMatchFlowContext{dropFlow: mFlow, dropFlowEnableLogging: false}
 	mFlow.EXPECT().FlowProtocol().Return(binding.Protocol("ip"))
 	mFlow.EXPECT().CopyToBuilder(priorityNormal+2, false).Return(EgressDefaultTable.ofTable.BuildFlow(priorityNormal + 2)).Times(1)
 	c.featureNetworkPolicy.globalConjMatchFlowCache["mockContext"] = ctx
diff --git a/pkg/agent/openflow/network_policy.go b/pkg/agent/openflow/network_policy.go
index b42651f80e3..489a956dc67 100644
--- a/pkg/agent/openflow/network_policy.go
+++ b/pkg/agent/openflow/network_policy.go
@@ -422,6 +422,8 @@ type conjMatchFlowContext struct {
 	// empty, and uninstalled when both two are empty. When the dropFlow is uninstalled from the switch, the
 	// conjMatchFlowContext is removed from the cache.
 	dropFlow binding.Flow
+	// dropFlowEnableLogging describes the logging requirement of the dropFlow.
+	dropFlowEnableLogging bool
 }
 
 // createOrUpdateConjunctiveMatchFlow creates or updates the conjunctive match flow with the latest actions. It returns
@@ -718,9 +720,10 @@ func (c *clause) addConjunctiveMatchFlow(featureNetworkPolicy *featureNetworkPol
 	context, found = featureNetworkPolicy.globalConjMatchFlowCache[matcherKey]
 	if !found {
 		context = &conjMatchFlowContext{
-			conjunctiveMatch:     match,
-			actions:              make(map[uint32]*conjunctiveAction),
-			featureNetworkPolicy: featureNetworkPolicy,
+			conjunctiveMatch:      match,
+			actions:               make(map[uint32]*conjunctiveAction),
+			featureNetworkPolicy:  featureNetworkPolicy,
+			dropFlowEnableLogging: enableLogging,
 		}
 		ctxType = insertion
 
@@ -731,6 +734,15 @@ func (c *clause) addConjunctiveMatchFlow(featureNetworkPolicy *featureNetworkPol
 				changeType: insertion,
 			}
 		}
+	} else if context.dropFlowEnableLogging != enableLogging {
+		// Logging requirement of the rule has changed, modify default drop flow accordingly.
+		context.dropFlowEnableLogging = enableLogging
+		if c.dropTable != nil && context.dropFlow != nil {
+			dropFlow = &flowChange{
+				flow:       context.featureNetworkPolicy.defaultDropFlow(c.dropTable, match.matchPairs, enableLogging),
+				changeType: modification,
+			}
+		}
 	}
 
 	// Calculate the change on the conjMatchFlowContext.
@@ -1178,9 +1190,10 @@ func (f *featureNetworkPolicy) addActionToConjunctiveMatch(clause *clause, match
 	context, found = f.globalConjMatchFlowCache[matcherKey]
 	if !found {
 		context = &conjMatchFlowContext{
-			conjunctiveMatch:     match,
-			actions:              make(map[uint32]*conjunctiveAction),
-			featureNetworkPolicy: f,
+			conjunctiveMatch:      match,
+			actions:               make(map[uint32]*conjunctiveAction),
+			featureNetworkPolicy:  f,
+			dropFlowEnableLogging: enableLogging,
 		}
 		// Generate the default drop flow if dropTable is not nil.
 		if clause.dropTable != nil {
diff --git a/pkg/controller/networkpolicy/clusternetworkpolicy.go b/pkg/controller/networkpolicy/clusternetworkpolicy.go
index 6062d2283c3..0a2cdb9b13e 100644
--- a/pkg/controller/networkpolicy/clusternetworkpolicy.go
+++ b/pkg/controller/networkpolicy/clusternetworkpolicy.go
@@ -227,7 +227,12 @@ func (n *NetworkPolicyController) updateNamespace(oldObj, curObj interface{}) {
 			n.reprocessCNP(cnp, false)
 		}
 	}
-	// TODO: Reprocess K8s NetworkPolicies when logging annotation has changed.
+	if oldNamespace.Annotations[EnableNPLoggingAnnotationKey] != curNamespace.Annotations[EnableNPLoggingAnnotationKey] {
+		affectedNPs, _ := n.networkPolicyLister.NetworkPolicies(curNamespace.Name).List(labels.Everything())
+		for _, np := range affectedNPs {
+			n.updateNetworkPolicy(np, np)
+		}
+	}
 }
 
 // deleteNamespace receives Namespace DELETE events and triggers all ClusterNetworkPolicies that have a