docs: adr regarding the cr deserialization problem #1506
Conversation
|
cc @vmuzikar |
| such a resource might run into deserialization problem as shown | ||
| in [this integration test](https://github.com/java-operator-sdk/java-operator-sdk/blob/07aab1a9914d865364d7236e496ef9ba5b50699e/operator-framework/src/test/java/io/javaoperatorsdk/operator/MultiVersionCRDIT.java#L55-L55) | ||
| . In the mentioned case two versions of a custom resource are not compatible with each other. The informer receives | ||
| both, but naturally not able to deserialize one of them. |
There was a problem hiding this comment.
Can we clarify somewhere in the doc that the informer/watcher receives an event for resource with a given Group Version Kind (GVK) but with the structure of a different (former or newer) version (for the same Group and Kind).
This is what in the end might lead to a deserialization Exception if the Serializer is configured in a strict mode and some of the fields might have unmatched target types because there was a change in between the two versions of the same resource definition.
Note that the deserializer can be set to be more lenient by configuring the Serialization Unmatched Field Type module:
Serialization.UNMATCHED_FIELD_TYPE_MODULE.setRestrictToTemplates(true);There was a problem hiding this comment.
Note that the deserializer can be set to be more lenient by configuring the Serialization Unmatched Field Type module:
I think this is something we want to avoid. So processing resource that are not complete in some cases, that can lead to some more confusion. But might be good to put this into the docs as a note.
There was a problem hiding this comment.
Can we clarify somewhere in the doc that the informer/watcher receives an event for resource with a given Group Version Kind (GVK) but with the structure of a different (former or newer) version (for the same Group and Kind).
added some explanation, not sure if you meant it this way
|
|
||
| Alternatives: | ||
|
|
||
| 1. The informer should skip the resource and should continue to process the resources with the correct version. |
There was a problem hiding this comment.
IMHO I think this shouldn't even be considered as an alternative. It might encourage users to ignore errors that in the long-term will definitely have an impact in their application's lifecycle.
There was a problem hiding this comment.
Just adding since it was working this way if until now, and before was mentioned in other conversations. But completely agree with what you say.
- add note on lenient deserializtion - better explain the case where the target scenario can happen
Co-authored-by: Václav Muzikář <vaclav@muzikari.cz>
|
Thank you @vmuzikar ! |
| Alternatives: | ||
|
|
||
| 1. The informer should skip the resource and should continue to process the resources with the correct version. | ||
| 2. Informer stops and makes a notification callback. |
There was a problem hiding this comment.
I do believe that this is the expected behavior, a possible improvement would be to include the GenericKubernetesResource that generated the issue in the Exception / Notification.
There was a problem hiding this comment.
The problem is that at that point is useless (or rather does not fit very well). So the informer is already stopped, event if the resource is fixed from the callback method, on next start there might a other resource with that problem. So it is a restart per problematic resource.
The solution here is to use conversion hooks.
|
|
||
| ## Notes | ||
|
|
||
| The informer implementation if fabric8 client changed in this regard, before it was not stopping on deserialization |
There was a problem hiding this comment.
| The informer implementation if fabric8 client changed in this regard, before it was not stopping on deserialization | |
| The informer implementation in the Fabric8 client has changed in this regard starting with version 5.12.4. It was previously not stopping on deserialization |
| ## Notes | ||
|
|
||
| The informer implementation if fabric8 client changed in this regard, before it was not stopping on deserialization | ||
| error, but as describe this change in behavior is completely acceptable. |
There was a problem hiding this comment.
| error, but as describe this change in behavior is completely acceptable. | |
| errors, but as described in this document, this change in behavior is completely acceptable. |
Created an ADR for this quesiton. So we are all aligned.
(Feel free to improve the content and/or wording)