Fix ClusterSet scoped policy rule not compatible with `namespaces` field #4571

Dyanngg · 2023-01-18T18:26:43Z

Antrea-native policies support the use of namespaces field in ingress/egress
rules to create per-namespace policy rules, which is useful for creating Namespace
level isolation. This field did not work with MultiCluster NetworkPolicy. This PR adds
the support so that cross-cluster Namespace isolation can be achieved.

In addition, this PR also adds a commit to assign unique podCIDR for member clusters
of the e2e test MCS deployment, which is the pre-req for Pod-to-Pod connectivity.
This enables the new testcase for cross-cluster Namespace isolation to be run correctly.

Signed-off-by: Dyanngg dingyang@vmware.com

Dyanngg · 2023-01-18T18:27:02Z

/test-multicluster-e2e

codecov · 2023-01-18T18:50:39Z

Codecov Report

Merging #4571 (cde4090) into main (aafea18) will increase coverage by 1.18%.
The diff coverage is 68.63%.

❗ Current head cde4090 differs from pull request most recent head 50e9f21. Consider uploading reports for the commit 50e9f21 to get more accurate results

@@            Coverage Diff             @@
##             main    #4571      +/-   ##
==========================================
+ Coverage   68.65%   69.84%   +1.18%     
==========================================
  Files         402      415      +13     
  Lines       59570    58692     -878     
==========================================
+ Hits        40900    40995      +95     
+ Misses      15847    14912     -935     
+ Partials     2823     2785      -38

Flag	Coverage Δ		*Carryforward flag
e2e-tests	`40.51% <57.88%> (+2.13%)`	⬆️
integration-tests	`34.48% <66.66%> (+0.07%)`	⬆️	Carriedforward from be83891
kind-e2e-tests	`47.69% <68.68%> (+8.13%)`	⬆️	Carriedforward from be83891
unit-tests	`59.83% <81.27%> (+0.03%)`	⬆️	Carriedforward from be83891

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files	Coverage Δ
cmd/antrea-agent/agent.go	`0.00% <0.00%> (ø)`
pkg/agent/cniserver/ipam/ipam_delegator.go	`68.18% <ø> (ø)`
pkg/agent/cniserver/ipam/ipam_service.go	`83.87% <ø> (ø)`
pkg/agent/cniserver/pod_configuration_linux.go	`100.00% <ø> (ø)`
pkg/agent/cniserver/pod_configuration_windows.go	`75.00% <ø> (ø)`
pkg/agent/cniserver/server_linux.go	`100.00% <ø> (ø)`
pkg/agent/cniserver/server_windows.go	`85.18% <ø> (ø)`
pkg/agent/cniserver/sriov.go	`31.37% <ø> (ø)`
pkg/agent/secondarynetwork/ipam/ipam_delegator.go	`3.70% <ø> (ø)`
pkg/agent/secondarynetwork/podwatch/controller.go	`75.76% <ø> (+0.34%)`	⬆️
... and 102 more

Dyanngg · 2023-01-18T20:04:03Z

/test-multicluster-e2e

Dyanngg · 2023-01-18T22:06:35Z

/skip-all /test-multicluster-e2e

Dyanngg · 2023-01-24T19:36:03Z

/test-all /test-multicluster-e2e

ci/jenkins/test-mc.sh

multicluster/test/e2e/antreapolicy_test.go

ci/jenkins/test-mc.sh

pkg/controller/labelidentity/label_group_index.go

Dyanngg · 2023-02-01T18:01:01Z

/test-all /test-multicluster-e2e

Dyanngg · 2023-03-09T01:02:02Z

/test-all /test-multicluster-e2e /test-windows-e2e

Dyanngg · 2023-03-09T01:42:34Z

/test-multicluster-e2e

Dyanngg · 2023-03-09T02:13:15Z

/test-multicluster-e2e

Dyanngg · 2023-03-09T02:57:07Z

/test-all /test-windows-e2e

Dyanngg · 2023-03-09T06:33:52Z

/test-all /test-multicluster-e2e /test-windows-e2e

tnqn

LGTM overall, some minor comments

multicluster/test/e2e/antreapolicy_test.go

pkg/controller/networkpolicy/antreanetworkpolicy.go

