Commit 3fb943be authored by Hordur Freyr Yngvason's avatar Hordur Freyr Yngvason 🌡️ Committed by Thong Kuah
Browse files

fix: Initialize DB on stable track 

In gitlab-org/gitlab#30465 we found that
non-stable releasese, e.g. canary and incremental rollouts, were not
working as intended when DB_INITIALIZE was set.

This was due to `helm upgrade` "failing" if no resources will be
created, see https://github.com/helm/helm/issues/4670. This affected us
because in the DB_INITIALIZE case, we do not create any deployments nor
do we activate any services.

The solution proposed in this commit is to always run the database
initialization step on the stable track, with services and postgres
configured accordingly. The fact that initializeCommand is set should be
enough to prevent deployments (this is how the chart works), but for
additional safety we set replicas to 0.
parent 403789c3
Show whitespace changes
Inline Side-by-side
  • Tracey Jaquith 💥 @traceypooh ·

    Hi @tkuah

    OK I finally tracked down the problem.

    In a nutshell, when postgres is disabled at the GL Auto DevOps chart level (when one has built their own copy of the chart) it needs to disable the requirements for postgresql chart's persistence.enabled in its values.yml

    https://github.com/helm/charts/blob/b90ad657e1a226eb52c3eb6a2a95ba3d6d494f58/stable/postgresql/values.yaml#L31

    We've made our own chart copy via:

    https://docs.gitlab.com/ee/topics/autodevops/#custom-helm-chart

    with only minimal changes to it. There might be some other way to pass a CI/CD variable (kinda doubt it). I'm rather new to helm so don't have a concrete suggestion of which place to suggest a patch to.

    But when my chart auto-deploy-app.tgz is made w/ the line 31 (above) switched to false, it finally got rid of the unwanted postgres pod & DB & PVC/PV.

    This was really kicking our butts because the main monolith repo for archive.org was using this - and I have ~20 active devs in it, pushing up ~10 or more branches and review apps/day.

    Couple that w/ our "everything on-prem" approach, and some not-always-rock-solid CEPH + ROOK occasional issues (and rare RAM => OOM issues) it was really starting to effect us (thankfully we merge reasonably quickly, but right now I got 31 branches/pods w/ spurious PV/PVC).

    Not being a 🦀 - just figured some color/stats from a sizable "all on prem" setup might be useful / interesting. Happy NY!

    Edited by Tracey Jaquith
  • Thong Kuah @tkuah ·
    Developer

    @traceypooh

    OK I finally tracked down the problem.

    Nice! :)

    In a nutshell, when postgres is disabled at the GL Auto DevOps chart level (when one has built their own copy of the chart) it needs to disable the requirements for postgresql chart's persistence.enabled in its values.yml

    Hmm, Based on https://docs.gitlab.com/ee/topics/autodevops/#using-external-postgresql-database-providers, setting POSTGRES_ENABLED to false should disable the whole postgresql chart.

    But FWIW, I'm helm making my own modified chart and notice that the old one untar-ed has requirements.yaml and Chart.yaml in top-level - and the new one doesn't have requirements.yaml but has them co-mingled. I figure that's probably as intended, but just in case, thought I'd mention.

    Hmm, if I untar the official chart from https://gitlab-charts.s3.amazonaws.com/auto-deploy-app-0.4.1.tgz, I see the following:

    tkgl:~ tkuah$ cat auto-deploy-app/requirements.yaml
dependencies:
  - name: postgresql
    version: "0.7.1"
    repository: "https://kubernetes-charts.storage.googleapis.com/"
    condition: postgresql.enabled

tkgl:~ tkuah$ cat auto-deploy-app/Chart.yaml 
apiVersion: v1
description: GitLab's Auto-deploy Helm Chart
icon: https://gitlab.com/gitlab-com/gitlab-artwork/raw/master/logo/logo-square.png
name: auto-deploy-app
version: 0.4.1

    So the lack of requirements.yaml for your tar seems odd. What version of Helm are you using by any change to package up your modified chart ?

  • Tracey Jaquith 💥 @traceypooh ·

    Actually, in between, I ended up putting the std requirements.yaml back in (to no effect either way).

    -rwxr-xr-x  0 0      0         180 Jan  4 22:42 auto-deploy-app/Chart.yaml
