test/extended: Add MultiNetworkPolicy IPv4/IPv6 test cases #27795

zeeke · 2023-03-14T11:56:56Z

Adding basic test cases to multinetwork policy feature.

This PR is mainly to avoid regressions like OCPBUGS-6917 from occurring again.

zeeke · 2023-03-15T07:51:47Z

/retest

s1061123 · 2023-03-16T06:01:49Z

test/extended/networking/multinetpolicy.go

+	o.Eventually(func() error {
+		return oc.AsAdmin().Run("get").Args("multi-networkpolicies.k8s.cni.cncf.io").Execute()
+	}, "30s", "2s").Should(o.Succeed())


How about to check daemonset, multus-networkpolicy, instead of CRD, to capture multi-networkpolicy-iptables Pod failure before actual tests?

Good point, it has also been raised here.

From the user perspective, the presence of the CRD is the only sign of the feature, and the daemonset is an implementation detail. In the future, for instance, we can decide to keep the daemonset always up and running but not do anything if the feature is disabled.

But if you think this check can reduce flakiness in some circumstances, I'll go for ti.

I believe we need to check the daemonset readiness here for further troubleshooting.

Let's imagine we have a trouble in multi-networkpolicy-iptables Pod (like crashed and never goes to 'Running' state). In such case, current test just files error at 'traffic testing': 'podShoudNotReach() is failed'. Usually if we get 'podShouldNotReach() is failed' then we suspect that policy rule/iptables functionality. So it may be hard to proceed to troubleshoot.

If we identify the pod crash here, then easy to understand what is happen, so could you please check DaemonSet readiness, such like that?

origin/test/extended/dr/common.go

Lines 204 to 208 in 9f98fce

sdnDaemonset, err := oc.AdminKubeClient().AppsV1().DaemonSets("openshift-sdn").Get(context.Background(), "sdn", metav1.GetOptions{})

if err != nil {

return false, nil

}

return sdnDaemonset.Status.NumberReady == sdnDaemonset.Status.NumberAvailable, nil

Got it! Thanks for pointing out

s1061123 · 2023-03-16T06:02:10Z

test/extended/networking/multinetpolicy.go

+	o.Eventually(func() error {
+		return oc.AsAdmin().Run("get").Args("multi-networkpolicies.k8s.cni.cncf.io").Execute()
+	}, "30s", "1s").Should(o.HaveOccurred())


Ditto (check daemonset)?

zeeke · 2023-03-16T08:08:50Z

/retest

zeeke · 2023-03-17T11:18:18Z

/retest

zeeke · 2023-03-21T12:53:25Z

/retest

zeeke · 2023-03-27T11:20:06Z

@bparees @dgoodwin @s1061123
Please, take a look at this

bparees · 2023-03-27T13:50:06Z

test/extended/networking/multinetpolicy.go

