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

Allow building with BuildKit #761

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

schummar
Copy link

@schummar schummar commented Apr 27, 2024

Adds a withBuildKit option. With the option enabled, a build will be done using BuildKit - within a docker container.
This should be relatively independent on the host system's BuildKit support.

Questions:

  • Should the env var DOCKER_BUILDKIT=1 enabled the setting by default?
  • Should it be enabled by default in general? For a "just works" for newer dockerfile features like mounts?

Solves #571

Copy link

netlify bot commented Apr 27, 2024

Deploy Preview for testcontainers-node ready!

Name Link
🔨 Latest commit 083e539
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-node/deploys/662d713c119bef0008f83c76
😎 Deploy Preview https://deploy-preview-761--testcontainers-node.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@cristianrgreco
Copy link
Collaborator

Hi @schummar, thanks for the contribution!

Because build kit is the default image builder I'd say this should be the default behaviour.

However because this implementation requires a separate container for the build, as well as copying potentially large files between images, I'm concerned about performance, which I assume is significantly worse when compared to a non-buildkit build. If we make this functionality opt-in (keeping withBuildKit), then I'm concerned about maintenance as it's a large and complex change. I haven't run the tests yet but I'm assuming issues with other container runtimes like podman too.

My plan was to wait for build kit to be natively supported by Docker engine and Dockerode, which should result in no or minor changes and no performance impact.

@schummar
Copy link
Author

Fair enough. I think the performance might not too bad in comparison. BuildKit builds are usually run in a container anyway, as far as I know - but my knowledge there is admittedly limited. I totally get the reluctance to add the complexity though.

Unfortunately, it's unclear when this might be supported (the referenced issues are 3,5 years old at this point). Usage of BuildKit features is getting more common - my very first try with testcontainers immediately failed because of this 😉. So, it would be good to have some kind of solution. What do you think about these options:

  1. I can create a separate npm package that provides the functionality. To load the image some access to container runtime would be great. Like keeping the proposed ImageClient.load method. Or exposing the underlying Dockerode instance?
  2. Would you be interested in having that package as a testcontainers module?

@schummar
Copy link
Author

schummar commented Jun 3, 2024

@cristianrgreco when you have a minute, could you give feedback?

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.

None yet

2 participants