search

observatorium / thanos-receive-controller

Kubernetes controller to automatically configure Thanos receive hashrings
Apache License 2.0
96 stars 46 forks source link

Set OwnerReferences for generated cm #59

Closed clyang82 closed 3 years ago

clyang82 commented 3 years ago

https://github.com/observatorium/operator/issues/10

clyang82 commented 3 years ago

/assign @squat

bwplotka commented 3 years ago

I would be happy to add test in separate PR 🤗

Maybe premature merge, but I think we can iterate @squat

clyang82 commented 3 years ago

Thanks @bwplotka Let me find time to add test for it.

squat commented 3 years ago

This does not seem like the right way to go and in fact I think this is kind of confusing for the user. The Owner Reference should point to the receive controller, not the owner of the statefulset IMO. If the statefulset is deleted, then the controller should delete the generated configmap but the configmap definitely does not belong to the process that created the statefulset. It is literally created and thus owned by a different controller.

bwplotka commented 3 years ago

Sorry for pre mature merge than

bwplotka commented 3 years ago

https://github.com/observatorium/thanos-receive-controller/pull/60

clyang82 commented 3 years ago

@squat I agree with your point. the configmap is created by the receive controller. so the owner of configmap is the receive controller. Do we have another cases to have different owner for receive controller and statefulset? Do we need to watch the statefulset to delete the configmap if the sts is deleted?

squat commented 3 years ago

yes exactly, we should delete configmaps when sts are deleted. we don't do that today but we absolutely should. that would be how we clean things up.