From 39b5a55a9c7f7646315043bdffb223f54eb17adf Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Tue, 15 Jul 2025 12:06:07 +0100 Subject: [PATCH 01/11] extract diff rendering from apply/deploy --- packages/wrangler/src/cloudchamber/apply.ts | 74 +----------------- .../wrangler/src/cloudchamber/helpers/diff.ts | 75 +++++++++++++++++- packages/wrangler/src/containers/deploy.ts | 77 +------------------ 3 files changed, 79 insertions(+), 147 deletions(-) diff --git a/packages/wrangler/src/cloudchamber/apply.ts b/packages/wrangler/src/cloudchamber/apply.ts index e949bd8b0156..8226bf5c1bea 100644 --- a/packages/wrangler/src/cloudchamber/apply.ts +++ b/packages/wrangler/src/cloudchamber/apply.ts @@ -11,7 +11,7 @@ import { success, updateStatus, } from "@cloudflare/cli"; -import { bold, brandColor, dim, green, red } from "@cloudflare/cli/colors"; +import { bold, brandColor, dim, green } from "@cloudflare/cli/colors"; import { ApiError, ApplicationsService, @@ -27,7 +27,6 @@ import { FatalError, UserError } from "../errors"; import { getAccountId } from "../user"; import { cleanForInstanceType, promiseSpinner } from "./common"; import { - createLine, diffLines, printLine, sortObjectRecursive, @@ -421,76 +420,7 @@ export async function apply( false ); - let printedLines: string[] = []; - let printedDiff = false; - // prints the lines we accumulated to bring context to the edited line - const printContext = () => { - let index = 0; - for (let i = printedLines.length - 1; i >= 0; i--) { - if (printedLines[i].trim().startsWith("[")) { - log(""); - index = i; - break; - } - } - - for (let i = index; i < printedLines.length; i++) { - log(printedLines[i]); - if (printedLines.length - i > 2) { - i = printedLines.length - 2; - printLine(dim("..."), " "); - } - } - - printedLines = []; - }; - - // go line by line and print diff results - for (const lines of results) { - const trimmedLines = (lines.value ?? "") - .split("\n") - .map((e) => e.trim()) - .filter((e) => e !== ""); - - for (const l of trimmedLines) { - if (lines.added) { - printContext(); - if (l.startsWith("[")) { - printLine(""); - } - - printedDiff = true; - printLine(l, green("+ ")); - } else if (lines.removed) { - printContext(); - if (l.startsWith("[")) { - printLine(""); - } - - printedDiff = true; - printLine(l, red("- ")); - } else { - // if we had printed a diff before this line, print a little bit more - // so the user has a bit more context on where the edit happens - if (printedDiff) { - let printDots = false; - if (l.startsWith("[")) { - printLine(""); - printDots = true; - } - - printedDiff = false; - printLine(l, " "); - if (printDots) { - printLine(dim("..."), " "); - } - continue; - } - - printedLines.push(createLine(l, " ")); - } - } - } + renderDiff(results); if (appConfigNoDefaults.rollout_kind !== "none") { actions.push({ diff --git a/packages/wrangler/src/cloudchamber/helpers/diff.ts b/packages/wrangler/src/cloudchamber/helpers/diff.ts index b40c1cbca754..d997f7250536 100644 --- a/packages/wrangler/src/cloudchamber/helpers/diff.ts +++ b/packages/wrangler/src/cloudchamber/helpers/diff.ts @@ -2,7 +2,7 @@ // It's been simplified so it can basically do line diffing only // and we can avoid the 600kb sized package. import { log } from "@cloudflare/cli"; -import { bold, brandColor, red } from "@cloudflare/cli/colors"; +import { bold, brandColor, dim, green, red } from "@cloudflare/cli/colors"; class Diff { diff(oldString: string[], newString: string[], callback: Callback) { @@ -488,3 +488,76 @@ export function sortObjectRecursive>( return sortObjectKeys(objectCopy) as T; } + +export const renderDiff = (results: Result[]) => { + let printedLines: string[] = []; + let printedDiff = false; + // prints the lines we accumulated to bring context to the edited line + const printContext = () => { + let index = 0; + for (let i = printedLines.length - 1; i >= 0; i--) { + if (printedLines[i].trim().startsWith("[")) { + log(""); + index = i; + break; + } + } + + for (let i = index; i < printedLines.length; i++) { + log(printedLines[i]); + if (printedLines.length - i > 2) { + i = printedLines.length - 2; + printLine(dim("..."), " "); + } + } + + printedLines = []; + }; + + // go line by line and print diff results + for (const lines of results) { + const trimmedLines = (lines.value ?? "") + .split("\n") + .map((e) => e.trim()) + .filter((e) => e !== ""); + + for (const l of trimmedLines) { + if (lines.added) { + printContext(); + if (l.startsWith("[")) { + printLine(""); + } + + printedDiff = true; + printLine(l, green("+ ")); + } else if (lines.removed) { + printContext(); + if (l.startsWith("[")) { + printLine(""); + } + + printedDiff = true; + printLine(l, red("- ")); + } else { + // if we had printed a diff before this line, print a little bit more + // so the user has a bit more context on where the edit happens + if (printedDiff) { + let printDots = false; + if (l.startsWith("[")) { + printLine(""); + printDots = true; + } + + printedDiff = false; + printLine(l, " "); + if (printDots) { + printLine(dim("..."), " "); + } + continue; + } + + printedLines.push(createLine(l, " ")); + } + } + } +}; diff --git a/packages/wrangler/src/containers/deploy.ts b/packages/wrangler/src/containers/deploy.ts index d23538456650..e79b89a49e49 100644 --- a/packages/wrangler/src/containers/deploy.ts +++ b/packages/wrangler/src/containers/deploy.ts @@ -10,7 +10,7 @@ import { success, updateStatus, } from "@cloudflare/cli"; -import { bold, brandColor, dim, green, red } from "@cloudflare/cli/colors"; +import { bold, brandColor, dim, green } from "@cloudflare/cli/colors"; import { ApiError, ApplicationsService, @@ -23,9 +23,9 @@ import { } from "@cloudflare/containers-shared"; import { cleanForInstanceType, promiseSpinner } from "../cloudchamber/common"; import { - createLine, diffLines, printLine, + renderDiff, sortObjectRecursive, stripUndefined, } from "../cloudchamber/helpers/diff"; @@ -380,78 +380,7 @@ export async function apply( `${brandColor.underline("EDIT")} ${application.name}`, false ); - - let printedLines: string[] = []; - let printedDiff = false; - // prints the lines we accumulated to bring context to the edited line - const printContext = () => { - let index = 0; - for (let i = printedLines.length - 1; i >= 0; i--) { - if (printedLines[i].trim().startsWith("[")) { - log(""); - index = i; - break; - } - } - - for (let i = index; i < printedLines.length; i++) { - log(printedLines[i]); - if (printedLines.length - i > 2) { - i = printedLines.length - 2; - printLine(dim("..."), " "); - } - } - - printedLines = []; - }; - - // go line by line and print diff results - for (const lines of results) { - const trimmedLines = (lines.value ?? "") - .split("\n") - .map((e) => e.trim()) - .filter((e) => e !== ""); - - for (const l of trimmedLines) { - if (lines.added) { - printContext(); - if (l.startsWith("[")) { - printLine(""); - } - - printedDiff = true; - printLine(l, green("+ ")); - } else if (lines.removed) { - printContext(); - if (l.startsWith("[")) { - printLine(""); - } - - printedDiff = true; - printLine(l, red("- ")); - } else { - // if we had printed a diff before this line, print a little bit more - // so the user has a bit more context on where the edit happens - if (printedDiff) { - let printDots = false; - if (l.startsWith("[")) { - printLine(""); - printDots = true; - } - - printedDiff = false; - printLine(l, " "); - if (printDots) { - printLine(dim("..."), " "); - } - continue; - } - - printedLines.push(createLine(l, " ")); - } - } - } - + renderDiff(results); if (appConfigNoDefaults.rollout_kind !== "none") { actions.push({ action: "modify", From 4b77a25b07d01b9c2f8208ba05e840ccfd3de957 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Tue, 15 Jul 2025 12:06:36 +0100 Subject: [PATCH 02/11] add config normalisation function --- packages/containers-shared/src/types.ts | 53 +++ .../__tests__/config/configuration.test.ts | 2 + .../src/__tests__/containers/config.test.ts | 443 ++++++++++++++++++ packages/wrangler/src/config/environment.ts | 25 +- packages/wrangler/src/config/validation.ts | 49 +- packages/wrangler/src/containers/config.ts | 119 +++++ 6 files changed, 661 insertions(+), 30 deletions(-) create mode 100644 packages/wrangler/src/__tests__/containers/config.test.ts create mode 100644 packages/wrangler/src/containers/config.ts diff --git a/packages/containers-shared/src/types.ts b/packages/containers-shared/src/types.ts index 98b370a385e0..bd15e97b0de8 100644 --- a/packages/containers-shared/src/types.ts +++ b/packages/containers-shared/src/types.ts @@ -1,3 +1,9 @@ +import { + CreateApplicationRolloutRequest, + InstanceType, + SchedulingPolicy, +} from "./client"; + export interface Logger { debug: (message: string) => void; log: (message: string) => void; @@ -20,6 +26,30 @@ export type BuildArgs = { setNetworkToHost?: boolean; }; +export type ContainerNormalisedConfig = RegistryLinkConfig | DockerfileConfig; +export type DockerfileConfig = SharedContainerConfig & { + /** absolute path, resolved relative to the wrangler config file */ + dockerfile: string; + /** absolute path, resolved relative to the wrangler config file. defaults to the directory of the dockerfile */ + image_build_context: string; + image_vars?: Record; +}; +export type RegistryLinkConfig = SharedContainerConfig & { + registry_link: string; +}; + +export type InstanceTypeOrLimits = + | { + /** if undefined in config, defaults to instance_type */ + disk_size?: number; + vcpu?: number; + memory_mib?: number; + } + | { + /** if undefined in config, defaults to "dev" */ + instance_type: InstanceType; + }; + /** build/pull agnostic container options */ export type ContainerDevOptions = { /** may be dockerfile or registry link */ @@ -32,3 +62,26 @@ export type ContainerDevOptions = { /** build time args */ args?: Record; }; + +/** + * Shared container config that is used regardless of whether the image is from a dockerfile or a registry link. + */ +export type SharedContainerConfig = { + /** if undefined in config, defaults to worker_name[-envName]-class_name. */ + name: string; + /** container's DO class name */ + class_name: string; + /** if undefined in config, defaults to 0 */ + max_instances: number; + /** if undefined in config, defaults to "default" */ + scheduling_policy: SchedulingPolicy; + /** if undefined in config, defaults to 25 */ + rollout_step_percentage: number; + /** if undefined in config, defaults to "full_auto" */ + rollout_kind: "full_auto" | "full_manual" | "none"; + constraints?: { + regions?: string[]; + cities?: string[]; + tier?: number; + }; +} & InstanceTypeOrLimits; diff --git a/packages/wrangler/src/__tests__/config/configuration.test.ts b/packages/wrangler/src/__tests__/config/configuration.test.ts index 01f8f3a7ae9a..36a116b2de18 100644 --- a/packages/wrangler/src/__tests__/config/configuration.test.ts +++ b/packages/wrangler/src/__tests__/config/configuration.test.ts @@ -2413,6 +2413,8 @@ describe("normalizeAndValidateConfig()", () => { }, class_name: "test-class", name: "test-worker-name-test-class", + // this has been set twice to accomodate both cloudchamber and containers + image: "something", }, ]); if (config.containers) { diff --git a/packages/wrangler/src/__tests__/containers/config.test.ts b/packages/wrangler/src/__tests__/containers/config.test.ts new file mode 100644 index 000000000000..942f276542d8 --- /dev/null +++ b/packages/wrangler/src/__tests__/containers/config.test.ts @@ -0,0 +1,443 @@ +import { isDockerfile } from "@cloudflare/containers-shared"; +import { vi } from "vitest"; +import { getNormalizedContainerOptions } from "../../containers/config"; +import { UserError } from "../../errors"; +import type { Config } from "../../config"; + +// Mock dependencies using vi.hoisted +vi.mock("@cloudflare/containers-shared"); + +const mockIsDockerfile = vi.mocked(isDockerfile); + +describe("getNormalizedContainerOptions", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("should return empty array when no containers are configured", async () => { + const config = { + name: "test-worker", + configPath: "/test/wrangler.toml", + userConfigPath: "/test/wrangler.toml", + topLevelName: "test-worker", + containers: [], + durable_objects: { + bindings: [], + }, + } as Partial as Config; + + const result = await getNormalizedContainerOptions(config); + expect(result).toEqual([]); + }); + + it("should return empty array when containers is undefined", async () => { + const config = { + name: "test-worker", + configPath: "/test/wrangler.toml", + userConfigPath: "/test/wrangler.toml", + topLevelName: "test-worker", + containers: undefined, + durable_objects: { + bindings: [], + }, + } as Partial as Config; + + const result = await getNormalizedContainerOptions(config); + expect(result).toEqual([]); + }); + + it("should throw error when container class_name doesn't match any durable object", async () => { + const config = { + name: "test-worker", + configPath: "/test/wrangler.toml", + userConfigPath: "/test/wrangler.toml", + topLevelName: "test-worker", + containers: [ + { + class_name: "TestContainer", + image: "test-image", + name: "test-container", + }, + ], + durable_objects: { + bindings: [], + }, + } as Partial as Config; + + await expect(getNormalizedContainerOptions(config)).rejects.toThrow( + UserError + ); + await expect(getNormalizedContainerOptions(config)).rejects.toThrow( + "The container class_name TestContainer does not match any durable object class_name defined in your Wrangler config file" + ); + }); + + it("should throw error when durable object has script_name defined", async () => { + const config: Config = { + name: "test-worker", + configPath: "/test/wrangler.toml", + userConfigPath: "/test/wrangler.toml", + topLevelName: "test-worker", + containers: [ + { + class_name: "TestContainer", + image: "test-image", + name: "test-container", + }, + ], + durable_objects: { + bindings: [ + { + name: "TEST_DO", + class_name: "TestContainer", + script_name: "other-script", + }, + ], + }, + } as Partial as Config; + + await expect(getNormalizedContainerOptions(config)).rejects.toThrow( + UserError + ); + await expect(getNormalizedContainerOptions(config)).rejects.toThrow( + "The container class_name TestContainer does not match any durable object class_name defined in your Wrangler config file" + ); + }); + + it("should normalize and set defaults for container with dockerfile", async () => { + mockIsDockerfile.mockReturnValue(true); + + const config: Config = { + name: "test-worker", + configPath: "/test/wrangler.toml", + userConfigPath: "/test/wrangler.toml", + topLevelName: "test-worker", + containers: [ + { + class_name: "TestContainer", + image: "./Dockerfile", + name: "test-container", + }, + ], + durable_objects: { + bindings: [ + { + name: "TEST_DO", + class_name: "TestContainer", + }, + ], + }, + } as Partial as Config; + + const result = await getNormalizedContainerOptions(config); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + name: "test-container", + class_name: "TestContainer", + max_instances: 0, + scheduling_policy: "default", + rollout_step_percentage: 25, + rollout_kind: "full_auto", + instance_type: "dev", + dockerfile: "/test/Dockerfile", + image_build_context: "/test", + image_vars: undefined, + constraints: undefined, + }); + }); + + it("should normalize and set defaults for container with registry image", async () => { + mockIsDockerfile.mockReturnValue(false); + + const config: Config = { + name: "test-worker", + configPath: "/test/wrangler.toml", + topLevelName: "test-worker", + containers: [ + { + class_name: "TestContainer", + image: "registry.example.com/test:latest", + name: "test-container", + }, + ], + durable_objects: { + bindings: [ + { + name: "TEST_DO", + class_name: "TestContainer", + }, + ], + }, + } as Partial as Config; + + const result = await getNormalizedContainerOptions(config); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + name: "test-container", + class_name: "TestContainer", + max_instances: 0, + scheduling_policy: "default", + rollout_step_percentage: 25, + rollout_kind: "full_auto", + instance_type: "dev", + registry_link: "registry.example.com/test:latest", + constraints: undefined, + }); + }); + + it("should handle (deprecated) disk size configuration", async () => { + mockIsDockerfile.mockReturnValue(false); + + const config: Config = { + name: "test-worker", + configPath: "/test/wrangler.toml", + userConfigPath: "/test/wrangler.toml", + topLevelName: "test-worker", + containers: [ + { + name: "test-container", + class_name: "TestContainer", + image: "registry.example.com/test:latest", + configuration: { + disk: { size: "5GiB" }, + }, + }, + ], + durable_objects: { + bindings: [ + { + name: "TEST_DO", + class_name: "TestContainer", + }, + ], + }, + } as Partial as Config; + + const result = await getNormalizedContainerOptions(config); + + expect(result[0]).toEqual({ + name: "test-container", + class_name: "TestContainer", + max_instances: 0, + scheduling_policy: "default", + rollout_step_percentage: 25, + rollout_kind: "full_auto", + disk_size: 5368709120, + registry_link: "registry.example.com/test:latest", + constraints: undefined, + }); + }); + + it("should handle instance type configuration", async () => { + mockIsDockerfile.mockReturnValue(false); + + const config: Config = { + name: "test-worker", + configPath: "/test/wrangler.toml", + userConfigPath: "/test/wrangler.toml", + topLevelName: "test-worker", + containers: [ + { + class_name: "TestContainer", + image: "registry.example.com/test:latest", + instance_type: "standard", + name: "test-container", + }, + ], + durable_objects: { + bindings: [ + { + name: "TEST_DO", + class_name: "TestContainer", + }, + ], + }, + } as Partial as Config; + + const result = await getNormalizedContainerOptions(config); + + expect(result[0]).toEqual({ + name: "test-container", + class_name: "TestContainer", + max_instances: 0, + scheduling_policy: "default", + rollout_step_percentage: 25, + rollout_kind: "full_auto", + instance_type: "standard", + registry_link: "registry.example.com/test:latest", + constraints: undefined, + }); + }); + + it("should handle all custom configuration options", async () => { + mockIsDockerfile.mockReturnValue(false); + + const config: Config = { + name: "test-worker", + configPath: "/test/wrangler.toml", + userConfigPath: "/test/wrangler.toml", + topLevelName: "test-worker", + containers: [ + { + name: "custom-name", + class_name: "TestContainer", + image: "registry.example.com/test:latest", + max_instances: 10, + scheduling_policy: "regional", + rollout_step_percentage: 50, + rollout_kind: "full_manual", + instance_type: "basic", + constraints: { + regions: ["us-east-1", "us-west-2"], + cities: ["NYC", "SF"], + tier: 1, + }, + }, + ], + durable_objects: { + bindings: [ + { + name: "TEST_DO", + class_name: "TestContainer", + }, + ], + }, + } as Partial as Config; + + const result = await getNormalizedContainerOptions(config); + + expect(result[0]).toEqual({ + name: "custom-name", + class_name: "TestContainer", + max_instances: 10, + scheduling_policy: "regional", + rollout_step_percentage: 50, + rollout_kind: "full_manual", + instance_type: "basic", + registry_link: "registry.example.com/test:latest", + constraints: { + regions: ["us-east-1", "us-west-2"], + cities: ["NYC", "SF"], + tier: 1, + }, + }); + }); + + it("should handle dockerfile with default build context", async () => { + mockIsDockerfile.mockReturnValue(true); + + const config: Config = { + name: "test-worker", + configPath: "/test/wrangler.toml", + userConfigPath: "/test/wrangler.toml", + topLevelName: "test-worker", + containers: [ + { + class_name: "TestContainer", + image: "./path/to/Dockerfile", + name: "test-container", + }, + ], + durable_objects: { + bindings: [ + { + name: "TEST_DO", + class_name: "TestContainer", + }, + ], + }, + } as Partial as Config; + + const result = await getNormalizedContainerOptions(config); + + expect(result[0]).toEqual({ + name: "test-container", + class_name: "TestContainer", + max_instances: 0, + scheduling_policy: "default", + rollout_step_percentage: 25, + rollout_kind: "full_auto", + instance_type: "dev", + dockerfile: "/test/path/to/Dockerfile", + image_build_context: "/test/path/to", + image_vars: undefined, + constraints: undefined, + }); + }); + + it("should handle multiple containers", async () => { + mockIsDockerfile.mockReturnValue(false); + + const config: Config = { + name: "test-worker", + configPath: "/test/wrangler.toml", + userConfigPath: "/test/wrangler.toml", + topLevelName: "test-worker", + containers: [ + { + class_name: "Container1", + image: "registry.example.com/test1:latest", + name: "test-container", + }, + { + class_name: "Container2", + image: "registry.example.com/test2:latest", + name: "test-container-two", + }, + ], + durable_objects: { + bindings: [ + { + name: "DO1", + class_name: "Container1", + }, + { + name: "DO2", + class_name: "Container2", + }, + ], + }, + } as Partial as Config; + + const result = await getNormalizedContainerOptions(config); + + expect(result).toHaveLength(2); + expect(result[0].class_name).toBe("Container1"); + expect(result[1].class_name).toBe("Container2"); + }); + + it("should handle config with no configPath", async () => { + mockIsDockerfile.mockReturnValue(true); + + const config: Config = { + name: "test-worker", + configPath: undefined, + userConfigPath: undefined, + topLevelName: "test-worker", + containers: [ + { + class_name: "TestContainer", + image: "./Dockerfile", + name: "test-container", + }, + ], + durable_objects: { + bindings: [ + { + name: "TEST_DO", + class_name: "TestContainer", + }, + ], + }, + } as Partial as Config; + + const result = await getNormalizedContainerOptions(config); + + // Check that it has dockerfile properties (not registry_link) + expect(result[0]).toHaveProperty("dockerfile"); + expect(result[0]).toHaveProperty("image_build_context"); + expect(result[0]).not.toHaveProperty("registry_link"); + }); +}); diff --git a/packages/wrangler/src/config/environment.ts b/packages/wrangler/src/config/environment.ts index 5e27462531d6..960e31ab06e6 100644 --- a/packages/wrangler/src/config/environment.ts +++ b/packages/wrangler/src/config/environment.ts @@ -92,25 +92,42 @@ export type ContainerApp = { * @optional * @default "default" */ - scheduling_policy?: "regional" | "moon" | "default"; + scheduling_policy?: "default" | "moon" | "regional"; /** - * The instance type to be used for the container. This sets preconfigured options for vcpu and memory + * The instance type to be used for the container. + * dev = 1/16 vCPU, 256 MiB memory, and 2 GB disk + * basic = 1/4 vCPU, 1 GiB memory, and 4 GB disk + * standard = 1/2 vCPU, 4 GiB memory, and 4 GB disk * @optional + * @default "dev" */ instance_type?: "dev" | "basic" | "standard"; /** * @deprecated Use top level `containers` fields instead. * `configuration.image` should be `image` - * `configuration.disk` should be set via `instance_type` + * limits should be set via `instance_type` * @hidden */ configuration?: { image?: string; labels?: { name: string; value: string }[]; secrets?: { name: string; type: "env"; secret: string }[]; - disk?: { size: string }; + disk?: + | { + /** + * @deprecated Use `size_mb` instead. + */ + size: string; + } + | { size_mb: number }; + vcpu?: number; + memory_mib?: number; + /** + * @deprecated Use `size_mb` instead. + */ + memory?: string; }; /** diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts index 33965f72e376..64659e3e2a01 100644 --- a/packages/wrangler/src/config/validation.ts +++ b/packages/wrangler/src/config/validation.ts @@ -2429,34 +2429,39 @@ function validateContainerApp( `"containers.image" field must be defined for each container app. This should be the path to your Dockerfile or a image URI pointing to the Cloudflare registry.` ); } + if ("configuration" in containerAppOptional) { diagnostics.warnings.push( `"containers.configuration" is deprecated. Use top level "containers" fields instead. "configuration.image" should be "image", "configuration.disk" should be set via "instance_type".` ); - } + if ( + typeof containerAppOptional !== "object" || + Array.isArray(containerAppOptional.configuration) + ) { + diagnostics.errors.push( + `"containers.configuration" should be an object` + ); + } - // Validate that we have an image configuration for this container app. - // For legacy reasons we have to check both at containerAppOptional.image and - // containerAppOptional.configuration.image. - // - // - // At the moment logic in other places downstream of this rely on containerAppOptional.configuration.image be set - // so we set it here regardless of which place it is set by the user. - if ( - "image" in containerAppOptional && - containerAppOptional.image !== undefined - ) { - if (containerAppOptional.configuration?.image !== undefined) { + if ( + containerAppOptional.instance_type && + (containerAppOptional.configuration.disk !== undefined || + containerAppOptional.configuration.vcpu !== undefined || + containerAppOptional.configuration.memory_mib !== undefined || + containerAppOptional.configuration.memory !== undefined) + ) { diagnostics.errors.push( - `"containers.image" and "containers.configuration.image" fields can't be defined at the same time.` + `Cannot set custom limits via "containers.configuration" and use preset "instance_type" limits at the same time.` ); - return false; } - // consolidate the image into the configuration object - // TODO: consolidate it into the top level image field instead + } + + // make sure image field is always set + if (containerAppOptional.configuration?.image !== undefined) { + containerAppOptional.image = containerAppOptional.configuration.image; + } else { containerAppOptional.configuration ??= {}; containerAppOptional.configuration.image = containerAppOptional.image; - delete containerAppOptional["image"]; } // Validate rollout related configs @@ -2473,14 +2478,6 @@ function validateContainerApp( `"containers.rollout_step_percentage" field should be a number between 25 and 100, but got ${containerAppOptional.rollout_step_percentage}` ); } - // Leaving for legacy reasons - // TODO: When cleaning up container.configuration usage in other places clean this up - // as well. - if (Array.isArray(containerAppOptional.configuration)) { - diagnostics.errors.push( - `"containers.configuration" is defined as an array, it should be an object` - ); - } validateOptionalProperty( diagnostics, field, diff --git a/packages/wrangler/src/containers/config.ts b/packages/wrangler/src/containers/config.ts new file mode 100644 index 000000000000..43dd8439885a --- /dev/null +++ b/packages/wrangler/src/containers/config.ts @@ -0,0 +1,119 @@ +import assert from "node:assert"; +import path, { dirname } from "node:path"; +import { + InstanceType, + isDockerfile, + SchedulingPolicy, +} from "@cloudflare/containers-shared"; +import { UserError } from "../errors"; +import { parseByteSize } from "../parse"; +import type { Config } from "../config"; +import type { + ContainerNormalisedConfig, + InstanceTypeOrLimits, + SharedContainerConfig, +} from "@cloudflare/containers-shared"; + +// This normalises config into an intermediate shape for building or pulling. +// We set defaults here too, because we can assume that if the value is undefined, +// we want to revert to the default rather than inheriting from the prev deployment + +// todo add args resolution +export const getNormalizedContainerOptions = async ( + config: Config +): Promise => { + if (!config.containers || config.containers.length === 0) { + return []; + } + + const normalizedContainers: ContainerNormalisedConfig[] = []; + + for (const container of config.containers) { + assert(container.name, "container name should have been set by validation"); + const targetDurableObject = config.durable_objects.bindings.find( + (durableObject) => + durableObject.class_name === container.class_name && + // the durable object must be defined in the same script as the container + durableObject.script_name === undefined + ); + + if (!targetDurableObject) { + throw new UserError( + `The container class_name ${container.class_name} does not match any durable object class_name defined in your Wrangler config file. Note that the durable object must be defined in the same script as the container.` + ); + } + + const shared: Omit = { + name: container.name, + class_name: container.class_name, + max_instances: container.max_instances ?? 0, // :( + scheduling_policy: (container.scheduling_policy ?? + SchedulingPolicy.DEFAULT) as SchedulingPolicy, + constraints: container.constraints, + rollout_step_percentage: container.rollout_step_percentage ?? 25, + rollout_kind: container.rollout_kind ?? "full_auto", // this is the default in the API, so we use it here too , + }; + + let instanceTypeOrDisk: InstanceTypeOrLimits; + + 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 + ) { + 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, + vcpu: container.configuration?.vcpu, + memory_mib: + container.configuration.memory !== undefined + ? Math.round(parseByteSize(container.configuration?.memory)) + : container.configuration?.memory_mib, + }; + } else { + instanceTypeOrDisk = { + instance_type: (container.instance_type ?? + InstanceType.DEV) as InstanceType, + }; + } + + const maybeDockerfile = isDockerfile(container.image, config.configPath); + if (maybeDockerfile) { + const baseDir = config.configPath + ? dirname(config.configPath) + : process.cwd(); + + const absoluteDockerfilePath = path.resolve(baseDir, container.image); + const absoluteBuildContextPath = container.image_build_context + ? path.resolve(baseDir, container.image_build_context) + : dirname(absoluteDockerfilePath); + normalizedContainers.push({ + ...shared, + ...instanceTypeOrDisk, + dockerfile: absoluteDockerfilePath, + image_build_context: absoluteBuildContextPath, + image_vars: container.image_vars, + }); + } else { + normalizedContainers.push({ + ...shared, + ...instanceTypeOrDisk, + registry_link: container.image, // if it is not a dockerfile, it must be a registry link or have thrown an error + }); + } + } + + return normalizedContainers; +}; From 7e2730cdf4c56e1d3d451d62c7ce8ab086f3de3f Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Mon, 14 Jul 2025 21:08:45 +0100 Subject: [PATCH 03/11] refactor dev to use normalised config --- packages/containers-shared/src/build.ts | 34 ++++------ packages/containers-shared/src/images.ts | 32 +++++----- packages/containers-shared/src/types.ts | 15 +---- packages/containers-shared/src/utils.ts | 7 ++- .../containers-shared/tests/utils.test.ts | 20 +++--- .../api/startDevWorker/ConfigController.ts | 10 +-- .../startDevWorker/LocalRuntimeController.ts | 63 ++++++------------- .../wrangler/src/api/startDevWorker/types.ts | 7 ++- packages/wrangler/src/dev/miniflare.ts | 5 +- 9 files changed, 77 insertions(+), 116 deletions(-) diff --git a/packages/containers-shared/src/build.ts b/packages/containers-shared/src/build.ts index ae63f440a0ee..0cd8e04286ae 100644 --- a/packages/containers-shared/src/build.ts +++ b/packages/containers-shared/src/build.ts @@ -1,12 +1,9 @@ import { spawn } from "child_process"; import { readFileSync } from "fs"; -import path from "path"; -import { BuildArgs, ContainerDevOptions, Logger } from "./types"; +import { BuildArgs, DockerfileConfig, Logger } from "./types"; export async function constructBuildCommand( options: BuildArgs, - /** wrangler config path. used to resolve relative dockerfile path */ - configPath: string | undefined, logger?: Logger ) { const platform = options.platform ?? "linux/amd64"; @@ -27,12 +24,11 @@ export async function constructBuildCommand( if (options.setNetworkToHost) { buildCmd.push("--network", "host"); } - const baseDir = configPath ? path.dirname(configPath) : process.cwd(); - const absDockerfilePath = path.resolve(baseDir, options.pathToDockerfile); - const dockerfile = readFileSync(absDockerfilePath, "utf-8"); + + const dockerfile = readFileSync(options.pathToDockerfile, "utf-8"); // pipe in the dockerfile buildCmd.push("-f", "-"); - buildCmd.push(options.buildContext ?? path.dirname(absDockerfilePath)); + buildCmd.push(options.buildContext); logger?.debug(`Building image with command: ${buildCmd.join(" ")}`); return { buildCmd, dockerfile }; } @@ -92,20 +88,16 @@ export function dockerBuild( export async function buildImage( dockerPath: string, - options: ContainerDevOptions, - configPath: string | undefined + options: DockerfileConfig, + imageTag: string ) { - // just let the tag default to latest - const { buildCmd, dockerfile } = await constructBuildCommand( - { - tag: options.imageTag, - pathToDockerfile: options.image, - buildContext: options.imageBuildContext, - args: options.args, - platform: "linux/amd64", - }, - configPath - ); + const { buildCmd, dockerfile } = await constructBuildCommand({ + tag: imageTag, + pathToDockerfile: options.dockerfile, + buildContext: options.image_build_context, + args: options.image_vars, + platform: "linux/amd64", + }); return dockerBuild(dockerPath, { buildCmd, dockerfile }); } diff --git a/packages/containers-shared/src/images.ts b/packages/containers-shared/src/images.ts index 0997f79cbb6e..8c6314e524b5 100644 --- a/packages/containers-shared/src/images.ts +++ b/packages/containers-shared/src/images.ts @@ -1,25 +1,26 @@ import { buildImage } from "./build"; import { getCloudflareContainerRegistry, + getDevContainerImageName, isCloudflareRegistryLink, } from "./knobs"; import { dockerLoginManagedRegistry } from "./login"; -import { ContainerDevOptions } from "./types"; +import { ContainerNormalisedConfig, RegistryLinkConfig } from "./types"; import { checkExposedPorts, - isDockerfile, runDockerCmd, verifyDockerInstalled, } from "./utils"; export async function pullImage( dockerPath: string, - options: ContainerDevOptions + options: RegistryLinkConfig, + tag: string ): Promise<{ abort: () => void; ready: Promise }> { await dockerLoginManagedRegistry(dockerPath); const pull = runDockerCmd(dockerPath, [ "pull", - options.image, + options.registry_link, // All containers running on our platform need to be built for amd64 architecture, but by default docker pull seems to look for an image matching the host system, so we need to specify this here "--platform", "linux/amd64", @@ -27,7 +28,7 @@ export async function pullImage( const ready = pull.ready.then(async ({ aborted }: { aborted: boolean }) => { if (!aborted) { // re-tag image with the expected dev-formatted image tag for consistency - await runDockerCmd(dockerPath, ["tag", options.image, options.imageTag]); + await runDockerCmd(dockerPath, ["tag", options.registry_link, tag]); } }); @@ -51,14 +52,14 @@ export async function pullImage( */ export async function prepareContainerImagesForDev( dockerPath: string, - containerOptions: ContainerDevOptions[], - configPath: string | undefined, + containerOptions: ContainerNormalisedConfig[], + containerBuildId: string, onContainerImagePreparationStart: (args: { - containerOptions: ContainerDevOptions; + containerOptions: ContainerNormalisedConfig; abort: () => void; }) => void, onContainerImagePreparationEnd: (args: { - containerOptions: ContainerDevOptions; + containerOptions: ContainerNormalisedConfig; }) => void ) { let aborted = false; @@ -69,8 +70,9 @@ export async function prepareContainerImagesForDev( } await verifyDockerInstalled(dockerPath); for (const options of containerOptions) { - if (isDockerfile(options.image, configPath)) { - const build = await buildImage(dockerPath, options, configPath); + const tag = getDevContainerImageName(options.class_name, containerBuildId); + if ("dockerfile" in options) { + const build = await buildImage(dockerPath, options, tag); onContainerImagePreparationStart({ containerOptions: options, abort: () => { @@ -83,13 +85,13 @@ export async function prepareContainerImagesForDev( containerOptions: options, }); } else { - if (!isCloudflareRegistryLink(options.image)) { + if (!isCloudflareRegistryLink(options.registry_link)) { throw new Error( - `Image "${options.image}" is a registry link but does not point to the Cloudflare container registry.\n` + + `Image "${options.registry_link}" is a registry link but does not point to the Cloudflare container registry.\n` + `To use an existing image from another repository, see https://developers.cloudflare.com/containers/image-management/#using-existing-images` ); } - const pull = await pullImage(dockerPath, options); + const pull = await pullImage(dockerPath, options, tag); onContainerImagePreparationStart({ containerOptions: options, abort: () => { @@ -103,7 +105,7 @@ export async function prepareContainerImagesForDev( }); } if (!aborted) { - await checkExposedPorts(dockerPath, options); + await checkExposedPorts(dockerPath, options, tag); } } } diff --git a/packages/containers-shared/src/types.ts b/packages/containers-shared/src/types.ts index bd15e97b0de8..678ce751aa11 100644 --- a/packages/containers-shared/src/types.ts +++ b/packages/containers-shared/src/types.ts @@ -17,7 +17,7 @@ export type BuildArgs = { tag: string; pathToDockerfile: string; /** image_build_context or args.PATH. if not provided, defaults to the dockerfile directory */ - buildContext?: string; + buildContext: string; /** any env vars that should be passed in at build time */ args?: Record; /** platform to build for. defaults to linux/amd64 */ @@ -50,19 +50,6 @@ export type InstanceTypeOrLimits = instance_type: InstanceType; }; -/** build/pull agnostic container options */ -export type ContainerDevOptions = { - /** may be dockerfile or registry link */ - image: string; - /** formatted as cloudflare-dev/workername-DOclassname:build-id */ - imageTag: string; - /** container's DO class name */ - class_name: string; - imageBuildContext?: string; - /** build time args */ - args?: Record; -}; - /** * Shared container config that is used regardless of whether the image is from a dockerfile or a registry link. */ diff --git a/packages/containers-shared/src/utils.ts b/packages/containers-shared/src/utils.ts index 87a5cbddb2f1..e1b6e60fbe39 100644 --- a/packages/containers-shared/src/utils.ts +++ b/packages/containers-shared/src/utils.ts @@ -2,7 +2,7 @@ import { execFile, spawn, StdioOptions } from "child_process"; import { existsSync, statSync } from "fs"; import path from "path"; import { dockerImageInspect } from "./inspect"; -import { ContainerDevOptions } from "./types"; +import { ContainerNormalisedConfig } from "./types"; /** helper for simple docker command call that don't require any io handling */ export const runDockerCmd = ( @@ -201,10 +201,11 @@ const getContainerIdsFromImage = async ( */ export async function checkExposedPorts( dockerPath: string, - options: ContainerDevOptions + options: ContainerNormalisedConfig, + imageTag: string ) { const output = await dockerImageInspect(dockerPath, { - imageTag: options.imageTag, + imageTag: imageTag, formatString: "{{ len .Config.ExposedPorts }}", }); if (output === "0") { diff --git a/packages/containers-shared/tests/utils.test.ts b/packages/containers-shared/tests/utils.test.ts index e94f1a0ef943..0e0569c6cf40 100644 --- a/packages/containers-shared/tests/utils.test.ts +++ b/packages/containers-shared/tests/utils.test.ts @@ -1,4 +1,5 @@ import { mkdirSync, writeFileSync } from "fs"; +import { ContainerNormalisedConfig } from "../src/types"; import { checkExposedPorts, isDockerfile } from "./../src/utils"; import { runInTempDir } from "./helpers/run-in-tmp-dir"; @@ -71,6 +72,10 @@ vi.mock("../src/inspect", async (importOriginal) => { }; }); +const containerConfig = { + dockerfile: "", + class_name: "MyContainer", +} as ContainerNormalisedConfig; describe("checkExposedPorts", () => { beforeEach(() => { docketImageInspectResult = "1"; @@ -79,23 +84,14 @@ describe("checkExposedPorts", () => { it("should not error when some ports are exported", async () => { docketImageInspectResult = "1"; await expect( - checkExposedPorts("./container-context/Dockerfile", { - image: "", - imageTag: "", - class_name: "MyContainer", - }) + checkExposedPorts("docker", containerConfig, "image:tag") ).resolves.toBeUndefined(); }); it("should error, with an appropriate message when no ports are exported", async () => { docketImageInspectResult = "0"; - expect( - checkExposedPorts("./container-context/Dockerfile", { - image: "", - imageTag: "", - class_name: "MyContainer", - }) - ).rejects.toThrowErrorMatchingInlineSnapshot(` + expect(checkExposedPorts("docker", containerConfig, "image:tag")).rejects + .toThrowErrorMatchingInlineSnapshot(` [Error: The container "MyContainer" does not expose any ports. In your Dockerfile, please expose any ports you intend to connect to. For additional information please see: https://developers.cloudflare.com/containers/local-dev/#exposing-ports. ] diff --git a/packages/wrangler/src/api/startDevWorker/ConfigController.ts b/packages/wrangler/src/api/startDevWorker/ConfigController.ts index ecddcaa8ed43..4a865db3ce06 100644 --- a/packages/wrangler/src/api/startDevWorker/ConfigController.ts +++ b/packages/wrangler/src/api/startDevWorker/ConfigController.ts @@ -1,11 +1,11 @@ import assert from "node:assert"; import path from "node:path"; -import { isDockerfile } from "@cloudflare/containers-shared"; import { watch } from "chokidar"; import { getAssetsOptions, validateAssetsArgsAndConfig } from "../../assets"; import { fillOpenAPIConfiguration } from "../../cloudchamber/common"; import { readConfig } from "../../config"; import { containersScope } from "../../containers"; +import { getNormalizedContainerOptions } from "../../containers/config"; import { getEntry } from "../../deployment-bundle/entry"; import { getBindings, @@ -347,7 +347,7 @@ async function resolveConfig( tsconfig: input.build?.tsconfig ?? config.tsconfig, exports: entry.exports, }, - containers: config.containers, + containers: await getNormalizedContainerOptions(config), dev: await resolveDevConfig(config, input), legacy: { site: legacySite, @@ -399,10 +399,10 @@ async function resolveConfig( // for pulling containers, we need to make sure the OpenAPI config for the // container API client is properly set so that we can get the correct permissions // from the cloudchamber API to pull from the repository. - const needsPulling = resolved.containers?.some( - (c) => !isDockerfile(c.image ?? c.configuration?.image, config.configPath) + const needsPulling = resolved.containers.some( + (c) => "registry_link" in c && c.registry_link ); - if (needsPulling && !resolved.dev.remote) { + if (resolved.containers.length && needsPulling && !resolved.dev.remote) { await fillOpenAPIConfiguration(config, containersScope); } diff --git a/packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts b/packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts index 64ddaddb7be4..15a9a1204954 100644 --- a/packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts +++ b/packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts @@ -27,7 +27,7 @@ import type { ReloadStartEvent, } from "./events"; import type { Binding, File, StartDevWorkerOptions } from "./types"; -import type { ContainerDevOptions } from "@cloudflare/containers-shared"; +import type { ContainerNormalisedConfig } from "@cloudflare/containers-shared"; async function getBinaryFileContents(file: File) { if ("contents" in file) { @@ -183,7 +183,7 @@ export class LocalRuntimeController extends RuntimeController { // Used to store the information and abort handle for the // current container that is being built containerBeingBuilt?: { - containerOptions: ContainerDevOptions; + containerOptions: ContainerNormalisedConfig; abort: () => void; abortRequested: boolean; }; @@ -220,21 +220,30 @@ export class LocalRuntimeController extends RuntimeController { } // Assemble container options and build if necessary - const containerOptions = await getContainerOptions(data.config); - this.#dockerPath = data.config.dev?.dockerPath ?? getDockerPath(); - // keep track of them so we can clean up later - for (const container of containerOptions ?? []) { - this.#containerImageTagsSeen.add(container.imageTag); - } + if ( - containerOptions && + data.config.containers?.length && + data.config.dev.enableContainers && this.#currentContainerBuildId !== data.config.dev.containerBuildId ) { + this.#dockerPath = data.config.dev?.dockerPath ?? getDockerPath(); + assert( + data.config.dev.containerBuildId, + "Build ID should be set if containers are enabled and defined" + ); + for (const container of data.config.containers) { + this.#containerImageTagsSeen.add( + getDevContainerImageName( + container.class_name, + data.config.dev.containerBuildId + ) + ); + } logger.log(chalk.dim("⎔ Preparing container image(s)...")); await prepareContainerImagesForDev( this.#dockerPath, - containerOptions, - data.config.config, + data.config.containers, + data.config.dev.containerBuildId, (buildStartEvent) => { this.containerBeingBuilt = { ...buildStartEvent, @@ -422,35 +431,3 @@ export class LocalRuntimeController extends RuntimeController { this.emit("reloadComplete", data); } } - -/** - * @returns Container options suitable for building or pulling images, - * with image tag set to well-known dev format. - * Undefined if containers are not enabled or not configured. - */ -export async function getContainerOptions( - config: BundleCompleteEvent["config"] -) { - if (!config.containers?.length || config.dev.enableContainers === false) { - return undefined; - } - // should be defined if containers are enabled - assert( - config.dev.containerBuildId, - "Build ID should be set if containers are enabled and defined" - ); - const containers: ContainerDevOptions[] = []; - for (const container of config.containers) { - containers.push({ - image: container.image ?? container.configuration?.image, - imageTag: getDevContainerImageName( - container.class_name, - config.dev.containerBuildId - ), - args: container.image_vars, - imageBuildContext: container.image_build_context, - class_name: container.class_name, - }); - } - return containers; -} diff --git a/packages/wrangler/src/api/startDevWorker/types.ts b/packages/wrangler/src/api/startDevWorker/types.ts index 4e6d4a9506dd..d8475245b9f7 100644 --- a/packages/wrangler/src/api/startDevWorker/types.ts +++ b/packages/wrangler/src/api/startDevWorker/types.ts @@ -37,6 +37,7 @@ import type { CfAccount } from "../../dev/create-worker-preview"; import type { EsbuildBundle } from "../../dev/use-esbuild"; import type { ConfigController } from "./ConfigController"; import type { DevEnv } from "./DevEnv"; +import type { ContainerNormalisedConfig } from "@cloudflare/containers-shared"; import type { DispatchFetch, Json, @@ -204,7 +205,10 @@ export interface StartDevWorkerInput { assets?: string; } -export type StartDevWorkerOptions = Omit & { +export type StartDevWorkerOptions = Omit< + StartDevWorkerInput, + "assets" | "containers" +> & { /** A worker's directory. Usually where the Wrangler configuration file is located */ projectRoot: string; build: StartDevWorkerInput["build"] & { @@ -227,6 +231,7 @@ export type StartDevWorkerOptions = Omit & { }; entrypoint: string; assets?: AssetsOptions; + containers?: ContainerNormalisedConfig[]; name: string; complianceRegion: Config["compliance_region"]; }; diff --git a/packages/wrangler/src/dev/miniflare.ts b/packages/wrangler/src/dev/miniflare.ts index adb6582be01a..d851455e7409 100644 --- a/packages/wrangler/src/dev/miniflare.ts +++ b/packages/wrangler/src/dev/miniflare.ts @@ -29,7 +29,7 @@ import { getClassNamesWhichUseSQLite } from "./class-names-sqlite"; import type { ServiceFetch } from "../api"; import type { AssetsOptions } from "../assets"; import type { Config } from "../config"; -import type { ContainerApp, ContainerEngine } from "../config/environment"; +import type { ContainerEngine } from "../config/environment"; import type { CfD1Database, CfDispatchNamespace, @@ -48,6 +48,7 @@ import type { WorkerRegistry } from "../dev-registry"; import type { LoggerLevel } from "../logger"; import type { LegacyAssetPaths } from "../sites"; import type { EsbuildBundle } from "./use-esbuild"; +import type { ContainerNormalisedConfig } from "@cloudflare/containers-shared"; import type { DOContainerOptions, MiniflareOptions, @@ -207,7 +208,7 @@ export interface ConfigBundle { bindVectorizeToProd: boolean; imagesLocalMode: boolean; testScheduled: boolean; - containers: ContainerApp[] | undefined; + containers: ContainerNormalisedConfig[] | undefined; containerBuildId: string | undefined; containerEngine: ContainerEngine | undefined; } From a94af93fc51c172b04e404e83082598539098209 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Mon, 14 Jul 2025 21:12:51 +0100 Subject: [PATCH 04/11] refactor containers deploy to use normalised config --- packages/wrangler/src/cloudchamber/build.ts | 5 +- packages/wrangler/src/cloudchamber/common.ts | 2 +- packages/wrangler/src/cloudchamber/deploy.ts | 142 ++--- packages/wrangler/src/containers/deploy.ts | 563 +++++++++---------- packages/wrangler/src/deploy/deploy.ts | 27 +- 5 files changed, 314 insertions(+), 425 deletions(-) diff --git a/packages/wrangler/src/cloudchamber/build.ts b/packages/wrangler/src/cloudchamber/build.ts index d59ccf5dc9be..349e48d0749f 100644 --- a/packages/wrangler/src/cloudchamber/build.ts +++ b/packages/wrangler/src/cloudchamber/build.ts @@ -80,7 +80,6 @@ export async function buildAndMaybePush( args: BuildArgs, pathToDocker: string, push: boolean, - configPath: string | undefined, containerConfig?: ContainerApp ): Promise<{ image: string; pushed: boolean }> { try { @@ -94,7 +93,6 @@ export async function buildAndMaybePush( platform: args.platform, setNetworkToHost: Boolean(getCIOverrideNetworkModeHost()), }, - configPath, logger ); @@ -166,7 +164,7 @@ export async function buildAndMaybePush( // account ID before checking if it exists in // the managed registry. const [image, hash] = digest.split("@"); - const resolvedImage = await resolveImageName( + const resolvedImage = resolveImageName( account.external_account_id, image ); @@ -239,7 +237,6 @@ export async function buildCommand( }, getDockerPath() ?? args.pathToDocker, args.push, - config.configPath, container ); } diff --git a/packages/wrangler/src/cloudchamber/common.ts b/packages/wrangler/src/cloudchamber/common.ts index 450a48c83d5a..81f9fcee2362 100644 --- a/packages/wrangler/src/cloudchamber/common.ts +++ b/packages/wrangler/src/cloudchamber/common.ts @@ -668,7 +668,7 @@ export function checkInstanceType( } // infers the instance type from a given configuration -function inferInstanceType( +export function inferInstanceType( configuration: UserDeploymentConfiguration ): InstanceType | undefined { if ( diff --git a/packages/wrangler/src/cloudchamber/deploy.ts b/packages/wrangler/src/cloudchamber/deploy.ts index c283449df7d1..857d169c6dca 100644 --- a/packages/wrangler/src/cloudchamber/deploy.ts +++ b/packages/wrangler/src/cloudchamber/deploy.ts @@ -1,6 +1,5 @@ -import { isDockerfile } from "@cloudflare/containers-shared"; +import assert from "assert"; import { type Config } from "../config"; -import { type ContainerApp } from "../config/environment"; import { containersScope } from "../containers"; import { apply } from "../containers/deploy"; import { getDockerPath } from "../environment-variables/misc-variables"; @@ -9,50 +8,44 @@ import { logger } from "../logger"; import { fetchVersion } from "../versions/api"; import { buildAndMaybePush } from "./build"; import { fillOpenAPIConfiguration } from "./common"; -import type { BuildArgs } from "@cloudflare/containers-shared/src/types"; +import type { ContainerNormalisedConfig } from "@cloudflare/containers-shared/src/types"; export async function maybeBuildContainer( - containerConfig: ContainerApp, + containerConfig: ContainerNormalisedConfig, /** just the tag component. will be prefixed with the container name */ imageTag: string, dryRun: boolean, - pathToDocker: string, - configPath?: string -): Promise<{ image: string; imageUpdated: boolean }> { - try { - if ( - !isDockerfile( - containerConfig.image ?? containerConfig.configuration?.image, - configPath - ) - ) { - return { - image: containerConfig.image ?? containerConfig.configuration?.image, - // We don't know at this point whether the image was updated or not - // but we need to make sure downstream checks if it was updated so - // we set this to true. - imageUpdated: true, - }; - } - } catch (err) { - if (err instanceof Error) { - throw new UserError(err.message); - } - - throw err; + pathToDocker: string +): Promise<{ newImageLink: string | undefined }> { + if ("registry_link" in containerConfig) { + return { + // We don't know at this point whether the image has changed + // but we need to make sure API checks so + // we always set this to the registry link. + newImageLink: containerConfig.registry_link, + }; } + const imageFullName = containerConfig.name + ":" + imageTag.split("-")[0]; + logger.log("Building image", imageFullName); - const options = getBuildArguments(containerConfig, imageTag); - logger.log("Building image", options.tag); const buildResult = await buildAndMaybePush( - options, + { + tag: imageFullName, + pathToDockerfile: containerConfig.dockerfile, + buildContext: containerConfig.image_build_context, + args: containerConfig.image_vars, + }, pathToDocker, !dryRun, - configPath, - containerConfig + "disk_size" in containerConfig ? containerConfig.disk_size : undefined ); - return { image: buildResult.image, imageUpdated: buildResult.pushed }; + if (buildResult.pushed) { + return { newImageLink: buildResult.image }; + } + // if the image has not changed, it will not have been pushed + // so we don't need to update anything when we apply the container config + return { newImageLink: undefined }; } export type DeployContainersArgs = { @@ -65,27 +58,25 @@ export type DeployContainersArgs = { export async function deployContainers( config: Config, - { versionId, accountId, scriptName, dryRun }: DeployContainersArgs + normalisedContainerConfig: ContainerNormalisedConfig[], + { versionId, accountId, scriptName }: DeployContainersArgs ) { - if (config.containers === undefined) { - return; - } + await fillOpenAPIConfiguration(config, containersScope); - if (!dryRun) { - await fillOpenAPIConfiguration(config, containersScope); - } const pathToDocker = getDockerPath(); - for (const container of config.containers) { - const version = await fetchVersion( - config, - accountId, - scriptName, - versionId + const version = await fetchVersion(config, accountId, scriptName, versionId); + for (const container of normalisedContainerConfig) { + const buildResult = await maybeBuildContainer( + container, + versionId, + false, + pathToDocker ); const targetDurableObject = version.resources.bindings.find( (durableObject) => durableObject.type === "durable_object_namespace" && durableObject.class_name === container.class_name && + // DO cannot be defined in a different script to the container durableObject.script_name === undefined && durableObject.namespace_id !== undefined ); @@ -96,61 +87,18 @@ export async function deployContainers( ); } - if ( - targetDurableObject.type !== "durable_object_namespace" || - targetDurableObject.namespace_id === undefined - ) { - throw new Error("unreachable"); - } - - const configuration = { - ...config, - containers: [ - { - ...container, - durable_objects: { - namespace_id: targetDurableObject.namespace_id, - }, - }, - ], - }; - - const buildResult = await maybeBuildContainer( - container, - versionId, - dryRun, - pathToDocker, - config.configPath + assert( + targetDurableObject.type === "durable_object_namespace" && + targetDurableObject.namespace_id !== undefined ); - container.configuration ??= {}; - container.configuration.image = buildResult.image; - container.image = buildResult.image; await apply( { - skipDefaults: false, - imageUpdateRequired: buildResult.imageUpdated, + newImageLink: buildResult.newImageLink, + durable_object_namespace_id: targetDurableObject.namespace_id, }, - configuration + container, + config ); } } - -// TODO: container app config should be normalized by now in config validation -// getBuildArguments takes the image from `container.image` or `container.configuration.image` -// if the first is not defined. It accepts either a URI or path to a Dockerfile. -// It will return options that are usable with the build() method from containers. -export function getBuildArguments( - container: ContainerApp, - idForImageTag: string -): BuildArgs { - const pathToDockerfile = container.image ?? container.configuration?.image; - const imageTag = container.name + ":" + idForImageTag.split("-")[0]; - - return { - tag: imageTag, - pathToDockerfile, - buildContext: container.image_build_context, - args: container.image_vars, - }; -} diff --git a/packages/wrangler/src/containers/deploy.ts b/packages/wrangler/src/containers/deploy.ts index e79b89a49e49..4f0fd0b5e341 100644 --- a/packages/wrangler/src/containers/deploy.ts +++ b/packages/wrangler/src/containers/deploy.ts @@ -2,6 +2,7 @@ * Note! Much of this is copied and modified from cloudchamber/apply.ts * However this code is only used for containers interactions, not cloudchamber ones! */ +import assert from "assert"; import { endSection, log, @@ -16,12 +17,10 @@ import { ApplicationsService, CreateApplicationRolloutRequest, DeploymentMutationError, - InstanceType, resolveImageName, RolloutsService, - SchedulingPolicy, } from "@cloudflare/containers-shared"; -import { cleanForInstanceType, promiseSpinner } from "../cloudchamber/common"; +import { inferInstanceType, promiseSpinner } from "../cloudchamber/common"; import { diffLines, printLine, @@ -29,20 +28,18 @@ import { sortObjectRecursive, stripUndefined, } from "../cloudchamber/helpers/diff"; -import { formatConfigSnippet } from "../config"; import { FatalError, UserError } from "../errors"; import { getAccountId } from "../user"; import type { Config } from "../config"; -import type { ContainerApp, Observability } from "../config/environment"; +import type { Observability } from "../config/environment"; import type { Application, ApplicationID, ApplicationName, + ContainerNormalisedConfig, CreateApplicationRequest, ModifyApplicationRequestBody, - ModifyDeploymentV2RequestBody, Observability as ObservabilityConfiguration, - UserDeploymentConfiguration, } from "@cloudflare/containers-shared"; function mergeDeep(target: T, source: Partial): T { @@ -87,6 +84,12 @@ function createApplicationToModifyApplication( }; } +/** + * + * // QUESTION: why create applicaiton request? this is only used when modifying + * Takes the output of listApplications (Application) and converts it to a CreateApplicationRequest. + * Used to diff the previous deployment with the current one. + */ function applicationToCreateApplication( accountId: string, application: Application @@ -172,66 +175,47 @@ function observabilityToConfiguration( } } -function containerAppToInstanceType( - containerApp: ContainerApp -): InstanceType | undefined { - if (containerApp.instance_type !== undefined) { - return containerApp.instance_type as InstanceType; - } - - // if no other configuration is set, we fall back to the default "dev" instance type - const configuration = - containerApp.configuration as UserDeploymentConfiguration; - if ( - configuration.disk === undefined && - configuration.vcpu === undefined && - configuration.memory === undefined && - configuration.memory_mib === undefined - ) { - return InstanceType.DEV; - } -} - -function containerAppToCreateApplication( +/** + * + * Turns the normalised container config from wrangler config into + * a CreateApplicationRequest that can be sent to the API. + * If we want to modify instead, the ModifyRequestBody is a subset of this + * + */ +function containerConfigToAPIConfig( accountId: string, - containerApp: ContainerApp, + containerApp: ContainerNormalisedConfig, observability: Observability | undefined, - existingApp: Application | undefined, - skipDefaults = false + imageRef: string, + durableObjectNamespaceId: string, + prevApp?: Application ): CreateApplicationRequest { - const observabilityConfiguration = observabilityToConfiguration( - observability, - existingApp?.configuration.observability - ); - const instanceType = containerAppToInstanceType(containerApp); - const configuration: UserDeploymentConfiguration = { - ...(containerApp.configuration as UserDeploymentConfiguration), - observability: observabilityConfiguration, - instance_type: instanceType, - }; - - // this should have been set to a default value of worker-name-class-name if unspecified by the user - if (containerApp.name === undefined) { - throw new FatalError("Container application name failed to be set", 1, { - telemetryMessage: true, - }); - } - const app: CreateApplicationRequest = { - ...containerApp, name: containerApp.name, + scheduling_policy: containerApp.scheduling_policy, + configuration: { - ...configuration, // De-sugar image name - image: resolveImageName(accountId, configuration.image), + image: resolveImageName(accountId, imageRef), + // if disk/memory/vcpu is not defined in config, AND instance_type is also not defined, this will already have been defaulted to 'dev' + ...("instance_type" in containerApp + ? { instance_type: containerApp.instance_type } + : { + disk: { size_mb: containerApp.disk_size }, + memory_mib: containerApp.memory_mib, + vcpu: containerApp.vcpu, + }), + observability: observabilityToConfiguration( + observability, + prevApp?.configuration.observability + ), }, - instances: containerApp.instances ?? 0, - scheduling_policy: - (containerApp.scheduling_policy as SchedulingPolicy) ?? - SchedulingPolicy.DEFAULT, + // TODO: do i need to carry over instances too? same re: durable_objects + // instances: containerApp.instances ?? 0, + instances: 0, + max_instances: containerApp.max_instances, constraints: { - ...(containerApp.constraints ?? - (!skipDefaults ? { tier: 1 } : undefined)), + ...(containerApp.constraints ?? { tier: 1 }), cities: containerApp.constraints?.cities?.map((city) => city.toLowerCase() ), @@ -239,25 +223,25 @@ function containerAppToCreateApplication( region.toUpperCase() ), }, + durable_objects: { + namespace_id: durableObjectNamespaceId, + }, }; - // delete the fields that should not be sent to API - delete (app as Record)["class_name"]; - delete (app as Record)["image"]; - delete (app as Record)["image_build_context"]; - delete (app as Record)["image_vars"]; - delete (app as Record)["rollout_step_percentage"]; - delete (app as Record)["rollout_kind"]; - delete (app as Record)["instance_type"]; - return app; } export async function apply( args: { - skipDefaults: boolean | undefined; imageUpdateRequired?: boolean; + /** + * If the image was built and pushed, or is a registry link, we have to update the image ref and this will be defined + * If it is undefined, the image has not change, and we do not need to update the image ref + */ + newImageLink: string | undefined; + durable_object_namespace_id: string; }, + containerConfig: ContainerNormalisedConfig, config: Config ) { if (!config.containers || config.containers.length === 0) { @@ -268,257 +252,241 @@ export async function apply( "deploy changes to your application" ); - const applications = await promiseSpinner( + const existingApplications = await promiseSpinner( ApplicationsService.listApplications(), { message: "Loading applications" } ); - applications.forEach((app) => + existingApplications.forEach((app) => cleanupObservability(app.configuration.observability) ); - const applicationByNames: Record = {}; // TODO: this is not correct right now as there can be multiple applications // with the same name. - for (const application of applications) { - applicationByNames[application.name] = application; - } - - const actions: ( - | { action: "create"; application: CreateApplicationRequest } - | { - action: "modify"; - application: ModifyApplicationRequestBody; - id: ApplicationID; - name: ApplicationName; - rollout_step_percentage?: number; - rollout_kind: CreateApplicationRolloutRequest.kind; - } - )[] = []; + /** Previous deployment of this app, if this exists */ + const prevApp = existingApplications.find( + (app) => app.name === containerConfig.name + ); // TODO: JSON formatting is a bit bad due to the trimming. // Try to do a conditional on `configFormat` log(dim("Container application changes\n")); - for (const appConfigNoDefaults of config.containers) { - const application = - applicationByNames[ - appConfigNoDefaults.name ?? - // we should never actually reach this point, but just in case - `${config.name}-${appConfigNoDefaults.class_name}` - ]; - - // while configuration.image is deprecated to the user, we still resolve to this for now. - if (!appConfigNoDefaults.configuration?.image && application) { - appConfigNoDefaults.configuration ??= {}; - } + const accountId = config.account_id || (await getAccountId(config)); + const imageRef = args.newImageLink ?? prevApp?.configuration.image; + assert(imageRef, "No changes detected but no previous image found"); + const appConfig = containerConfigToAPIConfig( + accountId, + containerConfig, + config.observability, + imageRef, + args.durable_object_namespace_id, + prevApp + ); - if (!args.imageUpdateRequired && application) { - appConfigNoDefaults.configuration ??= {}; - appConfigNoDefaults.configuration.image = application.configuration.image; + if (prevApp !== undefined && prevApp !== null) { + if (!prevApp.durable_objects?.namespace_id) { + throw new FatalError( + "The previous deploy of this container application was not associated with a durable object" + ); } - - const accountId = config.account_id || (await getAccountId(config)); - const appConfig = containerAppToCreateApplication( - accountId, - appConfigNoDefaults, - config.observability, - application, - args.skipDefaults - ); - - if (application !== undefined && application !== null) { - // we need to sort the objects (by key) because the diff algorithm works with - // lines - const prevApp = sortObjectRecursive( - stripUndefined(applicationToCreateApplication(accountId, application)) + if ( + prevApp.durable_objects.namespace_id !== args.durable_object_namespace_id + ) { + throw new UserError( + `Application "${prevApp.name}" is assigned to durable object ${prevApp.durable_objects.namespace_id}, but a new DO namespace is being assigned to the application, + you should delete the container application and deploy again` ); + } + // we need to sort the objects (by key) because the diff algorithm works with lines + const normalisedPrevApp = sortObjectRecursive( + stripUndefined(applicationToCreateApplication(accountId, prevApp)) + ); - // fill up fields that their defaults were changed over-time, - // maintaining retrocompatibility with the existing app - if (appConfigNoDefaults.scheduling_policy === undefined) { - appConfig.scheduling_policy = prevApp.scheduling_policy; - } + const prevContainer = cleanPrevious(normalisedPrevApp, containerConfig); - if ( - prevApp.durable_objects !== undefined && - appConfigNoDefaults.durable_objects !== undefined && - prevApp.durable_objects.namespace_id !== - appConfigNoDefaults.durable_objects.namespace_id - ) { - throw new UserError( - `Application "${prevApp.name}" is assigned to durable object ${prevApp.durable_objects.namespace_id}, but a new DO namespace is being assigned to the application, - you should delete the container application and deploy again` - ); - } + const nowContainer = mergeDeep( + prevContainer, + sortObjectRecursive(appConfig) + ); - const prevContainer = - appConfig.configuration.instance_type !== undefined - ? cleanForInstanceType(prevApp) - : (prevApp as ContainerApp); - const nowContainer = mergeDeep( - prevContainer as CreateApplicationRequest, - sortObjectRecursive(appConfig) - ) as ContainerApp; - - const prev = formatConfigSnippet( - { containers: [prevContainer] }, - config.configPath - ); + const prev = JSON.stringify({ containers: [normalisedPrevApp] }, null, 2); + const now = JSON.stringify({ containers: [nowContainer] }, null, 2); - const now = formatConfigSnippet( - { containers: [nowContainer] }, - config.configPath - ); - const results = diffLines(prev, now); - const changes = results.find((l) => l.added || l.removed) !== undefined; - if (!changes) { - updateStatus(`no changes ${brandColor(application.name)}`); - continue; - } + const results = diffLines(prev, now); + const changes = results.find((l) => l.added || l.removed) !== undefined; - updateStatus( - `${brandColor.underline("EDIT")} ${application.name}`, - false - ); - renderDiff(results); - if (appConfigNoDefaults.rollout_kind !== "none") { - actions.push({ - action: "modify", - application: createApplicationToModifyApplication(appConfig), - id: application.id, - name: application.name, - rollout_step_percentage: - application.durable_objects !== undefined - ? appConfigNoDefaults.rollout_step_percentage ?? 25 - : appConfigNoDefaults.rollout_step_percentage, - rollout_kind: - appConfigNoDefaults.rollout_kind == "full_manual" - ? CreateApplicationRolloutRequest.kind.FULL_MANUAL - : CreateApplicationRolloutRequest.kind.FULL_AUTO, - }); - } else { - log("Skipping application rollout"); - } + if (!changes) { + updateStatus(`no changes ${brandColor(prevApp.name)}`); + endSection("No changes to be made"); + return; + } - printLine(""); - continue; + updateStatus(`${brandColor.underline("EDIT")} ${prevApp.name}`, false); + + renderDiff(results); + + if (containerConfig.rollout_kind !== "none") { + await doAction({ + action: "modify", + application: createApplicationToModifyApplication(appConfig), + id: prevApp.id, + name: prevApp.name, + rollout_step_percentage: containerConfig.rollout_step_percentage, + rollout_kind: + containerConfig.rollout_kind == "full_manual" + ? CreateApplicationRolloutRequest.kind.FULL_MANUAL + : CreateApplicationRolloutRequest.kind.FULL_AUTO, + }); + } else { + log("Skipping application rollout"); } + } else { + // ************** + // *** CREATE *** + // ************** // print the header of the app updateStatus(bold.underline(green.underline("NEW")) + ` ${appConfig.name}`); - const s = formatConfigSnippet( - { - containers: [ - { - ...appConfig, - instances: - appConfig.max_instances !== undefined - ? // trick until we allow setting instances to undefined in the API - undefined - : appConfig.instances, - } as ContainerApp, - ], - }, - config.configPath - ); + const configStr = JSON.stringify({ containers: [appConfig] }, null, 2); // go line by line and pretty print it - s.split("\n") + configStr + .split("\n") .map((line) => line.trim()) .forEach((el) => { printLine(el, " "); }); - const configToPush = { ...appConfig }; - // add to the actions array to create the app later - actions.push({ + await doAction({ action: "create", - application: configToPush, + application: appConfig, }); } + printLine(""); + endSection("Applied changes"); +} - if (actions.length == 0) { - endSection("No changes to be made"); - return; +function formatError(err: ApiError): string { + // TODO: this is bad bad. Please fix like we do in create.ts. + // On Cloudchamber API side, we have to improve as well the object validation errors, + // so we can detect them here better and pinpoint to the user what's going on. + if ( + err.body.error === DeploymentMutationError.VALIDATE_INPUT && + err.body.details !== undefined + ) { + let message = ""; + for (const key in err.body.details) { + message += ` ${brandColor(key)} ${err.body.details[key]}\n`; + } + + return message; } - function formatError(err: ApiError): string { - // TODO: this is bad bad. Please fix like we do in create.ts. - // On Cloudchamber API side, we have to improve as well the object validation errors, - // so we can detect them here better and pinpoint to the user what's going on. - if ( - err.body.error === DeploymentMutationError.VALIDATE_INPUT && - err.body.details !== undefined - ) { - let message = ""; - for (const key in err.body.details) { - message += ` ${brandColor(key)} ${err.body.details[key]}\n`; + if (err.body.error !== undefined) { + return ` ${err.body.error}`; + } + + return JSON.stringify(err.body); +} + +const doAction = async ( + action: + | { action: "create"; application: CreateApplicationRequest } + | { + action: "modify"; + application: ModifyApplicationRequestBody; + id: ApplicationID; + name: ApplicationName; + rollout_step_percentage?: number; + rollout_kind: CreateApplicationRolloutRequest.kind; + } +) => { + if (action.action === "create") { + let application: Application; + try { + application = await promiseSpinner( + ApplicationsService.createApplication(action.application), + { message: `Creating ${action.application.name}` } + ); + } catch (err) { + if (!(err instanceof Error)) { + throw err; } - return message; - } + if (!(err instanceof ApiError)) { + throw new UserError( + `Unexpected error creating application: ${err.message}` + ); + } - if (err.body.error !== undefined) { - return ` ${err.body.error}`; + if (err.status === 400) { + throw new UserError( + `Error creating application due to a misconfiguration\n${formatError(err)}` + ); + } + + throw new UserError( + `Error creating application due to an internal error (request id: ${err.body.request_id}):\n${formatError(err)}` + ); } - return JSON.stringify(err.body); + success( + `Created application ${brandColor(action.application.name)} (Application ID: ${application.id})`, + { + shape: shapes.bar, + } + ); } - for (const action of actions) { - if (action.action === "create") { - let application: Application; - try { - application = await promiseSpinner( - ApplicationsService.createApplication(action.application), - { message: `Creating ${action.application.name}` } - ); - } catch (err) { - if (!(err instanceof Error)) { - throw err; - } - - if (!(err instanceof ApiError)) { - throw new UserError( - `Unexpected error creating application: ${err.message}` - ); - } + if (action.action === "modify") { + try { + await promiseSpinner( + ApplicationsService.modifyApplication(action.id, { + ...action.application, + instances: + action.application.max_instances !== undefined + ? undefined + : action.application.instances, + }), + { message: `Modifying ${action.application.name}` } + ); + } catch (err) { + if (!(err instanceof Error)) { + throw err; + } - if (err.status === 400) { - throw new UserError( - `Error creating application due to a misconfiguration\n${formatError(err)}` - ); - } + if (!(err instanceof ApiError)) { + throw new UserError( + `Unexpected error modifying application ${action.name}: ${err.message}` + ); + } + if (err.status === 400) { throw new UserError( - `Error creating application due to an internal error (request id: ${err.body.request_id}):\n${formatError(err)}` + `Error modifying application ${action.name} due to a misconfiguration:\n\n\t${formatError(err)}` ); } - success( - `Created application ${brandColor(action.application.name)} (Application ID: ${application.id})`, - { - shape: shapes.bar, - } + throw new UserError( + `Error modifying application ${action.name} due to an internal error (request id: ${err.body.request_id}):\n${formatError(err)}` ); - - printLine(""); - continue; } - if (action.action === "modify") { + if (action.rollout_step_percentage !== undefined) { try { await promiseSpinner( - ApplicationsService.modifyApplication(action.id, { - ...action.application, - instances: - action.application.max_instances !== undefined - ? undefined - : action.application.instances, + RolloutsService.createApplicationRollout(action.id, { + description: "Progressive update", + strategy: CreateApplicationRolloutRequest.strategy.ROLLING, + target_configuration: action.application.configuration ?? {}, + step_percentage: action.rollout_step_percentage, + kind: action.rollout_kind, }), - { message: `Modifying ${action.application.name}` } + { + message: `rolling out container version ${action.name}`, + } ); } catch (err) { if (!(err instanceof Error)) { @@ -527,68 +495,49 @@ export async function apply( if (!(err instanceof ApiError)) { throw new UserError( - `Unexpected error modifying application ${action.name}: ${err.message}` + `Unexpected error rolling out application ${action.name}:\n${err.message}` ); } if (err.status === 400) { throw new UserError( - `Error modifying application ${action.name} due to a misconfiguration:\n\n\t${formatError(err)}` + `Error rolling out application ${action.name} due to a misconfiguration:\n\n\t${formatError(err)}` ); } throw new UserError( - `Error modifying application ${action.name} due to an internal error (request id: ${err.body.request_id}):\n${formatError(err)}` + `Error rolling out application ${action.name} due to an internal error (request id: ${err.body.request_id}): ${formatError(err)}` ); } + } - if (action.rollout_step_percentage !== undefined) { - try { - await promiseSpinner( - RolloutsService.createApplicationRollout(action.id, { - description: "Progressive update", - strategy: CreateApplicationRolloutRequest.strategy.ROLLING, - target_configuration: - (action.application - .configuration as ModifyDeploymentV2RequestBody) ?? {}, - step_percentage: action.rollout_step_percentage, - kind: action.rollout_kind, - }), - { - message: `rolling out container version ${action.name}`, - } - ); - } catch (err) { - if (!(err instanceof Error)) { - throw err; - } - - if (!(err instanceof ApiError)) { - throw new UserError( - `Unexpected error rolling out application ${action.name}:\n${err.message}` - ); - } - - if (err.status === 400) { - throw new UserError( - `Error rolling out application ${action.name} due to a misconfiguration:\n\n\t${formatError(err)}` - ); - } - - throw new UserError( - `Error rolling out application ${action.name} due to an internal error (request id: ${err.body.request_id}): ${formatError(err)}` - ); - } - } - - success(`Modified application ${brandColor(action.name)}`, { - shape: shapes.bar, - }); + success(`Modified application ${brandColor(action.name)}`, { + shape: shapes.bar, + }); + } +}; - printLine(""); - continue; +/** + * clean up so we get a nicer diff + */ +export function cleanPrevious( + prev: CreateApplicationRequest, + currentConfig: ContainerNormalisedConfig +): CreateApplicationRequest { + if ("instance_type" in currentConfig) { + // returns undefined if we can't infer it. + const instance_type = inferInstanceType(prev.configuration); + if (!instance_type) { + // just leave as is if we can't infer the instance type + return prev; } + prev.configuration.instance_type = instance_type; + + delete prev.configuration.disk; + delete prev.configuration.memory; + delete prev.configuration.memory_mib; + delete prev.configuration.vcpu; } - endSection("Applied changes"); + return prev; } diff --git a/packages/wrangler/src/deploy/deploy.ts b/packages/wrangler/src/deploy/deploy.ts index ece7859e64f2..84c861271296 100644 --- a/packages/wrangler/src/deploy/deploy.ts +++ b/packages/wrangler/src/deploy/deploy.ts @@ -3,16 +3,14 @@ import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; import path from "node:path"; import { URLSearchParams } from "node:url"; import { cancel } from "@cloudflare/cli"; -import { - isDockerfile, - verifyDockerInstalled, -} from "@cloudflare/containers-shared"; +import { verifyDockerInstalled } from "@cloudflare/containers-shared"; import PQueue from "p-queue"; import { Response } from "undici"; import { syncAssets } from "../assets"; import { fetchListResult, fetchResult } from "../cfetch"; import { deployContainers, maybeBuildContainer } from "../cloudchamber/deploy"; import { configFileName, formatConfigSnippet } from "../config"; +import { getNormalizedContainerOptions } from "../containers/config"; import { getBindings, provisionBindings } from "../deployment-bundle/bindings"; import { bundleWorker } from "../deployment-bundle/bundle"; import { printBundleSize } from "../deployment-bundle/bundle-reporter"; @@ -434,6 +432,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m noBundle: props.noBundle ?? config.no_bundle, } ); + const normalisedContainerConfig = await getNormalizedContainerOptions(config); // Warn if user tries minify with no-bundle if (props.noBundle && minify) { @@ -774,13 +773,10 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m // and we have containers so that we don't get into a // disjointed state where the worker updates but the container // fails. - if (config.containers) { + if (normalisedContainerConfig.length) { // if you have a registry url specified, you don't need docker - const hasDockerfiles = config.containers.some((container) => - isDockerfile( - container.image ?? container.configuration?.image, - config.configPath - ) + const hasDockerfiles = normalisedContainerConfig.some( + (container) => "dockerfile" in container ); if (hasDockerfiles) { await verifyDockerInstalled(dockerPath, false); @@ -788,14 +784,13 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m } if (props.dryRun) { - if (config.containers) { - for (const container of config.containers) { + if (normalisedContainerConfig.length) { + for (const container of normalisedContainerConfig) { await maybeBuildContainer( container, workerTag ?? "worker-tag", props.dryRun, - dockerPath, - config.configPath + dockerPath ); } } @@ -1034,9 +1029,9 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m return { versionId, workerTag }; } - if (config.containers) { + if (normalisedContainerConfig.length) { assert(versionId && accountId); - await deployContainers(config, { + await deployContainers(config, normalisedContainerConfig, { versionId, accountId, scriptName, From 1a9b24c8a08758070c4e31074aa073c28087ec8a Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Mon, 14 Jul 2025 21:13:06 +0100 Subject: [PATCH 05/11] consolidate tests into containers/deploy.test.ts --- .../src/__tests__/cloudchamber/deploy.test.ts | 808 ------- .../src/__tests__/containers/apply.test.ts | 1959 ----------------- .../src/__tests__/containers/deploy.test.ts | 1282 +++++++++++ .../src/__tests__/helpers/normalize.ts | 11 +- 4 files changed, 1292 insertions(+), 2768 deletions(-) delete mode 100644 packages/wrangler/src/__tests__/cloudchamber/deploy.test.ts delete mode 100644 packages/wrangler/src/__tests__/containers/apply.test.ts create mode 100644 packages/wrangler/src/__tests__/containers/deploy.test.ts diff --git a/packages/wrangler/src/__tests__/cloudchamber/deploy.test.ts b/packages/wrangler/src/__tests__/cloudchamber/deploy.test.ts deleted file mode 100644 index 68f24f1f2542..000000000000 --- a/packages/wrangler/src/__tests__/cloudchamber/deploy.test.ts +++ /dev/null @@ -1,808 +0,0 @@ -import { spawn } from "node:child_process"; -import * as fs from "node:fs"; -import { PassThrough, Writable } from "node:stream"; -import { - getCloudflareContainerRegistry, - SchedulingPolicy, -} from "@cloudflare/containers-shared"; -import { http, HttpResponse } from "msw"; -import { maybeBuildContainer } from "../../cloudchamber/deploy"; -import { clearCachedAccount } from "../../cloudchamber/locations"; -import { mockAccountId, mockApiToken } from "../helpers/mock-account-id"; -import { mockConsoleMethods } from "../helpers/mock-console"; -import { mockLegacyScriptData } from "../helpers/mock-legacy-script"; -import { mockUploadWorkerRequest } from "../helpers/mock-upload-worker"; -import { mockSubDomainRequest } from "../helpers/mock-workers-subdomain"; -import { - createFetchResult, - msw, - mswSuccessDeploymentScriptMetadata, -} from "../helpers/msw"; -import { mswListNewDeploymentsLatestFull } from "../helpers/msw/handlers/versions"; -import { runInTempDir } from "../helpers/run-in-tmp"; -import { runWrangler } from "../helpers/run-wrangler"; -import { writeWranglerConfig } from "../helpers/write-wrangler-config"; -import { mockAccountV4 as mockContainersAccount } from "./utils"; -import type { - AccountRegistryToken, - Application, - ImageRegistryCredentialsConfiguration, -} from "@cloudflare/containers-shared"; -import type { ChildProcess } from "node:child_process"; - -vi.mock("node:child_process"); -describe("maybeBuildContainer", () => { - it("Should return imageUpdate: true if using an image URI", async () => { - const config = { - image: "registry.cloudflare.com/some-image:uri", - class_name: "Test", - }; - const result = await maybeBuildContainer( - config, - "some-tag:thing", - false, - "/usr/bin/docker", - undefined - ); - expect(result.image).toEqual(config.image); - expect(result.imageUpdated).toEqual(true); - }); -}); -describe("wrangler deploy with containers", () => { - runInTempDir(); - const std = mockConsoleMethods(); - mockAccountId(); - mockApiToken(); - beforeEach(() => { - msw.use(...mswSuccessDeploymentScriptMetadata); - msw.use(...mswListNewDeploymentsLatestFull); - fs.writeFileSync( - "index.js", - `export class ExampleDurableObject {}; export default{};` - ); - vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/docker"); - - mockSubDomainRequest(); - mockLegacyScriptData({ - scripts: [{ id: "test-name", migration_tag: "v1" }], - }); - mockContainersAccount(); - mockUploadWorkerRequest({ - expectedBindings: [ - { - class_name: "ExampleDurableObject", - name: "EXAMPLE_DO_BINDING", - type: "durable_object_namespace", - }, - ], - useOldUploadApi: true, - expectedContainers: [{ class_name: "ExampleDurableObject" }], - }); - }); - afterEach(() => { - vi.unstubAllEnvs(); - }); - it("should fail early if no docker is detected when deploying a container from a dockerfile", async () => { - vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/bad-docker-path"); - writeWranglerConfig({ - durable_objects: { - bindings: [ - { - name: "EXAMPLE_DO_BINDING", - class_name: "ExampleDurableObject", - }, - ], - }, - containers: [ - { - name: "my-container", - max_instances: 10, - class_name: "ExampleDurableObject", - image: "./Dockerfile", - }, - ], - migrations: [{ tag: "v1", new_sqlite_classes: ["ExampleDurableObject"] }], - }); - - fs.writeFileSync("./Dockerfile", "FROM scratch"); - - await expect(runWrangler("deploy index.js")).rejects - .toThrowErrorMatchingInlineSnapshot(` - [Error: The Docker CLI could not be launched. Please ensure that the Docker CLI is installed and the daemon is running. - Other container tooling that is compatible with the Docker CLI and engine may work, but is not yet guaranteed to do so. You can specify an executable with the environment variable WRANGLER_DOCKER_BIN and a socket with WRANGLER_DOCKER_HOST.] - `); - }); - it("should support durable object bindings to SQLite classes with containers (dockerfile flow)", async () => { - mockGetVersion("Galaxy-Class"); - - vi.mocked(spawn) - .mockImplementationOnce(mockDockerInfo()) - .mockImplementationOnce( - mockDockerBuild("my-container", "Galaxy", "FROM scratch", process.cwd()) - ) - .mockImplementationOnce(mockDockerImageInspect("my-container", "Galaxy")) - .mockImplementationOnce(mockDockerLogin("mockpassword")) - .mockImplementationOnce( - mockDockerManifestInspect("some-account-id/my-container", true) - ) - .mockImplementationOnce( - mockDockerTag("my-container", "some-account-id/my-container", "Galaxy") - ) - .mockImplementationOnce( - mockDockerPush("some-account-id/my-container", "Galaxy") - ) - .mockImplementationOnce( - mockDockerImageDelete("some-account-id/my-container", "Galaxy") - ); - - writeWranglerConfig({ - durable_objects: { - bindings: [ - { - name: "EXAMPLE_DO_BINDING", - class_name: "ExampleDurableObject", - }, - ], - }, - containers: [ - { - name: "my-container", - max_instances: 10, - class_name: "ExampleDurableObject", - image: "./Dockerfile", - }, - ], - migrations: [{ tag: "v1", new_sqlite_classes: ["ExampleDurableObject"] }], - }); - - mockGetApplications([]); - fs.writeFileSync("./Dockerfile", "FROM scratch"); - mockGenerateImageRegistryCredentials(); - - mockCreateApplication({ - name: "my-container", - max_instances: 10, - durable_objects: { namespace_id: "1" }, - configuration: { - image: - getCloudflareContainerRegistry() + - "/some-account-id/my-container:Galaxy", - }, - }); - - await runWrangler("deploy index.js"); - - expect(std.out).toMatchInlineSnapshot(` - "Total Upload: xx KiB / gzip: xx KiB - Worker Startup Time: 100 ms - Your Worker has access to the following bindings: - Binding Resource - env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object - - Uploaded test-name (TIMINGS) - Building image my-container:Galaxy - Image does not exist remotely, pushing: registry.cloudflare.com/some-account-id/my-container:Galaxy - Deployed test-name triggers (TIMINGS) - https://test-name.test-sub-domain.workers.dev - Current Version ID: Galaxy-Class" - `); - expect(std.err).toMatchInlineSnapshot(`""`); - expect(std.warn).toMatchInlineSnapshot(`""`); - }); - it("should support durable object bindings to SQLite classes with containers (image uri flow)", async () => { - // note no docker commands have been mocked here! - mockGetVersion("Galaxy-Class"); - - writeWranglerConfig({ - durable_objects: { - bindings: [ - { - name: "EXAMPLE_DO_BINDING", - class_name: "ExampleDurableObject", - }, - ], - }, - containers: [ - { - image: "docker.io/hello:world", - name: "my-container", - max_instances: 10, - class_name: "ExampleDurableObject", - }, - ], - migrations: [{ tag: "v1", new_sqlite_classes: ["ExampleDurableObject"] }], - }); - - mockGetApplications([]); - - mockCreateApplication({ - name: "my-container", - max_instances: 10, - durable_objects: { namespace_id: "1" }, - scheduling_policy: SchedulingPolicy.DEFAULT, - }); - - await runWrangler("deploy index.js"); - - expect(std.out).toMatchInlineSnapshot(` - "Total Upload: xx KiB / gzip: xx KiB - Worker Startup Time: 100 ms - Your Worker has access to the following bindings: - Binding Resource - env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object - - Uploaded test-name (TIMINGS) - Deployed test-name triggers (TIMINGS) - https://test-name.test-sub-domain.workers.dev - Current Version ID: Galaxy-Class" - `); - expect(std.err).toMatchInlineSnapshot(`""`); - expect(std.warn).toMatchInlineSnapshot(`""`); - }); - - it("resolve the docker build context path based on the dockerfile location, if image_build_context is not provided", async () => { - vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/docker"); - mockGetVersion("Galaxy-Class"); - - vi.mocked(spawn) - .mockImplementationOnce(mockDockerInfo()) - .mockImplementationOnce( - mockDockerBuild( - "my-container", - "Galaxy", - "FROM scratch", - // note that the cwd for the test is not the same as the cwd that the wrangler command is running in - // fortunately we are using an absolute path - process.cwd() - ) - ) - .mockImplementationOnce(mockDockerImageInspect("my-container", "Galaxy")) - .mockImplementationOnce(mockDockerLogin("mockpassword")) - .mockImplementationOnce( - mockDockerManifestInspect("some-account-id/my-container", true) - ) - .mockImplementationOnce( - mockDockerTag("my-container", "some-account-id/my-container", "Galaxy") - ) - .mockImplementationOnce( - mockDockerPush("some-account-id/my-container", "Galaxy") - ) - .mockImplementationOnce( - mockDockerImageDelete("some-account-id/my-container", "Galaxy") - ); - - mockContainersAccount(); - - writeWranglerConfig( - { - main: "./worker/index.js", - durable_objects: { - bindings: [ - { - name: "EXAMPLE_DO_BINDING", - class_name: "ExampleDurableObject", - }, - ], - }, - containers: [ - { - name: "my-container", - max_instances: 10, - class_name: "ExampleDurableObject", - image: "../Dockerfile", - }, - ], - migrations: [ - { tag: "v1", new_sqlite_classes: ["ExampleDurableObject"] }, - ], - }, - "src/wrangler.json" - ); - - fs.writeFileSync("Dockerfile", "FROM scratch"); - fs.mkdirSync("src/worker", { recursive: true }); - fs.writeFileSync( - "src/worker/index.js", - `export class ExampleDurableObject {}; export default{};` - ); - mockGetApplications([]); - - mockGenerateImageRegistryCredentials(); - - mockCreateApplication({ - name: "my-container", - max_instances: 10, - durable_objects: { namespace_id: "1" }, - configuration: { - image: - getCloudflareContainerRegistry() + - "/some-account-id/my-container:Galaxy", - }, - }); - - await runWrangler("deploy --cwd src"); - - expect(std.out).toMatchInlineSnapshot(` - "Total Upload: xx KiB / gzip: xx KiB - Worker Startup Time: 100 ms - Your Worker has access to the following bindings: - Binding Resource - env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object - - Uploaded test-name (TIMINGS) - Building image my-container:Galaxy - Image does not exist remotely, pushing: registry.cloudflare.com/some-account-id/my-container:Galaxy - Deployed test-name triggers (TIMINGS) - https://test-name.test-sub-domain.workers.dev - Current Version ID: Galaxy-Class" - `); - expect(std.err).toMatchInlineSnapshot(`""`); - expect(std.warn).toMatchInlineSnapshot(`""`); - }); - - it("should resolve dockerfile path relative to wrangler config path", async () => { - mockGetVersion("Galaxy-Class"); - vi.mocked(spawn) - .mockImplementationOnce(mockDockerInfo()) - .mockImplementationOnce( - mockDockerBuild( - "my-container", - "Galaxy", - // we pipe the dockerfile in, so a successful test should just show that the dockerfile content has been successfully read and matches what was written (FROM alpine) - "FROM alpine", - // note that the cwd for the test is not the same as the cwd that the wrangler command is running in - // fortunately we are using an absolute path - process.cwd() - ) - ) - .mockImplementationOnce(mockDockerImageInspect("my-container", "Galaxy")) - .mockImplementationOnce(mockDockerLogin("mockpassword")) - .mockImplementationOnce( - mockDockerManifestInspect("some-account-id/my-container", true) - ) - .mockImplementationOnce( - mockDockerTag("my-container", "some-account-id/my-container", "Galaxy") - ) - .mockImplementationOnce( - mockDockerPush("some-account-id/my-container", "Galaxy") - ) - .mockImplementationOnce( - mockDockerImageDelete("some-account-id/my-container", "Galaxy") - ); - - fs.mkdirSync("nested/src", { recursive: true }); - fs.writeFileSync("Dockerfile", "FROM alpine"); - fs.writeFileSync( - "nested/src/index.js", - `export class ExampleDurableObject {}; export default{};` - ); - - writeWranglerConfig( - { - main: "./src/index.js", - durable_objects: { - bindings: [ - { - name: "EXAMPLE_DO_BINDING", - class_name: "ExampleDurableObject", - }, - ], - }, - containers: [ - { - name: "my-container", - max_instances: 10, - class_name: "ExampleDurableObject", - image: "../Dockerfile", - }, - ], - migrations: [ - { tag: "v1", new_sqlite_classes: ["ExampleDurableObject"] }, - ], - }, - "nested/wrangler.json" - ); - - mockGetApplications([]); - mockGenerateImageRegistryCredentials(); - - mockCreateApplication({ - name: "my-container", - max_instances: 10, - durable_objects: { namespace_id: "1" }, - configuration: { - image: - getCloudflareContainerRegistry() + - "/some-account-id/my-container:Galaxy", - }, - }); - - await runWrangler("deploy -c nested/wrangler.json"); - - expect(std.out).toMatchInlineSnapshot(` - "Total Upload: xx KiB / gzip: xx KiB - Worker Startup Time: 100 ms - Your Worker has access to the following bindings: - Binding Resource - env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object - - Uploaded test-name (TIMINGS) - Building image my-container:Galaxy - Image does not exist remotely, pushing: registry.cloudflare.com/some-account-id/my-container:Galaxy - Deployed test-name triggers (TIMINGS) - https://test-name.test-sub-domain.workers.dev - Current Version ID: Galaxy-Class" - `); - - expect(std.err).toMatchInlineSnapshot(`""`); - expect(std.warn).toMatchInlineSnapshot(`""`); - }); - - it("should error when no scope for containers", async () => { - mockContainersAccount([]); - writeWranglerConfig({ - durable_objects: { - bindings: [ - { - name: "EXAMPLE_DO_BINDING", - class_name: "ExampleDurableObject", - }, - ], - }, - containers: [ - { - image: "docker.io/hello:world", - name: "my-container", - max_instances: 10, - class_name: "ExampleDurableObject", - }, - ], - migrations: [{ tag: "v1", new_sqlite_classes: ["ExampleDurableObject"] }], - }); - - await expect( - runWrangler("deploy index.js") - ).rejects.toThrowErrorMatchingInlineSnapshot( - `[Error: You need 'containers:write', try logging in again or creating an appropiate API token]` - ); - }); -}); - -// This is a separate describe block because we intentionally do not mock any -// API tokens, account ID, or authorization. The purpose of these tests is to -// ensure that --dry-run mode works without requiring any API access. -describe("wrangler deploy with containers dry run", () => { - runInTempDir(); - const std = mockConsoleMethods(); - - beforeEach(() => { - clearCachedAccount(); - }); - - afterEach(() => { - vi.unstubAllEnvs(); - }); - - it("builds the image without pushing", async () => { - vi.mocked(spawn) - .mockImplementationOnce(mockDockerInfo()) - .mockImplementationOnce( - mockDockerBuild("my-container", "worker", "FROM scratch", process.cwd()) - ) - .mockImplementationOnce(mockDockerImageInspect("my-container", "worker")) - .mockImplementationOnce(mockDockerLogin("mockpassword")) - .mockImplementationOnce(mockDockerManifestInspect("my-container", true)) - .mockImplementationOnce(mockDockerPush("my-container", "worker")); - - vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/docker"); - - fs.writeFileSync("Dockerfile", "FROM scratch"); - fs.writeFileSync( - "index.js", - `export class ExampleDurableObject {}; export default{};` - ); - - writeWranglerConfig({ - durable_objects: { - bindings: [ - { - name: "EXAMPLE_DO_BINDING", - class_name: "ExampleDurableObject", - }, - ], - }, - containers: [ - { - image: "./Dockerfile", - name: "my-container", - max_instances: 10, - class_name: "ExampleDurableObject", - }, - ], - migrations: [{ tag: "v1", new_sqlite_classes: ["ExampleDurableObject"] }], - }); - - await runWrangler("deploy --dry-run index.js"); - expect(std.out).toMatchInlineSnapshot(` - "Total Upload: xx KiB / gzip: xx KiB - Building image my-container:worker - Your Worker has access to the following bindings: - Binding Resource - env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object - - --dry-run: exiting now." - `); - }); -}); - -function mockGetVersion(versionId: string) { - msw.use( - http.get( - `*/accounts/:accountId/workers/scripts/:scriptName/versions/${versionId}`, - async () => { - return HttpResponse.json( - createFetchResult({ - id: versionId, - metadata: {}, - number: 2, - resources: { - bindings: [ - { - type: "durable_object_namespace", - namespace_id: "1", - class_name: "ExampleDurableObject", - }, - ], - }, - }) - ); - }, - { once: true } - ) - ); -} - -function defaultChildProcess() { - return { - stderr: Buffer.from([]), - stdout: Buffer.from("i promise I am a successful process"), - on: function (reason: string, cbPassed: (code: number) => unknown) { - if (reason === "close") { - cbPassed(0); - } - - return this; - }, - } as unknown as ChildProcess; -} - -function mockCreateApplication(expected?: Partial) { - msw.use( - http.post( - "*/applications", - async ({ request }) => { - const json = await request.json(); - if (expected !== undefined) { - expect(json).toMatchObject(expected); - } - - return HttpResponse.json({ success: true, result: json }); - }, - { once: true } - ) - ); -} - -function mockGenerateImageRegistryCredentials() { - msw.use( - http.post( - `*/registries/${getCloudflareContainerRegistry()}/credentials`, - async ({ request }) => { - const json = - (await request.json()) as ImageRegistryCredentialsConfiguration; - expect(json.permissions).toEqual(["push", "pull"]); - - return HttpResponse.json({ - success: true, - result: { - account_id: "some-account-id", - registry_host: getCloudflareContainerRegistry(), - username: "v1", - password: "mockpassword", - } as AccountRegistryToken, - }); - }, - { once: true } - ) - ); -} -function mockGetApplications(applications: Application[]) { - msw.use( - http.get( - "*/applications", - async () => { - return HttpResponse.json({ success: true, result: applications }); - }, - { once: true } - ) - ); -} - -function mockDockerInfo() { - return (cmd: string, args: readonly string[]) => { - expect(cmd).toBe("/usr/bin/docker"); - expect(args).toEqual(["info"]); - return defaultChildProcess(); - }; -} - -function mockDockerBuild( - containerName: string, - tag: string, - expectedDockerfile: string, - buildContext: string -) { - return (cmd: string, args: readonly string[]) => { - expect(cmd).toBe("/usr/bin/docker"); - expect(args).toEqual([ - "build", - "-t", - `${getCloudflareContainerRegistry()}/${containerName}:${tag}`, - "--platform", - "linux/amd64", - "--provenance=false", - "-f", - "-", - buildContext, - ]); - - let dockerfile = ""; - const readable = new Writable({ - write(chunk) { - dockerfile += chunk; - }, - final() {}, - }); - return { - pid: -1, - error: undefined, - stderr: Buffer.from([]), - stdout: Buffer.from("i promise I am a successful docker build"), - stdin: readable, - status: 0, - signal: null, - output: [null], - on: (reason: string, cbPassed: (code: number) => unknown) => { - if (reason === "exit") { - expect(dockerfile).toEqual(expectedDockerfile); - cbPassed(0); - } - }, - } as unknown as ChildProcess; - }; -} - -function mockDockerImageInspect(containerName: string, tag: string) { - return (cmd: string, args: readonly string[]) => { - expect(cmd).toBe("/usr/bin/docker"); - expect(args).toEqual([ - "image", - "inspect", - `${getCloudflareContainerRegistry()}/${containerName}:${tag}`, - "--format", - "{{ .Size }} {{ len .RootFS.Layers }} {{json .RepoDigests}}", - ]); - - const stdout = new PassThrough(); - const stderr = new PassThrough(); - - const child = { - stdout, - stderr, - on(event: string, cb: (code: number) => void) { - if (event === "close") { - setImmediate(() => cb(0)); - } - return this; - }, - }; - - setImmediate(() => { - stdout.emit( - "data", - `123456 4 ["${getCloudflareContainerRegistry()}/${containerName}@sha256:three"]` - ); - }); - - return child as unknown as ChildProcess; - }; -} - -function mockDockerImageDelete(containerName: string, tag: string) { - return (cmd: string, args: readonly string[]) => { - expect(cmd).toBe("/usr/bin/docker"); - expect(args).toEqual([ - "image", - "rm", - `${getCloudflareContainerRegistry()}/${containerName}:${tag}`, - ]); - return defaultChildProcess(); - }; -} - -function mockDockerLogin(expectedPassword: string) { - return (cmd: string, _args: readonly string[]) => { - expect(cmd).toBe("/usr/bin/docker"); - let password = ""; - const readable = new Writable({ - write(chunk) { - password += chunk; - }, - final() {}, - }); - return { - stdout: Buffer.from("i promise I am a successful docker login"), - stdin: readable, - on: function (reason: string, cbPassed: (code: number) => unknown) { - if (reason === "close") { - expect(password).toEqual(expectedPassword); - cbPassed(0); - } - return this; - }, - } as unknown as ChildProcess; - }; -} - -function mockDockerManifestInspect(containerName: string, shouldFail = true) { - return (cmd: string, args: readonly string[]) => { - expect(cmd).toBe("/usr/bin/docker"); - expect(args[0]).toBe("manifest"); - expect(args[1]).toBe("inspect"); - expect(args[2]).toEqual(`${containerName}@three`); - expect(args).toEqual([ - "manifest", - "inspect", - `${getCloudflareContainerRegistry()}/${containerName}@three`, - ]); - const readable = new Writable({ - write() {}, - final() {}, - }); - return { - stdout: Buffer.from( - "i promise I am an unsuccessful docker manifest call" - ), - stdin: readable, - on: function (reason: string, cbPassed: (code: number) => unknown) { - if (reason === "close") { - cbPassed(shouldFail ? 1 : 0); - } - return this; - }, - } as unknown as ChildProcess; - }; -} - -function mockDockerPush(containerName: string, tag: string) { - return (cmd: string, args: readonly string[]) => { - expect(cmd).toBe("/usr/bin/docker"); - expect(args).toEqual([ - "push", - `${getCloudflareContainerRegistry()}/${containerName}:${tag}`, - ]); - return defaultChildProcess(); - }; -} - -function mockDockerTag(from: string, to: string, tag: string) { - return (cmd: string, args: readonly string[]) => { - expect(cmd).toBe("/usr/bin/docker"); - expect(args).toEqual([ - "tag", - `${getCloudflareContainerRegistry()}/${from}:${tag}`, - `${getCloudflareContainerRegistry()}/${to}:${tag}`, - ]); - return defaultChildProcess(); - }; -} diff --git a/packages/wrangler/src/__tests__/containers/apply.test.ts b/packages/wrangler/src/__tests__/containers/apply.test.ts deleted file mode 100644 index 66dabca2ad09..000000000000 --- a/packages/wrangler/src/__tests__/containers/apply.test.ts +++ /dev/null @@ -1,1959 +0,0 @@ -import { - getCloudflareContainerRegistry, - OpenAPI, - SchedulingPolicy, - SecretAccessType, -} from "@cloudflare/containers-shared"; -import { http, HttpResponse } from "msw"; -import patchConsole from "patch-console"; -import { apply } from "../../containers/deploy"; -import { mockAccount } from "../cloudchamber/utils"; -import { mockAccountId, mockApiToken } from "../helpers/mock-account-id"; -import { mockCLIOutput } from "../helpers/mock-console"; -import { useMockIsTTY } from "../helpers/mock-istty"; -import { msw } from "../helpers/msw"; -import { runInTempDir } from "../helpers/run-in-tmp"; -import type { Config } from "../../config"; -import type { - Application, - CreateApplicationRequest, - ModifyApplicationRequestBody, -} from "@cloudflare/containers-shared"; - -function mockGetApplications(applications: Application[]) { - msw.use( - http.get( - "*/applications", - async () => { - return HttpResponse.json(applications); - }, - { once: true } - ) - ); -} - -function mockCreateApplication( - response?: Partial, - expected?: Partial -) { - msw.use( - http.post( - "*/applications", - async ({ request }) => { - const body = (await request.json()) as CreateApplicationRequest; - if (expected !== undefined) { - expect(body).toMatchObject(expected); - } - expect(body).toHaveProperty("instances"); - return HttpResponse.json(response); - }, - { once: true } - ) - ); -} - -function mockModifyApplication( - expected?: Application -): Promise { - let response: (value: ModifyApplicationRequestBody) => void; - const promise = new Promise((res) => { - response = res; - }); - - msw.use( - http.patch( - "*/applications/:id", - async ({ request }) => { - const json = await request.json(); - if (expected !== undefined) { - expect(json).toEqual(expected); - } - - expect((json as CreateApplicationRequest).name).toBeUndefined(); - response(json as ModifyApplicationRequestBody); - return HttpResponse.json(json); - }, - { once: true } - ) - ); - - return promise; -} - -const basicWranglerConfig = { - name: "my-container", - // // necessary to render the diff as toml - configPath: "wrangler.toml", - containers: [ - { - name: "my-container-app", - class_name: "DurableObjectClass", - max_instances: 2, - // this should be in the shape of wranglerConfig after the validation step - configuration: { image: "docker.io/hello:hi" }, - }, - ], -} as Config; - -describe("containers apply", () => { - /* eslint no-irregular-whitespace: ["error", { "skipTemplates": true }] - --- - Wrangler emits \u200a instead of "regular" whitespace in some cases. eslint doesn't like - this so we disable the warning when mixed whitespace is used in template strings. - */ - - const { setIsTTY } = useMockIsTTY(); - const std = mockCLIOutput(); - - mockAccountId(); - mockApiToken(); - beforeEach(mockAccount); - runInTempDir(); - afterEach(() => { - patchConsole(() => {}); - msw.resetHandlers(); - }); - - beforeAll(() => { - // populate OpenAPI.BASE with something so that msw gets a valid URL - OpenAPI.BASE = "https://example.com/"; - }); - afterAll(() => { - OpenAPI.BASE = ""; - }); - - test("can apply a simple application", async () => { - setIsTTY(false); - - mockGetApplications([]); - mockCreateApplication({ id: "abc" }); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - basicWranglerConfig - ); - - expect(std.stderr).toMatchInlineSnapshot(`""`); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ NEW my-container-app - │ - │ [[containers]] - │ name = \\"my-container-app\\" - │ max_instances = 2 - │ scheduling_policy = \\"default\\" - │ - │ [containers.configuration] - │ image = \\"docker.io/hello:hi\\" - │ instance_type = \\"dev\\" - │ - │ [containers.constraints] - │ tier = 1 - │ - │ - │  SUCCESS  Created application my-container-app (Application ID: abc) - │ - ╰ Applied changes - - " - `); - }); - - test("can apply a simple existing application", async () => { - setIsTTY(false); - - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - max_instances: 3, - instances: 0, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.DEFAULT, - configuration: { - image: "docker.io/hello:hi", - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 3, - }, - }, - ]); - const applicationReqBodyPromise = mockModifyApplication(); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - basicWranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [[containers]] - │ instances = 0 - │ - max_instances = 3 - │ + max_instances = 2 - │ name = \\"my-container-app\\" - │ - │ [containers.constraints] - │ - tier = 3 - │ + tier = 1 - │ - │ - │  SUCCESS  Modified application my-container-app - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - const app = await applicationReqBodyPromise; - expect(app.constraints?.tier).toEqual(1); - expect(app.max_instances).toEqual(2); - }); - - test("can apply a simple existing application and create other (max_instances)", async () => { - setIsTTY(false); - const wranglerConfig = { - name: "my-container", - configPath: "wrangler.toml", - containers: [ - { - name: "my-container-app", - class_name: "DurableObjectClass", - max_instances: 3, - configuration: { image: "docker.io/hello:hi" }, - }, - { - name: "my-container-app-2", - max_instances: 3, - class_name: "DurableObjectClass2", - configuration: { image: "docker.io/hello:hi" }, - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - max_instances: 4, - instances: 3, - created_at: new Date().toString(), - account_id: "1", - version: 1, - scheduling_policy: SchedulingPolicy.DEFAULT, - configuration: { - image: "docker.io/hello:hi", - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 1, - }, - }, - ]); - const res = mockModifyApplication(); - mockCreateApplication({ id: "abc" }); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - const body = await res; - expect(body).not.toHaveProperty("instances"); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [[containers]] - │ instances = 0 - │ - max_instances = 4 - │ + max_instances = 3 - │ name = \\"my-container-app\\" - │ - ├ NEW my-container-app-2 - │ - │ [[containers]] - │ name = \\"my-container-app-2\\" - │ max_instances = 3 - │ scheduling_policy = \\"default\\" - │ - │ [containers.configuration] - │ image = \\"docker.io/hello:hi\\" - │ instance_type = \\"dev\\" - │ - │ [containers.constraints] - │ tier = 1 - │ - │ - │  SUCCESS  Modified application my-container-app - │ - │ - │  SUCCESS  Created application my-container-app-2 (Application ID: abc) - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - }); - - test("can skip a simple existing application and create other", async () => { - setIsTTY(false); - const wranglerConfig = { - name: "my-container", - configPath: "wrangler.toml", - containers: [ - { - name: "my-container-app", - instances: 4, - class_name: "DurableObjectClass", - configuration: { image: "docker.io/hello:hi" }, - rollout_kind: "none", - }, - { - name: "my-container-app-2", - instances: 1, - class_name: "DurableObjectClass2", - configuration: { image: "docker.io/other:app" }, - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 3, - created_at: new Date().toString(), - account_id: "1", - version: 1, - scheduling_policy: SchedulingPolicy.DEFAULT, - configuration: { - image: "docker.io/hello:hi", - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 1, - }, - }, - ]); - mockCreateApplication({ id: "abc" }); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [[containers]] - │ - instances = 3 - │ + instances = 4 - │ name = \\"my-container-app\\" - │ Skipping application rollout - │ - ├ NEW my-container-app-2 - │ - │ [[containers]] - │ name = \\"my-container-app-2\\" - │ instances = 1 - │ scheduling_policy = \\"default\\" - │ - │ [containers.configuration] - │ image = \\"docker.io/other:app\\" - │ instance_type = \\"dev\\" - │ - │ [containers.constraints] - │ tier = 1 - │ - │ - │  SUCCESS  Created application my-container-app-2 (Application ID: abc) - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - }); - - test("can apply a simple existing application and create other", async () => { - setIsTTY(false); - const wranglerConfig = { - name: "my-container", - configPath: "wrangler.toml", - containers: [ - { - name: "my-container-app", - instances: 4, - class_name: "DurableObjectClass", - configuration: { age: "docker.io/hello:hi" }, - }, - { - name: "my-container-app-2", - instances: 1, - class_name: "DurableObjectClass2", - configuration: { image: "docker.io/other:app" }, - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 3, - created_at: new Date().toString(), - account_id: "1", - version: 1, - scheduling_policy: SchedulingPolicy.DEFAULT, - configuration: { - image: "docker.io/hello:hi", - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 1, - }, - }, - ]); - const res = mockModifyApplication(); - mockCreateApplication({ id: "abc" }); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - await res; - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [[containers]] - │ - instances = 3 - │ + instances = 4 - │ name = \\"my-container-app\\" - │ - │ [containers.configuration] - │ ... - │ instance_type = \\"dev\\" - │ + age = \\"docker.io/hello:hi\\" - │ - │ [containers.constraints] - │ ... - │ - ├ NEW my-container-app-2 - │ - │ [[containers]] - │ name = \\"my-container-app-2\\" - │ instances = 1 - │ scheduling_policy = \\"default\\" - │ - │ [containers.configuration] - │ image = \\"docker.io/other:app\\" - │ instance_type = \\"dev\\" - │ - │ [containers.constraints] - │ tier = 1 - │ - │ - │  SUCCESS  Modified application my-container-app - │ - │ - │  SUCCESS  Created application my-container-app-2 (Application ID: abc) - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - }); - - test("can apply an application, and there is no changes (retrocompatibility with regional scheduling policy)", async () => { - setIsTTY(false); - const wranglerConfig = { - name: "my-container", - configPath: "wrangler.toml", - containers: [ - { - class_name: "DurableObjectClass", - name: "my-container-app", - instances: 3, - configuration: { - image: "docker.io/hello:hi", - labels: [ - { - name: "name", - value: "value", - }, - { - name: "name-2", - value: "value-2", - }, - ], - secrets: [ - { - name: "MY_SECRET", - type: SecretAccessType.ENV, - secret: "SECRET_NAME", - }, - { - name: "MY_SECRET_1", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_1", - }, - { - name: "MY_SECRET_2", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_2", - }, - ], - }, - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 3, - version: 1, - created_at: new Date().toString(), - account_id: "1", - scheduling_policy: SchedulingPolicy.DEFAULT, - configuration: { - image: "docker.io/hello:hi", - labels: [ - { - name: "name", - value: "value", - }, - { - name: "name-2", - value: "value-2", - }, - ], - secrets: [ - { - name: "MY_SECRET", - type: SecretAccessType.ENV, - secret: "SECRET_NAME", - }, - { - name: "MY_SECRET_1", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_1", - }, - { - name: "MY_SECRET_2", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_2", - }, - ], - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - - constraints: { - tier: 1, - }, - }, - ]); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ no changes my-container-app - │ - ╰ No changes to be made - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - }); - - test("can apply an application, and there is no changes (two applications)", async () => { - setIsTTY(false); - const app = { - name: "my-container-app", - instances: 3, - class_name: "DurableObjectClass", - image: "./Dockerfile", - configuration: { - labels: [ - { - name: "name", - value: "value", - }, - { - name: "name-2", - value: "value-2", - }, - ], - secrets: [ - { - name: "MY_SECRET", - type: SecretAccessType.ENV, - secret: "SECRET_NAME", - }, - { - name: "MY_SECRET_1", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_1", - }, - { - name: "MY_SECRET_2", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_2", - }, - ], - }, - }; - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - containers: [app, { ...app, name: "my-container-app-2" }], - } as Config; - - const completeApp = { - id: "abc", - name: "my-container-app", - instances: 3, - created_at: new Date().toString(), - class_name: "DurableObjectClass", - account_id: "1", - scheduling_policy: SchedulingPolicy.DEFAULT, - configuration: { - image: "./Dockerfile", - labels: [ - { - name: "name", - value: "value", - }, - { - name: "name-2", - value: "value-2", - }, - ], - secrets: [ - { - name: "MY_SECRET", - type: SecretAccessType.ENV, - secret: "SECRET_NAME", - }, - { - name: "MY_SECRET_1", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_1", - }, - { - name: "MY_SECRET_2", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_2", - }, - ], - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - - constraints: { - tier: 1, - }, - }; - - mockGetApplications([ - { ...completeApp, version: 1 }, - { ...completeApp, version: 1, name: "my-container-app-2", id: "abc2" }, - ]); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ no changes my-container-app - │ - ├ no changes my-container-app-2 - │ - ╰ No changes to be made - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - }); - - test("can apply an application, and there is no changes", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - containers: [ - { - class_name: "DurableObjectClass", - name: "my-container-app", - instances: 3, - image: "docker.io/hello:hi", - configuration: { - labels: [ - { - name: "name", - value: "value", - }, - { - name: "name-2", - value: "value-2", - }, - ], - secrets: [ - { - name: "MY_SECRET", - type: SecretAccessType.ENV, - secret: "SECRET_NAME", - }, - { - name: "MY_SECRET_1", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_1", - }, - { - name: "MY_SECRET_2", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_2", - }, - ], - }, - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 3, - version: 1, - created_at: new Date().toString(), - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "docker.io/hello:hi", - labels: [ - { - name: "name", - value: "value", - }, - { - name: "name-2", - value: "value-2", - }, - ], - secrets: [ - { - name: "MY_SECRET", - type: SecretAccessType.ENV, - secret: "SECRET_NAME", - }, - { - name: "MY_SECRET_1", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_1", - }, - { - name: "MY_SECRET_2", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_2", - }, - ], - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - - constraints: { - tier: 1, - }, - }, - ]); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ no changes my-container-app - │ - ╰ No changes to be made - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - }); - - test("can apply an application, and there is no changes (two applications)", async () => { - setIsTTY(false); - const app = { - name: "my-container-app", - instances: 3, - class_name: "DurableObjectClass", - image: "./Dockerfile", - configuration: { - labels: [ - { - name: "name", - value: "value", - }, - { - name: "name-2", - value: "value-2", - }, - ], - secrets: [ - { - name: "MY_SECRET", - type: SecretAccessType.ENV, - secret: "SECRET_NAME", - }, - { - name: "MY_SECRET_1", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_1", - }, - { - name: "MY_SECRET_2", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_2", - }, - ], - }, - }; - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - containers: [app, { ...app, name: "my-container-app-2" }], - } as Config; - - const completeApp = { - id: "abc", - name: "my-container-app", - instances: 3, - created_at: new Date().toString(), - class_name: "DurableObjectClass", - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "./Dockerfile", - labels: [ - { - name: "name", - value: "value", - }, - { - name: "name-2", - value: "value-2", - }, - ], - secrets: [ - { - name: "MY_SECRET", - type: SecretAccessType.ENV, - secret: "SECRET_NAME", - }, - { - name: "MY_SECRET_1", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_1", - }, - { - name: "MY_SECRET_2", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_2", - }, - ], - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - - constraints: { - tier: 1, - }, - }; - - mockGetApplications([ - { ...completeApp, version: 1 }, - { ...completeApp, version: 1, name: "my-container-app-2", id: "abc2" }, - ]); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ no changes my-container-app - │ - ├ no changes my-container-app-2 - │ - ╰ No changes to be made - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - }); - - test("can enable observability logs (top-level field)", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - observability: { enabled: true }, - containers: [ - { - name: "my-container-app", - class_name: "DurableObjectClass", - instances: 1, - image: "docker.io/hello:hi", - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 1, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "docker.io/hello:hi", - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 1, - }, - }, - ]); - const applicationReqBodyPromise = mockModifyApplication(); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [containers.configuration] - │ ... - │ instance_type = \\"dev\\" - │ - │ + [containers.configuration.observability.logs] - │ + enabled = true - │ - │ [containers.constraints] - │ ... - │ - │ - │  SUCCESS  Modified application my-container-app - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - const app = await applicationReqBodyPromise; - expect(app.constraints?.tier).toEqual(1); - expect(app.instances).toEqual(1); - }); - - test("can enable observability logs (logs field)", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - observability: { logs: { enabled: true } }, - containers: [ - { - name: "my-container-app", - class_name: "DurableObjectClass", - instances: 1, - image: "docker.io/hello:hi", - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 1, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "docker.io/hello:hi", - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 1, - }, - }, - ]); - const applicationReqBodyPromise = mockModifyApplication(); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [containers.configuration] - │ ... - │ instance_type = \\"dev\\" - │ - │ + [containers.configuration.observability.logs] - │ + enabled = true - │ - │ [containers.constraints] - │ ... - │ - │ - │  SUCCESS  Modified application my-container-app - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - const app = await applicationReqBodyPromise; - expect(app.constraints?.tier).toEqual(1); - expect(app.instances).toEqual(1); - }); - - test("can disable observability logs (top-level field)", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - observability: { enabled: false }, - containers: [ - { - name: "my-container-app", - class_name: "DurableObjectClass", - instances: 1, - image: "docker.io/hello:hi", - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 1, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "docker.io/hello:hi", - observability: { - logs: { - enabled: true, - }, - }, - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 1, - }, - }, - ]); - const applicationReqBodyPromise = mockModifyApplication(); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [containers.configuration.observability.logs] - │ - enabled = true - │ + enabled = false - │ - │ [containers.constraints] - │ ... - │ - │ - │  SUCCESS  Modified application my-container-app - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - const app = await applicationReqBodyPromise; - expect(app.constraints?.tier).toEqual(1); - expect(app.instances).toEqual(1); - }); - - test("can disable observability logs (logs field)", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - observability: { logs: { enabled: false } }, - containers: [ - { - name: "my-container-app", - class_name: "DurableObjectClass", - instances: 1, - image: "docker.io/hello:hi", - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 1, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "docker.io/hello:hi", - observability: { - logs: { - enabled: true, - }, - }, - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 1, - }, - }, - ]); - const applicationReqBodyPromise = mockModifyApplication(); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [containers.configuration.observability.logs] - │ - enabled = true - │ + enabled = false - │ - │ [containers.constraints] - │ ... - │ - │ - │  SUCCESS  Modified application my-container-app - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - const app = await applicationReqBodyPromise; - expect(app.constraints?.tier).toEqual(1); - expect(app.instances).toEqual(1); - }); - - test("can disable observability logs (absent field)", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - containers: [ - { - name: "my-container-app", - class_name: "DurableObjectClass", - instances: 1, - image: "docker.io/hello:hi", - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 1, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "docker.io/hello:hi", - observability: { - logs: { - enabled: true, - }, - }, - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 1, - }, - }, - ]); - const applicationReqBodyPromise = mockModifyApplication(); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [containers.configuration.observability.logs] - │ - enabled = true - │ + enabled = false - │ - │ [containers.constraints] - │ ... - │ - │ - │  SUCCESS  Modified application my-container-app - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - const app = await applicationReqBodyPromise; - expect(app.constraints?.tier).toEqual(1); - expect(app.instances).toEqual(1); - }); - - test("ignores deprecated observability.logging", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - containers: [ - { - name: "my-container-app", - class_name: "DurableObjectClass", - instances: 1, - image: "docker.io/hello:hi", - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 1, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "docker.io/hello:hi", - observability: { - logs: { - enabled: true, - }, - logging: { - enabled: true, - }, - }, - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 1, - }, - }, - ]); - const applicationReqBodyPromise = mockModifyApplication(); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [containers.configuration.observability.logs] - │ - enabled = true - │ + enabled = false - │ - │ [containers.constraints] - │ ... - │ - │ - │  SUCCESS  Modified application my-container-app - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - const app = await applicationReqBodyPromise; - expect(app.constraints?.tier).toEqual(1); - expect(app.instances).toEqual(1); - }); - - test("keeps observability logs enabled", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - observability: { enabled: true }, - containers: [ - { - name: "my-container-app", - class_name: "DurableObjectClass", - instances: 1, - image: "docker.io/hello:hi", - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 1, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "docker.io/hello:hi", - observability: { - logs: { - enabled: true, - }, - logging: { - enabled: true, - }, - }, - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 1, - }, - }, - ]); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ no changes my-container-app - │ - ╰ No changes to be made - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - }); - - test("keeps observability logs disabled (undefined in the app)", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - containers: [ - { - name: "my-container-app", - class_name: "DurableObjectClass", - instances: 1, - image: "docker.io/hello:hi", - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 1, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "docker.io/hello:hi", - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 1, - }, - }, - ]); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ no changes my-container-app - │ - ╰ No changes to be made - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - }); - - test("keeps observability logs disabled (false in the app)", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - containers: [ - { - name: "my-container-app", - class_name: "DurableObjectClass", - instances: 1, - image: "docker.io/hello:hi", - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 1, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "docker.io/hello:hi", - observability: { - logs: { - enabled: false, - }, - logging: { - enabled: false, - }, - }, - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 1, - }, - }, - ]); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ no changes my-container-app - │ - ╰ No changes to be made - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - }); - - test("can apply a simple application (instance type)", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - containers: [ - { - name: "my-container-app", - instances: 3, - class_name: "DurableObjectClass", - instance_type: "dev", - image: "docker.io/hello:hi", - constraints: { - tier: 2, - }, - }, - ], - } as Config; - mockGetApplications([]); - mockCreateApplication({ id: "abc" }); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ NEW my-container-app - │ - │ [[containers]] - │ name = \\"my-container-app\\" - │ instances = 3 - │ scheduling_policy = \\"default\\" - │ - │ [containers.constraints] - │ tier = 2 - │ - │ [containers.configuration] - │ instance_type = \\"dev\\" - │ - │ - │  SUCCESS  Created application my-container-app (Application ID: abc) - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - }); - - test("can apply a simple existing application (instance type)", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - containers: [ - { - name: "my-container-app", - instances: 4, - class_name: "DurableObjectClass", - instance_type: "standard", - image: "docker.io/hello:hi", - constraints: { - tier: 2, - }, - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 3, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "docker.io/hello:hi", - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 3, - }, - }, - ]); - const applicationReqBodyPromise = mockModifyApplication(); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [[containers]] - │ - instances = 3 - │ + instances = 4 - │ name = \\"my-container-app\\" - │ - │ [containers.configuration] - │ image = \\"docker.io/hello:hi\\" - │ - instance_type = \\"dev\\" - │ + instance_type = \\"standard\\" - │ - │ [containers.constraints] - │ ... - │ - tier = 3 - │ + tier = 2 - │ - │ - │  SUCCESS  Modified application my-container-app - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - const app = await applicationReqBodyPromise; - expect(app.configuration?.instance_type).toEqual("standard"); - }); - - test("falls back on dev instance type when instance type is absent", async () => { - setIsTTY(false); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - containers: [ - { - name: "my-container-app", - instances: 4, - class_name: "DurableObjectClass", - image: "docker.io/hello:hi", - constraints: { - tier: 2, - }, - }, - ], - } as Config; - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 3, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "docker.io/hello:hi", - disk: { - size: "4GB", - size_mb: 4000, - }, - vcpu: 0.25, - memory: "1024MB", - memory_mib: 1024, - }, - constraints: { - tier: 3, - }, - }, - ]); - const applicationReqBodyPromise = mockModifyApplication(); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [[containers]] - │ - instances = 3 - │ + instances = 4 - │ name = \\"my-container-app\\" - │ - │ [containers.configuration] - │ image = \\"docker.io/hello:hi\\" - │ - instance_type = \\"basic\\" - │ + instance_type = \\"dev\\" - │ - │ [containers.constraints] - │ ... - │ - tier = 3 - │ + tier = 2 - │ - │ - │  SUCCESS  Modified application my-container-app - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - const app = await applicationReqBodyPromise; - expect(app.configuration?.instance_type).toEqual("dev"); - }); - - test("expands image names from managed registry when creating an application", async () => { - setIsTTY(false); - const registry = getCloudflareContainerRegistry(); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - containers: [ - { - name: "my-container-app", - instances: 3, - class_name: "DurableObjectClass", - configuration: { image: `${registry}/hello:1.0` }, - constraints: { - tier: 2, - }, - }, - ], - } as Config; - - mockGetApplications([]); - mockCreateApplication( - { id: "abc" }, - { - configuration: { - image: `${registry}/some-account-id/hello:1.0`, - }, - } - ); - - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stderr).toMatchInlineSnapshot(`""`); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ NEW my-container-app - │ - │ [[containers]] - │ name = \\"my-container-app\\" - │ instances = 3 - │ scheduling_policy = \\"default\\" - │ - │ [containers.configuration] - │ image = \\"registry.cloudflare.com/some-account-id/hello:1.0\\" - │ instance_type = \\"dev\\" - │ - │ [containers.constraints] - │ tier = 2 - │ - │ - │  SUCCESS  Created application my-container-app (Application ID: abc) - │ - ╰ Applied changes - - " - `); - }); - - test("expands image names from managed registry when modifying an application", async () => { - setIsTTY(false); - const registry = getCloudflareContainerRegistry(); - const wranglerConfig = { - configPath: "wrangler.toml", - name: "my-container", - containers: [ - { - name: "my-container-app", - instances: 3, - class_name: "DurableObjectClass", - image: `${registry}/hello:1.0`, - instance_type: "standard", - constraints: { - tier: 2, - }, - }, - ], - } as Config; - - mockGetApplications([ - { - id: "abc", - name: "my-container-app", - instances: 3, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: `${registry}/some-account-id/hello:1.0`, - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - constraints: { - tier: 3, - }, - }, - ]); - - const applicationReqBodyPromise = mockModifyApplication(); - await apply( - { skipDefaults: false, imageUpdateRequired: false }, - wranglerConfig - ); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ EDIT my-container-app - │ - │ [containers.configuration] - │ image = \\"${registry}/some-account-id/hello:1.0\\" - │ - instance_type = \\"dev\\" - │ + instance_type = \\"standard\\" - │ - │ [containers.constraints] - │ ... - │ - tier = 3 - │ + tier = 2 - │ - │ - │  SUCCESS  Modified application my-container-app - │ - ╰ Applied changes - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - const app = await applicationReqBodyPromise; - expect(app.configuration?.instance_type).toEqual("standard"); - }); -}); diff --git a/packages/wrangler/src/__tests__/containers/deploy.test.ts b/packages/wrangler/src/__tests__/containers/deploy.test.ts new file mode 100644 index 000000000000..e270b0ba7b70 --- /dev/null +++ b/packages/wrangler/src/__tests__/containers/deploy.test.ts @@ -0,0 +1,1282 @@ +import { spawn } from "node:child_process"; +import * as fs from "node:fs"; +import { PassThrough, Writable } from "node:stream"; +import { + getCloudflareContainerRegistry, + SchedulingPolicy, +} from "@cloudflare/containers-shared"; +import { http, HttpResponse } from "msw"; +import { maybeBuildContainer } from "../../cloudchamber/deploy"; +import { clearCachedAccount } from "../../cloudchamber/locations"; +import { mockAccountV4 as mockContainersAccount } from "../cloudchamber/utils"; +import { mockAccountId, mockApiToken } from "../helpers/mock-account-id"; +import { mockCLIOutput, mockConsoleMethods } from "../helpers/mock-console"; +import { mockLegacyScriptData } from "../helpers/mock-legacy-script"; +import { mockUploadWorkerRequest } from "../helpers/mock-upload-worker"; +import { mockSubDomainRequest } from "../helpers/mock-workers-subdomain"; +import { + createFetchResult, + msw, + mswSuccessDeploymentScriptMetadata, +} from "../helpers/msw"; +import { mswListNewDeploymentsLatestFull } from "../helpers/msw/handlers/versions"; +import { runInTempDir } from "../helpers/run-in-tmp"; +import { runWrangler } from "../helpers/run-wrangler"; +import { writeWranglerConfig } from "../helpers/write-wrangler-config"; +import type { + AccountRegistryToken, + Application, + ContainerNormalisedConfig, + CreateApplicationRequest, + ImageRegistryCredentialsConfiguration, + InstanceType, +} from "@cloudflare/containers-shared"; +import type { ChildProcess } from "node:child_process"; + +vi.mock("node:child_process"); +describe("maybeBuildContainer", () => { + it("Should return imageUpdate: true if using an image URI", async () => { + const config = { + registry_link: "registry.cloudflare.com/some-image:uri", + class_name: "Test", + } as ContainerNormalisedConfig; + const result = await maybeBuildContainer( + config, + "some-tag:thing", + false, + "/usr/bin/docker" + ); + expect(result.newImageLink).toEqual( + "registry.cloudflare.com/some-image:uri" + ); + }); +}); + +describe("wrangler deploy with containers", () => { + runInTempDir(); + const std = mockConsoleMethods(); + const cliStd = mockCLIOutput(); + mockAccountId(); + mockApiToken(); + beforeEach(() => { + setupCommonMocks(); + fs.writeFileSync( + "index.js", + `export class ExampleDurableObject {}; export default{};` + ); + vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/docker"); + }); + afterEach(() => { + vi.unstubAllEnvs(); + }); + it("should fail early if no docker is detected when deploying a container from a dockerfile", async () => { + vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/bad-docker-path"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + containers: [DEFAULT_CONTAINER_FROM_DOCKERFILE], + }); + + fs.writeFileSync("./Dockerfile", "FROM scratch"); + + await expect( + runWrangler("deploy index.js") + ).rejects.toThrowErrorMatchingInlineSnapshot( + ` + [Error: The Docker CLI could not be launched. Please ensure that the Docker CLI is installed and the daemon is running. + Other container tooling that is compatible with the Docker CLI and engine may work, but is not yet guaranteed to do so. You can specify an executable with the environment variable WRANGLER_DOCKER_BIN and a socket with WRANGLER_DOCKER_HOST.] + ` + ); + }); + it("should be able to deploy a new container from a dockerfile", async () => { + mockGetVersion("Galaxy-Class"); + setupDockerMocks("my-container", "Galaxy"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + containers: [DEFAULT_CONTAINER_FROM_DOCKERFILE], + }); + mockGetApplications([]); + fs.writeFileSync("./Dockerfile", "FROM scratch"); + mockGenerateImageRegistryCredentials(); + + mockCreateApplication({ + name: "my-container", + max_instances: 10, + configuration: { + image: + getCloudflareContainerRegistry() + + "/some-account-id/my-container:Galaxy", + }, + }); + + await runWrangler("deploy index.js"); + + expect(std.out).toMatchInlineSnapshot(` + "Total Upload: xx KiB / gzip: xx KiB + Worker Startup Time: 100 ms + Your Worker has access to the following bindings: + Binding Resource + env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object + + Uploaded test-name (TIMINGS) + Building image my-container:Galaxy + Image does not exist remotely, pushing: registry.cloudflare.com/some-account-id/my-container:Galaxy + Deployed test-name triggers (TIMINGS) + https://test-name.test-sub-domain.workers.dev + Current Version ID: Galaxy-Class" + `); + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ NEW my-container + │ + │ { + │ \\"containers\\": [ + │ { + │ \\"name\\": \\"my-container\\", + │ \\"scheduling_policy\\": \\"default\\", + │ \\"configuration\\": { + │ \\"image\\": \\"registry.cloudflare.com/some-account-id/my-container:Galaxy\\", + │ \\"instance_type\\": \\"dev\\" + │ }, + │ \\"instances\\": 0, + │ \\"max_instances\\": 10, + │ \\"constraints\\": { + │ \\"tier\\": 1 + │ } + │ } + │ ] + │ } + │ + │ SUCCESS Created application my-container (Application ID: undefined) + │ + ╰ Applied changes + + " + `); + }); + it("should be able to deploy a new container from an image uri", async () => { + // note no docker commands have been mocked here! + mockGetVersion("Galaxy-Class"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + containers: [DEFAULT_CONTAINER_FROM_REGISTRY], + }); + + mockGetApplications([]); + + mockCreateApplication({ + name: "my-container", + max_instances: 10, + scheduling_policy: SchedulingPolicy.DEFAULT, + }); + + await runWrangler("deploy index.js"); + + expect(std.out).toMatchInlineSnapshot(` + "Total Upload: xx KiB / gzip: xx KiB + Worker Startup Time: 100 ms + Your Worker has access to the following bindings: + Binding Resource + env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object + + Uploaded test-name (TIMINGS) + Deployed test-name triggers (TIMINGS) + https://test-name.test-sub-domain.workers.dev + Current Version ID: Galaxy-Class" + `); + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ NEW my-container + │ + │ { + │ \\"containers\\": [ + │ { + │ \\"name\\": \\"my-container\\", + │ \\"scheduling_policy\\": \\"default\\", + │ \\"configuration\\": { + │ \\"image\\": \\"docker.io/hello:world\\", + │ \\"instance_type\\": \\"dev\\" + │ }, + │ \\"instances\\": 0, + │ \\"max_instances\\": 10, + │ \\"constraints\\": { + │ \\"tier\\": 1 + │ } + │ } + │ ] + │ } + │ + │ SUCCESS Created application my-container (Application ID: undefined) + │ + ╰ Applied changes + + " + `); + }); + + it("should resolve the docker build context path based on the dockerfile location, if image_build_context is not provided", async () => { + vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/docker"); + mockGetVersion("Galaxy-Class"); + setupDockerMocks("my-container", "Galaxy"); + mockContainersAccount(); + + writeWranglerConfig( + { + main: "./worker/index.js", + ...DEFAULT_DURABLE_OBJECTS, + containers: [ + { + name: "my-container", + max_instances: 10, + class_name: "ExampleDurableObject", + image: "../Dockerfile", + }, + ], + }, + "src/wrangler.json" + ); + + fs.writeFileSync("Dockerfile", "FROM scratch"); + fs.mkdirSync("src/worker", { recursive: true }); + fs.writeFileSync( + "src/worker/index.js", + `export class ExampleDurableObject {}; export default{};` + ); + mockGetApplications([]); + + mockGenerateImageRegistryCredentials(); + + mockCreateApplication({ + name: "my-container", + max_instances: 10, + configuration: { + image: + getCloudflareContainerRegistry() + + "/some-account-id/my-container:Galaxy", + }, + }); + + await runWrangler("deploy --cwd src"); + + expect(std.out).toMatchInlineSnapshot(` + "Total Upload: xx KiB / gzip: xx KiB + Worker Startup Time: 100 ms + Your Worker has access to the following bindings: + Binding Resource + env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object + + Uploaded test-name (TIMINGS) + Building image my-container:Galaxy + Image does not exist remotely, pushing: registry.cloudflare.com/some-account-id/my-container:Galaxy + Deployed test-name triggers (TIMINGS) + https://test-name.test-sub-domain.workers.dev + Current Version ID: Galaxy-Class" + `); + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + }); + + it("should resolve dockerfile path relative to wrangler config path", async () => { + mockGetVersion("Galaxy-Class"); + setupDockerMocks("my-container", "Galaxy", "FROM alpine"); + + fs.mkdirSync("nested/src", { recursive: true }); + fs.writeFileSync("Dockerfile", "FROM alpine"); + fs.writeFileSync( + "nested/src/index.js", + `export class ExampleDurableObject {}; export default{};` + ); + + writeWranglerConfig( + { + main: "./src/index.js", + ...DEFAULT_DURABLE_OBJECTS, + containers: [ + { + name: "my-container", + max_instances: 10, + class_name: "ExampleDurableObject", + image: "../Dockerfile", + }, + ], + }, + "nested/wrangler.json" + ); + + mockGetApplications([]); + mockGenerateImageRegistryCredentials(); + + mockCreateApplication({ + name: "my-container", + max_instances: 10, + configuration: { + image: + getCloudflareContainerRegistry() + + "/some-account-id/my-container:Galaxy", + }, + }); + + await runWrangler("deploy -c nested/wrangler.json"); + + expect(std.out).toMatchInlineSnapshot(` + "Total Upload: xx KiB / gzip: xx KiB + Worker Startup Time: 100 ms + Your Worker has access to the following bindings: + Binding Resource + env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object + + Uploaded test-name (TIMINGS) + Building image my-container:Galaxy + Image does not exist remotely, pushing: registry.cloudflare.com/some-account-id/my-container:Galaxy + Deployed test-name triggers (TIMINGS) + https://test-name.test-sub-domain.workers.dev + Current Version ID: Galaxy-Class" + `); + + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + }); + + it("should be able to redeploy an existing application ", async () => { + mockGetVersion("Galaxy-Class"); + setupDockerMocks("my-container", "Galaxy"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + containers: [DEFAULT_CONTAINER_FROM_DOCKERFILE], + }); + mockGetApplications([ + { + id: "abc", + name: "my-container", + instances: 0, + max_instances: 2, + created_at: new Date().toString(), + version: 1, + account_id: "1", + scheduling_policy: SchedulingPolicy.DEFAULT, + configuration: { + image: "registry.cloudflare.com/some-account-id/my-container:old", + disk: { + size: "2GB", + size_mb: 2000, + }, + vcpu: 0.0625, + memory: "256MB", + memory_mib: 256, + }, + constraints: { + tier: 1, + }, + durable_objects: { + namespace_id: "1", + }, + }, + ]); + fs.writeFileSync("./Dockerfile", "FROM scratch"); + mockGenerateImageRegistryCredentials(); + mockModifyApplication({ + configuration: { + image: "registry.cloudflare.com/some-account-id/my-container:Galaxy", + }, + max_instances: 10, + }); + mockCreateApplicationRollout({ + description: "Progressive update", + strategy: "rolling", + kind: "full_auto", + }); + await runWrangler("deploy index.js"); + + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ EDIT my-container + │ { + │ ... + │ \\"configuration\\": { + │ - \\"image\\": \\"registry.cloudflare.com/some-account-id/my-container:old\\", + │ + \\"image\\": \\"registry.cloudflare.com/some-account-id/my-container:Galaxy\\", + │ \\"instance_type\\": \\"dev\\" + │ }, + │ ... + │ \\"instances\\": 0, + │ - \\"max_instances\\": 2, + │ + \\"max_instances\\": 10, + │ \\"name\\": \\"my-container\\", + │ + │ SUCCESS Modified application my-container + │ + ╰ Applied changes + + " + `); + }); + it("should be able to redeploy an existing application and create another", async () => { + mockGetVersion("Galaxy-Class", [ + { + type: "durable_object_namespace", + namespace_id: "1", + class_name: "ExampleDurableObject", + }, + { + type: "durable_object_namespace", + namespace_id: "2", + class_name: "DurableObjectClass2", + }, + ]); + + setupDockerMocks("my-container", "Galaxy"); + mockUploadWorkerRequest({ + expectedBindings: [ + { + name: "EXAMPLE_DO_BINDING2", + type: "durable_object_namespace", + class_name: "DurableObjectClass2", + }, + { + name: "EXAMPLE_DO_BINDING", + type: "durable_object_namespace", + class_name: "ExampleDurableObject", + }, + ], + useOldUploadApi: true, + expectedContainers: [ + { class_name: "ExampleDurableObject" }, + { class_name: "DurableObjectClass2" }, + ], + }); + writeWranglerConfig({ + durable_objects: { + bindings: [ + { + name: "EXAMPLE_DO_BINDING2", + class_name: "DurableObjectClass2", + }, + + { + name: "EXAMPLE_DO_BINDING", + class_name: "ExampleDurableObject", + }, + ], + }, + containers: [ + DEFAULT_CONTAINER_FROM_DOCKERFILE, + { + name: "my-container-app-2", + max_instances: 3, + class_name: "DurableObjectClass2", + image: "docker.io/hello:world", + }, + ], + }); + fs.writeFileSync( + "index.js", + `export class DurableObjectClass2 {}; export class ExampleDurableObject {}; export default{};` + ); + + mockGetApplications([ + { + id: "abc", + name: "my-container", + instances: 0, + max_instances: 2, + created_at: new Date().toString(), + version: 1, + account_id: "1", + scheduling_policy: SchedulingPolicy.DEFAULT, + configuration: { + image: "registry.cloudflare.com/some-account-id/my-container:old", + disk: { + size: "2GB", + size_mb: 2000, + }, + vcpu: 0.0625, + memory: "256MB", + memory_mib: 256, + }, + constraints: { + tier: 1, + }, + durable_objects: { + namespace_id: "1", + }, + }, + ]); + fs.writeFileSync("./Dockerfile", "FROM scratch"); + mockGenerateImageRegistryCredentials(); + mockModifyApplication({ + configuration: { + image: "registry.cloudflare.com/some-account-id/my-container:Galaxy", + }, + max_instances: 10, + }); + mockCreateApplication({ + name: "my-container-app-2", + max_instances: 3, + scheduling_policy: SchedulingPolicy.DEFAULT, + }); + mockCreateApplicationRollout({ + description: "Progressive update", + strategy: "rolling", + kind: "full_auto", + }); + await runWrangler("deploy index.js"); + + expect(std.err).toMatchInlineSnapshot(`""`); + + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ EDIT my-container + │ { + │ ... + │ \\"configuration\\": { + │ - \\"image\\": \\"registry.cloudflare.com/some-account-id/my-container:old\\", + │ + \\"image\\": \\"registry.cloudflare.com/some-account-id/my-container:Galaxy\\", + │ \\"instance_type\\": \\"dev\\" + │ }, + │ ... + │ \\"instances\\": 0, + │ - \\"max_instances\\": 2, + │ + \\"max_instances\\": 10, + │ \\"name\\": \\"my-container\\", + │ + │ SUCCESS Modified application my-container + │ + ╰ Applied changes + + ╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ NEW my-container-app-2 + │ + │ { + │ \\"containers\\": [ + │ { + │ \\"name\\": \\"my-container-app-2\\", + │ \\"scheduling_policy\\": \\"default\\", + │ \\"configuration\\": { + │ \\"image\\": \\"docker.io/hello:world\\", + │ \\"instance_type\\": \\"dev\\" + │ }, + │ \\"instances\\": 0, + │ \\"max_instances\\": 3, + │ \\"constraints\\": { + │ \\"tier\\": 1 + │ } + │ } + │ ] + │ } + │ + │ SUCCESS Created application my-container-app-2 (Application ID: undefined) + │ + ╰ Applied changes + + " + `); + }); + it("skips an existing application if there are no changes", async () => { + mockGetVersion("Galaxy-Class"); + setupDockerMocks("my-container", "Galaxy"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + containers: [DEFAULT_CONTAINER_FROM_DOCKERFILE], + }); + mockGetApplications([ + { + id: "abc", + name: "my-container", + instances: 0, + max_instances: 10, + created_at: new Date().toString(), + version: 1, + account_id: "1", + scheduling_policy: SchedulingPolicy.DEFAULT, + configuration: { + image: "registry.cloudflare.com/some-account-id/my-container:Galaxy", + disk: { + size: "2GB", + size_mb: 2000, + }, + vcpu: 0.0625, + memory: "256MB", + memory_mib: 256, + }, + constraints: { + tier: 1, + }, + durable_objects: { + namespace_id: "1", + }, + }, + ]); + fs.writeFileSync("./Dockerfile", "FROM scratch"); + mockGenerateImageRegistryCredentials(); + + await runWrangler("deploy index.js"); + + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ no changes my-container + │ + ╰ No changes to be made + + " + `); + }); + + it("should error when no scope for containers", async () => { + mockContainersAccount([]); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + containers: [DEFAULT_CONTAINER_FROM_REGISTRY], + }); + + await expect( + runWrangler("deploy index.js") + ).rejects.toThrowErrorMatchingInlineSnapshot( + `[Error: You need 'containers:write', try logging in again or creating an appropiate API token]` + ); + }); + + it("should be able to enable observability logs", async () => { + mockGetVersion("Galaxy-Class"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + observability: { enabled: true }, + containers: [DEFAULT_CONTAINER_FROM_REGISTRY], + }); + + mockGetApplications([ + { + id: "abc", + name: "my-container", + instances: 0, + max_instances: 10, + created_at: new Date().toString(), + version: 1, + account_id: "1", + scheduling_policy: SchedulingPolicy.DEFAULT, + configuration: { + image: "docker.io/hello:world", + disk: { + size: "2GB", + size_mb: 2000, + }, + vcpu: 0.0625, + memory: "256MB", + memory_mib: 256, + }, + constraints: { + tier: 1, + }, + durable_objects: { + namespace_id: "1", + }, + }, + ]); + + mockModifyApplication({ + configuration: { + image: "docker.io/hello:world", + observability: { logs: { enabled: true } }, + }, + }); + + mockCreateApplicationRollout({ + description: "Progressive update", + strategy: "rolling", + kind: "full_auto", + }); + + await runWrangler("deploy index.js"); + + expect(std.out).toMatchInlineSnapshot(` + "Total Upload: xx KiB / gzip: xx KiB + Worker Startup Time: 100 ms + Your Worker has access to the following bindings: + Binding Resource + env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object + + Uploaded test-name (TIMINGS) + Deployed test-name triggers (TIMINGS) + https://test-name.test-sub-domain.workers.dev + Current Version ID: Galaxy-Class" + `); + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + }); + + it("should be able to deploy with instance_type configuration", async () => { + mockGetVersion("Galaxy-Class"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + containers: [ + { + name: "my-container", + max_instances: 10, + class_name: "ExampleDurableObject", + image: "docker.io/hello:world", + instance_type: "standard" as const, + constraints: { + tier: 2, + }, + }, + ], + }); + + mockGetApplications([]); + + mockCreateApplication({ + name: "my-container", + max_instances: 10, + scheduling_policy: SchedulingPolicy.DEFAULT, + configuration: { + image: "docker.io/hello:world", + instance_type: "standard" as InstanceType.STANDARD, + }, + constraints: { + tier: 2, + }, + }); + + await runWrangler("deploy index.js"); + + expect(std.out).toMatchInlineSnapshot(` + "Total Upload: xx KiB / gzip: xx KiB + Worker Startup Time: 100 ms + Your Worker has access to the following bindings: + Binding Resource + env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object + + Uploaded test-name (TIMINGS) + Deployed test-name triggers (TIMINGS) + https://test-name.test-sub-domain.workers.dev + Current Version ID: Galaxy-Class" + `); + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + }); + + it("should expand image names from managed registry", async () => { + mockGetVersion("Galaxy-Class"); + const registry = getCloudflareContainerRegistry(); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + containers: [ + { + name: "my-container", + max_instances: 10, + class_name: "ExampleDurableObject", + image: `${registry}/hello:1.0`, + instance_type: "standard" as const, + constraints: { + tier: 2, + }, + }, + ], + }); + + mockGetApplications([]); + + mockCreateApplication({ + name: "my-container", + max_instances: 10, + configuration: { + image: `${registry}/some-account-id/hello:1.0`, + }, + }); + + await runWrangler("deploy index.js"); + + expect(std.out).toMatchInlineSnapshot(` + "Total Upload: xx KiB / gzip: xx KiB + Worker Startup Time: 100 ms + Your Worker has access to the following bindings: + Binding Resource + env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object + + Uploaded test-name (TIMINGS) + Deployed test-name triggers (TIMINGS) + https://test-name.test-sub-domain.workers.dev + Current Version ID: Galaxy-Class" + `); + expect(std.err).toMatchInlineSnapshot(`""`); + expect(std.warn).toMatchInlineSnapshot(`""`); + }); +}); + +// This is a separate describe block because we intentionally do not mock any +// API tokens, account ID, or authorization. The purpose of these tests is to +// ensure that --dry-run mode works without requiring any API access. +describe("wrangler deploy with containers dry run", () => { + runInTempDir(); + const std = mockConsoleMethods(); + const cliStd = mockCLIOutput(); + beforeEach(() => { + clearCachedAccount(); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it("builds the image without pushing", async () => { + // Reduced mock chain for dry run (no delete, modified push) + vi.mocked(spawn) + .mockImplementationOnce(mockDockerInfo()) + .mockImplementationOnce( + mockDockerBuild("my-container", "worker", "FROM scratch", process.cwd()) + ) + .mockImplementationOnce(mockDockerImageInspect("my-container", "worker")) + .mockImplementationOnce(mockDockerLogin("mockpassword")) + .mockImplementationOnce(mockDockerManifestInspect("my-container", true)) + .mockImplementationOnce(mockDockerPush("my-container", "worker")); + + vi.stubEnv("WRANGLER_DOCKER_BIN", "/usr/bin/docker"); + fs.writeFileSync("./Dockerfile", "FROM scratch"); + fs.writeFileSync( + "index.js", + `export class ExampleDurableObject {}; export default{};` + ); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + containers: [DEFAULT_CONTAINER_FROM_DOCKERFILE], + }); + + await runWrangler("deploy --dry-run index.js"); + expect(std.out).toMatchInlineSnapshot(` + "Total Upload: xx KiB / gzip: xx KiB + Building image my-container:worker + Your Worker has access to the following bindings: + Binding Resource + env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object + + --dry-run: exiting now." + `); + expect(cliStd.stdout).toMatchInlineSnapshot(`""`); + }); +}); + +// Docker mock factory +function createDockerMockChain( + containerName: string, + tag: string, + dockerfilePath?: string, + buildContext?: string +) { + const mocks = [ + mockDockerInfo(), + mockDockerBuild( + containerName, + tag, + dockerfilePath || "FROM scratch", + buildContext || process.cwd() + ), + mockDockerImageInspect(containerName, tag), + mockDockerLogin("mockpassword"), + mockDockerManifestInspect("some-account-id/" + containerName, true), + mockDockerTag(containerName, "some-account-id/" + containerName, tag), + mockDockerPush("some-account-id/" + containerName, tag), + mockDockerImageDelete("some-account-id/" + containerName, tag), + ]; + + return mocks; +} + +function setupDockerMocks( + containerName: string, + tag: string, + dockerfilePath?: string, + buildContext?: string +) { + const mocks = createDockerMockChain( + containerName, + tag, + dockerfilePath, + buildContext + ); + vi.mocked(spawn) + .mockImplementationOnce(mocks[0]) + .mockImplementationOnce(mocks[1]) + .mockImplementationOnce(mocks[2]) + .mockImplementationOnce(mocks[3]) + .mockImplementationOnce(mocks[4]) + .mockImplementationOnce(mocks[5]) + .mockImplementationOnce(mocks[6]) + .mockImplementationOnce(mocks[7]); +} + +// Common test setup +function setupCommonMocks() { + msw.use(...mswSuccessDeploymentScriptMetadata); + msw.use(...mswListNewDeploymentsLatestFull); + mockSubDomainRequest(); + mockLegacyScriptData({ + scripts: [{ id: "test-name", migration_tag: "v1" }], + }); + mockContainersAccount(); + mockUploadWorkerRequest({ + expectedBindings: [ + { + class_name: "ExampleDurableObject", + name: "EXAMPLE_DO_BINDING", + type: "durable_object_namespace", + }, + ], + useOldUploadApi: true, + expectedContainers: [{ class_name: "ExampleDurableObject" }], + }); +} + +const DEFAULT_DURABLE_OBJECTS = { + durable_objects: { + bindings: [ + { + name: "EXAMPLE_DO_BINDING", + class_name: "ExampleDurableObject", + }, + ], + }, + migrations: [{ tag: "v1", new_sqlite_classes: ["ExampleDurableObject"] }], +}; + +const DEFAULT_CONTAINER_FROM_REGISTRY = { + name: "my-container", + max_instances: 10, + class_name: "ExampleDurableObject", + image: "docker.io/hello:world", +}; +const DEFAULT_CONTAINER_FROM_DOCKERFILE = { + name: "my-container", + max_instances: 10, + class_name: "ExampleDurableObject", + image: "./Dockerfile", +}; + +const defaultDOBinding = { + type: "durable_object_namespace", + namespace_id: "1", + class_name: "ExampleDurableObject", +}; +function mockGetVersion(versionId: string, bindings = [defaultDOBinding]) { + msw.use( + http.get( + `*/accounts/:accountId/workers/scripts/:scriptName/versions/${versionId}`, + async () => { + return HttpResponse.json( + createFetchResult({ + id: versionId, + metadata: {}, + number: 2, + resources: { + bindings, + }, + }) + ); + } + ) + ); +} + +function defaultChildProcess() { + return { + stderr: Buffer.from([]), + stdout: Buffer.from("i promise I am a successful process"), + on: function (reason: string, cbPassed: (code: number) => unknown) { + if (reason === "close") { + cbPassed(0); + } + + return this; + }, + } as unknown as ChildProcess; +} + +function mockCreateApplication(expected?: Partial) { + msw.use( + http.post("*/applications", async ({ request }) => { + const json = await request.json(); + + if (expected !== undefined) { + expect(json).toMatchObject(expected); + } + + return HttpResponse.json({ success: true, result: json }); + }) + ); +} + +function mockModifyApplication(expected?: Partial) { + msw.use( + http.patch("*/applications/:id", async ({ request }) => { + const json = await request.json(); + if (expected !== undefined) { + // console.dir("expected" + JSON.stringify(expected, null, 2)); + // console.dir("json" + JSON.stringify(json, null, 2)); + expect(json).toMatchObject(expected); + } + expect((json as CreateApplicationRequest).name).toBeUndefined(); + return HttpResponse.json({ + success: true, + result: { + id: "abc", + name: "my-container", + ...(json as Record), + }, + }); + }) + ); +} + +function mockCreateApplicationRollout(expected?: Record) { + msw.use( + http.post("*/applications/:id/rollouts", async ({ request }) => { + const json = await request.json(); + if (expected !== undefined) { + // console.dir("expected" + JSON.stringify(expected, null, 2)); + // console.dir("json" + JSON.stringify(json, null, 2)); + expect(json).toMatchObject(expected); + } + return HttpResponse.json({ + success: true, + result: { + id: "rollout-123", + status: "pending", + ...(json as Record), + }, + }); + }) + ); +} + +function mockGenerateImageRegistryCredentials() { + msw.use( + http.post( + `*/registries/${getCloudflareContainerRegistry()}/credentials`, + async ({ request }) => { + const json = + (await request.json()) as ImageRegistryCredentialsConfiguration; + expect(json.permissions).toEqual(["push", "pull"]); + + return HttpResponse.json({ + success: true, + result: { + account_id: "some-account-id", + registry_host: getCloudflareContainerRegistry(), + username: "v1", + password: "mockpassword", + } as AccountRegistryToken, + }); + }, + { once: true } + ) + ); +} +function mockGetApplications(applications: Application[]) { + msw.use( + http.get("*/applications", async () => { + return HttpResponse.json({ success: true, result: applications }); + }) + ); +} + +function mockDockerInfo() { + return (cmd: string, args: readonly string[]) => { + expect(cmd).toBe("/usr/bin/docker"); + expect(args).toEqual(["info"]); + return defaultChildProcess(); + }; +} + +function mockDockerBuild( + containerName: string, + tag: string, + expectedDockerfile: string, + buildContext: string +) { + return (cmd: string, args: readonly string[]) => { + expect(cmd).toBe("/usr/bin/docker"); + expect(args).toEqual([ + "build", + "-t", + `${getCloudflareContainerRegistry()}/${containerName}:${tag}`, + "--platform", + "linux/amd64", + "--provenance=false", + "-f", + "-", + buildContext, + ]); + + let dockerfile = ""; + const readable = new Writable({ + write(chunk) { + dockerfile += chunk; + }, + final() {}, + }); + return { + pid: -1, + error: undefined, + stderr: Buffer.from([]), + stdout: Buffer.from("i promise I am a successful docker build"), + stdin: readable, + status: 0, + signal: null, + output: [null], + on: (reason: string, cbPassed: (code: number) => unknown) => { + if (reason === "exit") { + expect(dockerfile).toEqual(expectedDockerfile); + cbPassed(0); + } + }, + } as unknown as ChildProcess; + }; +} + +function mockDockerImageInspect(containerName: string, tag: string) { + return (cmd: string, args: readonly string[]) => { + expect(cmd).toBe("/usr/bin/docker"); + expect(args).toEqual([ + "image", + "inspect", + `${getCloudflareContainerRegistry()}/${containerName}:${tag}`, + "--format", + "{{ .Size }} {{ len .RootFS.Layers }} {{json .RepoDigests}}", + ]); + + const stdout = new PassThrough(); + const stderr = new PassThrough(); + + const child = { + stdout, + stderr, + on(event: string, cb: (code: number) => void) { + if (event === "close") { + setImmediate(() => cb(0)); + } + return this; + }, + }; + + setImmediate(() => { + stdout.emit( + "data", + `123456 4 ["${getCloudflareContainerRegistry()}/${containerName}@sha256:three"]` + ); + }); + + return child as unknown as ChildProcess; + }; +} + +function mockDockerImageDelete(containerName: string, tag: string) { + return (cmd: string, args: readonly string[]) => { + expect(cmd).toBe("/usr/bin/docker"); + expect(args).toEqual([ + "image", + "rm", + `${getCloudflareContainerRegistry()}/${containerName}:${tag}`, + ]); + return defaultChildProcess(); + }; +} + +function mockDockerLogin(expectedPassword: string) { + return (cmd: string, _args: readonly string[]) => { + expect(cmd).toBe("/usr/bin/docker"); + let password = ""; + const readable = new Writable({ + write(chunk) { + password += chunk; + }, + final() {}, + }); + return { + stdout: Buffer.from("i promise I am a successful docker login"), + stdin: readable, + on: function (reason: string, cbPassed: (code: number) => unknown) { + if (reason === "close") { + expect(password).toEqual(expectedPassword); + cbPassed(0); + } + return this; + }, + } as unknown as ChildProcess; + }; +} + +function mockDockerManifestInspect(containerName: string, shouldFail = true) { + return (cmd: string, args: readonly string[]) => { + expect(cmd).toBe("/usr/bin/docker"); + expect(args[0]).toBe("manifest"); + expect(args[1]).toBe("inspect"); + expect(args[2]).toEqual(`${containerName}@three`); + expect(args).toEqual([ + "manifest", + "inspect", + `${getCloudflareContainerRegistry()}/${containerName}@three`, + ]); + const readable = new Writable({ + write() {}, + final() {}, + }); + return { + stdout: Buffer.from( + "i promise I am an unsuccessful docker manifest call" + ), + stdin: readable, + on: function (reason: string, cbPassed: (code: number) => unknown) { + if (reason === "close") { + cbPassed(shouldFail ? 1 : 0); + } + return this; + }, + } as unknown as ChildProcess; + }; +} + +function mockDockerPush(containerName: string, tag: string) { + return (cmd: string, args: readonly string[]) => { + expect(cmd).toBe("/usr/bin/docker"); + expect(args).toEqual([ + "push", + `${getCloudflareContainerRegistry()}/${containerName}:${tag}`, + ]); + return defaultChildProcess(); + }; +} + +function mockDockerTag(from: string, to: string, tag: string) { + return (cmd: string, args: readonly string[]) => { + expect(cmd).toBe("/usr/bin/docker"); + expect(args).toEqual([ + "tag", + `${getCloudflareContainerRegistry()}/${from}:${tag}`, + `${getCloudflareContainerRegistry()}/${to}:${tag}`, + ]); + return defaultChildProcess(); + }; +} diff --git a/packages/wrangler/src/__tests__/helpers/normalize.ts b/packages/wrangler/src/__tests__/helpers/normalize.ts index 4520dcb34fc2..cdf100bfb9bd 100644 --- a/packages/wrangler/src/__tests__/helpers/normalize.ts +++ b/packages/wrangler/src/__tests__/helpers/normalize.ts @@ -14,7 +14,9 @@ export function normalizeString(input: string): string { replaceByte( stripTrailingWhitespace( normalizeSlashes( - normalizeCwd(normalizeTempDirs(stripTimings(input))) + normalizeCwd( + normalizeTempDirs(stripTimings(replaceThinSpaces(input))) + ) ) ) ) @@ -95,3 +97,10 @@ function replaceByte(stdout: string): string { function normalizeTempDirs(stdout: string): string { return stdout.replaceAll(/\/\/.+\/tmp.+/g, "//tmpdir"); } + +/** + * Replace thin space characters (U+200A) with regular spaces to normalize output + */ +function replaceThinSpaces(str: string): string { + return str.replaceAll(/\u200a/g, " "); +} From b4753be6c984ba1ee6228ffd5aa871e7b3d668fa Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Mon, 14 Jul 2025 21:13:49 +0100 Subject: [PATCH 06/11] fix up loop in build command and config handling in ensureDiskSize --- .../src/__tests__/cloudchamber/build.test.ts | 43 ++------------- packages/wrangler/src/cloudchamber/build.ts | 52 +++++++++---------- packages/wrangler/src/cloudchamber/common.ts | 14 ----- 3 files changed, 28 insertions(+), 81 deletions(-) diff --git a/packages/wrangler/src/__tests__/cloudchamber/build.test.ts b/packages/wrangler/src/__tests__/cloudchamber/build.test.ts index f18263e8096d..79f2e5feb88f 100644 --- a/packages/wrangler/src/__tests__/cloudchamber/build.test.ts +++ b/packages/wrangler/src/__tests__/cloudchamber/build.test.ts @@ -7,8 +7,6 @@ import { runDockerCmd, } from "@cloudflare/containers-shared"; import { ensureDiskLimits } from "../../cloudchamber/build"; -import { resolveAppDiskSize } from "../../cloudchamber/common"; -import { type ContainerApp } from "../../config/environment"; import { UserError } from "../../errors"; import { mockAccountId, mockApiToken } from "../helpers/mock-account-id"; import { runInTempDir } from "../helpers/run-in-tmp"; @@ -17,12 +15,7 @@ import { mockAccountV4 as mockAccount } from "./utils"; import type { CompleteAccountCustomer } from "@cloudflare/containers-shared"; const MiB = 1024 * 1024; -const defaultConfiguration: ContainerApp = { - name: "abc", - class_name: "", - instances: 0, - image: "", -}; + vi.mock("@cloudflare/containers-shared", async (importOriginal) => { const actual = await importOriginal(); return Object.assign({}, actual, { @@ -255,13 +248,7 @@ describe("buildAndMaybePush", () => { ensureDiskLimits({ requiredSize: 333 * MiB, // 333MiB account: accountBase, - containerApp: { - ...defaultConfiguration, - configuration: { - image: "", - disk: { size: "3GB" }, // This exceeds the account limit of 2GB - }, - }, + configDiskSize: 3000 * MiB, // This exceeds the account limit of 2GB }) ).rejects.toThrow("Exceeded account limits"); }); @@ -271,7 +258,7 @@ describe("buildAndMaybePush", () => { ensureDiskLimits({ requiredSize: 3000 * MiB, // 3GiB account: accountBase, - containerApp: undefined, + configDiskSize: undefined, }) ).rejects.toThrow("Image too large"); }); @@ -280,32 +267,10 @@ describe("buildAndMaybePush", () => { const result = await ensureDiskLimits({ requiredSize: 256 * MiB, // 256MiB account: accountBase, - containerApp: undefined, + configDiskSize: undefined, }); expect(result).toEqual(undefined); }); }); - - describe("resolveAppDiskSize", () => { - it("should return parsed app disk size", () => { - const result = resolveAppDiskSize({ - ...defaultConfiguration, - configuration: { image: "", disk: { size: "500MB" } }, - }); - expect(result).toBeCloseTo(500 * 1000 * 1000, -5); - }); - - it("should return default size when disk size not set", () => { - const result = resolveAppDiskSize({ - ...defaultConfiguration, - configuration: { image: "" }, - }); - expect(result).toBeCloseTo(2 * 1000 * 1000 * 1000, -5); - }); - - it("should return undefined if app is not passed", () => { - expect(resolveAppDiskSize(undefined)).toBeUndefined(); - }); - }); }); diff --git a/packages/wrangler/src/cloudchamber/build.ts b/packages/wrangler/src/cloudchamber/build.ts index 349e48d0749f..b75ec7b5a7ce 100644 --- a/packages/wrangler/src/cloudchamber/build.ts +++ b/packages/wrangler/src/cloudchamber/build.ts @@ -18,10 +18,8 @@ import { import { UserError } from "../errors"; import { logger } from "../logger"; import { getAccountId } from "../user"; -import { resolveAppDiskSize } from "./common"; import { loadAccount } from "./locations"; import type { Config } from "../config"; -import type { ContainerApp } from "../config/environment"; import type { CommonYargsArgv, StrictYargsOptionsToInterface, @@ -80,7 +78,7 @@ export async function buildAndMaybePush( args: BuildArgs, pathToDocker: string, push: boolean, - containerConfig?: ContainerApp + configDiskSize: number | undefined ): Promise<{ image: string; pushed: boolean }> { try { const imageTag = `${getCloudflareContainerRegistry()}/${args.tag}`; @@ -122,7 +120,7 @@ export async function buildAndMaybePush( await ensureDiskLimits({ requiredSize, account, - containerApp: containerConfig, + configDiskSize, }); await dockerLoginManagedRegistry(pathToDocker); @@ -214,8 +212,7 @@ export async function buildAndMaybePush( } export async function buildCommand( - args: StrictYargsOptionsToInterface, - config: Config + args: StrictYargsOptionsToInterface ) { // TODO: merge args with Wrangler config if available if (existsSync(args.PATH) && !isDir(args.PATH)) { @@ -223,23 +220,23 @@ export async function buildCommand( `${args.PATH} is not a directory. Please specify a valid directory path.` ); } - // if containers are not defined, the build should still work. - const containers = config.containers ?? [undefined]; + const pathToDockerfile = join(args.PATH, "Dockerfile"); - for (const container of containers) { - await buildAndMaybePush( - { - tag: args.tag, - pathToDockerfile, - buildContext: args.PATH, - platform: args.platform, - // no option to add env vars at build time...? - }, - getDockerPath() ?? args.pathToDocker, - args.push, - container - ); - } + + await buildAndMaybePush( + { + tag: args.tag, + pathToDockerfile, + buildContext: args.PATH, + platform: args.platform, + // no option to add env vars at build time...? + }, + getDockerPath() ?? args.pathToDocker, + args.push, + // this means we wont be able to read the disk size from the config, but that option is deprecated anyway at least for containers. + // and this never actually worked for cloudchamber as this command was previously reading it from config.containers not config.cloudchamber + undefined + ); } export async function pushCommand( @@ -269,23 +266,22 @@ export async function pushCommand( export async function ensureDiskLimits(options: { requiredSize: number; account: CompleteAccountCustomer; - containerApp: ContainerApp | undefined; + configDiskSize: number | undefined; }): Promise { const MB = 1000 * 1000; const MiB = 1024 * 1024; - const appDiskSize = resolveAppDiskSize(options.containerApp); const accountDiskSize = (options.account.limits.disk_mb_per_deployment ?? 2000) * MB; // if appDiskSize is defined and configured to be more than the accountDiskSize, error - if (appDiskSize && appDiskSize > accountDiskSize) { + if (options.configDiskSize && options.configDiskSize > accountDiskSize) { throw new UserError( - `Exceeded account limits: Your container is configured to use a disk size of ${appDiskSize / MB} MB. However, that exceeds the account limit of ${accountDiskSize / MB}` + `Exceeded account limits: Your container is configured to use a disk size of ${options.configDiskSize / MB} MB. However, that exceeds the account limit of ${accountDiskSize / MB}` ); } - const maxAllowedImageSizeBytes = appDiskSize ?? accountDiskSize; + const maxAllowedImageSizeBytes = options.configDiskSize ?? accountDiskSize; logger.debug( - `Disk size limits when building the container: appDiskSize:${appDiskSize}, accountDiskSize:${accountDiskSize}, maxAllowedImageSizeBytes=${maxAllowedImageSizeBytes}(${maxAllowedImageSizeBytes / MB} MB), requiredSized=${options.requiredSize}(${Math.ceil(options.requiredSize / MiB)}MiB)` + `Disk size limits when building the container: appDiskSize:${options.configDiskSize}, accountDiskSize:${accountDiskSize}, maxAllowedImageSizeBytes=${maxAllowedImageSizeBytes}(${maxAllowedImageSizeBytes / MB} MB), requiredSized=${options.requiredSize}(${Math.ceil(options.requiredSize / MiB)}MiB)` ); if (maxAllowedImageSizeBytes < options.requiredSize) { throw new UserError( diff --git a/packages/wrangler/src/cloudchamber/common.ts b/packages/wrangler/src/cloudchamber/common.ts index 81f9fcee2362..793919916613 100644 --- a/packages/wrangler/src/cloudchamber/common.ts +++ b/packages/wrangler/src/cloudchamber/common.ts @@ -616,20 +616,6 @@ export function resolveMemory( return undefined; } -// Return the amount of disk size in (MB) for an application, falls back to the account limits if the app config doesn't exist -// sometimes the user wants to just build a container here, we should allow checking those based on the account limits if -// app.configuration is not set -// ordering: app.configuration.disk.size -> account.limits.disk_mb_per_deployment -> default fallback to 2GB in bytes -export function resolveAppDiskSize( - app: ContainerApp | undefined -): number | undefined { - if (app === undefined) { - return undefined; - } - const disk = app.configuration?.disk?.size ?? "2GB"; - return Math.round(parseByteSize(disk)); -} - // Checks that instance type is one of 'dev', 'basic', or 'standard' and that it is not being set alongside memory or vcpu. // Returns the instance type to use if correctly set. export function checkInstanceType( From 703c0c25627a97efe3851c3679c5c4e68c183d40 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Tue, 15 Jul 2025 13:32:16 +0100 Subject: [PATCH 07/11] stray fixups for cloudchamber apply --- .../src/__tests__/cloudchamber/apply.test.ts | 216 ++++-------------- packages/wrangler/src/cloudchamber/apply.ts | 15 +- packages/wrangler/src/cloudchamber/common.ts | 6 +- 3 files changed, 53 insertions(+), 184 deletions(-) diff --git a/packages/wrangler/src/__tests__/cloudchamber/apply.test.ts b/packages/wrangler/src/__tests__/cloudchamber/apply.test.ts index f6b6bab024c2..b8add5235685 100644 --- a/packages/wrangler/src/__tests__/cloudchamber/apply.test.ts +++ b/packages/wrangler/src/__tests__/cloudchamber/apply.test.ts @@ -80,12 +80,6 @@ function mockModifyApplication( } describe("cloudchamber apply", () => { - /* eslint no-irregular-whitespace: ["error", { "skipTemplates": true }] - --- - Wrangler emits \u200a instead of "regular" whitespace in some cases. eslint doesn't like - this so we disable the warning when mixed whitespace is used in template strings. - */ - const { setIsTTY } = useMockIsTTY(); const std = mockCLIOutput(); @@ -123,7 +117,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ NEW my-container-app + ├ NEW my-container-app │ │ [[containers]] │ name = \\"my-container-app\\" @@ -138,7 +132,7 @@ describe("cloudchamber apply", () => { │ instance_type = \\"dev\\" │ │ - │  SUCCESS  Created application my-container-app (Application ID: abc) + │ SUCCESS Created application my-container-app (Application ID: abc) │ ╰ Applied changes @@ -193,7 +187,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [[containers]] │ - instances = 3 @@ -205,7 +199,7 @@ describe("cloudchamber apply", () => { │ + tier = 2 │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ ╰ Applied changes @@ -271,7 +265,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [[containers]] │ instances = 0 @@ -279,7 +273,7 @@ describe("cloudchamber apply", () => { │ + max_instances = 3 │ name = \\"my-container-app\\" │ - ├ NEW my-container-app-2 + ├ NEW my-container-app-2 │ │ [[containers]] │ name = \\"my-container-app-2\\" @@ -294,10 +288,10 @@ describe("cloudchamber apply", () => { │ tier = 1 │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ │ - │  SUCCESS  Created application my-container-app-2 (Application ID: abc) + │ SUCCESS Created application my-container-app-2 (Application ID: abc) │ ╰ Applied changes @@ -358,7 +352,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [[containers]] │ - instances = 3 @@ -366,7 +360,7 @@ describe("cloudchamber apply", () => { │ name = \\"my-container-app\\" │ Skipping application rollout │ - ├ NEW my-container-app-2 + ├ NEW my-container-app-2 │ │ [[containers]] │ name = \\"my-container-app-2\\" @@ -381,7 +375,7 @@ describe("cloudchamber apply", () => { │ tier = 1 │ │ - │  SUCCESS  Created application my-container-app-2 (Application ID: abc) + │ SUCCESS Created application my-container-app-2 (Application ID: abc) │ ╰ Applied changes @@ -442,14 +436,14 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [[containers]] │ - instances = 3 │ + instances = 4 │ name = \\"my-container-app\\" │ - ├ NEW my-container-app-2 + ├ NEW my-container-app-2 │ │ [[containers]] │ name = \\"my-container-app-2\\" @@ -464,10 +458,10 @@ describe("cloudchamber apply", () => { │ tier = 1 │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ │ - │  SUCCESS  Created application my-container-app-2 (Application ID: abc) + │ SUCCESS Created application my-container-app-2 (Application ID: abc) │ ╰ Applied changes @@ -576,7 +570,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [[containers]] │ - instances = 3 @@ -599,7 +593,7 @@ describe("cloudchamber apply", () => { │ name = \\"MY_SECRET_2\\" │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ ╰ Applied changes @@ -708,7 +702,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ no changes my-container-app + ├ no changes my-container-app │ ╰ No changes to be made @@ -820,9 +814,9 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ no changes my-container-app + ├ no changes my-container-app │ - ├ no changes my-container-app-2 + ├ no changes my-container-app-2 │ ╰ No changes to be made @@ -931,121 +925,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ no changes my-container-app - │ - ╰ No changes to be made - - " - `); - expect(std.stderr).toMatchInlineSnapshot(`""`); - }); - - test("can apply an application, and there is no changes (two applications)", async () => { - setIsTTY(false); - const app = { - name: "my-container-app", - instances: 3, - class_name: "DurableObjectClass", - image: "./Dockerfile", - configuration: { - labels: [ - { - name: "name", - value: "value", - }, - { - name: "name-2", - value: "value-2", - }, - ], - secrets: [ - { - name: "MY_SECRET", - type: SecretAccessType.ENV, - secret: "SECRET_NAME", - }, - { - name: "MY_SECRET_1", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_1", - }, - { - name: "MY_SECRET_2", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_2", - }, - ], - }, - }; - writeWranglerConfig({ - name: "my-container", - containers: [app, { ...app, name: "my-container-app-2" }], - }); - - const completeApp = { - id: "abc", - name: "my-container-app", - instances: 3, - created_at: new Date().toString(), - class_name: "DurableObjectClass", - account_id: "1", - scheduling_policy: SchedulingPolicy.REGIONAL, - configuration: { - image: "./Dockerfile", - labels: [ - { - name: "name", - value: "value", - }, - { - name: "name-2", - value: "value-2", - }, - ], - secrets: [ - { - name: "MY_SECRET", - type: SecretAccessType.ENV, - secret: "SECRET_NAME", - }, - { - name: "MY_SECRET_1", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_1", - }, - { - name: "MY_SECRET_2", - type: SecretAccessType.ENV, - secret: "SECRET_NAME_2", - }, - ], - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, - }, - - constraints: { - tier: 1, - }, - }; - - mockGetApplications([ - { ...completeApp, version: 1 }, - { ...completeApp, version: 1, name: "my-container-app-2", id: "abc2" }, - ]); - await runWrangler("cloudchamber apply"); - expect(std.stdout).toMatchInlineSnapshot(` - "╭ Deploy a container application deploy changes to your application - │ - │ Container application changes - │ - ├ no changes my-container-app - │ - ├ no changes my-container-app-2 + ├ no changes my-container-app │ ╰ No changes to be made @@ -1099,7 +979,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [containers.configuration] │ ... @@ -1112,7 +992,7 @@ describe("cloudchamber apply", () => { │ ... │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ ╰ Applied changes @@ -1169,7 +1049,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [containers.configuration] │ ... @@ -1182,7 +1062,7 @@ describe("cloudchamber apply", () => { │ ... │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ ╰ Applied changes @@ -1244,7 +1124,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [containers.configuration.observability.logs] │ - enabled = true @@ -1254,7 +1134,7 @@ describe("cloudchamber apply", () => { │ ... │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ ╰ Applied changes @@ -1316,7 +1196,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [containers.configuration.observability.logs] │ - enabled = true @@ -1326,7 +1206,7 @@ describe("cloudchamber apply", () => { │ ... │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ ╰ Applied changes @@ -1387,7 +1267,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [containers.configuration.observability.logs] │ - enabled = true @@ -1397,7 +1277,7 @@ describe("cloudchamber apply", () => { │ ... │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ ╰ Applied changes @@ -1461,7 +1341,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [containers.configuration.observability.logs] │ - enabled = true @@ -1471,7 +1351,7 @@ describe("cloudchamber apply", () => { │ ... │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ ╰ Applied changes @@ -1535,7 +1415,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ no changes my-container-app + ├ no changes my-container-app │ ╰ No changes to be made @@ -1587,7 +1467,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ no changes my-container-app + ├ no changes my-container-app │ ╰ No changes to be made @@ -1647,7 +1527,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ no changes my-container-app + ├ no changes my-container-app │ ╰ No changes to be made @@ -1681,7 +1561,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ NEW my-container-app + ├ NEW my-container-app │ │ [[containers]] │ name = \\"my-container-app\\" @@ -1696,7 +1576,7 @@ describe("cloudchamber apply", () => { │ instance_type = \\"dev\\" │ │ - │  SUCCESS  Created application my-container-app (Application ID: abc) + │ SUCCESS Created application my-container-app (Application ID: abc) │ ╰ Applied changes @@ -1753,7 +1633,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [[containers]] │ - instances = 3 @@ -1771,7 +1651,7 @@ describe("cloudchamber apply", () => { │ + tier = 2 │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ ╰ Applied changes @@ -1829,7 +1709,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [[containers]] │ - instances = 3 @@ -1847,7 +1727,7 @@ describe("cloudchamber apply", () => { │ + tier = 2 │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ ╰ Applied changes @@ -1893,7 +1773,7 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ NEW my-container-app + ├ NEW my-container-app │ │ [[containers]] │ name = \\"my-container-app\\" @@ -1904,11 +1784,11 @@ describe("cloudchamber apply", () => { │ tier = 2 │ │ [containers.configuration] - │ image = \\"${registry}/some-account-id/hello:1.0\\" + │ image = \\"registry.cloudflare.com/some-account-id/hello:1.0\\" │ instance_type = \\"dev\\" │ │ - │  SUCCESS  Created application my-container-app (Application ID: abc) + │ SUCCESS Created application my-container-app (Application ID: abc) │ ╰ Applied changes @@ -1967,10 +1847,10 @@ describe("cloudchamber apply", () => { │ │ Container application changes │ - ├ EDIT my-container-app + ├ EDIT my-container-app │ │ [containers.configuration] - │ image = \\"${registry}/some-account-id/hello:1.0\\" + │ image = \\"registry.cloudflare.com/some-account-id/hello:1.0\\" │ - instance_type = \\"dev\\" │ + instance_type = \\"standard\\" │ @@ -1980,7 +1860,7 @@ describe("cloudchamber apply", () => { │ + tier = 2 │ │ - │  SUCCESS  Modified application my-container-app + │ SUCCESS Modified application my-container-app │ ╰ Applied changes diff --git a/packages/wrangler/src/cloudchamber/apply.ts b/packages/wrangler/src/cloudchamber/apply.ts index 8226bf5c1bea..0552180e3e1d 100644 --- a/packages/wrangler/src/cloudchamber/apply.ts +++ b/packages/wrangler/src/cloudchamber/apply.ts @@ -29,6 +29,7 @@ import { cleanForInstanceType, promiseSpinner } from "./common"; import { diffLines, printLine, + renderDiff, sortObjectRecursive, stripUndefined, } from "./helpers/diff"; @@ -271,7 +272,6 @@ export async function apply( args: { skipDefaults: boolean | undefined; env?: string; - imageUpdateRequired?: boolean; }, config: Config ) { @@ -346,16 +346,6 @@ export async function apply( `${config.name}-${appConfigNoDefaults.class_name}` ]; - // while configuration.image is deprecated to the user, we still resolve to this for now. - if (!appConfigNoDefaults.configuration?.image && application) { - appConfigNoDefaults.configuration ??= {}; - } - - if (!args.imageUpdateRequired && application) { - appConfigNoDefaults.configuration ??= {}; - appConfigNoDefaults.configuration.image = application.configuration.image; - } - const accountId = config.account_id || (await getAccountId(config)); const appConfig = containerAppToCreateApplication( accountId, @@ -646,9 +636,6 @@ export async function applyCommand( { skipDefaults: args.skipDefaults, env: args.env, - // For the apply command we want this to default to true - // so that the image can be updated if the user modified it. - imageUpdateRequired: true, }, config ); diff --git a/packages/wrangler/src/cloudchamber/common.ts b/packages/wrangler/src/cloudchamber/common.ts index 793919916613..ec0ef2880741 100644 --- a/packages/wrangler/src/cloudchamber/common.ts +++ b/packages/wrangler/src/cloudchamber/common.ts @@ -684,8 +684,10 @@ export function inferInstanceType( } } -// removes any disk, memory, or vcpu that have been set in an objects configuration. Used for rendering -// diffs. +/** + * THIS IS ONLY USED FOR CLOUDCHAMBER APPLY + * removes any disk, memory, or vcpu that have been set in an objects configuration. Used for rendering diffs. + */ export function cleanForInstanceType( app: CreateApplicationRequest ): ContainerApp { From bb66e526dbc5331a2a162e0cdaa94e2ee0ff1446 Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Tue, 15 Jul 2025 13:44:38 +0100 Subject: [PATCH 08/11] pr feedback on instance_type --- packages/containers-shared/src/types.ts | 2 +- .../src/__tests__/containers/config.test.ts | 10 +++++--- packages/wrangler/src/cloudchamber/deploy.ts | 2 +- packages/wrangler/src/config/environment.ts | 13 +---------- packages/wrangler/src/config/validation.ts | 3 +-- packages/wrangler/src/containers/config.ts | 23 +++---------------- packages/wrangler/src/containers/deploy.ts | 4 ++-- 7 files changed, 16 insertions(+), 41 deletions(-) diff --git a/packages/containers-shared/src/types.ts b/packages/containers-shared/src/types.ts index 678ce751aa11..e2a1665f6f4c 100644 --- a/packages/containers-shared/src/types.ts +++ b/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; } diff --git a/packages/wrangler/src/__tests__/containers/config.test.ts b/packages/wrangler/src/__tests__/containers/config.test.ts index 942f276542d8..80c7bc96d8d0 100644 --- a/packages/wrangler/src/__tests__/containers/config.test.ts +++ b/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, }); diff --git a/packages/wrangler/src/cloudchamber/deploy.ts b/packages/wrangler/src/cloudchamber/deploy.ts index 857d169c6dca..44313b8d833a 100644 --- a/packages/wrangler/src/cloudchamber/deploy.ts +++ b/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) { diff --git a/packages/wrangler/src/config/environment.ts b/packages/wrangler/src/config/environment.ts index 960e31ab06e6..804a775b171a 100644 --- a/packages/wrangler/src/config/environment.ts +++ b/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; }; /** diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts index 64659e3e2a01..e7e941560142 100644 --- a/packages/wrangler/src/config/validation.ts +++ b/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.` diff --git a/packages/wrangler/src/containers/config.ts b/packages/wrangler/src/containers/config.ts index 43dd8439885a..2420b378821f 100644 --- a/packages/wrangler/src/containers/config.ts +++ b/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 = { diff --git a/packages/wrangler/src/containers/deploy.ts b/packages/wrangler/src/containers/deploy.ts index 4f0fd0b5e341..1a8909492c81 100644 --- a/packages/wrangler/src/containers/deploy.ts +++ b/packages/wrangler/src/containers/deploy.ts @@ -201,7 +201,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, }), @@ -518,7 +518,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, From bc6ffeb7b9126f5bba7801071de2520c1c4a2e9a Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Tue, 15 Jul 2025 17:16:13 +0100 Subject: [PATCH 09/11] add back in missing observability config resolution tests --- .../src/__tests__/containers/deploy.test.ts | 525 ++++++++++++++---- 1 file changed, 420 insertions(+), 105 deletions(-) diff --git a/packages/wrangler/src/__tests__/containers/deploy.test.ts b/packages/wrangler/src/__tests__/containers/deploy.test.ts index e270b0ba7b70..0bd6ebce7143 100644 --- a/packages/wrangler/src/__tests__/containers/deploy.test.ts +++ b/packages/wrangler/src/__tests__/containers/deploy.test.ts @@ -660,123 +660,420 @@ describe("wrangler deploy with containers", () => { ); }); - it("should be able to enable observability logs", async () => { - mockGetVersion("Galaxy-Class"); - writeWranglerConfig({ - ...DEFAULT_DURABLE_OBJECTS, - observability: { enabled: true }, - containers: [DEFAULT_CONTAINER_FROM_REGISTRY], + describe("observability config resolution", () => { + const sharedGetApplicationResult = { + id: "abc", + name: "my-container", + instances: 0, + max_instances: 10, + created_at: new Date().toString(), + version: 1, + account_id: "1", + scheduling_policy: SchedulingPolicy.DEFAULT, + configuration: { + image: "docker.io/hello:world", + disk: { + size: "2GB", + size_mb: 2000, + }, + vcpu: 0.0625, + memory: "256MB", + memory_mib: 256, + }, + constraints: { + tier: 1, + }, + durable_objects: { + namespace_id: "1", + }, + }; + it("should be able to enable observability logs (top level)", async () => { + mockGetVersion("Galaxy-Class"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + observability: { enabled: true }, + containers: [DEFAULT_CONTAINER_FROM_REGISTRY], + }); + + mockGetApplications([sharedGetApplicationResult]); + + mockModifyApplication({ + configuration: { + image: "docker.io/hello:world", + observability: { logs: { enabled: true } }, + }, + }); + mockCreateApplicationRollout(); + + await runWrangler("deploy index.js"); + + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ EDIT my-container + │ { + │ ... + │ \\"image\\": \\"docker.io/hello:world\\", + │ - \\"instance_type\\": \\"dev\\" + │ + \\"instance_type\\": \\"dev\\", + │ + \\"observability\\": { + │ + \\"logs\\": { + │ + \\"enabled\\": true + │ + } + │ + } + │ }, + │ + │ SUCCESS Modified application my-container + │ + ╰ Applied changes + + " + `); }); - mockGetApplications([ - { - id: "abc", - name: "my-container", - instances: 0, - max_instances: 10, - created_at: new Date().toString(), - version: 1, - account_id: "1", - scheduling_policy: SchedulingPolicy.DEFAULT, + it("should be able to enable observability logs (logs field)", async () => { + mockGetVersion("Galaxy-Class"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + observability: { logs: { enabled: true } }, + containers: [DEFAULT_CONTAINER_FROM_REGISTRY], + }); + + mockGetApplications([sharedGetApplicationResult]); + + mockModifyApplication({ configuration: { image: "docker.io/hello:world", - disk: { - size: "2GB", - size_mb: 2000, - }, - vcpu: 0.0625, - memory: "256MB", - memory_mib: 256, + observability: { logs: { enabled: true } }, }, - constraints: { - tier: 1, + }); + + mockCreateApplicationRollout(); + + await runWrangler("deploy index.js"); + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ EDIT my-container + │ { + │ ... + │ \\"image\\": \\"docker.io/hello:world\\", + │ - \\"instance_type\\": \\"dev\\" + │ + \\"instance_type\\": \\"dev\\", + │ + \\"observability\\": { + │ + \\"logs\\": { + │ + \\"enabled\\": true + │ + } + │ + } + │ }, + │ + │ SUCCESS Modified application my-container + │ + ╰ Applied changes + + " + `); + }); + + it("should be able to disable observability logs (top level)", async () => { + mockGetVersion("Galaxy-Class"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + observability: { enabled: false }, + containers: [DEFAULT_CONTAINER_FROM_REGISTRY], + }); + + mockGetApplications([ + { + ...sharedGetApplicationResult, + configuration: { + ...sharedGetApplicationResult.configuration, + observability: { + logs: { + enabled: true, + }, + }, + }, }, - durable_objects: { - namespace_id: "1", + ]); + + mockModifyApplication({ + configuration: { + image: "docker.io/hello:world", + observability: { logs: { enabled: false } }, }, - }, - ]); + }); - mockModifyApplication({ - configuration: { - image: "docker.io/hello:world", - observability: { logs: { enabled: true } }, - }, + mockCreateApplicationRollout(); + + await runWrangler("deploy index.js"); + + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ EDIT my-container + │ { + │ ... + │ \\"logs\\": { + │ - \\"enabled\\": true + │ + \\"enabled\\": false + │ } + │ + │ SUCCESS Modified application my-container + │ + ╰ Applied changes + + " + `); }); - mockCreateApplicationRollout({ - description: "Progressive update", - strategy: "rolling", - kind: "full_auto", - }); + it("should be able to disable observability logs (logs field)", async () => { + mockGetVersion("Galaxy-Class"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + observability: { logs: { enabled: false } }, + containers: [DEFAULT_CONTAINER_FROM_REGISTRY], + }); - await runWrangler("deploy index.js"); + mockGetApplications([ + { + ...sharedGetApplicationResult, + configuration: { + ...sharedGetApplicationResult.configuration, + observability: { + logs: { + enabled: true, + }, + }, + }, + }, + ]); - expect(std.out).toMatchInlineSnapshot(` - "Total Upload: xx KiB / gzip: xx KiB - Worker Startup Time: 100 ms - Your Worker has access to the following bindings: - Binding Resource - env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object + mockModifyApplication({ + configuration: { + image: "docker.io/hello:world", + observability: { logs: { enabled: false } }, + }, + }); - Uploaded test-name (TIMINGS) - Deployed test-name triggers (TIMINGS) - https://test-name.test-sub-domain.workers.dev - Current Version ID: Galaxy-Class" - `); - expect(std.err).toMatchInlineSnapshot(`""`); - expect(std.warn).toMatchInlineSnapshot(`""`); - }); + mockCreateApplicationRollout(); + + await runWrangler("deploy index.js"); + + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ EDIT my-container + │ { + │ ... + │ \\"logs\\": { + │ - \\"enabled\\": true + │ + \\"enabled\\": false + │ } + │ + │ SUCCESS Modified application my-container + │ + ╰ Applied changes + + " + `); + }); + it("should be able to disable observability logs (absent field)", async () => { + mockGetVersion("Galaxy-Class"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + containers: [DEFAULT_CONTAINER_FROM_REGISTRY], + }); - it("should be able to deploy with instance_type configuration", async () => { - mockGetVersion("Galaxy-Class"); - writeWranglerConfig({ - ...DEFAULT_DURABLE_OBJECTS, - containers: [ + mockGetApplications([ { - name: "my-container", - max_instances: 10, - class_name: "ExampleDurableObject", + ...sharedGetApplicationResult, + configuration: { + ...sharedGetApplicationResult.configuration, + observability: { + logs: { + enabled: true, + }, + }, + }, + }, + ]); + + mockModifyApplication({ + configuration: { image: "docker.io/hello:world", - instance_type: "standard" as const, - constraints: { - tier: 2, + observability: { logs: { enabled: false } }, + }, + }); + + mockCreateApplicationRollout(); + await runWrangler("deploy index.js"); + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ EDIT my-container + │ { + │ ... + │ \\"logs\\": { + │ - \\"enabled\\": true + │ + \\"enabled\\": false + │ } + │ + │ SUCCESS Modified application my-container + │ + ╰ Applied changes + + " + `); + }); + it("should ignore deprecated observability.logging field from the api", async () => { + mockGetVersion("Galaxy-Class"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + containers: [DEFAULT_CONTAINER_FROM_REGISTRY], + }); + + mockGetApplications([ + { + ...sharedGetApplicationResult, + configuration: { + ...sharedGetApplicationResult.configuration, + observability: { + logging: { + enabled: false, + }, + logs: { + enabled: true, + }, + }, }, }, - ], + ]); + + mockModifyApplication({ + configuration: { + image: "docker.io/hello:world", + observability: { logs: { enabled: false } }, + }, + }); + + mockCreateApplicationRollout(); + + await runWrangler("deploy index.js"); + + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ EDIT my-container + │ { + │ ... + │ \\"logs\\": { + │ - \\"enabled\\": true + │ + \\"enabled\\": false + │ } + │ + │ SUCCESS Modified application my-container + │ + ╰ Applied changes + + " + `); }); + it("should keep observability logs enabled", async () => { + mockGetVersion("Galaxy-Class"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + observability: { enabled: true }, + containers: [DEFAULT_CONTAINER_FROM_REGISTRY], + }); - mockGetApplications([]); + mockGetApplications([ + { + ...sharedGetApplicationResult, + configuration: { + ...sharedGetApplicationResult.configuration, + observability: { + logs: { + enabled: true, + }, + }, + }, + }, + ]); - mockCreateApplication({ - name: "my-container", - max_instances: 10, - scheduling_policy: SchedulingPolicy.DEFAULT, - configuration: { - image: "docker.io/hello:world", - instance_type: "standard" as InstanceType.STANDARD, - }, - constraints: { - tier: 2, - }, + mockModifyApplication({ + configuration: { + image: "docker.io/hello:world", + observability: { logs: { enabled: true } }, + }, + }); + + mockCreateApplicationRollout(); + + await runWrangler("deploy index.js"); + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ no changes my-container + │ + ╰ No changes to be made + + " + `); }); - await runWrangler("deploy index.js"); + it("should keep obserability logs disabled if api returns false and undefined in config", async () => { + mockGetVersion("Galaxy-Class"); + writeWranglerConfig({ + ...DEFAULT_DURABLE_OBJECTS, + containers: [DEFAULT_CONTAINER_FROM_REGISTRY], + }); - expect(std.out).toMatchInlineSnapshot(` - "Total Upload: xx KiB / gzip: xx KiB - Worker Startup Time: 100 ms - Your Worker has access to the following bindings: - Binding Resource - env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object + mockGetApplications([ + { + ...sharedGetApplicationResult, + configuration: { + ...sharedGetApplicationResult.configuration, + observability: { + logs: { + enabled: false, + }, + logging: { + enabled: false, + }, + }, + }, + }, + ]); - Uploaded test-name (TIMINGS) - Deployed test-name triggers (TIMINGS) - https://test-name.test-sub-domain.workers.dev - Current Version ID: Galaxy-Class" - `); - expect(std.err).toMatchInlineSnapshot(`""`); - expect(std.warn).toMatchInlineSnapshot(`""`); + await runWrangler("deploy index.js"); + + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ no changes my-container + │ + ╰ No changes to be made + + " + `); + }); }); it("should expand image names from managed registry", async () => { @@ -790,7 +1087,7 @@ describe("wrangler deploy with containers", () => { max_instances: 10, class_name: "ExampleDurableObject", image: `${registry}/hello:1.0`, - instance_type: "standard" as const, + instance_type: "standard", constraints: { tier: 2, }, @@ -805,25 +1102,43 @@ describe("wrangler deploy with containers", () => { max_instances: 10, configuration: { image: `${registry}/some-account-id/hello:1.0`, + instance_type: "standard" as InstanceType.STANDARD, }, }); await runWrangler("deploy index.js"); - expect(std.out).toMatchInlineSnapshot(` - "Total Upload: xx KiB / gzip: xx KiB - Worker Startup Time: 100 ms - Your Worker has access to the following bindings: - Binding Resource - env.EXAMPLE_DO_BINDING (ExampleDurableObject) Durable Object + expect(cliStd.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ NEW my-container + │ + │ { + │ \\"containers\\": [ + │ { + │ \\"name\\": \\"my-container\\", + │ \\"scheduling_policy\\": \\"default\\", + │ \\"configuration\\": { + │ \\"image\\": \\"registry.cloudflare.com/some-account-id/hello:1.0\\", + │ \\"instance_type\\": \\"standard\\" + │ }, + │ \\"instances\\": 0, + │ \\"max_instances\\": 10, + │ \\"constraints\\": { + │ \\"tier\\": 2 + │ } + │ } + │ ] + │ } + │ + │ SUCCESS Created application my-container (Application ID: undefined) + │ + ╰ Applied changes - Uploaded test-name (TIMINGS) - Deployed test-name triggers (TIMINGS) - https://test-name.test-sub-domain.workers.dev - Current Version ID: Galaxy-Class" + " `); - expect(std.err).toMatchInlineSnapshot(`""`); - expect(std.warn).toMatchInlineSnapshot(`""`); }); }); From a1ee29a2b58551c26212d86c5dacb44c7a9d6c8c Mon Sep 17 00:00:00 2001 From: emily-shen <69125074+emily-shen@users.noreply.github.com> Date: Tue, 15 Jul 2025 18:13:39 +0100 Subject: [PATCH 10/11] fix trailing comma causing unncessary diffs --- .../src/__tests__/containers/deploy.test.ts | 4 --- .../wrangler/src/cloudchamber/helpers/diff.ts | 27 +++++++++++++++++++ packages/wrangler/src/containers/deploy.ts | 4 ++- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/packages/wrangler/src/__tests__/containers/deploy.test.ts b/packages/wrangler/src/__tests__/containers/deploy.test.ts index 0bd6ebce7143..9ce6b3dec203 100644 --- a/packages/wrangler/src/__tests__/containers/deploy.test.ts +++ b/packages/wrangler/src/__tests__/containers/deploy.test.ts @@ -716,8 +716,6 @@ describe("wrangler deploy with containers", () => { │ { │ ... │ \\"image\\": \\"docker.io/hello:world\\", - │ - \\"instance_type\\": \\"dev\\" - │ + \\"instance_type\\": \\"dev\\", │ + \\"observability\\": { │ + \\"logs\\": { │ + \\"enabled\\": true @@ -762,8 +760,6 @@ describe("wrangler deploy with containers", () => { │ { │ ... │ \\"image\\": \\"docker.io/hello:world\\", - │ - \\"instance_type\\": \\"dev\\" - │ + \\"instance_type\\": \\"dev\\", │ + \\"observability\\": { │ + \\"logs\\": { │ + \\"enabled\\": true diff --git a/packages/wrangler/src/cloudchamber/helpers/diff.ts b/packages/wrangler/src/cloudchamber/helpers/diff.ts index d997f7250536..7db9dc836405 100644 --- a/packages/wrangler/src/cloudchamber/helpers/diff.ts +++ b/packages/wrangler/src/cloudchamber/helpers/diff.ts @@ -561,3 +561,30 @@ export const renderDiff = (results: Result[]) => { } } }; + +/** + * Filter out trailing comma differences that are just formatting changes. + * This prevents showing spurious diffs when JSON properties get trailing commas + * added/removed due to new properties being inserted. + */ +export function filterTrailingCommaChanges(results: Result[]): Result[] { + // First, identify pairs of removed/added lines that are just comma changes + const commaChangePairs = new Set(); + + results.forEach((result) => { + if (result.removed && result.value) { + // Look for a corresponding added line that's the same but with comma + const potentialMatch = results.find((r) => + r.added && r.value === result.value + ',' + ); + if (potentialMatch) { + // This is a trailing comma change, mark both for filtering + commaChangePairs.add(result); + commaChangePairs.add(potentialMatch); + } + } + }); + + // Filter out the identified comma change pairs + return results.filter((result) => !commaChangePairs.has(result)); +} diff --git a/packages/wrangler/src/containers/deploy.ts b/packages/wrangler/src/containers/deploy.ts index 1a8909492c81..f49c5f638af7 100644 --- a/packages/wrangler/src/containers/deploy.ts +++ b/packages/wrangler/src/containers/deploy.ts @@ -23,6 +23,7 @@ import { import { inferInstanceType, promiseSpinner } from "../cloudchamber/common"; import { diffLines, + filterTrailingCommaChanges, printLine, renderDiff, sortObjectRecursive, @@ -312,7 +313,8 @@ export async function apply( const prev = JSON.stringify({ containers: [normalisedPrevApp] }, null, 2); const now = JSON.stringify({ containers: [nowContainer] }, null, 2); - const results = diffLines(prev, now); + const rawResults = diffLines(prev, now); + const results = filterTrailingCommaChanges(rawResults); const changes = results.find((l) => l.added || l.removed) !== undefined; if (!changes) { From 8f109f12c8258fbc2cf6f9eafe0ee96937ece6ed Mon Sep 17 00:00:00 2001 From: Nikita Sharma Date: Mon, 14 Jul 2025 10:43:02 -0500 Subject: [PATCH 11/11] Add support for custom instance types Custom instance types could previously only be set through setting vcpu/memory/disk through the `configuration` block. As we deprecate `configuration`, move setting vcpu, memory, and disk to the instance type field of wrangler. By nature, this enforces that custom instance types are mutually exclusive with the named instance types and will allow us to remove a lot of the logic that verifies this once `configuration` is removed. --- .changeset/hungry-turtles-beam.md | 5 + .../src/__tests__/cloudchamber/apply.test.ts | 147 ++++++++++++++++++ packages/wrangler/src/cloudchamber/apply.ts | 32 +++- packages/wrangler/src/config/environment.ts | 10 +- packages/wrangler/src/config/validation.ts | 62 ++++++-- 5 files changed, 237 insertions(+), 19 deletions(-) create mode 100644 .changeset/hungry-turtles-beam.md diff --git a/.changeset/hungry-turtles-beam.md b/.changeset/hungry-turtles-beam.md new file mode 100644 index 000000000000..0bbb9de12820 --- /dev/null +++ b/.changeset/hungry-turtles-beam.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +Add support for custom instance types diff --git a/packages/wrangler/src/__tests__/cloudchamber/apply.test.ts b/packages/wrangler/src/__tests__/cloudchamber/apply.test.ts index b8add5235685..3700e7e9f26d 100644 --- a/packages/wrangler/src/__tests__/cloudchamber/apply.test.ts +++ b/packages/wrangler/src/__tests__/cloudchamber/apply.test.ts @@ -1585,6 +1585,63 @@ describe("cloudchamber apply", () => { expect(std.stderr).toMatchInlineSnapshot(`""`); }); + test("can apply a simple application (custom instance type)", async () => { + setIsTTY(false); + writeWranglerConfig({ + name: "my-container", + containers: [ + { + name: "my-container-app", + instances: 3, + class_name: "DurableObjectClass", + instance_type: { + vcpu: 1, + memory_mib: 1024, + disk_mb: 2000, + }, + image: "./Dockerfile", + constraints: { + tier: 2, + }, + }, + ], + }); + mockGetApplications([]); + mockCreateApplication({ id: "abc" }); + await runWrangler("cloudchamber apply"); + expect(std.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ NEW my-container-app + │ + │ [[containers]] + │ name = \\"my-container-app\\" + │ instances = 3 + │ scheduling_policy = \\"default\\" + │ + │ [containers.constraints] + │ tier = 2 + │ + │ [containers.configuration] + │ image = \\"./Dockerfile\\" + │ vcpu = 1 + │ memory_mib = 1_024 + │ + │ [containers.configuration.disk] + │ size_mb = 2_000 + │ + │ + │ SUCCESS Created application my-container-app (Application ID: abc) + │ + ╰ Applied changes + + " + `); + expect(std.stderr).toMatchInlineSnapshot(`""`); + }); + test("can apply a simple existing application (instance type)", async () => { setIsTTY(false); writeWranglerConfig({ @@ -1662,6 +1719,96 @@ describe("cloudchamber apply", () => { expect(app.configuration?.instance_type).toEqual("standard"); }); + test("can apply a simple existing application (custom instance type)", async () => { + setIsTTY(false); + writeWranglerConfig({ + name: "my-container", + containers: [ + { + name: "my-container-app", + instances: 4, + class_name: "DurableObjectClass", + instance_type: { + vcpu: 1, + memory_mib: 1024, + disk_mb: 6000, + }, + image: "./Dockerfile", + constraints: { + tier: 2, + }, + }, + ], + }); + mockGetApplications([ + { + id: "abc", + name: "my-container-app", + instances: 3, + created_at: new Date().toString(), + version: 1, + account_id: "1", + scheduling_policy: SchedulingPolicy.REGIONAL, + configuration: { + image: "./Dockerfile", + disk: { + size: "2GB", + size_mb: 2000, + }, + vcpu: 0.0625, + memory: "256MB", + memory_mib: 256, + }, + constraints: { + tier: 3, + }, + }, + ]); + const applicationReqBodyPromise = mockModifyApplication(); + await runWrangler("cloudchamber apply"); + expect(std.stdout).toMatchInlineSnapshot(` + "╭ Deploy a container application deploy changes to your application + │ + │ Container application changes + │ + ├ EDIT my-container-app + │ + │ [[containers]] + │ - instances = 3 + │ + instances = 4 + │ name = \\"my-container-app\\" + │ + │ [containers.configuration] + │ ... + │ memory = \\"256MB\\" + │ - memory_mib = 256 + │ + memory_mib = 1_024 + │ - vcpu = 0.0625 + │ + vcpu = 1 + │ + │ [containers.configuration.disk] + │ ... + │ size = \\"2GB\\" + │ - size_mb = 2_000 + │ + size_mb = 6_000 + │ + │ [containers.constraints] + │ ... + │ - tier = 3 + │ + tier = 2 + │ + │ + │ SUCCESS Modified application my-container-app + │ + ╰ Applied changes + + " + `); + expect(std.stderr).toMatchInlineSnapshot(`""`); + const app = await applicationReqBodyPromise; + expect(app.configuration?.instance_type).toBeUndefined(); + }); + test("falls back on dev instance type when instance type is absent", async () => { setIsTTY(false); writeWranglerConfig({ diff --git a/packages/wrangler/src/cloudchamber/apply.ts b/packages/wrangler/src/cloudchamber/apply.ts index 0552180e3e1d..18cbc2a2536f 100644 --- a/packages/wrangler/src/cloudchamber/apply.ts +++ b/packages/wrangler/src/cloudchamber/apply.ts @@ -189,22 +189,38 @@ function observabilityToConfiguration( function containerAppToInstanceType( containerApp: ContainerApp -): InstanceType | undefined { +): Partial { + let configuration = (containerApp.configuration ?? + {}) as Partial; + if (containerApp.instance_type !== undefined) { - return containerApp.instance_type as InstanceType; + if (typeof containerApp.instance_type === "string") { + return { instance_type: containerApp.instance_type as InstanceType }; + } + + configuration = { + vcpu: containerApp.instance_type.vcpu ?? containerApp.configuration?.vcpu, + memory_mib: + containerApp.instance_type.memory_mib ?? + containerApp.configuration?.memory_mib, + disk: { + size_mb: + containerApp.instance_type.disk_mb ?? + containerApp.configuration?.disk?.size_mb, + }, + }; } // if no other configuration is set, we fall back to the default "dev" instance type - const configuration = - containerApp.configuration as UserDeploymentConfiguration; if ( - configuration.disk === undefined && + configuration.disk?.size_mb === undefined && configuration.vcpu === undefined && - configuration.memory === undefined && configuration.memory_mib === undefined ) { - return InstanceType.DEV; + return { instance_type: InstanceType.DEV }; } + + return configuration; } function containerAppToCreateApplication( @@ -221,8 +237,8 @@ function containerAppToCreateApplication( const instanceType = containerAppToInstanceType(containerApp); const configuration: UserDeploymentConfiguration = { ...(containerApp.configuration as UserDeploymentConfiguration), + ...instanceType, observability: observabilityConfiguration, - instance_type: instanceType, }; // this should have been set to a default value of worker-name-class-name if unspecified by the user diff --git a/packages/wrangler/src/config/environment.ts b/packages/wrangler/src/config/environment.ts index 804a775b171a..dbe01a89d903 100644 --- a/packages/wrangler/src/config/environment.ts +++ b/packages/wrangler/src/config/environment.ts @@ -102,7 +102,15 @@ export type ContainerApp = { * @optional * @default "dev" */ - instance_type?: "dev" | "basic" | "standard"; + instance_type?: + | "dev" + | "basic" + | "standard" + | { + vcpu?: number; + memory_mib?: number; + disk_mb?: number; + }; /** * @deprecated Use top level `containers` fields instead. diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts index e7e941560142..3a4a6a577cb4 100644 --- a/packages/wrangler/src/config/validation.ts +++ b/packages/wrangler/src/config/validation.ts @@ -2426,13 +2426,13 @@ function validateContainerApp( !containerAppOptional.image ) { diagnostics.errors.push( - `"containers.image" field must be defined for each container app. This should be the path to your Dockerfile or a image URI pointing to the Cloudflare registry.` + `"containers.image" field must be defined for each container app. This should be the path to your Dockerfile or an image URI pointing to the Cloudflare registry.` ); } if ("configuration" in containerAppOptional) { diagnostics.warnings.push( - `"containers.configuration" is deprecated. Use top level "containers" fields instead. "configuration.image" should be "image", "configuration.disk" should be set via "instance_type".` + `"containers.configuration" is deprecated. Use top level "containers" fields instead. "configuration.image" should be "image", limits should be set via "instance_type".` ); if ( typeof containerAppOptional !== "object" || @@ -2485,14 +2485,6 @@ function validateContainerApp( "string", ["full_auto", "full_manual", "none"] ); - validateOptionalProperty( - diagnostics, - field, - "instance_type", - containerAppOptional.instance_type, - "string", - ["dev", "basic", "standard"] - ); validateOptionalProperty( diagnostics, field, @@ -2572,6 +2564,56 @@ function validateContainerApp( ["image", "secrets", "labels", "disk", "vcpu", "memory_mib"] ); } + + // Instance Type validation: When present, the instance type should be either (1) a string + // representing a predefined instance type or (2) an object that optionally defines vcpu, + // memory, and disk. + // + // If an instance type is not set, a 'dev' instance type will be used. If a custom instance + // type doesn't set a value, that value will default to the corresponding value in a 'dev' + // instance type + if (typeof containerAppOptional.instance_type === "string") { + // validate named instance type + validateOptionalProperty( + diagnostics, + field, + "instance_type", + containerAppOptional.instance_type, + "string", + ["dev", "basic", "standard"] + ); + } else if ( + validateOptionalProperty( + diagnostics, + field, + "instance_type", + containerAppOptional.instance_type, + "object" + ) && + containerAppOptional.instance_type + ) { + // validate custom instance type + const instanceTypeProperties = ["vcpu", "memory_mib", "disk_mb"]; + instanceTypeProperties.forEach((key) => { + if ( + !isOptionalProperty( + containerAppOptional.instance_type, + key, + "number" + ) + ) { + diagnostics.errors.push( + `"containers.instance_type.${key}", when present, should be a number.` + ); + } + }); + validateAdditionalProperties( + diagnostics, + `${field}.instance_type`, + Object.keys(containerAppOptional.instance_type), + instanceTypeProperties + ); + } } if (diagnostics.errors.length > 0) {