-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FAI-681: Integrate TrustyService with Quarkus DevServices services #21776
Conversation
@@ -70,6 +71,7 @@ public QuarkusPostgreSQLContainer(Optional<String> imageName, OptionalInt fixedE | |||
.asCompatibleSubstituteFor(DockerImageName.parse(PostgreSQLContainer.IMAGE))); | |||
this.fixedExposedPort = fixedExposedPort; | |||
this.useSharedNetwork = useSharedNetwork; | |||
withNetwork(Network.SHARED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PostgreSQL (and I suspect others) are not joining the default Docker shared network setup by TestContainers.
This, I suspect should be the case, since other Quarkus services join it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This need to be an extra licit user configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @cescoffier IIUC you're proposing to rollback my change here and add a similar mechanism to KafkaBuildTimeConfig
where the "user" of the PostgreSQL DevService can decide what Docker network to have the service join? Happy to do so, I just want to be clearer on your requirement. I'd also love to understand your reasoning why some DevServices join TestContainers SHARED
network already and others do not; so I can avoid making similar mistakes in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This need to be an extra licit user configuration.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, none of the dev services are added to the SHARED
network by default. The reason this was introduces was to support the use case of having @QuarkusIntegrationTest
+ DevServices
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some are (mainly around messaging) see here as an example. Anyway, no problem, I can add the User configuration instead once @cescoffier confirms my understanding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said, adding them to the SHARED
network by default should not cause any problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cescoffier @geoand I am thinking of adding a new property to io.quarkus.datasource.runtime.DevServicesBuildTimeConfig
called useTestContainerSharedNetwork
. This would be extracted in io.quarkus.datasource.deployment.devservices.DevServicesDatasourceProcessor.startDevDb(..)
and passed to devDbProvider.startDatabase(...)
along with the other properties; making necessary changes elsewhere to pass to the various DataSource container implementations. WDYT? A bit more extensive than me simply bolting something on the my PostgreSQL use-case; but at least it'd bring consistency to the different DataSources supported by DevServices.
To be honest I'd like to see a refactor of Quarkus' useSharedNetwork
flag - since its interpretation is not analogous with TestContainers
shared network and it really drives how services are exposed outside the Docker network. Anyhow.. that's not a discussion for today. That said it is internal and I only found it confusing as I was trying to work out my issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT?
Sounds good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I've added the change as a new commit to simplify (or so I hope) review.
Some containers that jump on TestContainer
's shared network remain:-
./extensions/smallrye-reactive-messaging-amqp/deployment/src/main/java/io/quarkus/smallrye/reactivemessaging/amqp/deployment/AmqpDevServicesProcessor.java:294: withNetwork(Network.SHARED);
./extensions/kafka-client/deployment/src/main/java/io/quarkus/kafka/client/deployment/DevServicesKafkaProcessor.java:353: withNetwork(Network.SHARED);
./extensions/vault/deployment/src/main/java/io/quarkus/vault/deployment/DevServicesVaultProcessor.java:148: vaultContainer.withNetwork(Network.SHARED);
./extensions/vault/deployment/src/main/java/io/quarkus/vault/deployment/DevServicesVaultProcessor.java:209: withNetwork(Network.SHARED);
./extensions/apicurio-registry-avro/deployment/src/main/java/io/quarkus/apicurio/registry/avro/DevServicesApicurioRegistryProcessor.java:267: withNetwork(Network.SHARED);
./extensions/smallrye-reactive-messaging-rabbitmq/deployment/src/main/java/io/quarkus/smallrye/reactivemessaging/rabbitmq/deployment/RabbitMQDevServicesProcessor.java:374: withNetwork(Network.SHARED);
Those I've "fixed":-
./extensions/devservices/mssql/src/main/java/io/quarkus/devservices/mssql/deployment/MSSQLDevServicesProcessor.java:78: withNetwork(Network.SHARED);
./extensions/devservices/oracle/src/main/java/io/quarkus/devservices/oracle/deployment/OracleDevServicesProcessor.java:82: withNetwork(Network.SHARED);
./extensions/devservices/mariadb/src/main/java/io/quarkus/devservices/mariadb/deployment/MariaDBDevServicesProcessor.java:77: withNetwork(Network.SHARED);
./extensions/devservices/db2/src/main/java/io/quarkus/devservices/db2/deployment/DB2DevServicesProcessor.java:79: withNetwork(Network.SHARED);
./extensions/devservices/postgresql/src/main/java/io/quarkus/devservices/postgresql/deployment/PostgresqlDevServicesProcessor.java:77: withNetwork(Network.SHARED);
./extensions/devservices/mysql/src/main/java/io/quarkus/devservices/mysql/deployment/MySQLDevServicesProcessor.java:76: withNetwork(Network.SHARED);
Some integration tests set it up too, but they're of less (read as "no") interest.
./integration-tests/kafka-oauth-keycloak/src/test/java/io/quarkus/it/kafka/containers/KafkaContainer.java:60: withNetwork(Network.SHARED);
./integration-tests/kafka-oauth-keycloak/src/test/java/io/quarkus/it/kafka/containers/KeycloakContainer.java:24: withNetwork(Network.SHARED);
./integration-tests/kafka-streams/src/test/java/io/quarkus/it/kafka/streams/KafkaSSLTestResource.java:93: super.withNetwork(Network.SHARED);
./integration-tests/kafka-ssl/src/test/java/io/quarkus/it/kafka/KafkaSSLTestResource.java:91: super.withNetwork(Network.SHARED);
./integration-tests/kafka-sasl-elytron/src/test/java/io/quarkus/it/kafka/containers/KafkaContainer.java:63: withNetwork(Network.SHARED);
./integration-tests/kafka-sasl-elytron/src/test/java/io/quarkus/it/kafka/containers/KerberosContainer.java:27: withNetwork(Network.SHARED);
./integration-tests/kafka-sasl/src/test/java/io/quarkus/it/kafka/KafkaSASLTestResource.java:83: super.withNetwork(Network.SHARED);
@@ -381,8 +377,10 @@ protected void containerIsStarting(InspectContainerResponse containerInfo, boole | |||
command += "/usr/bin/rpk redpanda start --check=false --node-id 0 --smp 1 "; | |||
command += "--memory 1G --overprovisioned --reserve-memory 0M "; | |||
command += "--kafka-addr PLAINTEXT://0.0.0.0:29092,OUTSIDE://0.0.0.0:9092 "; | |||
command += String.format("--advertise-kafka-addr PLAINTEXT://%s:29092,OUTSIDE://%s:%d", getHostToUse(), | |||
getHostToUse(), getPortToUse()); | |||
command += String.format("--advertise-kafka-addr PLAINTEXT://%s:29092,OUTSIDE://%s:%d ", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I can gather (I am not a Kafka expert by any stretch of anyone's imagination) we always need to define a listener for the "local docker network" and hence need the host name that will be available on the bridge. See https://rmoff.net/2018/08/02/kafka-listeners-explained/ that guided my change (and allows my environment to work).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ozangunalp cab you review this part? You did the network configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems correct, the broker will be accessible on hostname:29092
from inside the container's network.
And +1 for the article, it's a go-to for Kafka listeners config.
command += String.format("--advertise-kafka-addr PLAINTEXT://%s:29092,OUTSIDE://%s:%d", getHostToUse(), | ||
getHostToUse(), getPortToUse()); | ||
command += String.format("--advertise-kafka-addr PLAINTEXT://%s:29092,OUTSIDE://%s:%d ", | ||
hostName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both PLAINTEXT
and OUTSIDE
were previously using localhost
.
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 71c3fba
Failures⚙️ JVM Tests - JDK 17 #- Failing: extensions/reactive-pg-client/deployment
! Skipped: extensions/hibernate-reactive/deployment extensions/panache/hibernate-reactive-panache-common/deployment extensions/panache/hibernate-reactive-panache/deployment and 7 more 📦 extensions/reactive-pg-client/deployment✖ |
@cescoffier I believe the issues have been resolved. This is ready for your review. IDK how to re-trigger the JDK17 builds. The failures seem unrelated to my PR.. but happy to have a look if needed. |
All checks are green 👍 |
...tasource/runtime/src/main/java/io/quarkus/datasource/runtime/DevServicesBuildTimeConfig.java
Outdated
Show resolved
Hide resolved
Please squash the commits if it makes sense (the last commit for example doesn't need to be a separate one) |
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building b1abd45
Full information is available in the Build summary check run. Failures⚙️ Gradle Tests - JDK 11 Windows #- Failing: integration-tests/gradle
📦 integration-tests/gradle✖
|
@geoand Done. |
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 98b2ffd
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 11 #- Failing: extensions/reactive-pg-client/deployment
! Skipped: extensions/hibernate-reactive/deployment extensions/panache/hibernate-reactive-panache-common/deployment extensions/panache/hibernate-reactive-panache/deployment and 7 more 📦 extensions/reactive-pg-client/deployment✖ ⚙️ Native Tests - Messaging1 #- Failing: integration-tests/kafka
📦 integration-tests/kafka✖
|
@geoand Could you please re-start the CI jobs? The build prior to the new property's description change was green. |
Merged, thanks for this! |
This contains the changes I've had to implement to have the following running:-
kogito
application running onlocalhost:8080
kogito
application starting a Docker container for a service it needsTrustyService
.kogito
application connecting with and using Quarkus' DevServices Redpanda (Kafka) instance.TrustyService
connecting with and using Quarkus' DevServices Redpanda (Kafka) instance.TrustyService
connecting with and using Quarkus' DevServices PostgreSQL instance.So, by simply running
mvn compile quarkus:dev
I get a fully operational environment. Great.It is not using what Quarkus defines as a "shared network" that is used for Integration Testing.