Peering scopes #36

nolar · 2019-04-22T01:51:13Z

Issue : #32, related #31, #17

For the purpose of strict namespace isolation of the operators, the peering objects must be cluster- or namespace-scoped (previously: always cluster-scoped).

For this, split the old cluster-wide KopfPeering resource into the new ClusterKopfPeering & namespaced KopfPeering, and use one of them depending on the --namespace option.

The testing of peering is performed manually — it works. More tests will be added when the whole peering subsystem will be covered with tests (as part of #13).

Docs preview:

See also: #37 for the namespace isolation of the listing/watching API calls.

TODO:

Manually test how do the operators react to changes in the kind: KopfPeering when re-created from the cluster to the namespaced scope — in different running modes (with/without --peering, with/without --namespace). Ensure that the operators actually behave as before and as expected.
Fallback to the old CRD KopfPeering if that CRD is cluster-scoped (but not if namespaced).
Document the upgrade scenario to code first, CRDs second, never vice versa.

zincr · 2019-04-22T01:51:24Z

🤖 zincr found 0 problems , 1 warning

ℹ️ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

Details on how to resolve are provided below

Large Commits

Checks all commits for large additions to a single file. Large commits should be reviewed more carefully for potential copyright and licensing issues

This file contains a substantial change, please review to determine if the change comes from an external source and if there are any copyright or licensing issues to be aware of

ℹ️ docs/peering.rst had +100 lines of code changed in a single commit
Please review this file to determine the source of this change
ℹ️ kopf/reactor/peering.py had +100 lines of code changed in a single commit
Please review this file to determine the source of this change

s-soroosh · 2019-04-22T10:46:19Z

kopf/reactor/peering.py

-        name=peering,
-        body={'status': {peer.id: None if peer.is_dead else peer.as_dict() for peer in peers}},
-    )
+    if namespace is None:


what about wrapping this logic in a class? so we can use it later to fetch/patch peering custom objects.
e.g.

PeeringObjects.patch(body, peering, namespace: Optional[String]) PeeringObjects.exists(peering, namespace: Optional[String]) -> bool

