zammad-helm
zammad-helm copied to clipboard
Allow customizing init job
This change allows cusomizing the annotations on the init Job.
I use Argo CD to deploy this helm chart. Argo CD periodically checks the health of the application and applies the chart again in case of any inconsistency. Because the init Job is not a Helm hook, it is treated as a normal resource by Argo CD. When the job completes, it is deleted after 300 seconds, which causes Argo CD to recreate it, which then causes the job to run over and over again, causing the search to not be available sometimes because the index gets deleted and recreated.
With this change, I can declare the init Job as an Argo CD hook (which is less limited than the Helm counterparts) and disable the randomization of the Job name to make it not recreate the job indefinitely.
initJob:
randomName: false
annotations:
argocd.argoproj.io/hook: Sync
Checklist
- [x] Chart Version bumped
I just noticed another problem with this after opening the PR, I'll fix it and apply your suggestions there.
@monotek Applied your changes and squashed all commits. I had to add a toggle to disable the random Job name as well, because I found Argo CD re-rendering the chart in a loop on manual sync, which then caused it to create infinite init jobs.
@timoschwarzer looks nice in general. Thank you very much for contributing to improve zammad-helm!
I wonder if there is a way to detect that Helm is run inside of argocd (or that the template
command was used, which is what argo does). If we could detect that, there would not be a need to make the job name customizable -- we could just choose the proper behaviour directly.
In case it only works via customization, we should document it in zammad/README.md
. Maybe before the section "Deploying on OpenShift" as "Deploying via Argo CD"?
I would prefer to not add any ArgoCD deployments specific documentation. The chart should be total agnostic to what it is installed with. In fact i only approved that change because it's done in a generic way and could be used for other purposes too. The problem here is (as always) ArgoCD, which is just a pain in the ass if you work with helm charts.
Ok, I understand about not adding specific code, but why should we not document it? People are using it, and they will always run into the same trouble until they find a solution for themselves if we don't help them.
The supposed way to install the chart is: "helm install zammad" That works out of the box and is documented.
Sure, there are many other ways to do it but why should we add the documentation for it if they cant handle it right? How big should the documentation get to handle any non standard way to do it? If your deployment way / tool does not work out of the box it should be documented alongside the docuemtnation of the tool you use.
@monotek
The problem here is (as always) ArgoCD
While I'm with you that Argo CD has its weird quirks, this one I can't fully blame it for. The main reason this helm chart doesn't work well with it is the use of a random job name, which speaking for myself I've never seen before in other charts and seems to be a workaround for the post-install hook not working with how we do things in this chart.
I see that the intended installation method is helm install
, but I suppose I'm not the only one using a GitOps tool to deploy charts to the cluster. Whether to document this quirk or not is up to you, but personally I'd appreciate it.
I use FluxCD and never have to adjust helm charts to get it work.
As i maintain some other helm repos too i'm just annoyed by all the patches argocd users want to add.
Sorry for the rant, though. I'm meanwhile kind of triggered as soon as i read ArgoCD :D
@timoschwarzer you are right, the random name is kindof a workaround as described in the comment. I'm open for any better ideas!
My proposal would be that you rebase the PR and switch it to the new version 6.3.1, move the new settings to Values.zammadConfig
as described in the comment, and add a short section to the documentation for anyone intending to deploy on Argo CD. We can also add a friendly rant in there, that's fine. But I would prefer to do this in service of our users, even though we may not agree on the tool they use.
Thanks! I'll probably not get to do it until next monday though, will do it then.
@mgruner done.
...and now the linter is happy as well.
@timoschwarzer looks great! Maybe some short hint in the readme as well?
@mgruner Sorry, forgot about that! It's added now.
@timoschwarzer thanks a lot for your contribution!