quarkusio / quarkus

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

Environment Variable KAFKA_BOOTSTRAP_SERVERS not being read by smallrye client #7791

Closed ricardozanini closed 4 years ago

ricardozanini commented 4 years ago

Hi!

Describe the bug As stated by the docs, now we have the property kafka.boostrap.servers that can be set and it should be applied as the connection property to every Kafka connector within the application.

That said, we injected the KAFKA_BOOTSTRAP_SERVERS environment variable into our containers, without explicit setting it in the application.properties file. The application fails with:

2020-03-11 10:15:33,258 ERROR [io.sma.rea.mes.imp.ConfiguredChannelFactory] (main) Unable to create the publisher or subscriber during initialization: java.util.NoSuchElementException: Cannot find attribute bootstrap.serversfor channelkogito-processinstances-events. Has been tried: mp.messaging.outgoing.kogito-processinstances-events.bootstrap.servers and mp.messaging.connector.smallrye-kafka.bootstrap.servers 

Expected behavior That the environment variable KAFKA_BOOTSTRAP_SERVERS is being read by the application even if it's not defined in the application.properties file.

Actual behavior The variable is ignored.

To Reproduce Steps to reproduce the behavior:

  1. Create an example application that consumes a Kafka topic
  2. Clear any the properties with the suffix *.bootstrap.servers in the application.properties file
  3. Create an env var named KAFKA_BOOTSTRAP_SERVERS and set it with the actual location of the Kafka server (you can use Spotify's docker image in a different forward port for this test)
  4. The application will fail to connect

Configuration

# Add your application.properties here, if applicable.
quarkus.infinispan-client.server-list=localhost:11222
quarkus.http.cors=true

mp.messaging.outgoing.kogito-processinstances-events.bootstrap.servers=
mp.messaging.outgoing.kogito-processinstances-events.connector=smallrye-kafka
mp.messaging.outgoing.kogito-processinstances-events.topic=kogito-processinstances-events
mp.messaging.outgoing.kogito-processinstances-events.value.serializer=org.apache.kafka.common.serialization.StringSerializer

mp.messaging.outgoing.kogito-usertaskinstances-events.bootstrap.servers=
mp.messaging.outgoing.kogito-usertaskinstances-events.connector=smallrye-kafka
mp.messaging.outgoing.kogito-usertaskinstances-events.topic=kogito-usertaskinstances-events
mp.messaging.outgoing.kogito-usertaskinstances-events.value.serializer=org.apache.kafka.common.serialization.StringSerializer

Environment (please complete the following information):

Additional context We already had a similar problem before: https://github.com/quarkusio/quarkus/issues/5708 This one might be related: https://github.com/quarkusio/quarkus/issues/4571

Many thanks!

geoand commented 4 years ago

@ricardozanini Hi,

Would it be possible to provide a minimum reproducer (no Infinispan for example :))? I could of course do it myself, but I prefer to have it ready made to avoid me making any mistakes and loosing time in back and forths just to make sure I have the proper reproducer :)

geoand commented 4 years ago

FWIW, I was unable to reproduce this issue in our Kafka integration test (https://github.com/quarkusio/quarkus/blob/master/integration-tests/kafka/pom.xml)

I basically clear out kafka.bootstrap.servers from application.properties and ran the test like so:

 KAFKA_BOOTSTRAP_SERVERS=localhost:19092 mvn clean test

Everything worked as expected.

Note that this was with Quarkus master.

@ricardozanini any chance you can check your project with 1.3.0.CR2?

ricardozanini commented 4 years ago

@geoand sure thing, will work on a simpler reproducer and will get back to you soon.

ricardozanini commented 4 years ago

@geoand I can confirm that this is not working on 1.3.0.CR2 either and I think I've found the problem. I've disabled the Infinispan integration. Steps to reproduce:

  1. Start kafka instance with: docker run -p 2181:2181 -p 9093:9092 --env ADVERTISED_PORT=9092 spotify/kafka
  2. Git clone: https://github.com/kiegroup/kogito-examples
  3. Install kogito-examples BOM: mvn clean install -N
  4. Compile jbpm-quarkus-example from the same repo enabling Kafka integration: mvn clean install -DskipTests -Pevents
  5. Run: KAFKA_BOOTSTRAP_SERVERS=localhost:9093 java -jar target/jbpm-quarkus-example-8.0.0-SNAPSHOT-runner.jar

The application.properties file contains:

mp.messaging.outgoing.kogito-processinstances-events.bootstrap.servers=
(...)
mp.messaging.outgoing.kogito-usertaskinstances-events.bootstrap.servers=

If we delete them it works. I wonder if is this the expected behavior? I mean, since those are empty, the client could take into account the env property instead as a fallback.

geoand commented 4 years ago

Well, I don't think it should fallback to KAFKA_BOOTSTRAP_SERVERS but I am not sure what the expected behavior is. @cescoffier your thoughts?

radtriste commented 4 years ago

I will add that in native mode, QUARKUS_KAFKA_BOOTSTRAP_SERVERS is not handled whereas it is in non-native...

cescoffier commented 4 years ago

You can setup the broker location using: kafka.bootstrap.servers.

ricardozanini commented 4 years ago

@cescoffier that's the point. We are setting the broker location with KAFKA_BOOTSTRAP_SERVERS environment variable, which is not getting read in runtime if the properties file has mp.messaging.*.bootstrap.servers properties set.

We should get rid of them and use only kafka.bootstrap.servers? What's the priority?

cescoffier commented 4 years ago

Sorry, just coming back to this issue.

The priority is channel first, then the global value (because you can have multiple Kafka brokers). So, unfortunately, in your case, you cannot have both.

What we can do is to check if the value is empty and in that case use the global one. That would fix your issue.

I've created https://github.com/smallrye/smallrye-reactive-messaging/issues/649 to track this. It's an easy fix.

ricardozanini commented 4 years ago

Thank you @cescoffier!

cescoffier commented 4 years ago

Actually, I believe it's already fixed.

I've the following properties:

# Configure the Kafka sink (we write to it)
mp.messaging.outgoing.generated-price.connector=smallrye-kafka
mp.messaging.outgoing.generated-price.bootstrap.servers=
mp.messaging.outgoing.generated-price.topic=prices
mp.messaging.outgoing.generated-price.value.serializer=org.apache.kafka.common.serialization.IntegerSerializer

# Configure the Kafka source (we read from it)
mp.messaging.incoming.prices.connector=smallrye-kafka
mp.messaging.incoming.prices.topic=prices
mp.messaging.incoming.prices.value.deserializer=org.apache.kafka.common.serialization.IntegerDeserializer

The first channel uses a "blank" bootstrap.servers, the second channel omits it completely.

My broker runs on 9093 (while the default is 9092).

I tried:

export KAFKA_BOOTSTRAP_SERVERS=localhost:9093
mvn compile quarkus:dev
mvn compile quarkus:dev -Dkafka.boostrap.servers=localhost:9093
export KAFKA_BOOTSTRAP_SERVERS=localhost:9093
java -jar ...

All these configurations work.

Can you recheck with the latest release? Maybe I miss something obvious.

r00ta commented 4 years ago

Actually, I believe it's already fixed.

I've the following properties:

# Configure the Kafka sink (we write to it)
mp.messaging.outgoing.generated-price.connector=smallrye-kafka
mp.messaging.outgoing.generated-price.bootstrap.servers=
mp.messaging.outgoing.generated-price.topic=prices
mp.messaging.outgoing.generated-price.value.serializer=org.apache.kafka.common.serialization.IntegerSerializer

# Configure the Kafka source (we read from it)
mp.messaging.incoming.prices.connector=smallrye-kafka
mp.messaging.incoming.prices.topic=prices
mp.messaging.incoming.prices.value.deserializer=org.apache.kafka.common.serialization.IntegerDeserializer

The first channel uses a "blank" bootstrap.servers, the second channel omits it completely.

My broker runs on 9093 (while the default is 9092).

I tried:

export KAFKA_BOOTSTRAP_SERVERS=localhost:9093
mvn compile quarkus:dev
mvn compile quarkus:dev -Dkafka.boostrap.servers=localhost:9093
export KAFKA_BOOTSTRAP_SERVERS=localhost:9093
java -jar ...

All these configurations work.

Can you recheck with the latest release? Maybe I miss something obvious.

Hi @cescoffier , we've just upgraded our services to Quarkus 1.6 and it works fine. Thank you very much!

ricardozanini commented 4 years ago

Thanks @r00ta and @cescoffier, I'm closing this one for now. :)