[IPv6] Add 2 Network Policy tests #1399

lzhecheng · 2020-10-16T10:21:08Z

2 upstream Network Policy tests didn't consider netmask for IPv6, this patch
is to add correct tests. When bug is fixed in latest release, these 2 tests can be deleted.
Kubernetes PR:
kubernetes/kubernetes#93583
2 testcases
https://github.com/kubernetes/kubernetes/blob/v1.20.0-alpha.0/test/e2e/network/network_policy.go#L1365
https://github.com/kubernetes/kubernetes/blob/v1.20.0-alpha.0/test/e2e/network/network_policy.go#L1444

antrea-bot · 2020-10-16T10:21:22Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-hw-offload: to trigger ovs hardware offload test.
/skip-hw-offload: to skip ovs hardware offload test.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

codecov-io · 2020-10-16T10:25:33Z

Codecov Report

Merging #1399 into ipv6 will decrease coverage by 17.48%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             ipv6    #1399       +/-   ##
===========================================
- Coverage   58.95%   41.46%   -17.49%     
===========================================
  Files         157       85       -72     
  Lines       13072    10631     -2441     
===========================================
- Hits         7706     4408     -3298     
- Misses       4552     5826     +1274     
+ Partials      814      397      -417

Flag	Coverage Δ
#kind-e2e-tests	`?`
#unit-tests	`41.46% <ø> (-0.07%)`	⬇️

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

Impacted Files	Coverage Δ
pkg/agent/agent_linux.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/ovs/openflow/default.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/agent/config/node_config.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/ovs/openflow/logs.go	`9.52% <0.00%> (-90.48%)`	⬇️
pkg/controller/networkpolicy/tier.go	`0.00% <0.00%> (-90.00%)`	⬇️
pkg/apis/controlplane/register.go	`0.00% <0.00%> (-85.72%)`	⬇️
pkg/agent/client.go	`0.00% <0.00%> (-77.42%)`	⬇️
...kg/apiserver/registry/system/supportbundle/rest.go	`9.88% <0.00%> (-65.70%)`	⬇️
pkg/agent/util/iptables/iptables.go	`0.00% <0.00%> (-59.75%)`	⬇️
pkg/ovs/openflow/ofctrl_group.go	`0.00% <0.00%> (-53.13%)`	⬇️
... and 120 more

lzhecheng · 2020-10-16T11:31:16Z

/test-ipv6-conformance

antoninbas

Could you add a link in the PR description to the upstream K8s bug?

I assume these tests only affect IPv6 / dual-stack, and that's why we only add them to the ipv6 branch?

antoninbas · 2020-10-16T18:42:01Z

test/e2e/networkpolicy_test.go

@@ -270,6 +271,165 @@ func TestDefaultDenyEgressPolicy(t *testing.T) {
 	}
 }

+// TestEgressToServerInCIDRBlock is a duplicate case as "should allow egress access to server in CIDR block [Feature:NetworkPolicy]",
+// while a bug exists in TOT release. This should be deleted when upstream is updated.
+// https://github.com/kubernetes/kubernetes/blob/master/test/e2e/network/network_policy.go#L1373


please use perma links

Used link for v1.20.0-alpha.0. Also links in the PR description and commit message are provided.

Yes, code in latest release doesn't consider netmask for IPv6 environment.

lzhecheng · 2020-10-19T02:13:45Z

/test-e2e

lzhecheng · 2020-10-19T02:24:36Z

/test-e2e

antoninbas

I still don't see any link to an upstream K8s issue or PR in the commit message / PR description. You have included links to the upstream tests there but that's not as useful. Is there no upstream issue yet?

antoninbas · 2020-10-19T18:48:35Z

test/e2e/networkpolicy_test.go

@@ -270,6 +271,165 @@ func TestDefaultDenyEgressPolicy(t *testing.T) {
 	}
 }

+// TestEgressToServerInCIDRBlock is a duplicate case as "should allow egress access to server in CIDR block [Feature:NetworkPolicy]",


Not a correct sentence

TestEgressToServerInCIDRBlock is a duplicate of upstream test case "should allow egress access to server in CIDR block [Feature:NetworkPolicy]", which is currently buggy in TOT for clusters which use IPv6

antoninbas · 2020-10-19T18:48:45Z

test/e2e/networkpolicy_test.go

+// "should enforce except clause while egress access to server in CIDR block [Feature:NetworkPolicy]",
+// while a bug exists in TOT release. This should be deleted when upstream is updated.
+// https://github.com/kubernetes/kubernetes/blob/v1.20.0-alpha.0/test/e2e/network/network_policy.go#L1444
+func TestEgressToServerInCIDRBlockWithException(t *testing.T) {


same as above comment

antoninbas · 2020-10-19T18:49:58Z

test/e2e/networkpolicy_test.go

+// while a bug exists in TOT release. This should be deleted when upstream is updated.
+// https://github.com/kubernetes/kubernetes/blob/v1.20.0-alpha.0/test/e2e/network/network_policy.go#L1365
+func TestEgressToServerInCIDRBlock(t *testing.T) {
+	data, err := setupTest(t)


should we skip the test if IPv6 is not enabled in the cluster? There is no harm in running the test for IPv4-only clusters though...

Makes sense. Let's skip for cluster without IPv6 CIDR. It will avoid duplicate tests for IPv4 cluster.

antoninbas · 2020-10-19T18:50:58Z

test/e2e/networkpolicy_test.go

+			return
+		}
+		if err = data.deleteNetworkpolicy(np); err != nil {
+			t.Fatalf("Error when deleting network policy: %v", err)


let's not use Fatalf in deferred functions

Used Errorf instead.

antoninbas · 2020-10-19T18:53:52Z

test/e2e/networkpolicy_test.go

+		t.Fatalf("Error when creating network policy: %v", err)
+	}
+	cleanupNP := func() {
+		if np == nil {


if we get to that point, I think np is guaranteed not to be nil, otherwise createNetworkPolicy would have returned an error.

maybe this can be simplified to:

defer func() { if err := data.deleteNetworkpolicy(np); err != nil { t.Errorf("Error when deleting network policy: %v", err) } }

also not sure why you are explicitly setting np to nil at the end of the function

Yes... Actually I copied the code block from another Netpol testcase and I thought such defer func was expected.
https://github.com/vmware-tanzu/antrea/blob/master/test/e2e/networkpolicy_test.go#L292

lzhecheng · 2020-10-20T02:20:07Z

I still don't see any link to an upstream K8s issue or PR in the commit message / PR description. You have included links to the upstream tests there but that's not as useful. Is there no upstream issue yet?

Sorry, I thought it were the testcase links. I have added the link to the PR to support IPv6 in commit message.

lzhecheng · 2020-10-20T02:28:49Z

/test-e2e
/test-ipv6-e2e
/test-ipv6-only-e2e

lzhecheng · 2020-10-20T02:33:36Z

/test-ipv6-e2e
/test-ipv6-only-e2e

lzhecheng · 2020-10-20T03:00:09Z

/test-ipv6-e2e

antoninbas

LGTM
I know that in the past we have updated https://github.com/vmware-tanzu/antrea/blob/master/ci/k8s-conformance-image-version to workaround test issues fixed in master but not yet in an "official" k8s release. Here I guess we could set it to v1.20.0-alpha.2 and thus pick up a test image with correct test cases. However, I like your solution better as switching the test image version to v1.20.0-alpha.2 may create other issues.

antoninbas · 2020-10-20T03:18:52Z

test/e2e/networkpolicy_test.go

@@ -270,6 +271,152 @@ func TestDefaultDenyEgressPolicy(t *testing.T) {
 	}
 }

+// TestEgressToServerInCIDRBlock is a duplicate of upstream test case "should allow egress access to server in CIDR block
+// [Feature:NetworkPolicy]", which is currently buggy in TOT release for clusters which use IPv6.


s/TOT release/1.19 release

(after looking at the PR you shared, I think it's more appropriate)

Updated.

Yes, not only alpha version image may lead to other issues, but also sonobuoy pulls images from gcr.io and v1.20.0-alpha.0 is the latest, not v1.20.0-alpha.2. So I think this is the most efficient way to test these 2 cases.

antoninbas · 2020-10-20T03:19:08Z

test/e2e/networkpolicy_test.go

+}
+
+// TestEgressToServerInCIDRBlockWithException is a duplicate of upstream test case "should allow egress access to server
+// in CIDR block [Feature:NetworkPolicy]", which is currently buggy in TOT release for clusters which use IPv6.


2 upstream Network Policy tests didn't consider netmask for IPv6, this patch is to add correct tests. When bug is fixed in latest release, these 2 tests can be deleted. Kubernetes PR: kubernetes/kubernetes#93583 2 testcases: https://github.com/kubernetes/kubernetes/blob/v1.20.0-alpha.0/test/e2e/network/network_policy.go#L1365 https://github.com/kubernetes/kubernetes/blob/v1.20.0-alpha.0/test/e2e/network/network_policy.go#L1444

lzhecheng · 2020-10-20T04:22:36Z

/test-ipv6-e2e

lzhecheng · 2020-10-20T04:26:16Z

The 2 testcases are only for IPv6 environment and they need IPv6 Network Policy support. So currently, this PR can be merged without check.

2 upstream Network Policy tests didn't consider netmask for IPv6, this patch is to add correct tests. When bug is fixed in latest release, these 2 tests can be deleted. Kubernetes PR: kubernetes/kubernetes#93583 2 testcases: https://github.com/kubernetes/kubernetes/blob/v1.20.0-alpha.0/test/e2e/network/network_policy.go#L1365 https://github.com/kubernetes/kubernetes/blob/v1.20.0-alpha.0/test/e2e/network/network_policy.go#L1444

* Enable verbose logging through generate-manifest.sh (#1142) This patch enables verbose logging for antrea-agent and antrea-controller when generate manifest. This will help in troubleshooting with increased log level. * Bug in PR#1142 (#1248) manifest generation failing in release mode. Fixed the bug. * [IPv6] Consume Node.Spec.CIDRs to support dual-stack configuration (#971) 1. Consume Node.Spec.CIDRs to support IPv4/IPv6 dual-stack Pod Subnets 2. Change NodeConfig.PodCIDR as a slice 3. Change GatewayConfig.IP as a slice to support multiple addresses for antrea-gw0 4. Change InterfaceConfig.IP as a slice to support multiple address for a Pod * [IPv6] Change openflow pipeline for L2 Pod networking (#1040) 1. Add a new table named IPv6 to handle IPv6 ND Solicitation, ND advertisement and IPv6 Multicast traffic. 2. Add flows in openflow tables (spoofGuardTable, IPv6, conntrackTable, conntrackStateTable, conntrackCommitTable, L2ForwardingOutTable) for handling IPv6 L2 Pod networking. * [IPv6] Change host-local IPAM configuration for IPv6 (#1039) 1. Add new field Ranges in IPAMConfig for allocating both IPv4 and IPv6 addresses. 2. Populate subnet and gateway for both IPv4 range and IPv6 range. * [IPv6] Use separate fields for IPv4 and IPv6 in GatewayConfig (#1111) Replace IP slice in GatewayConfig with separate IPv4 and IPv6 fields. * [IPv6] Implement L3 connectivity for IPv6 traffic (#1011) 1. Use IPv6 in iptables and ipset configuration. 2. Identifiy IPv6 address and configure in OpenFlow. 3. Use Node Internal address for tunnel. * [IPv6] Handle Spec.PodCIDR with IPv6 CIDR (#1151) For IPv6 single stack case, node.Spec.PodCIDR is configured with IPv6 CIDR. This patch handles the case and sets nodeConfig.PodIPv6CIDR with parsed CIDR. * [IPv6] Add support for IPv6 address in antctl and agent's apiserver (#1118) * [IPv6] Add support for IPv6 address in antctl and agent's apiserver 1. Support using IPv6 address in OVS tracing. 2. Support displaying Node's and Pod's IPv6 address in agent apiserver. Co-authored-by: Zhecheng Li <zhechel1@uci.edu> * [IPv6] Support IPv6 in e2e (#1129) * [IPv6] Display dual stack NodeSubnet in antrea-octant-plugin (#1156) NodeSubnet can have two values in dual stack case and this patch enhances octant-plugin to show both subnets. * [IPv6] Handle dual stack NodeSubnet for monitoring CRD (#1182) 1. Rename NodeSubnet to NodeSubnets for AntreaAgentInfo. 2. Make a new string slice for dual stack node subnet instead of appending agentInfo.NodeSubnets directly to avoid duplicate CIDRs. * [IPv6][e2e] Fix testDeletePod (#1193) On a dual-stack cluster, podInterfaces[0].IP returns "[ipv4-address], [ipv6-address]". Current implementation doesn't distingush two. * [IPv6] Collect service CIDR in e2e * [IPv6] Add support for dual-stack when using kube-proxy for Service (#1200) 1. Add a config item for IPv6 Serivce CIDR if using kube-proxy to provide Service functions. 2. Output IPv6 traffic from host gateway if its destination is a Service address. 3. Use ct_mark to identify Service traffic and output the reply packet to the host gateway to ensure the DNAT processing in iptables. * [IPv6] Extend e2e tests for dual-stack (#1192) 1. Extend generic functions "podWaitForIP" to return all assigned IPs of a given Pod. 2. Validate each IP address in the cluster's network CIDR 3. Use each valid IP to check connectivity. 4. Use each valid IP to execute tests. * [IPv6] E2e bug fixes (#1311) 1. No -6 option in busybox nc So, no need to distinguish if it is an IPv6 environment for runNetcatCommandFromTestPod() nc BusyBox v1.31.1 (2019-10-28 18:40:01 UTC) multi-call binary. Usage: nc [OPTIONS] HOST PORT - connect nc [OPTIONS] -l -p PORT [HOST] [PORT] - listen -e PROG Run PROG after connect (must be last) -l Listen mode, for inbound connects -lk With -e, provides persistent server -p PORT Local port -s ADDR Local address -w SEC Timeout for connects and final net reads -i SEC Delay interval for lines sent -n Don't do DNS resolution -u UDP mode -v Verbose -o FILE Hex dump traffic -z Zero-I/O mode (scanning) 2. testCert * IPv6 address should be in "[]" * [IPv6] Fix TestReconcileGatewayRoutesOnStartup failure (#1313) Use "ip -6 route" for IPv6 network. * [IPv6] adjust MTU for IPv6 overhead (#1305) If Antrea MTU is too large in IPv6 environment, large packet with overhead exceeds node MTU cannot be transmitted successfully across nodes. IPv6ExtraOverhead, 20 is from observation of IPv4 and IPv6 packets under same situation. * [IPv6] Fix MTU config (#1317) Use Node's internal address to decide if extra IPv6 overhead is needed. * [IPv6] Skip IPsec e2e test (#1373) * With OVS v2.14.0, IPsec in IPv6 envinronment is not supported. * More user-friendly output for PodIPs. From: Retrieved all Pod IPs: map[test-pod-0-upgp1ung:0xc000708960 test-pod-1-pbva9007:0xc0006ec8a0] To: Retrieved all Pod IPs: map[test-pod-0-mudzj847:IPv6: fd74:ca9b:172:16::4, IP strings: fd74:ca9b:172:16::4 test-pod-1-apcmyd30:IPv6: fd74:ca9b:172:16:1::3c, IP strings: fd74:ca9b:172:16:1::3c] * [IPv6] Add 2 Network Policy tests (#1399) 2 upstream Network Policy tests didn't consider netmask for IPv6, this patch is to add correct tests. When bug is fixed in latest release, these 2 tests can be deleted. Kubernetes PR: kubernetes/kubernetes#93583 2 testcases: https://github.com/kubernetes/kubernetes/blob/v1.20.0-alpha.0/test/e2e/network/network_policy.go#L1365 https://github.com/kubernetes/kubernetes/blob/v1.20.0-alpha.0/test/e2e/network/network_policy.go#L1444 * Skip 2 Network Policy testcases before Network Policy IPv6 is supported (#1460) * [IPv6] Fix after rebasing * format code * fix TestPodTrafficShaping * fix TestIPv6RoutesAndNeighbors * [IPv6] Fix issues (#1496) * unit test * manifest * [IPv6] Skip TestAntctlProxy for IPv6 (#1498) * [IPv6] Add IPv6 support for NetworkPolicy 1. Add enhancement in Antrea Controller and Agent to support NetworkPolicy in IPv6. 2. Optimize test cases to support IPv6 3. Use regex in CRD to validate IPv4 or IPv6 string 4. Add TestEgressToServerInCIDRBlock and TestEgressToServerInCIDRBlockWithException 5. networkpolicy_controller.go: PodIPs includes PodIP * [IPv6] Fix issues * remove Github Actions integration test, Jenkins: jenkins-integration -> Integration tests * go fmt * add FlowProtocl() to interface Flow * remove extra lines when rebasing for an octant commit * TestIPv6RoutesAndNeighbors: routeClient.Initialize Co-authored-by: srikartati <stati@vmware.com> Co-authored-by: Wenying Dong <wenyingd@vmware.com> Co-authored-by: Mengdie Song <songm@vmware.com> Co-authored-by: Zhecheng Li <zhechel1@uci.edu>

lzhecheng requested review from antoninbas and wenyingd October 16, 2020 10:21

vmwclabot added the cla-not-required label Oct 16, 2020

lzhecheng force-pushed the ipv6-e2e branch from 8edb9db to f5793e0 Compare October 16, 2020 11:27

antoninbas reviewed Oct 16, 2020

View reviewed changes

lzhecheng force-pushed the ipv6-e2e branch from f5793e0 to becfec2 Compare October 19, 2020 01:51

lzhecheng requested a review from antoninbas October 19, 2020 01:52

wenyingd force-pushed the ipv6 branch from 5ad01f4 to 9e4bddc Compare October 19, 2020 02:37

lzhecheng force-pushed the ipv6-e2e branch from becfec2 to a8152ee Compare October 19, 2020 03:23

antoninbas reviewed Oct 19, 2020

View reviewed changes

lzhecheng force-pushed the ipv6-e2e branch 2 times, most recently from 6e2a416 to fc62157 Compare October 20, 2020 02:18

lzhecheng force-pushed the ipv6-e2e branch from fc62157 to deadc8c Compare October 20, 2020 02:22

antoninbas approved these changes Oct 20, 2020

View reviewed changes

lzhecheng force-pushed the ipv6-e2e branch from deadc8c to 8bb99cd Compare October 20, 2020 03:58

lzhecheng merged commit 07e113b into antrea-io:ipv6 Oct 20, 2020

lzhecheng deleted the ipv6-e2e branch October 20, 2020 04:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IPv6] Add 2 Network Policy tests #1399

[IPv6] Add 2 Network Policy tests #1399

lzhecheng commented Oct 16, 2020 •

edited

Loading

antrea-bot commented Oct 16, 2020

codecov-io commented Oct 16, 2020 •

edited

Loading

lzhecheng commented Oct 16, 2020

antoninbas left a comment

antoninbas Oct 16, 2020

lzhecheng Oct 19, 2020

lzhecheng commented Oct 19, 2020

lzhecheng commented Oct 19, 2020

antoninbas left a comment

antoninbas Oct 19, 2020

lzhecheng Oct 20, 2020

antoninbas Oct 19, 2020

lzhecheng Oct 20, 2020

antoninbas Oct 19, 2020

lzhecheng Oct 20, 2020

antoninbas Oct 19, 2020

lzhecheng Oct 20, 2020

antoninbas Oct 19, 2020

lzhecheng Oct 20, 2020

lzhecheng commented Oct 20, 2020

lzhecheng commented Oct 20, 2020

lzhecheng commented Oct 20, 2020

lzhecheng commented Oct 20, 2020

antoninbas left a comment

antoninbas Oct 20, 2020

lzhecheng Oct 20, 2020

antoninbas Oct 20, 2020

lzhecheng Oct 20, 2020

lzhecheng commented Oct 20, 2020

lzhecheng commented Oct 20, 2020

[IPv6] Add 2 Network Policy tests #1399

[IPv6] Add 2 Network Policy tests #1399

Conversation

lzhecheng commented Oct 16, 2020 • edited Loading

antrea-bot commented Oct 16, 2020

codecov-io commented Oct 16, 2020 • edited Loading

Codecov Report

lzhecheng commented Oct 16, 2020

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

lzhecheng commented Oct 19, 2020

lzhecheng commented Oct 19, 2020

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

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

Choose a reason for hiding this comment

lzhecheng commented Oct 20, 2020

lzhecheng commented Oct 20, 2020

lzhecheng commented Oct 20, 2020

lzhecheng commented Oct 20, 2020

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

lzhecheng commented Oct 20, 2020

lzhecheng commented Oct 20, 2020

lzhecheng commented Oct 16, 2020 •

edited

Loading

codecov-io commented Oct 16, 2020 •

edited

Loading