Skip to content

Commit a5a1426

Browse files
authored
cleanup some container images during local dev (#10289)
* cleanup duplicate tags * also delete old tag after hotkey * changeset * pr feedback
1 parent 4288a61 commit a5a1426

File tree

6 files changed

+127
-5
lines changed

6 files changed

+127
-5
lines changed

.changeset/great-rocks-shop.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"@cloudflare/containers-shared": patch
3+
"wrangler": patch
4+
---
5+
6+
Cleanup container images created during local dev if no changes have been made.
7+
8+
We now untag old images that were created by Wrangler/Vite if we find that the image content and configuration is unchanged, so that we don't keep accumulating image tags.

fixtures/interactive-dev-tests/tests/index.test.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,21 @@ baseDescribe.skipIf(process.platform !== "linux" && process.env.CI === "true")(
377377
"Hello World! Have an env var! I'm an env var!"
378378
);
379379
}, WAITFOR_OPTIONS);
380+
const output = wrangler.stdout;
381+
// Extract the Docker image name from the output
382+
const imageNameMatch = output.match(
383+
/cloudflare-dev\/fixturetestcontainer:[a-f0-9]+/
384+
);
385+
expect(imageNameMatch).not.toBe(null);
386+
const imageName = imageNameMatch![0];
387+
expect(
388+
JSON.parse(
389+
execSync(
390+
`docker image inspect ${imageName} --format "{{ json .RepoTags }}"`,
391+
{ encoding: "utf8" }
392+
)
393+
).length
394+
).toBeGreaterThanOrEqual(1);
380395

381396
fs.writeFileSync(
382397
path.join(tmpDir, "container", "simple-node-app.js"),
@@ -412,6 +427,12 @@ baseDescribe.skipIf(process.platform !== "linux" && process.env.CI === "true")(
412427
});
413428
expect(await res.text()).toBe("Blah! I'm an env var!");
414429
}, WAITFOR_OPTIONS);
430+
431+
// Verify that the old image tag has been deleted after rebuild
432+
expect(() => {
433+
execSync(`docker image inspect ${imageName}`, { encoding: "utf8" });
434+
}).toThrow();
435+
415436
wrangler.pty.kill();
416437
});
417438

@@ -424,11 +445,6 @@ baseDescribe.skipIf(process.platform !== "linux" && process.env.CI === "true")(
424445
await fetch(wrangler.url + "/start");
425446

426447
// wait container to be ready
427-
await vi.waitFor(async () => {
428-
const status = await fetch(wrangler.url + "/status");
429-
expect(await status.json()).toBe(true);
430-
}, WAITFOR_OPTIONS);
431-
432448
await vi.waitFor(async () => {
433449
const ids = getContainerIds();
434450
expect(ids.length).toBe(1);

packages/containers-shared/src/images.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
import { dockerLoginManagedRegistry } from "./login";
77
import {
88
checkExposedPorts,
9+
cleanupDuplicateImageTags,
910
runDockerCmd,
1011
verifyDockerInstalled,
1112
} from "./utils";
@@ -87,6 +88,7 @@ export async function prepareContainerImagesForDev(args: {
8788
},
8889
});
8990
await build.ready;
91+
9092
onContainerImagePreparationEnd({
9193
containerOptions: options,
9294
});
@@ -111,6 +113,9 @@ export async function prepareContainerImagesForDev(args: {
111113
});
112114
}
113115
if (!aborted) {
116+
// Clean up duplicate image tags. This is scoped to cloudflare-dev only
117+
await cleanupDuplicateImageTags(dockerPath, options.image_tag);
118+
114119
await checkExposedPorts(dockerPath, options);
115120
}
116121
}

