Skip to content
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

Replace hack/netpol/ with new upstream NetworkPolicy test suite #1740

Closed
antoninbas opened this issue Jan 13, 2021 · 12 comments · Fixed by #4793
Closed

Replace hack/netpol/ with new upstream NetworkPolicy test suite #1740

antoninbas opened this issue Jan 13, 2021 · 12 comments · Fixed by #4793
Assignees
Labels
area/exp/netpol Issues or PRs related to the netpol proposal. area/test/community Issues or PRs related to community testing good first issue Good for newcomers kind/design Categorizes issue or PR as related to design. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@antoninbas
Copy link
Contributor

The netpol test suite that we protoyped in Antrea (https://github.com/vmware-tanzu/antrea/tree/master/hack/netpol) was ported upstream by @jayunit100 and others: https://github.com/kubernetes/kubernetes/tree/master/test/e2e/network/netpol

As a result it now makes sense to remove the hack/netpol/ directory altogether and instead start running the upstream version of the test suite as part of Antrea CI. I think we can probably move from a Kind CI job to a Jenkins job on VMC (VMware on AWS), to avoid some known issues with Kind / the Open vSwitch netdev datapath (see #897 for an example).

My preference would be to simply run it as part of the existing jenkins-networkpolicy job:

  • because it makes sense :)
  • to avoid introducing yet another job
  • because the test suite is supposed to run pretty fast and should not add too much time compared to the current job

I am hoping we can simply update https://github.com/vmware-tanzu/antrea/blob/master/ci/run-k8s-e2e-tests.sh to avoid increasing the number of CI scripts we have to maintain.
The only issue I see is that at this time (01/12), there is no named tag of the k8s.gcr.io/conformance image which includes the upstream netpol tests. The latest tag seems to be v1.21.0-alpha.0, and it doesn't include them.
As a result, we can either:

  1. wait for a version of the k8s.gcr.io/conformance image with support for the netpol tests
  2. build the k8s.gcr.io/conformance image from the K8s source (https://github.com/kubernetes/kubernetes/blob/master/cluster/images/conformance/Makefile) and use it in run-k8s-e2e-tests.sh

I don't really have a preference for either. If someone wants to start working on this issue in the near future, then they can go with the second solution.

@antoninbas antoninbas added area/test/community Issues or PRs related to community testing kind/design Categorizes issue or PR as related to design. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Jan 13, 2021
@antoninbas
Copy link
Contributor Author

@jayunit100 were you still planning to handle this?

@antoninbas antoninbas added the area/exp/netpol Issues or PRs related to the netpol proposal. label Jan 13, 2021
@jayunit100
Copy link
Contributor

Happy to help but cycles limited

If any newcomers in the network policy testing area this is a good getting started issue that I could collaborate on

Otherwise I'll get it done sooner or later

@antoninbas antoninbas added the good first issue Good for newcomers label Jan 14, 2021
@github-actions
Copy link
Contributor

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment, or this will be closed in 180 days

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 14, 2021
@antoninbas antoninbas removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 14, 2021
@github-actions
Copy link
Contributor

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 18, 2021
@antoninbas antoninbas removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 18, 2021
@github-actions
Copy link
Contributor

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 18, 2022
@antoninbas antoninbas removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 18, 2022
@NamanAg30
Copy link
Contributor

I have started working on this issue

@antoninbas
Copy link
Contributor Author

@NamanAg30 I don't think this issue can be resolved before #2743, and that issue depends on the K8s testbeds we use for CI being upgraded to a more recent version

@github-actions
Copy link
Contributor

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 13, 2022
@antoninbas antoninbas removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 13, 2022
@github-actions
Copy link
Contributor

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 12, 2022
@antoninbas antoninbas removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 12, 2022
@github-actions
Copy link
Contributor

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 11, 2023
@antoninbas antoninbas removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 11, 2023
@antoninbas
Copy link
Contributor Author

@NamanAg30 are you still working on this? It would be good to remove this code.

@antoninbas
Copy link
Contributor Author

It's time we remove this folder, this issue is 2 years old now. I will take care of it.
I have observed by looking at an existing jenkins-networkpolicy job that we run upstream tests using the v1.25.5 conformance image, which includes Netpol tests.

$ yq '.items[0].items[0].items[] | select(.name == "*Netpol*")' sonobuoy_results.yaml
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should support allow-all policy [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should work with Ingress, Egress specified together [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy based on NamespaceSelector with MatchExpressions[Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce multiple egress policies with egress allow-all policy taking precedence [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy based on PodSelector or NamespaceSelector [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy to allow ingress traffic from pods in all namespaces [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should allow egress access on one named port [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should support a ''default-deny-all'' policy [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should allow ingress access from namespace on one named port [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce multiple ingress policies with ingress allow-all policy taking precedence [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy based on PodSelector and NamespaceSelector [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce multiple, stacked policies with overlapping podSelectors [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce updated policy [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy based on any PodSelectors [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy based on PodSelector with MatchExpressions[Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy to allow ingress traffic for a target [Feature:NetworkPolicy] '
status: passed
name: '[It] [sig-network] Netpol [Feature:SCTPConnectivity][LinuxOnly][Disruptive] NetworkPolicy between server and client using SCTP should support a ''default-deny-ingress'' policy [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should support a ''default-deny-ingress'' policy [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy to allow traffic from pods within server namespace based on PodSelector [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy based on NamespaceSelector with MatchExpressions using default ns label [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should deny egress from pods based on PodSelector [Feature:NetworkPolicy] '
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy to allow traffic only from a different namespace, based on NamespaceSelector [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policies to check ingress and egress policies can be controlled independently based on PodSelector [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol [LinuxOnly] NetworkPolicy between server and client using UDP should support a ''default-deny-ingress'' policy [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should not allow access by TCP when a policy specifies only UDP [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should properly isolate pods that are selected by a policy allowing SCTP, even if the plugin doesn''t support SCTP [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should allow egress access to server in CIDR block [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should deny egress from all pods in a namespace [Feature:NetworkPolicy] '
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should deny ingress access to updated pod [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should ensure an IP overlapping both IPBlock.CIDR and IPBlock.Except is allowed [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol [LinuxOnly] NetworkPolicy between server and client using UDP should enforce policy to allow traffic only from a pod in a different namespace based on PodSelector and NamespaceSelector [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should support denying of egress traffic on the client side (even if the server explicitly allows this traffic) [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should deny ingress from pods on other namespaces [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol [Feature:SCTPConnectivity][LinuxOnly][Disruptive] NetworkPolicy between server and client using SCTP should enforce policy based on Ports [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy based on Multiple PodSelectors and NamespaceSelectors [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should allow ingress access on one named port [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy based on Ports [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should allow ingress access from updated pod [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol API should support creating NetworkPolicy with Status subresource [Feature:NetworkPolicyStatus]'
status: skipped
name: '[It] [sig-network] Netpol [Feature:SCTPConnectivity][LinuxOnly][Disruptive] NetworkPolicy between server and client using SCTP should enforce policy to allow traffic only from a pod in a different namespace based on PodSelector and NamespaceSelector [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol API should support creating NetworkPolicy API with endport field'
status: skipped
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce except clause while egress access to server in CIDR block [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol [LinuxOnly] NetworkPolicy between server and client using UDP should enforce policy based on Ports [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy to allow traffic only from a pod in a different namespace based on PodSelector and NamespaceSelector [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should allow ingress access from updated namespace [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce policy to allow traffic based on NamespaceSelector with MatchLabels using default ns label [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce egress policy allowing traffic to a server in a different namespace based on PodSelector and NamespaceSelector [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should not mistakenly treat ''protocol: SCTP'' as ''protocol: TCP'', even if the plugin doesn''t support SCTP [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should stop enforcing policies after they are deleted [Feature:NetworkPolicy]'
status: passed
name: '[It] [sig-network] Netpol API should support creating NetworkPolicy API operations'
status: skipped
name: '[It] [sig-network] Netpol NetworkPolicy between server and client should enforce ingress policy allowing any port traffic to a server on a specific protocol [Feature:NetworkPolicy] [Feature:UDP]'
status: passed

Given that observation, I think it is fine to simply remove hack/netpol, and the corresponding Kind test. If someone else wants to open a PR to add a new Kind job dedicated to running the upstream Netpol tests, it is fine by me (in case we forget to run jenkins-networkpolicy, although the upstream tests can take a while to run).

Maybe @NamanAg30 will bring #3825 back to life

@antoninbas antoninbas self-assigned this Mar 31, 2023
antoninbas added a commit to antoninbas/antrea that referenced this issue Mar 31, 2023
hack/netpol was always meant as a temporary placeholder, while new
Netpol tests were being contributed to upstream K8s. We have been
meaning to remove the folder for 2 years.

Note that the jenkins-networkpolicy CI job runs a recent version of K8s
e2e tests (1.25+), which includes the Netpol test suite. We could also
consider adding a new Kind CI job (as a Github workflow) dedicated to
running the upstream Netpol tests, but it is not strictly necessary and
could be an extra burden on our Github CI, as the tests take some time
to complete.

Fixes antrea-io#1740

Signed-off-by: Antonin Bas <abas@vmware.com>
antoninbas added a commit to antoninbas/antrea that referenced this issue Apr 3, 2023
hack/netpol was always meant as a temporary placeholder, while new
Netpol tests were being contributed to upstream K8s. We have been
meaning to remove the folder for 2 years.

Note that the jenkins-networkpolicy CI job runs a recent version of K8s
e2e tests (1.25+), which includes the Netpol test suite. We could also
consider adding a new Kind CI job (as a Github workflow) dedicated to
running the upstream Netpol tests, but it is not strictly necessary and
could be an extra burden on our Github CI, as the tests take some time
to complete.

Fixes antrea-io#1740

Signed-off-by: Antonin Bas <abas@vmware.com>
antoninbas added a commit that referenced this issue Apr 6, 2023
hack/netpol was always meant as a temporary placeholder, while new
Netpol tests were being contributed to upstream K8s. We have been
meaning to remove the folder for 2 years.

Note that the jenkins-networkpolicy CI job runs a recent version of K8s
e2e tests (1.25+), which includes the Netpol test suite. We could also
consider adding a new Kind CI job (as a Github workflow) dedicated to
running the upstream Netpol tests, but it is not strictly necessary and
could be an extra burden on our Github CI, as the tests take some time
to complete.

Fixes #1740

Signed-off-by: Antonin Bas <abas@vmware.com>
jainpulkit22 pushed a commit to urharshitha/antrea that referenced this issue Apr 28, 2023
hack/netpol was always meant as a temporary placeholder, while new
Netpol tests were being contributed to upstream K8s. We have been
meaning to remove the folder for 2 years.

Note that the jenkins-networkpolicy CI job runs a recent version of K8s
e2e tests (1.25+), which includes the Netpol test suite. We could also
consider adding a new Kind CI job (as a Github workflow) dedicated to
running the upstream Netpol tests, but it is not strictly necessary and
could be an extra burden on our Github CI, as the tests take some time
to complete.

Fixes antrea-io#1740

Signed-off-by: Antonin Bas <abas@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/exp/netpol Issues or PRs related to the netpol proposal. area/test/community Issues or PRs related to community testing good first issue Good for newcomers kind/design Categorizes issue or PR as related to design. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
3 participants