Skip to content

CC-8419: make directory snapshot id optional #7039

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
flakey5 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

CC-8419: make directory snapshot id optional #7039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 161 additions & 0 deletions src/workerd/api/container-test.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,47 @@ class MockExecContainerServer final: public rpc::Container::Server {
kj::Maybe<kj::Own<kj::PromiseFulfiller<void>>> resizeFulfiller;
};

struct CapturedDirectorySnapshot {
bool hasSnapshotId = false;
kj::String snapshotId;
kj::String restorePath;
};

// Captures the directorySnapshots forwarded by Container::start() so tests can assert how each
// DirectorySnapshotRestoreParams is translated into the RPC request.
class DirectorySnapshotStartServer final: public rpc::Container::Server {
public:
explicit DirectorySnapshotStartServer(kj::Vector<CapturedDirectorySnapshot>& captured)
: captured(captured) {}

kj::Promise<void> start(StartContext context) override {
for (auto entry: context.getParams().getDirectorySnapshots()) {
captured.add(CapturedDirectorySnapshot{
.hasSnapshotId = entry.hasSnapshotId(),
.snapshotId = kj::str(entry.getSnapshotId()),
.restorePath = kj::str(entry.getRestorePath()),
});
}
return kj::READY_NOW;
}

kj::Promise<void> monitor(MonitorContext context) override {
return kj::NEVER_DONE;
}

private:
kj::Vector<CapturedDirectorySnapshot>& captured;
};

Container::DirectorySnapshot makeDirectorySnapshot(kj::StringPtr id, kj::StringPtr dir) {
return Container::DirectorySnapshot{
.id = kj::str(id),
.size = 0,
.dir = kj::str(dir),
.name = kj::none,
};
}

enum class TunnelReuseGate { DISABLED, ENABLED };

