Add PostgreSQL schema jobs

temporalio / helm-charts

Temporal Helm charts

MIT License

312 stars 339 forks source link

Add PostgreSQL schema jobs #350

Closed dkravetz closed 4 months ago

dkravetz commented 1 year ago

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

Closes #349
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

Any docs updates needed? No

CLAassistant commented 1 year ago

All committers have signed the CLA.

medamineamara commented 1 year ago

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 ?

brandonros commented 1 year ago

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

brandonros commented 1 year ago

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

peter-c-larsson commented 1 year ago

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.

robholland commented 4 months ago

Resolved via #405