NO-ISSUE: UPSTREAM: <carry>: make webhook case in parallel as possible #522
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@kuiwang02: 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kuiwang02 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/payload-aggregate periodic-ci-openshift-release-master-nightly-4.21-e2e-metal-ipi-ovn-ipv6-techpreview 5 |
|
/payload-aggregate periodic-ci-openshift-release-master-nightly-4.21-e2e-metal-ipi-serial-ovn-ipv6-techpreview-1of2 5 |
|
@kuiwang02: trigger 1 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/29ee9f50-a8d5-11f0-9faf-3ce017251e3c-0 |
|
/payload-aggregate periodic-ci-openshift-release-master-nightly-4.21-e2e-metal-ipi-serial-ovn-ipv6-techpreview-2of2 5 |
|
@kuiwang02: trigger 1 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/30c73f30-a8d5-11f0-8208-a2ad207adfea-0 |
|
/payload-aggregate periodic-ci-openshift-release-master-ci-4.21-e2e-gcp-ovn-techpreview 5 |
|
@kuiwang02: trigger 1 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/38daa900-a8d5-11f0-90de-e05da449773b-0 |
|
@kuiwang02: trigger 1 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/40989fd0-a8d5-11f0-94e4-4e81c75cc544-0 |
|
/payload-aggregate periodic-ci-openshift-release-master-ci-4.21-e2e-gcp-ovn-techpreview-serial 5 |
|
@kuiwang02: trigger 1 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/47f563d0-a8d5-11f0-8a0c-c0779eb1d1a3-0 |
|
@kuiwang02: 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. |
| var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServiceCA] OLMv1 operator with webhooks", | ||
| Ordered, Serial, func() { | ||
| // Parallel webhook tests - can run concurrently | ||
| var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServiceCA] OLMv1 operator with webhooks - parallel tests", |
There was a problem hiding this comment.
Do we need parallel test in the name?
| } | ||
| // Note: cleanup is now handled by DeferCleanup in BeforeEach, which ensures | ||
| // cleanup runs even if BeforeEach or the test fails | ||
| }) |
There was a problem hiding this comment.
If it passes in the payload aggregation, I am OK with it.
However, running as SERIAL we have the data that we need see:
- Go to : https://sippy.dptools.openshift.org/sippy-ng/tests/4.21
- Search for the suite ID like; NewOLMWebhook
- Then, you will see all tests. Adding image below
- By last if we click in ANY, we can see the execution with IPv6
Example
So, I am not sure if we need to change anything since our goal is only make the tests in a way that pass in the validation of this PR: openshift/api#2491. See the result from yesterday exxecution: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_api/[…]ft-api-master-verify-feature-promotion/1977693404213022720
It is no longer saying 0, but it is saying it has X execution and needs 14.
Is not accurate. For OLMv1 we specifically want to stay away from multi-tenancy type features. We should not be able to install the same operator twice. Upstream:CRD Gets deleted with ClusterExtension(olme2etests.olm.operatorframework.io is deleted) No two operators with the same CRDIf we try to install two packages with the same CRD, I get an error like: If we try to install the same operator twice, we'll get a conflict on cluster resources like ClusterRole. |
|
/hold Our goal is not to remove SERIAL, just to have a test that runs in a disconnected env / ipv6 to allow us to promote the feature to GA More info see: https://redhat-internal.slack.com/archives/C09EPHWT0NM/p1760460889991629 and #522 (comment) If we indeed change the name, it will delay us from achieving the goal. (We have a few X number of executions already ) |
openshift-ci
bot
added
the
do-not-merge/hold
|
/close |
|
@kuiwang02: Closed this PR. 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. |
4 participants
Summary
Enable parallel execution for webhook operator tests to reduce overall test execution time. This PR splits the webhook tests into two groups: parallel tests (validating, mutating, and conversion webhooks) and serial tests (TLS secret deletion).
Changes
1. Split Tests into Two Describe Blocks
Parallel Tests (Lines 42-177)
Serialmarker from the Describe block[Serial]marker from individual test cases:should have a working validating webhookshould have a working mutating webhookshould have a working conversion webhookBeforeEachto allow multiple ClusterExtensions to coexistSerial Tests (Lines 180-306)
SerialmarkerArchitecture Analysis
Resource Isolation Strategy
When parallel tests run in separate processes on the same cluster:
Per-Process Resources (Isolated)
webhook-operator-<random>(unique per test)Shared Resources (Cluster-Scoped)
webhooktests.webhook.operators.coreos.io(shared)CRD Conflict Resolution
Question: When multiple ClusterExtensions try to install the same CRD simultaneously, what happens?
Answer: OLM handles this gracefully because:
boxcutter.go:226), which allows multiple field managers to co-manage the same resourceEvidence from Code:
Cleanup Behavior
When a ClusterExtension is deleted:
This behavior is intentional and allows parallel tests to safely delete their ClusterExtensions without affecting other running tests.
Known Side Effects (Acceptable)
Each WebhookTest resource creation will be validated/mutated by all active webhook configurations (~3x in parallel execution).
Impact:
The CRD contains a hardcoded namespace reference (webhook-operator-system), but actual deployments use random namespaces (webhook-operator-).
Impact:
The serial test block uses helpers.EnsureCleanupClusterExtension() to clean up all ClusterExtensions and the shared CRD before running. This ensures the TLS deletion test runs in a clean environment.
Testing Strategy
Execution Flow
Phase 1: Parallel Tests (Processes 1-3)
T0: All processes start simultaneously
├─ Process 1: Creates CE "webhook-operator-abc12" → OLM creates CRD ✅
├─ Process 2: Creates CE "webhook-operator-def34" → OLM detects existing CRD, validates, continues ✅
└─ Process 3: Creates CE "webhook-operator-ghi56" → OLM detects existing CRD, validates, continues ✅
T1: All tests execute in parallel
└─ Each test uses its isolated namespace and resources
T2: Tests complete and clean up
├─ Process 1: Deletes CE (CRD remains) ✅
├─ Process 2: Deletes CE (CRD remains) ✅
└─ Process 3: Deletes CE (CRD remains) ✅
Phase 2: Serial Test (After all parallel tests complete)
T3: Serial test starts
├─ Cleanup all remaining ClusterExtensions
├─ Delete shared CRD
├─ Create fresh ClusterExtension
└─ Run TLS deletion test
Verification Plan (run them more times and all pass)
Assisted-by: Claude Code