Open jessebot opened 1 year ago
From my understanding, adding a hook means that the ConfigMap will be removed once the helm install completes?
https://helm.sh/docs/topics/charts_hooks/#hook-deletion-policies
In which case, either we need to pass everything the Job needs to it directly, or we create a separate ConfigMap just for the Job, which will also be cleared up after.
From my understanding, adding a hook means that the ConfigMap will be removed once the helm install completes?
Oh, great catch! but are we sure? This part of the docs makes it seem like they are left alone:
Hook resources are not managed with corresponding releases
The resources that a hook creates are currently not tracked or managed as part of the release. Once Helm verifies that the hook has reached its ready state, it will leave the hook resource alone. Garbage collection of hook resources when the corresponding release is deleted may be added to Helm 3 in the future, so any hook resources that must never be deleted should be annotated with
helm.sh/resource-policy: keep
.
So potentially we could just add a helm.sh/resource-policy: keep
annotation? 🤔
But the doc you linked says:
before-hook-creation
- Delete the previous resource before a new hook is launched (default) ... If no hook deletion policy annotation is specified, thebefore-hook-creation
behavior applies by default.
Those feel conflicting, but I think it means that the configMap in this instance would only be deleted before a new resource is launched, so at the time of another helm install/uninstall
, correct? I can totally be wrong on this, but regardless to your second point:
In which case, either we need to pass everything the Job needs to it directly
We could do that! I'd need to know if it needs everything from the configmap though?
or we create a separate ConfigMap just for the Job, which will also be cleared up after.
That could also work, but we still need to know if that job needs absolutely all the same config map values? I can modify this PR to do this quick and dirty by just copying configmap-env.yaml
to configmap-dbmigrate-env.yaml
and then reverting back the changes on configmap-env.yaml
?
I tested the full helm install and the hook is not automatically deleted right now, but we can add the helm.sh/resource-policy: keep
annotation anyway to be safe.
After I gave this PR a shot in my local cluster to make sure the behavior is as expected, it looks like the db-migrate job still fails because it needs to connect to redis, so I guess there's another question: is there a way to wait for the sub-charts that are dependencies of this one before installing the configmap and db-migreate job? Can we tell helm to finish setting up redis and postgres/mariadb before doing the migration job? 🤔
Actually, after reading this stack overflow post, I was thiking: would it make more sense to have an init container on the main deployment instead of all this helm hook stuff? That way we just run these commands on init:
- bundle
- exec
- rake
- db:migrate
I don't write ruby very often at all, so does db:migrate
do anything that we wouldn't want happening on each new pod creation? If it shouldn't be run everytime, we need some sort of location we can update to say it's done, and if it can run everything time, then I feel like an initContainer solves the whole shebang.
We'd probably also need a container for making sure postgres/mariadb were ready, similar to how we do in nextcloud/helm like:
{{- else if .Values.postgresql.enabled }}
- name: postgresql-isready
image: {{ .Values.postgresql.image.repository }}:{{ .Values.postgresql.image.tag }}
env:
- name: POSTGRES_USER
valueFrom:
secretKeyRef:
name: {{ .Values.externalDatabase.existingSecret.secretName | default (printf "%s-%s" .Release.Name "db") }}
key: {{ .Values.externalDatabase.existingSecret.usernameKey | default "db-username" }}
command:
- "sh"
- "-c"
- {{ printf "until pg_isready -h %s-postgresql -U ${POSTGRES_USER} ; do sleep 2 ; done" .Release.Name }}
{{- end }}
So, if I use both an external postgresql database and an external redis host, everything works fine after the adding the helm hook to the config map. I'm doing that over at jessebot/mastodon-helm-chart if anyone would like to take inspiration for their own forks. I've also made parameters for existingSecrets for all secure values, so you should be in a better state overall. I'll try to submit additional PRs to merge more changes back into this repo for the other changes too.
This is to solve #73 where the db-migrate job tries to run before the configmap it needs is in place. This change will just install the configmap before the db-migrate job.