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

Copy CRDs into the crds directory of the NACK chart #694

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

Conversation

dfr-exnaton
Copy link

@dfr-exnaton dfr-exnaton commented Apr 3, 2023

Summary

Based on the discussions in #398, this PR copies the required CRDs for NACK into the special crds directory of NACK helm chart and updates the README as well as the tests. Drawback is the increased maintenance in keeping the CRDs in sync with the nack repo.

Motivation

In my opinion, providing a caveat that CRDs have to be manually upgraded as part of a chart upgrade is necessary also when no crds directory is used. Thus its more user friendly if CRDs are provided as part of the chart. Not having an extra step to manually install & maintain CRDs when using this chart is also super helpful for GitOps users. Flux for example specifically has a special configuration object that allows to reinstall CRDs as part of an chart upgrade.

@ChristianGmw
Copy link

Could this PR be merged please. Very useful when you deploy everything via GitOps

@1995parham
Copy link
Contributor

What do you think about moving the chart into the main project and then push releases here? Then we have everything on the sample place and make updating things easier.

@svenfoo
Copy link
Contributor

svenfoo commented Jan 12, 2024

Can this be merged, or alternatively solved in some other way? Having to maintain a copy of the CRDs is tedious and error-prone.

@dfr-exnaton
Copy link
Author

Would love to update the PR (update CRD if necessary and fix merge conflicts) after a maintainer gives a thumbs up on this rather small, non-breaking change.

@vvinnich
Copy link

vvinnich commented Jun 7, 2024

@dfr-exnaton what do you think to add

annotations:
  helm.sh/resource-policy: keep

@dfr-exnaton
Copy link
Author

@dfr-exnaton what do you think to add

annotations:
  helm.sh/resource-policy: keep

This PR just copied over the crds to include them in the chart, nothing was modified. Do you have an example where this annotation would help? Because I could also see issues when keeping resources around...

@dfr-exnaton
Copy link
Author

@caleblloyd would be great if this minor PR can either get a thumbs up or thumbs down. If positive, I would obviously update the README and the crds to be in sync (after more than one year...)

@vavsab
Copy link

vavsab commented Nov 13, 2024

A bit frustrating that in 2024 we still need to copy-paste CRDs while many other charts include them by default.

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.

6 participants