From e97c23de133c9b341f33c6b805c4a51cf9370ccf Mon Sep 17 00:00:00 2001
From: Qiyue Yao <39061776+qiyueyao@users.noreply.github.com>
Date: Mon, 15 Aug 2022 00:58:26 -0700
Subject: [PATCH] Improve doc and test for Audit Logging (#4099)

Addresses followup comments in Audit Logging supports K8s Networkpolicy.
- Improved documentation
- Improved e2e test

Signed-off-by: Qiyue Yao <yaoq@vmware.com>
---
 docs/antrea-network-policy.md | 7 ++-----
 test/e2e/antreapolicy_test.go | 6 +++---
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/docs/antrea-network-policy.md b/docs/antrea-network-policy.md
index 8751363ab48..e6d2f89f65d 100644
--- a/docs/antrea-network-policy.md
+++ b/docs/antrea-network-policy.md
@@ -644,8 +644,8 @@ which the rule is applied. These log files can then be retrieved for further
 analysis. By default, rules are not logged. The example policy logs all
 traffic that matches the "DropToThirdParty" egress rule, while the rule
 "AllowFromFrontend" is not logged. Specifically for drop and reject rules,
-deduplication is applied to simplify multiple logs. Duplication buffer length is set as 1 second.
-The rules are logged in the following format:
+deduplication is applied to reduce duplicated logs, and duplication buffer
+length is set to 1 second. The rules are logged in the following format:
 
 ```text
     <yyyy/mm/dd> <time> <ovs-table-name> <antrea-native-policy-reference> <action> <openflow-priority> <source-ip> <source-port> <destination-ip> <destination-port> <protocol> <packet-length>
@@ -664,9 +664,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, users should not update Namespace
-logging Annotations, otherwise it would risk NetworkPolicies working in a stale state.
 The rules are logged in the following format:
 
 ```text
diff --git a/test/e2e/antreapolicy_test.go b/test/e2e/antreapolicy_test.go
index 8a2ca5f3c52..0f05d780d08 100644
--- a/test/e2e/antreapolicy_test.go
+++ b/test/e2e/antreapolicy_test.go
@@ -2646,7 +2646,7 @@ func testAuditLoggingBasic(t *testing.T, data *TestData) {
 // testAuditLoggingEnableNP tests that audit logs are generated when K8s NP is applied
 // tests both Allow traffic by K8s NP and Drop traffic by implicit K8s policy drop
 func testAuditLoggingEnableNP(t *testing.T, data *TestData) {
-	data.updateNamespaceWithAnnotations(namespaces["x"], map[string]string{networkpolicy.EnableNPLoggingAnnotationKey: "true"})
+	failOnError(data.updateNamespaceWithAnnotations(namespaces["x"], map[string]string{networkpolicy.EnableNPLoggingAnnotationKey: "true"}), t)
 	// Add a K8s namespaced NetworkPolicy in ns x that allow ingress traffic from
 	// Pod x/b to x/a which default denies other ingress including from Pod x/c to x/a
 	k8sNPBuilder := &NetworkPolicySpecBuilder{}
@@ -2731,9 +2731,9 @@ func testAuditLoggingEnableNP(t *testing.T, data *TestData) {
 		t.Errorf("Error when polling audit log files for required entries: %v", err)
 	}
 	failOnError(k8sUtils.DeleteNetworkPolicy(namespaces["x"], "allow-x-b-to-x-a"), t)
-	data.UpdateNamespace(namespaces["x"], func(namespace *v1.Namespace) {
+	failOnError(data.UpdateNamespace(namespaces["x"], func(namespace *v1.Namespace) {
 		delete(namespace.Annotations, networkpolicy.EnableNPLoggingAnnotationKey)
-	})
+	}), t)
 }
 
 func testAppliedToPerRule(t *testing.T) {