argo-helm icon indicating copy to clipboard operation
argo-helm copied to clipboard

Can't use argocd chart as a dependency with application resources in a single run

Open bradenwright-opunai opened this issue 2 years ago • 6 comments

Describe the bug

I use argocd as a dependency and have Application resources in my templates directory. CRD's don't get installed before other resources, I think since CRD's are in a template directory and not the top level of the chart. This cases my deployment to fail:

unable to build kubernetes objects from release manifest: unable to recognize "": no matches for kind "Application" in version "argoproj.io/v1alpha1"

With other charts that follow https://helm.sh/docs/chart_best_practices/custom_resource_definitions/ this approach works fine, b/c I install this chart via Terraform after K8s Clusters are created its not very clean or easy to either setup a separate Helm install for CRDs or run 2 install with enable/disable flags around the resources, so I'll probably have to break all the CRD resources out into a separate chart. Not the end of the world, but still annoying since the approach works well with most helm charts I use. Also I can't use argo pre/post sync b/c this is to setup ArgoCD

Related helm chart

argo-cd

Helm chart version

5.4.2

To Reproduce

helm create my-argocd update the Chart.yaml, setup argocd as a dependency:

dependencies:
  - name: argo-cd
    version: 5.4.2
    repository: https://argoproj.github.io/argo-helm

then create any CRD type resource like Application in the template directory of the new chart my-argocd that was create. When installing on a cluster that doesn't already have the CRDs the helm deploy will fail.

Expected behavior

The helm install will work when using argocd chart as a dependency and argocd crd resources in the same chart, or that argocd-crds would be in a chart / have a variable/method to install only the crds without the server and all the other components so I can more easily automate. Essentially use 1 of the 2 methods recommended by Helm docs (I dont care if its a separate chart if I can use a variable to only install crds and nothings else)

https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#install-a-crd-declaration-before-using-the-resource

Screenshots

No response

Additional context

No response

bradenwright-opunai avatar Sep 12 '22 03:09 bradenwright-opunai

@bradenwright-opunai what version of helm are you using? I've seen some weird CRD loading/not loading in earlier versions of helm 3 but latest has been good.

jmeridth avatar Sep 12 '22 19:09 jmeridth

I'm running into the same issue with Helm version 3.9.4 and ArgoCD Chart version 5.4.3. Switching Helm to 3.10.0-rc.1, 3.9.3 or 3.9.2 does not help either.

dangmai avatar Sep 13 '22 17:09 dangmai

I'll try upgrading and report back, right now I'm on:

braden@rltadmins-MBP-4 helm % helm version
version.BuildInfo{Version:"v3.5.2", GitCommit:"167aac70832d3a384f65f9745335e9fb40169dc2", GitTreeState:"dirty", GoVersion:"go1.15.7"}

bradenwright avatar Sep 14 '22 14:09 bradenwright

I upgraded to helm version.BuildInfo{Version:"v3.9.4", GitCommit:"dbc6d8e20fe1d58d50e6ed30f09a04a77e4c68db", GitTreeState:"clean", GoVersion:"go1.19"} which didn't help.

I've been able to fix the issue but putting the resources in top-level crds directory without templating as suggested by the helm docs method 1

Its just a manual process and something that needs to be remember on each upgrade of the ArgoCD chart (ok well tech only if there are changes to the CRDs). I had to run helm template and then create / copy only crds files to the top level crds directory.

I can def open a PR to move them to a top-level directory without templating but I want to get buy in before going thru the process.

bradenwright avatar Sep 14 '22 14:09 bradenwright

This PR moved CRDs to template folder https://github.com/argoproj/argo-helm/pull/1342. If I remember correctly it was done because lots of people were complaining that CRDs are not upgraded automatically with new Argo CD versions.

pdrastil avatar Sep 14 '22 16:09 pdrastil

@pdrastil the problem I see with that approach is that there isn't any guarantee that the CRD updates will occur before the custom resource is applied. I know that https://helm.sh/docs/chart_best_practices/custom_resource_definitions/ method #1 doesn't support upgrades / deleting CRDs

