Consolidate Azure volumes from 2 to 1 unified volume #65
Conversation
WalkthroughConsolidates separate Azure File shares (home and workspace) into a single unified volume throughout the codebase. Updates agent configuration, environment service logic, container provisioning, and Docker compose files to reference the consolidated storage model. Updates example API documentation URLs to reflect new workspace endpoints. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant EnvService as Environment Service
participant Volume as Azure File Share
participant Container as Container Group
rect rgb(200, 220, 255)
note over EnvService,Container: New Unified Flow
Client->>EnvService: Create Environment
EnvService->>Volume: Create unified fs-{workspaceID}
Volume-->>EnvService: Share created
EnvService->>Container: Create container with unified volume
Container-->>EnvService: Container started
EnvService-->>Client: Environment ready
end
rect rgb(255, 220, 200)
note over EnvService,Container: Previous Separate Flow (removed)
Client->>EnvService: Create Environment
EnvService->>Volume: Create home share (parallel)
EnvService->>Volume: Create workspace share (parallel)
Volume-->>EnvService: Both shares ready
EnvService->>Container: Create container with 2 volumes
Container-->>EnvService: Container started
EnvService-->>Client: Environment ready
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/agent/internal/azure/client.go (1)
107-117: Update Dockerfile defaults and entrypoint scripts to use new workspace path.The env var update is correct, but several foundational references still default to
/workspace:
docker/images/00-base/Dockerfile:85:WORKSPACE_DIR=/workspace→/home/dev8/workspacedocker/images/20-vscode/entrypoint.sh:18: fallback default/workspace→/home/dev8/workspacedocker/images/30-ai-tools/entrypoint.sh:18: fallback default/workspace→/home/dev8/workspaceapps/supervisor/internal/config/config.go:65: config fallback/workspace→/home/dev8/workspace- Volume mount targets in
docker/docker-compose.yml:73,docker/Makefile:64,docker/scripts/build.sh:148: ensure mount target aligns (either mount to/home/dev8/workspaceor keep/workspaceconsistency with all defaults)Without these updates, the env var set in
azure/client.go:112won't prevent fallback to unmounted paths.
🧹 Nitpick comments (1)
apps/agent/internal/config/config.go (1)
24-29: Consider adding validation for ContainerImageName.The new
ContainerImageNamefield is loaded from the environment but isn't validated in theValidate()method (lines 199-228), unlikeContainerImagewhich is validated at line 215. If this field is required for operation, consider adding validation. If it's optional, documenting this with a comment would be helpful.Can you verify whether
ContainerImageNamerequires validation, or if it's intentionally optional?Also applies to: 72-72
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/agent/API_DOCUMENTATION.md(1 hunks)apps/agent/internal/azure/client.go(2 hunks)apps/agent/internal/config/config.go(2 hunks)apps/agent/internal/models/environment.go(2 hunks)apps/agent/internal/services/environment.go(10 hunks)docker/CONTAINER_CAPABILITIES.md(1 hunks)docker/README.md(2 hunks)docker/deploy-to-aci.sh(2 hunks)docker/docker-compose.prod.yml(1 hunks)docker/docker-compose.yml(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
docker/deploy-to-aci.sh (1)
docker/shared/scripts/common.sh (1)
log_info(15-17)
apps/agent/internal/services/environment.go (1)
apps/agent/internal/models/environment.go (2)
ErrInternalServer(323-325)ErrNotFound(319-321)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-base
- GitHub Check: Build and Push
🔇 Additional comments (8)
docker/docker-compose.prod.yml (1)
73-85: LGTM! Clean consolidation to unified volume.The single volume configuration is well-documented and correctly structured. The comments clearly explain the new layout where workspace data resides at /home/dev8/workspace within the unified volume.
docker/README.md (2)
100-104: LGTM! Clear documentation of unified volume structure.The volume documentation accurately describes the consolidated storage approach with helpful details about what's stored under /home/dev8.
461-464: LGTM! Documentation reflects unified storage approach.The Azure File Share section correctly documents the single dev8-data share creation with appropriate quota for combined home and workspace data.
docker/docker-compose.yml (1)
69-75: LGTM! Unified volume configuration is consistent.The docker-compose configuration correctly implements the single dev8-data volume with clear documentation. The mount path and volume declaration align with the production configuration.
Also applies to: 99-109
apps/agent/internal/models/environment.go (1)
31-31: LGTM! Documentation updates reflect unified volume structure.The comment updates accurately describe the consolidated storage approach and clarify the purpose of the unified AzureFileShare field.
Also applies to: 56-56
docker/CONTAINER_CAPABILITIES.md (1)
247-263: LGTM! Backup/restore workflow correctly updated.All volume references in the backup/restore examples have been consistently updated from docker_dev8-home to docker_dev8-data, maintaining the workflow's functionality.
docker/deploy-to-aci.sh (2)
145-154: LGTM! Deployment script correctly implements unified storage.The script properly creates a single dev8-data file share with an appropriate 200GB quota for combined home and workspace data. The comments clearly explain the storage structure.
216-217: LGTM! Volume mount configuration is correct.The Azure File volume mount is correctly configured to use the unified dev8-data share at /home/dev8, consistent with the docker-compose configurations.
| "vscode": "http://ws-clxxx-yyyy-zzzz-aaaa-cccc.centralindia.azurecontainer.io", | ||
| "ssh": "ssh dev8@ws-clxxx-yyyy-zzzz-aaaa-cccc.centralindia.azurecontainer.io" |
There was a problem hiding this comment.
Fix inconsistent workspace IDs in example response.
The connection URLs use workspace ID clxxx-yyyy-zzzz-aaaa-cccc (line 274-275), but other fields in the same response use clxxx-yyyy-zzzz-aaaa-bbbb:
- Line 260:
"id": "clxxx-yyyy-zzzz-aaaa-bbbb" - Line 271:
"azureFileShare": "fs-clxxx-yyyy-zzzz-aaaa-bbbb" - Line 272:
"azureFqdn": "ws-clxxx-yyyy-zzzz-aaaa-bbbb.centralindia.azurecontainer.io"
All fields in the example response should use the same workspace ID for consistency.
Additionally, note the protocol change from https to http at line 274, while line 340 uses https for the same URL type. Ensure this is intentional.
Apply this diff to fix the workspace ID consistency:
- "vscode": "http://ws-clxxx-yyyy-zzzz-aaaa-cccc.centralindia.azurecontainer.io",
- "ssh": "ssh [email protected]"
+ "vscode": "http://ws-clxxx-yyyy-zzzz-aaaa-bbbb.centralindia.azurecontainer.io",
+ "ssh": "ssh [email protected]"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "vscode": "http://ws-clxxx-yyyy-zzzz-aaaa-cccc.centralindia.azurecontainer.io", | |
| "ssh": "ssh dev8@ws-clxxx-yyyy-zzzz-aaaa-cccc.centralindia.azurecontainer.io" | |
| "vscode": "http://ws-clxxx-yyyy-zzzz-aaaa-bbbb.centralindia.azurecontainer.io", | |
| "ssh": "ssh dev8@ws-clxxx-yyyy-zzzz-aaaa-bbbb.centralindia.azurecontainer.io" |
🤖 Prompt for AI Agents
In apps/agent/API_DOCUMENTATION.md around lines 260–275, the example response
mixes workspace IDs (some use clxxx-yyyy-zzzz-aaaa-bbbb while the vscode/ssh
URLs use clxxx-yyyy-zzzz-aaaa-cccc) and the vscode URL uses http while elsewhere
https is used; update the vscode and ssh connection strings to use the same
workspace ID clxxx-yyyy-zzzz-aaaa-bbbb as the other fields and change the
protocol to https for the vscode URL (ensure ssh remains a valid ssh URI with
the corrected host) so all example fields are consistent.
Summary
This PR consolidates the storage architecture from 2 separate volumes to 1 unified volume across both Docker and Azure Container Instances deployments.
Problem
Previously, we mounted 2 separate Azure File Shares:
workspacevolume →/workspace(user code/projects)homevolume →/home/dev8(configs, extensions, packages)This created unnecessary complexity in:
Solution
Now we use a single unified volume:
dev8-data→/home/dev8(contains everything)/home/dev8/home/dev8/workspacesubdirectoryChanges Made
1. Agent Service (
apps/agent/)internal/azure/client.go:HomeFileShareNamefield fromContainerGroupSpecdev8-datamounted to/home/dev8WORKSPACE_DIRenv var:/workspace→/home/dev8/workspaceinternal/services/environment.go:internal/models/environment.go:2. Docker Configuration
docker-compose.yml&docker-compose.prod.yml:dev8-datareplacesdev8-home+dev8-workspacedeploy-to-aci.sh:dev8-data(200GB quota)3. Documentation
Testing
Benefits
Migration Notes
For existing workspaces:
Volume Structure
Files Changed
apps/agent/internal/azure/client.go(-29 lines)apps/agent/internal/services/environment.go(-35 lines)apps/agent/internal/models/environment.go(comments)docker/docker-compose.yml(-29 lines)docker/docker-compose.prod.yml(-13 lines)docker/deploy-to-aci.sh(updated comments)Total: -76 net lines, simplified architecture
Summary by CodeRabbit
New Features
Chores