Skip to content

Commit 8545ea8

Browse files
committed
Refactor container naming: base image default, Python explicit
- Rename SandboxBase → SandboxPython in test worker - Flip header logic: X-Sandbox-Type: python selects Python image - Default (no header) now uses base image (smaller, no Python) - Update cleanup script for -python suffix - Remove per-run CI cleanup (resources persist until PR closes)
1 parent ff7138a commit 8545ea8

File tree

9 files changed

+100
-71
lines changed

9 files changed

+100
-71
lines changed

.github/workflows/pullrequest.yml

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -160,16 +160,10 @@ jobs:
160160
AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
161161
AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
162162

163-
# Cleanup: Delete test worker and container (only for PR environments)
164-
- name: Cleanup test deployment
165-
if: always() && github.event_name == 'pull_request'
166-
continue-on-error: true
167-
run: |
168-
cd tests/e2e/test-worker
169-
../../../scripts/cleanup-test-deployment.sh ${{ steps.env-name.outputs.worker_name }}
170-
env:
171-
CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }}
172-
CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
163+
# Note: Resources are NOT cleaned up after each run to speed up subsequent CI runs.
164+
# Cleanup happens via:
165+
# - cleanup.yml: Triggered when PR is closed
166+
# - cleanup-stale.yml: Daily cron job for orphaned/stale resources
173167

174168
# Validate changesets don't contain internal packages
175169
validate-changesets:

scripts/cleanup-test-deployment.sh

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@
22
set -e
33

44
# Cleanup Test Deployment Script
5-
# Deletes a test worker and its associated container with proper ordering and retry logic
5+
# Deletes a test worker and its associated containers with proper ordering and retry logic
66
#
77
# Usage: ./cleanup-test-deployment.sh <worker-name>
88
# Example: ./cleanup-test-deployment.sh sandbox-e2e-test-worker-pr-123
99
#
10+
# This script handles multiple container variants:
11+
# - <worker-name>: Base image container (no Python, default)
12+
# - <worker-name>-python: Python image container
13+
#
1014
# Environment variables required:
1115
# - CLOUDFLARE_API_TOKEN
1216
# - CLOUDFLARE_ACCOUNT_ID
@@ -21,30 +25,42 @@ fi
2125

2226
echo "=== Starting cleanup for $WORKER_NAME ==="
2327

24-
# Step 1: Get container ID BEFORE deleting worker (critical order!)
25-
echo "Looking up container ID..."
28+
# Step 1: Get container IDs BEFORE deleting worker (critical order!)
29+
echo "Looking up container IDs..."
2630

2731
# Get container list (wrangler outputs JSON by default, no --json flag needed)
2832
RAW_OUTPUT=$(npx wrangler containers list 2>&1)
2933

34+
CONTAINER_ID=""
35+
CONTAINER_PYTHON_ID=""
36+
3037
# Check if output looks like JSON (starts with '[')
3138
if echo "$RAW_OUTPUT" | grep -q '^\['; then
3239
echo "✓ Got JSON output from wrangler containers list"
3340

34-
# Parse JSON to find container
41+
# Parse JSON to find both containers
3542
CONTAINER_ID=$(echo "$RAW_OUTPUT" | jq -r ".[] | select(.name==\"$WORKER_NAME\") | .id" 2>/dev/null || echo "")
43+
CONTAINER_PYTHON_ID=$(echo "$RAW_OUTPUT" | jq -r ".[] | select(.name==\"$WORKER_NAME-python\") | .id" 2>/dev/null || echo "")
3644

3745
if [ -n "$CONTAINER_ID" ]; then
38-
echo "✓ Found container: $CONTAINER_ID"
46+
echo "✓ Found base container: $CONTAINER_ID"
3947
else
40-
echo "⚠️ No container found for worker $WORKER_NAME"
48+
echo "⚠️ No base container found for $WORKER_NAME"
49+
fi
50+
51+
if [ -n "$CONTAINER_PYTHON_ID" ]; then
52+
echo "✓ Found python container: $CONTAINER_PYTHON_ID"
53+
else
54+
echo "⚠️ No python container found for $WORKER_NAME-python"
55+
fi
56+
57+
if [ -z "$CONTAINER_ID" ] && [ -z "$CONTAINER_PYTHON_ID" ]; then
4158
echo "Available containers:"
4259
echo "$RAW_OUTPUT" | jq -r '.[].name' 2>/dev/null || echo "(unable to parse container names)"
4360
fi
4461
else
4562
echo "⚠️ Non-JSON output from wrangler containers list:"
4663
echo "$RAW_OUTPUT"
47-
CONTAINER_ID=""
4864
fi
4965

5066
# Step 2: Delete worker
@@ -55,23 +71,41 @@ else
5571
echo "⚠️ Worker deletion failed or already deleted"
5672
fi
5773

58-
# Step 3: Delete container with retry logic (if we found one)
59-
if [ -n "$CONTAINER_ID" ]; then
60-
echo "Deleting container with retry logic..."
74+
# Function to delete a container with retry logic
75+
delete_container() {
76+
local container_id=$1
77+
local container_name=$2
78+
79+
if [ -z "$container_id" ]; then
80+
return 0
81+
fi
82+
83+
echo "Deleting $container_name container with retry logic..."
6184
for i in 1 2 3; do
62-
if npx wrangler containers delete "$CONTAINER_ID" 2>/dev/null; then
63-
echo "Container deleted successfully"
64-
break
85+
if npx wrangler containers delete "$container_id" 2>/dev/null; then
86+
echo "$container_name container deleted successfully"
87+
return 0
6588
else
6689
if [ $i -lt 3 ]; then
67-
echo "⚠️ Container deletion attempt $i/3 failed, retrying in 5s..."
90+
echo "⚠️ $container_name container deletion attempt $i/3 failed, retrying in 5s..."
6891
sleep 5
6992
else
70-
echo "Container deletion failed after 3 attempts"
71-
exit 1
93+
echo "$container_name container deletion failed after 3 attempts"
94+
return 1
7295
fi
7396
fi
7497
done
98+
}
99+
100+
# Step 3: Delete containers
101+
CLEANUP_FAILED=false
102+
103+
delete_container "$CONTAINER_ID" "base" || CLEANUP_FAILED=true
104+
delete_container "$CONTAINER_PYTHON_ID" "python" || CLEANUP_FAILED=true
105+
106+
if [ "$CLEANUP_FAILED" = true ]; then
107+
echo "=== Cleanup completed with errors ==="
108+
exit 1
75109
fi
76110

77111
echo "=== Cleanup complete ==="

