From 5fd01d16017cfdae1eed46e525213455f343b91a Mon Sep 17 00:00:00 2001 From: digital88 Date: Sun, 23 Mar 2025 22:16:08 +0300 Subject: [PATCH 1/3] improved logging in InternalPortCheck --- .../docker/docker-no-shell/Dockerfile | 11 +++++ .../fixtures/docker/docker-no-shell/index.js | 14 ++++++ .../utils/port-check-real.test.ts | 49 +++++++++++++++++++ .../wait-strategies/utils/port-check.test.ts | 23 +++++++++ .../src/wait-strategies/utils/port-check.ts | 42 ++++++++-------- 5 files changed, 117 insertions(+), 22 deletions(-) create mode 100644 packages/testcontainers/fixtures/docker/docker-no-shell/Dockerfile create mode 100644 packages/testcontainers/fixtures/docker/docker-no-shell/index.js create mode 100644 packages/testcontainers/src/wait-strategies/utils/port-check-real.test.ts diff --git a/packages/testcontainers/fixtures/docker/docker-no-shell/Dockerfile b/packages/testcontainers/fixtures/docker/docker-no-shell/Dockerfile new file mode 100644 index 000000000..99dc67ab2 --- /dev/null +++ b/packages/testcontainers/fixtures/docker/docker-no-shell/Dockerfile @@ -0,0 +1,11 @@ +FROM node AS build +WORKDIR /app +RUN npm init -y \ + && npm install express@4.16.4 +COPY index.js . + +FROM gcr.io/distroless/nodejs20-debian12 AS app +EXPOSE 8080 +COPY --from=build /app /app +WORKDIR /app +CMD ["index.js"] diff --git a/packages/testcontainers/fixtures/docker/docker-no-shell/index.js b/packages/testcontainers/fixtures/docker/docker-no-shell/index.js new file mode 100644 index 000000000..ae93b36a4 --- /dev/null +++ b/packages/testcontainers/fixtures/docker/docker-no-shell/index.js @@ -0,0 +1,14 @@ +const http = require("http"); +const express = require("express"); + +const app = express(); + +app.get("/hello-world", (req, res) => { + res.status(200).send("hello-world"); +}); + +const PORT = 8080; + +http + .createServer(app) + .listen(PORT, () => console.log(`Listening on port ${PORT}`)); diff --git a/packages/testcontainers/src/wait-strategies/utils/port-check-real.test.ts b/packages/testcontainers/src/wait-strategies/utils/port-check-real.test.ts new file mode 100644 index 000000000..5f3b40426 --- /dev/null +++ b/packages/testcontainers/src/wait-strategies/utils/port-check-real.test.ts @@ -0,0 +1,49 @@ +import path from "path"; +import { getContainerRuntimeClient } from "../../container-runtime"; +import { GenericContainer } from "../../generic-container/generic-container"; +import { checkContainerIsHealthy } from "../../utils/test-helper"; +import { Wait } from "../wait"; +import { InternalPortCheck } from "./port-check"; + +describe("PortCheck", { timeout: 120_000 }, () => { + describe("InternalPortCheck", () => { + it("should work on container with shell", async () => { + const client = await getContainerRuntimeClient(); + const container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14") + .withExposedPorts(8080) + .withWaitStrategy(Wait.forLogMessage(/Listening on port 8080/)) + .start(); + + await checkContainerIsHealthy(container); + + const lowerLevelContainer = client.container.dockerode.getContainer(container.getId()); + const portCheck = new InternalPortCheck(client, lowerLevelContainer); + + const isBound = await portCheck.isBound(8080); + expect(isBound).toBeTruthy(); + + await container.stop(); + }); + + const fixtures = path.resolve(__dirname, "..", "..", "..", "fixtures", "docker"); + it("should fail on container without shell (distroless)", async () => { + const context = path.resolve(fixtures, "docker-no-shell"); + const container = await GenericContainer.fromDockerfile(context).build(); + const startedContainer = await container + .withExposedPorts(8080) + .withWaitStrategy(Wait.forLogMessage(/Listening on port 8080/)) + .start(); + + await checkContainerIsHealthy(startedContainer); + + const client = await getContainerRuntimeClient(); + const lowerLevelContainer = client.container.dockerode.getContainer(startedContainer.getId()); + const portCheck = new InternalPortCheck(client, lowerLevelContainer); + + const isBound = await portCheck.isBound(8080); + expect(isBound).toBeFalsy(); + + await startedContainer.stop(); + }); + }); +}); diff --git a/packages/testcontainers/src/wait-strategies/utils/port-check.test.ts b/packages/testcontainers/src/wait-strategies/utils/port-check.test.ts index 00df3ccc0..0b35cf409 100644 --- a/packages/testcontainers/src/wait-strategies/utils/port-check.test.ts +++ b/packages/testcontainers/src/wait-strategies/utils/port-check.test.ts @@ -140,5 +140,28 @@ describe("PortCheck", () => { ], ]); }); + + it("should error log the distroless image once, regardless of logs enabled or not", async () => { + // Make sure logging is disabled explicitly here + mockLogger.enabled.mockImplementation(() => false); + + mockContainerExec + .mockReturnValueOnce(Promise.resolve({ output: "ERROR 1", exitCode: 126 })) + .mockReturnValueOnce(Promise.resolve({ output: "ERROR 2", exitCode: 126 })) + .mockReturnValueOnce(Promise.resolve({ output: "ERROR 2", exitCode: 126 })) + .mockReturnValueOnce(Promise.resolve({ output: "ERROR 1", exitCode: 126 })) + .mockReturnValueOnce(Promise.resolve({ output: "ERROR 2", exitCode: 126 })) + .mockReturnValueOnce(Promise.resolve({ output: "ERROR 2", exitCode: 126 })); + + await portCheck.isBound(8080); + await portCheck.isBound(8080); + + expect(mockLogger.error.mock.calls).toEqual([ + [ + "The HostPortWaitStrategy will not work on a distroless image, use an alternate wait strategy", + { containerId: "containerId" }, + ], + ]); + }); }); }); diff --git a/packages/testcontainers/src/wait-strategies/utils/port-check.ts b/packages/testcontainers/src/wait-strategies/utils/port-check.ts index b43f8bee4..3b37bcfe5 100644 --- a/packages/testcontainers/src/wait-strategies/utils/port-check.ts +++ b/packages/testcontainers/src/wait-strategies/utils/port-check.ts @@ -33,7 +33,7 @@ export class HostPortCheck implements PortCheck { export class InternalPortCheck implements PortCheck { private isDistroless = false; - private commandOutputs = new Set(); + private readonly commandOutputs = new Set(); constructor( private readonly client: ContainerRuntimeClient, @@ -53,28 +53,26 @@ export class InternalPortCheck implements PortCheck { ); const isBound = commandResults.some((result) => result.exitCode === 0); + const shellExists = commandResults.some((result) => result.exitCode !== 126); + if (!isBound && !shellExists && !this.isDistroless) { + this.isDistroless = true; + log.error(`The HostPortWaitStrategy will not work on a distroless image, use an alternate wait strategy`, { + containerId: this.container.id, + }); + } + if (!isBound && log.enabled()) { - const shellExists = commandResults.some((result) => result.exitCode !== 126); - if (!shellExists) { - if (!this.isDistroless) { - this.isDistroless = true; - log.error(`The HostPortWaitStrategy will not work on a distroless image, use an alternate wait strategy`, { - containerId: this.container.id, - }); - } - } else { - commandResults - .map((result) => ({ ...result, output: result.output.trim() })) - .filter((result) => result.exitCode !== 126 && result.output.length > 0) - .forEach((result) => { - if (!this.commandOutputs.has(this.commandOutputsKey(result.output))) { - log.trace(`Port check result exit code ${result.exitCode}: ${result.output}`, { - containerId: this.container.id, - }); - this.commandOutputs.add(this.commandOutputsKey(result.output)); - } - }); - } + commandResults + .map((result) => ({ ...result, output: result.output.trim() })) + .filter((result) => result.exitCode !== 126 && result.output.length > 0) + .forEach((result) => { + if (!this.commandOutputs.has(this.commandOutputsKey(result.output))) { + log.trace(`Port check result exit code ${result.exitCode}: ${result.output}`, { + containerId: this.container.id, + }); + this.commandOutputs.add(this.commandOutputsKey(result.output)); + } + }); } return isBound; From 69751eabbb1e0a0aeea58cb8fc8ed5d1242e527c Mon Sep 17 00:00:00 2001 From: digital88 Date: Sun, 23 Mar 2025 22:53:59 +0300 Subject: [PATCH 2/3] add check for error 127 --- .../src/wait-strategies/utils/port-check.test.ts | 15 +++++++++++++++ .../src/wait-strategies/utils/port-check.ts | 5 ++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/testcontainers/src/wait-strategies/utils/port-check.test.ts b/packages/testcontainers/src/wait-strategies/utils/port-check.test.ts index 0b35cf409..54808a760 100644 --- a/packages/testcontainers/src/wait-strategies/utils/port-check.test.ts +++ b/packages/testcontainers/src/wait-strategies/utils/port-check.test.ts @@ -163,5 +163,20 @@ describe("PortCheck", () => { ], ]); }); + + it.for([126, 127])("should error log the distroless image when exit code is %i", async (code) => { + mockContainerExec.mockReturnValueOnce(Promise.resolve({ output: "ERROR 1", exitCode: code })); + mockContainerExec.mockReturnValueOnce(Promise.resolve({ output: "ERROR 2", exitCode: code })); + mockContainerExec.mockReturnValueOnce(Promise.resolve({ output: "ERROR 3", exitCode: code })); + + await portCheck.isBound(8080); + + expect(mockLogger.error.mock.calls).toEqual([ + [ + "The HostPortWaitStrategy will not work on a distroless image, use an alternate wait strategy", + { containerId: "containerId" }, + ], + ]); + }); }); }); diff --git a/packages/testcontainers/src/wait-strategies/utils/port-check.ts b/packages/testcontainers/src/wait-strategies/utils/port-check.ts index 3b37bcfe5..0969bb5fb 100644 --- a/packages/testcontainers/src/wait-strategies/utils/port-check.ts +++ b/packages/testcontainers/src/wait-strategies/utils/port-check.ts @@ -53,7 +53,10 @@ export class InternalPortCheck implements PortCheck { ); const isBound = commandResults.some((result) => result.exitCode === 0); - const shellExists = commandResults.some((result) => result.exitCode !== 126); + // https://www.gnu.org/software/bash/manual/html_node/Exit-Status.html + // If a command is not found, the child process created to execute it returns a status of 127. + // If a command is found but is not executable, the return status is 126. + const shellExists = commandResults.some((result) => result.exitCode !== 126 && result.exitCode !== 127); if (!isBound && !shellExists && !this.isDistroless) { this.isDistroless = true; log.error(`The HostPortWaitStrategy will not work on a distroless image, use an alternate wait strategy`, { From 7a8137587cfc59cd506a27fb007e7de84cfef545 Mon Sep 17 00:00:00 2001 From: Cristian Greco Date: Mon, 24 Mar 2025 10:18:09 +0000 Subject: [PATCH 3/3] Remove port-check integration tests --- .../utils/port-check-real.test.ts | 49 ------------------- 1 file changed, 49 deletions(-) delete mode 100644 packages/testcontainers/src/wait-strategies/utils/port-check-real.test.ts diff --git a/packages/testcontainers/src/wait-strategies/utils/port-check-real.test.ts b/packages/testcontainers/src/wait-strategies/utils/port-check-real.test.ts deleted file mode 100644 index 5f3b40426..000000000 --- a/packages/testcontainers/src/wait-strategies/utils/port-check-real.test.ts +++ /dev/null @@ -1,49 +0,0 @@ -import path from "path"; -import { getContainerRuntimeClient } from "../../container-runtime"; -import { GenericContainer } from "../../generic-container/generic-container"; -import { checkContainerIsHealthy } from "../../utils/test-helper"; -import { Wait } from "../wait"; -import { InternalPortCheck } from "./port-check"; - -describe("PortCheck", { timeout: 120_000 }, () => { - describe("InternalPortCheck", () => { - it("should work on container with shell", async () => { - const client = await getContainerRuntimeClient(); - const container = await new GenericContainer("cristianrgreco/testcontainer:1.1.14") - .withExposedPorts(8080) - .withWaitStrategy(Wait.forLogMessage(/Listening on port 8080/)) - .start(); - - await checkContainerIsHealthy(container); - - const lowerLevelContainer = client.container.dockerode.getContainer(container.getId()); - const portCheck = new InternalPortCheck(client, lowerLevelContainer); - - const isBound = await portCheck.isBound(8080); - expect(isBound).toBeTruthy(); - - await container.stop(); - }); - - const fixtures = path.resolve(__dirname, "..", "..", "..", "fixtures", "docker"); - it("should fail on container without shell (distroless)", async () => { - const context = path.resolve(fixtures, "docker-no-shell"); - const container = await GenericContainer.fromDockerfile(context).build(); - const startedContainer = await container - .withExposedPorts(8080) - .withWaitStrategy(Wait.forLogMessage(/Listening on port 8080/)) - .start(); - - await checkContainerIsHealthy(startedContainer); - - const client = await getContainerRuntimeClient(); - const lowerLevelContainer = client.container.dockerode.getContainer(startedContainer.getId()); - const portCheck = new InternalPortCheck(client, lowerLevelContainer); - - const isBound = await portCheck.isBound(8080); - expect(isBound).toBeFalsy(); - - await startedContainer.stop(); - }); - }); -});