-
Notifications
You must be signed in to change notification settings - Fork 544
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.
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
Change opm image from quay.io/operator-framework/upstream-opm-builder
to quay.io/operator-framework/opm
#2890
Change opm image from quay.io/operator-framework/upstream-opm-builder
to quay.io/operator-framework/opm
#2890
Conversation
Hi @StopMotionCuber. Thanks for your PR. I'm waiting for a operator-framework member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
@@ -8,7 +8,7 @@ import ( | |||
) | |||
|
|||
const ( | |||
defaultBinarySourceImage = "quay.io/operator-framework/upstream-opm-builder" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should not be directly changed. Instead, it seems we need OLM to bump its operator-registry version in go.mod
and then re-vendor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably the re-vendoring would have done the same string replacement. Anyway, I force-pushed that change away, is the vendoring done automatically? I guess yes, as there is a "vendor" step in the workflows
de37839
to
f78e0c4
Compare
9e12411
to
a48719d
Compare
Wanted to check-in if there is something blocking this from being merged, as it has been 5 month now. This seems to be blocking ARM64 support, though it can be patched locally on the cluster by overriding the image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: perdasilva, StopMotionCuber 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 |
/lgtm |
…r` to `quay.io/operator-framework/opm` Signed-off-by: Ruben Simons <r.simons@fz-juelich.de>
Had to fix DCO, should be good now |
/lgtm |
Any idea when this might make it into a release? |
Description of the change: Bumping the opm image from
quay.io/operator-framework/upstream-opm-builder
toquay.io/operator-framework/opm
Motivation for the change: We are having issues with opm jobs, as we have a multi-arch cluster, where jobs are failing if ran on ARM64 nodes. The OLM seems to be using a deprecated opm image. See details in #2823 (comment)
Note: I don't completely know what I'm doing and whether the
opm
image is actually able to fully replace theupstream-opm-builder
image. But I guess before digging down deeply into that rabbit hole, maintainers here should have a good understanding of that and evaluate that quicker than me.Closes #2823
Architectural changes:
Testing remarks:
Reviewer Checklist
/doc
[FLAKE]
are truly flaky and have an issue