Having them in the template directory breaks my automations, so ArgoCD repo doesn't want to follow method #1 because of that then method #2 is having a separate repo for the CRDs. This approach would also allow me to automate. If people don't want to separate out the chart, then I could still accomplish what I need if I create a variable for only_install_crds b/c I could apply the with only CRDS in the first deploy / helm release, and then install everything without CRDs in the 2nd install.

Approaches:

  1. move crds to top level
  2. move crds to a separate chart
  3. create a variable so chart can only install crds and nothing else

My preference would be approach (1) but sounds like that may not be an option since it was moved into template directory recently. Options 2/3 would allow more control how crds are applied and update and order around those things. If that's the case if I open a PR for approach (3) would it be merged / accepted (unless people prefer option 2).

bradenwright avatar Sep 14 '22 20:09 bradenwright

@bradenwright I have one more idea that might work for you. Could you try following to force CRDs to be rendered / applied before anything else?

crds:
  annotations:
     "helm.sh/hook": pre-install, pre-upgrade

pdrastil avatar Sep 18 '22 19:09 pdrastil

for sure let me give that a go and see how it works, its a good suggestion

bradenwright avatar Sep 19 '22 20:09 bradenwright

@bradenwright I have one more idea that might work for you. Could you try following to force CRDs to be rendered / applied before anything else?

crds:
  annotations:
     "helm.sh/hook": pre-install, pre-upgrade

I tried setting these values in the values.yaml file, but It didn't work for me.

Transmitt0r avatar Sep 27 '22 06:09 Transmitt0r

Yes, I've also tested the helm hook annotations on the CRDs and found that I get the same error as the original post.

jarrettprosser avatar Sep 27 '22 06:09 jarrettprosser

I see thanks for feedback. What do you think about this @mkilchhofer

pdrastil avatar Sep 27 '22 08:09 pdrastil

Yes, the terraform helm_release resource uses native helm method to deploy a chart. And helm natively does not support to deploy CRDs and an instance of these CRD in one helm release (the helm install/upgrade command) except when CRDs are living inside the <chart-root>/crds directory.

IMHO no one should do that inside one helm release as there are dangerous side-effects. We ran into finalizer issues multiple times already during CI testing our infrastructure.

mkilchhofer avatar Sep 27 '22 08:09 mkilchhofer

I am having the same issue while I try to use argo-cd as a dependency of my App of Apps:

dependencies:
  - name: argo-cd
    version: 5.5.6
    repository: https://argoproj.github.io/argo-helm/

now results in:

...
no matches for kind "Application" in version "argoproj.io/v1alpha1"
ensure CRDs are installed first

@bradenwright I have one more idea that might work for you. Could you try following to force CRDs to be rendered / applied before anything else?

crds:
  annotations:
     "helm.sh/hook": pre-install, pre-upgrade

I have also tried the Your suggestion @pdrastil but that does not resolve it, neither as a dependency value or global value:

argo-cd:
  crds:
    annotations:
       "helm.sh/hook": pre-install, pre-upgrade
  dex:
    enabled: false
crds:
  annotations:
     "helm.sh/hook": pre-install, pre-upgrade

Seems this issue was highlighted here https://github.com/argoproj/argo-helm/pull/1342#pullrequestreview-1019481838 , but I didn't see any solution for those using argo-cd in the dependencies if there is one.

DreamingRaven avatar Oct 05 '22 08:10 DreamingRaven

Because of this I adopted the approach to apply the argo-workflows helm chart and my application chart in two steps. This seems to be a better approach anyways because of the challenges that come with managing CRD's with Helm. You can check HIP-0011 for more information.

Transmitt0r avatar Oct 05 '22 08:10 Transmitt0r

~~Ah I see, ok, thats a shame but manageable. I take it then that argo-workflows is the separate chart with the CRDs? (seems to be so from the README). I am also guessing that you have your application chart which still uses the argo-cd helm chart in this second step.~~