Dyanngg · 2023-03-09T18:48:26Z

/test-all /test-multicluster-e2e /test-windows-e2e

Dyanngg · 2023-03-09T23:19:58Z

/test-all /test-multicluster-e2e /test-windows-e2e

Dyanngg · 2023-03-10T00:38:30Z

/test-multicluster-e2e

Dyanngg · 2023-03-10T03:29:59Z

/test-all /test-multicluster-e2e /test-windows-e2e

pkg/controller/networkpolicy/crd_utils.go

tnqn · 2023-03-10T13:00:11Z

pkg/controller/networkpolicy/crd_utils.go

@@ -152,17 +155,19 @@ func (n *NetworkPolicyController) toAntreaPeerForCRD(peers []v1alpha1.NetworkPol
 		// For other cases it uses the IPBlock "0.0.0.0/0" to avoid the overhead
 		// of handling member updates of the AddressGroup.
 		if dir == controlplane.DirectionIn || !namedPortExists {
-			return &matchAllPeer, nil
+			return &matchAllPeer, nil, sets.NewString()


Suggested change

return &matchAllPeer, nil, sets.NewString()

return &matchAllPeer, nil, nil

no need to allocate a new map

I was doing this to avoid nil checks from the caller side. Changed.

nil map or nil slice is not writable but readable, so there was no difference from caller's perspective regardless whether it returns empty or nil map but returning nil map can save an allocation. But the additional nil check added in the latest code could avoid another self-copy, which is better from performance's perspective.

pkg/controller/labelidentity/label_group_index.go

Signed-off-by: Dyanngg <dingyang@vmware.com>

Dyanngg · 2023-03-14T20:31:21Z

/test-all /test-multicluster-e2e

tnqn

LGTM

tnqn · 2023-03-15T10:39:53Z

/test-all /test-multicluster-e2e

…eld (antrea-io#4571) Signed-off-by: Dyanngg <dingyang@vmware.com>

Dyanngg force-pushed the fix-clusterset-namespaces-sel branch from c587578 to 210a517 Compare January 18, 2023 19:59

Dyanngg force-pushed the fix-clusterset-namespaces-sel branch 2 times, most recently from c8fd3d0 to 8a03a27 Compare January 18, 2023 22:05

Dyanngg force-pushed the fix-clusterset-namespaces-sel branch 11 times, most recently from ecc5e0d to 5a71a19 Compare January 24, 2023 19:35

Dyanngg changed the title ~~[WIP] Fix ClusterSet scope policy rule not compatible with namespaces field~~ Fix ClusterSet scoped policy rule not compatible with namespaces field Jan 24, 2023

Dyanngg requested review from tnqn, GraysonWu and luolanzone and removed request for GraysonWu January 27, 2023 19:59

luolanzone added the area/multi-cluster Issues or PRs related to multi cluster. label Jan 30, 2023

luolanzone reviewed Jan 30, 2023

View reviewed changes

Dyanngg force-pushed the fix-clusterset-namespaces-sel branch from 5a71a19 to 0381965 Compare February 1, 2023 17:58

Dyanngg requested a review from luolanzone February 1, 2023 18:01

Dyanngg force-pushed the fix-clusterset-namespaces-sel branch from 151c919 to 835f99b Compare March 9, 2023 01:42

Dyanngg force-pushed the fix-clusterset-namespaces-sel branch from 835f99b to ddf61a1 Compare March 9, 2023 02:13

Dyanngg requested review from wenyingd and jianjuns March 9, 2023 02:57

Dyanngg force-pushed the fix-clusterset-namespaces-sel branch from ddf61a1 to a8e6a7f Compare March 9, 2023 06:33

tnqn reviewed Mar 9, 2023

View reviewed changes

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

pkg/controller/networkpolicy/antreanetworkpolicy.go Outdated Show resolved Hide resolved

pkg/controller/networkpolicy/antreanetworkpolicy.go Outdated Show resolved Hide resolved

Dyanngg force-pushed the fix-clusterset-namespaces-sel branch from c1a102a to e4fcc23 Compare March 9, 2023 23:19

Dyanngg requested a review from tnqn March 10, 2023 04:57

tnqn reviewed Mar 10, 2023

View reviewed changes

Dyanngg added 5 commits March 14, 2023 10:56

Fix ClusterSet scope policy rule not compatible with namespaces field

8a0e6ce

Signed-off-by: Dyanngg <dingyang@vmware.com>

Add E2E testcase for ClusterSet scope with namespaces selection

13506a0

Signed-off-by: Dyanngg <dingyang@vmware.com>

Address final comments

9e9eac5

Signed-off-by: Dyanngg <dingyang@vmware.com>

Address more comments

7a5efab

Signed-off-by: Dyanngg <dingyang@vmware.com>

Address test flake

1910b2e

Signed-off-by: Dyanngg <dingyang@vmware.com>

Dyanngg force-pushed the fix-clusterset-namespaces-sel branch from 36acb08 to 1910b2e Compare March 14, 2023 20:30

tnqn approved these changes Mar 15, 2023

View reviewed changes

tnqn added the action/release-note Indicates a PR that should be included in release notes. label Mar 15, 2023

tnqn merged commit 910511d into antrea-io:main Mar 15, 2023

jainpulkit22 pushed a commit to urharshitha/antrea that referenced this pull request Apr 28, 2023

Fix ClusterSet scoped policy rule not compatible with namespaces fi…

58ed239

…eld (antrea-io#4571) Signed-off-by: Dyanngg <dingyang@vmware.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ClusterSet scoped policy rule not compatible with `namespaces` field #4571

Fix ClusterSet scoped policy rule not compatible with `namespaces` field #4571

Dyanngg commented Jan 18, 2023 •

edited

Dyanngg commented Jan 18, 2023

codecov bot commented Jan 18, 2023 •

edited

Dyanngg commented Jan 18, 2023

Dyanngg commented Jan 18, 2023

Dyanngg commented Jan 24, 2023

Dyanngg commented Feb 1, 2023

Dyanngg commented Mar 9, 2023

Dyanngg commented Mar 9, 2023

Dyanngg commented Mar 9, 2023

Dyanngg commented Mar 9, 2023

Dyanngg commented Mar 9, 2023

tnqn left a comment

Dyanngg commented Mar 9, 2023

Dyanngg commented Mar 9, 2023

Dyanngg commented Mar 10, 2023

Dyanngg commented Mar 10, 2023

tnqn Mar 10, 2023

Dyanngg Mar 14, 2023

tnqn Mar 15, 2023

Dyanngg commented Mar 14, 2023

tnqn left a comment

tnqn commented Mar 15, 2023

	return &matchAllPeer, nil, sets.NewString()
	return &matchAllPeer, nil, nil

Fix ClusterSet scoped policy rule not compatible with namespaces field #4571

Fix ClusterSet scoped policy rule not compatible with namespaces field #4571

Conversation

Dyanngg commented Jan 18, 2023 • edited

Dyanngg commented Jan 18, 2023

codecov bot commented Jan 18, 2023 • edited

Codecov Report

Dyanngg commented Jan 18, 2023

Dyanngg commented Jan 18, 2023

Dyanngg commented Jan 24, 2023

Dyanngg commented Feb 1, 2023

Dyanngg commented Mar 9, 2023

Dyanngg commented Mar 9, 2023

Dyanngg commented Mar 9, 2023

Dyanngg commented Mar 9, 2023

Dyanngg commented Mar 9, 2023

tnqn left a comment

Choose a reason for hiding this comment

Dyanngg commented Mar 9, 2023

Dyanngg commented Mar 9, 2023

Dyanngg commented Mar 10, 2023

Dyanngg commented Mar 10, 2023

tnqn Mar 10, 2023

Choose a reason for hiding this comment

Dyanngg Mar 14, 2023

Choose a reason for hiding this comment

tnqn Mar 15, 2023

Choose a reason for hiding this comment

Dyanngg commented Mar 14, 2023

tnqn left a comment

Choose a reason for hiding this comment

tnqn commented Mar 15, 2023

Fix ClusterSet scoped policy rule not compatible with `namespaces` field #4571

Fix ClusterSet scoped policy rule not compatible with `namespaces` field #4571

Dyanngg commented Jan 18, 2023 •

edited

codecov bot commented Jan 18, 2023 •

edited