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

refactor dockerfiles #602

Merged
merged 1 commit into from
Sep 13, 2021
Merged

refactor dockerfiles #602

merged 1 commit into from
Sep 13, 2021

Conversation

josefschabasser
Copy link
Contributor

@josefschabasser josefschabasser commented Aug 25, 2021

Pull request checklist

  • All tests pass. Demo project builds and runs.
  • I have resolved any merge conflicts.

This fixes issue #601.

What's in this pull request?

refactored dockerfiles using newer images, multistage builds, only required packages and other tweaks.
reduces image size by 840MB (deb), 1213MB (git), 1283MB (repo)
new image sizes: 450MB (deb), 207MB (git), 207MB (repo)

  • introduce multistage builds
  • build using standard image, run using slim image
  • upgrade base images
  • install only required packages
  • move ARGS down to maximize caching
  • add dockerignore file and ignore dist/peer-key/
  • do not modify lockfile during package installation
  • run tests after install/build
  • add compatible Node.js versions to readme
  • make repo URL configurable
  • introduce variable INSTALL_PATH
  • add another stage to install production dependencies and further trim image size
  • rename and move dockerfiles to better group them and get syntax highlighting
  • add ability to mount volumes to /nimiq in git/repo image (feature parity with deb image)
  • add syntax highlighting to docs/docker.md
  • fixed containers not shutting down on CTRL-C or kill 1
  • fixed permission mismatch when mounting volumes to /nimiq and sharing between different images (deb used different UID/GID)

@josefschabasser josefschabasser changed the title introduce multistage builds, refactor dockerfiles Aug 25, 2021
@philippfo9
Copy link

would welcome this change!

@josefschabasser
Copy link
Contributor Author

josefschabasser commented Aug 26, 2021

I think now I'm done. I don't like the second build stage in the git/repo dockerfiles, but I didn't want to change yarn/gulp/gyp behaviour. The second build stage works around build issues (yarn install --production still needs build-essential packages). Besides that, one can now mount images/directories to /nimiq in all images, size went down by a HUGE amount, branch, repo URL, data and install directories are configurable and docker layer caching is used to the max without enabling BuildKit (which should really be considered to share yarn cache between stages).

@josefschabasser
Copy link
Contributor Author

Okay, now I'm done. Fixed 2 more issues (container not shutting down at all and permission mismatch when sharing /nimiq between different images).

@jeffesquivels
Copy link
Member

LGTM in general, just have one question:

[...] docker layer caching is used to the max without enabling BuildKit (which should really be considered to share yarn cache between stages).

Is there any downside to enabling BuildKit?

It seems to me another advantage is that we could use a custom .dockerignore file per each Dockerfile, and thus make the process a bit more efficient by not sending any build context where none is needed (such as the deb or the git cases).

@josefschabasser
Copy link
Contributor Author

josefschabasser commented Sep 1, 2021

There is no issue at all, I just didn't want to break it for everyone out there. If you want me to, I can enable it, optimize cache sharing between stages and add a note to the readme.

@jeffesquivels jeffesquivels merged commit f41d9fa into nimiq:master Sep 13, 2021
@jeffesquivels jeffesquivels mentioned this pull request Oct 25, 2021
2 tasks
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