+
+	g.AfterEach(func() {
+		if !previousUseMultiNetworkPolicy {
+			disableMultiNetworkPolicy(oc)


is this the only test in the entire test suite that does this? because this seems likely to be racy unless you only run tests that do this serially. I think we normally run these kinds of tests at least as serial, if not as disruptive.

testA: sees mutlinetwork is not enabled
testB: sees multinetwork is not enabled
testA: enables multinetwork
testB: enables multinetwork (no-op)
testB: completes
testB: disables multinetwork (because it was not enabled when testB started)
testA: gets broken by multinetwork being disabled in the middle of the test

You could slightly improve the behavior by only disabling multinetwork if this test was the one that enabled it, but you'd still be subject to races in which other tests start, see multinetwork is already enabled(by another test), but than have it disabled out from under them.

Oh, I haven't thought about it. I think marking them as serial should be enough. They are the only tests that use multinetwork policies.

zeeke · 2023-03-28T07:47:56Z

/retest

Test case use Cluster Network Operator field `useMultiNetworkPolicy` field and a basic `deny-all-ingress` policy to ensure the feature is correctly setup. Fixture involves a macvlan net-attach-def and network policy. Connectivity tests are implemented using agnhost http server and cURL. Tested against IPv4 and IPv6 network stacks. Signed-off-by: Andrea Panattoni <apanatto@redhat.com>

zeeke · 2023-03-29T15:56:35Z

/retest

zeeke · 2023-03-30T09:07:02Z

/retest

zeeke · 2023-03-30T12:13:38Z

/retest

zeeke · 2023-03-30T15:19:57Z

/retest-required

zeeke · 2023-04-03T07:16:18Z

/retest

s1061123 · 2023-04-20T16:09:00Z

/lgtm

zeeke · 2023-05-11T14:21:56Z

@bparees Please, take another look

bparees · 2023-05-11T15:13:13Z

test is now marked serial so it shouldn't have race issues w/ setting/unsetting the policy.

/approve

openshift-ci · 2023-05-11T15:16:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, s1061123, zeeke

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~test/OWNERS~~ [bparees]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2023-05-11T18:58:24Z

/retest-required

Remaining retests: 0 against base HEAD 2d41e52 and 2 for PR HEAD 0d6eaf6 in total

zeeke · 2023-05-15T08:23:37Z

/retest-required

zeeke · 2023-05-15T09:39:16Z

/retest-required

zeeke · 2023-05-15T15:47:55Z

/retest-required

openshift-ci-robot · 2023-05-16T15:47:48Z

/retest-required

Remaining retests: 0 against base HEAD 81c9056 and 1 for PR HEAD 0d6eaf6 in total

openshift-ci · 2023-05-16T18:19:03Z

@zeeke: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-azure-ovn-etcd-scaling	`0d6eaf6`	link	false	`/test e2e-azure-ovn-etcd-scaling`
ci/prow/e2e-openstack-ovn	`0d6eaf6`	link	false	`/test e2e-openstack-ovn`
ci/prow/e2e-vsphere-ovn-etcd-scaling	`0d6eaf6`	link	false	`/test e2e-vsphere-ovn-etcd-scaling`
ci/prow/e2e-gcp-ovn-etcd-scaling	`0d6eaf6`	link	false	`/test e2e-gcp-ovn-etcd-scaling`
ci/prow/e2e-aws-ovn-single-node-upgrade	`0d6eaf6`	link	false	`/test e2e-aws-ovn-single-node-upgrade`

Full PR test history. Your PR dashboard.

Details

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/test-infra repository. I understand the commands that are listed here.

zeeke · 2023-05-17T06:45:15Z

/retest-required

zeeke changed the title ~~test/extended: Add MultiNetworkPolicy test case~~ test/extended: Add MultiNetworkPolicy IPv4/IPv6 test cases Mar 14, 2023

openshift-ci bot requested review from beekhof and bparees March 14, 2023 11:57

zeeke force-pushed the multinetpolicy-v6 branch 2 times, most recently from 69002dc to 3a5d626 Compare March 14, 2023 17:30

zeeke mentioned this pull request Mar 14, 2023

test/extended: Add MultiNetworkPolicy test case #27449

Closed

zeeke force-pushed the multinetpolicy-v6 branch 2 times, most recently from 777c99c to ce758c8 Compare March 15, 2023 16:34

s1061123 reviewed Mar 16, 2023

View reviewed changes

bparees reviewed Mar 27, 2023

View reviewed changes

zeeke force-pushed the multinetpolicy-v6 branch from ce758c8 to 87ba78f Compare March 27, 2023 15:48

zeeke force-pushed the multinetpolicy-v6 branch from 87ba78f to 7a7598e Compare March 28, 2023 16:23

zeeke force-pushed the multinetpolicy-v6 branch from 7a7598e to 0d6eaf6 Compare March 29, 2023 11:26

zeeke requested review from bparees and s1061123 April 3, 2023 10:38

openshift-ci bot assigned s1061123 Apr 20, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 20, 2023

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 11, 2023

openshift-merge-robot merged commit ad257c8 into openshift:master May 17, 2023

soltysh mentioned this pull request May 18, 2023

STOR-1263: Rebase 1.27.1 #27925

Closed

deepsm007 mentioned this pull request May 18, 2023

Revert "test/extended: Add MultiNetworkPolicy IPv4/IPv6 test cases" #27926

Merged

	sdnDaemonset, err := oc.AdminKubeClient().AppsV1().DaemonSets("openshift-sdn").Get(context.Background(), "sdn", metav1.GetOptions{})
	if err != nil {
	return false, nil
	}
	return sdnDaemonset.Status.NumberReady == sdnDaemonset.Status.NumberAvailable, nil

test/extended: Add MultiNetworkPolicy IPv4/IPv6 test cases #27795

test/extended: Add MultiNetworkPolicy IPv4/IPv6 test cases #27795

Uh oh!

Conversation

zeeke commented Mar 14, 2023

Uh oh!

zeeke commented Mar 15, 2023

Uh oh!

s1061123 Mar 16, 2023

Choose a reason for hiding this comment

Uh oh!

zeeke Mar 16, 2023

Choose a reason for hiding this comment

Uh oh!

s1061123 Mar 28, 2023

Choose a reason for hiding this comment

Uh oh!

zeeke Mar 28, 2023

Choose a reason for hiding this comment

Uh oh!

s1061123 Mar 16, 2023

Choose a reason for hiding this comment

Uh oh!

zeeke commented Mar 16, 2023

Uh oh!

zeeke commented Mar 17, 2023

Uh oh!

zeeke commented Mar 21, 2023

Uh oh!

zeeke commented Mar 27, 2023

Uh oh!

bparees Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

zeeke Mar 27, 2023

Choose a reason for hiding this comment

Uh oh!

zeeke commented Mar 28, 2023

Uh oh!

zeeke commented Mar 29, 2023

Uh oh!

zeeke commented Mar 30, 2023

Uh oh!

zeeke commented Mar 30, 2023

Uh oh!

zeeke commented Mar 30, 2023

Uh oh!

zeeke commented Apr 3, 2023

Uh oh!

s1061123 commented Apr 20, 2023

Uh oh!

zeeke commented May 11, 2023

Uh oh!

bparees commented May 11, 2023

Uh oh!

openshift-ci bot commented May 11, 2023

Uh oh!

openshift-ci-robot commented May 11, 2023

Uh oh!

zeeke commented May 15, 2023

Uh oh!

zeeke commented May 15, 2023

Uh oh!

zeeke commented May 15, 2023

Uh oh!

openshift-ci-robot commented May 16, 2023

Uh oh!

openshift-ci bot commented May 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeeke commented May 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

openshift-ci bot commented May 16, 2023 •

edited

Loading