feat(Chart)!: :package: add separated chart for CRDs

[darkweaver87]

What does this PR do?

This PR introduces a separate chart used by the main one to install CRDs. The dependency between the main chart and the CRDs one will be added once CRDs one will be released.

Motivation

https://github.com/traefik/traefik-helm-chart/issues/1141 https://github.com/traefik/traefik-helm-chart/issues/1209

More

• [x] Yes, I updated the tests accordingly
• [x] Yes, I updated the schema accordingly
• [x] Yes, I ran make test and all the tests passed

[darkweaver87]

Tested on my own account as seen with you :+1:

[mloiseleur]

@jnoordsij If you think we should not do it, it's still time and possible to explain why !

[jnoordsij]

I definitely have some mixed thoughts about it. While I totally see the issue with CRDs and Helm and the reason this might call for some different structure, that also implies that probably we should tread carefully in finding a solution that works around Helm limitations.

To be more concrete: I can agree with using a separate chart for CRDs, but I'd rather make it entirely stand-alone than add it as a dependency to the main chart. Reasoning:

1. By installing it as a dependency, it is now subject to all the possible issues that may arise when using Helm for managing CRDs: rollbacks or uninstalls of the chart may cause CRDs to be altered/removed in such a way that global cluster behavior might be impacted. This is particularly problematic for cases where one wants to install multiple instances of Traefik within the same cluster.
2. Creating a separate chart and then adding it as a dependency seems odd, as this makes it harder to manage them separately for end-users, given all operations will influence both.
3. Managing it as a dependency is somewhat of a burden (minor issue)
4. Helm docs seem to suggest separate charts as a way to deal with CRDs, but from what I understand from the wording they (probably for above reasoning) suggest to have them installed separately: https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#method-2-separate-charts.
5. Thus we somewhat go against Helm conventions, which may be confusing to some users, although probably these users know enough about CRDs already to tackle the issue.

Some other alternatives we may opt for are:

