-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Image Registry update for Z and Power #28642
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
Conversation
|
The preview will be available shortly at:
|
|
Had confirmed using this flag - |
|
LGTM |
|
@doris.xu Can you validate this on Z please? |
modules/olm-pruning-index-image.adoc
Outdated
| -f {index-image-pullspec} \// <1> | ||
| -p {package1},{package2},{package3} \// <2> | ||
| -t <target_registry>:<port>/<namespace>/{index-image} <3> | ||
| -i registry.redhat.io/openshift4/ose-operator-registry/{index-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.
should there be a <4> on this line and an explanation below? Also if this is only needed for P & Z, what's the default -i value for amd64?
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.
If that line isn't needed for other architectures I would suggest we remove the note and make it a callout.
<4> Use -i only with IBM Power Systems and IBM Z images.
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.
The default value of base image is quay.io/operator-framework/upstream-opm-builder which isn't multi-arch and thus prune command - opm index prune fails on Power ( should fail on Z too , yet to be validated though) . To resolve this had to replace it with - registry.redhat.io/openshift4/ose-operator-registry which is multi-arch passed using flag -i to prune command .... fyi @SNiemann15 @nickboldt
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.
So... leave it in with the default?
-i quay.io/operator-framework/upstream-opm-builder <4>
then below...
|
Deploy preview for osdocs ready! Built with commit ab707dd |
|
/lgtm |
|
@tdaleibm: changing LGTM is restricted to collaborators 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/test-infra repository. |
|
/lgtm |
|
@ghatwala: changing LGTM is restricted to collaborators 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/test-infra repository. |
modules/olm-pruning-index-image.adoc
Outdated
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.
So then:
<4> For IBM Power Systems and IBM Z images, a different image must be used: -i registry.redhat.io/openshift4/ose-operator-registry/{index-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.
Will it be mentioned somewhere else what {index-image} to actuallly use? I believe we can explicitly say to use -i registry.redhat.io/openshift4/ose-operator-registry:v4.6 for both P and Z.
modules/olm-pruning-index-image.adoc
Outdated
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.
| -i registry.redhat.io/openshift4/ose-operator-registry/{index-image} | |
| -i registry.redhat.io/openshift4/ose-operator-registry/{index-image} <4> |
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.
@ktania46 Remember also to put a trailing \ at the end of previous lines when you're adding a new final line to these sort of multi-line commands. For example:
-t <target_registry>:<port>/<namespace>/{index-image} \// <3>
-i registry.redhat.io/openshift4/ose-operator-registry/{index-image} <4>
(Note that \// is required when the line also includes a callout like <3>.)
I'm fixing this spot in another related PR, so just FYI for the future. Thank you for the PR! 🎉
modules/olm-pruning-index-image.adoc
Outdated
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.
| <4> Use `-i` only with IBM Power Systems and IBM Z images. | |
| <4> Use the `-i` option with only IBM Power Systems and IBM Z images. |
Is this option optional or required for IBM Z/Power? If it's optional, it's fine, but if a Z/Power needs to use this option, please clarify.
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 is required for IBM Z/Power.
|
hold for merge till after 4.7 GA at which point it should be Cherrypicked to 4.8 branch as well. |
|
/cherrypick enterprise-4.8 |
|
/cherrypick enterprise-4.7 |
|
/cherrypick enterprise-4.6 |
|
/cherrypick enterprise-4.5 |
|
@codyhoag: #28642 failed to apply on top of branch "enterprise-4.8": 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/test-infra repository. |
|
@codyhoag: new pull request created: #29846 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/test-infra repository. |
|
@codyhoag: new pull request created: #29847 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/test-infra repository. |
|
@codyhoag: #28642 failed to apply on top of branch "enterprise-4.5": 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/test-infra repository. |
|
@ktania46 the edited file in this PR did not exist for |
|
@codyhoag As you may know, this PR leads to a bug https://bugzilla.redhat.com/show_bug.cgi?id=1943150 reported by the customer. So, could you help ask the QE have a review before merging it in the future? Thanks! cc: @adellape |

BZ : https://bugzilla.redhat.com/show_bug.cgi?id=1894382
This update is for 4.5 , 4.6 and 4.7 release
@SNiemann15 @vikram-redhat