apache / camel-k

Apache Camel K is a lightweight integration platform, born on Kubernetes, with serverless superpowers
https://camel.apache.org/camel-k
Apache License 2.0
869 stars 348 forks source link

Error: io.quarkus.vertx.http.runtime.TrustedProxyCheckPartConverter not a subtype #5758

Closed lsergio closed 3 months ago

lsergio commented 3 months ago

What happened?

I have been randomly getting an error starting Integrations that use quartz and run on camel-k-runtime 3.2.3. The Integrations builds, but fails to start with:

Exception in thread "main" java.lang.reflect.InvocationTargetException
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:569)
    at io.quarkus.bootstrap.runner.QuarkusEntryPoint.doRun(QuarkusEntryPoint.java:61)
    at io.quarkus.bootstrap.runner.QuarkusEntryPoint.main(QuarkusEntryPoint.java:32)
Caused by: java.util.ServiceConfigurationError: org.eclipse.microprofile.config.spi.Converter: io.quarkus.vertx.http.runtime.TrustedProxyCheckPartConverter not a subtype
    at java.base/java.util.ServiceLoader.fail(ServiceLoader.java:593)
    at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNextService(ServiceLoader.java:1244)
    at java.base/java.util.ServiceLoader$LazyClassPathLookupIterator.hasNext(ServiceLoader.java:1273)
    at java.base/java.util.ServiceLoader$2.hasNext(ServiceLoader.java:1309)
    at java.base/java.util.ServiceLoader$3.hasNext(ServiceLoader.java:1393)
    at io.smallrye.config.SmallRyeConfigBuilder.discoverConverters(SmallRyeConfigBuilder.java:135)
    at io.smallrye.config.SmallRyeConfig.buildConverters(SmallRyeConfig.java:78)
    at io.smallrye.config.SmallRyeConfig.<init>(SmallRyeConfig.java:70)
    at io.smallrye.config.SmallRyeConfigBuilder.build(SmallRyeConfigBuilder.java:629)
    at io.quarkus.runtime.generated.Config.<clinit>(Unknown Source)
    at io.quarkus.runner.ApplicationImpl.<clinit>(Unknown Source)
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)
    at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:481)
    at io.quarkus.runtime.Quarkus.run(Quarkus.java:70)
    at io.quarkus.runtime.Quarkus.run(Quarkus.java:44)
    at io.quarkus.runtime.Quarkus.run(Quarkus.java:124)
    at io.quarkus.runner.GeneratedMain.main(Unknown Source)
    ... 6 more

Here is a sample Integration that eventually reproduces the error:

apiVersion: camel.apache.org/v1
kind: Integration
metadata:
  name: quartz-323
spec:  
  sources:
  - name: main.yaml
    content: |-
      - from:
          uri: quartz://ipaas/trigger?cron=0+*+*+?+*+*
          steps:
          - log:
              message: "It worked"
  traits:
    camel:
      runtimeVersion: 3.2.3
    cron:
      fallback: true

Steps to reproduce

No response

Relevant log output

No response

Camel K version

2.4.0

squakez commented 3 months ago

It seems to be a runtime problem. I understand moving to a newer runtime is not a possibility, right? then, my suggestion is to engage with Camel Quarkus project directly to understand why this may be happening.

lsergio commented 3 months ago

Some interesting info: I bash'ed into an image that is failing and searched for the TrustedProxyCheckPartConverter class in the depency jar files:

root@369fc154db17:/deployments/dependencies/lib/main# grep TrustedProxyCheckPartConverter *.jar
grep: io.quarkus.quarkus-vertx-http-3.2.9.Final.jar: binary file matches

Than I deleted this jar file and was able to start the application with:

