-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CRD watcher needs to handle out of order resource versions #609
Comments
Sorry I didn't quite understand the expectation here. From what I understand, the resource versions returned by kube-apiserver should not be interpreted as in alphabetical order, because the RV is an etcd feature and also an implementation detail that is subject to future change. Clients should not try to figure out order between events based on RV. Could you try running
I'm curious if some timeout happens and the watch gets restarted. |
I had the same issue. I checked |
@roycaihw I don't think the server necessarily needs to handle this. Rather, I think the client can deal with it gracefully. The issue comes down to the fact that we are tracking a single variable built incorrectly from many others. Each instance of the resource -- e.g. a pod -- has its own Currently the code does this by taking the most recently seen Another way of thinking of this is that we have some number of independent streams of resources to watch -- say N. For example, imagine I create N pods. Those N pods will each have their own sequence of updates that we can watch. When we watch all of them, we're not watching a single stream of updates; we're watching a stream built by interweaving the updates to all N streams. Consider the following example. We have three pods, A, B and C. A is created with resource version 1, then later updated with resource version 5. B is 2 then 4. C is 3. This is the state of the resources when we start watching. So, we should get A @ 5, B @ 4 and C @ 3. If the order of updates is C @ 3, then B @ 4 then A @ 5, the I think the fix is as simple as ensuring that the client python code only uses the "maximum" version of the resource when updating its cache. I.e. if the most recent It may not be so simple -- if there is a good reason for resourceVersions to not be monotonically increasing (integer overflow?), then we'll need another strategy. That's why I haven't submitted a PR myself. I don't understand the k8s machinery enough to know whether my "fix" is the right one. |
When using this code: resource_version = ''
while True:
stream = watch.Watch().stream(crds.list_cluster_custom_object, 'xxxx', 'v1', 'xxx', resource_version=resource_version)
try:
for event in stream:
obj = event["object"]
metadata = obj.get('metadata')
spec = obj.get('spec')
if not metadata or not spec:
logger.error('No metadata or spec in object, skipping: {0}'.format(json.dumps(obj, indent=1)))
resource_version = ''
break I occasionally have this error:
Is this error related to this issue? Should I handle it differently? |
It looks related to me. The "Error" message doesn't have the object type you'd expect associated with it, so nothing's filled in. I handled it by parsing out the version in the parenthesis (4535251 in your example), and setting the watcher's current resource version to that. It's not a very elegant solution, but it has worked for the most part. That said, for the reasons I gave above, I'd like it to be fixed in the client library itself.
I call |
Ahh nice. Thanks @klarose ! Yes, think this should definitely be handled in the python client. How does the go client handle this? Or other projects that use the go client? |
The code should not use the maximum of the resource version of each object it has seen, it should use the resource version of the top level listing object that was used to return the objects, that contains a resource version too and guarantees that the all objects with updates have been returned up to the resource version of the listing object. See the method used in #700 which is correct when the python library does not interfere and messes the resource versions up. |
nevermind, I had a more detailed look and it seems kubernetes does not actually give you the list object when you are watching even when you receive multiple objects, so you have no single resource version you should base your next watch on. |
@juliantaylor edit: I haven't tested this, so I may be wrong, but it seems reasonable. :) |
yes that is a good way to implement a watch using the api directly. But I do not know if it is guaranteed by kubernetes to return events in a watch in order of the resource version and the current code assumes that it does. |
fwiw etcd which kubernetes api is based on says this:
|
I cannot find any information on the overflow behavior, but the etcd code structures appear to be |
I'd hope k8s (and maybe even etcd...) would handle that for us, since I expect it'd mess up its internals. That said, 2^63 is a pretty big number. My rough calculations suggest that overflow would occur in close to 300K years with 1 million operations per second. Can the k8s API even handle close to that?
|
I guess one can ignore it, I'll open a PR for fixing my issue (#700) not sure if it would fix yours. I have not yet seen out of order events. |
fwiw here is a proof of concept branch which does reordering of potential unordered events https://github.com/juliantaylor/python-base/tree/unordered-events |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
@fejta-bot: Closing this issue. In response to this:
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. |
ref: #819 is newer issue for I believe same thing. |
I recently started running into an issue where my controller, which was watching CRDs, would fail with a 410 from its watch on the list function.
After digging into it a bit, I noticed that the version I was getting back at the beginning was not in order. Then, after some time, it would fail with a 410 error code.
After poking around a bit, it sound like out of order events are possibly expected?
In the following example, I have three instances of the resource, let's call them "A", "B" and "C". In order, their resource versions are: 835745, 797550 and 746039. This is the order they are provided by the list. Of interest, I think, is the fact that they appear in alphabetical order, despite not being created in that order.
Here are some logs from my application highlighting the issue. Here, I have "solved" the problem by extracting the "new" resource version from the error code, then updating the watcher's internal resource version to that value.
kubernetes-client/python-base#64 recently fixed the problem of the CRDs not updating the internal resourceVersion. However, should it be doing taking the max of the old and new resourceVersion to ensure things don't get messed up?
Ultimately there are a few things that are sub-par about the current state of affairs:
Regarding 1, I think that's definitely a bug in the watcher. Regarding # 2, is this an unreasonable expectation -- that is, should I be handling cases like this in my application?
I'm running my application on Ubuntu 18.04.
Python version: 3.6.5
python kubernetes client version: 7.0.0
Kubernetes server version: 1.11.1
Thanks,
Kyle
The text was updated successfully, but these errors were encountered: