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

Fix inconsistent image build issue #1010

Merged

Conversation

MonkeyCanCode
Copy link
Contributor

This PR fixed inconsistent image build issue after a successful build is made already. It contains two changes:

  1. add clean to the build command
  2. disable gradle build cache when building image

Here are the test:

Happy path (first time building)

➜  polaris git:(main) docker images
REPOSITORY   TAG       IMAGE ID   CREATED   SIZE
➜  polaris git:(main) ./gradlew :polaris-quarkus-server:assemble :polaris-quarkus-admin:assemble \
    -Dquarkus.container-image.build=true \
    -PeclipseLinkDeps=org.postgresql:postgresql:42.7.4 \
...
BUILD SUCCESSFUL in 38s
115 actionable tasks: 98 executed, 17 up-to-date
➜  polaris git:(main) docker images
REPOSITORY                  TAG                         IMAGE ID       CREATED          SIZE
apache/polaris-admin-tool   1.0.0-incubating-SNAPSHOT   4162495a3d93   29 seconds ago   535MB
apache/polaris-admin-tool   latest                      4162495a3d93   29 seconds ago   535MB
apache/polaris              1.0.0-incubating-SNAPSHOT   1d48a4908017   29 seconds ago   633MB
apache/polaris              latest                      1d48a4908017   29 seconds ago   633MB

Un-happy paths

users removed the images then reran the same command to get images back

➜  polaris git:(main) ./gradlew :polaris-quarkus-server:assemble :polaris-quarkus-admin:assemble \
    -Dquarkus.container-image.build=true \
    -PeclipseLinkDeps=org.postgresql:postgresql:42.7.4
...
BUILD SUCCESSFUL in 2s
96 actionable tasks: 96 up-to-date
➜  polaris git:(main) docker images
REPOSITORY   TAG       IMAGE ID   CREATED   SIZE

users removed the images then reran the same command to get images back with clean option

➜  polaris git:(main) ./gradlew clean :polaris-quarkus-server:assemble :polaris-quarkus-admin:assemble \
    -Dquarkus.container-image.build=true \
    -PeclipseLinkDeps=org.postgresql:postgresql:42.7.4
...
BUILD SUCCESSFUL in 13s
115 actionable tasks: 72 executed, 26 from cache, 17 up-to-date
➜  polaris git:(main) docker images
REPOSITORY   TAG       IMAGE ID   CREATED   SIZE
➜  polaris git:(main)

users removed the images then reran the same command to get images back with no-cache option

➜  polaris git:(main) ./gradlew :polaris-quarkus-server:assemble :polaris-quarkus-admin:assemble \
    -Dquarkus.container-image.build=true \
    -PeclipseLinkDeps=org.postgresql:postgresql:42.7.4 \
    --no-build-cache
...
BUILD SUCCESSFUL in 2s
96 actionable tasks: 96 up-to-date
➜  polaris git:(main) docker images
REPOSITORY   TAG       IMAGE ID   CREATED   SIZE

With the purposed change:

➜  polaris git:(main) ./gradlew clean :polaris-quarkus-server:assemble :polaris-quarkus-admin:assemble \
    -Dquarkus.container-image.build=true \
    -PeclipseLinkDeps=org.postgresql:postgresql:42.7.4 \
    --no-build-cache
...
BUILD SUCCESSFUL in 32s
115 actionable tasks: 98 executed, 17 up-to-date
➜  polaris git:(main) docker images
REPOSITORY                  TAG                         IMAGE ID       CREATED          SIZE
apache/polaris-admin-tool   1.0.0-incubating-SNAPSHOT   e771063a0c2b   10 seconds ago   535MB
apache/polaris-admin-tool   latest                      e771063a0c2b   10 seconds ago   535MB
apache/polaris              1.0.0-incubating-SNAPSHOT   78de7f511dcd   16 seconds ago   633MB
apache/polaris              latest                      78de7f511dcd   16 seconds ago   633MB

@adutra
Copy link
Contributor

adutra commented Feb 17, 2025

The changes look good to me, but it would be good to change also the following places:

regtests/README.md
quarkus/server/README.md

site/content/in-dev/unreleased/metastores.md
site/content/in-dev/unreleased/quickstart.md

getting-started/eclipselink/README.md
getting-started/spark/README.md
getting-started/telemetry/README.md
getting-started/trino/README.md

@MonkeyCanCode
Copy link
Contributor Author

The changes look good to me, but it would be good to change also the following places:

regtests/README.md
quarkus/server/README.md

site/content/in-dev/unreleased/metastores.md
site/content/in-dev/unreleased/quickstart.md

getting-started/eclipselink/README.md
getting-started/spark/README.md
getting-started/telemetry/README.md
getting-started/trino/README.md

Good catch. Will do it today.

@MonkeyCanCode
Copy link
Contributor Author

The changes look good to me, but it would be good to change also the following places:

regtests/README.md
quarkus/server/README.md

site/content/in-dev/unreleased/metastores.md
site/content/in-dev/unreleased/quickstart.md

getting-started/eclipselink/README.md
getting-started/spark/README.md
getting-started/telemetry/README.md
getting-started/trino/README.md

Good catch. Will do it today.

@adutra found couple more places with a quick grep commands. All of the image build commands should have this fix now (except github action as it is stateless)

Copy link
Member

@jbonofre jbonofre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, lgtm. Thanks!

@MonkeyCanCode MonkeyCanCode merged commit d81eadd into apache:main Feb 18, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants