Skip to content

Commit 7d23bd7

Browse files
committed
fix: address PR review — target deploy host for hooks, return container id from swarm wait, drop dead export
1 parent 327343f commit 7d23bd7

3 files changed

Lines changed: 51 additions & 34 deletions

File tree

apps/dokploy/__test__/deploy/application.deploy-hooks.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ const primeMocks = (app = createMockApplication()) => {
196196
vi.mocked(deploymentService.updateDeployment).mockResolvedValue({} as any);
197197
vi.mocked(hooks.runDeployHook).mockResolvedValue(undefined as any);
198198
vi.mocked(hooks.waitForSwarmServiceRunning).mockResolvedValue(
199-
undefined as any,
199+
"container-id-abc",
200200
);
201201
};
202202

@@ -254,6 +254,7 @@ describe("deployApplication - Deploy Hooks", () => {
254254
);
255255
vi.mocked(hooks.waitForSwarmServiceRunning).mockImplementation(async () => {
256256
order.push("wait");
257+
return "container-id-abc";
257258
});
258259
vi.mocked(hooks.runDeployHook).mockImplementation(async ({ kind }) => {
259260
order.push(`hook:${kind}`);
@@ -270,6 +271,7 @@ describe("deployApplication - Deploy Hooks", () => {
270271
expect.objectContaining({
271272
kind: "post",
272273
command: "npm run migrate",
274+
containerId: "container-id-abc",
273275
}),
274276
);
275277
});

packages/server/src/services/application.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -239,16 +239,19 @@ export const deployApplication = async ({
239239
await mechanizeDockerContainer(application);
240240

241241
if (deployHooks.post?.trim()) {
242-
await waitForSwarmServiceRunning(application.appName, application.serverId);
242+
const containerId = await waitForSwarmServiceRunning(
243+
application.appName,
244+
application.serverId,
245+
);
243246
await runDeployHook({
244247
kind: "post",
245248
appName: application.appName,
246249
serverId: application.serverId,
247250
command: deployHooks.post,
248251
logPath: deployment.logPath,
252+
containerId,
249253
});
250254
}
251-
}
252255

253256
await updateDeploymentStatus(deployment.deploymentId, "done");
254257
await updateApplicationStatus(applicationId, "done");
@@ -346,21 +349,25 @@ export const rebuildApplication = async ({
346349
await runDeployHook({
347350
kind: "pre",
348351
appName: application.appName,
349-
serverId,
352+
serverId: application.serverId,
350353
command: deployHooks.pre,
351354
logPath: deployment.logPath,
352355
});
353356

354357
await mechanizeDockerContainer(application);
355358

356359
if (deployHooks.post?.trim()) {
357-
await waitForSwarmServiceRunning(application.appName, serverId);
360+
const containerId = await waitForSwarmServiceRunning(
361+
application.appName,
362+
application.serverId,
363+
);
358364
await runDeployHook({
359365
kind: "post",
360366
appName: application.appName,
361-
serverId,
367+
serverId: application.serverId,
362368
command: deployHooks.post,
363369
logPath: deployment.logPath,
370+
containerId,
364371
});
365372
}
366373

packages/server/src/utils/docker/hooks.ts

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -27,26 +27,20 @@ export const parseDeployHooks = (
2727
return {};
2828
};
2929

30-
export const serializeDeployHooks = (hooks: DeployHooks): string | null => {
31-
const pre = hooks.pre?.trim();
32-
const post = hooks.post?.trim();
33-
if (!pre && !post) return null;
34-
return JSON.stringify({
35-
...(pre ? { pre } : {}),
36-
...(post ? { post } : {}),
37-
});
38-
};
39-
4030
interface WaitOptions {
4131
timeoutMs?: number;
4232
intervalMs?: number;
4333
}
4434

35+
// Polls a swarm service until a task reaches `State: running` and returns that
36+
// task's container ID. Returning the resolved container ID directly avoids
37+
// racing against `listContainers` during `start-first` rolling updates, where
38+
// both the old and new task briefly share the service label.
4539
export const waitForSwarmServiceRunning = async (
4640
appName: string,
4741
serverId: string | null | undefined,
4842
{ timeoutMs = 120_000, intervalMs = 2_000 }: WaitOptions = {},
49-
): Promise<void> => {
43+
): Promise<string> => {
5044
const deadline = Date.now() + timeoutMs;
5145
const remoteDocker = await getRemoteDocker(serverId);
5246

@@ -66,9 +60,14 @@ export const waitForSwarmServiceRunning = async (
6660
}),
6761
});
6862
const runningTask = tasks.find((t) => t.Status?.State === "running");
69-
if (runningTask) return;
70-
const latestTask = tasks[0];
71-
lastError = `Service task state: ${latestTask?.Status?.State ?? "unknown"}`;
63+
const containerId = runningTask?.Status?.ContainerStatus?.ContainerID;
64+
if (runningTask && containerId) return containerId;
65+
if (runningTask && !containerId) {
66+
lastError = "Running task has no container id yet";
67+
} else {
68+
const latestTask = tasks[0];
69+
lastError = `Service task state: ${latestTask?.Status?.State ?? "unknown"}`;
70+
}
7271
} else {
7372
lastError = "Service has 0 replicas";
7473
}
@@ -92,6 +91,11 @@ interface RunDeployHookParams {
9291
serverId: string | null | undefined;
9392
command: string | null | undefined;
9493
logPath: string;
94+
// If provided, skip the label-based container lookup and exec against this
95+
// container id directly. Post-deploy uses this to target the exact task
96+
// that `waitForSwarmServiceRunning` observed as running, avoiding any
97+
// ambiguity when multiple tasks briefly share the service label.
98+
containerId?: string;
9599
}
96100

97101
export const runDeployHook = async ({
@@ -100,30 +104,34 @@ export const runDeployHook = async ({
100104
serverId,
101105
command,
102106
logPath,
107+
containerId,
103108
}: RunDeployHookParams): Promise<void> => {
104109
const trimmed = command?.trim();
105110
if (!trimmed) return;
106111

107-
const container = await getServiceContainer(appName, serverId);
108-
109-
if (!container) {
110-
if (kind === "pre") {
111-
const skipLine = `echo "===== No previous container found; skipping pre-deploy hook =====" >> "${logPath}"`;
112-
if (serverId) {
113-
await execAsyncRemote(serverId, skipLine);
114-
} else {
115-
await execAsync(skipLine);
112+
let resolvedContainerId = containerId;
113+
if (!resolvedContainerId) {
114+
const container = await getServiceContainer(appName, serverId);
115+
if (!container) {
116+
if (kind === "pre") {
117+
const skipLine = `echo "===== No previous container found; skipping pre-deploy hook =====" >> "${logPath}"`;
118+
if (serverId) {
119+
await execAsyncRemote(serverId, skipLine);
120+
} else {
121+
await execAsync(skipLine);
122+
}
123+
return;
116124
}
117-
return;
125+
throw new Error(
126+
`post-deploy hook: no running container found for "${appName}"`,
127+
);
118128
}
119-
throw new Error(
120-
`post-deploy hook: no running container found for "${appName}"`,
121-
);
129+
resolvedContainerId = container.Id;
122130
}
123131

124132
const label = kind === "pre" ? "pre-deploy" : "post-deploy";
125133
const encoded = encodeBase64(trimmed);
126-
const scriptWrapper = `hook_cmd=$(echo "${encoded}" | base64 -d) && docker exec "${container.Id}" sh -c "$hook_cmd"`;
134+
const scriptWrapper = `hook_cmd=$(echo "${encoded}" | base64 -d) && docker exec "${resolvedContainerId}" sh -c "$hook_cmd"`;
127135
const wrappedCommand = `(echo "===== Running ${label} hook (length=${trimmed.length} chars) =====" && ${scriptWrapper} && echo "===== ${label} hook finished =====") >> "${logPath}" 2>&1`;
128136

129137
if (serverId) {

0 commit comments

Comments
 (0)