Add support for building a container from context archive #671

LNSD · 2023-11-13T10:10:47Z

This PR adds support for building a container image from a tar archive stream (i.e., NodeJs.ReadableStream).

Note
This functionality is already existing in testcontainers-go: dynamic-build-context

This covers the case where one wants to programmatically create a dynamic Dockerfile and context (e.g., using a template engine) without the overhead of writing to disk.

const tar = require('tar-stream');

const tarStream = tar.pack();
tarStream.entry({ name: 'alpine.Dockerfile' }, 
  `
    FROM alpine:latest
    CMD ["sleep", "infinity"]
  `
);
tarStream.finalize();

const container = await GenericContainer
  .fromContextArchive(tarStream, 'alpine.Dockerfile')
  .build();

netlify · 2023-11-13T10:10:53Z

✅ Deploy Preview for testcontainers-node ready!

Name	Link
🔨 Latest commit	`2382dba`
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-node/deploys/655e3bb5d063ee00080f24d2
😎 Deploy Preview	https://deploy-preview-671--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.

javierlopezdeancos · 2023-11-13T21:19:44Z

packages/testcontainers/package.json

-    "@types/tmp": "^0.2.5"
+    "@types/tar-stream": "^3.1.3",
+    "@types/tmp": "^0.2.5",
+    "tar-stream": "^3.1.6"


tar-stream shouldn't be in dependencies 🤔 ?

I only used it in the tests to generate a test tar stream. It is not used in the package's production code.

javierlopezdeancos · 2023-11-13T21:31:01Z

packages/testcontainers/src/generic-container/generic-container-builder.ts

    private readonly dockerfileName: string,
    private readonly uuid: Uuid = new RandomUuid()
  ) {}

-  public withBuildArgs(buildArgs: BuildArgs): GenericContainerBuilder {
+  public withBuildArgs(buildArgs: BuildArgs): this {


thanks to do the return this pattern consistent here 👏

javierlopezdeancos · 2023-11-13T21:38:30Z

packages/testcontainers/src/generic-container/generic-container-dockerfile.test.ts

@@ -103,3 +105,27 @@ describe("GenericContainer Dockerfile", () => {
    await startedContainer.stop();
  });
 });