class AutogateScope {
Expand Down Expand Up @@ -748,6 +789,126 @@ KJ_TEST("Container::snapshotContainer propagates the current span context") {
KJ_EXPECT(containerCalled);
}

KJ_TEST("Container::start restores a directory snapshot using the snapshot's own dir") {
kj::Vector<CapturedDirectorySnapshot> captured;
auto fixture = makeFixture();

fixture.runInIoContext([&](const TestFixture::Environment& env) -> kj::Promise<void> {
auto container = kj::heap<Container>(
rpc::Container::Client(kj::heap<DirectorySnapshotStartServer>(captured)), false);

auto snapshots = kj::heapArrayBuilder<Container::DirectorySnapshotRestoreParams>(1);
snapshots.add(Container::DirectorySnapshotRestoreParams{
.snapshot = makeDirectorySnapshot("snap-id"_kj, "/data"_kj),
.mountPoint = kj::none,
});
container->start(env.js,
Container::StartupOptions{
.directorySnapshots = snapshots.finish(),
});

// Give the queued start RPC bounded time to run.
for (auto i = 0; i < 10; ++i) {
co_await kj::yield();
}
});

KJ_ASSERT(captured.size() == 1);
KJ_EXPECT(captured[0].hasSnapshotId);
KJ_EXPECT(captured[0].snapshotId == "snap-id");
KJ_EXPECT(captured[0].restorePath == "/data");
}

KJ_TEST("Container::start lets mountPoint override the snapshot's dir as the restore path") {
kj::Vector<CapturedDirectorySnapshot> captured;
auto fixture = makeFixture();

fixture.runInIoContext([&](const TestFixture::Environment& env) -> kj::Promise<void> {
auto container = kj::heap<Container>(
rpc::Container::Client(kj::heap<DirectorySnapshotStartServer>(captured)), false);

auto snapshots = kj::heapArrayBuilder<Container::DirectorySnapshotRestoreParams>(1);
snapshots.add(Container::DirectorySnapshotRestoreParams{
.snapshot = makeDirectorySnapshot("snap-id"_kj, "/data"_kj),
.mountPoint = kj::str("/mnt/elsewhere"),
});
container->start(env.js,
Container::StartupOptions{
.directorySnapshots = snapshots.finish(),
});

for (auto i = 0; i < 10; ++i) {
co_await kj::evalLater([]() {});
}
});

KJ_ASSERT(captured.size() == 1);
KJ_EXPECT(captured[0].hasSnapshotId);
KJ_EXPECT(captured[0].snapshotId == "snap-id");
KJ_EXPECT(captured[0].restorePath == "/mnt/elsewhere");
}

KJ_TEST("Container::start restores to mountPoint when no snapshot is given") {
kj::Vector<CapturedDirectorySnapshot> captured;
auto fixture = makeFixture();

fixture.runInIoContext([&](const TestFixture::Environment& env) -> kj::Promise<void> {
auto container = kj::heap<Container>(
rpc::Container::Client(kj::heap<DirectorySnapshotStartServer>(captured)), false);

auto snapshots = kj::heapArrayBuilder<Container::DirectorySnapshotRestoreParams>(1);
snapshots.add(Container::DirectorySnapshotRestoreParams{
.snapshot = kj::none,
.mountPoint = kj::str("/mnt/data"),
});
container->start(env.js,
Container::StartupOptions{
.directorySnapshots = snapshots.finish(),
});

for (auto i = 0; i < 10; ++i) {
co_await kj::evalLater([]() {});
}
});

KJ_ASSERT(captured.size() == 1);
KJ_EXPECT(!captured[0].hasSnapshotId);
KJ_EXPECT(captured[0].snapshotId == "");
KJ_EXPECT(captured[0].restorePath == "/mnt/data");
}

KJ_TEST("Container::start requires mountPoint when no snapshot is given") {
kj::Vector<CapturedDirectorySnapshot> captured;
auto fixture = makeFixture();

fixture.runInIoContext([&](const TestFixture::Environment& env) -> kj::Promise<void> {
auto container = kj::heap<Container>(
rpc::Container::Client(kj::heap<DirectorySnapshotStartServer>(captured)), false);

auto snapshots = kj::heapArrayBuilder<Container::DirectorySnapshotRestoreParams>(1);
snapshots.add(Container::DirectorySnapshotRestoreParams{
.snapshot = kj::none,
.mountPoint = kj::none,
});

bool threw = false;
JSG_TRY(env.js) {
container->start(env.js,
Container::StartupOptions{
.directorySnapshots = snapshots.finish(),
});
}
JSG_CATCH(exception KJ_UNUSED) {
threw = true;
}
KJ_EXPECT(threw, "start() should throw when neither snapshot nor mountPoint is given");

return kj::READY_NOW;
});

KJ_EXPECT(captured.size() == 0);
}

KJ_TEST("Container::exec forwards pty options and resize() sends a resize RPC") {
ExecObservations observations;
auto fixture = makeFixture();
Expand Down
18 changes: 13 additions & 5 deletions src/workerd/api/container.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,17 +420,25 @@ void Container::start(jsg::Lock& js, jsg::Optional<StartupOptions> maybeOptions)
for (auto i: kj::indices(directorySnapshots)) {
auto entry = list[i];
auto& restore = directorySnapshots[i];
auto& snap = restore.snapshot;
auto effectiveRestorePath = snap.dir.asPtr();

kj::Maybe<kj::StringPtr> effectiveRestorePath;
KJ_IF_SOME(snap, restore.snapshot) {
effectiveRestorePath = snap.dir.asPtr();
}
KJ_IF_SOME(mp, restore.mountPoint) {
effectiveRestorePath = mp.asPtr();
}

JSG_REQUIRE_NONNULL(parseRestorePath(effectiveRestorePath), Error,
auto restorePath = JSG_REQUIRE_NONNULL(effectiveRestorePath, Error,
"Directory snapshot restore requires a mountPoint when no snapshot is given.");

JSG_REQUIRE_NONNULL(parseRestorePath(restorePath), Error,
"Directory snapshot cannot be restored to root directory.");

entry.setSnapshotId(snap.id);
entry.setRestorePath(effectiveRestorePath);
KJ_IF_SOME(snap, restore.snapshot) {
entry.setSnapshotId(snap.id);
}
entry.setRestorePath(restorePath);
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/workerd/api/container.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,14 @@ class Container: public jsg::Object {
};

struct DirectorySnapshotRestoreParams {
DirectorySnapshot snapshot;
jsg::Optional<DirectorySnapshot> snapshot;
jsg::Optional<kj::String> mountPoint;

JSG_STRUCT(snapshot, mountPoint);
JSG_STRUCT_TS_OVERRIDE(type ContainerDirectorySnapshotRestoreParams =
| { snapshot: ContainerDirectorySnapshot; mountPoint?: string }
| { snapshot?: undefined; mountPoint: string }
);
};

struct Snapshot {
Expand Down
13 changes: 9 additions & 4 deletions types/generated-snapshot/experimental/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4078,10 +4078,15 @@ interface ContainerDirectorySnapshotOptions {
dir: string;
name?: string;
}
interface ContainerDirectorySnapshotRestoreParams {
snapshot: ContainerDirectorySnapshot;
mountPoint?: string;
}
type ContainerDirectorySnapshotRestoreParams =
| {
snapshot: ContainerDirectorySnapshot;
mountPoint?: string;
}
| {
snapshot?: undefined;
mountPoint: string;
};
interface ContainerSnapshot {
id: string;
size: number;
Expand Down
13 changes: 9 additions & 4 deletions types/generated-snapshot/experimental/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4087,10 +4087,15 @@ export interface ContainerDirectorySnapshotOptions {
dir: string;
name?: string;
}
export interface ContainerDirectorySnapshotRestoreParams {
snapshot: ContainerDirectorySnapshot;
mountPoint?: string;
}
export type ContainerDirectorySnapshotRestoreParams =
| {
snapshot: ContainerDirectorySnapshot;
mountPoint?: string;
}
| {
snapshot?: undefined;
mountPoint: string;
};
export interface ContainerSnapshot {
id: string;
size: number;
Expand Down
13 changes: 9 additions & 4 deletions types/generated-snapshot/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3988,10 +3988,15 @@ interface ContainerDirectorySnapshotOptions {
dir: string;
name?: string;
}
interface ContainerDirectorySnapshotRestoreParams {
snapshot: ContainerDirectorySnapshot;
mountPoint?: string;
}
type ContainerDirectorySnapshotRestoreParams =
| {
snapshot: ContainerDirectorySnapshot;
mountPoint?: string;
}
| {
snapshot?: undefined;
mountPoint: string;
};
interface ContainerSnapshot {
id: string;
size: number;
Expand Down
13 changes: 9 additions & 4 deletions types/generated-snapshot/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3997,10 +3997,15 @@ export interface ContainerDirectorySnapshotOptions {
dir: string;
name?: string;
}
export interface ContainerDirectorySnapshotRestoreParams {
snapshot: ContainerDirectorySnapshot;
mountPoint?: string;
}
export type ContainerDirectorySnapshotRestoreParams =
| {
snapshot: ContainerDirectorySnapshot;
mountPoint?: string;
}
| {
snapshot?: undefined;
mountPoint: string;
};
export interface ContainerSnapshot {
id: string;
size: number;
Expand Down
Loading