Skip to content

Conversation

@zeeke
Copy link
Contributor

@zeeke zeeke commented May 18, 2023

Reverts

The original implementation of these tests contains a bug when the networks.operator.openshift.io.Spec.UseMultiNetworkPolicy is nil. This PR fix that.

CC: @s1061123 , @bparees, @deepsm007

@zeeke zeeke changed the title 2x Revert "test/extended: Add MultiNetworkPolicy IPv4/IPv6 test cases" #27926 OCPBUGS-13788: 2x Revert "test/extended: Add MultiNetworkPolicy IPv4/IPv6 test cases" #27926 May 18, 2023
@openshift-ci-robot openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels May 18, 2023
@openshift-ci-robot
Copy link

@zeeke: This pull request references Jira Issue OCPBUGS-13788, which is invalid:

  • expected the bug to target the "4.14.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Reverts

The original implementation of these tests contains a bug when the networks.operator.openshift.io.Spec.UseMultiNetworkPolicy is nil. This PR fix that.

CC: @s1061123 , @bparees

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.

@openshift-ci-robot
Copy link

@zeeke: This pull request references Jira Issue OCPBUGS-13788, which is invalid:

  • expected the bug to target the "4.14.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Reverts

The original implementation of these tests contains a bug when the networks.operator.openshift.io.Spec.UseMultiNetworkPolicy is nil. This PR fix that.

CC: @s1061123 , @bparees, @deepsm007

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.

@dcbw
Copy link
Contributor

dcbw commented May 18, 2023

/test e2e-metal-ipi-ovn-ipv6

@deepsm007
Copy link
Contributor

Can we hold off this PR until we land kube rebase in origin?
#27894
it should be in final checks but keeping you aware

1 similar comment
@deepsm007
Copy link
Contributor

Can we hold off this PR until we land kube rebase in origin?
#27894
it should be in final checks but keeping you aware

@zeeke
Copy link
Contributor Author

zeeke commented May 18, 2023

It's ok for me

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 18, 2023
@bparees
Copy link
Contributor

bparees commented May 18, 2023

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 18, 2023
@deepsm007
Copy link
Contributor

@zeeke the kube rebase PR has landed. We are good to go.

@stbenjam
Copy link
Member

Is there proof the test passes reliably? I'm sure this fixes the panic but we'd like to make sure this test isn't going to cause additional problems.

You can use /payload-aggregate to kick off a number of jobs, 10 sounds about right. I think that's what @deepsm007 suggested in #27926 (comment).

@zeeke
Copy link
Contributor Author

zeeke commented May 22, 2023

/payload-aggregate periodic-ci-openshift-release-master-nightly-4.14-e2e-aws-sdn-serial 10

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 22, 2023

@zeeke: trigger 1 job(s) for the /payload-(job|aggregate) command

  • periodic-ci-openshift-release-master-nightly-4.14-e2e-aws-sdn-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/d350c960-f8a6-11ed-8bec-76c5fc2add9a-0

@zeeke
Copy link
Contributor Author

zeeke commented May 23, 2023

/payload-aggregate periodic-ci-openshift-release-master-nightly-4.14-e2e-aws-sdn-serial 10

2 failure job out of 10:

failures:
- jobrunid: "1660642988310663168"
  humanurl: https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/openshift-origin-27927-nightly-4.14-e2e-aws-sdn-serial/1660642988310663168
  gcsartifacturl: https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/logs/openshift-origin-27927-nightly-4.14-e2e-aws-sdn-serial/1660642988310663168/artifacts
- jobrunid: "1660642990193905664"
  humanurl: https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/openshift-origin-27927-nightly-4.14-e2e-aws-sdn-serial/1660642990193905664
  gcsartifacturl: https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/logs/openshift-origin-27927-nightly-4.14-e2e-aws-sdn-serial/1660642990193905664/artifacts

Both have these test passing:

[sig-network][Feature:MultiNetworkPolicy][Serial] should enforce a network policies on secondary network IPv6 [Suite:openshift/conformance/serial]
[sig-network][Feature:MultiNetworkPolicy][Serial] should enforce a network policies on secondary network IPv4 [Suite:openshift/conformance/serial]

Is there any other job I can run to be more confident?

@zeeke
Copy link
Contributor Author

zeeke commented May 23, 2023

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 23, 2023
@stbenjam
Copy link
Member

stbenjam commented May 23, 2023

The IPv4 test case looks ok with 9/10 passes but IPv6 failed 4x, any idea why? Shouldn't it just be skipped on the IPv4 serial job?


Passed Tests