+
+describe("GenericContainer fromContextArchive", () => {


thanks to add tests 🎉

javierlopezdeancos · 2023-11-13T21:43:02Z

packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts

 import { getAuthConfig } from "../../auth/get-auth-config";
 import { ImageName } from "../../image-name";
 import { ImageClient } from "./image-client";
 import AsyncLock from "async-lock";
 import { log, buildLog, pullLog } from "../../../common";
+import stream from "stream";


this is the node stream api https://nodejs.org/api/stream.html#stream right?
sometimes when I dont read it as

const stream = require('node:stream');

has this doubt 🤔

Yes, it is. I can change the import name with node:stream, but I don't know the difference.

javierlopezdeancos · 2023-11-13T22:21:03Z

packages/testcontainers/src/generic-container/generic-container-builder.ts

@@ -105,3 +138,17 @@ export class GenericContainerBuilder {
      .reduce((prev, next) => ({ ...prev, ...next }), {} as RegistryConfig);
  }
 }
+
+async function newDockerignoreFilter(context: string): Promise<(path: string) => boolean> {


Only to curiosity in your approach, why newDockerignoreFilter function is not part from class GenericContainerBuilder as a private method take into account that you are only using it to this class responsabilities?

I believe class methods should access or modify the object instance.

In this case, newDockerignoreFilter does not access nor modify any object fields. It can be seen as a factory method. It is a function that, given a context path, returns a .dockerignore based filter function.

javierlopezdeancos · 2023-11-14T21:34:03Z

After reviewing the PR I think that for me it is LGTM and approved.

@LNSD Thanks for the explanations about how the docker and dockerode api works to build an image with a dockerfile (recipe) and a files source (ingredients aka context), either through a path on our disk or through an external data source, In both cases, we have to package these files into a tar file and create a stream from it which is as the docker api requires in its context.

The feature itself is very powerful, being able to programmatically create a dynamic dockerfile and feed the stream with ingredients that do not exactly belong to our disk, a use case, an AWS stream.

I see the refactor that you include to allow the feature appropriate, promoting the creation of the source tar file which we will create the stream from.

Only there are some questions that I put in my review to give you my approval

javierlopezdeancos · 2023-11-14T21:36:40Z

packages/testcontainers/src/generic-container/generic-container-builder.ts

@@ -44,6 +48,12 @@ export class GenericContainerBuilder {
    return this;
  }

+  /**


I really really really really appreciate these comments for our methods and functions but, since they are not anywhere in the library, I don't know if it would be convenient to start doing it. 🤔

cristianrgreco · 2023-11-23T18:14:59Z

packages/testcontainers/src/container-runtime/clients/image/docker-image-client.ts

    try {
-      log.debug(`Building image "${opts.t}" with context "${context}"...`);


What's the reasoning behind moving this logic to the GenericContainerBuilder? I liked the idea that the DockerImageClient can be used standalone and supports .dockerignore files out of the box. After this change this is no longer the case

I understand your point.

I decided to raise the filtering to a higher abstraction level, recognizing that the .dockerignore filtering is a specific instance of passing a tar stream to the container runtime client. This approach streamlines the code and renders the DockerImageClient agnostic to the underlying filesystem, enhancing its versatility.

At the time of implementation, I faced two options:

Introduce a method in GenericContainerBuilder and DockerImageClient for passing the stream directly to the container runtime client without .dockerignore filtering. This would create an additional flow in both classes.

Maintain the DockerImageClient agnostic to the filesystem (not utilizing tar-fs) and keep the API straightforward by passing a stream to the container runtime client. Since the filtered stream is a subset of the passed stream, this approach avoids needing two separate APIs in both the client and the GenericContainerBuilder class.

javierlopezdeancos reviewed Nov 13, 2023

View reviewed changes

javierlopezdeancos reviewed Nov 14, 2023

View reviewed changes

This comment was marked as resolved.

Sign in to view

Add support for building a container from context archive

2382dba

cristianrgreco added enhancement New feature or request minor Backward compatible functionality labels Nov 23, 2023

cristianrgreco reviewed Nov 23, 2023

View reviewed changes

LNSD requested review from javierlopezdeancos and cristianrgreco December 12, 2023 14:21

LNSD closed this Jan 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for building a container from context archive #671

Add support for building a container from context archive #671

LNSD commented Nov 13, 2023 •

edited

Loading

netlify bot commented Nov 13, 2023 •

edited

Loading

javierlopezdeancos Nov 13, 2023

LNSD Dec 12, 2023

javierlopezdeancos Nov 13, 2023

javierlopezdeancos Nov 13, 2023

javierlopezdeancos Nov 13, 2023

LNSD Dec 12, 2023

javierlopezdeancos Nov 13, 2023

LNSD Dec 12, 2023

javierlopezdeancos commented Nov 14, 2023 •

edited

Loading

javierlopezdeancos Nov 14, 2023

This comment was marked as resolved.

cristianrgreco Nov 23, 2023 •

edited

Loading

LNSD Dec 12, 2023

		try {
		log.debug(`Building image "${opts.t}" with context "${context}"...`);

Add support for building a container from context archive #671

Add support for building a container from context archive #671

Conversation

LNSD commented Nov 13, 2023 • edited Loading

netlify bot commented Nov 13, 2023 • edited Loading

✅ Deploy Preview for testcontainers-node ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javierlopezdeancos commented Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

cristianrgreco Nov 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LNSD commented Nov 13, 2023 •

edited

Loading

netlify bot commented Nov 13, 2023 •

edited

Loading

javierlopezdeancos commented Nov 14, 2023 •

edited

Loading

cristianrgreco Nov 23, 2023 •

edited

Loading