pr feedback on instance_type

Author: emily-shen

packages/containers-shared/src/types.ts

@@ -41,7 +41,7 @@ export type RegistryLinkConfig = SharedContainerConfig & {
 export type InstanceTypeOrLimits =
 	| {
 			/** if undefined in config, defaults to instance_type */
-			disk_size?: number;
+			disk_mb?: number;
 			vcpu?: number;
 			memory_mib?: number;
 	  }

packages/wrangler/src/__tests__/containers/config.test.ts

@@ -187,7 +187,7 @@ describe("getNormalizedContainerOptions", () => {
 		});
 	});
 
-	it("should handle (deprecated) disk size configuration", async () => {
+	it("should handle custom limit configuration", async () => {
 		mockIsDockerfile.mockReturnValue(false);
 
 		const config: Config = {
@@ -201,7 +201,9 @@ describe("getNormalizedContainerOptions", () => {
 					class_name: "TestContainer",
 					image: "registry.example.com/test:latest",
 					configuration: {
-						disk: { size: "5GiB" },
+						disk: { size_mb: 5000 },
+						memory_mib: 1024,
+						vcpu: 2,
 					},
 				},
 			],
@@ -224,7 +226,9 @@ describe("getNormalizedContainerOptions", () => {
 			scheduling_policy: "default",
 			rollout_step_percentage: 25,
 			rollout_kind: "full_auto",
-			disk_size: 5368709120,
+			disk_mb: 5000,
+			memory_mib: 1024,
+			vcpu: 2,
 			registry_link: "registry.example.com/test:latest",
 			constraints: undefined,
 		});

packages/wrangler/src/cloudchamber/deploy.ts

@@ -37,7 +37,7 @@ export async function maybeBuildContainer(
 		},
 		pathToDocker,
 		!dryRun,
-		"disk_size" in containerConfig ? containerConfig.disk_size : undefined
+		"disk_mb" in containerConfig ? containerConfig.disk_mb : undefined
 	);
 
 	if (buildResult.pushed) {

packages/wrangler/src/config/environment.ts

@@ -114,20 +114,9 @@ export type ContainerApp = {
 		image?: string;
 		labels?: { name: string; value: string }[];
 		secrets?: { name: string; type: "env"; secret: string }[];
-		disk?:
-			| {
-					/**
-					 * @deprecated Use `size_mb` instead.
-					 */
-					size: string;
-			  }
-			| { size_mb: number };
+		disk?: { size_mb: number };
 		vcpu?: number;
 		memory_mib?: number;
-		/**
-		 * @deprecated Use `size_mb` instead.
-		 */
-		memory?: string;
 	};
 
 	/**

packages/wrangler/src/config/validation.ts

@@ -2447,8 +2447,7 @@ function validateContainerApp(
 					containerAppOptional.instance_type &&
 					(containerAppOptional.configuration.disk !== undefined ||
 						containerAppOptional.configuration.vcpu !== undefined ||
-						containerAppOptional.configuration.memory_mib !== undefined ||
-						containerAppOptional.configuration.memory !== undefined)
+						containerAppOptional.configuration.memory_mib !== undefined)
 				) {
 					diagnostics.errors.push(
 						`Cannot set custom limits via "containers.configuration" and use preset "instance_type" limits at the same time.`

packages/wrangler/src/containers/config.ts

@@ -6,7 +6,6 @@ import {
 	SchedulingPolicy,
 } from "@cloudflare/containers-shared";
 import { UserError } from "../errors";
-import { parseByteSize } from "../parse";
 import type { Config } from "../config";
 import type {
 	ContainerNormalisedConfig,
@@ -59,28 +58,12 @@ export const getNormalizedContainerOptions = async (
 		if (
 			container.configuration?.disk !== undefined ||
 			container.configuration?.vcpu !== undefined ||
-			container.configuration?.memory_mib !== undefined ||
-			// this is doubly deprecated, do we need to support this?
-			container.configuration?.memory !== undefined
+			container.configuration?.memory_mib !== undefined
 		) {
-			let normalisedDiskSize: number | undefined;
-			if (container.configuration?.disk !== undefined) {
-				normalisedDiskSize =
-					"size" in container.configuration.disk
-						? // 	// have i got the right units here?
-							Math.round(
-								parseByteSize(container.configuration.disk.size ?? "2GB")
-							)
-						: container.configuration.disk.size_mb;
-			}
-
 			instanceTypeOrDisk = {
-				disk_size: normalisedDiskSize,
+				disk_mb: container.configuration.disk?.size_mb,
 				vcpu: container.configuration?.vcpu,
-				memory_mib:
-					container.configuration.memory !== undefined
-						? Math.round(parseByteSize(container.configuration?.memory))
-						: container.configuration?.memory_mib,
+				memory_mib: container.configuration?.memory_mib,
 			};
 		} else {
 			instanceTypeOrDisk = {

packages/wrangler/src/containers/deploy.ts

@@ -199,7 +199,7 @@ function containerConfigToAPIConfig(
 			...("instance_type" in containerApp
 				? { instance_type: containerApp.instance_type }
 				: {
-						disk: { size_mb: containerApp.disk_size },
+						disk: { size_mb: containerApp.disk_mb },
 						memory_mib: containerApp.memory_mib,
 						vcpu: containerApp.vcpu,
 					}),
@@ -509,7 +509,7 @@ const doAction = async (
 };
 
 /**
- * clean up so we get a nicer diff
+ * clean up fields so we get a nicer diff
  */
 export function cleanPrevious(
 	prev: CreateApplicationRequest,