Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PR] Post k8s-events in the background #125

Closed
2 tasks
kopf-archiver bot opened this issue Aug 18, 2020 · 0 comments
Closed
2 tasks

[PR] Post k8s-events in the background #125

kopf-archiver bot opened this issue Aug 18, 2020 · 0 comments
Labels
archive enhancement New feature or request

Comments

@kopf-archiver
Copy link

kopf-archiver bot commented Aug 18, 2020

A pull request by nolar at 2019-07-01 10:00:21+00:00
Original URL: zalando-incubator/kopf#125
Merged by nolar at 2019-07-03 08:23:44+00:00

Issue : closes #111, prepares #117

Problem

In #109, a lot of performance improvements were introduced to make all API calls asynchronous. One of them were the event-posting API calls, which are kind of logging for custom resource handling routines. However, there was a conceptual unresolvable issue:

Event-posting calls like kopf.event(), kopf.info(), so on — are synchronous functions. When called from an asynchronous coroutines (all of the Kopf internals), there is no way how the flow control can be returned to the asyncio event loop, as it requires await, which is a syntax error for sync-functions. The actual event-posting functions via API calls, however, must be async (explicitly or via thread executors), not sync and blocking.

This problem is nicely described, for example, in this article — https://www.aeracode.org/2018/02/19/python-async-simplified/ — specifically, in "Async From Sync" section. Briefly, the suggested solution is asgiref.sync.async_to_sync from Django, which is basically a trick with multiple threads and cross-thread cross-eventloop task orchestration — an overkill for a little event-posting porblem.

One way would be splitting the event-posting functions to sync and async, and letting the developers to decide which one to use. First of all, it adds complexity and complications. Second, API calls must be duplicated: in sync & async way; if we switch to async k8s client, the sync way will be impossible again.

Summary: There is no easy way how "async->sync->async" call chain can be made in asyncio.

Solution

This PR fixes the async-from-sync invocation dilemma by removing the API calls from the event-posting functions at all:

No async-from-sync calls — no problem.

Instead, those event-posting functions only remember the relevant object references (name, namespace, uid, etc), and put a k8s-event to a queue to be posted later. An ever-running background task posts the k8s-events to the API from the queue.

Side-effects: This can make some events to be slightly delayed. This is okay, with #117 in mind: the events will be not only delayed by time-windows, but also aggregated and maybe altered. Therefore, the concept of guaranteed event posting as part of the function call (and not later) should be abandoned.

Notes

More on that, Kopf is not a K8s API client library, and should not generally provide any means of calling the K8s API. The operator developers should use a K8s client library of their choice to do that.

Event posting is one of such things. Kopf only provides the tools for making some operator-related activities easier. It internally posts the events, so it has these functions, so it shares them with the operator developers.

Look-ahead: In the following PR (#??? — add when created), this whole system will be improved even more to to use the built-in Python logging instead of explicit event posting. The operator developers should use per-object logger since then; though the event-posting will stay.

Types of Changes

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Refactor/improvements

Public interfaces are unchanged. Only the async event-posting routines are removed, but they never got into any released version (i.e. not tagged); only to the master branch 1-2 merges before.

Review

  • Tests
  • Documentation
@kopf-archiver kopf-archiver bot closed this as completed Aug 18, 2020
@kopf-archiver kopf-archiver bot changed the title [archival placeholder] [PR] Post k8s-events in the background Aug 19, 2020
@kopf-archiver kopf-archiver bot added the enhancement New feature or request label Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archive enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

0 participants