OPRUN-4055: Migrate tasks from openshift/origin olmv1.go file which are remaining#418
Merged
openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom Aug 8, 2025
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
camilamacedo86
force-pushed
the
add-catalog-olmv1-tests
branch
3 times, most recently
from
8e3171e to
9a285e2
Compare
camilamacedo86
changed the title
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
camilamacedo86
changed the title
camilamacedo86
force-pushed
the
add-catalog-olmv1-tests
branch
from
9a285e2 to
209cf93
Compare
camilamacedo86
changed the title
|
@camilamacedo86: This pull request references OPRUN-4055 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 "4.20.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. |
openshift-ci-robot
added
the
jira/valid-reference
camilamacedo86
force-pushed
the
add-catalog-olmv1-tests
branch
2 times, most recently
from
503feae to
afb35c0
Compare
Contributor
Author
|
c/c @kuiwang02 |
anik120
reviewed
Aug 6, 2025
Contributor
anik120
left a comment
anik120
left a comment
There was a problem hiding this comment.
Code looks good to me mostly. Two questions:
- We don't need to check in the vendor folder right? I understand the vendor folder was introduced in a previous PR and if we want to make any change to that structure we should do it in a separate PR, I just wanted to confirm the we don't need the folder to be checked into git for any reason. Openshift CI env has access to the internet and therefore the ability to pull the dependencies based on
go.sumandgo.mod - Is the
ci/prow/test-extensionsrunning the exact tests being introduced in this PR? Or is there another new CI target that's running these extension tests?
| @@ -0,0 +1,88 @@ | |||
| package mocks | |||
Contributor
There was a problem hiding this comment.
why is this package called mocks?
Contributor
Author
There was a problem hiding this comment.
Because we have helpers to create the mocks/scenarios
Do you have a better name suggestion?
Contributor
Author
There was a problem hiding this comment.
I changed to helpers. I hope that addresses your concern. 🚀
Contributor
Author
|
HI @anik120 Thank you for looking on this one. Following the answers inline regards: #418 (review)
Yes, and we do. If we do not add the vendor properly, it fails in the ci job
We can look here what it does: This one is more of a sanity check Now, the tests are executed under: How it works can be found in the README: https://github.com/openshift/operator-framework-operator-controller/blob/main/openshift/tests-extension/README.md#e2e-test-configuration |
camilamacedo86
commented
Aug 6, 2025
| "environmentSelector": {} | ||
| }, | ||
| { | ||
| "name": "[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1 operator installation should install a cluster extension", |
Contributor
Author
There was a problem hiding this comment.
camilamacedo86
commented
Aug 6, 2025
| "environmentSelector": {} | ||
| }, | ||
| { | ||
| "name": "[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1 operator installation should fail to install a non-existing cluster extension", |
Contributor
Author
There was a problem hiding this comment.
camilamacedo86
commented
Aug 6, 2025
| "environmentSelector": {} | ||
| }, | ||
| { | ||
| "name": "[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1 operator installation should block cluster upgrades if an incompatible operator is installed", |
Contributor
Author
There was a problem hiding this comment.
|
@camilamacedo86: This pull request references OPRUN-4055 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 "4.20.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. |
camilamacedo86
force-pushed
the
add-catalog-olmv1-tests
branch
from
afb35c0 to
edd0cbe
Compare
Contributor
|
@camilamacedo86: The label(s) 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 kubernetes-sigs/prow repository. |
|
@camilamacedo86: This pull request references OPRUN-4055 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 "4.20.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. |
camilamacedo86
force-pushed
the
add-catalog-olmv1-tests
branch
from
edd0cbe to
eb59b9f
Compare
|
@camilamacedo86: This pull request references OPRUN-4055 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 "4.20.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. |
Contributor
Author
|
/test openshift-e2e-aws |
kuiwang02
reviewed
Aug 7, 2025
| specs.AddBeforeAll(func() { | ||
| env.Init() | ||
| // Ensure that OLMv1 is enabled | ||
| commons.CheckFeatureCapability() |
Contributor
There was a problem hiding this comment.
@camilamacedo86
each case needs to check feature capability to determine if it skip.
if you put it here, only one case check it. and other cases do not check it.
so, it needs to be in each Describe with BeforeEach. for example
var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM] OLMv1 CRDs", func() {
const olmv1GroupName = "olm.operatorframework.io"
BeforeEach(func() {
commons.CheckFeatureCapability()
})
It("should be installed", func(ctx SpecContext) {and
var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1 operator installation", func() {
BeforeEach(func() {
commons.CheckFeatureCapability()
})
It("should install a cluster extension", func(ctx SpecContext) {
Contributor
Author
There was a problem hiding this comment.
I thought all would check that.
It is fine we can keep in all of them
kuiwang02
reviewed
Aug 7, 2025
| extlogs.Warn("Skipping feature capability check: not OpenShift") | ||
| return | ||
| } | ||
| var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM][Skipped:Disconnected] OLMv1 operator installation", func() { |
| @@ -30,8 +30,8 @@ var _ = Describe("[sig-olmv1] OLMv1", func() { | |||
| }) | |||
|
|
|||
| var _ = Describe("[sig-olmv1][OCPFeatureGate:NewOLM] OLMv1 CRDs", func() { | |||
camilamacedo86
force-pushed
the
add-catalog-olmv1-tests
branch
2 times, most recently
from
b0f23a7 to
b7ad3df
Compare
… which are remaining This commit moves the final OLMv1 tests from openshift/origin/test/extended/olm/olmv1.go to their proper location in this repository. This migration is part of a larger effort to streamline development by co-locating tests with the component they validate. This will reduce CI overhead and allow for faster, more atomic changes. Assisted-by: Gemini
camilamacedo86
force-pushed
the
add-catalog-olmv1-tests
branch
from
b7ad3df to
a185061
Compare
Contributor
Author
|
Hi @kuiwang02 I appreciate your help. |
|
@camilamacedo86: This pull request references OPRUN-4055 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 "4.20.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. |
Contributor
|
@camilamacedo86: 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. |
Contributor
|
/lgtm |
Contributor
|
/approve |
1 similar comment
Contributor
|
/approve |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: anik120, camilamacedo86, kuiwang02, 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
openshift-merge-bot
Bot
merged commit af40962
into
openshift:main
12 of 13 checks passed
|
[ART PR BUILD NOTIFIER] Distgit: ose-olm-catalogd |
|
[ART PR BUILD NOTIFIER] Distgit: ose-olm-operator-controller |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit moves the final OLMv1 tests from openshift/origin/test/extended/olm/olmv1.go to their proper location in this repository. This migration is part of a larger effort to streamline development by co-locating tests with the component they validate. This will reduce CI overhead and allow for faster, more atomic changes. See: https://github.com/openshift/origin/blob/05eeb94cf14384be99323b2cbd00ab61b1b1c47c/test/extended/olm/olmv1.go#L34-L285
Here is the PR to delete the tests when this one get merged: openshift/origin#30072
Tests done locally
Assisted-by: Gemini