Passed: suite=[openshift-tests], [sig-network][Feature:MultiNetworkPolicy][Serial] should enforce a network policies on secondary network IPv6 [Suite:openshift/conformance/serial]
Passed: Passed 6 times, failed 4 times. The historical pass rate is 0%. The required number of passes is 0.

Failure - periodic-ci-openshift-release-master-nightly-4.14-e2e-aws-sdn-serial/1660642986024767488
Failure - periodic-ci-openshift-release-master-nightly-4.14-e2e-aws-sdn-serial/1660642986934931456
Failure - periodic-ci-openshift-release-master-nightly-4.14-e2e-aws-sdn-serial/1660642988755259392
Failure - periodic-ci-openshift-release-master-nightly-4.14-e2e-aws-sdn-serial/1660642989204049920


Passed: suite=[openshift-tests], [sig-network][Feature:MultiNetworkPolicy][Serial] should enforce a network policies on secondary network IPv4 [Suite:openshift/conformance/serial]
Passed: Passed 9 times, failed 1 times. The historical pass rate is 57%. The required number of passes is 2.

Failure - periodic-ci-openshift-release-master-nightly-4.14-e2e-aws-sdn-serial/1660642987853484032

@zeeke
Copy link
Contributor Author

zeeke commented May 23, 2023

@stbenjam thanks for that report. I'm not used to aggregated jobs.

I looked at the error, and it seems Cluster Network Operator can take more than 30s to turn off the multinetwork-policy feature.

failed test logs

May 22 15:40:26.188: INFO: Running 'oc --namespace=e2e-test-multinetpol-e2e-mjqg9 --kubeconfig=/tmp/kubeconfig-926409639 get multi-networkpolicies.k8s.cni.cncf.io'
NAME                      AGE
deny-ingress-pod-a7zb85   32s
[DeferCleanup (Each)] [sig-network][Feature:MultiNetworkPolicy][Serial]
  dump namespaces | framework.go:196
STEP: dump namespace information after failure 05/22/23 15:40:27.091
[DeferCleanup (Each)] [sig-network][Feature:MultiNetworkPolicy][Serial]
  tear down framework | framework.go:193
STEP: Destroying namespace "e2e-test-multinetpol-e2e-mjqg9" for this suite. 05/22/23 15:40:27.091
fail [github.com/openshift/origin/test/extended/networking/multinetpolicy.go:169]: Timed out after 30.001s.
Expected an error to have occurred.  Got:
    <nil>: nil

CNO logs, 5 seconds later

I0522 15:40:31.125368       1 log.go:194] Detected related object with GVK apiextensions.k8s.io/v1, Kind=CustomResourceDefinition, namespace  and name multi-networkpolicies.k8s.cni.cncf.io not rendered by manifests, deleting...

From the test point of view, it should be ok to increase the assertion timeout, but CNO seems to be very overloaded

@stbenjam
Copy link
Member

/payload-aggregate periodic-ci-openshift-release-master-nightly-4.14-e2e-aws-sdn-serial 10

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 23, 2023

@stbenjam: trigger 1 job(s) for the /payload-(job|aggregate) command

  • periodic-ci-openshift-release-master-nightly-4.14-e2e-aws-sdn-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/024331e0-f988-11ed-8160-06af795bddca-0

@stbenjam
Copy link
Member

Looks like it failed to build with some errors here:

 test/extended/networking/multinetpolicy.go:66:50: not enough arguments in call to e2enode.GetReadySchedulableNodes
	have (kubernetes.Interface)
	want ("context".Context, kubernetes.Interface)
test/extended/networking/multinetpolicy.go:73:70: not enough arguments in call to frameworkpod.CreateExecPodOrFail
	have (kubernetes.Interface, string, string, func(pod *"k8s.io/api/core/v1".Pod))
	want ("context".Context, kubernetes.Interface, string, string, func(*"k8s.io/api/core/v1".Pod))
test/extended/networking/multinetpolicy.go:81:74: not enough arguments in call to frameworkpod.CreateExecPodOrFail
	have (kubernetes.Interface, string, string, func(pod *"k8s.io/api/core/v1".Pod))
	want ("context".Context, kubernetes.Interface, string, string, func(*"k8s.io/api/core/v1".Pod))
test/extended/networking/multinetpolicy.go:88:62: not enough arguments in call to frameworkpod.CreateExecPodOrFail
	have (kubernetes.Interface, string, string, func(pod *"k8s.io/api/core/v1".Pod))
	want ("context".Context, kubernetes.Interface, string, string, func(*"k8s.io/api/core/v1".Pod))
