Improvements around docker in Playwright (#12261)

* Extract Postgres Docker to its own class

Signed-off-by: Michael Telatynski <[email protected]>

* Don't specify docker `--rm` in CI as it makes debugging harder

Signed-off-by: Michael Telatynski <[email protected]>

* Improve docker commands and introspection

Signed-off-by: Michael Telatynski <[email protected]>

* Remove `HOST_DOCKER_INTERNAL` magic in favour of `host.containers.internal`

Signed-off-by: Michael Telatynski <[email protected]>

* Iterate

Signed-off-by: Michael Telatynski <[email protected]>

* Always pipe

Signed-off-by: Michael Telatynski <[email protected]>

* Re-add pipe flag to silence pg_isready and podman checks

Signed-off-by: Michael Telatynski <[email protected]>

* Iterate

Signed-off-by: Michael Telatynski <[email protected]>

* Iterate

Signed-off-by: Michael Telatynski <[email protected]>

* Iterate

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>

playwright/e2e/register/email.spec.ts

@@ -25,7 +25,7 @@ test.describe("Email Registration", async () => {
             use({
                 template: "email",
                 variables: {
-                    SMTP_HOST: "{{HOST_DOCKER_INTERNAL}}", // This will get replaced in synapseStart
+                    SMTP_HOST: "host.containers.internal",
                     SMTP_PORT: mailhog.instance.smtpPort,
                 },
             }),

playwright/plugins/docker/index.ts

@@ -19,16 +19,48 @@ import * as crypto from "crypto";
 import * as childProcess from "child_process";
 import * as fse from "fs-extra";
 
+/**
+ * @param cmd - command to execute
+ * @param args - arguments to pass to executed command
+ * @param suppressOutput - whether to suppress the stdout and stderr resulting from this command.
+ * @return Promise which resolves to an object containing the string value of what was
+ *         written to stdout and stderr by the executed command.
+ */
+const exec = (cmd: string, args: string[], suppressOutput = false): Promise<{ stdout: string; stderr: string }> => {
+    return new Promise((resolve, reject) => {
+        if (!suppressOutput) {
+            const log = ["Running command:", cmd, ...args, "\n"].join(" ");
+            // When in CI mode we combine reports from multiple runners into a single HTML report
+            // which has separate files for stdout and stderr, so we print the executed command to both
+            process.stdout.write(log);
+            if (process.env.CI) process.stderr.write(log);
+        }
+        const { stdout, stderr } = childProcess.execFile(cmd, args, { encoding: "utf8" }, (err, stdout, stderr) => {
+            if (err) reject(err);
+            resolve({ stdout, stderr });
+            if (!suppressOutput) {
+                process.stdout.write("\n");
+                if (process.env.CI) process.stderr.write("\n");
+            }
+        });
+        if (!suppressOutput) {
+            stdout.pipe(process.stdout);
+            stderr.pipe(process.stderr);
+        }
+    });
+};
+
 export class Docker {
     public id: string;
 
     async run(opts: { image: string; containerName: string; params?: string[]; cmd?: string[] }): Promise<string> {
         const userInfo = os.userInfo();
         const params = opts.params ?? [];
 
-        if (params?.includes("-v") && userInfo.uid >= 0) {
+        const isPodman = await Docker.isPodman();
+        if (params.includes("-v") && userInfo.uid >= 0) {
             // Run the docker container as our uid:gid to prevent problems with permissions.
-            if (await Docker.isPodman()) {
+            if (isPodman) {
                 // Note: this setup is for podman rootless containers.
 
                 // In podman, run as root in the container, which maps to the current
@@ -45,75 +77,57 @@ export class Docker {
             }
         }
 
+        // Make host.containers.internal work to allow the container to talk to other services via host ports.
+        if (isPodman) {
+            params.push("--network");
+            params.push("slirp4netns:allow_host_loopback=true");
+        } else {
+            // Docker for Desktop includes a host-gateway mapping on host.docker.internal but to simplify the config
+            // we use the Podman variant host.containers.internal in all environments.
+            params.push("--add-host");
+            params.push("host.containers.internal:host-gateway");
+        }
+
+        // Provided we are not running in CI, add a `--rm` parameter.
+        // There is no need to remove containers in CI (since they are automatically removed anyway), and
+        // `--rm` means that if a container crashes this means its logs are wiped out.
+        if (!process.env.CI) params.unshift("--rm");
+
         const args = [
             "run",
             "--name",
             `${opts.containerName}-${crypto.randomBytes(4).toString("hex")}`,
             "-d",
-            "--rm",
             ...params,
             opts.image,
         ];
 
         if (opts.cmd) args.push(...opts.cmd);
 
-        this.id = await new Promise<string>((resolve, reject) => {
-            childProcess.execFile("docker", args, (err, stdout) => {
-                if (err) reject(err);
-                resolve(stdout.trim());
-            });
-        });
+        const { stdout } = await exec("docker", args);
+        this.id = stdout.trim();
         return this.id;
     }
 
-    stop(): Promise<void> {
-        return new Promise<void>((resolve, reject) => {
-            childProcess.execFile("docker", ["stop", this.id], (err) => {
-                if (err) reject(err);
-                resolve();
-            });
-        });
-    }
-
-    exec(params: string[]): Promise<void> {
-        return new Promise<void>((resolve, reject) => {
-            childProcess.execFile(
-                "docker",
-                ["exec", this.id, ...params],
-                { encoding: "utf8" },
-                (err, stdout, stderr) => {
-                    if (err) {
-                        console.log(stdout);
-                        console.log(stderr);
-                        reject(err);
-                        return;
-                    }
-                    resolve();
-                },
-            );
-        });
+    async stop(): Promise<void> {
+        try {
+            await exec("docker", ["stop", this.id]);
+        } catch (err) {
+            console.error(`Failed to stop docker container`, this.id, err);
+        }
     }
 
-    rm(): Promise<void> {
-        return new Promise<void>((resolve, reject) => {
-            childProcess.execFile("docker", ["rm", this.id], (err) => {
-                if (err) reject(err);
-                resolve();
-            });
-        });
+    /**
+     * @param params - list of parameters to pass to `docker exec`
+     * @param suppressOutput - whether to suppress the stdout and stderr resulting from this command.
+     */
+    async exec(params: string[], suppressOutput = true): Promise<void> {
+        await exec("docker", ["exec", this.id, ...params], suppressOutput);
     }
 
-    getContainerIp(): Promise<string> {
-        return new Promise<string>((resolve, reject) => {
-            childProcess.execFile(
-                "docker",
-                ["inspect", "-f", "{{ .NetworkSettings.IPAddress }}", this.id],
-                (err, stdout) => {
-                    if (err) reject(err);
-                    else resolve(stdout.trim());
-                },
-            );
-        });
+    async getContainerIp(): Promise<string> {
+        const { stdout } = await exec("docker", ["inspect", "-f", "{{ .NetworkSettings.IPAddress }}", this.id]);
+        return stdout.trim();
     }
 
     async persistLogsToFile(args: { stdoutFile?: string; stderrFile?: string }): Promise<void> {
@@ -134,20 +148,8 @@ export class Docker {
      * Detects whether the docker command is actually podman.
      * To do this, it looks for "podman" in the output of "docker --help".
      */
-    static isPodman(): Promise<boolean> {
-        return new Promise<boolean>((resolve, reject) => {
-            childProcess.execFile("docker", ["--help"], (err, stdout) => {
-                if (err) reject(err);
-                else resolve(stdout.toLowerCase().includes("podman"));
-            });
-        });
-    }
-
-    /**
-     * Supply the right hostname to use to talk to the host machine. On Docker this
-     * is "host.docker.internal" and on Podman this is "host.containers.internal".
-     */
-    static async hostnameOfHost(): Promise<"host.containers.internal" | "host.docker.internal"> {
-        return (await Docker.isPodman()) ? "host.containers.internal" : "host.docker.internal";
+    static async isPodman(): Promise<boolean> {
+        const { stdout } = await exec("docker", ["--help"], true);
+        return stdout.toLowerCase().includes("podman");
     }
 }

playwright/plugins/homeserver/dendrite/index.ts

@@ -46,7 +46,6 @@ export class Dendrite extends Synapse implements Homeserver, HomeserverInstance
         const dendriteId = await this.docker.run({
             image: this.image,
             params: [
-                "--rm",
                 "-v",
                 `${denCfg.configDir}:` + dockerConfigDir,
                 "-p",
@@ -140,7 +139,7 @@ async function cfgDirFromTemplate(
     const docker = new Docker();
     await docker.run({
         image: dendriteImage,
-        params: ["--rm", "--entrypoint=", "-v", `${tempDir}:/mnt`],
+        params: ["--entrypoint=", "-v", `${tempDir}:/mnt`],
         containerName: `react-sdk-playwright-dendrite-keygen`,
         cmd: ["/usr/bin/generate-keys", "-private-key", "/mnt/matrix_key.pem"],
     });

playwright/plugins/homeserver/synapse/index.ts

@@ -57,20 +57,9 @@ async function cfgDirFromTemplate(opts: StartHomeserverOpts): Promise<Omit<Homes
     if (opts.oAuthServerPort) {
         hsYaml = hsYaml.replace(/{{OAUTH_SERVER_PORT}}/g, opts.oAuthServerPort.toString());
     }
-    hsYaml = hsYaml.replace(/{{HOST_DOCKER_INTERNAL}}/g, await Docker.hostnameOfHost());
     if (opts.variables) {
-        let fetchedHostContainer: Awaited<ReturnType<typeof Docker.hostnameOfHost>> | null = null;
         for (const key in opts.variables) {
-            let value = String(opts.variables[key]);
-
-            if (value === "{{HOST_DOCKER_INTERNAL}}") {
-                if (!fetchedHostContainer) {
-                    fetchedHostContainer = await Docker.hostnameOfHost();
-                }
-                value = fetchedHostContainer;
-            }
-
-            hsYaml = hsYaml.replace(new RegExp("%" + key + "%", "g"), value);
+            hsYaml = hsYaml.replace(new RegExp("%" + key + "%", "g"), String(opts.variables[key]));
         }
     }
 
@@ -106,26 +95,13 @@ export class Synapse implements Homeserver, HomeserverInstance {
      * Start a synapse instance: the template must be the name of
      * one of the templates in the playwright/plugins/synapsedocker/templates
      * directory.
-     *
-     * Any value in `opts.variables` that is set to `{{HOST_DOCKER_INTERNAL}}'
-     * will be replaced with 'host.docker.internal' (if we are on Docker) or
-     * 'host.containers.internal' if we are on Podman.
      */
     public async start(opts: StartHomeserverOpts): Promise<HomeserverInstance> {
         if (this.config) await this.stop();
 
         const synCfg = await cfgDirFromTemplate(opts);
         console.log(`Starting synapse with config dir ${synCfg.configDir}...`);
-        const dockerSynapseParams = ["--rm", "-v", `${synCfg.configDir}:/data`, "-p", `${synCfg.port}:8008/tcp`];
-        if (await Docker.isPodman()) {
-            // Make host.containers.internal work to allow Synapse to talk to the test OIDC server.
-            dockerSynapseParams.push("--network");
-            dockerSynapseParams.push("slirp4netns:allow_host_loopback=true");
-        } else {
-            // Make host.docker.internal work to allow Synapse to talk to the test OIDC server.
-            dockerSynapseParams.push("--add-host");
-            dockerSynapseParams.push("host.docker.internal:host-gateway");
-        }
+        const dockerSynapseParams = ["-v", `${synCfg.configDir}:/data`, "-p", `${synCfg.port}:8008/tcp`];
         const synapseId = await this.docker.run({
             image: "matrixdotorg/synapse:develop",
             containerName: `react-sdk-playwright-synapse`,

playwright/plugins/homeserver/synapse/templates/default/homeserver.yaml

@@ -81,10 +81,8 @@ oidc_providers:
       issuer: "http://localhost:{{OAUTH_SERVER_PORT}}/oauth"
       authorization_endpoint: "http://localhost:{{OAUTH_SERVER_PORT}}/oauth/auth.html"
       # the token endpoint receives requests from synapse, rather than the webapp, so needs to escape the docker container.
-      # Hence, HOST_DOCKER_INTERNAL rather than localhost. This is set to
-      # host.docker.internal on Docker and host.containers.internal on Podman.
-      token_endpoint: "http://{{HOST_DOCKER_INTERNAL}}:{{OAUTH_SERVER_PORT}}/oauth/token"
-      userinfo_endpoint: "http://{{HOST_DOCKER_INTERNAL}}:{{OAUTH_SERVER_PORT}}/oauth/userinfo"
+      token_endpoint: "http://host.containers.internal:{{OAUTH_SERVER_PORT}}/oauth/token"
+      userinfo_endpoint: "http://host.containers.internal:{{OAUTH_SERVER_PORT}}/oauth/userinfo"
       client_id: "synapse"
       discover: false
       scopes: ["profile"]

playwright/plugins/mailhog/index.ts

@@ -38,7 +38,7 @@ export class MailHogServer {
         const containerId = await this.docker.run({
             image: "mailhog/mailhog:latest",
             containerName: `react-sdk-playwright-mailhog`,
-            params: ["--rm", "-p", `${smtpPort}:1025/tcp`, "-p", `${httpPort}:8025/tcp`],
+            params: ["-p", `${smtpPort}:1025/tcp`, "-p", `${httpPort}:8025/tcp`],
         });
         console.log(`Started mailhog on ports smtp=${smtpPort} http=${httpPort}.`);
         const host = await this.docker.getContainerIp();

playwright/plugins/postgres/index.ts

@@ -0,0 +1,72 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { Docker } from "../docker";
+
+export const PG_PASSWORD = "p4S5w0rD";
+
+/**
+ * Class to manage a postgres database in docker
+ */
+export class PostgresDocker extends Docker {
+    /**
+     * @param key an opaque string to use when naming the docker containers instantiated by this class
+     */
+    public constructor(private key: string) {
+        super();
+    }
+
+    private async waitForPostgresReady(): Promise<void> {
+        const waitTimeMillis = 30000;
+        const startTime = new Date().getTime();
+        let lastErr: Error | null = null;
+        while (new Date().getTime() - startTime < waitTimeMillis) {
+            try {
+                await this.exec(["pg_isready", "-U", "postgres"], true);
+                lastErr = null;
+                break;
+            } catch (err) {
+                console.log("pg_isready: failed");
+                lastErr = err;
+            }
+        }
+        if (lastErr) {
+            console.log("rethrowing");
+            throw lastErr;
+        }
+    }
+
+    public async start(): Promise<{
+        ipAddress: string;
+        containerId: string;
+    }> {
+        console.log(new Date(), "starting postgres container");
+        const containerId = await this.run({
+            image: "postgres",
+            containerName: `react-sdk-playwright-postgres-${this.key}`,
+            params: ["--tmpfs=/pgtmpfs", "-e", "PGDATA=/pgtmpfs", "-e", `POSTGRES_PASSWORD=${PG_PASSWORD}`],
+            // Optimise for testing - https://www.postgresql.org/docs/current/non-durability.html
+            cmd: ["-c", `fsync=off`, "-c", `synchronous_commit=off`, "-c", `full_page_writes=off`],
+        });
+
+        const ipAddress = await this.getContainerIp();
+        console.log(new Date(), "postgres container up");
+
+        await this.waitForPostgresReady();
+        console.log(new Date(), "postgres container ready");
+        return { ipAddress, containerId };
+    }
+}