pykube-ng does that, more or less. I prefer to not re-invent it or any other Kubernetes client. Instead, it is better to built on top of pykube-ng (see #15). I have a draft branch, but there are few issues both in pykube-ng, and in pykube-to-kopf connectivity — I am going to get back to them later, after I cover the functionality with the tests.

nolar · 2019-05-15T15:30:50Z

So far, as tested with Minikube, the behaviour is this:

Baseline: Old code, old peerings (KopfPeering is cluster-scoped, ClusterKopfPeering does not exist):
- No CLI options: Auto-detection mode, cluster-wide peering was used.
- --namespace=default: Uses the cluster-wide peering.
- --peering=default: Uses the cluster-wide peering.
- --namespace=default --peering=default: Uses the cluster-wide peering.
Mainline: New code, old peerings (KopfPeering is cluster-scoped, ClusterKopfPeering does not exist):
- No CLI options: Auto-switches to standalone mode with a warning (mismatches❗️).
- --namespace=default: Uses the cluster-wide peering (as expected ✅ ).
- --peering=default: Fails with "The peering was not found (mismatches❗️).
- --namespace=default --peering=default: Uses the cluster-wide peering (as expected ✅ ).
Edgecase: Old code, new peerings (KopfPeering is namespaced, ClusterKopfPeering is cluster-scoped).
- Fails with "Namespace parameter required" (mismatches❗️).

Both cases of (new code + old peerings) fail because they try to use the ClusterKopfPeering CRD, which does not exist there — due to if namespace is None condition. They work fine if the CRD is the old one, KopfPeering.

All cases of (old code + new peerings) fail since they use the now-namespaced CRD (KopfPeering) with a cluster-scoped method (get_cluster_custom_object()).

This is not solvable, since it can only be done with a new version which supports the fallback to ClusterKopfPeering — and if one needs to upgrade, why not to upgrade to the new code with both old & new peerings supported; i.e. the transitional releases make no sense.

nolar · 2019-05-15T16:21:04Z

Rebased on master, and added the fallback scenario for the legacy mode (cluster-scoped KopfPeering). As tested manually, the new code works as expected both in the legacy mode (of the cluster) and in the new mode (as documented).

@samurang87 @aweller It can be reviewed and merged to master now.

samurang87 · 2019-05-17T10:43:27Z

docs/deployment.rst

 * ``kind: KopfExample`` for the example operator objects.
-* ``kind: Pod/Job/PersistentVolumeClaim`` as the children objects.
+* ``kind: Pod/CluJob/PersistentVolumeClaim`` as the children objects.


samurang87 · 2019-05-17T10:56:50Z

docs/peering.rst

+
+    kopf run --priority=$RANDOM ...
+
+``$RANDOM`` is a feature of bash (if you use another shell, find your own way).


"find your own way" is kinda rude :P

nolar requested a review from samurang87 as a code owner April 22, 2019 01:51

This was referenced Apr 22, 2019

Restrict the API calls in namespaced mode #37

Merged

Use --standalone option in the readme of the examples #31

Closed

s-soroosh reviewed Apr 22, 2019

View reviewed changes

nolar added the enhancement New feature or request label Apr 22, 2019

nolar mentioned this pull request Apr 29, 2019

Fix the docs for the RBAC examples, and add "get" permission on peering #50

Merged

nolar force-pushed the peering-scopes branch 2 times, most recently from edacc35 to d9033fb Compare April 29, 2019 15:50

nolar added this to the 1.0 milestone Apr 30, 2019

Sergey Vasilyev added 5 commits May 15, 2019 18:11

Separate the single peering to cluster- & namespace-scoped peerings

ac93b00

Update the RBAC templates for the peering objects & their scopes

109531c

Document the peering separation to cluster- & namespace-scoped

0ba11ad

Bring the peering CRDs to the naming conventions of namespaced resources

71299a8

Fallback to the legacy KopfPeering if it is cluster-scoped

cb01b97

nolar force-pushed the peering-scopes branch from 06fa510 to cb01b97 Compare May 15, 2019 16:21

samurang87 reviewed May 17, 2019

View reviewed changes

samurang87 previously approved these changes May 17, 2019

View reviewed changes

Fix a typo and phrasing in docs

43ca20d

nolar dismissed samurang87’s stale review via 43ca20d May 17, 2019 11:05

nolar force-pushed the peering-scopes branch from faab545 to 43ca20d Compare May 17, 2019 11:05

samurang87 approved these changes May 17, 2019

View reviewed changes

nolar merged commit 2427749 into master May 17, 2019

nolar deleted the peering-scopes branch May 17, 2019 11:23

nolar mentioned this pull request May 17, 2019

Fix the case when KopfPeering does not exist at all #66

Merged

kopf-archiver bot mentioned this pull request Aug 19, 2020

[PR] Peering scopes nolar/kopf#36

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Peering scopes #36

Peering scopes #36

nolar commented Apr 22, 2019 •

edited

Loading

zincr bot commented Apr 22, 2019 •

edited

Loading

s-soroosh Apr 22, 2019

nolar Apr 22, 2019

nolar commented May 15, 2019 •

edited

Loading

nolar commented May 15, 2019

samurang87 May 17, 2019

nolar May 17, 2019

samurang87 May 17, 2019

nolar May 17, 2019


		kopf run --priority=$RANDOM ...

		``$RANDOM`` is a feature of bash (if you use another shell, find your own way).

Peering scopes #36

Peering scopes #36

Conversation

nolar commented Apr 22, 2019 • edited Loading

zincr bot commented Apr 22, 2019 • edited Loading

🤖 zincr found 0 problems , 1 warning

Large Commits

s-soroosh Apr 22, 2019

Choose a reason for hiding this comment

nolar Apr 22, 2019

Choose a reason for hiding this comment

nolar commented May 15, 2019 • edited Loading

nolar commented May 15, 2019

samurang87 May 17, 2019

Choose a reason for hiding this comment

nolar May 17, 2019

Choose a reason for hiding this comment

samurang87 May 17, 2019

Choose a reason for hiding this comment

nolar May 17, 2019

Choose a reason for hiding this comment

nolar commented Apr 22, 2019 •

edited

Loading

zincr bot commented Apr 22, 2019 •

edited

Loading

nolar commented May 15, 2019 •

edited

Loading