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

[k8up] Attach CRDs to GitHub release #419

Open
tobikris opened this issue Apr 29, 2022 · 7 comments
Open

[k8up] Attach CRDs to GitHub release #419

tobikris opened this issue Apr 29, 2022 · 7 comments
Labels
enhancement New feature or request

Comments

@tobikris
Copy link

tobikris commented Apr 29, 2022

Summary

As a k8up user
I want to install the chart and the corresponding CRDs automatically
So that I don't have to manually find the right CRD version for the app.

Context

Currently the chart releases for k8up do not contain the necessary CRDs. They are instead published alongside the binary releases in https://github.com/k8up-io/k8up. To install the correct CRD file one needs to look into the Chart.yaml, extract the appVersion, find the relevant release in the binary repo and fetch the CRDs from this release.
Because of the different versioning schemes it is also not possible (to my knowledge and according to several tries) to use renovate to automate those steps.

I created a workaround that automates those steps and republishes the CRDs as a helm chart with the appVersion as version.
This works for me, but I think the better solution would be to publish the CRDs directly with the chart in this repo here.

Thank you very much.

Further links

Acceptance criteria

  • When a helm chart is released, then the necessary CRDs are bundled with it together in the same release.
@tobikris tobikris added the enhancement New feature or request label Apr 29, 2022
@ccremer
Copy link
Contributor

ccremer commented May 2, 2022

Hi.
Thanks for your interest in K8up and the detailed description of this issue. I understand your inconvenience.

I don't remember which issue or conversation, but it was a conscious decision to not include the CRDs in the chart.
There are (at least) 2 reasons why:

  1. Helm supports putting CRDs in the special crds directory. So when you install the chart for the first time, Helm installs those CRDs as well. However, Helm doesn't touch the CRDs anymore, even new chart versions have newer CRDs. This means that they won't ever get upgraded. This is explained in more detail here: https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#install-a-crd-declaration-before-using-the-resource . That means that new K8up features that add new API fields to the CRDs are not going to work resp. you have to upgrade the CRDs with kubectl apply in any case.
  2. Including the CRDs in the templates makes cluster-scoped resources part of the Helm release, which comes with its own problems. For example uninstalling the Release removes the CRDs as well and with that, all K8up resources (when deleting CRDs, Kubernetes removes all resources belonging to that CRD). Or when upgrading, it's possible that the CRDs get replaced by removal/readding instead of upgraded, which again deletes all K8up resources. This is not something that the normal Helm user anticipates dealing with, since it's rare to see CRDs included in the templates (outside of the crds directory). Even if CRD installation has to be explicitly enabled with a parameter like crd.install=true, 3 months later you forget about this and accidentally the CRDs while re-installing a Release.

Due to the high risk of accidental removal of CRDs we opted to go for the CRD management outside of the chart. This puts the user in charge of CRDs and not Helm or its "hidden" features.

@tobikris
Copy link
Author

tobikris commented May 2, 2022

Hi, thanks for your response. I understand those points but my idea is simpler than that. Sorry for not being specific in the first post. I would love to have the CRDs published in the same "GitHub release". It is perfectly fine to not include them in the chart archive. This would allow to get the correct CRD version easily without my workaround.

Thanks for your help.

@ccremer
Copy link
Contributor

ccremer commented May 2, 2022

ah, sorry for misinterpreting it. "Bundle CRDs with chart" in the title is entirely ambiguous :)

But to understand you correctly, all you're asking is that the GitHub release (e.g. https://github.com/appuio/charts/releases/tag/k8up-2.0.3) has the K8up's crd.yaml redistributed as an additional attachment in the assets?

@ccremer ccremer changed the title [k8up] Bundle CRDs with chart [k8up] Attach CRDs to GitHub release May 2, 2022
@tobikris
Copy link
Author

tobikris commented May 2, 2022

Yes, exactly. This allows for much easier retrieval of the corresponding version. Sorry for the confusion.

@ccremer
Copy link
Contributor

ccremer commented May 2, 2022

I've discussed this with the engineering team internally.
We came to the conclusion that this is a great idea, however, short-term we currently can't spend the engineering power on this, as we ourselves don't have a strict need for this (we use https://github.com/projectsyn/component-backup-k8up/ to deploy K8up on our clusters) and we're busy with other projects (also the reason why there's not much development activity on K8up itself).

In the shortterm we would be accepting a PR that at least implements and automates your proposal if you're willing to help.

In the longterm, we'll investigate the possibility that this K8up chart may go the the K8up repository itself and become a single-chart repo. That way, we have the chart + code in one place. Again, this is neither a current priority nor a promise.

If you would like to contribute towards this short-term automation, please let me know so I can point you into the right direction.

@bo0tzz
Copy link

bo0tzz commented Jun 9, 2022

If the CRDs are not included in the Helm chart, that should at least be well documented. Currently https://k8up.io/k8up/2.3/how-tos/installation.html#_helm has no suggestion that you need an extra step to manually install the CRDs, leaving you with just a crashing operator.

@ccremer
Copy link
Contributor

ccremer commented Jun 20, 2022

@bo0tzz thanks for pointing that out. I've opened k8up-io/k8up#692 to replace the instructions with a link to the chart for the time being.

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

No branches or pull requests

3 participants