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

Bugfix: Dockerfile RUN cache #559

Merged
merged 4 commits into from
Apr 19, 2024

Conversation

carlomion
Copy link
Contributor

Following the introduction of the Dockerfile RUN cache in #554 , the build has started failing, likely due to concurrent access to the same cached folder during the multi-architecture build process: https://github.com/mediagis/nominatim-docker/actions/runs/8722047757

This PR does the following:

  • avoid sharing of the cache
  • uses the docker/metadata-action@v5 to define the Docker label and tag for the image
  • the docker build is always done in the CI pipeline, so that the arm64 build is also built, but only pushed to Dockerhub if on the master branch

Copy link

what-the-diff bot commented Apr 18, 2024

PR Summary

  • Enhanced Workflow for Docker Image Creation and Versioning
    An improvement to the system handling Docker image creation (a kind of "blueprint" for creating a program) in the continuous integration workflow file (.github/workflows/ci.yml). This update includes a new step to calculate Docker tags and labels, which helps to properly version and categorize each image.

  • Simplified Docker Tag Process
    The old method of setting the Docker image's date tag has now been removed, making the overall process more efficient and less error-prone.

  • Updated Docker Build and Push method
    The process of building (creating from the blueprint) and pushing (sending to a repository where it can be shared with others) the Docker image has been greatly improved. It now uses a trusted and efficient third-party software (docker/build-push-action@v5).

  • Optimized Docker Build Commands and Cache Incorporation
    Changes have been made to the commands that build the Docker image in the file 4.4/Dockerfile. These commands now feature improved caching. Caching accelerates the build process by reusing previously stored or calculated information rather than recalling or recalculating it.

  • Streamlined Docker Image with Package Removal
    Several packages, which can be viewed as sets of functionalities, were removed from the file 4.4/Dockerfile. This helps to trim down the Docker image size, increasing its efficiency and reducing the storage space it requires.

  • Improved Docker Image Cleanup
    The way cleaning up after building the Docker image works has been revised, making it more effective and efficient.

@leonardehrenfried
Copy link
Collaborator

Again, this is a really nice PR. Lets see if the build succeeds this time.

--mount=type=cache,target=/var/cache/apt \
# Inspired by https://github.com/reproducible-containers/buildkit-cache-dance?tab=readme-ov-file#apt-get-github-actions
RUN \
--mount=type=cache,target=/var/cache/apt,sharing=locked \
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit confused why concurrent access was even possible. I was under the impression that these are two totally different machines/vms and don't share anything, let alone disk. Where is this cache being saved to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As explained here https://docs.docker.com/reference/dockerfile/#run---mounttypecache by default the parameter RUN --cache sharing is shared, meaning that the arm64 and amd64 builds, running in parallel inside the same GitHub runner (here is a quick overview of the multi-platform build done by Docker) will conflict with each other.

@philipkozeny philipkozeny merged commit f2d2fec into mediagis:master Apr 19, 2024
30 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