gtsystem / lightkube

Modern lightweight kubernetes module for python
https://lightkube.readthedocs.io
MIT License
109 stars 13 forks source link

Add `sort_objects()` to make batch resource actions simpler #33

Closed ca-scribner closed 2 years ago

ca-scribner commented 2 years ago

This PR proposes some helpers to address a problem with handling batches of resources. I'm not married to the API, etc., and am happy to adjust and flesh out anything that is missing (for example, create_many()?), but I'm mainly curious atm whether this feature would be appreciated in lightkube or if I should finish it as a separate helper I keep elsewhere.

Working with a batch of resources has an order of operations challenge. For example, this valid YAML contains both a ClusterRoleBinding and the ClusterRole/ServiceAccount it references:

kind: ClusterRoleBinding
roleRef:
  kind: ClusterRole
  name: example-cluster-role-binding
subjects:
- kind: ServiceAccount
  name: example-service-account
...
---
kind: ClusterRole
metadata:
  name: example-cluster-role
...
---
kind: ServiceAccount
metadata:
  name: example-service-account

If users loop over the yaml and create the objects, they will get errors because the ClusterRoleBinding is created before the items it uses. Tools like kubectl or kapp handle this for the user.

This PR is a proposal to similarly automate this issue with lightkube by adding Client.apply_many() and Client.delete_many() helpers. They both accept iterables of resources and, before executing on them, order them in a safer order by kind. afaict this is sufficient to avoid any conflicts, but I'm flexible if there's something missing.

gtsystem commented 2 years ago

Did you find the corresponding logic in kubectl source code? Is it just based on resource names like in your implementation or something else?

In general I would like to keep this library sufficiently low level and easy to learn. So I would prefer to avoid new methods that can be created with few lines of code. The key component of your PR is the sorting function and that is something we could include as it's non trivial and can be used for creating more high level functionalities.

So something like

from lightkube import sort_objects

for obj in sort_objects(objs, by='kind-rank'):    # better name and signature TBD
    client.apply(obj)

for obj in sort_objects(objs, by='kind-rank', reverse=True):
    namespace = obj.metadata.namespace if isinstance(obj, NamespacedResource) else None
    client.delete(obj, name=obj.metadata.name, namespace= namespace)

We can also support other types of object sorting in the future.

ca-scribner commented 2 years ago

That makes sense to me - keeping this to a sorting helper works. In my opinion having a _many version of the create/apply/delete would be helpful without complicating the package too much (their names will be intuitive, which I think helps) but I can see your point as well.

I made a mistake in the original post - as far as I can tell, kubectl does not reorder objects before applying. I don't see any ordering in the source, and after testing it explicitly just now I think I have just been lucky in the past and always applied YAML that was already in a safe order. The tool kapp does reorder objects before applying, and their kind-rank is similar or the same to what is here.

ca-scribner commented 2 years ago

Sorry, between business trips and covid I dropped the ball on this. I'll put a workable solution together next week

ca-scribner commented 2 years ago

I have reverted the previous changes and added a sort_objects like you mentioned. What do you think? I wasn't sure where this function should live - it currently is in client.py, but it feels a bit out of place. Would appreciate suggestions or any requested changes - thanks!

ca-scribner commented 2 years ago

@gtsystem thanks for the great feedback. How does it look now?

I will write up a example for codecs.md tomorrow

gtsystem commented 2 years ago

Thanks a lot, changes are merged.