EDIT: I was about to report back that it did not work, but then I realised I was having a moment... I wanted argo-cd, however for some reason I thought argo-workflows was a CRD chart. So I installed argo-workflows then expected the app of apps for argo-cd to work. Woops. But yes ok two stages, first stage the full chart (argo-cd in my case), then launch the app of apps. So just completely separate out argo-cd which is a shame, it was so much cleaner as a dependency.

DreamingRaven avatar Oct 05 '22 08:10 DreamingRaven

So here's my setup: Argo-CD + Argo-Workflows charts get installed by terraform (stage 1, think of it like the "cluster management stage") and then I install my own app that uses argo-workflows with argo-cd in stage 2 (the application installation stage). Don't know if that's the best approach but seems ok for me. All the crd-installations are managed together with other cluster-related things.

Transmitt0r avatar Oct 05 '22 17:10 Transmitt0r

If you use terraform for stage 1, you could use the community argocd provider. It is awesome and TF wait until the app (of apps) is synced and healty.

mkilchhofer avatar Oct 05 '22 17:10 mkilchhofer

Yes, the terraform helm_release resource uses native helm method to deploy a chart. And helm natively does not support to deploy CRDs and an instance of these CRD in one helm release (the helm install/upgrade command) except when CRDs are living inside the <chart-root>/crds directory.

IMHO no one should do that inside one helm release as there are dangerous side-effects. We ran into finalizer issues multiple times already during CI testing our infrastructure.

Is not true, Helm supports that and is recommend to keep CRDs outside of templates (i'm pretty sure you know that :) ), especially for use resources based on this CRD's in same release, and it worked many years in ArgoCD chart, but now for just because not updating CRDs while update (what is mentioned by Helm docs), this method stop working. Not sure it was great idea, and how it was approved not clear.

CRDs are immutable structures, because many resource could use them already. Best practice for update CRDs is create new version, where new resources will use it or if is breaking changes, it will be mentioned while notes of Helm install, for inform developers to update their resources. Let's think about that bit more serious :)

evghen1 avatar Nov 04 '22 09:11 evghen1

@bradenwright-opunai @bradenwright Found workaround for this issue, just required to use "post-install" Helm hook for resources which use ArgoCD CRDs in same release [TESTED].

evghen1 avatar Nov 04 '22 10:11 evghen1

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Jan 04 '23 02:01 github-actions[bot]

Hello, sorry for reviving this thread :joy:

I encountered the same issue and using

  annotations:
    "helm.sh/hook": post-install

does solve the install. However I'm not able to update the main app (the bootstrap app) afterwords.

The main reason I need the main app to be upgradeable is because during chart development, I might need to branch off the main app. In fact the main app has among its values, a target revision field, that is usually set to HEAD, but if I'm developing a breaking change to any of the child apps, I might need to point to a different branch. This values/parameter is then forwards to child charts to deploy charts living on a branch.

I am not fully familiar w/ helm hooks, but I tried to add post-upgrade to the list of hooks above, but it doesn't seem to help.

How can I make the main app (app of apps or bootstrap app) upgradeable with helm hooks?

Any suggestions?

Thanks 🙏

nemo83 avatar Jul 18 '23 08:07 nemo83

Hello, sorry for reviving this thread 😂

I encountered the same issue and using

  annotations:
    "helm.sh/hook": post-install

does solve the install. However I'm not able to update the main app (the bootstrap app) afterwords.

The main reason I need the main app to be upgradeable is because during chart development, I might need to branch off the main app. In fact the main app has among its values, a target revision field, that is usually set to HEAD, but if I'm developing a breaking change to any of the child apps, I might need to point to a different branch. This values/parameter is then forwards to child charts to deploy charts living on a branch.

I am not fully familiar w/ helm hooks, but I tried to add post-upgrade to the list of hooks above, but it doesn't seem to help.

How can I make the main app (app of apps or bootstrap app) upgradeable with helm hooks?

Any suggestions?

Thanks 🙏

Okay, maybe I jumped the guns too quickly, using:

  annotations:
    "helm.sh/hook": post-install, pre-upgrade

seems to work as expected.

Opinions?

nemo83 avatar Jul 18 '23 08:07 nemo83