Allow building with BuildKit
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=1enabled the setting by default? - Should it be enabled by default in general? For a "just works" for newer dockerfile features like mounts?
Solves #571
Deploy Preview for testcontainers-node ready!
| Name | Link |
|---|---|
| Latest commit | 2d6e540be09754f9c611a681b041c12b9a4a6309 |
| Latest deploy log | https://app.netlify.com/sites/testcontainers-node/deploys/666804f7c9e93d00083dd011 |
| Deploy Preview | https://deploy-preview-761--testcontainers-node.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
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.
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:
- 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.loadmethod. Or exposing the underlyingDockerodeinstance? - Would you be interested in having that package as a testcontainers module?
@cristianrgreco when you have a minute, could you give feedback?
Ok, completely new development:
As @mikeseese pointed out (https://github.com/testcontainers/testcontainers-node/issues/571#issuecomment-2159657555), the version flag is in fact working.
Upon investigation I found that I can make it work from testcontainers as well with one caveat: When passing pull: "true" and version: "2", the build always fails with "node:10-alpine: failed to resolve source metadata for docker.io/library/node:10-alpine: no active sessions". And because of this issue that is handled in PR #771, pull: "true" is currently always passed and therefore vesion: "2" never seemed to work at all!
That make this PR so much more concise! More open questions:
- I suppose the mentioned error is a docker bug. Until it's resolved, should testcontainers emit a warning when using both options together?
- Currently dockerode types don't contain the
versionflag. Either we wait for https://github.com/DefinitelyTyped/DefinitelyTyped/pull/69780 or use a castas ImageBuildOptionsto cover it up for now
Edit: Well shit, the same problem arises when the image used in the FROM statement is not locally available yet. For some reason buildKit can't pull at all when run over the api!? At least on my machine...
Hi @schummar, thanks for staying on top of this issue! I've checked out this branch and run the test should work with buildKit and it passes. Are you saying that it doesn't for you? If it does could you please add a test for your valid example which is failing so I can also have a look? I'm on Docker version 24.0.6 on an M1 Mac
A simple manual way to reproduce it is to run docker image rm node:10-alpine, them execute just that test case: npm run test -- packages/testcontainers/src/generic-container/generic-container-dockerfile.test.ts -t "buildKit". It will fail.
Then run docker pull node:10-alpine and execute the test again and it will succeed.
I can try and come up with a test case later. Removing images is not covered by the ContainerRuntimeClient, so I have to find a way around that.