test: cover informer retry after a CR deserialization problem - #3558
test: cover informer retry after a CR deserialization problem#3558csviri wants to merge 2 commits into
Conversation
Sorry, something went wrong.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: f3f8b8af-29aa-4236-b401-8cf5e72f33d4 You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file. Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. ❤️ ShareComment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
Adds an IT for the situation of ADR 002 where the resource that cannot be deserialized is already present when the informer lists on startup, the counterpart of MultiVersionCRDIT, which covers the watch case. With stopOnInformerErrorDuringStartup set to false the operator starts, but the informer is not retried: the exception handler installed by the informer pool declines a retry for deserialization errors and the Reflector of the fabric8 client then completes its stop future, so fixing the problem in the cluster while the operator runs does not bring the informer back. The test asserts that current behavior, so it fails once the informer does get retried; the log of the informer pool no longer promises a periodic retry in this case.
There was a problem hiding this comment.
Pull request overview
This PR adds an integration test documenting current behavior when an informer fails to deserialize an already-present Custom Resource during the initial list-on-startup path (ADR 002 scenario), and adjusts runtime logging to avoid claiming periodic retries in a case where the informer won’t actually be retried.
Changes:
- Add MultiVersionCRDDeserializationRetryIT to cover deserialization failure during initial informer list and assert the informer does not recover after the offending CR is removed.
- Update AbstractInformerPool#start logging to explicitly state that deserialization-related startup failures won’t be retried when stopOnInformerErrorDuringStartup=false.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/multiversioncrd/MultiVersionCRDDeserializationRetryIT.java | New IT documenting non-retry behavior for startup list deserialization failures and validating runtime health remains degraded. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AbstractInformerPool.java | Refines startup error logging to avoid promising retries for deserialization exceptions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
|
This correctly tests the current behavior, although, visiting the topic again, I'm not entirely convinced, if this is the right thing to do, so not to retry on deserialization error. But will stick with this for now, and will re-visit on demand. |
Sorry, something went wrong.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Adds an IT for the situation of ADR 002 where the resource that cannot be
deserialized is already present when the informer lists on startup, the
counterpart of MultiVersionCRDIT, which covers the watch case. With
stopOnInformerErrorDuringStartup set to false the operator starts, but the
informer is not retried: the exception handler installed by the informer pool
declines a retry for deserialization errors and the Reflector of the fabric8
client then completes its stop future, so fixing the problem in the cluster
while the operator runs does not bring the informer back.
The test asserts that current behavior, so it fails once the informer does get
retried; the log of the informer pool no longer promises a periodic retry in
this case.