list_event_for_all_namespaces_with_http_info raises ValueError due to None value for event_time if there are events
haxsaw opened this issue · 11 comments
What happened (please include outputs or screenshots):
If you try to fetch events when there aren't any, the above mentioned method returns just fine. But if there are events, then an exception is raised in the internal setter for the 'event_time' property as the value being read in from K8s is None:
if self.local_vars_configuration.client_side_validation and event_time is None: # noqa: E501
raise ValueError("Invalid value for `event_time`, must not be `None`") # noqa: E501or, as it happens:
File ".../python3.8/site-packages/kubernetes/client/api_client.py", line 303, in __deserialize
return self.__deserialize_model(data, klass)
File ".../lib/python3.8/site-packages/kubernetes/client/api_client.py", line 641, in __deserialize_model
instance = klass(**kwargs)
File ".../lib/python3.8/site-packages/kubernetes/client/models/events_v1_event.py", line 112, in __init__
self.event_time = event_time
File ".../lib/python3.8/site-packages/kubernetes/client/models/events_v1_event.py", line 291, in event_time
raise ValueError("Invalid value for `event_time`, must not be `None`") # noqa: E501
ValueError: Invalid value for `event_time`, must not be `None`
It may be the case that turning off client_side_validation will fix this, however there seem to be problems in this regard as well. In issue #1353 there was a mention of a breaking change (in release 1.12?) that involved setting up default config where client_side_validation can supposedly be turned off, but setting default configs don't seem to have any effect. Here's the code inside of the __init__() of EventsV1Api (local_vars_configuration is a parameter):
if local_vars_configuration is None:
local_vars_configuration = Configuration()
self.local_vars_configuration = local_vars_configurationSo if you don't pass in a value of local_vars_configuration, you get a brand-new Configuration object in which client_side_validation is True. One would have thought that the assignment to local_vars_configuration should have been:
local_vars_configuration = Configuration.get_default_copy()which would have kind of lined up with the notes for release 12.0.1. More curiously, if you have a look into the __init__() for Configuration itself, you find this comment before setting client_side_validation:
# Disable client side validation
self.client_side_validation = TrueSo the comment implies that there shouldn't be any client side validation by default, but the code does the opposite.
This is kind of feeling like a regression.
What you expected to happen:
I was expecting that default behaviour wouldn't raise an exception for fetching regular event data from K8s (or in my case, k3s). My tests don't cover every resource type that could be fetched, so there may be other similar fields lurking within the library.
How to reproduce it (as minimally and precisely as possible):
from kubernetes.client import EventsV1Api
from kubernetes.client import CoreV1Api
from kubernetes import config
def tryit():
config.load_kube_config(config_file="/etc/rancher/k3s/k3s.yaml") # the config for k3s
# you need to first carry out some action that will result in an Event being reported, so we'll make a simple Pod
p = {'apiVersion': 'v1',
'kind': 'Pod',
'metadata': {'labels': {'app': 'myapp'}, 'name': 'myapp-pod'},
'spec': {'containers': [{'command': ['sh',
'-c',
'echo Hello Kubernetes! && sleep 3600'],
'image': 'busybox',
'name': 'myapp-container'}]}}
api = CoreV1Api()
t0 = api.create_namespaced_pod_with_http_info(namespace='default', body=p)
ev = EventsV1Api()
t1 = ev.list_event_for_all_namespaces_with_http_info(async_req=False) # this craters
if __name__ == "__main__":
tryit()Anything else we need to know?:
In my dev environment I use k3s, which may be the source for the empty event_time (versions below). Nonetheless, there's clearly something wrong in how default Configurations are being handled.
As this is impacting objects that are being built inside of the K8s Python client itself, it isn't clear that there's a feasible workaround that could involve the user supplying the config object since it appears the default config isn't used and there's no way to pass in a config into list_event_for_all_namespaces_with_http_info().
Environment:
- Kubernetes version (
kubectl version): Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.8", GitCommit:"9f2892aab98fe339f3bd70e3c470144299398ace", GitTreeState:"clean", BuildDate:"2020-08-13T16:12:48Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"20", GitVersion:"v1.20.5+k3s1", GitCommit:"355fff3017b06cde44dbd879408a3a6826fa7125", GitTreeState:"clean", BuildDate:"2021-03-31T06:21:52Z", GoVersion:"go1.15.10", Compiler:"gc", Platform:"linux/amd64"} - OS (e.g., MacOS 10.13.6): Ubuntu 20.04.3 LTS
- Python version (
python --version): 3.810 - Python client version (
pip list | grep kubernetes): 19.15.0
/assign
This is likely similar to kubernetes-client/gen#52. EventTime is marked as required in the openapi spec but kube-apiserver doesn't always set/validate it. The fix is to mark the field as optional in https://github.com/kubernetes/kubernetes/blob/ae550b62da15ca5fe4983c79aaa6b2a39e3e711a/staging/src/k8s.io/api/events/v1/types.go#L41-L42
/unassign
/help
@roycaihw:
This request has been marked as needing help from a contributor.
Guidelines
Please ensure that the issue body includes answers to the following questions:
- Why are we solving this issue?
- To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
- Does this issue have zero to low barrier of entry?
- How can the assignee reach out to you for help?
For more details on the requirements of such an issue, please see here and ensure that they are met.
If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.
In response to this:
This is likely similar to kubernetes-client/gen#52. EventTime is marked as required in the openapi spec but kube-apiserver doesn't always set/validate it. The fix is to mark the field as optional in https://github.com/kubernetes/kubernetes/blob/ae550b62da15ca5fe4983c79aaa6b2a39e3e711a/staging/src/k8s.io/api/events/v1/types.go#L41-L42
/unassign
/help
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
If this gets marked as optional will this then propagate to the swagger? And when does that happen; will there be a patch release? How does that address the issue I found of default configs not being applied when no configuration object is supplied to a constructor? Isn't that incorrect? Or else how are we to use the default config?
If this gets marked as optional will this then propagate to the swagger?
Yes, the Go type is the source of truth for the swagger in k/k
And when does that happen; will there be a patch release?
Let's fix the issue at HEAD first. Then we can do a patch in this client
So the comment implies that there shouldn't be any client side validation by default, but the code does the opposite
It was a typo in the upstream generator: OpenAPITools/openapi-generator#6850
default configs not being applied when no configuration object is supplied to a constructor
Not sure I follow. The default config is being used here (and used by EventsV1Api here). You can set the default config using Configuration.set_default(c) as https://github.com/kubernetes-client/python/blob/master/CHANGELOG.md#v1201 mentioned
I am also encountering this issue. One note on this comment by @roycaihw :
Not sure I follow. The default config is being used here (and used by EventsV1Api here). You can set the default config using Configuration.set_default(c) as https://github.com/kubernetes-client/python/blob/master/CHANGELOG.md#v1201 mentioned
@roycaihw the default config is not being used on the models. For example:
When this code is changed to local_vars_configuration = Configuration.get_default_copy() then it is possible to bypass client_side_validation by setting the default configuration's client_side_validation to False
@benholtzmansmith Ah that makes sense. @haxsaw Sorry I thought the models would use the config from the API client and misread your comment.
Unable to disable client side validation is a regression. It has been fixed in upstream code generator: OpenAPITools/openapi-generator#8500. We should upgrade the generator version we use to pick up the fix.
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.
This bot triages issues and PRs according to the following rules:
- After 90d of inactivity,
lifecycle/staleis applied - After 30d of inactivity since
lifecycle/stalewas applied,lifecycle/rottenis applied - After 30d of inactivity since
lifecycle/rottenwas applied, the issue is closed
You can:
- Mark this issue or PR as fresh with
/remove-lifecycle stale - Mark this issue or PR as rotten with
/lifecycle rotten - Close this issue or PR with
/close - Offer to help out with Issue Triage
Please send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale
I have implemented this workaound while finxing this issue in our project for those who are interested:
def _workaround_client_validation_side():
# Unable to disable client side validation is a regression.
# Kubernetes considers this fields required, but it is not
# implemented yet for any controller.
# It has been fixed in upstream code generator by the PR:
# https://github.com/OpenAPITools/openapi-generator/pull/8500 .
# The issue is not processed yet:
# https://github.com/kubernetes-client/python/issues/1616
# We hack his value and let our event class to get the
# good value.
if "event_time" in EventsV1Event.attribute_map:
EventsV1Event.attribute_map["event_time"] = "deprecatedLastTimestamp"Is there any plan to fix this?
Is there anything I can do to get this fixed?