cd /deployments
java -Xmx512m -cp ./resources:/etc/camel/application.properties:/etc/camel/resources:/etc/camel/resources.d/_configmaps:/etc/camel/resources.d/_secrets:/etc/camel/sources/main.yaml:dependencies/*:dependencies/app/*:dependencies/lib/boot/*:dependencies/lib/main/*:dependencies/quarkus/* io.quarkus.bootstrap.runner.QuarkusEntryPoint

Comparing to an image built for runtime 3.8.1, I don't see the vertx-http library in the dependencies library.

Now I replaced the quartz component with rest and did the same checks:

Now I deleted all my integration kits and created a single integration with quartz and camel-k-runtime 3.2.3. The integration started successfully and I don't see the vertx-http dependency.

It looks like this is related to incremental builds. If I build a rest and a quartz integration at the same time, and the rest one builds first, the quartz image comes up with some inconsistency in the jar files.

lsergio commented 3 months ago

Now I added both the rest and quartz endpoints to the same integration and if started succesfully with bother runtimes:

apiVersion: camel.apache.org/v1
kind: Integration
metadata:
  name: quartz-381
spec:  
  sources:
  - name: main.yaml
    content: |-
      - from:
          uri: quartz://ipaas/trigger?cron=0+*+*+?+*+*
          steps:
          - log:
              message: "It worked"
      - from:
          uri: rest:GET:/hello
          steps:
          - log:
              message: "It worked"
  traits:
    camel:
      runtimeVersion: 3.8.1 | 3.2.3
    cron:
      fallback: true
squakez commented 3 months ago

This last one should not work at all. I mean 3.8.1 | 3.2.3 is not a valid configuration as we do expect a semantic version (ie 3.x). I suspect it's getting some kit previously built instead.

squakez commented 3 months ago

In theory we should not mix incremental builds from various runtime versions. If that is happening, then, it's a different kind of bug we must solve.

lsergio commented 3 months ago

This last one should not work at all. I mean 3.8.1 | 3.2.3 is not a valid configuration as we do expect a semantic version (ie 3.x). I suspect it's getting some kit previously built instead.

ops, this was just to not duplicate the code here. The idea is to choose either one.

lsergio commented 3 months ago

Now I disabled incrementalImageBuild in the IntegrationPlatform and triggered many builds at the same time. The integrations are all running successfully.

lsergio commented 3 months ago

Finally, I managed to reproduce it without mixing 2 runtimes. IncrementalImageBuild is on again. Using my previous code as an example:

It will fail to start.

The same does not happen with runtime 3.8.1.

lsergio commented 3 months ago

For curiosity, I repeated the scenario from my previous comment with Camel K 2.3.3 and Camel K Runtime 3.2.3 and the Integrations ran successfully.

squakez commented 3 months ago

Then it is possibly some regression introduced lately.

lsergio commented 3 months ago

More info on this:

I compared the deployments generated by Camel K 2.3.3 and Camel K 2.4.0 and I see this difference:

While in 2.3.3 all jar files are listed explicitly in the java command: exec java -cp ./resources:/etc/camel/application.properties:/etc/camel/conf.d/_resources:/etc/camel/resources:/etc/camel/sources/main.yaml:dependencies/app/camel-k-integration-2.3.3.jar:dependencies/lib/boot/io.github.crac.org-crac-0.1.3.jar:dependencies/lib/boot/io.quarkus.quarkus-bootstrap-runner-3.2.9.Final.jar...

in 2.4.0 we use wildcards: java -Xmx512m -cp ./resources:/etc/camel/application.properties:/etc/camel/resources:/etc/camel/resources.d/_configmaps:/etc/camel/resources.d/_secrets:/etc/camel/sources/main.yaml:dependencies/*:dependencies/app/*:dependencies/lib/boot/*:dependencies/lib/main/*:dependencies/quarkus/* io.quarkus.bootstrap.runner.QuarkusEntryPoint

So maybe the wildcards are including jar files that shouldn't be included...

squakez commented 3 months ago

@lsergio good catch, that is very plausible. The problem is that when we use incremental image, we are getting all the dependencies that were used in the parent image. And now we're running with wildcards, including them in the classpath even though they are not really needed. We either need to revert (if it's even possible) the change on the jvm trait or understand how to safely use the incremental build image. I've been already wondering since some time if including dependencies which are not required is even a good approach. We discussed in the past with the original contributors a way to leverage layering for each dependency for instance. We may need to retake those discussion and see how to evolve.

This problem is also crossing the boundaries of #5539 which, at a certain point we'll need to understand how to properly support.

Another possibility would be to understand how much we'd be losing from dropping entirely the support of the incremental build image option. This is an optimization that happens at "package" time only, and the majority of the time we spend is more on the "build" time which happens anyway [1].

[1] https://camel.apache.org/camel-k/2.4.x/architecture/incremental-image.html

squakez commented 3 months ago

An alternative and even a possible final solution which we may use to keep the feature is to change instead the algorithm and make the incremental image kind of "additive" only. For instance, we can reuse only those base image that contains a subset of required dependencies but have no other "extra" dependency not required. If you think this is a good solution and you have time, you can have a look yourself as well.

lsergio commented 3 months ago

An alternative and even a possible final solution which we may use to keep the feature is to change instead the algorithm and make the incremental image kind of "additive" only. For instance, we can reuse only those base image that contains a subset of required dependencies but have no other "extra" dependency not required. If you think this is a good solution and you have time, you can have a look yourself as well.

This sounds pretty reasonable to me. Actually as I user I imagined that this was the current behavior and noticed that it wasn't while working on #5755. I'll try and have a look at this approach later this week.

squakez commented 3 months ago

Great @lsergio thanks so much for your help!