Add K8sNP endPort support #2190

GraysonWu · 2021-05-18T23:57:05Z

This PR fixes #1638.
Change the process of K8sNP -> InternalNP.
To use endPort in kube-apiserver, add featrue-gates flag.

codecov-commenter · 2021-05-19T00:04:36Z

Codecov Report

Merging #2190 (f5187aa) into main (cddfccd) will increase coverage by 3.28%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2190      +/-   ##
==========================================
+ Coverage   61.69%   64.98%   +3.28%     
==========================================
  Files         274      274              
  Lines       20673    20674       +1     
==========================================
+ Hits        12754    13434     +680     
+ Misses       6587     5857     -730     
- Partials     1332     1383      +51

Flag	Coverage Δ
e2e-tests	`56.23% <100.00%> (?)`
kind-e2e-tests	`52.39% <100.00%> (-0.28%)`	⬇️
unit-tests	`41.09% <100.00%> (-0.01%)`	⬇️

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

Impacted Files	Coverage Δ
...ntroller/networkpolicy/networkpolicy_controller.go	`84.59% <100.00%> (+0.02%)`	⬆️
pkg/apiserver/certificate/cacert_controller.go	`58.27% <0.00%> (-7.95%)`	⬇️
pkg/apiserver/apiserver.go	`89.51% <0.00%> (-1.62%)`	⬇️
pkg/ovs/openflow/ofctrl_bridge.go	`50.36% <0.00%> (+0.36%)`	⬆️
pkg/agent/openflow/network_policy.go	`76.41% <0.00%> (+0.59%)`	⬆️
pkg/agent/cniserver/pod_configuration.go	`54.68% <0.00%> (+0.78%)`	⬆️
pkg/controller/egress/store/egressgroup.go	`1.38% <0.00%> (+1.38%)`	⬆️
pkg/ovs/openflow/ofctrl_flow.go	`47.36% <0.00%> (+1.75%)`	⬆️
pkg/flowaggregator/certificate.go	`79.27% <0.00%> (+2.70%)`	⬆️
pkg/agent/openflow/cookie/allocator.go	`67.74% <0.00%> (+3.22%)`	⬆️
... and 38 more

antoninbas

LGTM, I'll defer to @Dyanngg for final approval

antoninbas · 2021-05-19T00:06:04Z

test/e2e/infra/vagrant/playbook/roles/control-plane/templates/kubeadm.conf.j2

+  extraArgs:
+    feature-gates: "NetworkPolicyEndPort=true"


you are enabling EndPort in the Vagrant testbed, which is fine by me, but there is no e2e test for EndPort yet?

I believe @GraysonWu mentioned that he is looking into adding the feature gate in antrea/ci/jenkins/ and antrea/ci/kind/, maybe you could share a bit more info on that front? Also, some amount of documentation on how the feature gate can be enabled for the k8s-apiserver will be very useful.

Just added e2e test and enabled endport feature gate in Kind.
About Jenkins, I asked zhecheng for more information. Let's see his reply if they support 1.21.

As @lzhecheng said, the testbed hasn't upgraded to 1.21. He will add that feature gate while upgrading.

pkg/controller/networkpolicy/networkpolicy_controller_test.go

test/e2e/networkpolicy_test.go

Dyanngg · 2021-05-19T22:11:21Z

We might also want to fix the DCO https://github.com/antrea-io/antrea/pull/2190/checks?check_run_id=2623926970

test/e2e/framework.go

test/e2e/networkpolicy_test.go

antoninbas

LGTM

test/e2e/networkpolicy_test.go

ci/kind/kind-setup.sh

tnqn · 2021-05-24T05:18:12Z

test/e2e/networkpolicy_test.go

@@ -806,6 +806,162 @@ func TestIngressPolicyWithoutPortNumber(t *testing.T) {
 	}
 }

+func TestIngressPolicyWithEndPort(t *testing.T) {


How the test works if the testbed doesn't support 1.21 yet?

Same question, maybe we can add some mechanism to check if the feature gate is enabled? Didn't find a clean way but a simple look into the api server process seems to do the trick https://stackoverflow.com/questions/50441452/check-if-the-feature-gate-is-enabled-disabled-in-kubernetes

ps aux | grep apiserver | grep feature-gates root 15458 7.8 19.7 1449248 402504 ? Ssl May23 185:47 kube-apiserver --advertise-address=192.168.77.100 .............. --etcd-servers=https://127.0.0.1:2379 --feature-gates=NetworkPolicyEndPort=true...........

Good point. Addressed.

test/e2e/fixtures.go

tnqn · 2021-05-25T11:47:44Z

test/e2e/fixtures.go

@@ -109,6 +110,16 @@ func skipIfHasWindowsNodes(tb testing.TB) {
 	}
 }

