-
Notifications
You must be signed in to change notification settings - Fork 118
chore(manifests): add openshift capability annotation #462
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
chore(manifests): add openshift capability annotation #462
Conversation
Register CVO-managed manifests under the optional `marketplace` capability.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: njhale 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 |
|
cc @bparees |
|
where is the OperatorHub CRD itself defined? i'd expect it to be excluded when this cap is not enabled, also. |
|
@njhale do we want to add some documentation to the readme as well to say where this annotation and value come from? |
It looks like is defined in o/api: https://github.com/openshift/api/blob/master/config/v1/0000_03_config-operator_01_operatorhub.crd.yaml |
|
ok so openshift/api#1150 should be marking it as a capability unless there is another reason to have that api defined when the marketplace operator is disabled (it'll be interesting to see what happens to the console when marketplace is disabled) |
@bparees just pushed that change actually: https://github.com/openshift/api/pull/1150/files#diff-c53669fc670c26b116439080f132967cca87dec5864c511ed7772de465e0bf7fR9 |
ah, nice. |
I'm not too sure. The annotation is namespaced to openshift, so it should be pretty simple to look up if someone tweaking these manifests is unsure of their origin; not to mention, they can look at the git blame and find this PR with its description as well. |
|
@timflannagan are these prow-job failures due to the same resource starvation on AWS we're seeing in OLM? |
|
/retest |
|
It seems like our AWS dev account should be slightly more stable now. /retest |
|
The o/api PR just landed - we should just be waiting on this marketplace PR to land now, but we could use clusterbot to build a custom release image that we pass to the openshift installer to verify these changes work out-of-band to this PR merging? |
|
openshift/release#27304 moves these prowjobs to GCP. |
|
/test all |
|
/retest-required |
|
/retest |
|
@njhale: The following tests 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/test-infra repository. I understand the commands that are listed here. |
|
/test okd-e2e-gcp |
|
/lgtm |
Description of the change:
Register CVO-managed manifests under the optional
marketplacecapability.See also:
Motivation for the change:
We want OpenShift users to be able to disable/enable the marketplace operator without putting their cluster into an unmanaged state.
Jira: OLM-2474