Skip to content

Commit cd9d69b

Browse files
committed
Merge #289: docs: [#288] add ADR for bind mount standardization
4134459 docs: [#288] add ADR for bind mount standardization (Jose Celano) Pull request description: ## Summary This PR adds an Architectural Decision Record (ADR) documenting the decision to standardize on bind mounts for all Docker Compose volumes. ## Related Issues - Part of Epic #287: Docker Compose Topology Domain Model Refactoring - Closes #288 ## Changes - **New ADR**: `docs/decisions/bind-mount-standardization.md` - Documents 9 compelling reasons for using bind mounts over named volumes - Covers all services: tracker, database, Prometheus, Grafana, Caddy - Includes SELinux considerations (`:Z` suffix) - Documents permission requirements for non-root containers - Lists 3 rejected alternatives with rationale - **Updated ADR Index**: `docs/decisions/README.md` - Added new entry at top of ADR table ## Key Points from ADR ### Reasons for Bind Mounts 1. **Visibility** - Data visible in project directory 2. **Backup/Restore** - Standard file system tools work 3. **Git Integration** - Can version control config files 4. **Multi-environment** - Easy copy across environments 5. **Debugging** - Inspect without entering containers 6. **Consistency** - Same pattern for all volumes 7. **Documentation** - Directory structure self-documents 8. **Permissions** - Direct control over ownership 9. **Development Workflow** - Direct file editing ### Rejected Alternatives 1. Named volumes only - Hidden, harder to manage 2. Mixed approach - Inconsistent, cognitive overhead 3. Docker volumes with plugins - Adds complexity ## Checklist - [x] ADR follows template format - [x] All linters pass - [x] ADR registered in index ACKs for top commit: josecelano: ACK 4134459 Tree-SHA512: af62bbb0800fdc0411cd7568257cf8ac3c1b8b14b3e6f4a7e3cb05d32961b4b63318c665e958ad575e657748ffcc465731b22af716b196dd462e183e105697e4
2 parents a3a8541 + 4134459 commit cd9d69b

File tree

10 files changed

+195
-23
lines changed

10 files changed

+195
-23
lines changed

docs/decisions/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ This directory contains architectural decision records for the Torrust Tracker D
66

77
| Status | Date | Decision | Summary |
88
| ------------- | ---------- | --------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------ |
9+
| ✅ Accepted | 2026-01-24 | [Bind Mount Standardization](./bind-mount-standardization.md) | Use bind mounts exclusively for all Docker Compose volumes for observability and backup |
910
| ✅ Accepted | 2026-01-21 | [TryFrom for DTO to Domain Conversion](./tryfrom-for-dto-to-domain-conversion.md) | Use standard TryFrom trait for self-documenting, discoverable DTO→Domain conversions |
1011
| ✅ Accepted | 2026-01-21 | [Validated Deserialization for Domain Types](./validated-deserialization-for-domain-types.md) | Use custom Deserialize impl with Raw struct to enforce domain invariants during parsing |
1112
| ✅ Accepted | 2026-01-20 | [Grafana Default Port 3000](./grafana-default-port-3000.md) | Use Grafana's default port 3000 instead of 3100 for dedicated VM deployments |
Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
# Decision: Bind Mount Standardization for Docker Compose
2+
3+
## Status
4+
5+
Accepted
6+
7+
## Date
8+
9+
2026-01-24
10+
11+
## Context
12+
13+
The Docker Compose template currently uses a mix of named volumes and bind mounts for persistent data:
14+
15+
```yaml
16+
# Bind mounts (host path → container path) - CURRENT PATTERN
17+
- ./storage/tracker/lib:/var/lib/torrust/tracker:Z
18+
19+
# Named volumes (volume name → container path) - PROBLEMATIC
20+
- caddy_data:/data
21+
- grafana_data:/var/lib/grafana
22+
- mysql_data:/var/lib/mysql
23+
```
24+
25+
This inconsistency creates several problems:
26+
27+
### 1. Observability
28+
29+
- Named volumes hide data in `/var/lib/docker/volumes/` - not obvious to users
30+
- Users cannot easily see where persistent data is stored
31+
- File system tools (ls, du, find) don't work directly on named volume data
32+
33+
### 2. Backup Complexity
34+
35+
- Named volumes require `docker volume` commands or finding the internal path
36+
- No single command can back up all data
37+
- Docker-specific tooling is required
38+
- Standard backup scripts don't work without modification
39+
40+
### 3. Restore Complexity
41+
42+
- Restoring named volumes requires Docker volume recreation
43+
- Cannot simply copy files to restore data
44+
- Migration between hosts requires Docker volume export/import
45+
46+
### 4. Inconsistency
47+
48+
- Some services use bind mounts, others use named volumes
49+
- Different patterns for different services create cognitive overhead
50+
- No predictable directory structure
51+
52+
### 5. Portability Limitations
53+
54+
- Named volumes cannot be moved between hosts by copying files
55+
- Docker volume export/import dance is required
56+
- Tied to Docker's internal storage format
57+
58+
### 6. Debugging & Troubleshooting Difficulties
59+
60+
- Cannot directly inspect files without entering containers
61+
- Checking file permissions, ownership, disk usage is difficult
62+
- Cannot modify config files directly for debugging
63+
- Log files not accessible without `docker logs`
64+
65+
### 7. Development Experience
66+
67+
- Cannot easily reset state by deleting directories
68+
- Cannot pre-populate data for testing scenarios
69+
- IDE file watchers cannot observe changes in named volumes
70+
71+
### 8. Deployment Architecture Complexity
72+
73+
- Named volumes require a top-level `volumes:` section in docker-compose.yml
74+
- Must derive which volumes are required based on enabled services
75+
- Ansible must manage both directories and Docker volumes
76+
77+
### 9. Security Visibility
78+
79+
- File permissions are hidden inside Docker volume directories
80+
- SELinux labels cannot be applied consistently
81+
- Data locations are not transparent to users
82+
83+
## Decision
84+
85+
Standardize on **bind mounts exclusively** for all persistent data in Docker Compose deployments.
86+
87+
All persistent data will be stored under `./storage/{service}/`:
88+
89+
| Service | Bind Mount | Data Location |
90+
| ---------- | ------------------------------------------ | -------------------------- |
91+
| Tracker | `./storage/tracker/lib:/var/lib/torrust` | `./storage/tracker/lib` |
92+
| Tracker | `./storage/tracker/log:/var/log/torrust` | `./storage/tracker/log` |
93+
| Tracker | `./storage/tracker/etc:/etc/torrust` | `./storage/tracker/etc` |
94+
| Caddy | `./storage/caddy/data:/data` | `./storage/caddy/data` |
95+
| Caddy | `./storage/caddy/config:/config` | `./storage/caddy/config` |
96+
| Caddy | `./storage/caddy/etc/Caddyfile:/etc/...` | `./storage/caddy/etc` |
97+
| Grafana | `./storage/grafana/data:/var/lib/grafana` | `./storage/grafana/data` |
98+
| Prometheus | `./storage/prometheus/etc:/etc/prometheus` | `./storage/prometheus/etc` |
99+
| MySQL | `./storage/mysql/data:/var/lib/mysql` | `./storage/mysql/data` |
100+
101+
Mount options:
102+
103+
- `:ro` - Read-only for config files that shouldn't be modified
104+
- `:Z` - SELinux private relabeling for writable data directories
105+
106+
## Consequences
107+
108+
### Positive
109+
110+
- **Simplified backup**: Single command `cp -r ./storage/ backup/` backs up everything
111+
- **Easy restore**: Copy files back to `./storage/` to restore
112+
- **Full observability**: All persistent data is visible at predictable paths
113+
- **Consistent pattern**: Same approach for all services
114+
- **Portable**: Data directory can be moved between hosts by copying
115+
- **Easy debugging**: Direct file inspection without entering containers
116+
- **Better development experience**: Reset state by deleting directories
117+
- **Simpler deployment**: No top-level `volumes:` section needed in docker-compose.yml
118+
- **Security visibility**: File permissions are visible and controllable
119+
120+
### Negative
121+
122+
- **Explicit directory creation**: Directories must be created with correct permissions before container start
123+
- **Permission management**: Must ensure correct ownership for non-root containers (Grafana: 472:472, MySQL: 999:999)
124+
- **SELinux handling**: Must apply `:Z` suffix for writable directories on SELinux systems
125+
- **Additional Ansible playbooks**: Need playbooks to create directories with correct ownership
126+
127+
### Risks
128+
129+
- **Breaking change**: Existing deployments using named volumes will need migration
130+
- **Permission errors**: Incorrect directory ownership will prevent containers from starting
131+
132+
### Mitigation
133+
134+
- Create new Ansible playbooks for Grafana and MySQL directory creation with correct ownership
135+
- Document migration path for existing deployments
136+
- E2E tests will verify correct permission handling
137+
138+
## Alternatives Considered
139+
140+
### 1. Named Volumes Only
141+
142+
**Rejected** because:
143+
144+
- Data is hidden in `/var/lib/docker/volumes/`
145+
- Backup requires Docker-specific commands
146+
- Inconsistent with our observability principles
147+
- Users cannot easily access or inspect persistent data
148+
149+
### 2. Mixed Approach (Current State)
150+
151+
**Rejected** because:
152+
153+
- Inconsistency creates confusion and maintenance burden
154+
- Different services have different storage patterns
155+
- No single backup strategy works for all services
156+
- Cognitive overhead for developers and operators
157+
158+
### 3. Docker Volume Plugins
159+
160+
**Rejected** because:
161+
162+
- Overkill for single-VM deployments
163+
- Adds complexity and external dependencies
164+
- Our deployment model is single-VM, not distributed
165+
- Standard bind mounts meet all our requirements
166+
167+
## Related Decisions
168+
169+
- [Grafana Integration Pattern](./grafana-integration-pattern.md) - This ADR supersedes the volume recommendations in that decision
170+
- [Configuration Directories as Secrets](./configuration-directories-as-secrets.md) - Related security considerations for data directories
171+
172+
## References
173+
174+
- [Docker Compose bind mounts documentation](https://docs.docker.com/compose/compose-file/07-volumes/)
175+
- [SELinux and Docker](https://docs.docker.com/storage/bind-mounts/#configure-the-selinux-label)
176+
- [Refactoring Plan: Docker Compose Topology Domain Model](../refactors/plans/docker-compose-topology-domain-model.md)

packages/dependency-installer/tests/containers/image_builder.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,7 @@ impl ImageBuilder {
9898
.arg("inspect")
9999
.arg(full_image_name)
100100
.output()
101-
.map(|output| output.status.success())
102-
.unwrap_or(false)
101+
.is_ok_and(|output| output.status.success())
103102
}
104103
}
105104

packages/linting/src/utils.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ pub fn is_command_available(command: &str) -> bool {
1212
Command::new("which")
1313
.arg(command)
1414
.output()
15-
.map(|output| output.status.success())
16-
.unwrap_or(false)
15+
.is_ok_and(|output| output.status.success())
1716
}
1817

1918
/// Install a tool using npm globally

src/application/command_handlers/configure/handler.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,8 @@ impl ConfigureCommandHandler {
144144

145145
// Allow tests or CI to skip Docker installation
146146
// (useful for container-based tests where Docker is already installed via Dockerfile)
147-
let skip_docker = std::env::var("TORRUST_TD_SKIP_DOCKER_INSTALL_IN_CONTAINER")
148-
.map(|v| v == "true")
149-
.unwrap_or(false);
147+
let skip_docker =
148+
std::env::var("TORRUST_TD_SKIP_DOCKER_INSTALL_IN_CONTAINER").is_ok_and(|v| v == "true");
150149

151150
let current_step = ConfigureStep::InstallDocker;
152151
if skip_docker {
@@ -185,9 +184,8 @@ impl ConfigureCommandHandler {
185184
// Allow tests or CI to explicitly skip the firewall configuration step
186185
// (useful for container-based test runs where iptables/ufw require
187186
// elevated kernel capabilities not available in unprivileged containers).
188-
let skip_firewall = std::env::var("TORRUST_TD_SKIP_FIREWALL_IN_CONTAINER")
189-
.map(|v| v == "true")
190-
.unwrap_or(false);
187+
let skip_firewall =
188+
std::env::var("TORRUST_TD_SKIP_FIREWALL_IN_CONTAINER").is_ok_and(|v| v == "true");
191189

192190
if skip_firewall {
193191
info!(

src/domain/environment/state/common.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ mod tests {
104104

105105
let context: BaseFailureContext = serde_json::from_str(&json).unwrap();
106106
assert_eq!(context.error_summary, "Deserialized error");
107-
assert_eq!(context.execution_duration, Duration::from_secs(60));
107+
assert_eq!(context.execution_duration, Duration::from_mins(1));
108108
}
109109

110110
#[test]

src/presentation/views/progress/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,7 @@ mod tests {
685685

686686
#[test]
687687
fn it_should_format_seconds_correctly() {
688-
let duration = Duration::from_millis(1000);
688+
let duration = Duration::from_secs(1);
689689
assert_eq!(format_duration(duration), "1.0s");
690690

691691
let duration = Duration::from_millis(2345);

src/testing/e2e/containers/image_builder.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ impl ContainerImageBuilder {
155155
tag: "latest".to_string(),
156156
dockerfile_path: None,
157157
context_path: PathBuf::from("."),
158-
build_timeout: Duration::from_secs(300),
158+
build_timeout: Duration::from_mins(5),
159159
}
160160
}
161161

@@ -433,7 +433,7 @@ mod tests {
433433
assert_eq!(builder.tag(), "latest");
434434
assert_eq!(builder.dockerfile_path(), None);
435435
assert_eq!(builder.context_path(), &PathBuf::from("."));
436-
assert_eq!(builder.build_timeout(), Duration::from_secs(300));
436+
assert_eq!(builder.build_timeout(), Duration::from_mins(5));
437437
}
438438

439439
#[test]
@@ -480,7 +480,7 @@ mod tests {
480480

481481
#[test]
482482
fn it_should_configure_build_timeout() {
483-
let timeout = Duration::from_secs(600);
483+
let timeout = Duration::from_mins(10);
484484
let builder = ContainerImageBuilder::new().with_build_timeout(timeout);
485485

486486
assert_eq!(builder.build_timeout(), timeout);
@@ -493,7 +493,7 @@ mod tests {
493493
.with_tag("v2.0")
494494
.with_dockerfile(PathBuf::from("custom/Dockerfile"))
495495
.with_context(PathBuf::from("./src"))
496-
.with_build_timeout(Duration::from_secs(900));
496+
.with_build_timeout(Duration::from_mins(15));
497497

498498
assert_eq!(builder.image_name(), Some("my-app"));
499499
assert_eq!(builder.tag(), "v2.0");
@@ -503,7 +503,7 @@ mod tests {
503503
Some(&PathBuf::from("custom/Dockerfile"))
504504
);
505505
assert_eq!(builder.context_path(), &PathBuf::from("./src"));
506-
assert_eq!(builder.build_timeout(), Duration::from_secs(900));
506+
assert_eq!(builder.build_timeout(), Duration::from_mins(15));
507507
}
508508

509509
#[test]

src/testing/e2e/containers/timeout.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ pub struct ContainerTimeouts {
2424
impl Default for ContainerTimeouts {
2525
fn default() -> Self {
2626
Self {
27-
docker_build: Duration::from_secs(300), // 5 minutes
28-
container_start: Duration::from_secs(60), // 1 minute
29-
ssh_ready: Duration::from_secs(30), // 30 seconds
30-
ssh_setup: Duration::from_secs(15), // 15 seconds
27+
docker_build: Duration::from_mins(5), // 5 minutes
28+
container_start: Duration::from_mins(1), // 1 minute
29+
ssh_ready: Duration::from_secs(30), // 30 seconds
30+
ssh_setup: Duration::from_secs(15), // 15 seconds
3131
}
3232
}
3333
}

src/testing/e2e/tasks/black_box/test_runner.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,7 @@ impl E2eTestRunner {
341341
pub fn run_services(&self) -> Result<()> {
342342
// Check if run should be skipped (Docker not available in test container)
343343
let skip_run = std::env::var("TORRUST_TD_SKIP_RUN_IN_CONTAINER")
344-
.map(|v| v.to_lowercase() == "true")
345-
.unwrap_or(false);
344+
.is_ok_and(|v| v.to_lowercase() == "true");
346345

347346
if skip_run {
348347
info!(

0 commit comments

Comments
 (0)