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

OCPBUGS-14907: Fix operator backed catalog page when copied CSVs disabled #12932

Merged
merged 1 commit into from Jul 15, 2023

Conversation

vikram-raj
Copy link
Member

Fixes: https://issues.redhat.com/browse/OCPBUGS-14907

Descriptions:
Show catalog items in operator backed even if copied to CSVs is disabled.

@openshift-ci openshift-ci bot added component/olm Related to OLM approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 23, 2023
@spadgett
Copy link
Member

/cc @TheRealJon

Copy link
Member

@TheRealJon TheRealJon left a comment

Choose a reason for hiding this comment

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

There are two main cases we need to cover when listing CSVs and copied CSVs are disabaled:

  1. Listing in a namespaced context: We are currently in a namespace other than 'openshift', 'openshift-operators', or 'All Namespaces'.
  2. We are listing in a global context: we are in one of the three excluded namespaces from (1).

Namespaced context

List CSVs from the current namespace and copied CSVs from the 'openshift' namespace

Global Context

a. All Namespaces: List CSVs from all namespaces, but filter out copied CSVs. (This is probably already covered since this is not a new case).
b. 'openshift-operators' namespace: List all operators in this namespace only and they are all available in all namespaces (they have copies in the 'openshift' namespace).
c. 'openshift' namespace: List all operators in this namespace only. Copied CSVs are available in all namespaces, otherwise, they are only available in the current namespace.

@tlwu2013 Could you help confirm this logic? Mainly with (b). Are all CSVs in the 'openshift-operators' namespace considered global or can namespaced operators be installed their as well?

@vikram-raj vikram-raj changed the title Fix operator backed catalog page when copied CSVs disabled OCPBUGS-14907: Fix operator backed catalog page when copied CSVs disabled Jun 23, 2023
@openshift-ci-robot openshift-ci-robot added jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jun 23, 2023
@openshift-ci-robot
Copy link
Contributor

@vikram-raj: This pull request references Jira Issue OCPBUGS-14907, which is invalid:

  • expected the bug to target the "4.14.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Fixes: https://issues.redhat.com/browse/OCPBUGS-14907

Descriptions:
Show catalog items in operator backed even if copied to CSVs is disabled.

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.

@vikram-raj
Copy link
Member Author

/jira refresh

@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jun 23, 2023
@openshift-ci-robot
Copy link
Contributor

@vikram-raj: This pull request references Jira Issue OCPBUGS-14907, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.14.0) matches configured target version for branch (4.14.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @sanketpathak

In response to this:

/jira refresh

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.

@openshift-ci openshift-ci bot requested a review from sanketpathak June 23, 2023 21:52
@tlwu2013
Copy link
Contributor

tlwu2013 commented Jun 26, 2023

#12932 (review)

@tlwu2013 Could you help confirm this logic? Mainly with (b). Are all CSVs in the 'openshift-operators' namespace considered global or can namespaced operators be installed their as well?

hey @TheRealJon, thanks for reaching out.

Are all CSVs in the ‘openshift-operators’ namespace considered global

yes, all Operators in openshift-operators namespace are global.
(but global Operators can exist somewhere else, like Serverless does)

or can namespaced operators be installed their as well?

no, they cannot.

--
if users tried to install an Operator as namespaced scope in openshift-operators namespace, they got blocked by this:

Namespace does not support installation mode
The openshift-operators Namespace is reserved for global Operators that watch all Namespaces. To install an Operator in a single Namespace, select a different Namespace where the operand should run.

@TheRealJon
Copy link
Member

TheRealJon commented Jun 26, 2023

Thanks @tlwu2013!

@vikram-raj As Tony mentioned, we also need to handle the case where an operator is installed globally, but not in the default openshift-operators namespace. He used Serverless as an example. By default, that operator is installed globally in the openshift-serverless namespace. This means the source CSV will be in that namespace, while a copy will be made in the 'openshift' namespace. Therefore, when listing CSVs in the 'openshift-serverless' namespace, the source CSV should be listed and not the copied CSV from 'openshift'. Any operator can define its own suggested global namespace, so however we handle it needs to take this feature into account. We do handle this from the admin perspective, so we just need to make the dev perspective reflect the same results. A hook is a good way to do that, but it has to reflect the same results as the Installed Operators page.

@vikram-raj
Copy link
Member Author

@TheRealJon I updated the can you PTAL again. Thanks.

Copy link
Member

@TheRealJon TheRealJon left a comment

Choose a reason for hiding this comment

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

/lgtm

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

openshift-ci bot commented Jul 14, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: TheRealJon, vikram-raj

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

@vikram-raj
Copy link
Member Author

/retest

@vikram-raj
Copy link
Member Author

/cherry-pick release-4.13

@openshift-cherrypick-robot

@vikram-raj: once the present PR merges, I will cherry-pick it on top of release-4.13 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.13

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.

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 97aeb04 and 2 for PR HEAD b964058 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 0a9a296 and 1 for PR HEAD b964058 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 15, 2023

@vikram-raj: 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 89b63ea into openshift:master Jul 15, 2023
6 checks passed
@openshift-ci-robot
Copy link
Contributor

@vikram-raj: Jira Issue OCPBUGS-14907: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-14907 has been moved to the MODIFIED state.

In response to this:

Fixes: https://issues.redhat.com/browse/OCPBUGS-14907

Descriptions:
Show catalog items in operator backed even if copied to CSVs is disabled.

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.

@openshift-cherrypick-robot

@vikram-raj: new pull request created: #13019

In response to this:

/cherry-pick release-4.13

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. component/olm Related to OLM jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants