Add an ability to remove secrets and configmaps when database is gone #168
Conversation
controllers/database_controller.go
Outdated
| if err != nil { | ||
| return r.manageError(ctx, dbcr, err, true) | ||
| } else if dbSecret != nil && dbcr.Spec.Cleanup { | ||
| logrus.Infof("Setting up OwnerReference %v", ownership) | ||
| dbSecret.SetOwnerReferences(ownership) |
There was a problem hiding this comment.
why not define the ownership at creation in the related function?
There was a problem hiding this comment.
I've tried to add it to SecretBuilder and ConfigmapBuilder, but they are called from places where there is no context that is required for creating an owner reference. I could add it to the main reconcile function, pass it through a big chain of functions, add conditions everywhere, or just put it here. It may be not the best way of doing this, but otherwise, a lot of code should be restructured, and I'm not sure if it should be done within this PR.
For example, fn SecretBuilder() is called from every function that modifies templateSecrets and connectionString that are called from the main templateSecrets creator function that is moved to database_helper.go. So it was not really obvious where to put the ownership stuff.
There was a problem hiding this comment.
Would mostly supply the ownership as argument to the function like createInstanceAccessSecret, and apply there the SetOwnerReferences function.
and for the ownership variable definition at 191, think you could only make there the if condition if the ownership is an empty struct or filled with the reference to the db-operator.
But my assumption is also, that an empty ownership struct, should not really do anything at SetOwnerReferences. But maybe I'm wrong.
Another question for me, issue is only about secrets and cm. But we also create the proxy deployment and cronjob. Should we also include this here, or a separate issue.
There was a problem hiding this comment.
Sounds good, I didn't think about that. I'll try.
I think other resources should be removed too, but I just forgot about them. So I'll do that
allanger
force-pushed
the
163-remove-resources-created-by-db
branch
from
d110d7c to
3ec4945
Compare
After database resource is removed, there are still some "leftovers". If ownership is set to database resource explicitly, those "leftovers" will be removed with a database.
But since we've faced some unexpected removing of needed secrets in a past, the "cleanup" should be optional.
Close #163