-
Notifications
You must be signed in to change notification settings - Fork 366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add K8sNP endPort support #2190
Conversation
Codecov Report
@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I'll defer to @Dyanngg for final approval
extraArgs: | ||
feature-gates: "NetworkPolicyEndPort=true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are enabling EndPort in the Vagrant testbed, which is fine by me, but there is no e2e test for EndPort yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As @lzhecheng said, the testbed hasn't upgraded to 1.21. He will add that feature gate while upgrading.
6460825
to
454ed24
Compare
We might also want to fix the DCO https://github.com/antrea-io/antrea/pull/2190/checks?check_run_id=2623926970 |
56e61d0
to
bc42f76
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -806,6 +806,162 @@ func TestIngressPolicyWithoutPortNumber(t *testing.T) { | |||
} | |||
} | |||
|
|||
func TestIngressPolicyWithEndPort(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How the test works if the testbed doesn't support 1.21 yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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...........
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Addressed.
662649c
to
69bfa6f
Compare
test/e2e/fixtures.go
Outdated
@@ -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") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Addressed your last suggestion.
aa798fc
to
ff436c6
Compare
/test-e2e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/test-all |
Signed-off-by: wgrayson <wgrayson@vmware.com>
network policy -> NetworkPolicy Change `Fatalf` in defer func to `Errorf` Remove double quotation Signed-off-by: wgrayson <wgrayson@vmware.com>
Signed-off-by: wgrayson <wgrayson@vmware.com>
/test-all |
1 similar comment
/test-all |
/test-conformance |
/test-conformance |
/test-networkpolicy |
This PR fixes #1638.
Change the process of K8sNP -> InternalNP.
To use
endPort
in kube-apiserver, add featrue-gates flag.