Antrea Network Policy Audit Logging Deduplication and Unit Tests #2294
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #2294 +/- ##
==========================================
+ Coverage 60.09% 65.04% +4.95%
==========================================
Files 281 282 +1
Lines 22243 25545 +3302
==========================================
+ Hits 13366 16617 +3251
+ Misses 7467 7393 -74
- Partials 1410 1535 +125
Flags with carried forward coverage won't be shown. Click here to find out more.
|
GraysonWu
reviewed
Jun 22, 2021
antoninbas
reviewed
Jun 23, 2021
qiyueyao
force-pushed
the
LogDedup
branch
2 times, most recently
from
8628df6
to
7fbbd19
Compare
antoninbas
reviewed
Jun 28, 2021
qiyueyao
changed the title
qiyueyao
force-pushed
the
LogDedup
branch
3 times, most recently
from
23ced17
to
903e38d
Compare
qiyueyao
changed the title
Dyanngg
reviewed
Jul 9, 2021
qiyueyao
force-pushed
the
LogDedup
branch
2 times, most recently
from
f017a21
to
34eb14e
Compare
|
/test-all |
abhiraut
previously approved these changes
Aug 10, 2021
Signed-off-by: Qiyue Yao <yaoq@vmware.com>
Signed-off-by: Qiyue Yao <yaoq@vmware.com>
Signed-off-by: Qiyue Yao <yaoq@vmware.com>
Signed-off-by: Qiyue Yao <yaoq@vmware.com>
Signed-off-by: Qiyue Yao <yaoq@vmware.com>
Signed-off-by: Qiyue Yao <yaoq@vmware.com>
Signed-off-by: Qiyue Yao <yaoq@vmware.com>
Signed-off-by: Qiyue Yao <yaoq@vmware.com>
Signed-off-by: Qiyue Yao <yaoq@vmware.com>
abhiraut
approved these changes
Aug 10, 2021
antoninbas
approved these changes
Aug 11, 2021
|
/test-all |
tnqn
reviewed
Aug 11, 2021
| anpLogger: log.New(logOutput, "", log.Ldate|log.Lmicroseconds), | ||
| logDeduplication: logRecordDedupMap{logMap: make(map[string]*logDedupRecord)}, | ||
| } | ||
| klog.V(2).Infof("Initialized Antrea-native Policy Logger for audit logging with log file '%s'", logFile) |
There was a problem hiding this comment.
If you are going to make a change, better to change this logging to structured logging too.
| actual := <-mockAnpLogger.logged | ||
| assert.Contains(t, actual, expectedLogWithCount(expected, 9)) | ||
| actual = <-mockAnpLogger.logged | ||
| assert.Contains(t, actual, expected) |
There was a problem hiding this comment.
The test requires the clock and go routine scheduling works precisely. I think it may be flaky when the testbed is overloaded, could you try go test -count 100 to see if it's stable?
|
Unrelated Kind test failure (in TestFlowAggregator/IPv4/IntraNodeFlows). @srikartati FYI |
@dreamtalen added the pod label test. Could you take a look at this? |
Sure, I'm trying to reproduce it locally. |
annakhm
pushed a commit
to annakhm/antrea
that referenced
this pull request
Aug 16, 2021
…rea-io#2294) Added log deduplication for Antrea Network Policy Audit Logging. - Directly log if disposition is Allow, use 1 sec buffer otherwise. - For each log, create an entry in the global map, which includes a 1 sec timer and counter. - Increase count upon receiving a duplication log. - Write to new log when timer stops, include [count, duration] for the log additionally. example: `2021/06/24 23:56:41.346165 AntreaPolicyIngressRule AntreaNetworkPolicy:default/test-anp2 Drop 44900 SRC: 10.10.2.5 DEST: 10.10.1.10 84 ICMP [2 packets in 1.011379442s]` Changed Controller structure. - Removed `loggingEnabled`, which was set to always equal to `antreaPolicyEnabled`. - Added `AntreaPolicyLogger`, used for audit logging. Added unit tests for Antrea Network Policy Audit Logging. - Allow: directly log once. - Drop: log once, no additional log info. - DedupDrop: log once, include duplicate log info. - Multi DedupDrop: two different logs, includes duplicate log info.
antoninbas
pushed a commit
that referenced
this pull request
Aug 20, 2021
…t Tests (#2578) This is a follow up PR for [Antrea Network Policy Audit Logging Deduplication and Unit Tests](#2294) to address comments. - Converted back to include both `antreaPolicyEnabled` and `loggingEnabled` for extensible usage. - Changed `AntreaPolicyLogger` method naming. - Moved `bufferLength` to a member of `AntreaPolicyLogger` to avoid confusion. Signed-off-by: Qiyue Yao <yaoq@vmware.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added log deduplication for Antrea Network Policy Audit Logging.
example:
2021/06/24 23:56:41.346165 AntreaPolicyIngressRule AntreaNetworkPolicy:default/test-anp2 Drop 44900 SRC: 10.10.2.5 DEST: 10.10.1.10 84 ICMP [2 packets in 1.011379442s]Changed Controller structure.
loggingEnabled, which was set to always equal toantreaPolicyEnabled.AntreaPolicyLogger, used for audit logging.Added unit tests for Antrea Network Policy Audit Logging.
Testing:
ping -i 0.2 <ip-address>