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 CRD adoption hook #139

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add CRD adoption hook #139

wants to merge 2 commits into from

Conversation

fhielpos
Copy link
Member

In order to install the PolicyException CRD from the policy-meta-operator app, we need to add some labels and annotations to it. Since KPO owned it, this would be the best place to do it.

@fhielpos fhielpos requested a review from a team as a code owner August 22, 2024 17:23
@fhielpos fhielpos self-assigned this Aug 22, 2024
@stone-z
Copy link
Contributor

stone-z commented Aug 22, 2024

I'm fine with this but I wonder how sure the upgrade path is. How likely is it that a cluster has the CRD already, but won't update KPO again prior to trying to install PolMO?
Would this logic work as a pre-install hook on PolMO?

@fhielpos
Copy link
Member Author

How likely is it that a cluster has the CRD already, but won't update KPO again prior to trying to install PolMO?

All clusters will need to update KPO to make it work with policy-meta-operator. The CRD would exist in every cluster that already exists. We can make POLMO depend on KPO for now as a solution to avoid POLMO inconsistencies but I want to see how bad it breaks.

Would this logic work as a pre-install hook on PolMO?

I tried, but no, Helm will template the chart and identify that the CRD is already present before even running the hooks.

Copy link
Contributor

@stone-z stone-z left a comment

Choose a reason for hiding this comment

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

Fine with me. This means there will be a required bundle version or range before going to PolMO. I know of at least one customer who has KPO disabled currently, so we'll have to do some communication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants