Snapshot CRDs are referenced two charts

[phoenix-bjoern]

I've stumbled over a pretty wired issue: The snapshot-controller and the snapshot-validation-webhook chart both import the "external snapshot CRDs" within their Makefile. When declaring both charts as a dependency in Chart.yaml and run helm template an error is returned.

Steps to reproduce:

1. Create a Chart.yaml in an empty directory:

apiVersion: v2
name: snapshot-test
description: A Helm chart for Piraeus Operator
type: application
version: 0.1.0
appVersion: 0.1.0
dependencies:
  - name: "snapshot-controller"
    version: "1.9.1"
    repository: https://piraeus.io/helm-charts/
  - name: "snapshot-validation-webhook"
    version: "1.8.2"
    repository: https://piraeus.io/helm-charts/

2. Run helm dep up to pull the dependencies.
3. Generate the template and write them to an output dir:

$ helm template --include-crds --output-dir test .
wrote test/snapshot-test/charts/snapshot-controller/crds/groupsnapshot.storage.k8s.io_volumegroupsnapshotclasses.yaml
wrote test/snapshot-test/charts/snapshot-controller/crds/snapshot.storage.k8s.io_volumesnapshotclasses.yaml
wrote test/snapshot-test/charts/snapshot-controller/crds/snapshot.storage.k8s.io_volumesnapshotcontents.yaml
wrote test/snapshot-test/charts/snapshot-controller/crds/snapshot.storage.k8s.io_volumesnapshots.yaml
Error: open test/snapshot-test/charts/snapshot-validation-webhook/crds/groupsnapshot.storage.k8s.io_volumegroupsnapshotclasses.yaml: no such file or directory

I will also file a bug in the Helm Github repo, but from my tests I'm pretty sure this issue is caused by importing the same CRDs in both charts.

The question here is: Is this intended or should the CRDs only exist in the snapshot-controller chart?

[phoenix-bjoern]

Filed an issue in Helm repo as well: helm/helm#12513

[phoenix-bjoern]

The helm template issues has been fixed in the Helm 3.13.0 release.
Leaving the issue open for testing with ArgoCD.

[phoenix-bjoern]

As expected ArgoCD is complaining about the duplicate CRDs in the manifest.
So the initial question remains: Do both charts need to bundle the CRDs or can be they removed (at least in one of the charts)?

[WanzenBug]

Is there a way to disable CRD rendering in ArgoCD?
If not, we could also switch to providing the CRDs as a default-enabled normal template, like cert-manager does it with installCRDs=true 🤔

[phoenix-bjoern]

@WanzenBug yes, but in Argo CD it is an "all or nothing toggle": either install CRDs or not. And of course it makes sense that Argo CD installs the CRDs on the first deployment. (You don't want to do it manually, instead Argo CD should handle all for you, also updates, this is what GitOps is about!)

Actually the problem is not limited to CRDs. The problem is that the same resources are declared within the same Argo CD application twice.

Here is my "meta" Chart.yaml:

apiVersion: v2
name: linstor
description: A Helm chart for Piraeus Operator
type: application
version: 0.1.0
appVersion: 0.1.0
dependencies:
  - name: "piraeus"
    version: "1.10.7"
    repository: "file://../../external-charts/piraeus-operator/charts/piraeus"
  - name: "snapshot-controller"
    version: "1.9.1"
    repository: https://piraeus.io/helm-charts/
    condition: snapshot-controller.enabled
  - name: "snapshot-validation-webhook"
    version: "1.8.2"
    repository: https://piraeus.io/helm-charts/
    condition: snapshot-validation-webhook.enabled
  - name: "piraeus-ha-controller"
    version: "1.1.4"
    repository: "https://piraeus.io/helm-charts/"
    condition: piraeus-ha-controller.enabled

Argo CD accepts a resource only once. As long as the snapshot-controller and the snapshot-validation-webhook chart define the same resource (="(namespace +)? kind + name") , they can not be deployed together. As a workaround you could deploy e.g. the snapshot-validation-webhook separately, but technically it doesn't make much sense to deploy one without the other chart, right?

If both charts rely on the CRDs then it maybe makes more sense to merge both charts in a "2.0" version and add a toggle for the controller and the webhook to switch them on/off individually.

I'm happy to create a PR, just let me know which solution makes more sense to you.

[WanzenBug]

Yeah, merging it makes sense. I'm still thinking it would also make sense to not use the Helm ./crd directory, as that has a lot of known limitations, and we don't actually need them to be installed first.