From bee6918d9a7d2403cd02d107705ba0487c24c450 Mon Sep 17 00:00:00 2001 From: Narciso Date: Wed, 19 Aug 2026 22:47:10 -0400 Subject: [PATCH] fix(monitoring): remove legacy container and default empty cronJob The swarm migration in 3848fa9c0 dropped the container.remove({force:true}) that the standalone deploy path used to run. Swarm tasks are named dokploy-monitoring.., so there is no name collision and the pre-v0.30.0 container survives every redeploy. It also stays pinned to an orphaned image ID once pullRemoteImage moves the latest tag, so neither a pull nor a Save clears it and it restarts forever. Cloud setup spread metricsConfig straight from the row, shipping cronJob: "" to the agent. robfig/cron rejects an empty spec, so the Go binary exits before Fiber binds 4500 and Docker restarts it every ~60s. - remove the legacy container in deployMonitoringService, which covers both setupMonitoring and setupWebMonitoring. Cleanup is best effort: a failure is logged and the deploy continues, matching the pre-migration behaviour - default cronJob when configuring monitoring for cloud - on build servers, clean up the legacy container but deploy no service. They never join the swarm, yet cloud setup did create the standalone container there before v0.30.0, and the monitoring form has always been hidden for them, so those agents are all stuck with an empty cronJob - cover the above with real-docker tests --- .../setup/monitoring-setup.real.test.ts | 258 ++++++++++++++++++ packages/server/src/setup/monitoring-setup.ts | 26 ++ packages/server/src/setup/server-setup.ts | 1 + 3 files changed, 285 insertions(+) create mode 100644 apps/dokploy/__test__/setup/monitoring-setup.real.test.ts diff --git a/apps/dokploy/__test__/setup/monitoring-setup.real.test.ts b/apps/dokploy/__test__/setup/monitoring-setup.real.test.ts new file mode 100644 index 000000000..60594329d --- /dev/null +++ b/apps/dokploy/__test__/setup/monitoring-setup.real.test.ts @@ -0,0 +1,258 @@ +import { execSync } from "node:child_process"; +import { docker } from "@dokploy/server/constants"; +import { setupMonitoring } from "@dokploy/server/setup/monitoring-setup"; +import { afterAll, beforeEach, describe, expect, it, vi } from "vitest"; + +const REAL_TEST_TIMEOUT = 120000; +const SERVICE_NAME = "dokploy-monitoring"; +const TEST_IMAGE = "busybox:latest"; + +// Mock ONLY db-backed lookups and remote I/O. Dockerode stays real and talks to +// the local daemon, so the legacy-container cleanup is exercised for real. +vi.mock("@dokploy/server/services/server", () => ({ + findServerById: vi.fn().mockResolvedValue({ + serverId: "test-server", + serverType: "deploy", + sshKeyId: null, // -> getRemoteDocker returns the local docker instance + metricsConfig: { + server: { + type: "Remote", + port: 4500, + token: "test-token", + urlCallback: "http://localhost/callback", + cronJob: "0 0 * * *", + retentionDays: 2, + refreshRate: 60, + thresholds: { cpu: 0, memory: 0 }, + }, + containers: { refreshRate: 60, services: { include: [], exclude: [] } }, + }, + }), +})); + +vi.mock("@dokploy/server/services/settings", () => ({ + getDokployImageTag: vi.fn(() => "latest"), +})); + +vi.mock("@dokploy/server/utils/docker/utils", () => ({ + pullImage: vi.fn().mockResolvedValue(undefined), + pullRemoteImage: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@dokploy/server/utils/process/execAsync", () => ({ + execAsync: vi.fn().mockResolvedValue({ stdout: "", stderr: "" }), + execAsyncRemote: vi.fn().mockResolvedValue({ stdout: "", stderr: "" }), +})); + +const containerExists = async (name: string) => { + try { + await docker.getContainer(name).inspect(); + return true; + } catch (error: any) { + if (error.statusCode === 404) return false; + throw error; + } +}; + +const serviceExists = async (name: string) => { + try { + await docker.getService(name).inspect(); + return true; + } catch (error: any) { + if (error.statusCode === 404) return false; + throw error; + } +}; + +const swarmTaskNames = async () => { + const list = await docker.listContainers({ all: true }); + return list + .flatMap((c) => c.Names.map((n) => n.replace(/^\//, ""))) + .filter((n) => n.startsWith(`${SERVICE_NAME}.`)); +}; + +const cleanup = async () => { + try { + await docker.getService(SERVICE_NAME).remove(); + } catch {} + try { + await docker.getContainer(SERVICE_NAME).remove({ force: true }); + } catch {} + for (const name of await swarmTaskNames()) { + try { + await docker.getContainer(name).remove({ force: true }); + } catch {} + } +}; + +// Recreates the pre-v0.30.0 standalone agent stuck in a crash loop. +const createLegacyZombie = async () => { + const container = await docker.createContainer({ + name: SERVICE_NAME, + Image: TEST_IMAGE, + Cmd: [ + "sh", + "-c", + "echo 'Error starting metrics cleanup system: empty spec string'; exit 1", + ], + HostConfig: { RestartPolicy: { Name: "always" }, NetworkMode: "host" }, + }); + await container.start(); +}; + +const pullTestImage = async () => { + try { + await docker.getImage(TEST_IMAGE).inspect(); + return; + } catch {} + await new Promise((resolve, reject) => + docker.pull(TEST_IMAGE, (err: any, stream: any) => + err + ? reject(err) + : docker.modem.followProgress(stream, (e: any) => + e ? reject(e) : resolve(), + ), + ), + ); +}; + +// The code under test hardcodes the production name, so this suite cannot +// namespace its fixtures. Skip rather than wipe a real agent on a Dokploy host. +const hasRealMonitoring = () => { + const query = (cmd: string) => { + try { + return execSync(cmd, { stdio: ["ignore", "pipe", "ignore"] }) + .toString() + .trim(); + } catch { + return ""; + } + }; + + return ( + query( + `docker service ls --filter name=${SERVICE_NAME} --format '{{.Name}}'`, + ) !== "" || + query( + `docker ps -a --filter name=^${SERVICE_NAME}$ --format '{{.Names}}'`, + ) !== "" + ); +}; + +describe.skipIf(hasRealMonitoring())( + "setupMonitoring - legacy container cleanup (real docker)", + () => { + beforeEach(async () => { + await pullTestImage(); + await cleanup(); + }, REAL_TEST_TIMEOUT); + + afterAll(async () => { + await cleanup(); + }, REAL_TEST_TIMEOUT); + + it( + "removes the legacy standalone container left behind by the swarm migration", + async () => { + await createLegacyZombie(); + expect(await containerExists(SERVICE_NAME)).toBe(true); + + await setupMonitoring("test-server"); + + expect(await containerExists(SERVICE_NAME)).toBe(false); + expect(await serviceExists(SERVICE_NAME)).toBe(true); + }, + REAL_TEST_TIMEOUT, + ); + + it( + "removes the legacy container without touching swarm tasks, which are named dokploy-monitoring..", + async () => { + const taskName = `${SERVICE_NAME}.1.qh3ldvg2h9x0test`; + const task = await docker.createContainer({ + name: taskName, + Image: TEST_IMAGE, + Cmd: ["sleep", "3600"], + }); + await task.start(); + await createLegacyZombie(); + + await setupMonitoring("test-server"); + + expect(await containerExists(SERVICE_NAME)).toBe(false); + expect(await containerExists(taskName)).toBe(true); + const inspect = await docker.getContainer(taskName).inspect(); + expect(inspect.State.Running).toBe(true); + }, + REAL_TEST_TIMEOUT, + ); + + it( + "is idempotent when no legacy container exists", + async () => { + expect(await containerExists(SERVICE_NAME)).toBe(false); + + await expect(setupMonitoring("test-server")).resolves.not.toThrow(); + await expect(setupMonitoring("test-server")).resolves.not.toThrow(); + + expect(await serviceExists(SERVICE_NAME)).toBe(true); + }, + REAL_TEST_TIMEOUT, + ); + + it( + "cleans up the legacy container on a build server but deploys no service", + async () => { + const { findServerById } = await import( + "@dokploy/server/services/server" + ); + const server = await vi.mocked(findServerById)("test-server"); + vi.mocked(findServerById).mockResolvedValueOnce({ + ...server, + serverType: "build", + } as any); + + await createLegacyZombie(); + + await setupMonitoring("test-server"); + + expect(await containerExists(SERVICE_NAME)).toBe(false); + expect(await serviceExists(SERVICE_NAME)).toBe(false); + }, + REAL_TEST_TIMEOUT, + ); + + it( + "deploys the service even when removing the legacy container fails", + async () => { + const failingDocker = { + getContainer: () => ({ + remove: async () => { + const error: any = new Error("device or resource busy"); + error.statusCode = 500; + throw error; + }, + }), + getService: docker.getService.bind(docker), + createService: docker.createService.bind(docker), + }; + + const remoteDocker = await import( + "@dokploy/server/utils/servers/remote-docker" + ); + const spy = vi + .spyOn(remoteDocker, "getRemoteDocker") + .mockResolvedValue(failingDocker as any); + + try { + await expect(setupMonitoring("test-server")).resolves.not.toThrow(); + expect(spy).toHaveBeenCalled(); // guards against the spy silently not intercepting + expect(await serviceExists(SERVICE_NAME)).toBe(true); + } finally { + spy.mockRestore(); + } + }, + REAL_TEST_TIMEOUT, + ); + }, +); diff --git a/packages/server/src/setup/monitoring-setup.ts b/packages/server/src/setup/monitoring-setup.ts index b2d3f6f4d..1d9e18a83 100644 --- a/packages/server/src/setup/monitoring-setup.ts +++ b/packages/server/src/setup/monitoring-setup.ts @@ -21,11 +21,31 @@ const getMonitoringImage = () => { return imageName; }; +// Swarm tasks are dokploy-monitoring.., so this only matches the +// pre-v0.30.0 standalone container. A cleanup failure must not block the deploy. +const removeLegacyContainer = async ( + docker: Awaited>, + serviceName: string, +) => { + try { + await docker.getContainer(serviceName).remove({ force: true }); + console.log("Removed legacy monitoring container ✅"); + } catch (error: any) { + if (error?.statusCode !== 404) { + console.warn( + `Could not remove legacy monitoring container: ${error?.message ?? error}`, + ); + } + } +}; + const deployMonitoringService = async ( docker: Awaited>, serviceName: string, settings: CreateServiceOptions, ) => { + await removeLegacyContainer(docker, serviceName); + try { const service = docker.getService(serviceName); const inspect = await service.inspect(); @@ -53,6 +73,12 @@ export const setupMonitoring = async (serverId: string) => { const serviceName = "dokploy-monitoring"; const imageName = getMonitoringImage(); + // No swarm on build servers: clean up the legacy container, deploy nothing. + if (server.serverType === "build") { + await removeLegacyContainer(await getRemoteDocker(serverId), serviceName); + return; + } + const settings: CreateServiceOptions = { Name: serviceName, TaskTemplate: { diff --git a/packages/server/src/setup/server-setup.ts b/packages/server/src/setup/server-setup.ts index 8d0475977..7336961d0 100644 --- a/packages/server/src/setup/server-setup.ts +++ b/packages/server/src/setup/server-setup.ts @@ -86,6 +86,7 @@ export const serverSetup = async ( ...server.metricsConfig.server, token: token, urlCallback: urlCallback, + cronJob: server.metricsConfig.server.cronJob || "0 0 * * *", }, containers: server.metricsConfig.containers, },