From ee1919b347643a5ae22753552780a20a2a45f7ad Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 30 Sep 2022 15:07:03 +0200 Subject: [PATCH 1/4] docs: adr regarding the cr deserialization problem --- ...Custom-Resource-Deserialization-Problem.md | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 adr/002-Custom-Resource-Deserialization-Problem.md diff --git a/adr/002-Custom-Resource-Deserialization-Problem.md b/adr/002-Custom-Resource-Deserialization-Problem.md new file mode 100644 index 0000000000..dd05ebdccd --- /dev/null +++ b/adr/002-Custom-Resource-Deserialization-Problem.md @@ -0,0 +1,37 @@ +# Multi Version Custom Resources Deserialization Problem + +## Status + +accepted + +## Context + +In case there are multiple versions of a custom resource in 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) +. 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. + +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. +2. Informer stops and makes a notification callback. + +## Decision + +From the JOSDK perspective 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 on 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 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. \ No newline at end of file From 826adf22feca4b5d520f428aa7444f0fdf53780e Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 30 Sep 2022 15:38:11 +0200 Subject: [PATCH 2/4] Text improvements - add note on lenient deserializtion - better explain the case where the target scenario can happen --- ...002-Custom-Resource-Deserialization-Problem.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/adr/002-Custom-Resource-Deserialization-Problem.md b/adr/002-Custom-Resource-Deserialization-Problem.md index dd05ebdccd..baf0a5e55b 100644 --- a/adr/002-Custom-Resource-Deserialization-Problem.md +++ b/adr/002-Custom-Resource-Deserialization-Problem.md @@ -9,8 +9,11 @@ accepted In case there are multiple versions of a custom resource in 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) -. 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. +. +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? @@ -33,5 +36,9 @@ would fail again on the same error. ## 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. \ No newline at end of file +- 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. + +- 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. \ No newline at end of file From c103b32a64f5e0bc95ea6458e9619e35198b6274 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 30 Sep 2022 15:52:42 +0200 Subject: [PATCH 3/4] Update adr/002-Custom-Resource-Deserialization-Problem.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Václav Muzikář --- adr/002-Custom-Resource-Deserialization-Problem.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/adr/002-Custom-Resource-Deserialization-Problem.md b/adr/002-Custom-Resource-Deserialization-Problem.md index baf0a5e55b..b53ab541bc 100644 --- a/adr/002-Custom-Resource-Deserialization-Problem.md +++ b/adr/002-Custom-Resource-Deserialization-Problem.md @@ -36,8 +36,8 @@ would fail again on the same error. ## 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. +- 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 From 009fbfbb61978054b2971ed980ca4ae9a1009918 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 3 Oct 2022 13:42:55 +0200 Subject: [PATCH 4/4] fix: wording --- adr/002-Custom-Resource-Deserialization-Problem.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/adr/002-Custom-Resource-Deserialization-Problem.md b/adr/002-Custom-Resource-Deserialization-Problem.md index b53ab541bc..d1314f73a7 100644 --- a/adr/002-Custom-Resource-Deserialization-Problem.md +++ b/adr/002-Custom-Resource-Deserialization-Problem.md @@ -6,7 +6,7 @@ accepted ## Context -In case there are multiple versions of a custom resource in can happen that a controller/informer tracking +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) . @@ -24,10 +24,10 @@ Alternatives: ## Decision -From the JOSDK perspective is fine if the informer stops, and the users decides if the whole operator should stop +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 on platform level. Possibly introducing conversion hooks, or labeling for the target resource. +addressed at the platform level. Possibly introducing conversion hooks, or labeling for the target resource. ## Consequences