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

DRAFT: Add the possibility of split crds and templates inside the helm chart #1818

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

gfa
Copy link

@gfa gfa commented Apr 10, 2024

this helps with upgrades on multi tenant clusters since it separates the lifecycle of CRDs and templates

SUMMARY

This change adds a new Makefile target which splits templates and crds which helps with upgrades on multi tenant k8s clusters (many awx instances on different namespaces)

ISSUE TYPE
  • New or Enhanced Feature
ADDITIONAL INFORMATION

Described here in helm documentation

the naming is consistent but ugly, i welcome suggestions of better naming

this helps with upgrades on multi tenant clusters since it separates the
lifecycle of CRDs and templates
@gfa gfa changed the title Add the possibility of split crds and templates inside the helm chart DRAFT: Add the possibility of split crds and templates inside the helm chart Apr 12, 2024
@rooftopcellist
Copy link
Member

rooftopcellist commented May 1, 2024

Other people in the community who use helm, what do you think about this approach?

@gfa can you point out the problem you are solving by splitting the lifecycle of the CRD's and the templates? Is it to avoid conflicts between two different CRD versions (since they are a global resource in k8s)?

We have made it a point to only make additive and backwards compatible changes to the CRD to date.

Sidenote, this might be a good topic to bring up in forum.ansible.com/tag/awx-operator!

@gfa
Copy link
Author

gfa commented May 11, 2024

Hi @rooftopcellist

at $DAYJOB our k8s clusters are multi tenant and the clusters running awx are no exception; to help with upgrades and avoiding one tenant to surprise others we make the install of CRDs optional (defaulting to no install them)

We have made it a point to only make additive and backwards compatible changes to the CRD to date.

i wasn't aware of this but it surely is good news :)

I can spend time finishing this PR in a few weeks if it would be useful

@kurokobo
Copy link
Contributor

kurokobo commented May 11, 2024

Hmm, as @rooftopcellist mentioned, CRDs are global resources, and current CRDs has only one version (like v1beta1 for AWX CRD).
So, indeed making installation of CRDs optional can avoid one tenant to suprise orhers, but this also means newer AWX Operator may be failed to deploy AWX CR, if newer AWX Operator requires newly implemented fields in CRD.

For example:

  • Your cluster has CRDs on 2.12,x
  • Some AWX Operators 2.12.x are working on your cluster
  • One tenant want to use newer Operator 2.13.x or later. So they install new Operator without updaing CRDs (I believe this is your design)
  • However Operator 2.13.x or later fails to deploy AWX CR, since AWX CR does not have web_liveness_period field due to outdated CRDs (since web_liveness_period is newly added in CRDs on newer 2.13.x or later)
  • So the tenant MUST upgrades CRDs, but since CRDs are global resources, upgrading CRDs MUST suprise other tenants, as you mentiond.
  • Hmm 🤔

How do you handle this situation by this PR?

One countermeasure would be to increment the version of CRDs (like v1beta1, v1beta2, ...) each time when any changes are made to CRDs, but this does not seem to be the policy, at least in this current repository (and I'm on the side of the current policy).

@rooftopcellist
Copy link
Member

Isn't it the case that when you upgrade with helm it does not affect your CRD's and that it is a separate manual step for upgrade/apply the CRD's?

Between operator versions, we make sure to not make breaking schema changes by only making additive changes (no removing fields, no new required fields, etc.). In the future, we will create a new apiVersion for the AWX resource when we make some breaking changes, but that won't happen for quite some time (6-12 mo if I had to guess).

@gfa
Copy link
Author

gfa commented Jun 8, 2024

apologies for my late response

@kurokobo my PR does not handle the case you mention, which is fine since I want the tenant which wants the newer version of the operator to communicate with other tenants/cluster admin before changing the CRDs

In my case this is implemented via argocd, we don't allow anybody but cluster admins to manage CRDs (cluster-admin vs cluster-user project in argocd parlance)

@rooftopcellist I dont use helm directly, as mentioned before i use argocd which (AFAIK) does not have the capability you mention

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.

3 participants