Skip to content
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

Add labels to indicate availability for other arches #768

Merged
merged 2 commits into from
Feb 15, 2022

Conversation

manojnkumar
Copy link
Contributor

We have done the validation for the ocp4-cis and ocp4-cis-node profiles on ppc64le and are ready to for this operator to be supported on this architecture.

@mrogers950
Copy link
Contributor

Thanks @manojnkumar !
It looks good, but I'm going to put this on hold for the moment because the CSV file will sync to the downstream repos. My concern is that we'll advertise other arches before the downstream arch bundles are ready. I'll unhold once I can confirm that.
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 10, 2022
@mrogers950 mrogers950 self-assigned this Jan 10, 2022
@manojnkumar
Copy link
Contributor Author

/retest

@@ -167,6 +167,11 @@ metadata:
support: Red Hat Inc.
name: compliance-operator.v0.1.47
namespace: placeholder
labels:
operatorframework.io/arch.amd64: supported
operatorframework.io/arch.arm64: supported

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd love to have compliance operator for ARM too, but currently it's not being built (downstream) so it MUST NOT be included here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good catch, this should be corrected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, Dropped arm64.

@manojnkumar
Copy link
Contributor Author

/retest

@yselkowitz
Copy link

The change looks good for downstream, but will this also affect the upstream (community) version as well? The community build is still amd64-only so it shouldn't have these flags. Can these be separated somehow?

@mrogers950
Copy link
Contributor

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 11, 2022
@mrogers950
Copy link
Contributor

The change looks good for downstream, but will this also affect the upstream (community) version as well? The community build is still amd64-only so it shouldn't have these flags. Can these be separated somehow?

It could be separated. We'd have to code the labels to the CSV when the downstream bundle repo's render_templates script runs. The downside is that it's another special piece of the midstream pipeline that we have to maintain.

I'm not sure how common it is to use OLM to install the upstream version, as opposed to using deploy/ directly. It's provided as a courtesy for upstream, but In general, we treat the CSV as a downstream component (some of the elements like the variable names "RELATED_IMAGE_" are downstream specific).

Overall I'm inclined to have the labels here upstream so they are sourced directly downstream, and to avoid confusion, Add a comment / document that only amd64 builds are provided upstream, even though the labels are there.

@yselkowitz
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 14, 2022
@mrogers950
Copy link
Contributor

/approve

@openshift-ci
Copy link

openshift-ci bot commented Feb 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: manojnkumar, mrogers950

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 14, 2022
@manojnkumar
Copy link
Contributor Author

/retest

@openshift-ci
Copy link

openshift-ci bot commented Feb 15, 2022

@manojnkumar: all tests passed!

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.

@openshift-merge-robot openshift-merge-robot merged commit 30ba2f7 into openshift:master Feb 15, 2022
@gquillar
Copy link
Contributor

PR-795 to be able to build the ppc64le/s390x images has been merged. I think that this PR can be merged now.

@yselkowitz
Copy link

/cherry-pick release-v0.1.48

@openshift-cherrypick-robot

@yselkowitz: new pull request created: #806

In response to this:

/cherry-pick release-v0.1.48

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants