Fix manifest inspect failing due to wrong image ref (#10923)

* update manifest inspect command

* changeset

* e2e test

* fixups and pr feedback

* remove retries

* only run e2e tests on linux now that we build the image

* linting

Author: emily-shen

.changeset/rich-wasps-sell.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+fix: update `docker manifest inspect` to use full image registry uri when checking if the image exists remotely

packages/wrangler/e2e/deployments.test.ts

@@ -1,9 +1,11 @@
 import assert from "node:assert";
+import isCI from "is-ci";
 import dedent from "ts-dedent";
 import { fetch } from "undici";
 import {
 	afterAll,
 	afterEach,
+	beforeAll,
 	beforeEach,
 	describe,
 	expect,
@@ -842,11 +844,21 @@ Current Version ID: 00000000-0000-0000-0000-000000000000`);
 			await checkAssets(testCases, deployedUrl);
 		});
 	});
+});
 
-	describe("durable objects [containers]", () => {
-		beforeEach(async () => {
-			await helper.seed({
-				"wrangler.toml": dedent`
+const skipContainersTest =
+	!CLOUDFLARE_ACCOUNT_ID || (isCI && process.platform !== "linux");
+describe.skipIf(skipContainersTest)("containers", () => {
+	let helper: WranglerE2ETestHelper;
+	let workerName: string;
+	let applicationId: string | undefined;
+	let deployedUrl: string;
+
+	beforeAll(async () => {
+		helper = new WranglerE2ETestHelper();
+		workerName = generateResourceName();
+		await helper.seed({
+			"wrangler.toml": dedent`
 						name = "${workerName}"
 						main = "src/index.ts"
 						compatibility_date = "2023-01-01"
@@ -859,14 +871,36 @@ Current Version ID: 00000000-0000-0000-0000-000000000000`);
 						[[containers]]
 						name = "e2e-test-${workerName}"
 						class_name = "MyDurableObject"
-						image = "registry.cloudchamber.cfdata.org/e2e-test:1.0"
+						image = "./Dockerfile"
 						max_instances = 1
 
 						[[migrations]]
 						tag = "v1"
 						new_sqlite_classes = ["MyDurableObject"]
+
+						[observability]
+						enabled = true
 				`,
-				"src/index.ts": dedent`
+			"container/index.js": dedent`
+				const { createServer } = require("http");
+
+				const server = createServer((req, res) => {
+				  res.writeHead(200, { 'Content-Type': 'text/plain' });
+				  res.end('hello from container');
+				});
+
+				server.listen(80, () => {
+				  console.log('Server running on port 80');
+				});
+				`,
+			Dockerfile: dedent`
+				FROM node:18-slim
+				WORKDIR /app
+				COPY container/index.js .
+				CMD ["node", "index.js"]
+				EXPOSE 80
+				`,
+			"src/index.ts": dedent`
               export default {
                 async fetch(req, env) {
                   const url = new URL(req.url)
@@ -902,45 +936,55 @@ Current Version ID: 00000000-0000-0000-0000-000000000000`);
                   return this.ctx.container.getTcpPort(80).fetch(new Request("http://foo"));
                 }
               }`,
-			});
 		});
+	});
 
-		it.skip(
-			"can fetch DO container",
-			{ timeout: 60 * 3 * 1000, retry: 3 },
-			async () => {
-				const output = await helper.run(`wrangler deploy`);
-
-				const deployedUrl = getDeployedUrl(output);
-
-				const matchApplicationId = output.stdout.match(
-					/([(]Application ID: (?<applicationId>.+?)[)])/
-				);
-				assert(matchApplicationId?.groups);
-
-				try {
-					await vi.waitFor(
-						async () => {
-							const response = await fetch(`${deployedUrl}/do`);
-							if (!response.ok) {
-								throw new Error(
-									"Durable object transient error: " + (await response.text())
-								);
-							}
+	afterAll(async () => {
+		// clean up user Worker after each test
+		const deleteWorker = helper.run(`wrangler delete`);
+		const deleteContainer = helper.run(
+			`wrangler containers delete ${applicationId}`
+		);
+		await Promise.allSettled([deleteWorker, deleteContainer]);
+	});
 
-							expect(await response.text()).toEqual("hello from container");
-						},
+	it(
+		"won't rebuild unchanged containers",
+		{ timeout: 60 * 2 * 1000 },
+		async () => {
+			const outputOne = await helper.run(`wrangler deploy`);
 
-						// big timeout for containers
-						// (3m)
-						{ timeout: 60 * 3 * 1000, interval: 1000 }
-					);
-				} finally {
-					await helper.run(
-						`wrangler containers delete ${matchApplicationId.groups.applicationId}`
+			deployedUrl = getDeployedUrl(outputOne);
+
+			const matchApplicationId = outputOne.stdout.match(
+				/([(]Application ID: (?<applicationId>.+?)[)])/
+			);
+			applicationId = matchApplicationId?.groups?.applicationId;
+			assert(matchApplicationId?.groups);
+
+			const outputTwo = await helper.run(`wrangler deploy`);
+			expect(outputTwo.stdout).toContain(`No changes to be made`);
+		}
+	);
+
+	it("can fetch DO container", { timeout: 60 * 2 * 1000 }, async () => {
+		await vi.waitFor(
+			async () => {
+				const response = await fetch(`${deployedUrl}/do`, {
+					signal: AbortSignal.timeout(5_000),
+				});
+				if (!response.ok) {
+					throw new Error(
+						"Durable object transient error: " + (await response.text())
 					);
 				}
-			}
+
+				expect(await response.text()).toEqual("hello from container");
+			},
+
+			// big timeout for containers
+			// (3m)
+			{ timeout: 60 * 2 * 1000, interval: 1000 }
 		);
 	});
 });

packages/wrangler/src/__tests__/cloudchamber/build.test.ts

@@ -73,8 +73,8 @@ describe("buildAndMaybePush", () => {
 			dockerfile,
 		});
 
-		// 3 calls: docker tag + docker push + docker rm
-		expect(runDockerCmd).toHaveBeenCalledTimes(3);
+		// 3 calls: docker tag + docker push
+		expect(runDockerCmd).toHaveBeenCalledTimes(2);
 		expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
 			"tag",
 			`test-app:tag`,
@@ -84,15 +84,11 @@ describe("buildAndMaybePush", () => {
 			"push",
 			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
 		]);
-		expect(runDockerCmd).toHaveBeenNthCalledWith(3, "docker", [
-			"image",
-			"rm",
-			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
-		]);
+
 		expect(dockerImageInspect).toHaveBeenCalledTimes(2);
 		expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
 			imageTag: `test-app:tag`,
-			formatString: "{{ json .RepoDigests }} {{ .Id }}",
+			formatString: "{{ json .RepoDigests }}",
 		});
 		expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
 			imageTag: `test-app:tag`,
@@ -121,8 +117,8 @@ describe("buildAndMaybePush", () => {
 			dockerfile,
 		});
 
-		// 3 calls: docker tag + docker push + docker rm
-		expect(runDockerCmd).toHaveBeenCalledTimes(3);
+		// 3 calls: docker tag + docker push
+		expect(runDockerCmd).toHaveBeenCalledTimes(2);
 		expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
 			"tag",
 			`registry.cloudflare.com/test-app:tag`,
@@ -132,15 +128,10 @@ describe("buildAndMaybePush", () => {
 			"push",
 			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
 		]);
-		expect(runDockerCmd).toHaveBeenNthCalledWith(3, "docker", [
-			"image",
-			"rm",
-			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
-		]);
 		expect(dockerImageInspect).toHaveBeenCalledTimes(2);
 		expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
 			imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
-			formatString: "{{ json .RepoDigests }} {{ .Id }}",
+			formatString: "{{ json .RepoDigests }}",
 		});
 		expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
 			imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
@@ -169,8 +160,8 @@ describe("buildAndMaybePush", () => {
 			dockerfile,
 		});
 
-		// 3 calls: docker tag + docker push + docker rm
-		expect(runDockerCmd).toHaveBeenCalledTimes(3);
+		// 3 calls: docker tag + docker push
+		expect(runDockerCmd).toHaveBeenCalledTimes(2);
 		expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
 			"tag",
 			`registry.cloudflare.com/some-account-id/test-app:tag`,
@@ -180,15 +171,10 @@ describe("buildAndMaybePush", () => {
 			"push",
 			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
 		]);
-		expect(runDockerCmd).toHaveBeenNthCalledWith(3, "docker", [
-			"image",
-			"rm",
-			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
-		]);
 		expect(dockerImageInspect).toHaveBeenCalledTimes(2);
 		expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
 			imageTag: `registry.cloudflare.com/some-account-id/test-app:tag`,
-			formatString: "{{ json .RepoDigests }} {{ .Id }}",
+			formatString: "{{ json .RepoDigests }}",
 		});
 		expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
 			imageTag: `registry.cloudflare.com/some-account-id/test-app:tag`,
@@ -222,7 +208,7 @@ describe("buildAndMaybePush", () => {
 			"/custom/docker/path",
 			{
 				imageTag: `test-app:tag`,
-				formatString: "{{ json .RepoDigests }} {{ .Id }}",
+				formatString: "{{ json .RepoDigests }}",
 			}
 		);
 		expect(dockerImageInspect).toHaveBeenNthCalledWith(
@@ -262,8 +248,8 @@ describe("buildAndMaybePush", () => {
 			dockerfile,
 		});
 
-		// 3 calls: docker tag + docker push + docker rm
-		expect(runDockerCmd).toHaveBeenCalledTimes(3);
+		// 3 calls: docker tag + docker push
+		expect(runDockerCmd).toHaveBeenCalledTimes(2);
 		expect(runDockerCmd).toHaveBeenNthCalledWith(1, "docker", [
 			"tag",
 			`test-app:tag`,
@@ -273,15 +259,10 @@ describe("buildAndMaybePush", () => {
 			"push",
 			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
 		]);
-		expect(runDockerCmd).toHaveBeenNthCalledWith(3, "docker", [
-			"image",
-			"rm",
-			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
-		]);
 		expect(dockerImageInspect).toHaveBeenCalledTimes(2);
 		expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
 			imageTag: `test-app:tag`,
-			formatString: "{{ json .RepoDigests }} {{ .Id }}",
+			formatString: "{{ json .RepoDigests }}",
 		});
 		expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
 			imageTag: `test-app:tag`,
@@ -298,12 +279,12 @@ describe("buildAndMaybePush", () => {
 		vi.mocked(dockerImageInspect).mockReset();
 		vi.mocked(dockerImageInspect)
 			.mockResolvedValueOnce(
-				'["registry.cloudflare.com/test-app@sha256:three"] matching-config-sha'
+				'["registry.cloudflare.com/test-app@sha256:three"]'
 			)
 			.mockResolvedValueOnce("53387881 2");
 		vi.mocked(runDockerCmdWithOutput).mockReset();
 		vi.mocked(runDockerCmdWithOutput).mockImplementationOnce(() => {
-			return '{"Descriptor":{"digest":"matching-config-sha"}}';
+			return '{"Descriptor":{"digest":"three}}';
 		});
 
 		await runWrangler(
@@ -330,16 +311,10 @@ describe("buildAndMaybePush", () => {
 			"-v",
 			`${getCloudflareContainerRegistry()}/some-account-id/test-app@sha256:three`,
 		]);
-		expect(runDockerCmd).toHaveBeenCalledOnce();
-		expect(runDockerCmd).toHaveBeenCalledWith("docker", [
-			"image",
-			"rm",
-			`test-app:tag`,
-		]);
 		expect(dockerImageInspect).toHaveBeenCalledTimes(2);
 		expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
 			imageTag: `test-app:tag`,
-			formatString: "{{ json .RepoDigests }} {{ .Id }}",
+			formatString: "{{ json .RepoDigests }}",
 		});
 		expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
 			imageTag: `test-app:tag`,