spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
73.81k stars 40.37k forks source link

Build of OCI image fails with both AOT and CDS when Flyway used #41348

Closed nwholloway closed 1 week ago

nwholloway commented 1 week ago

Tested with Spring Boot 3.3.1

I was investigating creating an OCI image of a Spring Boot application using both AOT and CDS to create a container with reduced start up time with a JVM, without the additional complexity of building a native image.

This was fine with a do-nothing application using Spring Web, but it fails when you have Flyway as a dependency.

The training run to build the CDS archive does not honour the disabling of Flyway migration (presumably due to being built into the AOT generated classes), so you get the following build error:

[creator]     org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'flywayInitializer': Unable to obtain connection from database: Connection to localhost:5432 refused. Check that the hostname and port are correct and that the postmaster is accepting TCP/IP connections.
[creator]     --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
[creator]     SQL State  : 08001
[creator]     Error Code : 0
[creator]     Message    : Connection to localhost:5432 refused. Check that the hostname and port are correct and that the postmaster is accepting TCP/IP connections.

It would be good to have the dual benefits of AOT processing and CDS training when building an OCI image.

nwholloway commented 1 week ago

I have created a repository that demonstrates the problem at https://github.com/nwholloway/spring-boot-demo-41348

The build task uses bootBuildImage to create an OCI image.

It uses Gradle properties to enable CDS and/or AOT for the build. Enable CDS using -Pcds and enable AOT using -Paot.

With both AOT and CDS, -Pcds -Paot, you get the above error.

wilkinsona commented 1 week ago

Thanks very much for the sample, @nwholloway. You can work around the current limitation using a custom migration strategy:

@Bean
FlywayMigrationStrategy flywayMigrationStrategy() {
    return (flyway) -> {
        String flywayEnabled = System.getProperty("spring.flyway.enabled");
        if (flywayEnabled == null || Boolean.valueOf(flywayEnabled)) {
            flyway.migrate();
        }
    };
}

With this bean in place, ./gradlew bootBuildImage -Paot -Pcds should succeed and then docker compose up should successfully start the app and migrate the database.

scottfrederick commented 1 week ago

Setting BP_SPRING_AOT_ENABLED along with BP_JVM_CDS_ENABLED will cause AOT to be enabled during the CDS training run, in addition to AOT being enabled by default in the run command for the generated image.

Setting BPL_SPRING_AOT_ENABLED instead of BP_SPRING_AOT_ENABLED will give the benefit of enabling AOT by default in the image's run command, without enabling AT during the CDS training run. This might be closer to what you want.

When using bootBuildImage in this way, the behavior is under the control of the Paketo Buildpack for Spring Boot, not Spring Boot itself. Spring Boot includes some documentation for discoverability of the Paketo buildpack features, and I think that documentation (along with some of the extended Spring portfolio documentation) would benefit from some clarification about this combination.

nwholloway commented 1 week ago

Thank you @wilkinsona for the custom migration strategy, and @scottfrederick for the suggestion of not using AOT during CDS training.

I think the FlywayMigrationStrategy approach is preferable to not using AOT during the CDS training run, as the CDS archive will contain the AOT generated classes that will be used at runtime, rather than the Spring auto-configuration classes that won't be.

scottfrederick commented 1 week ago

@sdeleuze Is this scenario something that could be covered in the lifecycle smoke test documentation?

sdeleuze commented 1 week ago

After discussion with @snicoll and the Buildpacks team and as mentioned in the PR linked above, I think the issue raised here is serious and "by design" enough to not promote this AOT Buildpacks flag anymore as for some use cases, it won't work due to conflicts between training run configuration and AOT configuration precomputation.

This flag was not really dedicated feature, more a shortcut to add -Dspring.aot.enabled=true to JAVA_TOOL_OPTIONS and CDS_TRAINING_JAVA_TOOL_OPTIONS. If users want to combine both, they can still do that with such slightly more verbose configuration, which is IMO the right call for a feature not supported for all use cases.

I am not sure we should add for now specific documentation in https://github.com/spring-projects/spring-lifecycle-smoke-tests as this is not a use case fully supported, but we can discuss that if you strongly think we should.

@nwholloway Thanks for catching that shortly after our GA.

snicoll commented 1 week ago

For those who would like to explore that route, it does require two rounds of AOT processing unfortunately. I haven't tested it but something like:

  1. Run AOT processing with the CDS_TRAINING_JAVA_TOOL_OPTIONS being set on the AOT processor so that whatever should be tuned is tuned accordingly
  2. Perform the training run on the output of 1
  3. Run AOT processing again, without the CDS_TRAINING_JAVA_TOOL_OPTIONS
  4. Package the application generated by 3 and the cache generated by 2

There are a lot of gotchas doing this yourself that buildpacks shield you from so definitely more involved. For now, I'd go with only CDS.