Skip to content

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Aug 27, 2025

This is needed to unclutter the application logs when Docker is not available

This is needed in order to unclutter the application logs
when Docker is not available

Closes: quarkusio#49716
@geoand geoand requested a review from gsmet August 27, 2025 07:16
@gsmet
Copy link
Member

gsmet commented Aug 27, 2025

Do we have an understanding as to why we try to detect Docker while we don't need it? Because apart from the verbose logging, it actually takes time to do this check.

@geoand
Copy link
Contributor Author

geoand commented Aug 27, 2025

Great question. It happens because of

boolean isContainerRuntimeAvailable = dockerStatusBuildItem.isContainerRuntimeAvailable();

@holly-cummins
Copy link
Contributor

It seems like something changed in 3.25, though, and it would be useful to know what it was. Did we start checking the build item status more eagerly? I wonder if #49716 is another manifestation of #49118? The fix for #49118, #49309, clearly wouldn't fix #49716, but there might be a common root cause that we should be worried about.

Even if something changed, being silent is probably a good idea... but I worry, like @gsmet says, that being silent for the check could mask a performance regression that crept in in 3.25.

@geoand
Copy link
Contributor Author

geoand commented Aug 27, 2025

I'm more than happy to let people figure that out, I'm just working on clearing my backlog at this point :)

@geoand
Copy link
Contributor Author

geoand commented Aug 27, 2025

I'll also note that, regardless of whether this masks something else, this change still makes sense. There is no reason not to have all io.quarkus.deployment.IsContainerRuntimeWorking$Strategy implementations properly implement the silent behavior.
Without this change, only the TestContainersStrategy implementation used silent, and didn't even do it correctly.

Copy link

quarkus-bot bot commented Aug 27, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 9ebaddf.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ Gradle Tests - JDK 17

📦 integration-tests/gradle

io.quarkus.gradle.BuildConfigurationTest.buildNoOverride - History

  • Multiple Failures (1 failure) -- failure 1 -- [sub project 'without-configuration', package type 'fast-jar'] Expecting path: - org.assertj.core.error.AssertJMultipleFailuresError
org.assertj.core.error.AssertJMultipleFailuresError: 

Multiple Failures (1 failure)
-- failure 1 --
[sub project 'without-configuration', package type 'fast-jar'] 
Expecting path:
  /home/runner/_work/quarkus/quarkus/integration-tests/gradle/target/classes/build-configuration/without-configuration/build/quarkus-app/quarkus-run.jar
to exist (symbolic links were followed).
  • Multiple Failures (1 failure) -- failure 1 -- [sub project 'without-configuration', package type 'fast-jar'] Expecting path: - org.assertj.core.error.AssertJMultipleFailuresError
org.assertj.core.error.AssertJMultipleFailuresError: 

Multiple Failures (1 failure)
-- failure 1 --
[sub project 'without-configuration', package type 'fast-jar'] 
Expecting path:
  /home/runner/_work/quarkus/quarkus/integration-tests/gradle/target/classes/build-configuration/without-configuration/build/quarkus-app/quarkus-run.jar
to exist (symbolic links were followed).

@gsmet
Copy link
Member

gsmet commented Aug 28, 2025

@holly-cummins so as @geoand pointed it out, there's at least this line that is problematic:

boolean isContainerRuntimeAvailable = dockerStatusBuildItem.isContainerRuntimeAvailable();
.

We are checking if a container is available far too early to include the info in the definitions. We should collect first if Dev Services are required and only then check if the container runtime is available.

@gsmet
Copy link
Member

gsmet commented Aug 29, 2025

@holly-cummins will you handle this one ^? Because I'm not sure I have a good enough understanding of the new Dev Services infra to drive this puppy home. IMHO, we should fix this behavior before we ship the LTS (i.e. soon!).

@holly-cummins
Copy link
Contributor

Will do! I'm hoping it's just a question of moving the relevant line further down in the file, but I guess the tests will tell. :) I'll raise another WI so we can merge @geoand's fix now and then also do the performance/not-doing-needless-work fix: #49789

@holly-cummins
Copy link
Contributor

My plan did not survive contact with reality. It will probably be Monday ...

@gsmet gsmet merged commit 25ceb27 into quarkusio:main Aug 29, 2025
57 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.28 - main milestone Aug 29, 2025
@gsmet gsmet modified the milestones: 3.28 - main, 3.26.2 Sep 2, 2025
@geoand geoand deleted the #49716 branch September 3, 2025 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dev services starting even though quarkus.datasource.jdbc.url is set
3 participants