Skip to content

Container stop timeouts should be in milliseconds #962

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/features/compose.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,11 @@ const environment = await new DockerComposeEnvironment(composeFilePath, composeF
await environment.down();
```

If you need to wait for the environment to be downed, you can provide a timeout. The unit of timeout here is **second**:
If you need to wait for the environment to be downed, you can provide a timeout:

```javascript
const environment = await new DockerComposeEnvironment(composeFilePath, composeFile).up();
await environment.down({ timeout: 10 }); // timeout after 10 seconds
await environment.down({ timeout: 10_000 }); // 10 seconds
```

Volumes created by the environment are removed when stopped. This is configurable:
Expand Down
4 changes: 2 additions & 2 deletions docs/features/containers.md
Original file line number Diff line number Diff line change
Expand Up @@ -334,11 +334,11 @@ const container = await new GenericContainer("alpine").start();
await container.stop();
```

If you need to wait for the container to be stopped, you can provide a timeout. The unit of timeout option here is **second**:
If you need to wait for the container to be stopped, you can provide a timeout:

```javascript
const container = await new GenericContainer("alpine").start();
await container.stop({ timeout: 10 }); // 10 seconds
await container.stop({ timeout: 10_000 }); // 10 seconds
```

You can disable automatic removal of the container, which is useful for debugging, or if for example you want to copy content from the container once it has stopped:
Expand Down
30 changes: 15 additions & 15 deletions docs/features/wait-strategies.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
# Wait Strategies

Note that the startup timeout of all wait strategies is configurable. The unit of timeout of wait strategies is **millisecond**:
Note that the startup timeout of all wait strategies is configurable:

```javascript
const { GenericContainer } = require("testcontainers");

const container = await new GenericContainer("alpine")
.withStartupTimeout(120000) // wait 120 seconds
.withStartupTimeout(120_000) // 120 seconds
.start();
```

Expand Down Expand Up @@ -73,18 +73,18 @@ const { GenericContainer, Wait } = require("testcontainers");
const container = await new GenericContainer("alpine").withWaitStrategy(Wait.forHealthCheck()).start();
```

Define your own health check. The unit of timeouts and intervals here is **millisecond**:
Define your own health check:

```javascript
const { GenericContainer, Wait } = require("testcontainers");

const container = await new GenericContainer("alpine")
.withHealthCheck({
test: ["CMD-SHELL", "curl -f http://localhost || exit 1"],
interval: 1000,
timeout: 3000,
interval: 1000, // 1 second
timeout: 3000, // 3 seconds
retries: 5,
startPeriod: 1000,
startPeriod: 1000, // 1 second
})
.withWaitStrategy(Wait.forHealthCheck())
.start();
Expand Down Expand Up @@ -148,7 +148,7 @@ const container = await new GenericContainer("redis")
.withMethod("POST")
.withHeaders({ X_CUSTOM_VALUE: "custom" })
.withBasicCredentials("username", "password")
.withReadTimeout(10000)) // timeout after 10 seconds
.withReadTimeout(10_000)) // 10 seconds
```

### Use TLS
Expand Down Expand Up @@ -186,7 +186,7 @@ This strategy is intended for use with containers that only run briefly and exit
const { GenericContainer, Wait } = require("testcontainers");

const container = await new GenericContainer("alpine")
.withWaitStrategy(Wait.forOneShotStartup()))
.withWaitStrategy(Wait.forOneShotStartup())
.start();
```

Expand All @@ -202,11 +202,11 @@ const container = await new GenericContainer("alpine")
.start();
```

The composite wait strategy by default will respect each individual wait strategy's startup timeout. The unit of timeouts here is **millisecond**. For example:
The composite wait strategy by default will respect each individual wait strategy's startup timeout. For example:

```javascript
const w1 = Wait.forListeningPorts().withStartupTimeout(1000); // wait 1 second
const w2 = Wait.forLogMessage("READY").withStartupTimeout(2000); // wait 2 seconds
const w1 = Wait.forListeningPorts().withStartupTimeout(1000); // 1 second
const w2 = Wait.forLogMessage("READY").withStartupTimeout(2000); // 2 seconds

const composite = Wait.forAll([w1, w2]);

Expand All @@ -217,21 +217,21 @@ expect(w2.getStartupTimeout()).toBe(2000);
The startup timeout of inner wait strategies that have not defined their own startup timeout can be set by setting the startup timeout on the composite:

```javascript
const w1 = Wait.forListeningPorts().withStartupTimeout(1000); // wait 1 second
const w1 = Wait.forListeningPorts().withStartupTimeout(1000); // 1 second
const w2 = Wait.forLogMessage("READY");

const composite = Wait.forAll([w1, w2]).withStartupTimeout(2000); // wait 2 seconds
const composite = Wait.forAll([w1, w2]).withStartupTimeout(2000); // 2 seconds

expect(w1.getStartupTimeout()).toBe(1000);
expect(w2.getStartupTimeout()).toBe(2000);
```

The startup timeout of all wait strategies can be controlled by setting a deadline on the composite. In this case, the composite will throw unless all inner wait strategies have resolved before the deadline. The unit of deadline timeout is **millisecond**.
The startup timeout of all wait strategies can be controlled by setting a deadline on the composite. In this case, the composite will throw unless all inner wait strategies have resolved before the deadline.

```javascript
const w1 = Wait.forListeningPorts();
const w2 = Wait.forLogMessage("READY");
const composite = Wait.forAll([w1, w2]).withDeadline(2000); // wait 2 seconds
const composite = Wait.forAll([w1, w2]).withDeadline(2000); // 2 seconds
```

## Other startup strategies
Expand Down
1 change: 1 addition & 0 deletions packages/testcontainers/src/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ export { hash } from "./hash";
export { buildLog, composeLog, containerLog, execLog, log, Logger, pullLog } from "./logger";
export { IntervalRetry, Retry } from "./retry";
export { streamToString } from "./streams";
export * from "./time";
export * from "./type-guards";
export { RandomUuid, Uuid } from "./uuid";
12 changes: 6 additions & 6 deletions packages/testcontainers/src/common/retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export interface Retry<T, U> {
fn: () => Promise<T>,
predicate: (result: T) => boolean | Promise<boolean>,
onTimeout: () => U,
timeout: number
timeoutMs: number
): Promise<T | U>;
}

Expand All @@ -16,11 +16,11 @@ abstract class AbstractRetry<T, U> implements Retry<T, U> {
fn: () => Promise<T>,
predicate: (result: T) => boolean | Promise<boolean>,
onTimeout: () => U,
timeout: number
timeoutMs: number
): Promise<T | U>;

protected hasTimedOut(timeout: number, startTime: Time): boolean {
return this.clock.getTime() - startTime > timeout;
protected hasTimedOut(timeoutMs: number, startTime: Time): boolean {
return this.clock.getTime() - startTime > timeoutMs;
}

protected wait(duration: number): Promise<void> {
Expand All @@ -37,15 +37,15 @@ export class IntervalRetry<T, U> extends AbstractRetry<T, U> {
fn: (attempt: number) => Promise<T>,
predicate: (result: T) => boolean | Promise<boolean>,
onTimeout: () => U,
timeout: number
timeoutMs: number
): Promise<T | U> {
const startTime = this.clock.getTime();

let attemptNumber = 0;
let result = await fn(attemptNumber++);

while (!(await predicate(result))) {
if (this.hasTimedOut(timeout, startTime)) {
if (this.hasTimedOut(timeoutMs, startTime)) {
return onTimeout();
}
await this.wait(this.interval);
Expand Down
25 changes: 25 additions & 0 deletions packages/testcontainers/src/common/time.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { toNanos, toSeconds } from "./time";

test.for([
[0, 0],
[10, 0],
[999, 0],
[1010, 1],
[1999, 1],
[10_000, 10],
[-10, -0],
[-999, -0],
[-1010, -1],
[-1999, -1],
[-10_000, -10],
])("should convert %i ms to %i seconds", ([ms, s]) => {
expect(toSeconds(ms)).toEqual(s);
});

test.for([
[0, 0],
[1, 1_000_000],
[-1, -1_000_000],
])("should convert %i ms to %i ns", ([ms, ns]) => {
expect(toNanos(ms)).toEqual(ns);
});
3 changes: 3 additions & 0 deletions packages/testcontainers/src/common/time.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const toSeconds = (ms: number) => Math.trunc(ms * 1e-3);

export const toNanos = (ms: number) => ms * 1e6;
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { default as dockerComposeV1, default as v1, v2 as dockerComposeV2, v2 } from "docker-compose";
import { log, pullLog } from "../../../common";
import { log, pullLog, toSeconds } from "../../../common";
import { ComposeInfo } from "../types";
import { defaultComposeOptions } from "./default-compose-options";
import { ComposeDownOptions, ComposeOptions } from "./types";
Expand Down Expand Up @@ -53,10 +53,10 @@ class ComposeV1Client implements ComposeClient {
try {
if (services) {
log.info(`Upping Compose environment services ${services.join(", ")}...`);
await v1.upMany(services, await defaultComposeOptions(this.environment, options));
await v1.upMany(services, defaultComposeOptions(this.environment, options));
} else {
log.info(`Upping Compose environment...`);
await v1.upAll(await defaultComposeOptions(this.environment, options));
await v1.upAll(defaultComposeOptions(this.environment, options));
}
log.info(`Upped Compose environment`);
} catch (err) {
Expand All @@ -75,10 +75,10 @@ class ComposeV1Client implements ComposeClient {
try {
if (services) {
log.info(`Pulling Compose environment images "${services.join('", "')}"...`);
await v1.pullMany(services, await defaultComposeOptions(this.environment, { ...options, logger: pullLog }));
await v1.pullMany(services, defaultComposeOptions(this.environment, { ...options, logger: pullLog }));
} else {
log.info(`Pulling Compose environment images...`);
await v1.pullAll(await defaultComposeOptions(this.environment, { ...options, logger: pullLog }));
await v1.pullAll(defaultComposeOptions(this.environment, { ...options, logger: pullLog }));
}
log.info(`Pulled Compose environment`);
} catch (err) {
Expand All @@ -91,7 +91,7 @@ class ComposeV1Client implements ComposeClient {
async stop(options: ComposeOptions): Promise<void> {
try {
log.info(`Stopping Compose environment...`);
await v1.stop(await defaultComposeOptions(this.environment, options));
await v1.stop(defaultComposeOptions(this.environment, options));
log.info(`Stopped Compose environment`);
} catch (err) {
await handleAndRethrow(err, async (error: Error) =>
Expand All @@ -104,7 +104,7 @@ class ComposeV1Client implements ComposeClient {
try {
log.info(`Downing Compose environment...`);
await v1.down({
...(await defaultComposeOptions(this.environment, options)),
...defaultComposeOptions(this.environment, options),
commandOptions: composeDownCommandOptions(downOptions),
});
log.info(`Downed Compose environment`);
Expand All @@ -126,10 +126,10 @@ class ComposeV2Client implements ComposeClient {
try {
if (services) {
log.info(`Upping Compose environment services ${services.join(", ")}...`);
await v2.upMany(services, await defaultComposeOptions(this.environment, options));
await v2.upMany(services, defaultComposeOptions(this.environment, options));
} else {
log.info(`Upping Compose environment...`);
await v2.upAll(await defaultComposeOptions(this.environment, options));
await v2.upAll(defaultComposeOptions(this.environment, options));
}
log.info(`Upped Compose environment`);
} catch (err) {
Expand All @@ -148,10 +148,10 @@ class ComposeV2Client implements ComposeClient {
try {
if (services) {
log.info(`Pulling Compose environment images "${services.join('", "')}"...`);
await v2.pullMany(services, await defaultComposeOptions(this.environment, { ...options, logger: pullLog }));
await v2.pullMany(services, defaultComposeOptions(this.environment, { ...options, logger: pullLog }));
} else {
log.info(`Pulling Compose environment images...`);
await v2.pullAll(await defaultComposeOptions(this.environment, { ...options, logger: pullLog }));
await v2.pullAll(defaultComposeOptions(this.environment, { ...options, logger: pullLog }));
}
log.info(`Pulled Compose environment`);
} catch (err) {
Expand All @@ -164,7 +164,7 @@ class ComposeV2Client implements ComposeClient {
async stop(options: ComposeOptions): Promise<void> {
try {
log.info(`Stopping Compose environment...`);
await v2.stop(await defaultComposeOptions(this.environment, options));
await v2.stop(defaultComposeOptions(this.environment, options));
log.info(`Stopped Compose environment`);
} catch (err) {
await handleAndRethrow(err, async (error: Error) =>
Expand All @@ -177,7 +177,7 @@ class ComposeV2Client implements ComposeClient {
try {
log.info(`Downing Compose environment...`);
await v2.down({
...(await defaultComposeOptions(this.environment, options)),
...defaultComposeOptions(this.environment, options),
commandOptions: composeDownCommandOptions(downOptions),
});
log.info(`Downed Compose environment`);
Expand Down Expand Up @@ -222,7 +222,7 @@ function composeDownCommandOptions(options: ComposeDownOptions): string[] {
result.push("-v");
}
if (options.timeout) {
result.push("-t", `${options.timeout / 1000}`);
result.push("-t", `${toSeconds(options.timeout)}`);
}
return result;
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import Dockerode, {
} from "dockerode";
import { IncomingMessage } from "http";
import { PassThrough, Readable } from "stream";
import { execLog, log, streamToString } from "../../../common";
import { execLog, log, streamToString, toSeconds } from "../../../common";
import { ContainerClient } from "./container-client";
import { ContainerCommitOptions, ContainerStatus, ExecOptions, ExecResult } from "./types";

Expand Down Expand Up @@ -120,8 +120,7 @@ export class DockerContainerClient implements ContainerClient {

async inspect(container: Dockerode.Container): Promise<ContainerInspectInfo> {
try {
const inspectInfo = await container.inspect();
return inspectInfo;
return await container.inspect();
} catch (err) {
log.error(`Failed to inspect container: ${err}`, { containerId: container.id });
throw err;
Expand All @@ -131,7 +130,7 @@ export class DockerContainerClient implements ContainerClient {
async stop(container: Container, opts?: { timeout: number }): Promise<void> {
try {
log.debug(`Stopping container...`, { containerId: container.id });
await container.stop({ t: opts?.timeout });
await container.stop({ t: toSeconds(opts?.timeout ?? 0) });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: these are the reasons why this is a breaking change

log.debug(`Stopped container`, { containerId: container.id });
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} catch (err: any) {
Expand Down Expand Up @@ -267,7 +266,7 @@ export class DockerContainerClient implements ContainerClient {
async restart(container: Container, opts?: { timeout: number }): Promise<void> {
try {
log.debug(`Restarting container...`, { containerId: container.id });
await container.restart({ t: opts?.timeout });
await container.restart({ t: toSeconds(opts?.timeout ?? 0) });
log.debug(`Restarted container`, { containerId: container.id });
} catch (err) {
log.error(`Failed to restart container: ${err}`, { containerId: container.id });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class DockerComposeEnvironment {
private pullPolicy: ImagePullPolicy = PullPolicy.defaultPolicy();
private defaultWaitStrategy: WaitStrategy = Wait.forListeningPorts();
private waitStrategy: { [containerName: string]: WaitStrategy } = {};
private startupTimeout?: number;
private startupTimeoutMs?: number;

constructor(composeFilePath: string, composeFiles: string | string[], uuid: Uuid = new RandomUuid()) {
this.composeFilePath = composeFilePath;
Expand Down Expand Up @@ -74,8 +74,8 @@ export class DockerComposeEnvironment {
return this;
}

public withStartupTimeout(startupTimeout: number): this {
this.startupTimeout = startupTimeout;
public withStartupTimeout(startupTimeoutMs: number): this {
this.startupTimeoutMs = startupTimeoutMs;
return this;
}

Expand Down Expand Up @@ -147,8 +147,8 @@ export class DockerComposeEnvironment {
const waitStrategy = this.waitStrategy[containerName]
? this.waitStrategy[containerName]
: this.defaultWaitStrategy;
if (this.startupTimeout !== undefined) {
waitStrategy.withStartupTimeout(this.startupTimeout);
if (this.startupTimeoutMs !== undefined) {
waitStrategy.withStartupTimeout(this.startupTimeoutMs);
}

if (containerLog.enabled()) {
Expand Down
Loading