-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
SDN-5017: Add UDN Network Policy e2e tests #29195
Conversation
cb413c2
to
ba251b5
Compare
Job Failure Risk Analysis for sha: ba251b5
|
/test e2e-gcp-ovn-techpreview |
/test e2e-aws-ovn-single-node-techpreview |
ba251b5
to
25b598c
Compare
/test e2e-gcp-ovn-techpreview |
1 similar comment
/test e2e-gcp-ovn-techpreview |
25b598c
to
9475099
Compare
/test e2e-gcp-ovn-techpreview |
Job Failure Risk Analysis for sha: 9475099
|
9475099
to
a8af350
Compare
/assign @ricky-rav Th tests are ready for review now except NetPol tests are failing due to the bug OCPBUGS-43519. |
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.
Overall looks good, I just added a minor comment.
I compared the code in test/extended/networking/network_segmentation_policy.go
to what we have upstream in https://github.com/ovn-kubernetes/ovn-kubernetes/blob/master/test/e2e/network_segmentation_policy.go . I see you made a few changes especially for node selection and to adapt the code to what we have in origin to test reachability between two pods. All that looks good to me.
Can you just add a few words to the commit message saying that we're porting to origin the e2e tests we have upstream for network policies and can you also mention also the command you used to add the generate code?
Thanks, Peri!
return err | ||
}, "30s", "1s").ShouldNot(o.HaveOccurred(), "cmd output: %s", out) | ||
} | ||
|
||
func podShouldNotReach(oc *exutil.CLI, podName, address string) { | ||
namespacePodShouldNotReach(oc, "", podName, address) |
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.
Your version is totally equivalent, but I think it's cleaner to just pass "default" as namespace, instead of relying on the empty string.
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.
@ricky-rav passing empty string for namespace is intentional here so that oc exec uses pod from f.Namespace.Name
namespace.
out := "" | ||
o.EventuallyWithOffset(1, func() error { | ||
var err error | ||
out, err = oc.AsAdmin().Run("exec").Args(podName, "--", "curl", "--connect-timeout", "1", address).Output() | ||
if namespace == "" { |
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 thing here
clientPodConfig.namespace = f.Namespace.Name | ||
clientPodConfig.nodeSelector = map[string]string{nodeHostnameKey: workerNodes[0].Name} | ||
serverPodConfig.namespace = f.Namespace.Name | ||
serverPodConfig.nodeSelector = map[string]string{nodeHostnameKey: workerNodes[len(workerNodes)-1].Name} |
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.
OK, so in case of a single-node cluster, this well schedule the server pod on the only available node.
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.
yes, right. guess that should be fine.
Job Failure Risk Analysis for sha: a8af350
|
This commit is porting e2e tests (https://github.com/ovn-kubernetes/ovn-kubernetes/blob/master/test/e2e/network_segmentation_policy.go) we have upstream for network policies into origin. The `make update` command is run to generate required artifacts for the tests. Signed-off-by: Periyasamy Palanisamy <[email protected]>
a8af350
to
544fc05
Compare
/hold cancel NetPol tests are passing now for primary UDN. |
@pperiyasamy: This pull request references SDN-5017 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
/lgtm |
/test e2e-metal-ipi-ovn-ipv6-techpreview |
/test e2e-metal-ipi-ovn-ipv6-techpreview |
@pperiyasamy: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/payload-job periodic-ci-openshift-release-master-nightly-4.19-e2e-metal-ipi-ovn-techpreview |
@pperiyasamy: trigger 3 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/82215190-bc72-11ef-99f1-280124d36f30-0 |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dgoodwin, pperiyasamy, ricky-rav The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/cherry-pick release-4.18 |
@tssurya: once the present PR merges, I will cherry-pick it on top of In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
1 similar comment
@pperiyasamy: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Job Failure Risk Analysis for sha: 544fc05
|
b331d38
into
openshift:master
@tssurya: new pull request created: #29385 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
[ART PR BUILD NOTIFIER] Distgit: openshift-enterprise-tests |
No description provided.