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

Allow several controllers to watch the same resource #283

Open
pshchelo opened this issue Jan 2, 2020 · 5 comments
Open

Allow several controllers to watch the same resource #283

pshchelo opened this issue Jan 2, 2020 · 5 comments
Labels
enhancement New feature or request

Comments

@pshchelo
Copy link
Contributor

pshchelo commented Jan 2, 2020

(this is feature request, so forgive me for skipping the issue template)

Currently it is not possible to watch for the same k8s resource by several controllers - except for on.event spy handler, but events are ephemeral in k8s, so we need a more robust solution.

This is needed mostly to only observe the state of some (cluster-shared) resource.
In our case this is several very stateful applications (each managed by separate kopf controller) that have to take some action on node status / metadata / annotation changes.

The problem boils down to the annotation kopf adds to the resource with the last reacted to state, and the name of this annotation field is hardcoded (kopf.zalando.org/last-handled-configuration).

Technically we already have a run-time / configuration parameter that can distinguish different controllers - which is peering object name (kopf run -P ..)
I propose to add this peering object name as a prefix to the annotation key that kopf adds (in case it is not 'default') like <peering-name>.kopf.zalando.org/last-handled-configuration. In this way several sets of different controllers (each using different peering object, which they almost anyway should do) will be able to watch the same resource without stepping on each other toes.

This however might pose some problem on upgrade, so if such breaking change is to be avoided, we could add a separate CLI option to set this prefix name, or a CLI flag that will tell kopf to use peering object name as prefix for annotation field name.

@Jc2k
Copy link
Contributor

Jc2k commented Jan 6, 2020

I would find this useful too. Right now in one of my controllers i monkey-patch kopf.structs.lastseen.LAST_SEEN_ANNOTATION . This works but is obviously brittle.

I would quite like to be able to set this in code rather than adding additonal CLI flags though.

@pshchelo
Copy link
Contributor Author

pshchelo commented Jan 6, 2020

@Jc2k IMO this still better be some 'external' thing (like CLI flag or env var), otherwise you can't run the same code for several instances of controller.
Anyway I prepped a PR that implements the option 3 (CLI flag to re-use -P <peering name> as prefix), will propose this week.

@nolar
Copy link
Contributor

nolar commented Jan 7, 2020

Related: #23

@nolar nolar added the enhancement New feature or request label Jan 7, 2020
@nolar
Copy link
Contributor

nolar commented Jan 7, 2020

There will be another issue beside the annotations. Kopf stores its handlers' statuses in .status.kopf.progress, and purges the whole section once done regardless of the content. If multiple operators handle the same object at the same time, their handlers could collide (with the same id or function names), and they will be purging each other's handling progress.

So, the suffix/prefix should be also applied to the status sections of Kopf. E.g., by storing it in .status.kopf-{id}.progress.

As a little addition, it could be a good idea to move the last-handled-state from metadata.annotations to the same status.kopf-{id} structure, e.g. status.kopf-{id}.last-handled-state — so that all operators have their own value of the last-handled-state (which totally makes sense).

Beside, having this huge JSON structure in the annotations is sometimes annoying to see (originally, it was done so to mimic kubectl's last-applied-configuration, which is also used for diff computation by kubectl — basically, for the same purpose as in Kopf).

Both in case of the status and the annotation, the old names should be supported at least for reading, so that the change is not breaking. Writing back to those old places can be done if it is easy to implement — to support the non-breaking roll-back of Kopf's versions (but only if the old place exists there; ignore if it is non-existent).

pshchelo added a commit to pshchelo/kopf-old that referenced this issue Jan 7, 2020
this patch adds -A / --annotate-with-peering flag to `kopf run` command,
also defaulting to KOPF_RUN_ANNOTATE_WITH_PEERING env var.
When set, the key name for annotation used by kopf to store state
is prepended with peering object name used by this controller(s).

In this wau several sets of kopf-based controllers, each using their own
peering object name, can watch the same resource, storing their state
separately.

Issue zalando-incubator#283
@klarose
Copy link

klarose commented Feb 6, 2020

I wouldn't mind having this as well. I have controllers that can manage conflicts internally, but I want to use kopf to watch for updates/etc since it's much more elegant than using the k8s client directly.

Alternatively, if I could just hint to a handler that I didn't want it to annotate the object, but still have it break out the raw k8s event type (update, delete, create), it'd still be nice.

I notice that @pshchelo has a some changes for this. Are they ready for a PR?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants