[Argo Workflows] CRDs are not deployed as part of the Helm Chart

[christophercutajar]

Describe the bug

Argo Workflow CRDs located in https://github.com/argoproj/argo-helm/tree/main/charts/argo-workflows/crds aren't deployed automatically.

Related helm chart

argo-workflows

Helm chart version

0.17.1

To Reproduce

1. Add argo-workflows in the requirements.yaml

dependencies:
  - name: argo-workflows
    repository: "https://argoproj.github.io/argo-helm"
    version: 0.17.1

2. Sample Values file and deploy

argo-workflows:
  workflow:
    serviceAccount:
      create: true
    rbac: 
      create: true
  controller:
    rbac:
      create: true
    serviceAccount: 
      create: true

3. Check the installed CRDs such as argoproj.io_workflowtaskresults.yaml is not automatically installed

Expected behavior

All CRDs should be installed

Screenshots

kubectl get crds

NAME                                             CREATED AT
applications.argoproj.io                         2021-12-02T15:55:07Z
applicationsets.argoproj.io                      2022-01-10T19:51:08Z
appprojects.argoproj.io                          2021-12-02T15:55:08Z
backendconfigs.cloud.google.com                  2021-11-30T20:21:46Z
capacityrequests.internal.autoscaling.gke.io     2021-11-30T20:21:09Z
certificaterequests.cert-manager.io              2022-05-03T17:26:17Z
certificates.cert-manager.io                     2022-05-03T17:26:17Z
challenges.acme.cert-manager.io                  2022-05-03T17:26:17Z
clusterissuers.cert-manager.io                   2022-05-03T17:26:17Z
clusterworkflowtemplates.argoproj.io             2021-12-02T15:55:06Z
cronworkflows.argoproj.io                        2021-12-02T15:55:06Z
eventbus.argoproj.io                             2022-08-23T15:58:43Z
eventsources.argoproj.io                         2022-08-24T11:26:35Z
frontendconfigs.networking.gke.io                2021-11-30T20:21:46Z
issuers.cert-manager.io                          2022-05-03T17:26:17Z
managedcertificates.networking.gke.io            2021-11-30T20:21:13Z
orders.acme.cert-manager.io                      2022-05-03T17:26:17Z
sensors.argoproj.io                              2022-08-23T15:58:43Z
serviceattachments.networking.gke.io             2021-11-30T20:21:47Z
servicenetworkendpointgroups.networking.gke.io   2021-11-30T20:21:46Z
storagestates.migration.k8s.io                   2021-11-30T20:21:15Z
storageversionmigrations.migration.k8s.io        2021-11-30T20:21:15Z
updateinfos.nodemanagement.gke.io                2021-11-30T20:21:16Z
volumesnapshotclasses.snapshot.storage.k8s.io    2021-11-30T20:21:14Z
volumesnapshotcontents.snapshot.storage.k8s.io   2021-11-30T20:21:15Z
volumesnapshots.snapshot.storage.k8s.io          2021-11-30T20:21:15Z
workfloweventbindings.argoproj.io                2021-12-02T15:55:07Z
workflows.argoproj.io                            2021-12-02T15:55:08Z
workflowtemplates.argoproj.io                    2021-12-02T15:55:08Z

Additional context

https://github.com/argoproj/argo-helm/tree/main/charts/argo-workflows/crds folder should be moved into the https://github.com/argoproj/argo-helm/tree/main/charts/argo-workflows/templates folder

[christophercutajar]

If folks agree with the solution, I can create a PR and update accordingly.

[jmeridth]

They are installing for me currently.

Also they are not in the templates directory because there is no helm templating logic in them like the other charts. argo-events and argo-rollouts have helm conditional at the top of them. Example

[fazilhero]

I also had to install manually via kubectl command, didn't install it for me as well.

[christophercutajar]

They are installing for me currently.

Also they are not in the templates directory because there is no helm templating logic in them like the other charts. argo-events and argo-rollouts have helm conditional at the top of them. Example

@jmeridth Agreed, the solution should be to include {{- if .Values.crds.install }} to all CRDs so they are installed automatically via the helm chart. For me it's a recurring issue that these are not installed and had to install them manually.

[stefansedich]

@christophercutajar what version of helm are you using and how are you installing your release? The crds folder is the correct Helm3 way of doing things

[christophercutajar]

@christophercutajar what version of helm are you using and how are you installing your release? The crds folder is the correct Helm3 way of doing things

@stefansedich We're using Helm3 and we've mainly tried deploying the chart manually and using Terraform.

Manually:

Using the files mentioned in the description together with a Chart.yaml file, we deployed Argo Workflows using the below commands.

helm upgrade --install --namespace default --values ./values.yaml argo .

Terraform:

resource "helm_release" "argo" {
  name              = "argo"
  chart             = <chart_path>
  dependency_update = true
  version           = <dedicated_chart_version>

  values = [
    "${file(<path>/values.yaml")}"
  ]

  depends_on = [
    helm_release.storage
  ]
}

In both cases, we had to deploy the CRDs manually.

[jmeridth]

I have completely uninstalled all argo-workflows CRDs and then reinstalled the helm chart. Locally and in remote clusters via argo-cd. CRDs install for me. Helm v3.4.9. I'll try your command next.

Putting the crds folder in templates folder and wrapping in conditional causes the install to work for you? Have you built the chart locally and tested this?

I don't doubt your experience but want to test this fully.

[christophercutajar]

I have completely uninstalled all argo-workflows CRDs and then reinstalled the helm chart. Locally and in remote clusters via argo-cd. CRDs install for me. Helm v3.4.9. I'll try your command next.

Putting the crds folder in templates folder and wrapping in conditional causes the install to work for you? Have you built the chart locally and tested this?

I don't doubt your experience but want to test this fully.

@jmeridth as a collaborator, it's ok to question everything and everyone! All good mate, that's how the product get's better 😄 Also, I should have posted all my testing. The below are the outputs from my re-test done this morning for my PR. The only thing, I didn't test was deploying it using Terraform!

helm upgrade --install --namespace default --values ./values.yaml argo .
Release "argo" does not exist. Installing it now.
NAME: argo
LAST DEPLOYED: Thu Sep  1 11:23:18 2022
NAMESPACE: default
STATUS: deployed
REVISION: 1
TEST SUITE: None

CRDs:

kubectl get crds | grep argo
clusterworkflowtemplates.argoproj.io                 2022-09-01T09:23:34Z
cronworkflows.argoproj.io                            2022-09-01T09:23:34Z
workfloweventbindings.argoproj.io                    2022-09-01T09:23:34Z
workflows.argoproj.io                                2022-09-01T09:23:34Z
workflowtaskresults.argoproj.io                      2022-09-01T09:23:34Z
workflowtasksets.argoproj.io                         2022-09-01T09:23:34Z
workflowtemplates.argoproj.io                        2022-09-01T09:23:34Z

So, I'm running Helm v3.9.0 and this great discussion made me trying to re-install our production deployment in another k8s test cluster and re-produce last week scenario. This morning for both 0.16.5 and 0.17.1 charts, CRDs were installed! I'm not sure why this inconsistency is happening, when as highlighted also during this discussion, Helm 3 should deploy CRDs by default. Unfortunately, I didn't save any logs from last week. 😞 I remembered that around 2 months ago, during one of our upgrades, we had the same issue with CRDs not being installed and I had a discussion on slack about this too. Now I'm not sure if it's something related to Terraform or not (I'm going to spend some time and try to test it using Terraform).

Going forward, I know this is a potential breaking change and I'm ok to not merge my PR so that Helm processes are followed and so close this ticket. We can always re-open this if it is noticed that folks are reporting the same behaviour.

[jmeridth]

@christophercutajar thank you so much for your understanding. LOVE LOVE LOVE the contribution and conversation. Thank you for testing and showing the upgrade helps. We do need to discuss backwards compatibility with helm 3 that is < 3.9.4 (latest). There obviously seems to be an issue. I'm going to downgrade also and see what happens. You may be onto something.

Again, thank you very much for your understanding and patience.

[jmeridth]

@christophercutajar sorry for the delay. I'm going to test the downgrade today. Cheers.

[christophercutajar]

This might be fixed in https://github.com/hashicorp/terraform-provider-helm/releases/tag/v2.9.0 as specifically in hashicorp/terraform-provider-helm#1050 a new enhancement was added which added a new attribute crds which when include_crds is set to true will be populated with a list of the manifests from the crds/ folder of the chart