Skip to content
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

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

Closed
nwholloway opened this issue Jul 8, 2024 · 7 comments
Closed

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

nwholloway opened this issue Jul 8, 2024 · 7 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: documentation A documentation update

Comments

@nwholloway
Copy link

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
Copy link
Author

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.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 8, 2024
@wilkinsona
Copy link
Member

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
Copy link
Contributor

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.

@scottfrederick scottfrederick added type: documentation A documentation update and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 8, 2024
@scottfrederick scottfrederick added this to the 3.2.x milestone Jul 8, 2024
@nwholloway
Copy link
Author

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.

@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jul 9, 2024
@philwebb philwebb removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Jul 10, 2024
@scottfrederick
Copy link
Contributor

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

anthonydahanne added a commit to paketo-buildpacks/spring-boot that referenced this issue Jul 10, 2024
* it could break apps that had AOT'ed some migration library like flyway during build, which would need some extra code (java) to not automatically use them at runtime (and training run...)
see: spring-projects/spring-boot#41348
* after confirmation we won't ever re introduce AOT support along with CDS in Spring Boot buildpack, we'll deprecate the feature
@sdeleuze
Copy link
Contributor

sdeleuze commented Jul 11, 2024

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
Copy link
Member

snicoll commented Jul 11, 2024

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.

@wilkinsona wilkinsona closed this as not planned Won't fix, can't repro, duplicate, stale Jul 11, 2024
@wilkinsona wilkinsona removed this from the 3.2.x milestone Jul 11, 2024
@wilkinsona wilkinsona added the status: declined A suggestion or change that we don't feel we should currently apply label Jul 11, 2024
sdeleuze added a commit to sdeleuze/spring-boot that referenced this issue Jul 12, 2024
As explained in spring-projectsgh-41348, the BP_SPRING_AOT_ENABLED flag should
not be promoted as it can't work by design with our current
support when combined with CDS for various use cases and provides
little added value as the same behavior can be achieved by adding
-Dspring.aot.enabled=true to JAVA_TOOL_OPTIONS and
CDS_TRAINING_JAVA_TOOL_OPTIONS.
wilkinsona pushed a commit that referenced this issue Jul 12, 2024
As explained in gh-41348, the BP_SPRING_AOT_ENABLED flag should
not be promoted as it can't work by design with our current
support when combined with CDS for various use cases and provides
little added value as the same behavior can be achieved by adding
-Dspring.aot.enabled=true to JAVA_TOOL_OPTIONS and
CDS_TRAINING_JAVA_TOOL_OPTIONS.

See gh-41464
anthonydahanne added a commit to paketo-buildpacks/spring-boot that referenced this issue Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

7 participants