Skip to content

Commit 8f0d4e9

Browse files
cristianrgrecocristianrgreco
authored
Down and throw if DockerComposeEnvironment wait strategy fails (#188)
1 parent e043609 commit 8f0d4e9

File tree

6 files changed

+93
-45
lines changed

6 files changed

+93
-45
lines changed

‎src/docker-compose-environment.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import path from "path";
33
import { DockerComposeEnvironment } from "./docker-compose-environment";
44
import { Wait } from "./wait";
55
import Dockerode from "dockerode";
6+
import { getRunningContainerNames } from "./test-helper";
67

78
describe("DockerComposeEnvironment", () => {
89
jest.setTimeout(60000);
@@ -74,6 +75,28 @@ describe("DockerComposeEnvironment", () => {
7475
await startedEnvironment.down();
7576
});
7677

78+
it("should stop the container when the log message wait strategy times out", async () => {
79+
await expect(
80+
new DockerComposeEnvironment(fixtures, "docker-compose-with-name.yml")
81+
.withWaitStrategy("custom_container_name", Wait.forLogMessage("unexpected"))
82+
.withStartupTimeout(0)
83+
.up()
84+
).rejects.toThrowError(`Log message "unexpected" not received after 0ms`);
85+
86+
expect(await getRunningContainerNames(dockerodeClient)).not.toContain("custom_container_name");
87+
});
88+
89+
it("should stop the container when the health check wait strategy times out", async () => {
90+
await expect(
91+
new DockerComposeEnvironment(fixtures, "docker-compose-with-healthcheck.yml")
92+
.withWaitStrategy("container_1", Wait.forHealthCheck())
93+
.withStartupTimeout(0)
94+
.up()
95+
).rejects.toThrowError(`Health check not healthy after 0ms`);
96+
97+
expect(await getRunningContainerNames(dockerodeClient)).not.toContain("container_1");
98+
});
99+
77100
it("should support health check wait strategy", async () => {
78101
const startedEnvironment = await new DockerComposeEnvironment(fixtures, "docker-compose-with-healthcheck.yml")
79102
.withWaitStrategy("container_1", Wait.forHealthCheck())

‎src/docker-compose-environment.ts‎

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as dockerCompose from "docker-compose";
2-
import Dockerode from "dockerode";
2+
import Dockerode, { ContainerInfo } from "dockerode";
33
import { BoundPorts } from "./bound-ports";
44
import { Container } from "./container";
55
import { ContainerState } from "./container-state";
@@ -50,7 +50,7 @@ export class DockerComposeEnvironment {
5050
}
5151

5252
public async up(): Promise<StartedDockerComposeEnvironment> {
53-
log.info(`Starting DockerCompose environment`);
53+
log.info(`Starting DockerCompose environment ${this.projectName}`);
5454

5555
const dockerClient = await DockerClientInstance.getInstance();
5656

@@ -61,6 +61,16 @@ export class DockerComposeEnvironment {
6161
(container) => container.Labels["com.docker.compose.project"] === this.projectName
6262
);
6363

64+
const startedContainerNames = startedContainers.reduce(
65+
(containerNames: string[], startedContainer: ContainerInfo) => [
66+
...containerNames,
67+
startedContainer.Names.join(", "),
68+
],
69+
[]
70+
);
71+
72+
log.info(`Started the following containers: ${startedContainerNames.join(", ")}`);
73+
6474
const startedGenericContainers = (
6575
await Promise.all(
6676
startedContainers.map(async (startedContainer) => {
@@ -75,7 +85,20 @@ export class DockerComposeEnvironment {
7585
const boundPorts = this.getBoundPorts(startedContainer);
7686
const containerState = new ContainerState(inspectResult);
7787

78-
await this.waitForContainer(dockerClient, container, containerName, containerState, boundPorts);
88+
try {
89+
log.info(`Waiting for container ${containerName} to be ready`);
90+
await this.waitForContainer(dockerClient, container, containerName, containerState, boundPorts);
91+
log.info(`Container ${containerName} is ready`);
92+
} catch (err) {
93+
log.error(`Container ${containerName} failed to be ready: ${err}`);
94+
95+
try {
96+
await down(this.composeFilePath, this.composeFiles, this.projectName);
97+
} catch {
98+
log.warn(`Failed to stop DockerCompose environment after failed up`);
99+
}
100+
throw err;
101+
}
79102

80103
return new StartedGenericContainer(
81104
container,
@@ -115,22 +138,19 @@ export class DockerComposeEnvironment {
115138
env: { ...defaultOptions.env, ...this.env },
116139
};
117140

118-
log.info(`Starting DockerCompose environment`);
141+
log.info(`Upping DockerCompose environment`);
119142
try {
120143
await dockerCompose.upAll(options);
121-
log.info(`Started DockerCompose environment`);
144+
log.info(`Upped DockerCompose environment`);
122145
} catch (err) {
123-
log.error(`Failed to start DockerCompose environment: ${err}`);
146+
const errorMessage = err.err ? err.err.trim() : err;
147+
log.error(`Failed to up DockerCompose environment: ${errorMessage}`);
124148
try {
125149
await down(this.composeFilePath, this.composeFiles, this.projectName);
126150
} catch {
127-
log.warn(`Failed to stop DockerCompose environment after failed start`);
128-
}
129-
if (err.err) {
130-
throw new Error(err.err.trim());
131-
} else {
132-
throw new Error(err);
151+
log.warn(`Failed to stop DockerCompose environment after failed up`);
133152
}
153+
throw new Error(errorMessage);
134154
}
135155
}
136156

@@ -147,10 +167,8 @@ export class DockerComposeEnvironment {
147167
containerState: ContainerState,
148168
boundPorts: BoundPorts
149169
): Promise<void> {
150-
log.debug("Waiting for container to be ready");
151170
const waitStrategy = this.getWaitStrategy(dockerClient, container, containerName);
152171
await waitStrategy.withStartupTimeout(this.startupTimeout).waitUntilReady(container, containerState, boundPorts);
153-
log.info("Container is ready");
154172
}
155173

156174
private getWaitStrategy(dockerClient: DockerClient, container: Container, containerName: string): WaitStrategy {

‎src/generic-container.test.ts‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { Readable } from "stream";
99
import { RandomUuid } from "./uuid";
1010
import { TestContainers } from "./test-containers";
1111
import { RandomPortClient } from "./port-client";
12+
import { getRunningContainerNames } from "./test-helper";
1213

1314
describe("GenericContainer", () => {
1415
jest.setTimeout(180_000);
@@ -263,7 +264,7 @@ describe("GenericContainer", () => {
263264
.start()
264265
).rejects.toThrowError("Port 8081 not bound after 0ms");
265266

266-
expect(await getRunningContainerNames()).not.toContain(containerName);
267+
expect(await getRunningContainerNames(dockerodeClient)).not.toContain(containerName);
267268
});
268269

269270
it("should stop the container when the log message wait strategy times out", async () => {
@@ -278,7 +279,7 @@ describe("GenericContainer", () => {
278279
.start()
279280
).rejects.toThrowError(`Log message "unexpected" not received after 0ms`);
280281

281-
expect(await getRunningContainerNames()).not.toContain(containerName);
282+
expect(await getRunningContainerNames(dockerodeClient)).not.toContain(containerName);
282283
});
283284

284285
it("should stop the container when the health check wait strategy times out", async () => {
@@ -295,7 +296,7 @@ describe("GenericContainer", () => {
295296
.start()
296297
).rejects.toThrowError("Health check not healthy after 0ms");
297298

298-
expect(await getRunningContainerNames()).not.toContain(containerName);
299+
expect(await getRunningContainerNames(dockerodeClient)).not.toContain(containerName);
299300
});
300301

301302
it("should stop the container when the health check fails", async () => {
@@ -312,7 +313,7 @@ describe("GenericContainer", () => {
312313
.start()
313314
).rejects.toThrowError("Health check failed");
314315

315-
expect(await getRunningContainerNames()).not.toContain(containerName);
316+
expect(await getRunningContainerNames(dockerodeClient)).not.toContain(containerName);
316317
});
317318

318319
it("should honour .dockerignore file", async () => {
@@ -429,12 +430,4 @@ describe("GenericContainer", () => {
429430
await startedContainer.stop();
430431
});
431432
});
432-
433-
const getRunningContainerNames = async (): Promise<string[]> => {
434-
const containers = await dockerodeClient.listContainers();
435-
return containers
436-
.map((container) => container.Names)
437-
.reduce((result, containerNames) => [...result, ...containerNames], [])
438-
.map((containerName) => containerName.replace("/", ""));
439-
};
440433
});

‎src/test-helper.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import Dockerode from "dockerode";
2+
3+
export const getRunningContainerNames = async (dockerode: Dockerode): Promise<string[]> => {
4+
const containers = await dockerode.listContainers();
5+
return containers
6+
.map((container) => container.Names)
7+
.reduce((result, containerNames) => [...result, ...containerNames], [])
8+
.map((containerName) => containerName.replace("/", ""));
9+
};

‎src/wait-strategy.ts‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -42,34 +42,37 @@ export class HostPortWaitStrategy extends AbstractWaitStrategy {
4242
containerState: ContainerState,
4343
boundPorts: BoundPorts
4444
): Promise<void> {
45-
await Promise.all([this.waitForHostPorts(containerState), this.waitForInternalPorts(boundPorts)]);
45+
await Promise.all([
46+
this.waitForHostPorts(container, containerState),
47+
this.waitForInternalPorts(container, boundPorts),
48+
]);
4649
}
4750

48-
private async waitForHostPorts(containerState: ContainerState): Promise<void> {
51+
private async waitForHostPorts(container: Container, containerState: ContainerState): Promise<void> {
4952
for (const hostPort of containerState.getHostPorts()) {
50-
log.debug(`Waiting for host port ${hostPort}`);
51-
await this.waitForPort(hostPort, this.hostPortCheck);
52-
log.debug(`Host port ${hostPort} ready`);
53+
log.debug(`Waiting for host port ${hostPort} for ${container.getId()}`);
54+
await this.waitForPort(container, hostPort, this.hostPortCheck);
55+
log.debug(`Host port ${hostPort} ready for ${container.getId()}`);
5356
}
5457
}
5558

56-
private async waitForInternalPorts(boundPorts: BoundPorts): Promise<void> {
59+
private async waitForInternalPorts(container: Container, boundPorts: BoundPorts): Promise<void> {
5760
for (const [internalPort] of boundPorts.iterator()) {
58-
log.debug(`Waiting for internal port ${internalPort}`);
59-
await this.waitForPort(internalPort, this.internalPortCheck);
60-
log.debug(`Internal port ${internalPort} ready`);
61+
log.debug(`Waiting for internal port ${internalPort} for ${container.getId()}`);
62+
await this.waitForPort(container, internalPort, this.internalPortCheck);
63+
log.debug(`Internal port ${internalPort} ready for ${container.getId()}`);
6164
}
6265
}
6366

64-
private async waitForPort(port: Port, portCheck: PortCheck): Promise<void> {
67+
private async waitForPort(container: Container, port: Port, portCheck: PortCheck): Promise<void> {
6568
const retryStrategy = new IntervalRetryStrategy<boolean, Error>(100);
6669

6770
await retryStrategy.retryUntil(
6871
() => portCheck.isBound(port),
6972
(isBound) => isBound,
7073
() => {
7174
const timeout = this.startupTimeout;
72-
throw new Error(`Port ${port} not bound after ${timeout}ms`);
75+
throw new Error(`Port ${port} not bound after ${timeout}ms for ${container.getId()}`);
7376
},
7477
this.startupTimeout
7578
);
@@ -89,10 +92,11 @@ export class LogWaitStrategy extends AbstractWaitStrategy {
8992

9093
return new Promise((resolve, reject) => {
9194
const startupTimeout = this.startupTimeout;
92-
const timeout = setTimeout(
93-
() => reject(new Error(`Log message "${this.message}" not received after ${startupTimeout}ms`)),
94-
startupTimeout
95-
);
95+
const timeout = setTimeout(() => {
96+
const message = `Log message "${this.message}" not received after ${startupTimeout}ms for ${container.getId()}`;
97+
log.error(message);
98+
reject(new Error(message));
99+
}, startupTimeout);
96100

97101
const comparisonFn: (line: string) => boolean = (line: string) => {
98102
if (this.message instanceof RegExp) {
@@ -120,7 +124,7 @@ export class LogWaitStrategy extends AbstractWaitStrategy {
120124
.on("end", () => {
121125
stream.destroy();
122126
clearTimeout(timeout);
123-
reject(new Error(`Log stream ended and message "${this.message}" was not received`));
127+
reject(new Error(`Log stream ended and message "${this.message}" was not received for ${container.getId()}`));
124128
});
125129
});
126130
}
@@ -137,13 +141,13 @@ export class HealthCheckWaitStrategy extends AbstractWaitStrategy {
137141
(status) => status === "healthy" || status === "unhealthy",
138142
() => {
139143
const timeout = this.startupTimeout;
140-
throw new Error(`Health check not healthy after ${timeout}ms`);
144+
throw new Error(`Health check not healthy after ${timeout}ms for ${container.getId()}`);
141145
},
142146
this.startupTimeout
143147
);
144148

145149
if (status !== "healthy") {
146-
throw new Error(`Health check failed: ${status}`);
150+
throw new Error(`Health check failed: ${status} for ${container.getId()}`);
147151
}
148152
}
149153
}

‎tsconfig.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
"exclude": [
1212
"dist",
1313
"node_modules",
14-
"src/**/*.test.ts"
14+
"src/**/*.test.ts",
15+
"src/test-helper.ts",
1516
]
1617
}

0 commit comments

Comments
 (0)