Add finalizers to ObjectMetaBuilder #1094
Conversation
| } | ||
|
|
||
| /// This will replace all existing finalizers | ||
| pub fn finalizers(&mut self, finalizers: Vec<String>) -> &mut Self { |
There was a problem hiding this comment.
Maybe it should be called replace_finalizers. Could be easy to misuse otherwise.
Also, I think (not 100% sure) finalizers might be a free-for-all (anything with permission can add a finalizer to stop something being deleted too early). SO maybe it isn't a good idea to allow replacing them. You should only be allowed to add or remove "your" finalizer.
There was a problem hiding this comment.
Ideally each entity (for lack of a better name... think operator/controller) should have a const for it's finalizer name (its stamp that it puts on things that it depends on being alive until it does some process, then it removes the stamp and the k8s reconciler continues).
There was a problem hiding this comment.
That makes sense, but these are mostly pure builders, and its consistent with other stuff. You could argue anyone could add / remove labels as well?
There was a problem hiding this comment.
I'd say only the admin and the owning-operators should change labels.
Finalizers are for a different reason which is to prevent the object from disappearing when things unknown to the owning-operator need to do things.
One example is Ingress... The resource is an internal k8s Resource, but three can be various operators/controllers that can "do things".
For example, An ingress controller creating a cloud load balancer for Ingresses of an applicable ingressClass - it will add a finalizer (to the Ingress) so that when an admin deletes the Ingress resource, it has to wait until the cloud load balancer has finished being deleted.
If K8s just removes the resource while the controller managing the Load Balancer restarts, when it starts up it will not know about the deleted Ingress and therefore won't know that it needed to delete the Load Balancer (which has cost implications and minimum).
There was a problem hiding this comment.
Im not sure i get this.
This is a framework, among other things, i can create / manipulate k8s resources. If i wanna reset labels, annotations or finalizers i should be able to do so. If this kills a whole cluster, then i would not say this was the frameworks mistake? This probably should have been prevented via rbac permissions?
Its a little bit like the bike and the pipe?
There was a problem hiding this comment.
What I mean is, imagine someone has an operator that does "something" (we don't know what it does)... and it wants to do "something" when a Pod with specific labels is deleted, but it needs to block the deletion until it has finished its work...
We shouldn't be allowed to clear/replace the finalizer list just because we want all of our stuff gone, because we cannot predict what other finalizers are there and why.
Now that example was for Pods... But that same mysterious operator might want to do things on deletion of a NifiCluster, or KafkaCluster for example. It's not up to us to say "no you can't" because we cannot predict the case where it might be a good idea, or even necessary.
So, finalizers that are not our own (the operator's own) should be respected and left alone.
3 participants
Description
This adds finalizers to ObjectMetaBuilder.
Definition of Done Checklist
Author
Reviewer
Acceptance