Configurable/optional finalizers

[kopf-archiver]

An issue by nolar at 2019-04-06 07:10:32+00:00
Original URL: zalando-incubator/kopf#24
 

Actual Behavior

Finalizers are marks that are stored as a list of arbitrary strings in metadata.finalizers. When the object is requested to be deleted, its deletion timestamp is set only. Until such marks exist on the object, it will not be deleted, and the deletion request will wait until the marks are removed by the operators/controllers (which should react to the deletion timestamp appearance). Only when all the finalizers are removed, the object is actually deleted.

Currently, when a resource is handled by the operator, the finalizers are always added on the object on its first appearance (before the handlers), and remove when it is marked for deletion (after the handlers).

If such an operator is stopped, the objects cannot be deleted, and the deletion command freezes — while there is no actual need to wait and to notify the operator (it will do nothing).

Expected Behavior

The finalisers should be optional. If there are no deletion handlers, the finalizers are not needed. If there are deletion handlers, the finalizers should be added.

Some deletion handlers can be explicitly marked as optional, thus ignored for the decision whether the finalizers are needed. The default assumption is that if the deletion handler exists, it is required.

@kopf.on.delete('', 'v1', 'pods', optional=True)
def pod_deleted(**_):
    pass

Two special cases:

• If the object was created when there were no deletion handlers, and the finalizers were not added, but then a new operator version is started with the deletion handlers — the finalizers must be auto-added.
• If the object was created when there were some deletion handlers, and the finalizers were added, but then a new operator version is started with no deletion handlers — the finalizers must be auto-removed.

---

Commented by dlmiddlecote at 2019-06-02 20:06:52+00:00
 

Hey nolar

I’d like to have a go at this issue, I should have some time over the coming week.

I have a preliminary question, if that’s ok.
In kopf.reactor.causation.detect_cause the finalizers are used to check whether the event is NEW or not. I think by not adding the finalizers sometimes we will break that check, and so we’ll probably need some other way to do the check.

Do you have any ideas what we could do? Or do you think things will still work fine?

---

Commented by nolar at 2019-06-04 07:24:53+00:00
 

dlmiddlecote Okay, I will find a way to assign it to you (it didn't work that easily as I thought).

Regarding the solution, there is one important criterion: The finalizers are mandatory if the operator contains the deletion handlers, and they are not optional. The reasons are simple: if the finalizers are absent, then

(1) the deletion handlers can be called when the object is already gone, thus breaking the handler's logic (e.g. 404 errors), and the after-handling patching (also 404);

(2) the deletion handlers are not guaranteed to execute — the finalizers block the object until all the handlers have succeeded (unlike the @kopf.on.event handlers, which are executed as "now or never").

I see two ways of solving it:

A: Signal to detect_cause() that you are fine with no NEW cause, and then detect it as CREATE — naturally happens if if ...: return NEW block is skipped. The signal comes based on registry.has_mandatory_deletion_handlers() or registry.has_blocking_handlers() (to be made; the names are just for example), similar to registry.has_cause_handlers() (exists and used) — somewhere in kopf.reactor.handling.custom_object_handler().

B: Always detect the NEW cause as it does now, but then, depending on registry.has_deletion_handlers(), treat it as CREATE. This implies that the NEW causes will trigger the creation handlers.

To my personal belief, the way A is more clean, and easier to test (there are no different reactions to the same cause depending on external factors, as the cause already represents those external factors (conceptually)).

In both cases, the unit-tests must be updated accordingly.

---

Commented by nolar at 2019-06-04 13:30:50+00:00
 

dlmiddlecote Okay, I've assigned it to myself to just have it assigned (not free). Consider this one as assigned to you — until I find a way to do this properly. Seems this feature is absent in GitHub.

---

Commented by dlmiddlecote at 2019-07-04 21:41:46+00:00
 

nolar - should this be closed?