quarkusio / quarkus

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

When building images with jib the fast-jar-lib layer is always changed #39130

Closed Malandril closed 8 months ago

Malandril commented 8 months ago

Describe the bug

When using jib to build an image for your project the layer containing the libraries is always changed, which can means, that pushes can be a lot longer than necessary, when the project has a lot of libraries. I could only confirm it with the Jib extension, the Docker seems to keep the library layer between builds.

You can use a tool like dive to inspect the layers, and see that the layer digest is always different for the fast-jar-lib

Expected behavior

When building an image after a change in the source code, the fast-jar-lib layer should not be different if the libraries didn't change.

Actual behavior

Each time you rebuild an image, even with no changes, for example after a clean, the library layer is different.

How to Reproduce?

You can create a reproducer from the quarkus cli

quarkus create app  --gradle-kotlin-dsl  -x 'jib,quarkus-resteasy-reactive-jackson' quarkus-jib -P 3.8.1
cd quarkus-jib
./gradlew imageBuild

Then if you run again the build, by simply changing a property, or simply running ./gradlew clean imageBuild you can inspect both generated images, and see that the fast-jar-lib is different in each despite no changes in the libraries

Output of uname -a or ver

No response

Output of java -version

openjdk version "21" 2023-09-19 LTS

Quarkus version or git rev

3.8.1

Build tool (ie. output of mvnw --version or gradlew --version)

Gradle 8.5

Additional information

A dive screenshot of two successive ./gradlew imageBuild showing a diffrence in the lib layer

image

quarkus-bot[bot] commented 8 months ago

/cc @geoand (jib,kotlin)

geoand commented 8 months ago

I can confirm this happens, although at the moment, I am unsure how to resolve it

gsmet commented 8 months ago

Not exactly sure how the hash is built but I would start with the following patch:

diff --git a/core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/JarResultBuildStep.java b/core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/JarResultBuildStep.java
index 4266d04727a..60cbc6cf60a 100644
--- a/core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/JarResultBuildStep.java
+++ b/core/deployment/src/main/java/io/quarkus/deployment/pkg/steps/JarResultBuildStep.java
@@ -926,7 +926,8 @@ private void copyDependency(Set<ArtifactKey> parentFirstArtifacts, OutputTargetB
                     }
                 }
                 if (removedFromThisArchive.isEmpty()) {
-                    Files.copy(resolvedDep, targetPath, StandardCopyOption.REPLACE_EXISTING);
+                    Files.copy(resolvedDep, targetPath, StandardCopyOption.REPLACE_EXISTING,
+                            StandardCopyOption.COPY_ATTRIBUTES);
                 } else {
                     //we have removed classes, we need to handle them correctly
                     filterZipFile(resolvedDep, targetPath, removedFromThisArchive);
gsmet commented 8 months ago

And another thing I noticed is that in the legacy thin jar format, we copied the modified jars separately. From a quick look, it seems that the transformed jars are also in lib/ for fast jar (at least the ones for which we remove entries) and I'm not entirely sure these transformations will produce the exact same content. Especially since we create a new Zip file.

geoand commented 8 months ago

From a quick look, it seems that the transformed jars are also in lib/ for fast jar (at least the ones for which we remove entries)

Not true :)

geoand commented 8 months ago

Not exactly sure how the hash is built but I would start with the following patch:

Definitely worth a try, especially since my current experiments did not change anything

gsmet commented 8 months ago

Yeah I was looking at the exact same thing and your changes make a lot of sense.

(And should make mine useless since you override the modification time anyway).

That is very odd. What is interesting IMO is that even with your patch, it seems the lib/boot layer has the same hash. So somehow we are doing something right. The problem starts with the lib/main layer.

geoand commented 8 months ago

(And should make mine useless since you override the modification time anyway).

Definitely.

The problem starts with the lib/main layer

Yeah, my quick (around 30 minutes) investigation was not able to uncover the problem :(

gsmet commented 8 months ago

FWIW, I maintain what I said about the jars for which we remove entries: they are put in lib/main but I checked the md5sum and it seems OK.

geoand commented 8 months ago

Hm, that's odd.

The transformed bytecode should be in quarkus-app/transformed-bytecode.jar, so I am not sure what we're doing to the original jars

gsmet commented 8 months ago

I think I have something working. Let me give it a few tests to see if it's just luck or not.

gsmet commented 8 months ago

Yeah so it seems to work for me but I have no idea why your changes didn't fix the issue too as it should do the same thing in the end.

Let me clean my patch so that you can test it on your side too.

geoand commented 8 months ago

Sure thing

gsmet commented 8 months ago

@geoand see https://github.com/quarkusio/quarkus/pull/39147

What surprises me a bit is that in the end it does something very similar to what you did, except you enforced a specific timestamp.

Note that I was testing it with Maven and the Hibernate Validator IT instead of the original reproducer. The Hibernate Validator IT is interesting as we actually remove an entry from the H2 jar, which triggers what I was mentioning.

gsmet commented 8 months ago

With this patch, I did ~6 builds and I got the exact same digest for the lib/main layer.

Malandril commented 8 months ago

I cloned #39147 and tried a few builds, it seems to be working !

Nice work !

geoand commented 8 months ago

Thanks for checking!