fix(olm): remove dependency on marketplace-provided content #26993
Conversation
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| if !ok { | ||
| g.Skip("redhat-operators source not found in enabled sources") |
There was a problem hiding this comment.
should we be concerned that this check was not working? Does that mean we're not actually disabling some aspect of the redhat-operators source when marketplace is not enabled?
There was a problem hiding this comment.
It was testing the OperatorHub API, which still exists as part of a separate problem that I'm currently tracking -- I'll post a PR here when there are more updates.
| // configure OperatorGroup before tests | ||
| configFile, err := oc.AsAdmin().Run("process").Args("--ignore-unknown-parameters=true", "-f", operatorGroup, "-p", "NAME=test-operator", fmt.Sprintf("NAMESPACE=%s", oc.Namespace())).OutputToFile("config.json") | ||
| // Configure CatalogSource before tests | ||
| const image = "registry.redhat.io/redhat/redhat-operator-index:v4.10" |
There was a problem hiding this comment.
this is going to be a source of bit-rot someday.
can we identify the clusterversion and pick the version tag based on that, dynamically?
There was a problem hiding this comment.
or pull the value in from elsewhere? (i know we also have the challenge that the day we start v4.12, there may not be a v4.12 catalog published, so that could be a problem too... maybe n-1 is the tag to use)
There was a problem hiding this comment.
I've asked the same questions before and I think the long-term solution is for the catalog fixture to be built as a part of the test setup.
or pull the value in from elsewhere? (i know we also have the challenge that the day we start v4.12, there may not be a v4.12 catalog published, so that could be a problem too... maybe n-1 is the tag to use)
I like this, but it seems like we'd just be shifting the problem over one release; i.e. when we're testing 4.12, if we grabbed the n-1 tag, we'd be assuming the same image name and convention were carried over from 4.10 to 4.11 (since the test in 4.11 points at the image in 4.10) -- moreover, we'd still inherit the primary issue of today's tests: there's no guarantee that the content we're using as a fixture hasn't changed.
There was a problem hiding this comment.
yeah i guess maybe pinning to 4.10 and if someday that breaks on a v4.16 cluster, we can bump it at that point, is the most straightforward answer for now. not ideal, but not worth holding this up over.
njhale
force-pushed
the
fix/olm-test
branch
2 times, most recently
from
3358ac1
to
69821cf
Compare
|
/hold this change is fine, but we need to sort out why the operatorhub api is getting installed when it should not be, and right now the existing state of this test(failing) is a handy way to sort that out. |
openshift-ci
bot
added
the
do-not-merge/hold
|
/uncc |
openshift-ci
bot
added
the
needs-rebase
openshift/cluster-config-operator#245 just landed, which should be the last step in dancing the CRD over into the marketplace operator's repo. /hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
openshift-ci
bot
removed
the
needs-rebase
|
/retest |
|
/approve |
openshift-ci
bot
added
lgtm
Remove a test dependency on the marketplace-provided redhat-operators catalog by introducing a new catalog test fixture, pinning to content released during 4.10. This makes less assumptions about future content allowing for a more stable test. This also allows us to run the test when when the marketplace capability is disabled. Signed-off-by: Nick Hale <njohnhale@gmail.com>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, njhale 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 |
|
/hold Revision bb07462 was retested 9 times: holding |
openshift-ci
bot
added
the
do-not-merge/hold
|
/test e2e-aws-single-node |
|
/test e2e-aws-single-node-upgrade |
|
@njhale: 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/test-infra repository. I understand the commands that are listed here. |
|
closing in favor of #27302 /close |
|
@anik120: 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/test-infra repository. |
Remove a test dependency on the marketplace-provided redhat-operators
catalog by introducing a new catalog test fixture, pinning to content released
during 4.10. This makes less assumptions about future content allowing for a
more stable test. This also allows us to run the test when when the marketplace
capability is disabled.
Signed-off-by: Nick Hale njohnhale@gmail.com
See https://issues.redhat.com/browse/OLM-2542 for more details.