Merge pull request #88 from nolar/87-events-optional

Suppress errors in event posting, and prevent them when possible

kopf/k8s/events.py

@@ -1,6 +1,12 @@
 import datetime
+import logging
 
-import kubernetes
+import kubernetes.client.rest
+
+logger = logging.getLogger(__name__)
+
+MAX_MESSAGE_LENGTH = 1024
+CUT_MESSAGE_INFIX = '...'
 
 
 def post_event(*, obj, type, reason, message=''):
@@ -11,6 +17,13 @@ def post_event(*, obj, type, reason, message=''):
     now = datetime.datetime.utcnow()
     namespace = obj['metadata']['namespace']
 
+    # Prevent a common case of event posting errors but shortening the message.
+    if len(message) > MAX_MESSAGE_LENGTH:
+        infix = CUT_MESSAGE_INFIX
+        prefix = message[:MAX_MESSAGE_LENGTH // 2 - (len(infix) // 2)]
+        suffix = message[-MAX_MESSAGE_LENGTH // 2 + (len(infix) - len(infix) // 2):]
+        message = f'{prefix}{infix}{suffix}'
+
     # Object reference - similar to the owner reference, but different.
     ref = dict(
         apiVersion=obj['apiVersion'],
@@ -43,8 +56,15 @@ def post_event(*, obj, type, reason, message=''):
         event_time=now.isoformat() + 'Z',  # '2019-01-28T18:25:03.000000Z'
     )
 
-    api = kubernetes.client.CoreV1Api()
-    api.create_namespaced_event(
-        namespace=namespace,
-        body=body,
-    )
+    try:
+        api = kubernetes.client.CoreV1Api()
+        api.create_namespaced_event(
+            namespace=namespace,
+            body=body,
+        )
+    except kubernetes.client.rest.ApiException as e:
+        # Events are helpful but auxiliary, they should not fail the handling cycle.
+        # Yet we want to notice that something went wrong (in logs).
+        logger.warning("Failed to post an event. Ignoring and continuing. "
+                       f"Error: {e!r}. "
+                       f"Event: type={type!r}, reason={reason!r}, message={message!r}.")

tests/k8s/conftest.py

@@ -1,4 +1,8 @@
 import pytest
+from kubernetes.client import V1Event as V1Event
+from kubernetes.client import V1EventSource as V1EventSource
+from kubernetes.client import V1ObjectMeta as V1ObjectMeta
+from kubernetes.client import V1beta1Event as V1beta1Event
 from kubernetes.client.rest import ApiException  # to avoid mocking it
 
 
@@ -8,4 +12,8 @@
 def client_mock(mocker):
     client_mock = mocker.patch('kubernetes.client')
     client_mock.rest.ApiException = ApiException  # to be raises and caught
+    client_mock.V1Event = V1Event
+    client_mock.V1beta1Event = V1beta1Event
+    client_mock.V1EventSource = V1EventSource
+    client_mock.V1ObjectMeta = V1ObjectMeta
     return client_mock

tests/k8s/test_events.py

@@ -1,18 +1,10 @@
+import pytest
 from asynctest import call, ANY
-from kubernetes.client import V1Event as V1Event_orig
-from kubernetes.client import V1EventSource as V1EventSource_orig
-from kubernetes.client import V1ObjectMeta as V1ObjectMeta_orig
-from kubernetes.client import V1beta1Event as V1beta1Event_orig
 
 from kopf.k8s.events import post_event
 
 
 def test_posting(client_mock):
-    client_mock.V1Event = V1Event_orig
-    client_mock.V1beta1Event = V1beta1Event_orig
-    client_mock.V1EventSource = V1EventSource_orig
-    client_mock.V1ObjectMeta = V1ObjectMeta_orig
-
     result = object()
     apicls_mock = client_mock.CoreV1Api
     apicls_mock.return_value.create_namespaced_event.return_value = result
@@ -32,7 +24,7 @@ def test_posting(client_mock):
         body=ANY,
     )]
 
-    event: V1Event_orig = postfn_mock.call_args_list[0][1]['body']
+    event = postfn_mock.call_args_list[0][1]['body']
     assert event.type == 'type'
     assert event.reason == 'reason'
     assert event.message == 'message'
@@ -44,11 +36,26 @@ def test_posting(client_mock):
     assert event.involved_object['uid'] == 'uid'
 
 
-def test_type_is_v1_not_v1beta1(client_mock, resource):
-    client_mock.V1Event = V1Event_orig
-    client_mock.V1beta1Event = V1beta1Event_orig
+def test_type_is_v1_not_v1beta1(client_mock):
+    apicls_mock = client_mock.CoreV1Api
+    postfn_mock = apicls_mock.return_value.create_namespaced_event
+
+    obj = {'apiVersion': 'group/version',
+           'kind': 'kind',
+           'metadata': {'namespace': 'ns',
+                        'name': 'name',
+                        'uid': 'uid'}}
+    post_event(obj=obj, type='type', reason='reason', message='message')
+
+    event = postfn_mock.call_args_list[0][1]['body']
+    assert isinstance(event, client_mock.V1Event)
+    assert not isinstance(event, client_mock.V1beta1Event)
+
 
+def test_api_errors_logged_but_suppressed(client_mock, assert_logs):
+    error = client_mock.rest.ApiException('boo!')
     apicls_mock = client_mock.CoreV1Api
+    apicls_mock.return_value.create_namespaced_event.side_effect = error
     postfn_mock = apicls_mock.return_value.create_namespaced_event
 
     obj = {'apiVersion': 'group/version',
@@ -58,6 +65,45 @@ def test_type_is_v1_not_v1beta1(client_mock, resource):
                         'uid': 'uid'}}
     post_event(obj=obj, type='type', reason='reason', message='message')
 
+    assert postfn_mock.called
+    assert_logs([
+        "Failed to post an event.*boo!",
+    ])
+
+
+def test_regular_errors_escalate(client_mock):
+    error = Exception('boo!')
+    apicls_mock = client_mock.CoreV1Api
+    apicls_mock.return_value.create_namespaced_event.side_effect = error
+
+    obj = {'apiVersion': 'group/version',
+           'kind': 'kind',
+           'metadata': {'namespace': 'ns',
+                        'name': 'name',
+                        'uid': 'uid'}}
+
+    with pytest.raises(Exception) as excinfo:
+        post_event(obj=obj, type='type', reason='reason', message='message')
+
+    assert excinfo.value is error
+
+
+def test_message_is_cut_to_max_length(client_mock):
+    result = object()
+    apicls_mock = client_mock.CoreV1Api
+    apicls_mock.return_value.create_namespaced_event.return_value = result
+    postfn_mock = apicls_mock.return_value.create_namespaced_event
+
+    obj = {'apiVersion': 'group/version',
+           'kind': 'kind',
+           'metadata': {'namespace': 'ns',
+                        'name': 'name',
+                        'uid': 'uid'}}
+    message = 'start' + ('x' * 2048) + 'end'
+    post_event(obj=obj, type='type', reason='reason', message=message)
+
     event = postfn_mock.call_args_list[0][1]['body']
-    assert isinstance(event, V1Event_orig)
-    assert not isinstance(event, V1beta1Event_orig)
+    assert len(event.message) <= 1024  # max supported API message length
+    assert '...' in event.message
+    assert event.message.startswith('start')
+    assert event.message.endswith('end')