1. Keeping things as is and improving documentation on CRDs and upgrading; probably quite tricky given how hard are CRDs to deal with and the amount of complexity involved in explaining the issues (see https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#method-1-let-helm-do-it-for-you).
2. Rather than create the overhead of a dependency chart, move the crds from the Helm-default crd directory to templates and install from there. Then add a resource-policy to the CRDs to prevent accidental This allows for upgrading them through the chart without the burden/complexity of an entire new chart. I've seen this pattern to be used in the cert-manager Helm chart, which might serve as a working example employed elsewhere.
3. Removing CRD management with Helm entirely and let users manually manage them all the way.

I think the most important trade-off here is how much knowledge/action is required from the end-user vs. potential risks by simplifying things for them, but in turn making some seemly harmless operations actually unsafe. The usecase which is probably most impacted by any change is whenever there's multiple instance of the chart being installed in the cluster. Given this is an advanced usecase we can probably expect end-users employing this pattern to generally know quite a bit more about Helm/CRDs and thus focus on the "simple" usecase in terms of end-user convenience, as long as we make sure the choice is documented clearly for those with a more advanced installation.

All in all I think my preference would be the following:

1. Create the separate chart for managing CRDs, but NOT as a dependency
   • For basic usage, the installation instruction would then be install the CRD chart, then install the main chart; then upgrading instructions would be first update your CRD chart, then update the regular chart. This requires some additional action for basic usage, but limited to 'simple' Helm operations so probably ok?
   • This prevents accidents with operations applied while altering/upgrading the main chart.
   • For advanced usage, we can simply tell people: use the CRD chart if you want to manage it in that way; manually install them if you prefer that, otherwise just don't and manage on your own. This is quite easy and more or less corresponds to current behavior
2. Add the CRDs as templates within the chart like in the cert-manager chart
   • This pattern is used elsewhere already and thus has some real-world proof of being applied
   • Probably the most easy for end-users without knowledge about CRDs, while potential breakages are limited by the resource policy
   • For advanced usage we just need to document the flag that disables this and leaves management to the user. However they can then not really benefit from updates added here.
3. Keep things as is, but find a way to better reach users on how to update CRDs
   • I think CRDs are just slightly too complex as a concept, especially with Helm, and this puts too much burden on end-users with a simple setup
4. Add separate chart for managing CRDs as a dependency
   • Still possible, however I think the options above are just better alternatives
5. Removing CRD management with Helm entirely and let users manually manage them all the way
   • Basically option 3. but even more complex, so this probably helps no-one

Final concern: for all cases, I've not yet checked how upgrading will go from the current system towards the new one. Have you tested this yet? I'm not sure if/how Helm can deal with CRDs that have previously been installed by the main chart, and might suddenly be claimed by the new CRD chart after applying this. Similarly issues may arise in other options.

(This has become a bit longer that I intended, hopefully I managed to get my main concerns addressed. Make sure to ask me on anything unclear, as I understand this might raise quite a few questions.)

[mloiseleur]

Damn it, that's right. It will break many configuration with the dependency :warning: ! And without the dependency, the install instruction needs to be changed, it won't work out of the box. Even simple blog post with helm install will fail :(.

I'm quite interested in option 2, with doubts ; I'm unsure if the CRDs can be deployed before being used by the dashboard or the healthcheck IngressRoute. As you said, it is the most easy for the users, and that's what we want. We want to keep this and allow advanced users / new features like Gateway API to be more flexible.

=> I'll make some tests on this and open a PR if it looks good.

Many thanks for this review :pray:

[darkweaver87]

Thanks @jnoordsij for your review :pray:

To be honnest, I didn't test to install the chart with CRDs and then upgrade with the one with dependencies. It's very important to validate it before anything else.

That said, for the rest of the concerns, I can understand it but let's summarize the use cases. I see 3 scenarios:

1. people who just want a simple installation and rely on us: this is currently a nightmare to handle CRDs
2. people who have a custom installation and just want to install CRDs in a simpler manner
3. people who have multiple instances of this chart and want to manage CRDs on one hand (I want CRDs for v32.1.0 of the chart with gateway API enabled) and the resources (v32.1.0)

For this, we have multiple solutions, adding your propositions (@jnoordsij):

• doing nothing. I can't even think about this. As a user I would prefer seeing people developing the software I use trying to solve my issues, failing and iterating rather than doing nothing for me :-)
• having crds in a crd directory might work. I've checked the ordering and it's good. It can address all scenarios but it will require from us somehow to add big {{ if .Values.addCRDs }} or {{ if .Values.onlyAddCRDs }} in all our templates which is, I think, not elegant at all
• create a separate chart for CRDs but not as dependency. This will not cover use case 1. Also, the won't ensure people to get the right version of the CRDs along with the current chart version.
• create a separate chart for managing CRDs as a dependency. We will lose people. I've seen to many projects where a lot of charts have been created, without deprecation notices, and I didn't know which one to choose
• create a separate chart for CRDs as a dependency like this PR propose. This will cover use case 1, 2 and 3. For use case 2 and 3, the dependency will still be here but having all boolean to false will ensure CRDs not being installed. Also, we will have the capability to specify which version goes with a given chart version (like the crd directory solution).

[mloiseleur]

Add the CRDs as templates within the chart like in the cert-manager chart

This morning, I have explored this option. I have encountered two major issues:

1. On install: It fails to deploy dashboard IngressRoute (CRDs are not loaded at this stage)
2. On upgrade: It fails to deploy CRD: invalid ownership on metadata and annotations

I haven't found a way to switch CRDs from current Helm CRD management to Helm Template management without failure. Instructions can be provided to patch resources beforehand, and thus get around this failure. Maybe it's possible to deploy IngressRoute in post-install hook, but should we really do this ?

We are really fighting the tide, here.

This use case of selectively picking CRDs is specific. It's clearly not the common use case.

I think we are left with two options on this:

1. Option Standalone CRD Chart:
   • The common use case works is not modified (current Chart stays as it is today)
   • Users has to opt-in to use this new Chart
   • There is no link between them.
   • Like karpenter, we provide two Charts, the current one with helm native CRDs and a new one with helm templated CRDs.
2. Option Documentation:
   • Instructions in the README on how to install CRDs separately
   • Users with specifics needs on CRDs deploy them separately and can install the Chart without CRDs after

@jnoordsij @darkweaver87 : Wdyt ?

[mloiseleur]

Additional notes:

• Helm maintainers have started to work on Helm v4, see here.
• CRDs management are discussed in the roadmap. Helm maintainers have also detailed why it's working this way in HIP-0011.

[jnoordsij]

I think I'm still with option 1 like I was before when explaining my thoughts. CRDs in Helm are just quite complicated at the moment, so I don't think there is a solve-all solution without adjustments on the Helm side. Although managing two charts at once might be complicated for users, if we document matching versions it hopefully should not be to complicated and will provide a way of updating CRDs that is a lot easier then users having to do it manually.

[darkweaver87]

I'm not strongly against option 1, but I think we will clearly lost users on the way. The approach of having a dependency chart seduced me at first because of managing all this compatibility complexity being a responsibility delegated to us. With option 1, we will somehow, have to complete a compatibility matrix for users to know which version of CRD chart comes with which chart version or traefik/traefik-hub version. Even for us, managing a crds will be a nightmare as we will have to maintain a CRD chat and a crds directory. Or, we need to test if symlinks work on charts.

The other option is keeping it that way:

• people who don't want to bother with it and let us manage their CRDs for them will just need to set:

  traefik-crds:
  traefik: true

• people who want to manage two or more instances of traefik will just need to install the chart as previously and either manage CRDs by themselves or install the CRD chart (they will be able to find which version of the CRD chart comes with which version of traefik or traefik-hub following the dependencies section of the main chart)
• people who want a specific version of traefik will fall in the previous case

However, even with the option 1, the main points remains how users can migrate from the previous system to the new one ? Adding the annotation works but the command I provided is mainly linux friendly :-)

[genebean]

It might be worth checking out how https://github.com/prometheus-community/helm-charts/tree/main/charts/kube-prometheus-stack is setup. It bundles https://github.com/prometheus-community/helm-charts/tree/main/charts/prometheus-operator-crds in but has an option to not install them for when you want to manage them separately. I’ve been using that lately and it works well and isn’t too difficult.