tests/e2e/code-interpreter-workflow.test.ts

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import { getTestWorkerUrl, WranglerDevRunner } from './helpers/wrangler-runner';
2727
import {
2828
createSandboxId,
2929
createTestHeaders,
30-
createBaseImageHeaders,
30+
createPythonImageHeaders,
3131
cleanupSandbox
3232
} from './helpers/test-fixtures';
3333
import type { CodeContext, ExecutionResult } from '@repo/shared';
@@ -64,7 +64,7 @@ describe('Code Interpreter Workflow (E2E)', () => {
6464

6565
test('should create and list code contexts', async () => {
6666
currentSandboxId = createSandboxId();
67-
const headers = createTestHeaders(currentSandboxId);
67+
const headers = createPythonImageHeaders(currentSandboxId);
6868

6969
// Create Python context
7070
const pythonCtxResponse = await fetch(
@@ -112,7 +112,7 @@ describe('Code Interpreter Workflow (E2E)', () => {
112112

113113
test('should delete code context', async () => {
114114
currentSandboxId = createSandboxId();
115-
const headers = createTestHeaders(currentSandboxId);
115+
const headers = createPythonImageHeaders(currentSandboxId);
116116

117117
// Create context
118118
const createResponse = await fetch(`${workerUrl}/api/code/context/create`, {
@@ -154,7 +154,7 @@ describe('Code Interpreter Workflow (E2E)', () => {
154154

155155
test('should execute simple Python code', async () => {
156156
currentSandboxId = createSandboxId();
157-
const headers = createTestHeaders(currentSandboxId);
157+
const headers = createPythonImageHeaders(currentSandboxId);
158158

159159
// Create Python context
160160
const ctxResponse = await fetch(`${workerUrl}/api/code/context/create`, {
@@ -185,7 +185,7 @@ describe('Code Interpreter Workflow (E2E)', () => {
185185

186186
test('should maintain Python state across executions', async () => {
187187
currentSandboxId = createSandboxId();
188-
const headers = createTestHeaders(currentSandboxId);
188+
const headers = createPythonImageHeaders(currentSandboxId);
189189

190190
// Create context
191191
const ctxResponse = await fetch(`${workerUrl}/api/code/context/create`, {
@@ -228,7 +228,7 @@ describe('Code Interpreter Workflow (E2E)', () => {
228228

229229
test('should handle Python errors gracefully', async () => {
230230
currentSandboxId = createSandboxId();
231-
const headers = createTestHeaders(currentSandboxId);
231+
const headers = createPythonImageHeaders(currentSandboxId);
232232

233233
// Create context
234234
const ctxResponse = await fetch(`${workerUrl}/api/code/context/create`, {
@@ -267,7 +267,7 @@ describe('Code Interpreter Workflow (E2E)', () => {
267267

268268
test('should execute simple JavaScript code', async () => {
269269
currentSandboxId = createSandboxId();
270-
const headers = createBaseImageHeaders(currentSandboxId);
270+
const headers = createTestHeaders(currentSandboxId);
271271

272272
// Create JavaScript context
273273
const ctxResponse = await fetch(`${workerUrl}/api/code/context/create`, {
@@ -297,7 +297,7 @@ describe('Code Interpreter Workflow (E2E)', () => {
297297

298298
test('should maintain JavaScript state across executions', async () => {
299299
currentSandboxId = createSandboxId();
300-
const headers = createBaseImageHeaders(currentSandboxId);
300+
const headers = createTestHeaders(currentSandboxId);
301301

302302
// Create context
303303
const ctxResponse = await fetch(`${workerUrl}/api/code/context/create`, {
@@ -337,7 +337,7 @@ describe('Code Interpreter Workflow (E2E)', () => {
337337

338338
test('should handle JavaScript errors gracefully', async () => {
339339
currentSandboxId = createSandboxId();
340-
const headers = createBaseImageHeaders(currentSandboxId);
340+
const headers = createTestHeaders(currentSandboxId);
341341

342342
// Create context
343343
const ctxResponse = await fetch(`${workerUrl}/api/code/context/create`, {
@@ -375,7 +375,7 @@ describe('Code Interpreter Workflow (E2E)', () => {
375375

376376
test('should stream Python execution output', async () => {
377377
currentSandboxId = createSandboxId();
378-
const headers = createTestHeaders(currentSandboxId);
378+
const headers = createPythonImageHeaders(currentSandboxId);
379379

380380
// Create context
381381
const ctxResponse = await fetch(`${workerUrl}/api/code/context/create`, {
@@ -457,7 +457,7 @@ for i in range(3):
457457

458458
test('should process data in Python and consume in JavaScript', async () => {
459459
currentSandboxId = createSandboxId();
460-
const headers = createTestHeaders(currentSandboxId);
460+
const headers = createPythonImageHeaders(currentSandboxId);
461461

462462
// Create Python context
463463
const pythonCtxResponse = await fetch(
@@ -528,7 +528,7 @@ console.log('Sum:', sum);
528528

529529
test('should isolate variables between contexts', async () => {
530530
currentSandboxId = createSandboxId();
531-
const headers = createTestHeaders(currentSandboxId);
531+
const headers = createPythonImageHeaders(currentSandboxId);
532532

533533
// Create two Python contexts
534534
const ctx1Response = await fetch(`${workerUrl}/api/code/context/create`, {
@@ -583,7 +583,7 @@ console.log('Sum:', sum);
583583

584584
test('should maintain isolation across many contexts (12+)', async () => {
585585
currentSandboxId = createSandboxId();
586-
const headers = createBaseImageHeaders(currentSandboxId);
586+
const headers = createTestHeaders(currentSandboxId);
587587

588588
// Create 12 contexts
589589
const contexts: CodeContext[] = [];
@@ -653,7 +653,7 @@ console.log('Sum:', sum);
653653

654654
test('should maintain state isolation with concurrent context execution', async () => {
655655
currentSandboxId = createSandboxId();
656-
const headers = createBaseImageHeaders(currentSandboxId);
656+
const headers = createTestHeaders(currentSandboxId);
657657

658658
// Create contexts sequentially
659659
const contexts: CodeContext[] = [];
@@ -734,7 +734,7 @@ console.log('Sum:', sum);
734734

735735
test('should prevent concurrent execution on same context', async () => {
736736
currentSandboxId = createSandboxId();
737-
const headers = createBaseImageHeaders(currentSandboxId);
737+
const headers = createTestHeaders(currentSandboxId);
738738

739739
// Create single context
740740
const ctxResponse = await fetch(`${workerUrl}/api/code/context/create`, {
@@ -896,7 +896,8 @@ console.log('Sum:', sum);
896896

897897
test('should return helpful error when Python unavailable on base image', async () => {
898898
currentSandboxId = createSandboxId();
899-
const headers = createBaseImageHeaders(currentSandboxId);
899+
// Use default headers (base image, no Python) to test Python-not-available error
900+
const headers = createTestHeaders(currentSandboxId);
900901

901902
// Try to create Python context on base image (no Python installed)
902903
const response = await fetch(`${workerUrl}/api/code/context/create`, {

tests/e2e/helpers/test-fixtures.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,21 +60,21 @@ export function createTestHeaders(
6060
}
6161

6262
/**
63-
* Create headers for base image sandbox (without Python)
63+
* Create headers for Python image sandbox (with Python)
6464
*
65-
* Use this for testing the lean image variant that doesn't include Python.
66-
* The base image is smaller but only supports JavaScript/shell execution.
65+
* Use this for testing the full image variant that includes Python.
66+
* The Python image is larger but supports Python code execution.
6767
*
6868
* @param sandboxId - Which container instance to use
6969
* @param sessionId - (Optional) Which session within that container
7070
*/
71-
export function createBaseImageHeaders(
71+
export function createPythonImageHeaders(
7272
sandboxId: string,
7373
sessionId?: string
7474
): Record<string, string> {
7575
return {
7676
...createTestHeaders(sandboxId, sessionId),
77-
'X-Sandbox-Type': 'base'
77+
'X-Sandbox-Type': 'python'
7878
};
7979
}
8080

tests/e2e/test-worker/Dockerfile

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
# Integration test Dockerfile
2-
# Uses the -python variant because E2E tests include Python code execution tests
3-
FROM docker.io/cloudflare/sandbox-test:0.5.6-python
1+
# Base image Dockerfile (no Python)
2+
# Used for testing Python-not-available error handling
3+
FROM docker.io/cloudflare/sandbox-test:0.5.6
44

55
# Expose ports used for testing
66
EXPOSE 8080

tests/e2e/test-worker/Dockerfile.base

Lines changed: 0 additions & 6 deletions
This file was deleted.
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# Integration test Dockerfile
2+
# Uses the -python variant because E2E tests include Python code execution tests
3+
FROM docker.io/cloudflare/sandbox-test:0.5.6-python
4+
5+
# Expose ports used for testing
6+
EXPOSE 8080

tests/e2e/test-worker/index.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@
55
* Supports both default sessions (implicit) and explicit sessions via X-Session-Id header.
66
*
77
* Two sandbox types are available:
8-
* - Sandbox: Full image with Python (default)
9-
* - SandboxBase: Base image without Python (for testing Python-not-available errors)
8+
* - Sandbox: Base image without Python (default, lean image)
9+
* - SandboxPython: Full image with Python (for code interpreter tests)
1010
*
11-
* Use X-Sandbox-Type header to select: 'base' for SandboxBase, anything else for Sandbox
11+
* Use X-Sandbox-Type header to select: 'python' for SandboxPython, anything else for Sandbox
1212
*/
1313
import { Sandbox, getSandbox, proxyToSandbox } from '@cloudflare/sandbox';
1414
import type {
@@ -25,14 +25,14 @@ import type {
2525
ErrorResponse
2626
} from './types';
2727

28-
// Export Sandbox twice - once as Sandbox (python image) and once as SandboxBase (base image)
28+
// Export Sandbox twice - once as Sandbox (base image) and once as SandboxPython (python image)
2929
// The actual image is determined by the container binding in wrangler.jsonc
3030
export { Sandbox };
31-
export { Sandbox as SandboxBase };
31+
export { Sandbox as SandboxPython };
3232

3333
interface Env {
3434
Sandbox: DurableObjectNamespace<Sandbox>;
35-
SandboxBase: DurableObjectNamespace<Sandbox>;
35+
SandboxPython: DurableObjectNamespace<Sandbox>;
3636
TEST_BUCKET: R2Bucket;
3737
// R2 credentials for bucket mounting tests
3838
CLOUDFLARE_ACCOUNT_ID?: string;
@@ -66,10 +66,10 @@ export default {
6666
const keepAliveHeader = request.headers.get('X-Sandbox-KeepAlive');
6767
const keepAlive = keepAliveHeader === 'true';
6868

69-
// Select sandbox type: 'base' uses SandboxBase (no Python), anything else uses Sandbox (with Python)
69+
// Select sandbox type: 'python' uses SandboxPython (with Python), anything else uses Sandbox (base, no Python)
7070
const sandboxType = request.headers.get('X-Sandbox-Type');
7171
const sandboxNamespace =
72-
sandboxType === 'base' ? env.SandboxBase : env.Sandbox;
72+
sandboxType === 'python' ? env.SandboxPython : env.Sandbox;
7373
const sandbox = getSandbox(sandboxNamespace, sandboxId, {
7474
keepAlive
7575
});

0 commit comments

Comments
 (0)