-
Notifications
You must be signed in to change notification settings - Fork 14
Fix handling of Watch and Status objects in GenericJSONDecoder #1070
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
Conversation
What: This commit fixes the handling of objects in the decoder by properly dispatching watch & status types. Why: Those types were not handled properly before, causing WatchList to sometimes fail, if watch encountered retryable errors. Signed-off-by: Igor Suleymanov <[email protected]>
k8s/negotiator.go
Outdated
|
|
||
| switch { | ||
| case chk.Type != "": // Watch | ||
| obj, ok := into.(*metav1.WatchEvent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If into is nil here, should we instead set into into a *metav1.WatchEvent? In the CodecDecoder we have the into type checks if into is non-nil, and then fallback handling for a nil into (checking defaults, then trying the check unmarshal and examining that).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated!
Signed-off-by: Igor Suleymanov <[email protected]>
0f9c173 to
18aca36
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing that I missed before
k8s/wrappers.go
Outdated
| return codec.Read(bytes.NewReader(o.object), into) | ||
| } | ||
|
|
||
| // UntypedListObjectWrapper wraps a list of UntypedObjectWrappers, and implements runtime.Object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This godoc seems incorrect, given that items is a json.RawMessage. Does UntypedListObjectWrapper need to allow for access to the items by the reflector, or does it get iterated over by internal machinery to get those? IIRC for the informer to process a list, it needs to have an exported Items field which is a slice of objects.
Signed-off-by: Igor Suleymanov <[email protected]>
What
This commit fixes the handling of objects in the decoder by properly dispatching watch & status types.
Why
Those types were not handled properly before, causing WatchList to sometimes fail, if watch encountered retryable errors.