search

Alvearie / alvearie-helm

repository for the helm chart source and package for Alvearie projects
https://artifacthub.io/packages/helm/linuxforhealth
Apache License 2.0
3 stars 5 forks source link

Add switch to schema migration job auto-removal #68

Closed bauerjs1 closed 2 years ago

bauerjs1 commented 2 years ago

Hi everyone,

for our local deployment, we use ArgoCD as the Kubernetes CI tool. When the TTL for finished schema migration jobs is set and they are removed after completion, we hit an infinite loop where Argo attempts to recreate the job (because it doesn't exist anymore), which is then removed again, and so on...

My suggestion is to add a flag in the Helm values, so that the auto-removal can be disabled. I was not sure what might be the better default value for this, so i left it enabled by now.

Cheers, Hannes

lmsurpre commented 2 years ago

Thanks @bauerjs1 and apologies for taking so long to get to this...your changeset seems reasonable to me.

We have some linting in place that will block us from merging the PR without bumping the Chart version number. This is because we're currently configured to do a release each time we merge to main.

We probably need to document how to do this properly, but for now we need two changes with each PR:

  1. bump the chart version at https://github.com/Alvearie/alvearie-helm/blob/main/charts/ibm-fhir-server/Chart.yaml#L4 (this is minor change so should become 0.5.2)
  2. replace the changes list in Chart.yaml with whatever changes are included in the PR
  3. run helm-docs to generate an updated README

There may be a better way to configure that automation, but for now thats the process... I am happy to make the changes on top of your PR if thats easier for you.

lmsurpre commented 2 years ago

Since its been a while since this was opened and I don't have write access to the fork, here's what I'll do:

  1. merge this into our integration branch
  2. add my commit on top
  3. open a new PR to get this into main
bauerjs1 commented 2 years ago

Sorry as well, @lmsurpre, for taking so long... thank you for the integration! Good idea to just make the timeout configurable.