From 42c6f00d2e570e26d424277e03a90c690a273dda Mon Sep 17 00:00:00 2001 From: Narciso Date: Wed, 7 Oct 2026 17:04:38 -0400 Subject: [PATCH] fix(log-management): report missing agents as not deployed and drop deleted providers on save - Inspect the dokploy-vector service before listing tasks; listTasks returns [] for a missing service - Filter provider assignments to existing providers after a deploy so a provider deleted mid-deploy is not written back --- .../log-management/vector-agent.test.ts | 40 +++++++++++++++++++ .../log-management/vector-status.test.ts | 13 ++++-- .../src/services/log-management/service.ts | 14 ++++++- .../services/log-management/vector-agent.ts | 12 +++++- packages/server/src/setup/vector-setup.ts | 20 +++++----- 5 files changed, 84 insertions(+), 15 deletions(-) diff --git a/apps/dokploy/__test__/log-management/vector-agent.test.ts b/apps/dokploy/__test__/log-management/vector-agent.test.ts index 2a4a29ae0..41a7d9259 100644 --- a/apps/dokploy/__test__/log-management/vector-agent.test.ts +++ b/apps/dokploy/__test__/log-management/vector-agent.test.ts @@ -10,6 +10,7 @@ const mocks = vi.hoisted(() => ({ setupVectorAgent: vi.fn(), assertServerBelongsToOrg: vi.fn(), assertLogProvidersBelongToOrg: vi.fn(), + filterExistingLogProviderIds: vi.fn(), })); const loadAgent = async (isCloud: boolean) => { @@ -33,6 +34,7 @@ const loadAgent = async (isCloud: boolean) => { vi.doMock("@dokploy/server/services/log-management/service", () => ({ assertServerBelongsToOrg: mocks.assertServerBelongsToOrg, assertLogProvidersBelongToOrg: mocks.assertLogProvidersBelongToOrg, + filterExistingLogProviderIds: mocks.filterExistingLogProviderIds, })); return await import("@dokploy/server/services/log-management/vector-agent"); }; @@ -43,6 +45,9 @@ const sessionB = { userId: "user-b", activeOrganizationId: "org-b" }; describe("local Vector agent guards", () => { beforeEach(() => { for (const mock of Object.values(mocks)) mock.mockReset(); + mocks.filterExistingLogProviderIds.mockImplementation( + async (ids: string[]) => ids, + ); mocks.getAccessibleServerIds.mockResolvedValue(new Set(["server-1"])); }); @@ -132,6 +137,9 @@ describe("local Vector agent guards", () => { describe("remote server access", () => { beforeEach(() => { for (const mock of Object.values(mocks)) mock.mockReset(); + mocks.filterExistingLogProviderIds.mockImplementation( + async (ids: string[]) => ids, + ); mocks.getAccessibleServerIds.mockResolvedValue(new Set(["server-1"])); }); @@ -167,6 +175,20 @@ describe("remote server access", () => { ]); }); + it("does not save a provider deleted while the server deploy ran", async () => { + mocks.filterExistingLogProviderIds.mockResolvedValue(["lp-1"]); + const { deployLogManagement } = await loadAgent(false); + + await deployLogManagement(sessionA, "server-1", ["lp-1", "lp-2"]); + expect(mocks.filterExistingLogProviderIds).toHaveBeenCalledWith([ + "lp-1", + "lp-2", + ]); + expect(mocks.updateServerLogProviders).toHaveBeenCalledWith("server-1", [ + "lp-1", + ]); + }); + it("keeps the saved providers when the server deploy fails", async () => { mocks.setupVectorAgent.mockRejectedValue(new Error("validation failed")); const { deployLogManagement } = await loadAgent(false); @@ -192,6 +214,9 @@ describe("remote server access", () => { describe("local agent deploy", () => { beforeEach(() => { for (const mock of Object.values(mocks)) mock.mockReset(); + mocks.filterExistingLogProviderIds.mockImplementation( + async (ids: string[]) => ids, + ); mocks.claimWebServerLogManagement.mockResolvedValue({}); }); @@ -218,6 +243,21 @@ describe("local agent deploy", () => { ); }); + it("does not save a provider deleted while the local deploy ran", async () => { + mocks.getWebServerSettings.mockResolvedValue({ + logManagementOrganizationId: "org-a", + logProviderIds: ["lp-1"], + }); + mocks.filterExistingLogProviderIds.mockResolvedValue(["lp-1"]); + const { deployLogManagement } = await loadAgent(false); + + await deployLogManagement(sessionA, undefined, ["lp-1", "lp-2"]); + expect(mocks.claimWebServerLogManagement).toHaveBeenLastCalledWith( + "org-a", + ["lp-1"], + ); + }); + it("keeps the previous providers when a redeploy fails", async () => { mocks.getWebServerSettings.mockResolvedValue({ logManagementOrganizationId: "org-a", diff --git a/apps/dokploy/__test__/log-management/vector-status.test.ts b/apps/dokploy/__test__/log-management/vector-status.test.ts index 0fb46ed8b..fecdfb7ea 100644 --- a/apps/dokploy/__test__/log-management/vector-status.test.ts +++ b/apps/dokploy/__test__/log-management/vector-status.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ listTasks: vi.fn(), + inspectService: vi.fn(), findServersByOrganizationForLogManagement: vi.fn(), getAccessibleServerIds: vi.fn(), getWebServerSettings: vi.fn(), @@ -22,7 +23,10 @@ vi.mock("@dokploy/server/services/web-server-settings", () => ({ getWebServerSettings: mocks.getWebServerSettings, })); vi.mock("@dokploy/server/utils/servers/remote-docker", () => ({ - getRemoteDocker: async () => ({ listTasks: mocks.listTasks }), + getRemoteDocker: async () => ({ + listTasks: mocks.listTasks, + getService: () => ({ inspect: mocks.inspectService }), + }), })); const { getLogManagementServerStatus, withVectorTargetLock } = await import( @@ -35,6 +39,7 @@ describe("getLogManagementServerStatus", () => { beforeEach(() => { for (const mock of Object.values(mocks)) mock.mockReset(); mocks.findServersByOrganizationForLogManagement.mockResolvedValue([]); + mocks.inspectService.mockResolvedValue({}); mocks.getAccessibleServerIds.mockResolvedValue(new Set()); mocks.getWebServerSettings.mockResolvedValue({ logManagementOrganizationId: "org-1", @@ -63,16 +68,18 @@ describe("getLogManagementServerStatus", () => { }); it("reports stopped when the service does not exist", async () => { - mocks.listTasks.mockRejectedValue( + mocks.inspectService.mockRejectedValue( Object.assign(new Error("not found"), { statusCode: 404 }), ); + mocks.listTasks.mockResolvedValue([]); const [local] = await getLogManagementServerStatus(session); expect(local?.status).toBe("stopped"); + expect(mocks.listTasks).not.toHaveBeenCalled(); }); it("reports unknown on any other docker error", async () => { - mocks.listTasks.mockRejectedValue(new Error("connect ECONNREFUSED")); + mocks.inspectService.mockRejectedValue(new Error("connect ECONNREFUSED")); const [local] = await getLogManagementServerStatus(session); expect(local?.status).toBe("unknown"); diff --git a/packages/server/src/services/log-management/service.ts b/packages/server/src/services/log-management/service.ts index cd32e112e..9ffc7e414 100644 --- a/packages/server/src/services/log-management/service.ts +++ b/packages/server/src/services/log-management/service.ts @@ -7,7 +7,7 @@ import { } from "@dokploy/server/db/schema"; import { findServerById } from "@dokploy/server/services/server"; import { TRPCError } from "@trpc/server"; -import { and, arrayContains, eq, sql } from "drizzle-orm"; +import { and, arrayContains, eq, inArray, sql } from "drizzle-orm"; import type { z } from "zod"; import { getLogProviderAdapter } from "./providers/registry"; import type { LogProviderRuntimeConfig, LogProviderType } from "./types"; @@ -317,6 +317,18 @@ export const assertServerBelongsToOrg = async ( } }; +export const filterExistingLogProviderIds = async ( + logProviderIds: string[], +): Promise => { + if (logProviderIds.length === 0) return []; + const rows = await db + .select({ logProviderId: logProvider.logProviderId }) + .from(logProvider) + .where(inArray(logProvider.logProviderId, logProviderIds)); + const existing = new Set(rows.map((row) => row.logProviderId)); + return logProviderIds.filter((id) => existing.has(id)); +}; + export const assertLogProvidersBelongToOrg = async ( logProviderIds: string[], organizationId: string, diff --git a/packages/server/src/services/log-management/vector-agent.ts b/packages/server/src/services/log-management/vector-agent.ts index 4fa79bec1..f4c459203 100644 --- a/packages/server/src/services/log-management/vector-agent.ts +++ b/packages/server/src/services/log-management/vector-agent.ts @@ -18,6 +18,7 @@ import { TRPCError } from "@trpc/server"; import { assertLogProvidersBelongToOrg, assertServerBelongsToOrg, + filterExistingLogProviderIds, } from "./service"; const assertLocalAgentAllowed = (serverId: string | undefined) => { @@ -59,7 +60,11 @@ export const deployLogManagement = async ( await withVectorTargetLock(serverId, async () => { if (serverId) { await setupVectorAgent(organizationId, serverId, logProviderIds); - await updateServerLogProviders(serverId, logProviderIds); + // A provider deleted during the deploy must not be written back. + await updateServerLogProviders( + serverId, + await filterExistingLogProviderIds(logProviderIds), + ); return; } @@ -86,7 +91,10 @@ export const deployLogManagement = async ( } throw error; } - await claimWebServerLogManagement(organizationId, logProviderIds); + await claimWebServerLogManagement( + organizationId, + await filterExistingLogProviderIds(logProviderIds), + ); }); }; diff --git a/packages/server/src/setup/vector-setup.ts b/packages/server/src/setup/vector-setup.ts index cfe57582e..abf37da2f 100644 --- a/packages/server/src/setup/vector-setup.ts +++ b/packages/server/src/setup/vector-setup.ts @@ -375,22 +375,24 @@ const inspectVectorStatus = async ( serverId?: string, ): Promise => { const docker = await getRemoteDocker(serverId); + // listTasks returns [] instead of 404 for a missing service, so inspect first. try { - const tasks = await docker.listTasks({ - filters: JSON.stringify({ - service: [VECTOR_SERVICE_NAME], - "desired-state": ["running"], - }), - }); - return tasks.some((task) => task.Status?.State === "running") - ? "running" - : "not-running"; + await docker.getService(VECTOR_SERVICE_NAME).inspect(); } catch (error: any) { if (error?.statusCode === 404) { return "stopped"; } throw error; } + const tasks = await docker.listTasks({ + filters: JSON.stringify({ + service: [VECTOR_SERVICE_NAME], + "desired-state": ["running"], + }), + }); + return tasks.some((task) => task.Status?.State === "running") + ? "running" + : "not-running"; }; const VECTOR_CHECK_TIMEOUT_MS = 5000;