OPRUN-4437: test: add allow-case for operator maxOCPVersion > cluster version#765
OPRUN-4437: test: add allow-case for operator maxOCPVersion > cluster version#765tmshort wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@tmshort: This pull request references OPRUN-4437 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 "5.0.0" version, but no target version was set. DetailsIn 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 APPROVED This pull-request has been approved by: tmshort The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
WalkthroughThe OLMv1 incompatibility test computes the next OpenShift minor version, uses it for bundle replacement during operator installation, and verifies that ChangesOLMv1 upgradeability validation
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant OLMv1Test
participant Cluster
participant ClusterExtension
OLMv1Test->>Cluster: Check InstalledOLMOperatorUpgradable
OLMv1Test->>ClusterExtension: Install operator with next minor VERSION
ClusterExtension->>Cluster: Report installed operator
OLMv1Test->>Cluster: Re-check InstalledOLMOperatorUpgradable
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openshift/tests-extension/test/olmv1-incompatible.go`:
- Around line 108-118: Update the two By progress messages in the test around
waitForOlmUpgradeStatus to use the asserted condition name,
InstalledOLMOperatorsUpgradeable, matching the condition checked by
waitForOlmUpgradeStatus.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 24014f06-4e03-4d82-a555-6d538f998b46
📒 Files selected for processing (1)
openshift/tests-extension/test/olmv1-incompatible.go
| It("should not block cluster upgrades if the operator's maxOCPVersion exceeds the current cluster version", func(ctx SpecContext) { | ||
| By("waiting for InstalledOLMOperatorUpgradable to be true") | ||
| waitForOlmUpgradeStatus(ctx, operatorv1.ConditionTrue, "") | ||
|
|
||
| By("creating the ClusterExtension") | ||
| ceName, ceCleanup := helpers.CreateClusterExtension(opName, "", nsName, unique, helpers.WithCatalogNameSelector(ccName)) | ||
| DeferCleanup(ceCleanup) | ||
| helpers.ExpectClusterExtensionToBeInstalled(ctx, ceName) | ||
|
|
||
| By("verifying InstalledOLMOperatorUpgradable remains true after installing the operator") | ||
| waitForOlmUpgradeStatus(ctx, operatorv1.ConditionTrue, "") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the asserted condition name in progress output.
Line 109 and Line 117 say InstalledOLMOperatorUpgradable, but waitForOlmUpgradeStatus checks InstalledOLMOperatorsUpgradeable. Correcting this keeps CI timelines and failure diagnostics unambiguous.
Proposed fix
- By("waiting for InstalledOLMOperatorUpgradable to be true")
+ By("waiting for InstalledOLMOperatorsUpgradeable to be true")
...
- By("verifying InstalledOLMOperatorUpgradable remains true after installing the operator")
+ By("verifying InstalledOLMOperatorsUpgradeable remains true after installing the operator")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| It("should not block cluster upgrades if the operator's maxOCPVersion exceeds the current cluster version", func(ctx SpecContext) { | |
| By("waiting for InstalledOLMOperatorUpgradable to be true") | |
| waitForOlmUpgradeStatus(ctx, operatorv1.ConditionTrue, "") | |
| By("creating the ClusterExtension") | |
| ceName, ceCleanup := helpers.CreateClusterExtension(opName, "", nsName, unique, helpers.WithCatalogNameSelector(ccName)) | |
| DeferCleanup(ceCleanup) | |
| helpers.ExpectClusterExtensionToBeInstalled(ctx, ceName) | |
| By("verifying InstalledOLMOperatorUpgradable remains true after installing the operator") | |
| waitForOlmUpgradeStatus(ctx, operatorv1.ConditionTrue, "") | |
| It("should not block cluster upgrades if the operator's maxOCPVersion exceeds the current cluster version", func(ctx SpecContext) { | |
| By("waiting for InstalledOLMOperatorsUpgradeable to be true") | |
| waitForOlmUpgradeStatus(ctx, operatorv1.ConditionTrue, "") | |
| By("creating the ClusterExtension") | |
| ceName, ceCleanup := helpers.CreateClusterExtension(opName, "", nsName, unique, helpers.WithCatalogNameSelector(ccName)) | |
| DeferCleanup(ceCleanup) | |
| helpers.ExpectClusterExtensionToBeInstalled(ctx, ceName) | |
| By("verifying InstalledOLMOperatorsUpgradeable remains true after installing the operator") | |
| waitForOlmUpgradeStatus(ctx, operatorv1.ConditionTrue, "") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openshift/tests-extension/test/olmv1-incompatible.go` around lines 108 - 118,
Update the two By progress messages in the test around waitForOlmUpgradeStatus
to use the asserted condition name, InstalledOLMOperatorsUpgradeable, matching
the condition checked by waitForOlmUpgradeStatus.
…cluster version Add a second ReleaseGate-eligible OTE test verifying that an operator whose olm.maxOpenShiftVersion exceeds the current cluster version does not block cluster upgrade (InstalledOLMOperatorsUpgradeable stays True). The existing test only covered the blocking path (maxOCPVersion == current version → False). This covers the complementary allow path (maxOCPVersion == next minor → True), directly exercising the normalization logic introduced for the 4.23/5.0 co-release boundary. A nextMinorVersion() helper mirrors the 4.23→5.1 special case so the bundle annotation is always set to the correct next upgrade target. Run 'make build-update' to register the new allow-case test in the extension metadata after adding it to olmv1-incompatible.go. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>
tmshort
force-pushed
the
oprun-4432-ote-allow-case
branch
from
c148d87 to
477aa25
Compare
|
@tmshort: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
2 participants
Add a second ReleaseGate-eligible OTE test verifying that an operator whose olm.maxOpenShiftVersion exceeds the current cluster version does not block cluster upgrade (InstalledOLMOperatorsUpgradeable stays True).
The existing test only covered the blocking path (maxOCPVersion == current version → False). This covers the complementary allow path (maxOCPVersion == next minor → True), directly exercising the normalization logic introduced for the 4.23/5.0 co-release boundary.
A nextMinorVersion() helper mirrors the 4.23→5.1 special case so the bundle annotation is always set to the correct next upgrade target.
Summary by CodeRabbit