From d3c6627f1a3b1320ef905b0b897bad04177d6329 Mon Sep 17 00:00:00 2001 From: merge-check <140916359+agentHits@users.noreply.github.com> Date: Wed, 7 Oct 2026 09:15:32 -0400 Subject: [PATCH] feat: never count the new and the installed build in image cleanup "Keep the last N" used to count the downloaded build and the one running now, so keeping fewer old images meant risking the rollback target, and the smallest choice was 3. Now both always stay and are not counted; N (0-5) applies only to the older images behind them. The update command records the image of the running container in DOKPLOY_PREVIOUS_IMAGE before replacing it, so the startup cleanup in the new container knows which build to keep for a rollback. Without a recorded id the newest older image stands in for it. The setting moved to DOKPLOY_KEEP_OLD_IMAGES ("off" disables it): an existing DOKPLOY_KEEP_IMAGES=0 meant off and must not read as "keep none". The update dialog now shows the installed fork build and official base in a full-width card, refetches it when opened, and stops offering a build that is already installed (e.g. updated from another tab). Co-Authored-By: Claude Opus 5.5 --- .../settings/dokploy-image-cleanup.test.ts | 132 +++++++++++++++--- .../settings/dokploy-image-plan.test.ts | 53 +++++-- .../settings/web-server/dokploy-image-plan.ts | 72 ++++++---- .../web-server/dokploy-images-info.tsx | 41 ++++-- .../web-server/toggle-image-cleanup.tsx | 13 +- .../settings/web-server/update-server.tsx | 75 ++++++---- packages/server/src/services/settings.ts | 88 +++++++++--- 7 files changed, 348 insertions(+), 126 deletions(-) diff --git a/apps/dokploy/__test__/settings/dokploy-image-cleanup.test.ts b/apps/dokploy/__test__/settings/dokploy-image-cleanup.test.ts index e11518b6ce5..28bcc41de57 100644 --- a/apps/dokploy/__test__/settings/dokploy-image-cleanup.test.ts +++ b/apps/dokploy/__test__/settings/dokploy-image-cleanup.test.ts @@ -1,4 +1,13 @@ import { execFileSync } from "node:child_process"; +import { + chmodSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const { execAsync } = vi.hoisted(() => ({ execAsync: vi.fn() })); @@ -25,10 +34,13 @@ const { UPDATE_IMAGE_PULLED_MARKER } = await import( "@dokploy/server/services/web-server-update" ); +const PREVIOUS = `sha256:${"a".repeat(64)}`; + describe("Dokploy image cleanup", () => { beforeEach(() => { vi.clearAllMocks(); - vi.stubEnv("DOKPLOY_KEEP_IMAGES", undefined); + vi.stubEnv("DOKPLOY_KEEP_OLD_IMAGES", undefined); + vi.stubEnv("DOKPLOY_PREVIOUS_IMAGE", undefined); vi.stubEnv("DOKPLOY_AGENTHITS_UPDATE_IMAGE", undefined); vi.stubEnv("DOKPLOY_AGENTHITS_UPDATE_TAG", undefined); }); @@ -39,18 +51,25 @@ describe("Dokploy image cleanup", () => { it.each([ [undefined, null], - ["0", null], - ["2", null], - ["3", 3], + ["", null], + ["off", null], + ["0", 0], + ["1", 1], ["5", 5], ["6", null], + ["-1", null], ["3.5", null], ["abc", null], ])("reads keep count %s as %s", (value, expected) => { - vi.stubEnv("DOKPLOY_KEEP_IMAGES", value); + vi.stubEnv("DOKPLOY_KEEP_OLD_IMAGES", value); expect(getDokployImageKeepCount()).toBe(expected); }); + it("ignores the old keep setting, whose 0 meant off", () => { + vi.stubEnv("DOKPLOY_KEEP_IMAGES", "0"); + expect(getDokployImageKeepCount()).toBeNull(); + }); + it("targets the fork and official image repositories without tags", () => { expect(getDokployImageRepositories()).toEqual([ "ghcr.io/agenthits/dokploy", @@ -68,7 +87,7 @@ describe("Dokploy image cleanup", () => { }); it("keeps the newest images and only removes exited dokploy task containers", () => { - const command = getDokployImageCleanupCommand(4); + const command = getDokployImageCleanupCommand(4, PREVIOUS); expect(command).toContain( "--filter reference\\=ghcr.io/agenthits/dokploy --filter reference\\=dokploy/dokploy", @@ -81,22 +100,87 @@ describe("Dokploy image cleanup", () => { expect(command).not.toMatch(/\brm (-f|--force)\b/); }); + it("skips one more image when the previous build was not recorded", () => { + expect(getDokployImageCleanupCommand(0, null)).toContain("tail -n +2"); + expect(getDokployImageCleanupCommand(0, PREVIOUS)).toContain("tail -n +1"); + }); + + it.each([ + [1, "sha256:cur", ["sha256:old2", "sha256:old3"]], + [0, "sha256:cur", ["sha256:old1", "sha256:old2", "sha256:old3"]], + [0, null, ["sha256:old1", "sha256:old2", "sha256:old3"]], + [3, "sha256:cur", []], + ])( + "with keep %s and previous %s, removes %j and never the running or previous image", + (keep, previous, expectedRemoved) => { + const dir = mkdtempSync(join(tmpdir(), "dokploy-cleanup-")); + const log = join(dir, "log"); + try { + writeFileSync( + join(dir, "docker"), + `#!/bin/sh +echo "$*" >> "${log}" +case "$*" in + "image ls"*) printf 'sha256:new\nsha256:cur\nsha256:old1\nsha256:old2\nsha256:old3\n' ;; + "ps -q"*) echo task1 ;; + "inspect --format {{.Image}} task1") echo sha256:new ;; + "image inspect --format {{.Created}} {{.Id}}"*) printf '2026-10-07T05 sha256:new\n2026-10-07T04 sha256:cur\n2026-10-07T03 sha256:old1\n2026-10-07T02 sha256:old2\n2026-10-07T01 sha256:old3\n' ;; +esac +`, + ); + chmodSync(join(dir, "docker"), 0o755); + writeFileSync(log, ""); + + execFileSync( + "sh", + ["-c", getDokployImageCleanupCommand(keep, previous)], + { + env: { ...process.env, PATH: `${dir}:${process.env.PATH}` }, + }, + ); + + const removed = readFileSync(log, "utf8") + .split("\n") + .filter((line) => line.startsWith("image rm ")) + .map((line) => line.slice("image rm ".length)); + expect(removed).toEqual(expectedRemoved); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + ); + it("does nothing at startup when the cleanup is off", async () => { await cleanupOldDokployImages(); expect(execAsync).not.toHaveBeenCalled(); }); it("runs the cleanup at startup when a keep count is set", async () => { - vi.stubEnv("DOKPLOY_KEEP_IMAGES", "3"); + vi.stubEnv("DOKPLOY_KEEP_OLD_IMAGES", "0"); + vi.stubEnv("DOKPLOY_PREVIOUS_IMAGE", PREVIOUS); + execAsync.mockResolvedValue({ stdout: "", stderr: "" }); + + await cleanupOldDokployImages(); + + expect(execAsync).toHaveBeenCalledWith( + getDokployImageCleanupCommand(0, PREVIOUS), + ); + }); + + it("does not trust a malformed previous image id", async () => { + vi.stubEnv("DOKPLOY_KEEP_OLD_IMAGES", "1"); + vi.stubEnv("DOKPLOY_PREVIOUS_IMAGE", "sha256:abc; rm -rf /"); execAsync.mockResolvedValue({ stdout: "", stderr: "" }); await cleanupOldDokployImages(); - expect(execAsync).toHaveBeenCalledWith(getDokployImageCleanupCommand(3)); + expect(execAsync).toHaveBeenCalledWith( + getDokployImageCleanupCommand(1, null), + ); }); it("does not throw when the cleanup fails", async () => { - vi.stubEnv("DOKPLOY_KEEP_IMAGES", "3"); + vi.stubEnv("DOKPLOY_KEEP_OLD_IMAGES", "3"); execAsync.mockRejectedValue(new Error("docker unavailable")); vi.spyOn(console, "error").mockImplementation(() => {}); @@ -105,15 +189,29 @@ describe("Dokploy image cleanup", () => { it("stores the keep count on the service only when it is given", () => { expect(getAgentHitsUpdateCommand("v0.30.6")).not.toContain( - "DOKPLOY_KEEP_IMAGES", + "DOKPLOY_KEEP_OLD_IMAGES", ); - expect(getAgentHitsUpdateCommand("v0.30.6", null, null, 3)).toContain( - "--env-add DOKPLOY_KEEP_IMAGES\\=3 \\\n\tdokploy", + expect(getAgentHitsUpdateCommand("v0.30.6", null, null, 0)).toContain( + "--env-add DOKPLOY_KEEP_OLD_IMAGES\\=0 \\\n", ); expect(getAgentHitsUpdateCommand("v0.30.6", null, null, null)).toContain( - "--env-add DOKPLOY_KEEP_IMAGES\\=0", + "--env-add DOKPLOY_KEEP_OLD_IMAGES\\=off", ); }); + + it("records the running image before replacing it", () => { + for (const command of [ + getAgentHitsUpdateCommand("v0.30.6"), + getOfficialUpdateCommand("v0.30.7"), + ]) { + const recordAt = command.indexOf("previous_image=$(docker inspect"); + expect(recordAt).toBeGreaterThan(-1); + expect(recordAt).toBeLessThan(command.indexOf("docker service update")); + expect(command).toContain( + '--env-add "DOKPLOY_PREVIOUS_IMAGE=$previous_image"', + ); + } + }); }); describe("Dokploy image list", () => { @@ -243,10 +341,10 @@ describe("Dokploy update commands", () => { expectPullBeforeUpdate(command, "docker pull dokploy/dokploy\\:v0.30.7"); expect(command).toContain( - "--image dokploy/dokploy\\:v0.30.7 --env-add DOKPLOY_KEEP_IMAGES\\=3 dokploy", + "--image dokploy/dokploy\\:v0.30.7 --env-add DOKPLOY_KEEP_OLD_IMAGES\\=3 ", ); expect(getOfficialUpdateCommand("v0.30.7")).not.toContain( - "DOKPLOY_KEEP_IMAGES", + "DOKPLOY_KEEP_OLD_IMAGES", ); }); @@ -256,6 +354,8 @@ describe("Dokploy update commands", () => { await getDokployImages(); expect(execAsync.mock.calls[0]?.[0]).toMatch(/^docker image ls -a -q /); - expect(getDokployImageCleanupCommand(3)).toContain("docker image ls -a -q"); + expect(getDokployImageCleanupCommand(3, null)).toContain( + "docker image ls -a -q", + ); }); }); diff --git a/apps/dokploy/__test__/settings/dokploy-image-plan.test.ts b/apps/dokploy/__test__/settings/dokploy-image-plan.test.ts index 0b20b7f57a0..6cd75b06d0e 100644 --- a/apps/dokploy/__test__/settings/dokploy-image-plan.test.ts +++ b/apps/dokploy/__test__/settings/dokploy-image-plan.test.ts @@ -34,7 +34,7 @@ describe("planDokployImageCleanup", () => { const plan = planDokployImageCleanup(images, null, "v5"); expect(actions(plan)).toEqual([ "v5:new", - "v4:keep", + "v4:current", "v3:keep", "v2:keep", "v1:keep", @@ -42,40 +42,63 @@ describe("planDokployImageCleanup", () => { expect(plan.removeCount).toBe(0); }); - it("counts the pending build as one of the kept images", () => { - const plan = planDokployImageCleanup(images, 3, "v5"); + it("counts only the images older than the new and the installed build", () => { + const plan = planDokployImageCleanup(images, 1, "v5"); expect(actions(plan)).toEqual([ "v5:new", - "v4:keep", + "v4:current", "v3:keep", "v2:remove", "v1:remove", ]); + expect(plan.rows.map((row) => row.position)).toEqual([null, null, 1, 2, 3]); expect(plan.freedBytes).toBe(8_200_000_000); expect(plan.removedContainers).toBe(1); }); - it("does not add a pending row when the new build is already pulled", () => { - const plan = planDokployImageCleanup(images, 3, "v4"); + it("can delete every older image while the new and installed builds stay", () => { + const plan = planDokployImageCleanup(images, 0, "v5"); expect(actions(plan)).toEqual([ - "v4:keep", + "v5:new", + "v4:current", + "v3:remove", + "v2:remove", + "v1:remove", + ]); + }); + + it("protects an already pulled new build without adding a pending row", () => { + const plan = planDokployImageCleanup([image("v5"), ...images], 0, "v5"); + expect(actions(plan)).toEqual([ + "v5:new", + "v4:current", + "v3:remove", + "v2:remove", + "v1:remove", + ]); + }); + + it("does not protect an extra image when the new build is the installed one", () => { + const plan = planDokployImageCleanup(images, 1, "v4"); + expect(actions(plan)).toEqual([ + "v4:current", "v3:keep", - "v2:keep", + "v2:remove", "v1:remove", ]); }); - it("never plans to delete images that other containers or the running server use", () => { + it("never plans to delete images that other containers use", () => { const plan = planDokployImageCleanup( [ - image("v3"), - image("v2", { otherContainers: 1 }), - image("v1", { isCurrent: true }), + image("v3", { isCurrent: true }), + image("v2"), + image("v1", { otherContainers: 1 }), ], - 1, + 0, null, ); - expect(actions(plan)).toEqual(["v3:keep", "v2:blocked", "v1:blocked"]); - expect(plan.removeCount).toBe(0); + expect(actions(plan)).toEqual(["v3:current", "v2:remove", "v1:blocked"]); + expect(plan.removeCount).toBe(1); }); }); diff --git a/apps/dokploy/components/dashboard/settings/web-server/dokploy-image-plan.ts b/apps/dokploy/components/dashboard/settings/web-server/dokploy-image-plan.ts index c21d359638c..7a04d8d031e 100644 --- a/apps/dokploy/components/dashboard/settings/web-server/dokploy-image-plan.ts +++ b/apps/dokploy/components/dashboard/settings/web-server/dokploy-image-plan.ts @@ -1,12 +1,19 @@ import type { DokployImageInfo } from "@dokploy/server/index"; -export type DokployImageAction = "new" | "keep" | "remove" | "blocked"; +export type DokployImageAction = + | "new" + | "current" + | "keep" + | "remove" + | "blocked"; export interface PlannedDokployImage { key: string; version: string; image: DokployImageInfo | null; action: DokployImageAction; + /** Position among the images the keep count applies to; null for protected ones. */ + position: number | null; } export interface DokployImagePlan { @@ -28,45 +35,54 @@ const isImageOfVersion = (image: DokployImageInfo, version: string) => image.tags.some((tag) => tag.endsWith(`:${version}`)); /** - * Mirrors getDokployImageCleanupCommand: the newest `keep` images stay, older - * ones go unless another container still uses them. A pending update counts - * as the newest image because the cleanup runs after it is pulled. + * Mirrors getDokployImageCleanupCommand: the new build and the build running + * now always stay (after the update the latter is the rollback target), and + * the keep count applies only to the older images behind them. */ export const planDokployImageCleanup = ( images: DokployImageInfo[], keep: number | null, pendingVersion: string | null, ): DokployImagePlan => { - const hasPending = - !!pendingVersion && - !images.some((image) => isImageOfVersion(image, pendingVersion)); - const ordered: (DokployImageInfo | null)[] = [ - ...(hasPending ? [null] : []), - ...images, - ]; + const pendingImage = pendingVersion + ? images.find((image) => isImageOfVersion(image, pendingVersion)) + : undefined; + const rows: PlannedDokployImage[] = []; - const rows = ordered.map((image, index): PlannedDokployImage => { - if (!image) { - return { - key: "pending", - version: pendingVersion ?? "", - image: null, - action: "new", - }; - } - let action: DokployImageAction = "keep"; - if (keep !== null && index >= keep) { - // Without an update the current container keeps running, so its image stays. - const stillRunning = image.isCurrent && !pendingVersion; - action = image.otherContainers > 0 || stillRunning ? "blocked" : "remove"; + if (pendingVersion && !pendingImage) { + rows.push({ + key: "pending", + version: pendingVersion, + image: null, + action: "new", + position: null, + }); + } + + let position = 0; + for (const image of images) { + let action: DokployImageAction; + let rowPosition: number | null = null; + if (image.isCurrent) { + action = "current"; + } else if (image === pendingImage) { + action = "new"; + } else { + rowPosition = ++position; + if (keep === null || rowPosition <= keep) { + action = "keep"; + } else { + action = image.otherContainers > 0 ? "blocked" : "remove"; + } } - return { + rows.push({ key: image.id, version: getDokployImageVersion(image), image, action, - }; - }); + position: rowPosition, + }); + } const removed = rows.filter((row) => row.action === "remove"); return { diff --git a/apps/dokploy/components/dashboard/settings/web-server/dokploy-images-info.tsx b/apps/dokploy/components/dashboard/settings/web-server/dokploy-images-info.tsx index 02af515316c..dbaec3e2f29 100644 --- a/apps/dokploy/components/dashboard/settings/web-server/dokploy-images-info.tsx +++ b/apps/dokploy/components/dashboard/settings/web-server/dokploy-images-info.tsx @@ -30,7 +30,8 @@ interface PopoverProps { } const rowStyles: Record = { - new: "border-dashed border-emerald-500/50 bg-emerald-500/5", + new: "border-border", + current: "border-border", keep: "border-emerald-500/30 bg-emerald-500/5", remove: "border-red-500/50 bg-red-500/10", blocked: "border-yellow-500/40 bg-yellow-500/10", @@ -40,12 +41,27 @@ const actionBadges: Record< DokployImageAction, { label: string; className: string } > = { - new: { label: "New build", className: "bg-emerald-600 text-white" }, + new: { label: "New build", className: "" }, + current: { label: "Installed now", className: "" }, keep: { label: "Keep", className: "bg-emerald-600/80 text-white" }, remove: { label: "Delete", className: "bg-red-600 text-white" }, blocked: { label: "In use", className: "bg-yellow-600 text-white" }, }; +const describeCleanup = (keepImages: number | null, hasPending: boolean) => { + if (keepImages === null) { + return "Cleanup is off: every image stays."; + } + const protectedText = hasPending + ? "The new build and the one installed now always stay." + : "The build installed now always stays."; + const olderText = + keepImages === 0 + ? "All older images are deleted after the update." + : `Of the older images, the newest ${keepImages} ${keepImages === 1 ? "stays" : "stay"}; red ones are deleted after the update.`; + return `${protectedText} ${olderText}`; +}; + export const DokployImagesInfo = ({ keepImages, pendingVersion, @@ -93,9 +109,7 @@ export const DokployImagesPopover = ({

Dokploy images on this server

- {keepImages === null - ? "Cleanup is off: every image stays." - : `The newest ${keepImages} stay${pendingVersion ? ", counting the new build" : ""}. Red ones are deleted after the update.`} + {describeCleanup(keepImages, !!pendingVersion)}

@@ -112,9 +126,10 @@ export const DokployImagesPopover = ({ {plan && plan.rows.length > 0 && (