Skip to content
This repository has been archived by the owner on Sep 14, 2020. It is now read-only.

Fix the never-ending handling cycle #205

Merged
merged 8 commits into from
Oct 22, 2019
9 changes: 7 additions & 2 deletions examples/03-exceptions/example.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,21 @@

import kopf

E2E_TRACEBACKS = True
E2E_CREATE_TIME = 3.5
E2E_SUCCESS_COUNTS = {}
E2E_FAILURE_COUNTS = {'create_fn': 1}


class MyException(Exception):
pass


@kopf.on.create('zalando.org', 'v1', 'kopfexamples')
def create_fn(retry, **kwargs):
time.sleep(2) # for different timestamps of the events
time.sleep(0.1) # for different timestamps of the events
if not retry:
raise kopf.TemporaryError("First failure.", delay=10)
raise kopf.TemporaryError("First failure.", delay=1)
elif retry == 1:
raise MyException("Second failure.")
else:
Expand Down
1 change: 1 addition & 0 deletions examples/08-events/example.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

# Marks for the e2e tests (see tests/e2e/test_examples.py):
E2E_TRACEBACKS = True
E2E_SUCCESS_COUNTS = {'normal_event_fn': 2}


@kopf.on.event('zalando.org', 'v1', 'kopfexamples')
Expand Down
2 changes: 2 additions & 0 deletions examples/99-all-at-once/example.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
# Marks for the e2e tests (see tests/e2e/test_examples.py):
E2E_CREATE_TIME = 5
E2E_DELETE_TIME = 1
E2E_SUCCESS_COUNTS = {'create_1': 1, 'create_2': 1, 'create_pod': 1, 'delete': 1}
E2E_FAILURE_COUNTS = {}
E2E_TRACEBACKS = True

try:
Expand Down
6 changes: 4 additions & 2 deletions kopf/structs/dicts.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import collections.abc
import enum
from typing import (TypeVar, Any, Union, MutableMapping, Mapping, Tuple, List,
Iterable, Iterator, Optional)
Iterable, Iterator, Callable, Optional)

FieldPath = Tuple[str, ...]
FieldSpec = Union[None, str, FieldPath, List[str]]
Expand Down Expand Up @@ -96,14 +96,16 @@ def cherrypick(
src: Mapping[Any, Any],
dst: MutableMapping[Any, Any],
fields: Optional[Iterable[FieldSpec]],
picker: Optional[Callable[[_T], _T]] = None,
) -> None:
"""
Copy all specified fields between dicts (from src to dst).
"""
picker = picker if picker is not None else lambda x: x
fields = fields if fields is not None else []
for field in fields:
try:
ensure(dst, field, resolve(src, field))
ensure(dst, field, picker(resolve(src, field)))
except KeyError:
pass # absent in the source, nothing to merge

Expand Down
4 changes: 2 additions & 2 deletions kopf/structs/lastseen.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def get_state(
dicts.cherrypick(src=body, dst=essence, fields=[
'metadata.labels',
'metadata.annotations', # but not all of them! deleted below.
])
], picker=copy.deepcopy)

