diff --git a/.github/workflows/source-lints.yml b/.github/workflows/source-lints.yml index 5af3365ed742..7bc28f0cc8e1 100644 --- a/.github/workflows/source-lints.yml +++ b/.github/workflows/source-lints.yml @@ -31,6 +31,7 @@ on: - "test/harness.ts" - "test/tsconfig.json" - "test/_util/**" + - "test/docker/**" - "test/internal/source-lints/**" - ".github/workflows/*.yml" - ".github/actions/setup-bun/**" @@ -55,6 +56,7 @@ on: - "test/harness.ts" - "test/tsconfig.json" - "test/_util/**" + - "test/docker/**" - "test/internal/source-lints/**" - ".github/workflows/*.yml" - ".github/actions/setup-bun/**" diff --git a/test/docker/README.md b/test/docker/README.md index 483f20af5bbc..313ea2fc33a7 100644 --- a/test/docker/README.md +++ b/test/docker/README.md @@ -287,7 +287,10 @@ A: No! `ensure()` starts them automatically if needed. A: Add it to docker-compose.yml and create a PR. **Q: How do I update a service version?** -A: Edit docker-compose.yml and run `docker-compose pull`. +A: Edit the `FROM` line of the service's Dockerfile. The next `ensure()` builds the image again. + +**Q: When do the CI machines get a new or changed image?** +A: At the next bake of the CI machine images. A change under `test/docker` starts no bake. Until then, each CI test machine builds the image before it starts the service. `scripts/build/ci-images/CLAUDE.md` ("Refresh what is prefetched") says how to start a bake. **Q: Can I run tests in parallel?** A: Yes! Each service can handle multiple connections. @@ -320,7 +323,7 @@ A: This tells Docker to pick any available port, preventing conflicts. To add a new service: -1. Add service definition to `docker-compose.yml` +1. Add service definition to `docker-compose.yml`, with a `build:` section (a Dockerfile of one line, `FROM `, is enough) 2. Use dynamic ports unless specific port required 3. Add health check if possible 4. Document in this README diff --git a/test/docker/docker-compose.yml b/test/docker/docker-compose.yml index ca8b2c8af0c5..52a18c1f9fad 100644 --- a/test/docker/docker-compose.yml +++ b/test/docker/docker-compose.yml @@ -153,16 +153,6 @@ services: start_interval: 1s # Redis/Valkey Services - redis_plain: - image: redis:7-alpine - command: redis-server --bind 0.0.0.0 --protected-mode no - ports: - - target: 6379 - published: 0 - protocol: tcp - tmpfs: - - /data - redis_unified: build: context: ../js/valkey/docker-unified diff --git a/test/docker/index.ts b/test/docker/index.ts index bb07f7a7753d..1290320c5206 100644 --- a/test/docker/index.ts +++ b/test/docker/index.ts @@ -14,7 +14,6 @@ export type ServiceName = | "mysql_native_password" | "mysql_tls" | "mariadb_plain" - | "redis_plain" | "redis_unified" | "autobahn" | "squid"; @@ -59,7 +58,6 @@ const serviceMeta: Record { - // Pre-build the service (a no-op for image-only services) so build time - // doesn't eat into the `up --wait` timeout below. CI pre-bakes everything - // via buildServices(); this covers local dev where that wasn't run. + // Pre-build the service so build time doesn't eat into the `up --wait` + // timeout below. CI pre-bakes everything via buildServices(); this covers + // local dev where that wasn't run. const buildResult = await this.exec(["build", service]); if (buildResult.exitCode !== 0) { throw new Error(`Failed to build service ${service}: ${buildResult.stderr}`); @@ -244,13 +242,29 @@ class DockerComposeHelper { // mysqld`); 180 is generous headroom so a slow host or a service whose // init regresses surfaces as a single diagnosable failure here rather than // cascading through every test file that asks for it. - const { exitCode, stderr } = await this.exec(["up", "-d", "--wait", "--wait-timeout", "180", service]); + // --pull never: an image comes from `compose build` only, above or in the + // bake of a CI machine image (buildServices()). Without it, compose pulls + // the image of a service with no `build:` section on every test machine. + const { exitCode, stderr } = await this.exec([ + "up", + "-d", + "--wait", + "--wait-timeout", + "180", + "--pull", + "never", + service, + ]); if (exitCode !== 0) { const ps = await this.exec(["ps", "-a", service]); const logs = await this.exec(["logs", "--tail", "50", service]); + const note = stderr.includes("No such image") + ? `note: \`compose up\` runs with \`--pull never\`. The image of ${service} has to come from a \`build:\` section in ${this.composeFile}.\n` + : ""; throw new Error( - `Failed to start service ${service}: ${stderr}\n` + `--- ps ---\n${ps.stdout}\n--- logs ---\n${logs.stdout}`, + `Failed to start service ${service}: ${stderr}\n${note}` + + `--- ps ---\n${ps.stdout}\n--- logs ---\n${logs.stdout}`, ); } @@ -418,7 +432,6 @@ class DockerComposeHelper { } break; - case "redis_plain": case "redis_unified": env.REDIS_HOST = info.host; env.REDIS_PORT = info.ports[6379].toString(); @@ -484,38 +497,17 @@ class DockerComposeHelper { } /** - * Pull all Docker images explicitly - useful for CI - */ - async pullImages(): Promise { - console.log("Pulling Docker images..."); - const { exitCode, stderr } = await this.exec(["pull", "--ignore-pull-failures"]); - - if (exitCode !== 0) { - // Don't fail on pull errors since some services need building - console.warn(`Warning during image pull: ${stderr}`); - } - } - - /** - * Build all services that need building - useful for CI + * Build the image of every service - what the bake of a CI machine image runs */ async buildServices(): Promise { // Bare `compose build` builds every service that has a `build:` section, - // so there's no hardcoded list to keep in sync as services are converted. + // which is every service: doUp() starts none that compose would pull. console.log("Building all services with a build section..."); const { exitCode, stderr } = await this.exec(["build"]); if (exitCode !== 0) { throw new Error(`Failed to build services: ${stderr}`); } } - - /** - * Prepare all images (pull and build) - useful for CI - */ - async prepareImages(): Promise { - await this.pullImages(); - await this.buildServices(); - } } // Global instance @@ -553,18 +545,10 @@ export async function waitTcp(host: string, port: number, timeout?: number): Pro return getHelper().waitTcp(host, port, timeout); } -export async function pullImages(): Promise { - return getHelper().pullImages(); -} - export async function buildServices(): Promise { return getHelper().buildServices(); } -export async function prepareImages(): Promise { - return getHelper().prepareImages(); -} - // Higher-level wrappers for tests export async function withPostgres( opts: { variant?: "plain" | "tls" | "auth" }, @@ -602,24 +586,6 @@ export async function withMySQL( } } -export async function withRedis( - opts: { variant?: "plain" | "unified" }, - fn: (info: ServiceInfo & { url: string; tlsUrl?: string }) => Promise, -): Promise { - const variant = opts.variant || "plain"; - const serviceName = `redis_${variant}` as ServiceName; - const info = await ensure(serviceName); - - const url = `redis://${info.host}:${info.ports[6379]}`; - const tlsUrl = info.ports[6380] ? `rediss://${info.host}:${info.ports[6380]}` : undefined; - - try { - await fn({ ...info, url, tlsUrl }); - } finally { - // Services persist - no teardown - } -} - export async function withAutobahn(fn: (info: ServiceInfo & { url: string }) => Promise): Promise { const info = await ensure("autobahn"); diff --git a/test/docker/prepare-ci.ts b/test/docker/prepare-ci.ts index 3eecd3a1f815..33c05ff49d04 100644 --- a/test/docker/prepare-ci.ts +++ b/test/docker/prepare-ci.ts @@ -2,19 +2,19 @@ /** * CI preparation script for Docker test services * - * This script pre-pulls and builds all Docker images needed for tests + * This script builds all Docker images needed for tests * to avoid failures during test execution. * * Usage: bun test/docker/prepare-ci.ts */ -import { prepareImages } from "./index"; +import { buildServices } from "./index"; async function main() { console.log("Preparing Docker test infrastructure for CI..."); try { - await prepareImages(); + await buildServices(); console.log("✅ Docker test infrastructure is ready"); process.exit(0); } catch (error) { diff --git a/test/harness.ts b/test/harness.ts index 60fffcbc865c..ff45f18e50dd 100644 --- a/test/harness.ts +++ b/test/harness.ts @@ -1205,7 +1205,6 @@ export async function describeWithContainer( "mariadb_plain": 3306, "mysql:8": 3306, // Map mysql:8 to mysql_plain "mysql:9": 3306, // Map mysql:9 to mysql_native_password - "redis_plain": 6379, "redis_unified": 6379, "autobahn": 9002, }; diff --git a/test/internal/docker-compose-helper.test.ts b/test/internal/docker-compose-helper.test.ts new file mode 100644 index 000000000000..78e73797aa53 --- /dev/null +++ b/test/internal/docker-compose-helper.test.ts @@ -0,0 +1,70 @@ +// test/docker/index.ts starts a service with `docker compose up --pull never`. +// The image of a service comes from `docker compose build` only, which is also +// all that a bake of a CI machine image runs. Without the flag, compose pulls +// the image of a service with no `build:` section: the bake leaves that image +// out, and each test machine then fetches it from a registry. +// +// `docker` is a shell script on PATH here. It answers like compose for such a +// service when its image is not on the machine: `up --pull never` fails with +// the error of the daemon, and `up` without the flag pulls the image and +// starts the service. A shell script does not run as a program on Windows, +// hence the skip. +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; +import { chmodSync } from "node:fs"; +import { join } from "node:path"; + +const docker = join(import.meta.dir, "../docker"); + +const fakeDocker = [ + "#!/bin/sh", + '[ "$1" = version ] && exit 0', + '[ "$2" = version ] && exit 0', + "# docker compose -p -f [options] ", + "shift 5", + 'case "$1" in', + " up)", + ' case " $* " in', + ' *" --pull never "*)', + ' echo "Error response from daemon: No such image: redis:7-alpine" >&2', + " exit 1 ;;", + " esac ;;", + ' port) echo "0.0.0.0:49153" ;;', + "esac", + "", +].join("\n"); + +test.skipIf(isWindows)("ensure() pulls no image: a service with no `build:` section does not start", async () => { + using dir = tempDir("docker-compose-helper", { "bin/docker": fakeDocker }); + const bin = join(String(dir), "bin"); + chmodSync(join(bin, "docker"), 0o755); + + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `import { ensure } from ${JSON.stringify(join(docker, "index.ts"))}; + process.stdout.write(await ensure("squid").then(() => "started", error => error.message));`, + ], + env: { + ...bunEnv, + PATH: `${bin}:${bunEnv.PATH}`, + BUN_TEST_SERVICE_squid: undefined, + BUN_DOCKER_COORDINATOR: undefined, + BUN_DOCKER_COMPOSE_FILE: undefined, + }, + stdout: "pipe", + stderr: "pipe", + }); + const [message, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect({ message, stderr, exitCode }).toEqual({ + message: expect.stringContaining( + "Failed to start service squid: Error response from daemon: No such image: redis:7-alpine\n\n" + + "note: `compose up` runs with `--pull never`. The image of squid has to come from a `build:` section in " + + `${join(docker, "docker-compose.yml")}.\n`, + ), + stderr: expect.any(String), + exitCode: 0, + }); +}); diff --git a/test/internal/source-lints/docker-compose-build.test.ts b/test/internal/source-lints/docker-compose-build.test.ts new file mode 100644 index 000000000000..524c2df3a45a --- /dev/null +++ b/test/internal/source-lints/docker-compose-build.test.ts @@ -0,0 +1,23 @@ +// The Docker image of a test service comes from `docker compose build` only. +// A bake of a CI machine image runs that and nothing else +// (test/docker/prepare-ci.ts), and a test starts a service with `--pull never` +// (test/docker/index.ts). So a service that `compose build` leaves out is on +// no CI machine. +import { expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +test("`docker compose build` makes the image of every service in docker-compose.yml", () => { + const compose = Bun.YAML.parse(readFileSync(join(import.meta.dir, "../../docker/docker-compose.yml"), "utf8")) as { + include?: unknown; + services: Record; + }; + const services = Object.entries(compose.services); + // `compose build` skips a service with no `build:` section and a service + // behind a profile, and this lint does not read a file that `include:` names. + expect({ + include: compose.include, + noBuildSection: services.filter(([, service]) => service.build === undefined).map(([name]) => name), + behindProfile: services.filter(([, service]) => service.profiles !== undefined).map(([name]) => name), + }).toEqual({ include: undefined, noBuildSection: [], behindProfile: [] }); +});