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

Use local build artifacts for docker build #268

Closed
wants to merge 1 commit into from

Conversation

dimas-b
Copy link
Contributor

@dimas-b dimas-b commented Sep 5, 2024

The docker build will reuse regular gradle artifacts now and rely on gradle to keep them up-to-date.

The Dockerfile is simplified to remove the build layers.

This saves downloads and compile time in local builds (at least).

Details:

  • Add gradle sub-project for docker-related stuff

  • Use org.jetbrains.gradle.docker plugin to push gradle artifacts into docker build.

  • Add scripts configuration to :polaris-service for reusing scripts across modules.

@dimas-b
Copy link
Contributor Author

dimas-b commented Sep 6, 2024

@collado-mike WDYT?

The docker build will reuse regular gradle artifacts now
and rely on gradle to keep them up-to-date.

The Dockerfile is simplified to remove the `build` layers.

This saves downloads and compile time in local builds (at least).

Details:

* Add `gradle` sub-project for docker-related stuff

* Use `org.jetbrains.gradle.docker` plugin to push gradle
  artifacts into docker build.

* Add `scripts` configuration to `:polaris-service` for
  reusing scripts across modules.
@dimas-b
Copy link
Contributor Author

dimas-b commented Sep 12, 2024

rebased, fixed conflicts, added java 21 to regtest CI

- name: fix permissions
run: mkdir -p regtests/output && chmod 777 regtests/output && chmod 777 regtests/t_*/ref/*
- name: Regression Test
env:
AWS_ACCESS_KEY_ID: ${{secrets.AWS_ACCESS_KEY_ID}}
AWS_SECRET_ACCESS_KEY: ${{secrets.AWS_SECRET_ACCESS_KEY}}
run: docker compose up --build --exit-code-from regtest
run: |
./gradlew dockerBuild --info
Copy link
Member

Choose a reason for hiding this comment

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

Nit:

Suggested change
./gradlew dockerBuild --info
./gradlew dockerBuild --stacktrace

@@ -22,3 +22,4 @@ polaris-core=polaris-core
polaris-service=polaris-service
polaris-eclipselink=extension/persistence/eclipselink
aggregated-license-report=aggregated-license-report
polaris-docker=docker
Copy link
Member

Choose a reason for hiding this comment

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

Hm - can you move docker/ to somewhere else or use a nested folder structure?
I could imagine that there'll be more docker related things.

into(project.layout.buildDirectory.dir("docker-dist"))
from(startScripts) { into("bin") }
from(shadowJar) { into("lib") }
doFirst { delete(project.layout.buildDirectory.dir("regtest-dist")) }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
doFirst { delete(project.layout.buildDirectory.dir("regtest-dist")) }
doFirst { delete(project.layout.buildDirectory.dir("docker-dist")) }

However, the delete shouldn't be necessary, singe it's a Sync task. Can you verify this and if that's true remove this doFirst?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do (although in this PR, the change was only formatting :) )

@collado-mike
Copy link
Contributor

@collado-mike WDYT?

I really prefer to have deterministic builds. Admittedly, always downloading the maven artifacts is a pain (and is non-deterministic if we don't lock the dependency versions), but I would vote for a solution that leaned on docker caching rather than just using local artifacts. The primary benefit to using docker for builds is to guarantee that the build works the same across all developers' machines. This strictly bypasses that benefit.

@dimas-b
Copy link
Contributor Author

dimas-b commented Sep 13, 2024

I really prefer to have deterministic builds.

In CI (GH actions) the difference is between building java code on the worker node vs. inside docker, running on the worker node.

Do you mean that GH actions are not deterministic enough for java builds?

The primary benefit to using docker for builds is to guarantee that the build works the same across all developers' machines. This strictly bypasses that benefit.

When developers run gradle it is already grounded in local env.

Do developers rely on the docker image (as opposed to gradle artifacts) in daily tasks?

@collado-mike
Copy link
Contributor

Do developers rely on the docker image (as opposed to gradle artifacts) in daily tasks?

I always run the e2e tests by running the docker-compose script in order to avoid any surprises when it runs in CI. For day-to-day tasks, of course, I rely on unit/integration tests in my IDE, but I think it's good practice to run the build in the docker image before pushing my branch to the public repo.

@dimas-b dimas-b closed this Sep 23, 2024
@adutra adutra mentioned this pull request Dec 11, 2024
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