gtsystem / lightkube

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

API change in k8s 1.23 breaking StatefulSetStatus? #30

Closed ca-scribner closed 2 years ago

ca-scribner commented 2 years ago

I might have this wrong as I don't fully understand how lightkube and lightkube-models interacts, but...

I've recently found this code that worked with lightkube-models==1.21.0.4 (and I believe, 1.22.X) now fails:

import lightkube
from lightkube.resources.apps_v1 import StatefulSet

client = lightkube.Client()

found_resources = list(client.list(StatefulSet))
# Will raise with: {TypeError}__init__() missing 1 required positional argument: 'availableReplicas'
found_resources[0].status

I think it is because for class StatefulSetStatus the availableReplicas has changed from an optional kwarg (in 1.21.0.4) to a positional arg (in 1.23.X). Am I configuring something wrong between lightkube and lightkube-models, or is this an actual bug?

gtsystem commented 2 years ago

The models are generated from the swagger specifications for kubernetes. It seems that availableReplicas was introduced in 1.22 and set as required. Follow is the spec in 1.23: https://raw.githubusercontent.com/kubernetes/kubernetes/v1.23.5/api/openapi-spec/swagger.json

    "io.k8s.api.apps.v1.StatefulSetStatus": {
      "description": "StatefulSetStatus represents the current state of a StatefulSet.",
      "properties": {
        "availableReplicas": {
          "description": "Total number of available pods (ready for at least minReadySeconds) targeted by this statefulset. This is a beta field and enabled/disabled by StatefulSetMinReadySeconds feature gate.",
          "format": "int32",
          "type": "integer"
        },
        ...
      },
      "required": [
        "replicas",
        "availableReplicas"
      ],
      "type": "object"
    },

As you can see, this attribute is formally defined as required, so the code generator is ensuring that. I can however interpret from the description, that this field may not be provided without the feature gate StatefulSetMinReadySeconds enabled. To me this is a bug in the kubernetes definition: I believe they should have introduce this attribute as optional, at least until a k8s version is released where this attribute is always provided. Feel free to report this issue in https://github.com/kubernetes/kubernetes/issues and if it get fixed, I can generate a new version from the specs.

ca-scribner commented 2 years ago

Being discussed in kubernetes/kubernetes#109210

ca-scribner commented 2 years ago

I'm curious, say this gets implemented and 1.24 has this required field that 1.21 did not. How is that reflected in the lightkube side of things? Does lightkube have a way of properly passing the new required field to some but not all lightkube-model versions?

gtsystem commented 2 years ago

While lightkube itself is mostly version agnostic, each lightkube-model version is designed to work with the corresponding k8s api version. Of course there is some level of retro-compatibility so that the models for 1.23 will work fine in a server with 1.24 (after they address the issue above). The opposite (new models in old api server) may not always work. Regarding your second question and assuming you want to support 1.23 (let's call it old) and 1.24 (let's call it new):

Alternatively, you can check the models version (from lightkube.models import __version__) and based on that fill up the models as needed.

gtsystem commented 2 years ago

Original k8s ticket is closed but this change still didn't got backported to 1.22 and 1.23 branches. kubernetes/kubernetes#109274 should add this attribute for 1.23.

gtsystem commented 2 years ago

@ca-scribner I released lightkube-models 1.23.6.4 . Since the issue as been acknowledged and will be fixed I back-ported it to the last available spec for 1.23.

After checking again, it seems 1.22 is not affected.