+func skipIfKubeApiServerFeatureGateNotEnable(tb testing.TB, featureGateName string) {
+	_, stdout, _, err := provider.RunCommandOnNode(clusterInfo.controlPlaneNodeName, "ps aux | grep apiserver | grep feature-gates")


the function sounds generic but will work for features that are not enabled by default. If a newer K8s version enables it by default, it will skip a feature by mistake. It may not be a real problem in CI if we test few K8s versions only. But we should add a comment to explain its limitation.

Or for this particular case, you could create a Policy with endport set, check if the created policy still has it. If not, the feature is not enabled and the test can skip.

Sounds good. Addressed your last suggestion.

GraysonWu · 2021-05-25T19:46:52Z

/test-e2e

tnqn

LGTM

tnqn · 2021-05-26T00:15:23Z

/test-all

Signed-off-by: wgrayson <[email protected]>

network policy -> NetworkPolicy Change `Fatalf` in defer func to `Errorf` Remove double quotation Signed-off-by: wgrayson <[email protected]>

Signed-off-by: wgrayson <[email protected]>

GraysonWu · 2021-05-26T02:31:33Z

/test-all

GraysonWu · 2021-05-26T08:19:24Z

/test-all

GraysonWu · 2021-05-26T08:58:10Z

/test-conformance

tnqn · 2021-05-26T10:27:05Z

/test-conformance
/test-networkpolicy

tnqn · 2021-05-26T15:21:54Z

/test-networkpolicy

GraysonWu requested review from antoninbas and Dyanngg May 18, 2021 23:57

antoninbas reviewed May 19, 2021

View reviewed changes

tnqn reviewed May 19, 2021

View reviewed changes

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

GraysonWu force-pushed the k8s-np-endport branch 2 times, most recently from 6460825 to 454ed24 Compare May 19, 2021 20:06

Dyanngg reviewed May 19, 2021

View reviewed changes

test/e2e/networkpolicy_test.go Show resolved Hide resolved

GraysonWu force-pushed the k8s-np-endport branch 2 times, most recently from 56e61d0 to bc42f76 Compare May 19, 2021 23:34

antoninbas reviewed May 21, 2021

View reviewed changes

GraysonWu force-pushed the k8s-np-endport branch from bc42f76 to 86e9222 Compare May 21, 2021 19:09

antoninbas previously approved these changes May 21, 2021

View reviewed changes

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

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

GraysonWu dismissed antoninbas’s stale review via aee85d6 May 21, 2021 23:07

GraysonWu force-pushed the k8s-np-endport branch from aee85d6 to 9679871 Compare May 21, 2021 23:09

tnqn reviewed May 24, 2021

View reviewed changes

GraysonWu force-pushed the k8s-np-endport branch 2 times, most recently from 662649c to 69bfa6f Compare May 24, 2021 21:58

tnqn reviewed May 25, 2021

View reviewed changes

GraysonWu force-pushed the k8s-np-endport branch 3 times, most recently from aa798fc to ff436c6 Compare May 25, 2021 19:45

tnqn previously approved these changes May 26, 2021

View reviewed changes

GraysonWu added 3 commits May 25, 2021 17:55

Add K8sNP endPort support

879ef8a

Signed-off-by: wgrayson <[email protected]>

Address small comments

24d1945

network policy -> NetworkPolicy Change `Fatalf` in defer func to `Errorf` Remove double quotation Signed-off-by: wgrayson <[email protected]>

Add kube-apiserver feature-gates check

f5187aa

Signed-off-by: wgrayson <[email protected]>

GraysonWu dismissed tnqn’s stale review via f5187aa May 26, 2021 00:55

GraysonWu force-pushed the k8s-np-endport branch from ff436c6 to f5187aa Compare May 26, 2021 00:55

tnqn approved these changes May 26, 2021

View reviewed changes

Dyanngg approved these changes May 26, 2021

View reviewed changes

Dyanngg merged commit 4fc653a into antrea-io:main May 26, 2021

rikatz mentioned this pull request Jun 13, 2021

Promote endPort field in netpolicies to beta kubernetes/kubernetes#102834

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add K8sNP endPort support #2190

Add K8sNP endPort support #2190

GraysonWu commented May 18, 2021 •

edited

Loading

codecov-commenter commented May 19, 2021 •

edited

Loading

antoninbas left a comment

antoninbas May 19, 2021

Dyanngg May 19, 2021

GraysonWu May 19, 2021

GraysonWu May 20, 2021

Dyanngg commented May 19, 2021

antoninbas left a comment

tnqn May 24, 2021

Dyanngg May 24, 2021

GraysonWu May 24, 2021

tnqn May 25, 2021

GraysonWu May 25, 2021

GraysonWu commented May 25, 2021

tnqn left a comment

tnqn commented May 26, 2021

GraysonWu commented May 26, 2021

GraysonWu commented May 26, 2021

GraysonWu commented May 26, 2021

tnqn commented May 26, 2021

tnqn commented May 26, 2021

Add K8sNP endPort support #2190

Add K8sNP endPort support #2190

Conversation

GraysonWu commented May 18, 2021 • edited Loading

codecov-commenter commented May 19, 2021 • edited Loading

Codecov Report

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

Dyanngg commented May 19, 2021

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

GraysonWu commented May 25, 2021

tnqn left a comment

Choose a reason for hiding this comment

tnqn commented May 26, 2021

GraysonWu commented May 26, 2021

GraysonWu commented May 26, 2021

GraysonWu commented May 26, 2021

tnqn commented May 26, 2021

tnqn commented May 26, 2021

GraysonWu commented May 18, 2021 •

edited

Loading

codecov-commenter commented May 19, 2021 •

edited

Loading