packages/containers-shared/src/utils.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,3 +313,41 @@ export const getDockerHostFromEnv = (): string => {
313313
? "//./pipe/docker_engine"
314314
: "unix:///var/run/docker.sock";
315315
};
316+
317+
/**
318+
* Get all repository tags for a given image
319+
*/
320+
export async function getImageRepoTags(
321+
dockerPath: string,
322+
imageTag: string
323+
): Promise<string[]> {
324+
try {
325+
const output = await dockerImageInspect(dockerPath, {
326+
imageTag,
327+
formatString: "{{ range .RepoTags }}{{ . }}\n{{ end }}",
328+
});
329+
return output.split("\n").filter((tag) => tag.trim() !== "");
330+
} catch {
331+
return [];
332+
}
333+
}
334+
335+
/**
336+
* Checks if the given image has any duplicate tags from previous dev sessions,
337+
* and remove them if so.
338+
*/
339+
export async function cleanupDuplicateImageTags(
340+
dockerPath: string,
341+
imageTag: string
342+
): Promise<void> {
343+
try {
344+
const repoTags = await getImageRepoTags(dockerPath, imageTag);
345+
// Remove all cloudflare-dev tags from previous sessions except the current one
346+
const tagsToRemove = repoTags.filter(
347+
(tag) => tag !== imageTag && tag.startsWith("cloudflare-dev")
348+
);
349+
if (tagsToRemove.length > 0) {
350+
runDockerCmdWithOutput(dockerPath, ["rmi", ...tagsToRemove]);
351+
}
352+
} catch {}
353+
}

packages/wrangler/e2e/containers.dev.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { execSync } from "child_process";
2+
import path from "node:path";
23
import {
34
afterAll,
45
beforeAll,
@@ -8,6 +9,8 @@ import {
89
it,
910
vi,
1011
} from "vitest";
12+
import { buildImage } from "../../containers-shared/src/build";
13+
import { generateContainerBuildId } from "../../containers-shared/src/utils";
1114
import { getDockerPath } from "../src/environment-variables/misc-variables";
1215
import { dedent } from "../src/utils/dedent";
1316
import { CLOUDFLARE_ACCOUNT_ID } from "./helpers/account-id";
@@ -241,6 +244,47 @@ for (const source of imageSource) {
241244
await worker.stop();
242245
});
243246

247+
it("should clean up duplicate image tags after build", async () => {
248+
const dockerPath = getDockerPath();
249+
const fakeBuildID = generateContainerBuildId();
250+
const initialImageTag = `cloudflare-dev/test-cleanup:${fakeBuildID}`;
251+
252+
// First, build an image directly to create a duplicate tag scenario
253+
const build = await buildImage(dockerPath, {
254+
dockerfile: path.resolve(helper.tmpPath, "./Dockerfile"),
255+
image_tag: initialImageTag,
256+
class_name: "TestContainer",
257+
image_build_context: helper.tmpPath,
258+
image_vars: {},
259+
});
260+
await build.ready;
261+
262+
const initialRepoTags = JSON.parse(
263+
execSync(
264+
`${dockerPath} image inspect ${initialImageTag} --format "{{ json .RepoTags }}"`,
265+
{ encoding: "utf8" }
266+
)
267+
);
268+
expect(initialRepoTags.length).toBeGreaterThan(0);
269+
270+
// wrangler dev will rebuild/pull and trigger cleanup
271+
const worker = helper.runLongLived("wrangler dev");
272+
const ready = await worker.waitForReady();
273+
274+
// check that the container can still start
275+
await vi.waitFor(async () => {
276+
const response = await fetch(`${ready.url}/status`);
277+
expect(response.status).toBe(200);
278+
const status = await response.json();
279+
expect(status).toBe(false);
280+
});
281+
282+
// expect the original tag not to be there any more
283+
expect(() => {
284+
execSync(`${dockerPath} image inspect ${initialImageTag}`);
285+
}).toThrow();
286+
});
287+
244288
it("won't start the container service if no containers are present", async () => {
245289
await helper.seed({
246290
"wrangler.json": JSON.stringify({

packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
cleanupContainers,
66
getDevContainerImageName,
77
prepareContainerImagesForDev,
8+
runDockerCmdWithOutput,
89
} from "@cloudflare/containers-shared";
910
import chalk from "chalk";
1011
import { Miniflare, Mutex } from "miniflare";
@@ -251,6 +252,16 @@ export class LocalRuntimeController extends RuntimeController {
251252
);
252253

253254
for (const container of containerDevOptions) {
255+
// if this was triggered by the rebuild hotkey, delete the old image
256+
if (this.#currentContainerBuildId !== undefined) {
257+
runDockerCmdWithOutput(this.dockerPath, [
258+
"rmi",
259+
getDevContainerImageName(
260+
container.class_name,
261+
this.#currentContainerBuildId
262+
),
263+
]);
264+
}
254265
this.containerImageTagsSeen.add(container.image_tag);
255266
}
256267
logger.log(chalk.dim("⎔ Preparing container image(s)..."));

0 commit comments

Comments
 (0)