Support antctl in the flow aggregator #2878

yanjunz97 · 2021-10-08T23:03:23Z

This commit supports antctl in the flow aggregator pod. It supports three commands only running locally inside the flow-aggregator container.

Showing or changing log verbosity level: The antctl get log-level command works the same as the one in antrea-agent or antrea-controller.
Dumping flow records: The antctl get flowrecords [-o json] command can dump all matched flow records. The command provides a compact display of the flow records in the default table output format. Using the json or yaml antctl output format can print more information of the flow records. The command supports the 5-tuple flow key or a subset of 5-tuples as a filter. A 5-tuple flow key contains Source IP, Destination IP, Source Port, Destination Port and Transport Protocol. If the filter is empty, all flow records will be dumped.
Example outputs of dumping flow records:

$ antctl get flowrecords --srcip 10.10.0.1 --srcport 50497 -o json
[
  {
    "destinationClusterIPv4": "0.0.0.0",
    "destinationIPv4Address": "10.10.1.2",
    "destinationNodeName": "k8s-node-worker-1",
    "destinationPodName": "coredns-78fcd69978-x2twv",
    "destinationPodNamespace": "kube-system",
    "destinationServicePort": 0,
    "destinationServicePortName": "",
    "destinationTransportPort": 53,
    "egressNetworkPolicyName": "",
    "egressNetworkPolicyNamespace": "",
    "egressNetworkPolicyRuleAction": 0,
    "egressNetworkPolicyRuleName": "",
    "egressNetworkPolicyType": 0,
    "flowEndReason": 3,
    "flowEndSeconds": 1635546893,
    "flowStartSeconds": 1635546867,
    "flowType": 2,
    "ingressNetworkPolicyName": "",
    "ingressNetworkPolicyNamespace": "",
    "ingressNetworkPolicyRuleAction": 0,
    "ingressNetworkPolicyRuleName": "",
    "ingressNetworkPolicyType": 0,
    "octetDeltaCount": 99,
    "octetDeltaCountFromDestinationNode": 99,
    "octetDeltaCountFromSourceNode": 0,
    "octetTotalCount": 99,
    "octetTotalCountFromDestinationNode": 99,
    "octetTotalCountFromSourceNode": 0,
    "packetDeltaCount": 1,
    "packetDeltaCountFromDestinationNode": 1,
    "packetDeltaCountFromSourceNode": 0,
    "packetTotalCount": 1,
    "packetTotalCountFromDestinationNode": 1,
    "packetTotalCountFromSourceNode": 0,
    "protocolIdentifier": 17,
    "reverseOctetDeltaCount": 192,
    "reverseOctetDeltaCountFromDestinationNode": 192,
    "reverseOctetDeltaCountFromSourceNode": 0,
    "reverseOctetTotalCount": 192,
    "reverseOctetTotalCountFromDestinationNode": 192,
    "reverseOctetTotalCountFromSourceNode": 0,
    "reversePacketDeltaCount": 1,
    "reversePacketDeltaCountFromDestinationNode": 1,
    "reversePacketDeltaCountFromSourceNode": 0,
    "reversePacketTotalCount": 1,
    "reversePacketTotalCountFromDestinationNode": 1,
    "reversePacketTotalCountFromSourceNode": 0,
    "sourceIPv4Address": "10.10.0.1",
    "sourceNodeName": "",
    "sourcePodName": "",
    "sourcePodNamespace": "",
    "sourceTransportPort": 50497,
    "tcpState": ""
  }
]

Flow records metrics: The antctl get recordmetrics command can print all metrics related to the Flow Aggregator. The metrics include number of records received by the collector process in the Flow Aggregator, number of records exported by the Flow Aggregator, number of active flows that are being tracked, number of exporters connected to the Flow Aggregator

Fixes: #1966

codecov-commenter · 2021-10-08T23:51:04Z

Codecov Report

Merging #2878 (7c797d5) into main (ca264d3) will decrease coverage by 0.15%.
The diff coverage is 39.21%.

@@            Coverage Diff             @@
##             main    #2878      +/-   ##
==========================================
- Coverage   61.06%   60.91%   -0.16%     
==========================================
  Files         289      292       +3     
  Lines       24548    24698     +150     
==========================================
+ Hits        14991    15045      +54     
- Misses       7938     8015      +77     
- Partials     1619     1638      +19

Flag	Coverage Δ
kind-e2e-tests	`48.16% <45.66%> (-0.03%)`	⬇️
unit-tests	`40.14% <6.25%> (-0.07%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/antctl/antctl.go	`66.66% <ø> (ø)`
pkg/antctl/client.go	`1.29% <0.00%> (-0.10%)`	⬇️
pkg/antctl/command_definition.go	`51.97% <0.00%> (-2.01%)`	⬇️
pkg/antctl/transform/common/transform.go	`0.00% <0.00%> (ø)`
...gregator/apiserver/handlers/flowrecords/handler.go	`23.63% <23.63%> (ø)`
...egator/apiserver/handlers/recordmetrics/handler.go	`44.44% <44.44%> (ø)`
pkg/antctl/command_list.go	`47.69% <50.00%> (+0.07%)`	⬆️
pkg/flowaggregator/apiserver/apiserver.go	`65.21% <65.21%> (ø)`
pkg/flowaggregator/flowaggregator.go	`68.19% <100.00%> (+0.63%)`	⬆️
pkg/ipfix/ipfix_intermediate.go	`89.47% <100.00%> (+0.58%)`	⬆️
... and 10 more

lgtm-com · 2021-10-11T22:22:03Z

This pull request introduces 2 alerts when merging 579433b into 4d0eea7 - view on LGTM.com

new alerts:

2 for Incorrect conversion between integer types

dreamtalen

Thanks Yanjun working on this.

docs/antctl.md

pkg/antctl/antctl.go

pkg/flowaggregator/flowaggregator.go

pkg/antctl/antctl.go

test/e2e/antctl_test.go

build/yamls/flow-aggregator.yml

docs/antctl.md

srikartati · 2021-10-15T00:31:21Z

docs/antctl.md

-verbosity level of Antrea Controller or Agent using the `antctl log-level`
-command. The command can only run locally inside the `antrea-controller` or
-`antrea-agent` container.
+verbosity level of Antrea Controller, Antrea Agent or Flow Aggregator using the


Add this separately for Flow Aggregator, as the support version number is different

Updated. Use 1.4 for now, if it does not catch up the release 1.4, will update to later version.

docs/troubleshooting.md

pkg/apis/ports.go

docs/antctl.md

pkg/antctl/antctl.go

yanjunz97 · 2021-11-02T17:29:30Z

@antoninbas @srikartati Would you like to take another look at this PR before I upgrade go-ipfix to a new version? Thanks a lot!

srikartati

It looks good to me @yanjunz97

docs/antctl.md

docs/network-flow-visibility.md

pkg/antctl/antctl.go

test/e2e/antctl_test.go

antoninbas · 2021-11-02T19:37:12Z

test/e2e/antctl_test.go

+func TestAntctlFlowAggregator(t *testing.T) {
+	skipIfHasWindowsNodes(t)
+	skipIfNotRequired(t, "mode-irrelevant")
+
+	data, _, _, err := setupTestWithIPFIXCollector(t)
+	if err != nil {
+		t.Fatalf("Error when setting up test: %v", err)
+	}
+	defer teardownFlowAggregator(t, data)
+	defer teardownTest(t, data)
+
+	t.Run("testAntctlFlowAggregatorLocalAccess", func(t *testing.T) {
+		testAntctlFlowAggregatorLocalAccess(t, data)
+	})
+}


I think it is unfortunate that we have to setup everything just for this small test, then tear it down. Can we merge this test with the other FlowAggregator e2e tests instead, so we only do the set up once?

I have tried to merge the tests by using setupTestWithIPFIXCollector for all antctl test, but noticed some of the testAntctlControllerRemoteAccess may fail when using coverage on CI. Still looking for the reason.

it's not really what I had in mind. We are still calling setupTestWithIPFIXCollector twice (once for antctl tests and once for the Flow Aggregator tests). We should try to call it once IMO, and move the antctl tests for the Flow Aggregator to test/e2e/flowaggregator_test.go.

Got it, thanks. Updated.

antoninbas

2 small comments, otherwise LGTM

Suggestions for follow-up PRs:

ability to filter based on K8s names instead of network header fields
use Prometheus to export FlowAggregator metrics and leverage that instead in the CLI if possible
for the FlowAggregator antctl e2e tests, write some "real" tests instead of just calling the commands without arguments. In particular we can call antctl get flowrecords -o json and check that the contents are as expected.

antoninbas · 2021-11-05T17:41:53Z

test/e2e/framework.go

@@ -1326,6 +1326,21 @@ func (data *TestData) getAntreaPodOnNode(nodeName string) (podName string, err e
 	return pods.Items[0].Name, nil
 }

+// getFlowAggregator retrieves the name of the Flow-Aggregator Pod (flow-aggregator-*) running on a specific Node.
+func (data *TestData) getFlowAggregator(nodeName string) (podName string, err error) {


please use this function in the implementation of gracefulExitFlowAggregator to avoid code duplication

antoninbas · 2021-11-05T17:42:34Z

test/e2e/flowaggregator_test.go

+// testAntctlLocalAccess ensures antctl is accessible in a Flow Aggregator Pod.
+func testAntctlLocalAccess(t *testing.T, data *TestData) {


maybe call it something more specific to avoid potential name collisions in the future

for example testFlowAggregatorAntctl

antoninbas · 2021-11-05T22:24:57Z

/test-all

yanjunz97 · 2021-11-06T00:20:02Z

/test-all-features-conformance
/test-integration

yanjunz97 · 2021-11-08T20:35:54Z

All tests passed. This PR can be merged together with #2909 if no other comment.

antoninbas · 2021-11-08T22:57:21Z

@yanjunz97 I merged #2909 but looks like you have to rebase now

Signed-off-by: Yanjun Zhou <[email protected]>

yanjunz97 · 2021-11-08T23:03:50Z

/test-all
/test-integration

yanjunz97 · 2021-11-09T01:45:53Z

/test-integration

yanjunz97 · 2021-11-09T17:39:24Z

/test-integration

yanjunz97 · 2021-11-09T18:06:42Z

@yanjunz97 I merged #2909 but looks like you have to rebase now

Thanks! Rebased and passed the tests

This reverts commit 617ce25.

yanjunz97 force-pushed the flow-aggregator-antctl branch from 387ef70 to fac4249 Compare October 8, 2021 23:06

yanjunz97 marked this pull request as draft October 8, 2021 23:14

yanjunz97 force-pushed the flow-aggregator-antctl branch from fac4249 to 579433b Compare October 11, 2021 21:56

yanjunz97 force-pushed the flow-aggregator-antctl branch from 579433b to a8900d4 Compare October 11, 2021 23:20

antrea-io deleted a comment from lgtm-com bot Oct 11, 2021

yanjunz97 force-pushed the flow-aggregator-antctl branch from a8900d4 to d40f7da Compare October 11, 2021 23:32

yanjunz97 marked this pull request as ready for review October 12, 2021 00:00

yanjunz97 requested review from dreamtalen and srikartati October 12, 2021 00:01

dreamtalen reviewed Oct 12, 2021

View reviewed changes

yanjunz97 force-pushed the flow-aggregator-antctl branch from 1b3502a to ff5fed0 Compare October 12, 2021 17:58

abhiraut added area/flow-visibility/aggregator Issues or PRs related to Flow Aggregator review-manager-test labels Oct 12, 2021

github-actions bot requested review from antoninbas, dreamtalen and zyiou October 12, 2021 18:34

abhiraut added the area/component/antctl Issues or PRs releated to the command line interface component label Oct 12, 2021

github-actions bot requested review from jianjuns and tnqn October 12, 2021 19:20

yanjunz97 force-pushed the flow-aggregator-antctl branch 2 times, most recently from 3c7598e to 2f7a180 Compare October 14, 2021 01:20

dreamtalen reviewed Oct 14, 2021

View reviewed changes

test/e2e/antctl_test.go Outdated Show resolved Hide resolved

yanjunz97 force-pushed the flow-aggregator-antctl branch from 2f7a180 to 2f13613 Compare October 14, 2021 22:42

github-actions bot requested a review from dreamtalen October 14, 2021 22:43

yanjunz97 force-pushed the flow-aggregator-antctl branch from 2f13613 to 01f9374 Compare October 18, 2021 17:00

srikartati reviewed Oct 18, 2021

View reviewed changes

jianjuns reviewed Oct 18, 2021

View reviewed changes

yanjunz97 force-pushed the flow-aggregator-antctl branch from 01f9374 to 917c66d Compare October 18, 2021 23:32

srikartati reviewed Nov 2, 2021

View reviewed changes

antoninbas reviewed Nov 2, 2021

View reviewed changes

antoninbas modified the milestones: Antrea v1.4 release, Antrea v1.5 release Nov 2, 2021

github-actions bot requested review from antoninbas and srikartati November 3, 2021 00:50

yanjunz97 force-pushed the flow-aggregator-antctl branch 4 times, most recently from 5c507aa to 647965c Compare November 4, 2021 23:08

antoninbas reviewed Nov 5, 2021

View reviewed changes

yanjunz97 force-pushed the flow-aggregator-antctl branch from 647965c to e99815a Compare November 5, 2021 22:14

github-actions bot requested a review from antoninbas November 5, 2021 22:15

antoninbas previously approved these changes Nov 5, 2021

View reviewed changes

Support antctl in the flow aggregator

7c797d5

Signed-off-by: Yanjun Zhou <[email protected]>

yanjunz97 dismissed antoninbas’s stale review via 7c797d5 November 8, 2021 22:59

yanjunz97 force-pushed the flow-aggregator-antctl branch from e99815a to 7c797d5 Compare November 8, 2021 22:59

github-actions bot requested a review from antoninbas November 8, 2021 23:03

antoninbas approved these changes Nov 9, 2021

View reviewed changes

antoninbas merged commit 617ce25 into antrea-io:main Nov 9, 2021

qiyueyao added a commit to Dyanngg/antrea that referenced this pull request Nov 9, 2021

Revert "Support antctl for the Flow Aggregator (antrea-io#2878)"

d0419f9

This reverts commit 617ce25.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support antctl in the flow aggregator #2878

Support antctl in the flow aggregator #2878

yanjunz97 commented Oct 8, 2021 •

edited

Loading

codecov-commenter commented Oct 8, 2021 •

edited

Loading

lgtm-com bot commented Oct 11, 2021

dreamtalen left a comment

srikartati Oct 15, 2021

yanjunz97 Oct 18, 2021

yanjunz97 commented Nov 2, 2021

srikartati left a comment

antoninbas Nov 2, 2021

yanjunz97 Nov 3, 2021 •

edited

Loading

yanjunz97 Nov 4, 2021

antoninbas Nov 4, 2021

yanjunz97 Nov 5, 2021

antoninbas left a comment

antoninbas Nov 5, 2021

yanjunz97 Nov 5, 2021

antoninbas Nov 5, 2021

yanjunz97 Nov 5, 2021

antoninbas commented Nov 5, 2021

yanjunz97 commented Nov 6, 2021

yanjunz97 commented Nov 8, 2021

antoninbas commented Nov 8, 2021

yanjunz97 commented Nov 8, 2021

yanjunz97 commented Nov 9, 2021

yanjunz97 commented Nov 9, 2021

yanjunz97 commented Nov 9, 2021

		// testAntctlLocalAccess ensures antctl is accessible in a Flow Aggregator Pod.
		func testAntctlLocalAccess(t testing.T, data TestData) {

Support antctl in the flow aggregator #2878

Support antctl in the flow aggregator #2878

Conversation

yanjunz97 commented Oct 8, 2021 • edited Loading

codecov-commenter commented Oct 8, 2021 • edited Loading

Codecov Report

lgtm-com bot commented Oct 11, 2021

dreamtalen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yanjunz97 commented Nov 2, 2021

srikartati left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yanjunz97 Nov 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas commented Nov 5, 2021

yanjunz97 commented Nov 6, 2021

yanjunz97 commented Nov 8, 2021

antoninbas commented Nov 8, 2021

yanjunz97 commented Nov 8, 2021

yanjunz97 commented Nov 9, 2021

yanjunz97 commented Nov 9, 2021

yanjunz97 commented Nov 9, 2021

yanjunz97 commented Oct 8, 2021 •

edited

Loading

codecov-commenter commented Oct 8, 2021 •

edited

Loading

yanjunz97 Nov 3, 2021 •

edited

Loading