helm-charts icon indicating copy to clipboard operation
helm-charts copied to clipboard

Add PostgreSQL schema jobs

Open dkravetz opened this issue 2 years ago • 5 comments

What was changed

Add kubernetes job to support database schema migration and updates on PostgreSQL. Also updated values.postgresql.yaml to reflect this

Why?

Current workplace uses a Bring Your Own PostgreSQL and having this change upstream would be simple for more people.

Checklist

  1. Closes #349

  2. How was this tested: Set up a brand new PostgreSQL; provide hostname, username, and password, and set enable

schema:
  setup:
    enabled: true
  update:
    enabled: true
  1. Any docs updates needed? No

dkravetz avatar Jan 06 '23 14:01 dkravetz

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jan 06 '23 14:01 CLAassistant

I have been working with this feature

But I had to fix some bugs

  • The PORT as an env should be quoted.
  • The cassandra Job should be disabled when the driver is postgres.
  • Update job should wait for create job .. so I passed extra arg to pg_isready to verify the DB (--dbname).
  • DB names were hardcoded but they should be retrieved from the values.
  • used temporal.persistence.schema in _helpers.tpl to set schema directory name.
  • removed the replace '_' by '-' in the DB name when updating schema.

I made a branch with this feature including my changes https://github.com/medamineamara/Temporal-helm-charts/tree/fix-postgresql-schema

This commit contains all the listed changes https://github.com/medamineamara/Temporal-helm-charts/commit/1f9863778d53ff70fa350cbec4d717300378cf62

Should I open a new Pull Request ?

medamineamara avatar Apr 04 '23 13:04 medamineamara

@medamineamara can you update your branch? it's a bit behind master and open a PR please? thank you so much for this!

brandonros avatar May 16 '23 03:05 brandonros

this should probably be closed in favor https://github.com/temporalio/helm-charts/pull/281 i'm guessing

brandonros avatar Jun 12 '23 19:06 brandonros

Think this one can be closed now, or?

Was about to start similar work myself but no point in case there is already open pull request. The referenced issue #281 has been closed in favour of #405 which is merged but there is no documentation updates making it a little bit tricky and not sure it actually works for our use case where we use the gcp auth proxy sidecar.

peter-c-larsson avatar Aug 30 '23 10:08 peter-c-larsson

Resolved via #405

robholland avatar Jun 08 '24 12:06 robholland