-rwxr-xr-x  0 0      0        2278 Jan  4 22:42 auto-deploy-app/values.yaml
-rwxr-xr-x  0 0      0         214 Jan  4 22:42 auto-deploy-app/templates/NOTES.txt
-rwxr-xr-x  0 0      0        1071 Jan  4 22:42 auto-deploy-app/templates/_helpers.tpl
-rwxr-xr-x  0 0      0        1252 Jan  4 22:42 auto-deploy-app/templates/db-initialize-job.yaml
-rwxr-xr-x  0 0      0        1242 Jan  4 22:42 auto-deploy-app/templates/db-migrate-hook.yaml
-rwxr-xr-x  0 0      0        4332 Jan  4 22:42 auto-deploy-app/templates/deployment.yaml
-rwxr-xr-x  0 0      0         647 Jan  4 22:42 auto-deploy-app/templates/hpa.yaml
-rwxr-xr-x  0 0      0        1873 Jan  4 22:42 auto-deploy-app/templates/ingress.yaml
-rwxr-xr-x  0 0      0         395 Jan  4 22:42 auto-deploy-app/templates/network-policy.yaml
-rwxr-xr-x  0 0      0         706 Jan  4 22:42 auto-deploy-app/templates/pdb.yaml
-rwxr-xr-x  0 0      0         388 Jan  4 22:42 auto-deploy-app/templates/postgres-instance.yaml
-rwxr-xr-x  0 0      0         858 Jan  4 22:42 auto-deploy-app/templates/service.yaml
-rwxr-xr-x  0 0      0        3744 Jan  4 22:42 auto-deploy-app/templates/worker-deployment.yaml
-rwxr-xr-x  0 0      0         563 Jan  4 22:42 auto-deploy-app/.gitlab-ci.yml
-rwxr-xr-x  0 0      0         333 Jan  4 22:42 auto-deploy-app/.helmignore
-rwxr-xr-x  0 0      0        1095 Jan  4 22:42 auto-deploy-app/CONTRIBUTING.md
-rwxr-xr-x  0 0      0        1046 Jan  4 22:42 auto-deploy-app/LICENSE
-rwxr-xr-x  0 0      0        7463 Jan  4 22:42 auto-deploy-app/README.md
-rwxr-xr-x  0 0      0         241 Jan  4 22:42 auto-deploy-app/requirements.lock
-rwxr-xr-x  0 0      0         158 Jan  4 22:42 auto-deploy-app/requirements.yaml
-rwxr-xr-x  0 0      0         261 Jan  4 22:42 auto-deploy-app/test/go.mod
-rwxr-xr-x  0 0      0       21695 Jan  4 22:42 auto-deploy-app/test/go.sum
-rwxr-xr-x  0 0      0        9259 Jan  4 22:42 auto-deploy-app/test/template_test.go
-rwxr-xr-x  0 0      0         236 Jan  4 22:42 auto-deploy-app/test/testdata/custom-policy.yaml
-rwxr-xr-x  0 0      0         304 Jan  4 22:42 auto-deploy-app/test/testdata/full-spec-policy.yaml
-rwxr-xr-x  0 0      0         473 Jan  4 22:42 auto-deploy-app/charts/postgresql/Chart.yaml
-rwxr-xr-x  0 0      0        2311 Jan  4 22:42 auto-deploy-app/charts/postgresql/values.yaml
-rwxr-xr-x  0 0      0        1603 Jan  4 22:42 auto-deploy-app/charts/postgresql/templates/NOTES.txt
-rwxr-xr-x  0 0      0         517 Jan  4 22:42 auto-deploy-app/charts/postgresql/templates/_helpers.tpl
-rwxr-xr-x  0 0      0        3356 Jan  4 22:42 auto-deploy-app/charts/postgresql/templates/deployment.yaml
-rwxr-xr-x  0 0      0         743 Jan  4 22:42 auto-deploy-app/charts/postgresql/templates/pvc.yaml
-rwxr-xr-x  0 0      0         595 Jan  4 22:42 auto-deploy-app/charts/postgresql/templates/secrets.yaml
-rwxr-xr-x  0 0      0         671 Jan  4 22:42 auto-deploy-app/charts/postgresql/templates/svc.yaml
-rwxr-xr-x  0 0      0        7507 Jan  4 22:42 auto-deploy-app/charts/postgresql/README.md

    the helm I'm using to helm package (and helm repo index) is v3.0.1

    Edited by Tracey Jaquith
  • Thong Kuah @tkuah ·
    Developer

    the helm I'm using to helm package (and helm repo index) is v3.0.1

    Hmm, reading https://helm.sh/docs/topics/charts/#the-apiversion-field, we may not support Helm v3 yet because https://gitlab.com/gitlab-org/charts/auto-deploy-app/blob/master/Chart.yaml#L1 is still on apiVersion: v1. @traceypooh maybe worth packaging with Helm v2 ?

  • Tracey Jaquith 💥 @traceypooh ·

    OK, I could try that the next time I end up rebuilding the .tgz / helm package.

    Sometimes it's weeks to months before a rebuild (I'm "watching" changes to auto-deploy-app repo and GL CHANGES so something of interest there tend to trigger the rebuild).

    So actually, I can try to unit test this out in a new repo and see if helm v3 -v- v2 makes a difference.

  • Tracey Jaquith 💥 @traceypooh ·

    OK, in complete unit test w/ v3 helm (ie: no changes to helm chart - 100% stock -- simply tgz it up AS IS) and POSTGRES_ENABLED === false (per group k8s CI/CD Variables), the (unwanted) PVC/PV was created.

    As I'm using a mac (ie: brew install helm and/or brew install kubernetes-helm) always a bit more work to downgrade. But should be able to try that, clone repo, and see if doesn't happen w/ helm v2...

    Edited by Tracey Jaquith
  • Thong Kuah @tkuah ·
    Developer

    I am using a mac too. It's relatively straightforward to unzip one of the Helm 2.X releases from https://github.com/helm/helm/releases and use the helm binary from there.

  • Tracey Jaquith 💥 @traceypooh ·

    oh, fantastic, thanks!! 🙏

  • Tracey Jaquith 💥 @traceypooh ·

    Wow that indeed seemed to not make unwanted PV/PVC show up.

    ( https://git.archive.org/www/gltest2 )

    I logically cloned same prior "all stock defaults" repo, but this time downloaded and used helm v2(.16.1) (from mac) to make the two files:

    • auto-deploy-app.tgz
    • index.yaml

    https://git.archive.org/www/gltest2/blob/master/x/README.sh

    (Aside from change pointing to my chart in .gitlab-ci.yml, and in same file, setting POSTGRES_ENABLED .. false (and same CI/CD var, for good measure, in the k8s GL group and the k8s GL repo/project), it's pretty much stock).

    Upon deployment to the same cluster, no PV/PVC -- but the repo I used helm v3 (from mac) to make the helm files made the unwanted PV/PVC for PG.

    So maybe I found a future minor bug/issue :D

  • Thong Kuah @tkuah

    mentioned in issue gitlab-org/charts/auto-deploy-app#44 (moved)

     ·

    mentioned in issue gitlab-org/charts/auto-deploy-app#44 (moved)

    Toggle commit list
  • Thong Kuah @tkuah ·
    Developer

    Thanks @traceypooh - That indeeds confirms my suspicion. I have linked gitlab-org/charts/auto-deploy-app#44 (moved) to your comment above.

  • Shinya Maeda 2️⃣ @shinya.maeda

    mentioned in issue #84

     ·

    mentioned in issue #84

    Toggle commit list
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment