mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 02:26:12 +02:00
fix(daytona): clean up sandbox allocation after create failure
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
1b0551bb7c
commit
c09ec0a843
3 files changed
+129
-6
No files matched your search
@@ -27,6 +27,7 @@ Notes:
|
||||
|
||||
- The current published Daytona SDK package is `@daytonaio/sdk`.
|
||||
- The driver supports both `snapshot`-based and `image`-based sandbox creation. If both are set, validation rejects the config as ambiguous.
|
||||
- Each cold create uses a unique provider name and ownership labels. If creation fails after Daytona has allocated a sandbox, the driver looks up that exact name, checks every ownership label, and waits for deletion. Failed, missing, or timed-out lookups and failed deletions report unconfirmed cleanup with the provider name; they never return a usable lease. A missing lookup after an uncertain create is not proof that a delayed provider request cannot create a resource.
|
||||
- Reusable leases map to Daytona stop/start semantics. Non-reusable leases are deleted on release. A provider-resolved `target` does not change the identity of an existing sandbox. Release closes the same scoped lease that a later sentinel-verified resume reopens.
|
||||
- A sandbox record can survive the loss of its underlying container. Resume treats it as expired only when a fresh provider read confirms the exact missing-container error for that sandbox and marks it unrecoverable. Unknown errors and failed confirmation reads preserve the lease. The host still requires a verified native-runner backup before replacement.
|
||||
|
||||
|
||||
@@ -523,6 +523,105 @@ describe("Daytona sandbox provider plugin", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("failed sandbox creation cleanup", () => {
|
||||
const params = {
|
||||
driverKey: "daytona", companyId: "company-1", environmentId: "env-1", runId: "run-1",
|
||||
config: { image: "node:20", timeoutMs: 300_000, reuseLease: false, livenessTimeoutMs: 100 },
|
||||
};
|
||||
const createError = new MockDaytonaTimeoutError("Failed to create and start sandbox within 300 seconds");
|
||||
|
||||
beforeEach(() => {
|
||||
process.env.DAYTONA_API_KEY = "host-key";
|
||||
mockCreate.mockRejectedValue(createError);
|
||||
});
|
||||
|
||||
function ownedSandbox() {
|
||||
const requested = mockCreate.mock.calls.at(-1)![0];
|
||||
return { ...createMockSandbox(), name: requested.name, labels: { ...requested.labels } };
|
||||
}
|
||||
|
||||
it("deletes the exact sandbox left behind when create times out, then preserves the create error", async () => {
|
||||
let orphan: ReturnType<typeof ownedSandbox>;
|
||||
mockGet.mockImplementation(async () => (orphan = ownedSandbox()));
|
||||
await expect(plugin.definition.onEnvironmentAcquireLease?.(params)).rejects.toBe(createError);
|
||||
expect(mockCreate.mock.calls[0][0].name).toMatch(/^paperclip-create-[0-9a-f-]{36}$/);
|
||||
expect(mockGet).toHaveBeenCalledWith(mockCreate.mock.calls[0][0].name);
|
||||
expect(orphan!.delete).toHaveBeenCalledWith(10, true);
|
||||
expect(orphan!.process.executeCommand).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(["paperclip-company-id", "paperclip-environment-id", "paperclip-run-id", "paperclip-create-attempt", "paperclip-provider"])("never deletes a sandbox with a mismatched %s", async (label) => {
|
||||
let orphan: ReturnType<typeof ownedSandbox>;
|
||||
mockGet.mockImplementation(async () => {
|
||||
orphan = ownedSandbox(); orphan.labels[label] = "different-owner"; return orphan;
|
||||
});
|
||||
await expect(plugin.definition.onEnvironmentAcquireLease?.(params)).rejects.toThrow("cleanup could not be confirmed");
|
||||
expect(orphan!.delete).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("never deletes a sandbox returned for a different name", async () => {
|
||||
const wrong = { ...createMockSandbox(), labels: {} };
|
||||
mockGet.mockResolvedValue(wrong);
|
||||
await expect(plugin.definition.onEnvironmentAcquireLease?.(params)).rejects.toThrow("cleanup could not be confirmed");
|
||||
expect(wrong.delete).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not treat a not-found lookup after uncertain creation as confirmed cleanup", async () => {
|
||||
mockGet.mockRejectedValue(new MockDaytonaNotFoundError("missing"));
|
||||
await expect(plugin.definition.onEnvironmentAcquireLease?.(params)).rejects.toThrow("cleanup could not be confirmed");
|
||||
expect(mockGet).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("reports unconfirmed cleanup when the provider lookup fails", async () => {
|
||||
mockGet.mockRejectedValue(new Error("provider unavailable"));
|
||||
await expect(plugin.definition.onEnvironmentAcquireLease?.(params)).rejects.toThrow("cleanup could not be confirmed");
|
||||
});
|
||||
|
||||
it("reports unconfirmed cleanup when provider deletion fails", async () => {
|
||||
mockGet.mockImplementation(async () => {
|
||||
const orphan = ownedSandbox(); orphan.delete.mockRejectedValue(new Error("delete failed")); return orphan;
|
||||
});
|
||||
await expect(plugin.definition.onEnvironmentAcquireLease?.(params)).rejects.toThrow("cleanup could not be confirmed");
|
||||
});
|
||||
|
||||
it("bounds a hung provider lookup and preserves both failure causes", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
mockGet.mockImplementation(() => new Promise(() => {}));
|
||||
const result = plugin.definition.onEnvironmentAcquireLease!(params).catch((error: unknown) => error);
|
||||
await vi.advanceTimersByTimeAsync(10_001);
|
||||
const error = await result;
|
||||
expect(error).toBeInstanceOf(AggregateError);
|
||||
expect((error as AggregateError).errors[0]).toBe(createError);
|
||||
expect((error as Error).message).toContain(mockCreate.mock.calls[0][0].name);
|
||||
} finally { vi.useRealTimers(); }
|
||||
});
|
||||
|
||||
it("bounds a hung provider deletion without returning a lease", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
mockGet.mockImplementation(async () => {
|
||||
const orphan = ownedSandbox(); orphan.delete.mockImplementation(() => new Promise(() => {})); return orphan;
|
||||
});
|
||||
const result = plugin.definition.onEnvironmentAcquireLease!(params).catch((error: unknown) => error);
|
||||
await vi.advanceTimersByTimeAsync(15_001);
|
||||
expect(await result).toBeInstanceOf(AggregateError);
|
||||
} finally { vi.useRealTimers(); }
|
||||
});
|
||||
|
||||
it("uses distinct creation identities for concurrent attempts on the same run", async () => {
|
||||
mockGet.mockRejectedValue(new MockDaytonaNotFoundError("missing"));
|
||||
await Promise.allSettled([
|
||||
plugin.definition.onEnvironmentAcquireLease?.(params),
|
||||
plugin.definition.onEnvironmentAcquireLease?.(params),
|
||||
]);
|
||||
const requests = mockCreate.mock.calls.map(([request]) => request);
|
||||
expect(requests).toHaveLength(2);
|
||||
expect(new Set(requests.map((r) => r.name)).size).toBe(2);
|
||||
expect(new Set(requests.map((r) => r.labels["paperclip-create-attempt"])).size).toBe(2);
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects ambiguous or invalid config", async () => {
|
||||
await expect(plugin.definition.onEnvironmentValidateConfig?.({
|
||||
driverKey: "daytona",
|
||||
|
||||
@@ -942,18 +942,41 @@ async function createSandbox(
|
||||
throw new Error(resourceRequestError);
|
||||
}
|
||||
const client = createDaytonaClient(config);
|
||||
const createParams = buildCreateParams(config, buildSandboxLabels({
|
||||
// The SDK can create a sandbox and then throw while waiting for it to start.
|
||||
// Keep an attempt-specific provider name so that failure cannot lose ownership.
|
||||
const attemptId = randomUUID();
|
||||
const name = `paperclip-create-${attemptId}`;
|
||||
const labels = { ...buildSandboxLabels({
|
||||
companyId: params.companyId,
|
||||
environmentId: params.environmentId,
|
||||
runId: "runId" in params ? params.runId : undefined,
|
||||
setupSessionId: "sessionId" in params ? params.sessionId : undefined,
|
||||
purpose: options.purpose,
|
||||
reuseLease: config.reuseLease,
|
||||
}));
|
||||
const sandbox = await client.create(createParams, {
|
||||
timeout: toTimeoutSeconds(config.timeoutMs),
|
||||
});
|
||||
return sandbox;
|
||||
}), "paperclip-create-attempt": attemptId };
|
||||
const createParams = { ...buildCreateParams(config, labels), name };
|
||||
try {
|
||||
return await client.create(createParams, {
|
||||
timeout: toTimeoutSeconds(config.timeoutMs),
|
||||
});
|
||||
} catch (createError) {
|
||||
try {
|
||||
// A not-found lookup after an uncertain create is not a deletion receipt:
|
||||
// the provider may still materialize the request. Keep the name in the
|
||||
// error so cleanup can be reconciled rather than declaring success.
|
||||
const orphan = await withLivenessTimeout("sandbox.failedCreateLookup", 10_000, () => client.get(name));
|
||||
if (orphan.name !== name || Object.entries(labels).some(([key, value]) => orphan.labels?.[key] !== value)) {
|
||||
throw new Error("Failed-create sandbox ownership does not match");
|
||||
}
|
||||
// Wait for provider deletion, rather than treating a deletion request as
|
||||
// a cleanup receipt. Never run tools or install credentials in this sandbox.
|
||||
await withLivenessTimeout("sandbox.failedCreateDelete", 15_000, () => orphan.delete(10, true));
|
||||
} catch (cleanupError) {
|
||||
throw new AggregateError([createError, cleanupError],
|
||||
`Daytona sandbox creation failed; cleanup could not be confirmed for ${name}`);
|
||||
}
|
||||
throw createError;
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Per-lease started-sandbox handle cache ──────────────────────────────────
|
||||
|
||||
Reference in new issue
Block a user