validate container configuration in wrangler (#9774)

* CC-5513 Validate instance type in wrangler

Validate instance type against account limits to give early feedback to
the user.

* validate instance type in containers/deploy.ts

* clean up

* Update .changeset/twenty-seas-cross.md

Co-authored-by: emily-shen <[email protected]>

* refactor how limits are checked

* fix test

* Update changeset

* Update packages/wrangler/src/cloudchamber/limits.ts

Co-authored-by: emily-shen <[email protected]>

* Apply suggestions from code review

Co-authored-by: emily-shen <[email protected]>

* update comment

* fix tests

---------

Co-authored-by: emily-shen <[email protected]>

Author: nikitassharma

.changeset/twenty-seas-cross.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+Validate container configuration against account limits in wrangler to give early feedback to the user

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

@@ -6,15 +6,11 @@ import {
 	getCloudflareContainerRegistry,
 	runDockerCmd,
 } from "@cloudflare/containers-shared";
-import { ensureDiskLimits } from "../../cloudchamber/build";
 import { UserError } from "../../errors";
 import { mockAccountId, mockApiToken } from "../helpers/mock-account-id";
 import { runInTempDir } from "../helpers/run-in-tmp";
 import { runWrangler } from "../helpers/run-wrangler";
 import { mockAccountV4 as mockAccount } from "./utils";
-import type { CompleteAccountCustomer } from "@cloudflare/containers-shared";
-
-const MiB = 1024 * 1024;
 
 vi.mock("@cloudflare/containers-shared", async (importOriginal) => {
 	const actual = await importOriginal();
@@ -36,7 +32,11 @@ describe("buildAndMaybePush", () => {
 
 	beforeEach(() => {
 		vi.clearAllMocks();
-		vi.mocked(dockerImageInspect).mockResolvedValue("53387881 2 []");
+		vi.mocked(dockerImageInspect)
+			// return empty array of repo digests (i.e. image does not exist remotely)
+			.mockResolvedValueOnce("[]")
+			// return image size and number of layers
+			.mockResolvedValueOnce("53387881 2");
 		mkdirSync("./container-context");
 
 		writeFileSync("./container-context/Dockerfile", dockerfile);
@@ -65,11 +65,23 @@ describe("buildAndMaybePush", () => {
 			],
 			dockerfile,
 		});
-		expect(dockerImageInspect).toHaveBeenCalledWith("/custom/docker/path", {
-			imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
-			formatString:
-				"{{ .Size }} {{ len .RootFS.Layers }} {{json .RepoDigests}}",
-		});
+		expect(dockerImageInspect).toHaveBeenCalledTimes(2);
+		expect(dockerImageInspect).toHaveBeenNthCalledWith(
+			1,
+			"/custom/docker/path",
+			{
+				imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
+				formatString: "{{json .RepoDigests}}",
+			}
+		);
+		expect(dockerImageInspect).toHaveBeenNthCalledWith(
+			2,
+			"/custom/docker/path",
+			{
+				imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
+				formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
+			}
+		);
 		expect(runDockerCmd).toHaveBeenCalledWith("/custom/docker/path", [
 			"push",
 			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
@@ -115,11 +127,14 @@ describe("buildAndMaybePush", () => {
 			"rm",
 			`${getCloudflareContainerRegistry()}/some-account-id/test-app:tag`,
 		]);
-		expect(dockerImageInspect).toHaveBeenCalledOnce();
-		expect(dockerImageInspect).toHaveBeenCalledWith("docker", {
+		expect(dockerImageInspect).toHaveBeenCalledTimes(2);
+		expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
+			imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
+			formatString: "{{json .RepoDigests}}",
+		});
+		expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
 			imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
-			formatString:
-				"{{ .Size }} {{ len .RootFS.Layers }} {{json .RepoDigests}}",
+			formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
 		});
 		expect(dockerLoginManagedRegistry).toHaveBeenCalledOnce();
 	});
@@ -129,9 +144,12 @@ describe("buildAndMaybePush", () => {
 			abort: () => {},
 			ready: Promise.resolve({ aborted: false }),
 		});
-		vi.mocked(dockerImageInspect).mockResolvedValue(
-			'53387881 2 ["registry.cloudflare.com/test-app@sha256:three"]'
-		);
+		vi.mocked(dockerImageInspect).mockReset();
+		vi.mocked(dockerImageInspect)
+			.mockResolvedValueOnce(
+				'["registry.cloudflare.com/test-app@sha256:three"]'
+			)
+			.mockResolvedValueOnce("53387881 2");
 		await runWrangler(
 			"containers build ./container-context -t test-app:tag -p"
 		);
@@ -165,11 +183,14 @@ describe("buildAndMaybePush", () => {
 			"rm",
 			`${getCloudflareContainerRegistry()}/test-app:tag`,
 		]);
-		expect(dockerImageInspect).toHaveBeenCalledOnce();
-		expect(dockerImageInspect).toHaveBeenCalledWith("docker", {
+		expect(dockerImageInspect).toHaveBeenCalledTimes(2);
+		expect(dockerImageInspect).toHaveBeenNthCalledWith(1, "docker", {
 			imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
-			formatString:
-				"{{ .Size }} {{ len .RootFS.Layers }} {{json .RepoDigests}}",
+			formatString: "{{json .RepoDigests}}",
+		});
+		expect(dockerImageInspect).toHaveBeenNthCalledWith(2, "docker", {
+			imageTag: `${getCloudflareContainerRegistry()}/test-app:tag`,
+			formatString: "{{ .Size }} {{ len .RootFS.Layers }}",
 		});
 		expect(dockerLoginManagedRegistry).toHaveBeenCalledOnce();
 	});
@@ -266,44 +287,4 @@ describe("buildAndMaybePush", () => {
 			runWrangler("containers build ./container-context -t test-app:tag -p")
 		).rejects.toThrow(new UserError(errorMessage));
 	});
-
-	describe("ensureDiskLimits", () => {
-		const accountBase = {
-			limits: { disk_mb_per_deployment: 2000 },
-		} as CompleteAccountCustomer;
-
-		it("should throw error if app configured disk exceeds account limit", async () => {
-			await expect(() =>
-				ensureDiskLimits({
-					requiredSizeInBytes: 333 * MiB, // 333MiB
-					account: accountBase,
-					configDiskInBytes: 3000 * MiB, // ie 3GB - this exceeds the account limit of 2GB
-				})
-			).rejects.toThrowErrorMatchingInlineSnapshot(
-				`[Error: Exceeded account limits: Your container is configured to use a disk size of 3146MB. However, that exceeds the account limit of 2000MB]`
-			);
-		});
-
-		it("should throw error if image size exceeds allowed size", async () => {
-			await expect(() =>
-				ensureDiskLimits({
-					requiredSizeInBytes: 3000 * MiB, // 3GiB
-					account: accountBase,
-					configDiskInBytes: undefined,
-				})
-			).rejects.toThrowErrorMatchingInlineSnapshot(
-				`[Error: Image too large: needs 3146MB, but your app is limited to images with size 2000MB. Your account needs more disk size per instance to run this container. The default disk size is 2GB.]`
-			);
-		});
-
-		it("should not throw when disk size is within limits", async () => {
-			const result = await ensureDiskLimits({
-				requiredSizeInBytes: 256 * MiB, // 256MiB
-				account: accountBase,
-				configDiskInBytes: undefined,
-			});
-
-			expect(result).toEqual(undefined);
-		});
-	});
 });