E2E test for MC ACNP copy-span #3435

Dyanngg · 2022-03-11T00:50:36Z

PR is based on #3424 and #3363. Only the last commit is relevant to this PR.

Dyanngg · 2022-03-11T00:54:22Z

/test-multicluster-e2e

codecov-commenter · 2022-03-11T00:57:24Z

Codecov Report

Merging #3435 (33af3b3) into main (2dcc257) will decrease coverage by 11.42%.
The diff coverage is n/a.

❗ Current head 33af3b3 differs from pull request most recent head 0011cb8. Consider uploading reports for the commit 0011cb8 to get more accurate results

@@             Coverage Diff             @@
##             main    #3435       +/-   ##
===========================================
- Coverage   65.51%   54.08%   -11.43%     
===========================================
  Files         277      247       -30     
  Lines       27500    35027     +7527     
===========================================
+ Hits        18016    18945      +929     
- Misses       7567    14295     +6728     
+ Partials     1917     1787      -130

Flag	Coverage Δ
e2e-tests	`54.08% <ø> (?)`
kind-e2e-tests	`?`
unit-tests	`?`

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

Impacted Files	Coverage Δ
pkg/controller/egress/controller.go	`0.00% <0.00%> (-88.45%)`	⬇️
pkg/controller/networkpolicy/endpoint_querier.go	`4.58% <0.00%> (-86.85%)`	⬇️
pkg/controller/ipam/validate.go	`0.00% <0.00%> (-82.26%)`	⬇️
pkg/agent/util/iptables/lock.go	`0.00% <0.00%> (-81.82%)`	⬇️
pkg/controller/ipam/antrea_ipam_controller.go	`0.00% <0.00%> (-80.29%)`	⬇️
pkg/agent/cniserver/ipam/antrea_ipam_controller.go	`0.00% <0.00%> (-80.00%)`	⬇️
pkg/controller/externalippool/validate.go	`0.00% <0.00%> (-76.20%)`	⬇️
pkg/agent/cniserver/ipam/antrea_ipam.go	`3.62% <0.00%> (-76.17%)`	⬇️
pkg/cni/client.go	`0.00% <0.00%> (-75.52%)`	⬇️
.../registry/networkpolicy/clustergroupmember/rest.go	`11.11% <0.00%> (-73.21%)`	⬇️
... and 267 more

Dyanngg · 2022-03-11T01:12:05Z

/test-multicluster-e2e

multicluster/test/yamls/test-acnp-copy-span-ns-isolation.yml

multicluster/test/e2e/mcs_antreapolicy_test.go

Dyanngg · 2022-03-11T04:41:48Z

/test-multicluster-e2e

Dyanngg · 2022-03-11T05:10:12Z

/test-multicluster-e2e

Dyanngg · 2022-03-11T18:56:11Z

Multi-cluster e2e already passing for the copy-span feature.

Dyanngg · 2022-03-11T21:55:07Z

/test-all

luolanzone

LGTM overall, two nits:

multicluster/test/e2e/fixtures.go

multicluster/test/e2e/antreapolicy_test.go

Dyanngg · 2022-03-15T21:18:00Z

Rebased and resolved merge conflicts /test-all /test-multicluster-e2e

Dyanngg · 2022-03-20T03:57:11Z

Rebased and resolved merge conflicts /test-all /test-multicluster-e2e

Dyanngg · 2022-03-20T04:44:55Z

/test-multicluster-e2e

luolanzone · 2022-03-23T08:13:20Z

multicluster/test/e2e/antreapolicy_test.go

+					_, wrong, _ := step.Reachability.Summary()
+					if wrong != 0 {
+						t.Errorf("Failure in cluster %s -- %d wrong results", clusterName, wrong)
+						reachability.PrintSummary(true, true, true)


not introduced in this PR, but I see all callers of reachability.PrintSummary() are using (true, true, true), maybe we can remove all three parameters. do you think if this can be enhanced in #3286 by enhao?

Sure. /cc @enhaocui for awareness.

tnqn

LGTM, one question

tnqn · 2022-03-24T17:59:14Z

multicluster/test/yamls/test-acnp-copy-span-ns-isolation.yml

+              match: Self
+          - podSelector:
+              matchLabels:
+                k8s-app: kube-dns


Why it must allow kube-dns to access the test Pods?

It does not need to allow kube-dns for the test to pass. I am merely trying to align the yaml with https://github.com/antrea-io/antrea/blob/main/docs/antrea-network-policy.md#acnp-for-strict-namespace-isolation

But I didn't see this in that doc. And the rule should only make sense if it's egress rule, no reason to allow kube-dns accessing all Pods.

But I didn't see this in that doc.

Sorry I was meant to say in here #3512. I've created a PR to update the sample yamls of Antrea-native policies.

And the rule should only make sense if it's egress rule, no reason to allow kube-dns accessing all Pods.

I was thinking in the lines of the DNS return packets need to be allowed. But I guess your point is that does not warrant an ingress rule since there's going to be some bypass flow. In that case let me just remove this section from ingress then.

Yes, the policy is stateful, DNS return packets will be allowed if the requests are allowed regardless of the existence of this rule. The rule will only apply to packets in request direction.

Dyanngg · 2022-03-25T00:02:24Z

/test-all /test-multicluster-e2e

tnqn · 2022-03-25T02:34:21Z

@Dyanngg could you rebase the PR and check if #3435 (comment) makes sense?

luolanzone · 2022-03-25T03:09:52Z

/test-all /test-multicluster-e2e

Signed-off-by: Yang Ding <[email protected]>

Dyanngg · 2022-03-25T03:48:06Z

/test-all /test-multicluster-e2e

Dyanngg added the area/multi-cluster Issues or PRs related to multi cluster. label Mar 11, 2022

Dyanngg added this to the Antrea v1.6 release milestone Mar 11, 2022

Dyanngg requested review from hjiajing, jianjuns and luolanzone March 11, 2022 00:50

Dyanngg force-pushed the mcs-copy-span-e2e branch from d2a63f4 to 0d9e3ff Compare March 11, 2022 01:10

luolanzone reviewed Mar 11, 2022

View reviewed changes

Dyanngg force-pushed the mcs-copy-span-e2e branch from 0d9e3ff to 1ceeb94 Compare March 11, 2022 04:41

Dyanngg force-pushed the mcs-copy-span-e2e branch from 1ceeb94 to 251d2fa Compare March 11, 2022 05:08

Dyanngg requested a review from luolanzone March 11, 2022 18:55

Dyanngg mentioned this pull request Mar 11, 2022

Add ACNP copy span for multi-cluster #3363

Merged

luolanzone previously approved these changes Mar 15, 2022

View reviewed changes

multicluster/test/e2e/fixtures.go Outdated Show resolved Hide resolved

multicluster/test/e2e/antreapolicy_test.go Outdated Show resolved Hide resolved

Dyanngg dismissed luolanzone’s stale review via 4fb0587 March 15, 2022 21:14

Dyanngg force-pushed the mcs-copy-span-e2e branch 2 times, most recently from 4fb0587 to f53058d Compare March 15, 2022 21:16

Dyanngg force-pushed the mcs-copy-span-e2e branch 2 times, most recently from 40781ae to ea5e639 Compare March 20, 2022 03:56

Dyanngg force-pushed the mcs-copy-span-e2e branch from ea5e639 to 1d6f264 Compare March 20, 2022 04:44

Dyanngg force-pushed the mcs-copy-span-e2e branch from 1d6f264 to 32ee56d Compare March 22, 2022 21:52

luolanzone added the area/test/e2e Issues or PRs related to Antrea specific end-to-end testing. label Mar 23, 2022

luolanzone mentioned this pull request Mar 23, 2022

Remove unnecessary CRDs and RBAC rules #3491

Merged

luolanzone previously approved these changes Mar 23, 2022

View reviewed changes

tnqn reviewed Mar 24, 2022

View reviewed changes

Dyanngg dismissed luolanzone’s stale review via 0011cb8 March 24, 2022 20:53

Dyanngg force-pushed the mcs-copy-span-e2e branch from 32ee56d to 0011cb8 Compare March 24, 2022 20:53

luolanzone force-pushed the mcs-copy-span-e2e branch from 0011cb8 to 57a340a Compare March 25, 2022 03:07

Add E2E test for the ACNP copy-span feature

f1c1dfa

Signed-off-by: Yang Ding <[email protected]>

Dyanngg force-pushed the mcs-copy-span-e2e branch from 57a340a to f1c1dfa Compare March 25, 2022 03:47

tnqn approved these changes Mar 25, 2022

View reviewed changes

tnqn merged commit 7599ffb into antrea-io:main Mar 25, 2022

Dyanngg deleted the mcs-copy-span-e2e branch March 25, 2022 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

E2E test for MC ACNP copy-span #3435

E2E test for MC ACNP copy-span #3435

Dyanngg commented Mar 11, 2022

Dyanngg commented Mar 11, 2022

codecov-commenter commented Mar 11, 2022 •

edited

Loading

Dyanngg commented Mar 11, 2022

Dyanngg commented Mar 11, 2022

Dyanngg commented Mar 11, 2022

Dyanngg commented Mar 11, 2022

Dyanngg commented Mar 11, 2022

luolanzone left a comment

Dyanngg commented Mar 15, 2022

Dyanngg commented Mar 20, 2022

Dyanngg commented Mar 20, 2022

luolanzone Mar 23, 2022 •

edited

Loading

Dyanngg Mar 24, 2022

tnqn left a comment

tnqn Mar 24, 2022

Dyanngg Mar 24, 2022

tnqn Mar 25, 2022

Dyanngg Mar 25, 2022

tnqn Mar 25, 2022

Dyanngg commented Mar 25, 2022

tnqn commented Mar 25, 2022

luolanzone commented Mar 25, 2022

Dyanngg commented Mar 25, 2022

E2E test for MC ACNP copy-span #3435

E2E test for MC ACNP copy-span #3435

Conversation

Dyanngg commented Mar 11, 2022

Dyanngg commented Mar 11, 2022

codecov-commenter commented Mar 11, 2022 • edited Loading

Codecov Report

Dyanngg commented Mar 11, 2022

Dyanngg commented Mar 11, 2022

Dyanngg commented Mar 11, 2022

Dyanngg commented Mar 11, 2022

Dyanngg commented Mar 11, 2022

luolanzone left a comment

Choose a reason for hiding this comment

Dyanngg commented Mar 15, 2022

Dyanngg commented Mar 20, 2022

Dyanngg commented Mar 20, 2022

luolanzone Mar 23, 2022 • edited Loading

Choose a reason for hiding this comment

Dyanngg Mar 24, 2022

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

tnqn Mar 24, 2022

Choose a reason for hiding this comment

Dyanngg Mar 24, 2022

Choose a reason for hiding this comment

tnqn Mar 25, 2022

Choose a reason for hiding this comment

Dyanngg Mar 25, 2022

Choose a reason for hiding this comment

tnqn Mar 25, 2022

Choose a reason for hiding this comment

Dyanngg commented Mar 25, 2022

tnqn commented Mar 25, 2022

luolanzone commented Mar 25, 2022

Dyanngg commented Mar 25, 2022

codecov-commenter commented Mar 11, 2022 •

edited

Loading

luolanzone Mar 23, 2022 •

edited

Loading