# But we do not want not all of the annotations, only potentially useful.
annotations = essence.get('metadata', {}).get('annotations', {})
Expand All @@ -74,7 +74,7 @@ def get_state(
del annotations[annotation]

# Restore all explicitly whitelisted extra-fields from the original body.
dicts.cherrypick(src=body, dst=essence, fields=extra_fields)
dicts.cherrypick(src=body, dst=essence, fields=extra_fields, picker=copy.deepcopy)

# Cleanup the parent structs if they have become empty, for consistent state comparison.
if 'annotations' in essence.get('metadata', {}) and not essence['metadata']['annotations']:
Expand Down
10 changes: 0 additions & 10 deletions tests/cli/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,6 @@ def srcdir(tmpdir):
sys.path.remove(str(tmpdir))


@pytest.fixture(autouse=True)
def clean_default_registry():
registry = kopf.get_default_registry()
kopf.set_default_registry(kopf.GlobalRegistry())
try:
yield
finally:
kopf.set_default_registry(registry)


@pytest.fixture(autouse=True)
def clean_modules_cache():
# Otherwise, the first loaded test-modules remain there forever,
Expand Down
21 changes: 21 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import pytest
import pytest_mock

import kopf
from kopf.config import configure
from kopf.engines.logging import ObjectPrefixingFormatter
from kopf.structs.resources import Resource
Expand Down Expand Up @@ -75,6 +76,26 @@ def resource():
""" The resource used in the tests. Usually mocked, so it does not matter. """
return Resource('zalando.org', 'v1', 'kopfexamples')


#
# Mocks for Kopf's internal but global variables.
#


@pytest.fixture(autouse=True)
def clear_default_registry():
"""
Ensure that the tests have a fresh new global (not re-used) registry.
"""
old_registry = kopf.get_default_registry()
new_registry = kopf.GlobalRegistry()
kopf.set_default_registry(new_registry)
try:
yield new_registry
finally:
kopf.set_default_registry(old_registry)


#
# Mocks for Kubernetes API clients (any of them). Reasons:
# 1. We do not test the clients, we test the layers on top of them,
Expand Down
28 changes: 28 additions & 0 deletions tests/dicts/test_cherrypicking.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import copy

import pytest

from kopf.structs.dicts import cherrypick
Expand Down Expand Up @@ -64,3 +66,29 @@ def test_fails_on_nonmapping_dst_key():
dst = {'sub': 'scalar-value'}
with pytest.raises(TypeError):
cherrypick(src=src, dst=dst, fields=['sub.tested-key'])


def test_exact_object_picked_by_default():
src = {'tested-key': {'key': 'val'}}
dst = {}
cherrypick(src=src, dst=dst, fields=['tested-key'])

assert dst == {'tested-key': {'key': 'val'}}
assert dst['tested-key'] == src['tested-key']
assert dst['tested-key'] is src['tested-key']

src['tested-key']['key'] = 'replaced-val'
assert dst['tested-key']['key'] == 'replaced-val'


def test_copied_object_picked_on_request():
src = {'tested-key': {'key': 'val'}}
dst = {}
cherrypick(src=src, dst=dst, fields=['tested-key'], picker=copy.copy)

assert dst == {'tested-key': {'key': 'val'}}
assert dst['tested-key'] == src['tested-key']
assert dst['tested-key'] is not src['tested-key']

src['tested-key']['key'] = 'another-val'
assert dst['tested-key']['key'] == 'val'
45 changes: 38 additions & 7 deletions tests/e2e/test_examples.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import collections
import re
import shlex
import subprocess
import time

Expand All @@ -13,12 +13,21 @@ def test_all_examples_are_runnable(mocker, with_crd, exampledir):
# If the example has its own opinion on the timing, try to respect it.
# See e.g. /examples/99-all-at-once/example.py.
example_py = exampledir / 'example.py'
m = re.search(r'^E2E_CREATE_TIME\s*=\s*(\d+)$', example_py.read_text(), re.M)
m = re.search(r'^E2E_CREATE_TIME\s*=\s*(.+)$', example_py.read_text(), re.M)
e2e_create_time = eval(m.group(1)) if m else None
m = re.search(r'^E2E_DELETE_TIME\s*=\s*(\d+)$', example_py.read_text(), re.M)
m = re.search(r'^E2E_DELETE_TIME\s*=\s*(.+)$', example_py.read_text(), re.M)
e2e_delete_time = eval(m.group(1)) if m else None
m = re.search(r'^E2E_TRACEBACKS\s*=\s*(\w+)$', example_py.read_text(), re.M)
m = re.search(r'^E2E_TRACEBACKS\s*=\s*(.+)$', example_py.read_text(), re.M)
e2e_tracebacks = eval(m.group(1)) if m else None
m = re.search(r'^E2E_SUCCESS_COUNTS\s*=\s*(.+)$', example_py.read_text(), re.M)
e2e_success_counts = eval(m.group(1)) if m else None
m = re.search(r'^E2E_FAILURE_COUNTS\s*=\s*(.+)$', example_py.read_text(), re.M)
e2e_failure_counts = eval(m.group(1)) if m else None
m = re.search(r'@kopf.on.create\(', example_py.read_text(), re.M)
e2e_test_creation = bool(m)
m = re.search(r'@kopf.on.(create|update|delete)\(', example_py.read_text(), re.M)
e2e_test_highlevel = bool(m)

# check whether there are mandatory deletion handlers or not
m = re.search(r'@kopf\.on\.delete\((\s|.*)?(optional=(\w+))?\)', example_py.read_text(), re.M)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (\s|.*)? seems unnecessarily complicated, as it is equivalent to .* with the re.S flag, creates an unused capturing group, and doesn't match multiple newlines, comments, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not like this approach with the source code parsing with regular expressions at all. Especially when new criteria are added, as in this case. This "examples as e2e tests" approach should be revised to something else, but I don't know to what (yet). Any ideas are welcome. The goal is to test that the examples are still relevant and functional, while keeping in mind that they all are different.

requires_finalizer = False
Expand All @@ -45,17 +54,39 @@ def test_all_examples_are_runnable(mocker, with_crd, exampledir):
subprocess.run("kubectl delete -f examples/obj.yaml", shell=True, check=True)
time.sleep(e2e_delete_time or 1) # give it some time to react

# Ensure that the operator did not die on start, or during the operation.
# Verify that the operator did not die on start, or during the operation.
assert runner.exception is None
assert runner.exit_code == 0

# There are usually more than these messages, but we only check for the certain ones.
# This just shows us that the operator is doing something, it is alive.
if requires_finalizer:
assert '[default/kopf-example-1] Adding the finalizer' in runner.stdout
assert '[default/kopf-example-1] Creation event:' in runner.stdout
if e2e_test_creation:
assert '[default/kopf-example-1] Creation event:' in runner.stdout
if requires_finalizer:
assert '[default/kopf-example-1] Deletion event:' in runner.stdout
assert '[default/kopf-example-1] Deleted, really deleted' in runner.stdout
if e2e_test_highlevel:
assert '[default/kopf-example-1] Deleted, really deleted' in runner.stdout
if not e2e_tracebacks:
assert 'Traceback (most recent call last):' not in runner.stdout

# Verify that once a handler succeeds, it is never re-executed again.
handler_names = re.findall(r"Handler '(.+?)' succeeded", runner.stdout)
if e2e_success_counts is not None:
checked_names = [name for name in handler_names if name in e2e_success_counts]
name_counts = collections.Counter(checked_names)
assert name_counts == e2e_success_counts
else:
name_counts = collections.Counter(handler_names)
assert set(name_counts.values()) == {1}

# Verify that once a handler fails, it is never re-executed again.
handler_names = re.findall(r"Handler '(.+?)' failed permanently", runner.stdout)
if e2e_failure_counts is not None:
checked_names = [name for name in handler_names if name in e2e_failure_counts]
name_counts = collections.Counter(checked_names)
assert name_counts == e2e_failure_counts
else:
name_counts = collections.Counter(handler_names)
assert not name_counts
11 changes: 0 additions & 11 deletions tests/handling/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,6 @@ def k8s_mocked(mocker, req_mock):
)


@pytest.fixture(autouse=True)
def clear_default_registry():
old_registry = kopf.get_default_registry()
new_registry = kopf.GlobalRegistry()
kopf.set_default_registry(new_registry)
try:
yield new_registry
finally:
kopf.set_default_registry(old_registry)


@pytest.fixture()
def registry(clear_default_registry):
return clear_default_registry
Expand Down
14 changes: 0 additions & 14 deletions tests/registries/conftest.py

This file was deleted.