make: *** [vendor/github.com/openshift/build-machinery-go/make/targets/golang/build.mk:16: build] Error 1 

/payload-abort

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 23, 2023

@stbenjam: aborted active payload jobs for pull request #27927

zeeke added 3 commits May 24, 2023 08:55
`networks.operator.openshift.io.Spec.UseMultiNetworkPolicy` is nil by
default. Avoid referencing it without checking it before.

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
Enabling MultiNetworkPolicy feature on Cluster Network
Operator may take more than 30s and lead to flaky tests.

Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
@zeeke zeeke force-pushed the multinetpolicy-revert branch from 6448b0e to fd0255a Compare May 24, 2023 07:08
@zeeke
Copy link
Contributor Author

zeeke commented May 24, 2023

/retest

@zeeke
Copy link
Contributor Author

zeeke commented May 24, 2023

/payload-aggregate periodic-ci-openshift-release-master-nightly-4.14-e2e-aws-sdn-serial 10

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 24, 2023

@zeeke: trigger 1 job(s) for the /payload-(job|aggregate) command

  • periodic-ci-openshift-release-master-nightly-4.14-e2e-aws-sdn-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/2180a9f0-fa3e-11ed-8d47-a10ffec82daa-0

@zeeke
Copy link
Contributor Author

zeeke commented May 25, 2023

@stbenjam
Copy link
Member

Great thanks
/lgtm
/jira-refresh

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 25, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 25, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, stbenjam, 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:

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

@zeeke
Copy link
Contributor Author

zeeke commented May 25, 2023

/jira refresh

@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels May 25, 2023
@openshift-ci-robot
Copy link

@zeeke: This pull request references Jira Issue OCPBUGS-13788, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.14.0) matches configured target version for branch (4.14.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira (weliang@redhat.com), skipping review request.

Details

In response to this:

/jira refresh

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.

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD ace92ea and 2 for PR HEAD fd0255a in total

@zeeke
Copy link
Contributor Author

zeeke commented May 26, 2023

/re

@zeeke zeeke closed this May 26, 2023
@openshift-ci-robot
Copy link

@zeeke: This pull request references Jira Issue OCPBUGS-13788. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state.

Details

In response to this:

Reverts

The original implementation of these tests contains a bug when the networks.operator.openshift.io.Spec.UseMultiNetworkPolicy is nil. This PR fix that.

CC: @s1061123 , @bparees, @deepsm007

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.

@zeeke
Copy link
Contributor Author

zeeke commented May 26, 2023

Closed by mistake. reopening

@zeeke zeeke reopened this May 26, 2023
@openshift-ci-robot
Copy link

@zeeke: This pull request references Jira Issue OCPBUGS-13788, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.14.0) matches configured target version for branch (4.14.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

No GitHub users were found matching the public email listed for the QA contact in Jira (weliang@redhat.com), skipping review request.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Reverts

The original implementation of these tests contains a bug when the networks.operator.openshift.io.Spec.UseMultiNetworkPolicy is nil. This PR fix that.

CC: @s1061123 , @bparees, @deepsm007

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.

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 8156220 and 1 for PR HEAD fd0255a in total

@zeeke
Copy link
Contributor Author

zeeke commented May 26, 2023

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 26, 2023

@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-aws-ovn-etcd-scaling 04029f7 link false /test e2e-aws-ovn-etcd-scaling
ci/prow/e2e-azure-ovn-etcd-scaling 04029f7 link false /test e2e-azure-ovn-etcd-scaling
ci/prow/e2e-vsphere-ovn-etcd-scaling 04029f7 link false /test e2e-vsphere-ovn-etcd-scaling
ci/prow/e2e-aws-ovn-single-node-upgrade fd0255a link false /test e2e-aws-ovn-single-node-upgrade
ci/prow/e2e-aws-ovn-single-node-serial fd0255a link false /test e2e-aws-ovn-single-node-serial
ci/prow/e2e-agnostic-ovn-cmd fd0255a link false /test e2e-agnostic-ovn-cmd
ci/prow/e2e-aws-ovn-upgrade fd0255a link false /test e2e-aws-ovn-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.

@openshift-merge-robot openshift-merge-robot merged commit f47ed15 into openshift:master May 26, 2023
@openshift-ci-robot
Copy link

@zeeke: Jira Issue OCPBUGS-13788: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-13788 has been moved to the MODIFIED state.

Details

In response to this:

Reverts

The original implementation of these tests contains a bug when the networks.operator.openshift.io.Spec.UseMultiNetworkPolicy is nil. This PR fix that.

CC: @s1061123 , @bparees, @deepsm007

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants