quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.75k stars 2.67k forks source link

Informer is unable to deserialize Kubernetes resource in native mode #26117

Closed metacosm closed 2 years ago

metacosm commented 2 years ago

Describe the bug

Would appreciate help figuring out https://github.com/quarkiverse/quarkus-operator-sdk/issues/359. It seems that some resources (not even sure which) are not properly deserialized, resulting in an NPE downstream (the observed exception). /cc @iocanel @manusa

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

See: https://github.com/quarkiverse/quarkus-operator-sdk/issues/359#issuecomment-1151126745

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

quarkus-bot[bot] commented 2 years ago

/cc @geoand, @iocanel

metacosm commented 2 years ago

Note that a similar issue can be seen in https://github.com/marcnuri-demo/kubernetes-controller-jl-port if you comment out these lines: https://github.com/marcnuri-demo/kubernetes-controller-jl-port/blob/main/src/main/java/com/marcnuri/demo/booternetes/port/KubernetesControllerApplication.java#L34-L39 which seems to point out to GraalVM needing some "warm-up" code to load the classes required by the informers to work properly. Note also that these classes should already be properly registered for reflection, since the code works fine with the "warm-up" code.

geoand commented 2 years ago

Where is the registration of the model class for reflection coming from?

metacosm commented 2 years ago

Where is the registration of the model class for reflection coming from?

In the case of the application linked to in the description field, the error occurs before any custom resources are created so the registration for reflection is supposed to be handled by the Kubernetes client extension, as far as I'm aware: the only resources dealt with are CRDs and ConfigMap.

It appears the deserialisation error happens when the informer starts which results in it trying to list all existing config maps and deserialise them in a list. The list is created but the items are null which leads to the NPE later on when the operator SDK tries to manipulate the list item when it's notified by the informer that a resource has been added to the watched set.

geoand commented 2 years ago

so the registration for reflection is supposed to be handled by the Kubernetes client extension

This is sort of true, but it's not the whole story. Quarkus does not register the entire Kubernetes model for reflection, as that model is huge. It needs to know what is actually being used in order to register the proper classes. The way it knows is by looking at the parameter types, return types or generic types of known methods / interfaces. If the application in question does not fall under one of the known categories, it will need to register reflection manually.

metacosm commented 2 years ago

So that might be what's going on… and hence, why "warming" the code results in the proper result… That said, which interfaces / methods are considered? I think it would make sense to add the Informer registration methods to that set since it seems they're not part of it and Informers are quite widely used, in particular for operators.

geoand commented 2 years ago

That said, which interfaces / methods are considered?

See https://github.com/quarkusio/quarkus/blob/main/extensions/kubernetes-client/deployment/src/main/java/io/quarkus/kubernetes/client/deployment/KubernetesClientProcessor.java#L42

I think it would make sense to add the Informer registration methods to that set since it seems they're not part of it and Informers are quite widely used, in particular for operators

If you show me a simple example, we can work from there to make it happen.

metacosm commented 2 years ago

There are some issues with the current code. In particular, when it comes to processing CustomResource "implementors", which looking at git blame, is actually my own fault! 😅 Indeed, the code tries to extract types from interface implementations but CustomResource is an abstract class, not an interface so the code doesn't work as expected. Also, in the case of CustomResource (and, ultimately, abstract classes that we want to process in a similar way), we also want to register the extending class itself, not just its type parameters. This is addressed by #26188.

There are more issues at play but, after discussing with @geoand, we agreed that it makes more sense to provide the required support as part of the Quarkus extension for the Java Operator SDK as the required changes would not be appropriate, nor practical, in the Kubernetes client extension.

metacosm commented 2 years ago

Note that the issue is still occurring after changes to the Quarkus operator SDK extension and I would appreciate help solving this issue.

geoand commented 2 years ago

What changes were made and what is the current problem?

metacosm commented 2 years ago

26188 was implemented in Quarkus and https://github.com/quarkiverse/quarkus-operator-sdk/pull/364 was merged to better register classes for reflection but the core issue of the informers not being able to deserialize the resources still persist.

geoand commented 2 years ago

So there is still an issue where reflection cannot determine the properties / methods of the classes?

If so, do the steps outlined in the description still reflect how the issue can be reproduced?

metacosm commented 2 years ago

Let's close this for now, since I'm not sure this still happens. Will open a new issue if needed.