gen icon indicating copy to clipboard operation
gen copied to clipboard

Required arrays with null value

Open mbohlool opened this issue 7 years ago • 20 comments

Looks like there is a problem with 1.8 spec going forward [TODO:ref issue] that an array that marked as required can be null. Assuming an empty array is an acceptable array in these situation (and it look like it is as kubernetes return empty array in some of those cases, TODO: add ref issues) we should consider fixing it in the main repo (TODO: create an issue) and also here as a quick fix until main repo issue has been fixed. This is my brain dump on the issue in case of anyone wants to work on this:

  • we should first research the problem and see if that is the case for all required arrays or a subset of them
  • as a quick fix, we can remove those arrays (or subset of them based on previous findings) from required list in a preprocessing step in this repo.
  • A regeneration of python (and other languages) clients should fix the issues (TODO: ref those issues)
  • Next, there should be a more permanent fix in the main repo.

mbohlool avatar Jan 31 '18 10:01 mbohlool

Gathering known required API fields issues in python client:

  • [x] io.k8s.apimachinery.pkg.apis.meta.v1.APIVersions requires serverAddressByClientCIDRs (https://github.com/kubernetes-client/python/issues/418, https://github.com/kubernetes-client/python/issues/394, https://github.com/kubernetes-client/python/issues/464)

    • serverAddressByClientCIDRs is required by server may return null as a valid response https://github.com/kubernetes/kubernetes/blob/9bd4f12c336af339c35e773e1d71a37cad342f57/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go#L407-L414
    • Fixed in https://github.com/kubernetes/kubernetes/pull/61963
  • [ ] v1beta1.CustomResourceDefinitionStatus requires conditions (https://github.com/kubernetes-client/python/pull/447)

  • [ ] DisruptedPods in policy.v1beta1.PodDisruptionBudgetStatus (kubernetes-client/python#466)

  • [x] extensions_v1beta1_deployment_rollback. Unrelated. There is a k8s pr fixing the wrong openapi spec. (https://github.com/kubernetes-client/python/issues/491, https://github.com/kubernetes-client/python/issues/427)

  • [x] involved_object in v1.Event. Unrelated. The bug is in watch client not handling HTTP error before unmarshaling https://github.com/kubernetes-client/python-base/issues/57 (kubernetes-client/python#484)

The issue started in release-4.0. Although we had the required-field validation generated since release-1.0, the api_client would assign [] for the null fields before deserialize the model (before release-3.0). Since release-4.0 the generated api_client doesn't convert null to empty array anymore, and the ValueError gets raised.

Agree that the problem is in our API convention (can an required array be null?), and there should be a permanent fix in the main repo.

roycaihw avatar Mar 26 '18 22:03 roycaihw

There are a number of other API fields with this problem. From watching the issues in the two repositories in question, I have the following list. (This is just straight links, I don't have time right now to make the formatting pretty. I tried to avoid duplicates.)

Kubernetes

https://github.com/kubernetes-client/python/issues/491 https://github.com/kubernetes-client/python/issues/484 https://github.com/kubernetes-client/python/issues/466 https://github.com/kubernetes-client/python/issues/427 https://github.com/kubernetes-client/python/issues/424

OpenShift

https://github.com/kubernetes-client/python/issues/464 https://github.com/openshift/openshift-restclient-python/issues/138

There are probably more that I haven't run into and that no one else has bothered to post an issue for yet.

ceridwen avatar Mar 27 '18 01:03 ceridwen

Looking at related issues (specifically #394) This has been an issue since at least November.

Is there any word on a fix or a workaround? As someone new to Kubernetes and even newer to the Python client, I have absolutely no idea how to move past this and make my application work.

smiller171 avatar May 08 '18 15:05 smiller171

@smiller171 For good or ill, I've managed to work around this in my kubernetes-python module by commenting out the subject enforcement check at lines 184 & 185 of my /usr/local/lib/python3.6/site-packages/kubernetes/client/models/v1beta1_cluster_role_binding.py file (the content being):

        if subjects is None:
            raise ValueError("Invalid value for `subjects`, must not be `None`")

StephanX avatar May 17 '18 19:05 StephanX

I just came across this issue with the "limits" array in the LimitRangeSpec resource. If I write a LimitRange object with an empty array, when reading it back, I get the exception: "Invalid value for limits, must not be None". I worked around it by catching those exceptions and dealing with them - but I'm wondering if removing the test raising the exception like StephanX did would be better.

This seems to be a serious, pervasive issue with at least the 5.0.0 library. Is that accurate, or am I misunderstanding something?

Is there any fix in the works? Or suggested workaround other than dealing with each and every occurrence? So far, in my project, we've had to implement workarounds for this limits case, and the PodDisruptionBudget case (https://github.com/kubernetes-client/python/issues/466). This issue (#52) makes it appear there could be many more problems lurking out there. Do we need to give up on 5.0.0, and go back to an earlier stable library release?

djmckinley avatar May 17 '18 21:05 djmckinley

Upstream PR fixing serverAddressByClientCIDRs: https://github.com/kubernetes/kubernetes/pull/61963

roycaihw avatar May 21 '18 23:05 roycaihw

Fixes for Conditions in CustomResourceDefinitionStatus https://github.com/kubernetes/kubernetes/pull/64996 DisruptedPods in policy.v1beta1.PodDisruptionBudgetStatus https://github.com/kubernetes/kubernetes/pull/65041

roycaihw avatar Jun 12 '18 23:06 roycaihw

Given that apiserver ignores the rule today, everything just works better if we adjust the specs to mark these fields as optional, no?

lavalamp avatar Jun 22 '18 21:06 lavalamp

Also, see https://github.com/kubernetes/kube-openapi/issues/87

lavalamp avatar Jun 22 '18 21:06 lavalamp

@roycaihw you can check v1beta1.CustomResourceDefinitionStatus requires conditions (kubernetes-client/python#447) because it is closed already.

mydockergit avatar Sep 26 '18 12:09 mydockergit

@mydockergit The field is still marked as required, as openapi spec shows:

   "io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1beta1.CustomResourceDefinitionStatus": {
    "description": "CustomResourceDefinitionStatus indicates the state of the CustomResourceDefinition",
    "required": [
     "conditions",
     "acceptedNames",
     "storedVersions"
    ],

I think kubernetes-client/python#447 was closed because we want to fix the API upstream

roycaihw avatar Sep 27 '18 00:09 roycaihw

Any luck with this?

agassner avatar Nov 20 '18 18:11 agassner

Hi There, I am running python 3.7.2 and hit the same problem trying to deploy sql aag onto AKS, by running "python ./deploy-ag.py ...". ←[31m[ERROR] ←[0mCaught exception: Invalid value for conditions, must not be None ←[31m[ERROR] ←[0m<bound method ApiextensionsV1beta1Api.list_custom_resource_definition of <kubernetes.client.apis.apiextensions_v1beta1_api.ApiextensionsV1beta1Api object at 0x04178A10>> Could not complete successfully.

But the AKS is created. Is there anything i need to do to fix up the errors and how?

Thanks & regards, Irene

ikflc avatar Apr 02 '19 05:04 ikflc

It seems caused by the issue of type hinting or some other similar I can monkey patch it in my code but it is too tricky in product environment Is there any other way to work around it ?

@ikflc here's a quick but dirty patch

from kubernetes import client

# ======== Hack for V1beta1CustomResourceDefinitionStatus bug ==================
# Things happened in /site-packages/kubernetes/client/api_client.py:635
# We should override models.V1beta1CustomResourceDefinitionStatus to avoid "Empty conditions" error
# Such issue relates to https://github.com/kubernetes-client/gen/issues/52
# I have nothing to do but monkey patch it

class Hack_Status_Conditions(client.api_client.models.V1beta1CustomResourceDefinitionStatus):
    
    def __init__(self, *args, **kwargs):
        super(Hack_Status_Conditions, self).__init__(*args, **kwargs)
        
    @property
    def conditions(self):
        return super(Hack_Status_Conditions, self).conditions

    @conditions.setter
    def conditions(self, cond):
        self._conditions = list() if cond == None else cond

client.api_client.models.V1beta1CustomResourceDefinitionStatus = Hack_Status_Conditions

# =================== DONE hacking here ================================================

Test passed in Python 3.7.2 / Kubeclient-Python v9.0.0 / Kubernetes 1.13.5

NKUCodingCat avatar Apr 20 '19 17:04 NKUCodingCat

@roycaihw looks like some of your fix PRs didn't get enough attention, maybe you can revisit them?

lavalamp avatar Apr 22 '19 16:04 lavalamp

@kubernetes/sig-api-machinery-bugs, we have the same problem Can anyone help to look at this ?

zhangmingld avatar Apr 28 '19 10:04 zhangmingld

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot avatar Jul 27 '19 10:07 fejta-bot

/remove-lifecycle stale /lifecycle frozen

roycaihw avatar Jul 30 '19 20:07 roycaihw

Am I running into this here, or shall I raise a separate issue? Code straight from one of the examples:

deployment = client.AppsV1beta1Deployment()
/opt/conda/lib/python3.6/site-packages/kubernetes/client/models/apps_v1beta1_deployment_spec.py in __init__(self, min_ready_seconds, paused, progress_deadline_seconds, replicas, revision_history_limit, rollback_to, selector, strategy, template)
     87         if strategy is not None:
     88           self.strategy = strategy
---> 89         self.template = template
     90 
     91     @property

/opt/conda/lib/python3.6/site-packages/kubernetes/client/models/apps_v1beta1_deployment_spec.py in template(self, template)
    294         """
    295         if template is None:
--> 296             raise ValueError("Invalid value for `template`, must not be `None`")
    297 
    298         self._template = template

ValueError: Invalid value for `template`, must not be `None`

kierenj avatar Nov 06 '19 21:11 kierenj

I get similar results when calling list_api_service(): ValueError: Invalid value for service, must not be None

However, service is None whenever a service is Local (which is most of them).

And when calling list_controller_revision_for_all_namespaces(): ValueError: Invalid value for raw, must not be None

RuntimeRawExtension is explicitly listed as [optional] in the documentation for V1ControllerRevision, but is treated as mandatory by the bindings.

I've only tested with version 10.1.0 of the bindings, but checking the source code for 11.0.0 seems to indicate that the issue is still there.

dweineha avatar Mar 24 '20 15:03 dweineha