From 261bf62e85e6138e804a5eb2a2767f337efe9caa Mon Sep 17 00:00:00 2001 From: Greg Anders Date: Thu, 13 Aug 2026 12:14:53 -0500 Subject: [PATCH] Remove experimental flag for container start image and instance --- src/workerd/api/container-test.c++ | 38 +++++++++++++----- src/workerd/api/container.c++ | 60 ++++++++++++++--------------- src/workerd/api/container.h | 20 +++++++--- types/generated-snapshot/index.d.ts | 21 ++++++++-- types/generated-snapshot/index.ts | 21 ++++++++-- 5 files changed, 109 insertions(+), 51 deletions(-) diff --git a/src/workerd/api/container-test.c++ b/src/workerd/api/container-test.c++ index 545e46f71ba..e47f5e9a16a 100644 --- a/src/workerd/api/container-test.c++ +++ b/src/workerd/api/container-test.c++ @@ -39,6 +39,7 @@ class TracingRequestObserver final: public RequestObserver { struct CapturedInstance { bool isCustom = false; kj::String named; + kj::String image; double vcpu = 0; uint64_t memoryMib = 0; uint64_t diskMb = 0; @@ -53,8 +54,13 @@ class MockContainerServer final: public rpc::Container::Server { containerCalled(containerCalled) {} kj::Promise start(StartContext context) override { - auto instance = context.getParams().getInstance(); + auto params = context.getParams(); + auto instance = params.getInstance(); CapturedInstance captured; + auto source = params.getSource(); + if (source.which() == rpc::Container::StartParams::Source::IMAGE) { + captured.image = kj::str(source.getImage()); + } switch (instance.which()) { case rpc::Container::StartInstance::NAMED: captured.named = kj::str(instance.getNamed()); @@ -633,10 +639,7 @@ KJ_TEST("Container::destroy updates running before restart and clears the old re } KJ_TEST("Container::start forwards a named instance type") { - capnp::MallocMessageBuilder message; - auto flags = message.initRoot(); - flags.setWorkerdExperimental(true); - auto fixture = TestFixture({.featureFlags = flags.asReader(), .useRealTimers = false}); + auto fixture = makeFixture(); auto paf = kj::newPromiseAndFulfiller(); auto promise = kj::mv(paf.promise); @@ -657,10 +660,7 @@ KJ_TEST("Container::start forwards a named instance type") { } KJ_TEST("Container::start forwards custom instance resources") { - capnp::MallocMessageBuilder message; - auto flags = message.initRoot(); - flags.setWorkerdExperimental(true); - auto fixture = TestFixture({.featureFlags = flags.asReader(), .useRealTimers = false}); + auto fixture = makeFixture(); auto paf = kj::newPromiseAndFulfiller(); auto promise = kj::mv(paf.promise); @@ -687,6 +687,26 @@ KJ_TEST("Container::start forwards custom instance resources") { }); } +KJ_TEST("Container::start forwards an image") { + auto fixture = makeFixture(); + auto paf = kj::newPromiseAndFulfiller(); + auto promise = kj::mv(paf.promise); + + fixture.runInIoContext([promise = kj::mv(promise), fulfiller = kj::mv(paf.fulfiller)]( + const TestFixture::Environment& env) mutable { + auto container = kj::rc( + rpc::Container::Client(kj::heap(kj::mv(fulfiller))), false); + container->start(env.js, + Container::StartupOptions{ + .image = kj::str("registry.example.com/image:tag"), + }); + return kj::mv(promise) + .then([](CapturedInstance captured) { + KJ_EXPECT(captured.image == "registry.example.com/image:tag"); + }).attach(kj::mv(container)); + }); +} + KJ_TEST("Container::snapshotDirectory propagates the current span context") { bool directoryCalled = false; bool containerCalled = false; diff --git a/src/workerd/api/container.c++ b/src/workerd/api/container.c++ index 806b30bfcf9..71f3389e2b0 100644 --- a/src/workerd/api/container.c++ +++ b/src/workerd/api/container.c++ @@ -365,42 +365,42 @@ void Container::start(jsg::Lock& js, jsg::Optional maybeOptions) } } + JSG_REQUIRE(options.image == kj::none || options.containerSnapshot == kj::none, TypeError, + "`image` and `containerSnapshot` are mutually exclusive."); if (flags.getWorkerdExperimental()) { - JSG_REQUIRE(options.image == kj::none || options.containerSnapshot == kj::none, TypeError, - "`image` and `containerSnapshot` are mutually exclusive."); KJ_IF_SOME(hardTimeoutMs, options.hardTimeout) { JSG_REQUIRE(hardTimeoutMs > 0, RangeError, "Hard timeout must be greater than 0"); req.setHardTimeoutMs(hardTimeoutMs); } - KJ_IF_SOME(image, options.image) { - JSG_REQUIRE(image.size() <= MAX_IMAGE_REFERENCE_SIZE, TypeError, - "Container image reference cannot exceed ", MAX_IMAGE_REFERENCE_SIZE, " bytes."); - for (auto c: image) { - auto byte = static_cast(c); - JSG_REQUIRE(byte > 0x20 && byte < 0x7f, TypeError, - "Container image reference must contain only non-space printable ASCII characters."); - } - req.getSource().setImage(image); + } + KJ_IF_SOME(image, options.image) { + JSG_REQUIRE(image.size() <= MAX_IMAGE_REFERENCE_SIZE, TypeError, + "Container image reference cannot exceed ", MAX_IMAGE_REFERENCE_SIZE, " bytes."); + for (auto c: image) { + auto byte = static_cast(c); + JSG_REQUIRE(byte > 0x20 && byte < 0x7f, TypeError, + "Container image reference must contain only non-space printable ASCII characters."); } - KJ_IF_SOME(instance, options.instance) { - auto instanceBuilder = req.initInstance(); - KJ_SWITCH_ONEOF(instance) { - KJ_CASE_ONEOF(named, kj::String) { - JSG_REQUIRE( - kj::arrayPtr(VALID_CONTAINER_INSTANCE_TYPES).findFirst(named.asPtr()) != kj::none, - TypeError, "Invalid container instance type."); - instanceBuilder.setNamed(named); - } - KJ_CASE_ONEOF(custom, StartResources) { - JSG_REQUIRE(std::isfinite(custom.vcpu) && custom.vcpu > 0, RangeError, - "Container resource vcpu must be a finite number greater than 0."); - auto memoryMib = requireResourceAmount(custom.memoryMib, "memoryMib"_kj); - auto diskMb = requireResourceAmount(custom.diskMb, "diskMb"_kj); - auto resources = instanceBuilder.initCustom(); - resources.setVcpu(custom.vcpu); - resources.setMemoryMib(memoryMib); - resources.setDiskMb(diskMb); - } + req.getSource().setImage(image); + } + KJ_IF_SOME(instance, options.instance) { + auto instanceBuilder = req.initInstance(); + KJ_SWITCH_ONEOF(instance) { + KJ_CASE_ONEOF(named, kj::String) { + JSG_REQUIRE( + kj::arrayPtr(VALID_CONTAINER_INSTANCE_TYPES).findFirst(named.asPtr()) != kj::none, + TypeError, "Invalid container instance type."); + instanceBuilder.setNamed(named); + } + KJ_CASE_ONEOF(custom, StartResources) { + JSG_REQUIRE(std::isfinite(custom.vcpu) && custom.vcpu > 0, RangeError, + "Container resource vcpu must be a finite number greater than 0."); + auto memoryMib = requireResourceAmount(custom.memoryMib, "memoryMib"_kj); + auto diskMb = requireResourceAmount(custom.diskMb, "diskMb"_kj); + auto resources = instanceBuilder.initCustom(); + resources.setVcpu(custom.vcpu); + resources.setMemoryMib(memoryMib); + resources.setDiskMb(diskMb); } } } diff --git a/src/workerd/api/container.h b/src/workerd/api/container.h index 8684b56ea16..d097b597211 100644 --- a/src/workerd/api/container.h +++ b/src/workerd/api/container.h @@ -301,17 +301,25 @@ class Container: public jsg::Object { } )); } else { - JSG_TS_OVERRIDE(ContainerStartupOptions { + JSG_TS_OVERRIDE(type ContainerStartupOptions = { entrypoint?: string[]; enableInternet: boolean; env?: Record; - hardTimeout?: never; - image?: never; - instance?: never; + instance?: "lite" | "standard-1" | "standard-2" | "standard-3" | "standard-4" | ContainerStartResources; labels?: Record; directorySnapshots?: ContainerDirectorySnapshotRestoreParams[]; - containerSnapshot?: ContainerSnapshotRestoreParams; - }); + } & ( + | { + /** Cannot be used with `containerSnapshot`. */ + image: string; + containerSnapshot?: never; + } + | { + image?: never; + /** Cannot be used with `image`. */ + containerSnapshot?: ContainerSnapshotRestoreParams; + } + )); } } }; diff --git a/types/generated-snapshot/index.d.ts b/types/generated-snapshot/index.d.ts index 18bf68a6767..5147ad420ea 100755 --- a/types/generated-snapshot/index.d.ts +++ b/types/generated-snapshot/index.d.ts @@ -4003,14 +4003,29 @@ interface ContainerSnapshotRestoreParams { interface ContainerSnapshotOptions { name?: string; } -interface ContainerStartupOptions { +type ContainerStartupOptions = { entrypoint?: string[]; enableInternet: boolean; env?: Record; + instance?: + | "lite" + | "standard-1" + | "standard-2" + | "standard-3" + | "standard-4" + | ContainerStartResources; labels?: Record; directorySnapshots?: ContainerDirectorySnapshotRestoreParams[]; - containerSnapshot?: ContainerSnapshotRestoreParams; -} +} & ( + | { + image: string; + containerSnapshot?: never; + } + | { + image?: never; + containerSnapshot?: ContainerSnapshotRestoreParams; + } +); interface ContainerStartResources { vcpu: number; memoryMib: number; diff --git a/types/generated-snapshot/index.ts b/types/generated-snapshot/index.ts index beed08a500f..8dcd74b783c 100755 --- a/types/generated-snapshot/index.ts +++ b/types/generated-snapshot/index.ts @@ -4012,14 +4012,29 @@ export interface ContainerSnapshotRestoreParams { export interface ContainerSnapshotOptions { name?: string; } -export interface ContainerStartupOptions { +export type ContainerStartupOptions = { entrypoint?: string[]; enableInternet: boolean; env?: Record; + instance?: + | "lite" + | "standard-1" + | "standard-2" + | "standard-3" + | "standard-4" + | ContainerStartResources; labels?: Record; directorySnapshots?: ContainerDirectorySnapshotRestoreParams[]; - containerSnapshot?: ContainerSnapshotRestoreParams; -} +} & ( + | { + image: string; + containerSnapshot?: never; + } + | { + image?: never; + containerSnapshot?: ContainerSnapshotRestoreParams; + } +); export interface ContainerStartResources { vcpu: number; memoryMib: number;