diff --git a/kopf/reactor/causation.py b/kopf/reactor/causation.py index f928e4e0..8c350572 100644 --- a/kopf/reactor/causation.py +++ b/kopf/reactor/causation.py @@ -51,8 +51,6 @@ class Reason(str, enum.Enum): NOOP = 'noop' FREE = 'free' GONE = 'gone' - ACQUIRE = 'acquire' - RELEASE = 'release' def __str__(self) -> str: return str(self.value) @@ -70,8 +68,6 @@ def __str__(self) -> str: Reason.NOOP, Reason.FREE, Reason.GONE, - Reason.ACQUIRE, - Reason.RELEASE, ) ALL_REASONS = HANDLER_REASONS + REACTOR_REASONS @@ -153,7 +149,6 @@ def detect_resource_changing_cause( *, event: bodies.Event, diff: Optional[diffs.Diff] = None, - requires_finalizer: bool = True, initial: bool = False, **kwargs: Any, ) -> ResourceChangingCause: @@ -183,19 +178,6 @@ def detect_resource_changing_cause( if finalizers.is_deleted(body): return ResourceChangingCause(reason=Reason.DELETE, **kwargs) - # For a fresh new object, first block it from accidental deletions without our permission. - # The actual handler will be called on the next call. - # Only return this cause if the resource requires finalizers to be added. - if requires_finalizer and not finalizers.has_finalizers(body): - return ResourceChangingCause(reason=Reason.ACQUIRE, **kwargs) - - # Check whether or not the resource has finalizers, but doesn't require them. If this is - # the case, then a resource may not be able to be deleted completely as finalizers may - # not be removed by the operator under normal operation. We remove the finalizers first, - # and any handler that should be called will be done on the next call. - if not requires_finalizer and finalizers.has_finalizers(body): - return ResourceChangingCause(reason=Reason.RELEASE, **kwargs) - # For an object seen for the first time (i.e. just-created), call the creation handlers, # then mark the state as if it was seen when the creation has finished. # Creation never mixes with resuming, even if an object is detected on startup (first listing). diff --git a/kopf/reactor/handling.py b/kopf/reactor/handling.py index 6b163d4c..9b1fb36c 100644 --- a/kopf/reactor/handling.py +++ b/kopf/reactor/handling.py @@ -208,7 +208,6 @@ async def resource_handler( diff=diff, memo=memory.user_data, initial=memory.noticed_by_listing and not memory.fully_handled_once, - requires_finalizer=registry.requires_finalizer(resource=resource, body=body), ) delay = await handle_resource_changing_cause( lifecycle=lifecycle, @@ -293,6 +292,19 @@ async def handle_resource_changing_cause( done = None skip = None + requires_finalizer = registry.requires_finalizer(resource=cause.resource, body=cause.body) + has_finalizer = finalizers.has_finalizers(body=cause.body) + + if requires_finalizer and not has_finalizer: + logger.debug("Adding the finalizer, thus preventing the actual deletion.") + finalizers.append_finalizers(body=body, patch=patch) + return None + + if not requires_finalizer and has_finalizer: + logger.debug("Removing the finalizer, as there are no handlers requiring it.") + finalizers.remove_finalizers(body=body, patch=patch) + return None + # Regular causes invoke the handlers. if cause.reason in causation.HANDLER_REASONS: title = causation.TITLES.get(cause.reason, repr(cause.reason)) @@ -344,21 +356,6 @@ async def handle_resource_changing_cause( if cause.reason == causation.Reason.NOOP: logger.debug("Something has changed, but we are not interested (the essence is the same).") - # For the case of a newly created object, or one that doesn't have the correct - # finalizers, lock it to this operator. Not all newly created objects will - # produce an 'ACQUIRE' causation event. This only happens when there are - # mandatory deletion handlers registered for the given object, i.e. if finalizers - # are required. - if cause.reason == causation.Reason.ACQUIRE: - logger.debug("Adding the finalizer, thus preventing the actual deletion.") - finalizers.append_finalizers(body=body, patch=patch) - - # Remove finalizers from an object, since the object currently has finalizers, but - # shouldn't, thus releasing the locking of the object to this operator. - if cause.reason == causation.Reason.RELEASE: - logger.debug("Removing the finalizer, as there are no handlers requiring it.") - finalizers.remove_finalizers(body=body, patch=patch) - # The delay is then consumed by the main handling routine (in different ways). return delay diff --git a/tests/causation/test_detection.py b/tests/causation/test_detection.py index d9a687db..1805d026 100644 --- a/tests/causation/test_detection.py +++ b/tests/causation/test_detection.py @@ -142,7 +142,6 @@ def test_for_gone(kwargs, event, finalizers, deletion_ts, requires_finalizer): event['object']['metadata'].update(deletion_ts) cause = detect_resource_changing_cause( event=event, - requires_finalizer=requires_finalizer, **kwargs) assert cause.reason == Reason.GONE check_kwargs(cause, kwargs) @@ -158,7 +157,6 @@ def test_for_free(kwargs, event, finalizers, deletion_ts, requires_finalizer): event['object']['metadata'].update(deletion_ts) cause = detect_resource_changing_cause( event=event, - requires_finalizer=requires_finalizer, **kwargs) assert cause.reason == Reason.FREE check_kwargs(cause, kwargs) @@ -174,44 +172,11 @@ def test_for_delete(kwargs, event, finalizers, deletion_ts, requires_finalizer): event['object']['metadata'].update(deletion_ts) cause = detect_resource_changing_cause( event=event, - requires_finalizer=requires_finalizer, **kwargs) assert cause.reason == Reason.DELETE check_kwargs(cause, kwargs) -@requires_finalizer -@no_finalizers -@no_deletions -@regular_events -def test_for_acquire(kwargs, event, finalizers, deletion_ts, requires_finalizer): - event = {'type': event, 'object': {'metadata': {}}} - event['object']['metadata'].update(finalizers) - event['object']['metadata'].update(deletion_ts) - cause = detect_resource_changing_cause( - event=event, - requires_finalizer=requires_finalizer, - **kwargs) - assert cause.reason == Reason.ACQUIRE - check_kwargs(cause, kwargs) - - -@doesnt_require_finalizer -@our_finalizers -@no_deletions -@regular_events -def test_for_release(kwargs, event, finalizers, deletion_ts, requires_finalizer): - event = {'type': event, 'object': {'metadata': {}}} - event['object']['metadata'].update(finalizers) - event['object']['metadata'].update(deletion_ts) - cause = detect_resource_changing_cause( - event=event, - requires_finalizer=requires_finalizer, - **kwargs) - assert cause.reason == Reason.RELEASE - check_kwargs(cause, kwargs) - - @requires_finalizer @absent_lastseen @our_finalizers @@ -225,7 +190,6 @@ def test_for_create(kwargs, event, finalizers, deletion_ts, annotations, content event['object']['metadata'].update(annotations) cause = detect_resource_changing_cause( event=event, - requires_finalizer=requires_finalizer, **kwargs) assert cause.reason == Reason.CREATE check_kwargs(cause, kwargs) @@ -241,7 +205,6 @@ def test_for_create_skip_acquire(kwargs, event, finalizers, deletion_ts, require event['object']['metadata'].update(deletion_ts) cause = detect_resource_changing_cause( event=event, - requires_finalizer=requires_finalizer, **kwargs) assert cause.reason == Reason.CREATE check_kwargs(cause, kwargs) @@ -260,7 +223,6 @@ def test_for_no_op(kwargs, event, finalizers, deletion_ts, annotations, content, event['object']['metadata'].update(annotations) cause = detect_resource_changing_cause( event=event, - requires_finalizer=requires_finalizer, **kwargs) assert cause.reason == Reason.NOOP check_kwargs(cause, kwargs) @@ -279,7 +241,6 @@ def test_for_update(kwargs, event, finalizers, deletion_ts, annotations, content event['object']['metadata'].update(annotations) cause = detect_resource_changing_cause( event=event, - requires_finalizer=requires_finalizer, diff=True, **kwargs) assert cause.reason == Reason.UPDATE diff --git a/tests/handling/conftest.py b/tests/handling/conftest.py index 3a4bfd7a..8285b9e0 100644 --- a/tests/handling/conftest.py +++ b/tests/handling/conftest.py @@ -210,6 +210,7 @@ def new_detect_fn(**kwargs): body.setdefault('metadata', {}).setdefault('namespace', 'some-namespace') body.setdefault('metadata', {}).setdefault('name', 'some-name') body.setdefault('metadata', {}).setdefault('uid', 'some-uid') + body.setdefault('metadata', {}).setdefault('finalizers', ['kopf.zalando.org/KopfFinalizerMarker']) return cause diff --git a/tests/handling/test_cause_handling.py b/tests/handling/test_cause_handling.py index a0923e47..c6ed8b99 100644 --- a/tests/handling/test_cause_handling.py +++ b/tests/handling/test_cause_handling.py @@ -13,44 +13,6 @@ EVENT_TYPES = [None, 'ADDED', 'MODIFIED', 'DELETED'] -@pytest.mark.parametrize('event_type', EVENT_TYPES) -async def test_acquire(registry, handlers, resource, cause_mock, event_type, - caplog, assert_logs, k8s_mocked): - caplog.set_level(logging.DEBUG) - - cause_mock.reason = Reason.ACQUIRE - - event_queue = asyncio.Queue() - await resource_handler( - lifecycle=kopf.lifecycles.all_at_once, - registry=registry, - resource=resource, - memories=ResourceMemories(), - event={'type': event_type, 'object': cause_mock.body}, - replenished=asyncio.Event(), - event_queue=event_queue, - ) - - assert not handlers.create_mock.called - assert not handlers.update_mock.called - assert not handlers.delete_mock.called - - assert k8s_mocked.asyncio_sleep.call_count == 0 - assert k8s_mocked.sleep_or_wait.call_count == 0 - assert k8s_mocked.patch_obj.call_count == 1 - assert event_queue.empty() - - patch = k8s_mocked.patch_obj.call_args_list[0][1]['patch'] - assert 'metadata' in patch - assert 'finalizers' in patch['metadata'] - assert FINALIZER in patch['metadata']['finalizers'] - - assert_logs([ - "Adding the finalizer", - "Patching with", - ]) - - @pytest.mark.parametrize('event_type', EVENT_TYPES) async def test_create(registry, handlers, resource, cause_mock, event_type, caplog, assert_logs, k8s_mocked): @@ -172,54 +134,6 @@ async def test_delete(registry, handlers, resource, cause_mock, event_type, ]) -@pytest.mark.parametrize('event_type', EVENT_TYPES) -async def test_release(registry, resource, handlers, cause_mock, event_type, - caplog, k8s_mocked, assert_logs): - caplog.set_level(logging.DEBUG) - cause_mock.reason = Reason.RELEASE - cause_mock.body.setdefault('metadata', {})['finalizers'] = [FINALIZER] - - # register handlers (no deletion handlers) - registry.register_resource_changing_handler( - group=resource.group, - version=resource.version, - plural=resource.plural, - reason=Reason.RESUME, - fn=lambda **_: None, - requires_finalizer=False, - ) - - event_queue = asyncio.Queue() - await resource_handler( - lifecycle=kopf.lifecycles.all_at_once, - registry=registry, - resource=resource, - memories=ResourceMemories(), - event={'type': event_type, 'object': cause_mock.body}, - replenished=asyncio.Event(), - event_queue=event_queue, - ) - - assert not handlers.create_mock.called - assert not handlers.update_mock.called - assert not handlers.delete_mock.called - - assert k8s_mocked.asyncio_sleep.call_count == 0 - assert k8s_mocked.sleep_or_wait.call_count == 0 - assert k8s_mocked.patch_obj.call_count == 1 - assert event_queue.empty() - - patch = k8s_mocked.patch_obj.call_args_list[0][1]['patch'] - assert 'metadata' in patch - assert 'finalizers' in patch['metadata'] - assert [] == patch['metadata']['finalizers'] - - assert_logs([ - "Removing the finalizer", - "Patching with", - ]) - - # # Informational causes: just log, and do nothing else. # diff --git a/tests/handling/test_no_handlers.py b/tests/handling/test_no_handlers.py index 18d90097..4ba90400 100644 --- a/tests/handling/test_no_handlers.py +++ b/tests/handling/test_no_handlers.py @@ -16,6 +16,7 @@ async def test_skipped_with_no_handlers( caplog, assert_logs, k8s_mocked): caplog.set_level(logging.DEBUG) cause_mock.reason = cause_type + cause_mock.body['metadata']['finalizers'] = [] assert not registry.has_resource_changing_handlers(resource=resource) # prerequisite registry.register_resource_changing_handler(