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

Create isolated chart for operator CRDs #1203

Closed
wants to merge 1 commit into from

Conversation

jaronoff97
Copy link
Contributor

Creates an isolated chart for operator CRDs per #677

@andrewdinunzio
Copy link

andrewdinunzio commented May 29, 2024

Not to be a pain but just wanted to mention the issue with the crds directory that helm provides in that you cannot add new CRDs (or even new versions of CRDs) if you use it. Upgrades won't add them.

There are 3 ways I know of that you can handle CRDs, 2 of which have issues:

  1. Use the crds directory. This has the issue mentioned above.
  2. Put the CRDs in the templates. This has the issue with the stale discovery cache mentioned in the other issue.
  3. Put the CRDs in the templates of a separate helm chart. This works if you install the CRDs chart first, but runs into the issues from number 2 if you try to use the CRDs helm chart as a dependency of another chart.

Possible that I'm missing something, but I've been struggling with helm and CRDs for a while and haven't found any possible option for installing CRDs and CRs in one helm install/upgrade in a way that is reliable.

@jaronoff97
Copy link
Contributor Author

@andrewdinunzio this is a good point, I am opting for simplifying only the installation process and eventually centralizing instructions for upgrading CRDs as prometheus does here. I am recommending we do it this way because managing the CRDs via helm has a huge amount of thorns as we discussed in the issue. i.e. we simplify the installation process and give very specific upgrading instructions for CRDs. I have also been struggling with helm and CRDs for a while now and I don't think any of these solutions is great.

@andrewdinunzio
Copy link

Gotcha. Personally I'm not a fan of manual steps as part of upgrades as it makes it harder to keep things up to date and doesn't fit well with CI/CD pipelines, but so far I've had no issues with just pulling the CRDs out and kubectl applying them before helm installing the main chart (setting the value to disable CRDs in the helm install). It gets trickier now that the CRDs are templated, but I'll figure something out, probably just helm template it first then pull the CRDs out.

@jaronoff97
Copy link
Contributor Author

@andrewdinunzio definitely – that's exactly why i want to get them out of templates. This chart will only work when a user doesn't want the conversion webhook. I think I will leave it to power-users like yourself to decide how to handle automatic upgrades. Hopefully helm will eventually improve this lifecycle 🤞

@JaredTan95
Copy link
Member

Not to be a pain but just wanted to mention the issue with the crds directory that helm provides in that you cannot add new CRDs (or even new versions of CRDs) if you use it. Upgrades won't add them.

There are 3 ways I know of that you can handle CRDs, 2 of which have issues:

  1. Use the crds directory. This has the issue mentioned above.
  2. Put the CRDs in the templates. This has the issue with the stale discovery cache mentioned in the other issue.
  3. Put the CRDs in the templates of a separate helm chart. This works if you install the CRDs chart first, but runs into the issues from number 2 if you try to use the CRDs helm chart as a dependency of another chart.

Possible that I'm missing something, but I've been struggling with helm and CRDs for a while and haven't found any possible option for installing CRDs and CRs in one helm install/upgrade in a way that is reliable.

I'd like to share my approach here.

Again, we put the crds directory in the crds directory (it doesn't matter what directory we put in for this solution) and mount the contents of the crds directory into a job with "helm.sh/hook": pre-install,pre-upgrade annotations added, the pre-hook-install/upgrade job will exec kubectl apply -- server-side-f /crds --force-conflicts before helm install or upgrade executes.

@TylerHelmuth
Copy link
Member

@JaredTan95 have you used that technique for other CRDs already? Do you know of any other helm charts doing that?

@JaredTan95
Copy link
Member

@JaredTan95 have you used that technique for other CRDs already? Do you know of any other helm charts doing that?

@jaronoff97
Copy link
Contributor Author

CLosing this in favor of #1214

@jaronoff97 jaronoff97 closed this Jun 5, 2024
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.

4 participants