Skip to content

docs: adr regarding the cr deserialization problem #1506

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 3, 2022
Merged

docs: adr regarding the cr deserialization problem #1506

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ee1919b
docs: adr regarding the cr deserialization problem
csviri Sep 30, 2022
826adf2
Text improvements
csviri Sep 30, 2022
c103b32
Update adr/002-Custom-Resource-Deserialization-Problem.md
csviri Sep 30, 2022
009fbfb
fix: wording
metacosm Oct 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions adr/002-Custom-Resource-Deserialization-Problem.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Multi Version Custom Resources Deserialization Problem

## Status

accepted

## Context

In case there are multiple versions of a custom resource it can happen that a controller/informer tracking
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)
.
Such case is possible (as seen in the test) if there are no conversion hooks in place, so the two custom resources
which have different version are stored in the original form (not converted) and are not compatible.
In this case, if there is no further filtering (by labels) informer receives both, but naturally not able to deserialize
one of them.

How should the framework or the underlying informer behave?

Alternatives:

1. The informer should skip the resource and should continue to process the resources with the correct version.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

2. Informer stops and makes a notification callback.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


## Decision

From the JOSDK perspective, it is fine if the informer stops, and the users decides if the whole operator should stop
(usually the preferred way). The reason, that this is an obvious issue on platform level (not on operator/controller
level). Thus, the controller should not receive such custom resources in the first place, so the problem should be
addressed at the platform level. Possibly introducing conversion hooks, or labeling for the target resource.

## Consequences

If an Informer stops on such deserialization error, even explicitly restarting it won't solve the problem, since
would fail again on the same error.

## Notes

- The informer implementation in fabric8 client changed in this regard, before it was not stopping on deserialization
error, but as described this change in behavior is completely acceptable.

- the deserializer can be set to be more lenient by configuring the Serialization Unmatched Field Type module:
`Serialization.UNMATCHED_FIELD_TYPE_MODULE.setRestrictToTemplates(true);`. In general is not desired to
process custom resources that are not deserialized correctly.