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

Use suggested namespaces of packages #266

Conversation

JustinKuli
Copy link
Member

@JustinKuli JustinKuli commented Jun 10, 2024

(Originally this PR had 2 commits - 1 has been removed to be handled later if necessary)

Previously, if the namespace was not set for a Subscription in an
OperatorPolicy, the default namespace on the controller (if set) would
be used in all cases. But many packages specify a suggested namespace in
their PackageManifests, which would be better to use, when provided.

Additionally, if the OperatorPolicy was already managing a subscription,
the namespace of that subscription will be used (if not set in the
policy). This may prevent some issues that could occur if the suggested
namespace for an operator changes at some point.

Refs:

@JustinKuli
Copy link
Member Author

/hold

Another approach I'm thinking about now (an alternative to the admittedly odd annotation thing) is to only prevent the name/space change when the policy is musthave & enforce. Then to "force" the change, the user could change the policy to inform briefly, then back to enforce. This would also give them a chance to make the policy mustnothave if they did want to change the namespace of the operator, and clean up the old installation.

@mprahl what do you think? Or it sounded like you were thinking preventing this kind of change might not be necessary? The situation I'm trying to prevent is where the operator's suggested namespace changes at some point, and OperatorPolicy creates a duplicate operator installation in the new namespace without any indication of the potential issue.

Previously, if the namespace was not set for a Subscription in an
OperatorPolicy, the default namespace on the controller (if set) would
be used in all cases. But many packages specify a suggested namespace in
their PackageManifests, which would be better to use, when provided.

Additionally, if the OperatorPolicy was already managing a subscription,
the namespace of that subscription will be used (if not set in the
policy). This may prevent some issues that could occur if the suggested
namespace for an operator changes at some point.

Refs:
 - https://issues.redhat.com/browse/ACM-12057

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
@JustinKuli JustinKuli force-pushed the 12057-packagemanifest-suggested-ns branch from f26f665 to e6d912d Compare June 11, 2024 20:47
@JustinKuli
Copy link
Member Author

/unhold

I've removed the "Prevent unintended changes to operator name/space" commit, and made some changes to based on the initial review comments, which may avoid that whole potential issue.

Copy link

openshift-ci bot commented Jun 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JustinKuli, mprahl

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-merge-bot openshift-merge-bot bot merged commit 125c471 into open-cluster-management-io:main Jun 12, 2024
9 checks passed
@JustinKuli JustinKuli deleted the 12057-packagemanifest-suggested-ns branch July 25, 2024 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants