Skip to content

Commit 9036cae

Browse files
EmmaLouise2018EmmaLouise2018
authored
Merge pull request #634 from adcontextprotocol/EmmaLouise2018/fix-docker-test-cleanup
fix: Docker test cleanup to prevent 100GB+ resource accumulation
2 parents 3291ae6 + 9ed12fd commit 9036cae

File tree

4 files changed

+272
-5
lines changed

4 files changed

+272
-5
lines changed

‎docs/testing/docker-cleanup.md‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
# Docker Cleanup for AdCP Tests
2+
3+
## The Problem
4+
5+
Running AdCP tests can accumulate Docker resources over time:
6+
7+
1. **E2E tests** create full Docker Compose stacks (~1.5GB per run)
8+
2. **Conductor workspaces** create unique Docker images per workspace (`amman-adcp-server`, `monaco-adcp-server`, etc.)
9+
3. **Integration tests** create per-test PostgreSQL databases (cleaned up in Python but leave Docker state)
10+
4. **Dangling volumes** accumulate from incomplete test runs
11+
12+
**Impact**: 100GB+ of Docker resources can accumulate over weeks of development.
13+
14+
## The Solution
15+
16+
### Automatic Cleanup (Built-in)
17+
18+
**✅ Fixed in this PR:**
19+
- E2E tests now clean up Docker resources automatically after each test session
20+
- `run_all_tests.sh` prunes dangling volumes on teardown
21+
- Only test containers are affected - your local dev environment is safe
22+
23+
### Manual Cleanup Script
24+
25+
Run the cleanup script periodically:
26+
27+
```bash
28+
./scripts/cleanup_docker.sh
29+
```
30+
31+
This removes:
32+
- ✅ Stopped test containers (`adcp-test-*`)
33+
- ✅ Dangling/unused volumes
34+
- ✅ Old workspace images (keeps 2 most recent)
35+
- ✅ Dangling intermediate images
36+
37+
**Safe to run**: Your local dev environment (`docker-compose up`) is not affected.
38+
39+
### What Gets Cleaned
40+
41+
| Resource Type | Pattern | Cleanup Method | Safe? |
42+
|--------------|---------|----------------|-------|
43+
| Test containers | `adcp-test-*` | `docker rm -f` | ✅ Yes |
44+
| Test volumes | Dangling/unused | `docker volume prune -f` | ✅ Yes |
45+
| Workspace images | `*-adcp-server` | Keep 2 newest, remove rest | ✅ Yes |
46+
| Cache volumes | `adcp_global_*_cache` | Preserved (labeled) | ✅ Never removed |
47+
| Dev containers | `docker-compose.yml` | Not touched | ✅ Never removed |
48+
49+
### Best Practices
50+
51+
**1. Prefer Quick Mode for Iteration**
52+
```bash
53+
./run_all_tests.sh quick # No Docker (fast, ~1 min)
54+
```
55+
56+
**2. Use CI Mode Sparingly**
57+
```bash
58+
./run_all_tests.sh ci # Full Docker stack (~5 min)
59+
```
60+
61+
**3. Run Cleanup Weekly**
62+
```bash
63+
./scripts/cleanup_docker.sh
64+
```
65+
66+
**4. Check Docker Usage**
67+
```bash
68+
docker system df # See what's using space
69+
```
70+
71+
### Understanding Docker Usage
72+
73+
```bash
74+
# See all AdCP-related images
75+
docker images --filter "reference=*adcp*"
76+
77+
# See all test containers (running and stopped)
78+
docker ps -a --filter "name=adcp-test-"
79+
80+
# See all volumes
81+
docker volume ls --filter "name=adcp"
82+
83+
# Total Docker disk usage
84+
docker system df
85+
```
86+
87+
### When to Run Cleanup
88+
89+
**Weekly maintenance:**
90+
- After heavy development sessions
91+
- Before/after working on different Conductor workspaces
92+
- When Docker disk usage is high (`docker system df`)
93+
94+
**Symptoms of accumulation:**
95+
- Docker using 50GB+ (`docker system df`)
96+
- Slow Docker operations
97+
- "No space left on device" errors
98+
99+
## Technical Details
100+
101+
### Why This Happened
102+
103+
1. **E2E tests (`tests/e2e/conftest.py`):**
104+
- Cleanup code at end of `docker_services_e2e` was commented out
105+
- Each E2E test run left containers and volumes behind
106+
- **Fixed**: Added proper cleanup in fixture teardown
107+
108+
2. **Conductor workspaces:**
109+
- Each workspace (`amman`, `monaco`, etc.) builds its own Docker image
110+
- Images are 1.4-1.7GB each
111+
- **Fixed**: Cleanup script removes old workspace images (keeps 2 most recent)
112+
113+
3. **Integration tests:**
114+
- Create unique PostgreSQL databases (`test_a3f8d92c`) per test
115+
- Databases cleaned in Python, but Docker metadata accumulates
116+
- **Fixed**: Added `docker volume prune` to teardown
117+
118+
### Implementation
119+
120+
**E2E Test Cleanup (`tests/e2e/conftest.py`):**
121+
```python
122+
# After yield in docker_services_e2e fixture
123+
if not use_existing_services:
124+
print("\n🧹 Cleaning up Docker resources...")
125+
subprocess.run(["docker-compose", "down", "-v"], capture_output=True)
126+
subprocess.run(["docker", "volume", "prune", "-f"], capture_output=True)
127+
```
128+
129+
**Test Runner Cleanup (`run_all_tests.sh`):**
130+
```bash
131+
teardown_docker_stack() {
132+
docker-compose -p "$COMPOSE_PROJECT_NAME" down -v
133+
docker volume prune -f --filter "label!=preserve"
134+
}
135+
```
136+
137+
**Manual Cleanup Script (`scripts/cleanup_docker.sh`):**
138+
- Removes test containers by name pattern
139+
- Prunes dangling volumes (except labeled as `preserve`)
140+
- Removes old workspace images (keeps 2 most recent)
141+
- Shows before/after disk usage
142+
143+
### Prevention
144+
145+
**1. Quick mode for development:**
146+
Uses integration_db fixture with real PostgreSQL but no Docker Compose overhead.
147+
148+
**2. CI mode only when necessary:**
149+
Full Docker stack is needed for E2E tests but not for rapid iteration.
150+
151+
**3. Automatic cleanup:**
152+
Tests now clean up after themselves - no manual intervention needed.
153+
154+
**4. Periodic maintenance:**
155+
Run `./scripts/cleanup_docker.sh` weekly to remove orphaned resources.
156+
157+
## References
158+
159+
- Issue: 100GB of Docker resources accumulated from test runs
160+
- Root cause: E2E test cleanup was disabled, workspace images accumulated
161+
- Fix: [This PR] Re-enabled cleanup + added maintenance script
162+
- Prevention: Use quick mode for iteration, CI mode for validation

‎run_all_tests.sh‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,12 @@ print(' '.join(map(str, ports)))
177177
teardown_docker_stack() {
178178
echo -e "${BLUE}🐳 Stopping TEST Docker stack (project: $COMPOSE_PROJECT_NAME)...${NC}"
179179
docker-compose -p "$COMPOSE_PROJECT_NAME" down -v 2>/dev/null || true
180-
echo -e "${GREEN}✓ Test containers cleaned up (your local dev containers are untouched)${NC}"
180+
181+
# Prune dangling volumes created by tests (only removes unused volumes)
182+
echo "Cleaning up dangling Docker volumes..."
183+
docker volume prune -f --filter "label!=preserve" 2>/dev/null || true
184+
185+
echo -e "${GREEN}✓ Test containers and volumes cleaned up (your local dev containers are untouched)${NC}"
181186
}
182187

183188
# Trap to ensure cleanup on exit

‎scripts/cleanup_docker.sh‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
#!/bin/bash
2+
# Docker cleanup script for test resources
3+
# Safe to run - only removes test containers/images/volumes, not your local dev environment
4+
5+
set -e
6+
7+
# Color codes
8+
RED='\033[0;31m'
9+
GREEN='\033[0;32m'
10+
YELLOW='\033[1;33m'
11+
BLUE='\033[0;34m'
12+
NC='\033[0m' # No Color
13+
14+
echo -e "${BLUE}🧹 AdCP Test Docker Cleanup Script${NC}"
15+
echo ""
16+
echo "This script removes:"
17+
echo " • Stopped test containers (adcp-test-*)"
18+
echo " • Dangling/unused volumes"
19+
echo " • Old workspace images (keeps latest)"
20+
echo ""
21+
echo -e "${YELLOW}⚠️ Your local dev environment (docker-compose up) will NOT be affected${NC}"
22+
echo ""
23+
24+
# Ask for confirmation
25+
read -p "Continue? (y/N) " -n 1 -r
26+
echo
27+
if [[ ! $REPLY =~ ^[Yy]$ ]]; then
28+
echo "Cancelled."
29+
exit 0
30+
fi
31+
32+
echo ""
33+
echo -e "${BLUE}Step 1: Stopping and removing test containers...${NC}"
34+
# Remove all containers with adcp-test prefix
35+
TEST_CONTAINERS=$(docker ps -a --filter "name=adcp-test-" --format "{{.ID}}" | wc -l | tr -d ' ')
36+
if [ "$TEST_CONTAINERS" -gt 0 ]; then
37+
echo "Found $TEST_CONTAINERS test containers"
38+
docker ps -a --filter "name=adcp-test-" --format "{{.ID}}" | xargs -r docker rm -f 2>/dev/null || true
39+
echo -e "${GREEN}✓ Removed test containers${NC}"
40+
else
41+
echo "No test containers found"
42+
fi
43+
44+
echo ""
45+
echo -e "${BLUE}Step 2: Removing dangling volumes...${NC}"
46+
# Prune volumes not attached to any container
47+
BEFORE_VOLUMES=$(docker volume ls -q | wc -l | tr -d ' ')
48+
docker volume prune -f --filter "label!=preserve" 2>/dev/null || true
49+
AFTER_VOLUMES=$(docker volume ls -q | wc -l | tr -d ' ')
50+
REMOVED_VOLUMES=$((BEFORE_VOLUMES - AFTER_VOLUMES))
51+
echo -e "${GREEN}✓ Removed $REMOVED_VOLUMES dangling volumes${NC}"
52+
53+
echo ""
54+
echo -e "${BLUE}Step 3: Cleaning up old workspace images...${NC}"
55+
# List all adcp-server images sorted by date
56+
echo "Current workspace images:"
57+
docker images --filter "reference=*-adcp-server" --format "table {{.Repository}}\t{{.Tag}}\t{{.Size}}\t{{.CreatedAt}}"
58+
59+
echo ""
60+
echo "Keeping only the 2 most recent workspace images..."
61+
62+
# Get all workspace image IDs except the 2 most recent
63+
OLD_IMAGES=$(docker images --filter "reference=*-adcp-server" --format "{{.ID}}" | tail -n +3)
64+
if [ -n "$OLD_IMAGES" ]; then
65+
echo "$OLD_IMAGES" | xargs -r docker rmi -f 2>/dev/null || true
66+
echo -e "${GREEN}✓ Removed old workspace images${NC}"
67+
else
68+
echo "No old workspace images to remove"
69+
fi
70+
71+
echo ""
72+
echo -e "${BLUE}Step 4: Pruning dangling images...${NC}"
73+
# Remove dangling images (intermediate layers not used by any container)
74+
docker image prune -f 2>/dev/null || true
75+
echo -e "${GREEN}✓ Pruned dangling images${NC}"
76+
77+
echo ""
78+
echo -e "${GREEN}✅ Cleanup complete!${NC}"
79+
echo ""
80+
echo "Current Docker usage:"
81+
docker system df
82+
83+
echo ""
84+
echo -e "${BLUE}💡 Tips to prevent accumulation:${NC}"
85+
echo " • Run this script weekly: ./scripts/cleanup_docker.sh"
86+
echo " • Use './run_all_tests.sh quick' for fast iteration (no Docker)"
87+
echo " • Use './run_all_tests.sh ci' only when needed (full Docker stack)"
88+
echo " • Docker volumes for dev (adcp_global_*_cache) are preserved"

‎tests/e2e/conftest.py‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,10 +324,22 @@ def docker_services_e2e(request):
324324
# Yield port information for use by other fixtures
325325
yield {"mcp_port": mcp_port, "a2a_port": a2a_port, "admin_port": admin_port, "postgres_port": postgres_port}
326326

327-
# Cleanup based on --keep-data flag
328-
# Note: pytest.config.getoption is not available in yield, would need request fixture
329-
# For now, skip cleanup
330-
pass
327+
# Cleanup Docker resources (unless --skip-docker was used, meaning services are external)
328+
if not use_existing_services:
329+
print("\n🧹 Cleaning up Docker resources...")
330+
try:
331+
# Stop and remove containers + volumes
332+
subprocess.run(["docker-compose", "down", "-v"], capture_output=True, check=False, timeout=30)
333+
print("✓ Docker containers and volumes cleaned up")
334+
335+
# Prune dangling volumes (created by tests but not tracked by docker-compose)
336+
result = subprocess.run(["docker", "volume", "prune", "-f"], capture_output=True, text=True, timeout=10)
337+
if result.stdout:
338+
print(f"✓ Pruned volumes: {result.stdout.strip()}")
339+
except subprocess.TimeoutExpired:
340+
print("⚠️ Warning: Docker cleanup timed out (non-fatal)")
341+
except Exception as e:
342+
print(f"⚠️ Warning: Docker cleanup failed (non-fatal): {e}")
331343

332344

333345
@pytest.fixture

0 commit comments

Comments
 (0)