allow force drop of metadata db #19
Conversation
| @@ -104,3 +104,9 @@ variable "postgres_version" { | |||
| type = string | |||
| default = "15" | |||
| } | |||
|
|
|||
| variable "metadata_database_force_recreate" { | |||
There was a problem hiding this comment.
This sounds a bit dangerous, as it will force recreate the database for all instances. Would it be better if we had it on the instances variable level?
There was a problem hiding this comment.
I've prepared a PR for that, let me know what you think:
There was a problem hiding this comment.
@bobbyiliev I was trying to get a that here:
This should be doable with a lifecycle replace triggered by with each.key on the db init, but it seems to crash.
We should not merge this until replacement is figured out.
force_recreate is perhaps misnamed. The goal is to recreate the database if and only if the db_init job needs to be re-run.
I think this is a weird one. I actually wouldn't expect all materialize CRDs to block on all init_db commands. They should only depend on their respective init_db resource. Additionally materialize resources must get fully recreated (not just re-rolled) if their db were to get dropped and recreated.
I do like that your pr makes this configurable per materialize instance. That is totally worth making its way into whatever solution we choose. I'm currently leaning towards randomizing the db_name and leaving the old one as cruft. I think it's got a relatively ignorable downside but is otherwise safer and more flexible.
There was a problem hiding this comment.
Ah yes! This makes sense now! I don't have any strong opinions on which approach to take; main concerns that I would have for each would be:
- With the lifecycle, maybe this will not be very obvious for users, and they might end up accidentally recreating a database
- With the randomized name, users might end up with some orphan dbs without being able to easily pin point which instances they belong to
There was a problem hiding this comment.
I like your pr more than this one. I was trying to keep things simple, but this pr won't handle things like password changes very gracefully. We may also want to do a huddle sometime or a discussion item to the next team hangout. Graceful password rotation is a good item to discuss.
Getting together a list of the behaviors I think we want. I'm not sure we can have all of these with a simple init_db k8s job.
- If the database needs to get recreated we have to recreate the materialize instance
- If the materialize instance is recreated we need to recreate the database (although maybe not when we think about restore?)
- If there are changes like password rotation we can do a create if not exists, but we need to do a rollout
- If there are just job changes it should do a create if not exists and do nothing to the materialize instance
- If a fresh materialize instance is deployed it should fail if the DB already exists (maybe a dupe of 2?).
Allow forced recreation of the metadata database... this must cause forced recreation of the environment CRD
This should be doable with a lifecycle replace triggered by with each.key on the db init, but it seems to crash.
We should not merge this until replacement is figured out.