ArgoCD support for helm chart

[jesco39]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
• If you are interested in working on this issue or have submitted a pull request, please leave a comment.

---

Is your feature request related to a problem? Please describe.

I am deploying the consul helm chart via ArgoCD. When I set global.acls.manageSystemACLs to true and sync via ArgoCD, the deployment is successful but the application remains out of sync. I believe the reason is because of the absence of helm hooks and a hook delete policy for the job, and instead a cleanup job exists for server-acl-init. I tried to fork the chart and add hooks but I believe I found out why this was done this way. I tried a number of combination of hooks and was not able to get consul to fully sync and deployed as there definitely seems to be an order of operations that needs to happen that the helm hooks are unable to resolve.

I found a workaround by setting ArgoCD annotations on the server-acl-init-job resource.

metadata:
  annotations:
    argocd.argoproj.io/hook: Sync
    argocd.argoproj.io/hook-delete-policy: HookSucceeded

I have not tested this on an upgrade, so I am unsure if these hooks are going to allow me to upgrade without doing some sort of song and dance.

Feature Description

I would like to have either the server-acl-init-job resource delete itself in a clean way via helm hooks or have the ability to provide custom metadta.annotations so that applications like ArgoCD can sync successfully.

Use Case(s)

This would cover applications like ArgoCD to manage consul helm deployments and keep the helm chart in sync.

Contributions

yes, but my current attempts have bared no fruit.

[david-yu]

@jesco39 Would it be possible to create a PR for this enhancement. I am also aware that better ArgoCD support is needed but we have not prioritized looking into this at the moment. We can review a PR if you are able to put one up. Thank you.

[jesco39]

@david-yu sure, if the proposed solution of adding metdata.annotations to server-acl-init-job is acceptable, I can work on a PR.

[david-yu]

Hi @jesco39 that seems acceptable. Essentially any component that is deployed with a Helm hook would also need to have such annotations to accommodate for ArgoCD. Let us know if you are able to make progress and see if upgrades also work based on some modifications.

[bo0ts]

I've had a short look at the situation and I'm a bit confused by the current state. ArgoCD supports almost all helm.sh/hook annotations: https://argo-cd.readthedocs.io/en/stable/user-guide/helm/#helm-hooks

The problem seems to be the extra clean-up job/server-acl-init-cleanup that is used to delete job/server-acl-init-job. This job has a "helm.sh/hook-delete-policy": hook-succeeded,hook-failed which is correctly understood by ArgoCD, but why use this job at all? It seems to me that "helm.sh/hook-delete-policy": hook-succeeded could just be added to job/server-acl-init and get the same behavior?

[liad5h]

I don't see a way to pass these annotations without forking the chart, am I missing something?

[david-yu]

Hi folks, we added the ability to add those annotations as part of this PR: https://github.com/hashicorp/consul-k8s/pull/2525. This should be available in our 1.0.x, 1.1.x, and 1.2.x patch releases within a month or so. If there are other blockers to ArgoCD please open up a separate issue so we can track. Thank you.