Skip to content
Merged
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
43 changes: 42 additions & 1 deletion packages/backend/src/docker/docker.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DockerAPIHttpError, DockerSocket } from '@hallmaster/docker.js';
import type { Cluster } from '@hallmaster/prisma-client';
import type { Bot, Cluster } from '@hallmaster/prisma-client';
import { ConfigService } from '@nestjs/config';
import type { TestingModule } from '@nestjs/testing';
import { Test } from '@nestjs/testing';
Expand Down Expand Up @@ -68,4 +68,45 @@ describe('DockerService', () => {
});
});
});

describe('start() with name collision', () => {
const bot: Bot = {
id: 'bot-1',
token: 'discord-token',
totalShards: 1,
} as unknown as Bot;

const cluster: Cluster = {
botId: 'bot-1',
id: 0,
containerId: null,
shardIds: [0],
status: 'STOPPED',
};

it('cleans up the stale container and retries when create returns 409', async () => {
configService.getOrThrow.mockReturnValue('ENV_NAME');
prismaService.dockerImage.findUnique.mockResolvedValue({
botId: 'bot-1',
serverName: 'docker.io',
image: 'foo/bar',
tag: 'latest',
username: null,
password: null,
});

// Image pull succeeds
// First create attempt fails with 409. Cleanup succeeds.
// Second create attempt succeeds. Then start succeeds.
dockerSocket.apiCall
.mockResolvedValueOnce(undefined) // pull
.mockRejectedValueOnce(new DockerAPIHttpError(409, 'name already in use')) // create #1
.mockResolvedValueOnce(undefined) // stop the stale one
.mockResolvedValueOnce(undefined) // remove the stale one
.mockResolvedValueOnce({ Id: 'new-container-id' } as never) // create #2
.mockResolvedValueOnce(undefined); // start

await expect(service.start(bot, cluster)).resolves.toBe('new-container-id');
});
});
});
49 changes: 46 additions & 3 deletions packages/backend/src/docker/docker.service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import type { Readable } from 'node:stream';

import { DockerContainersAPI, DockerImagesAPI, DockerSocket, DockerAPIHttpError } from '@hallmaster/docker.js';
import {
DockerContainersAPI,
DockerImagesAPI,
DockerSocket,
DockerAPIHttpError,
type DockerContainerCreated,
type DockerContainerCreationBody,
} from '@hallmaster/docker.js';
import { Bot, Cluster, DockerImage } from '@hallmaster/prisma-client';
import { BadRequestException, Injectable, InternalServerErrorException, NotFoundException } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
Expand All @@ -16,8 +23,12 @@ export class DockerService {
private readonly dockerSocket: DockerSocket,
) {}

private isDockerApiError(error: unknown, status: number): boolean {
return error instanceof DockerAPIHttpError && error.status === status;
}

private isContainerNotFound(error: unknown): boolean {
return error instanceof DockerAPIHttpError && error.status === 404;
return this.isDockerApiError(error, 404);
}

private async clearContainerId(cluster: Cluster): Promise<void> {
Expand All @@ -27,6 +38,37 @@ export class DockerService {
});
}

private async cleanupStaleContainerByName(name: string): Promise<void> {
const api = new DockerContainersAPI(this.dockerSocket);

try {
await api.stop(name);
} catch {
// proceed to remove even if stop fails
}

try {
await api.remove(name);
} catch (e) {
// Any 404 is treated as success: the container is gone, which is the goal.
if (!this.isContainerNotFound(e)) throw e;
}
}

private async createContainerOrCleanup(
api: DockerContainersAPI,
body: Partial<DockerContainerCreationBody>,
name: string,
): Promise<DockerContainerCreated> {
try {
return await api.create(body, name);
} catch (e) {
if (!this.isDockerApiError(e, 409)) throw e;
await this.cleanupStaleContainerByName(name);
return await api.create(body, name);
}
}

async verifyImage(dockerImage: {
serverName: string;
image: string;
Expand Down Expand Up @@ -140,7 +182,8 @@ export class DockerService {
if (null === containerId) {
await this.pullDockerImage(cluster, dockerImage);
try {
const container = await dockerContainersAPI.create(
const container = await this.createContainerOrCleanup(
dockerContainersAPI,
{
Env: [
`${discordBotTokenEnvName}=${bot.token}`,
Expand Down
Loading