Skip to content

NO-ISSUE: Remove remaining webhook tests from serial #520

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

NO-ISSUE: Remove remaining webhook tests from serial #520

wants to merge 1 commit into from

Conversation

perdasilva
Copy link
Contributor

@perdasilva perdasilva commented Oct 13, 2025 •
edited
Loading

In the current state only the validating webhook test was not in the [Serial] OTE profile. This PR updates the remaining specs out of serial.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 13, 2025
@openshift-ci-robot
Copy link

@perdasilva: This pull request explicitly references no jira issue.

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.

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Oct 13, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: perdasilva

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 13, 2025
@perdasilva perdasilva force-pushed the webhook-test-no-serial branch from b4475b9 to bc138ad Compare October 13, 2025 15:59
@tmshort
Copy link
Contributor

tmshort commented Oct 13, 2025

/hold
This needs to occur after PR #519

@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 Oct 13, 2025
anik120
anik120 reviewed Oct 13, 2025
View reviewed changes
Copy link
Contributor

@anik120 anik120 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 13, 2025
camilamacedo86
camilamacedo86 reviewed Oct 13, 2025
View reviewed changes
openshift/tests-extension/.openshift-tests-extension/openshift_payload_olmv1.json
},
{
"name": "[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServiceCA] OLMv1 operator with webhooks should have a working mutating webhook [Serial]",
"name": "[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServiceCA] OLMv1 operator with webhooks should have a working mutating webhook",
Copy link
Contributor

@camilamacedo86 camilamacedo86 Oct 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/hold

Those tests were created as SERIAL because all tests were using the same sample.
Therefore, they could not run in parallel.

Now, if we want to move away from SERIAL we must:

  • Rename as done
  • Then, ensure that they are not serial see Ordered, Serial, func() { <- It must be changed
  • Ensure that each test will create a sample with a unique name ( bundle, CE and all that is used by each test must be unique )
  • Ensure that it works fine and will not cause flakes by testing with payload-aggregation 10 times.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure with Todd's changes we are generating unique images now, so no serial should work. I'll start the aggregate jobs. If those pass please unblock and merge.

Copy link
Contributor

@camilamacedo86 camilamacedo86 Oct 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are using unqiue names then:
We still needing to change Ordered, Serial, func() { <- That is what make the test run as serial
See;

operator-framework-operator-controller/openshift/tests-extension/test/webhooks.go

Line 43 in bc138ad

Ordered, Serial, func() {

The TAG [SERIAL] is valid for OCP CI ecosystem only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird - maybe these have different meanings then because now the tests run in the periodic-ci-openshift-release-master-nightly-4.20-e2e-metal-ipi-ovn-ipv6-techpreview job... I'll remove them though

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Oct 13, 2025

@perdasilva: all tests passed!

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.

@perdasilva
Copy link
Contributor Author

/payload-aggregate periodic-ci-openshift-release-master-nightly-4.20-e2e-metal-ipi-ovn-ipv6-techpreview 10

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Oct 13, 2025

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

  • periodic-ci-openshift-release-master-nightly-4.20-e2e-metal-ipi-ovn-ipv6-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/39cb2850-a879-11f0-8afe-24e1601928fb-0

@kuiwang02
Copy link
Contributor

@perdasilva

currently these are four webhook cases:
1, should have a working validating webhook
2, should have a working mutating webhook
3, should have a working conversion webhook
4, should be tolerant to tls secret deletion
they are using same package, and olmv1 install package at cluster level.
so, they are possible unstable if they are running in parallel. it is why I change only one to parallel, and keep others as serial.

we could research and verify if they are running in parallel without any conflicts. I will try it soon.
before that, I would expect it is held.

/hold

but for 4), it could be parallel even we found way to make webhook case in parallel because it is deletion operation.
so, finally 1,2,and 3 maybe be parallel, and 4 still is serial.

@perdasilva perdasilva force-pushed the webhook-test-no-serial branch from bc138ad to 41f798c Compare October 14, 2025 07:33
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2025
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Oct 14, 2025

New changes are detected. LGTM label has been removed.

@perdasilva
Copy link
Contributor Author

/payload-aggregate periodic-ci-openshift-release-master-nightly-4.20-e2e-metal-ipi-ovn-ipv6-techpreview 10

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Oct 14, 2025

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

  • periodic-ci-openshift-release-master-nightly-4.20-e2e-metal-ipi-ovn-ipv6-techpreview

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/a59c4030-a8d0-11f0-810e-0ecd61737c15-0

@perdasilva perdasilva closed this Oct 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@grokspawn grokspawn Awaiting requested review from grokspawn

2 more reviewers

@anik120 anik120 anik120 left review comments

@camilamacedo86 camilamacedo86 camilamacedo86 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@anik120 anik120

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@perdasilva @openshift-ci-robot @tmshort @kuiwang02 @anik120 @camilamacedo86