Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/decisions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ This directory contains architectural decision records for the Torrust Tracker D

| Status | Date | Decision | Summary |
| ------------- | ---------- | --------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------ |
| ✅ Accepted | 2026-01-24 | [Bind Mount Standardization](./bind-mount-standardization.md) | Use bind mounts exclusively for all Docker Compose volumes for observability and backup |
| ✅ 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 |
| ✅ 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 |
| ✅ 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 |
Expand Down
176 changes: 176 additions & 0 deletions docs/decisions/bind-mount-standardization.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
# Decision: Bind Mount Standardization for Docker Compose

## Status

Accepted

## Date

2026-01-24

## Context

The Docker Compose template currently uses a mix of named volumes and bind mounts for persistent data:

```yaml
# Bind mounts (host path → container path) - CURRENT PATTERN
- ./storage/tracker/lib:/var/lib/torrust/tracker:Z

# Named volumes (volume name → container path) - PROBLEMATIC
- caddy_data:/data
- grafana_data:/var/lib/grafana
- mysql_data:/var/lib/mysql
```

This inconsistency creates several problems:

### 1. Observability

- Named volumes hide data in `/var/lib/docker/volumes/` - not obvious to users
- Users cannot easily see where persistent data is stored
- File system tools (ls, du, find) don't work directly on named volume data

### 2. Backup Complexity

- Named volumes require `docker volume` commands or finding the internal path
- No single command can back up all data
- Docker-specific tooling is required
- Standard backup scripts don't work without modification

### 3. Restore Complexity

- Restoring named volumes requires Docker volume recreation
- Cannot simply copy files to restore data
- Migration between hosts requires Docker volume export/import

### 4. Inconsistency

- Some services use bind mounts, others use named volumes
- Different patterns for different services create cognitive overhead
- No predictable directory structure

### 5. Portability Limitations

- Named volumes cannot be moved between hosts by copying files
- Docker volume export/import dance is required
- Tied to Docker's internal storage format

### 6. Debugging & Troubleshooting Difficulties

- Cannot directly inspect files without entering containers
- Checking file permissions, ownership, disk usage is difficult
- Cannot modify config files directly for debugging
- Log files not accessible without `docker logs`

### 7. Development Experience

- Cannot easily reset state by deleting directories
- Cannot pre-populate data for testing scenarios
- IDE file watchers cannot observe changes in named volumes

### 8. Deployment Architecture Complexity

- Named volumes require a top-level `volumes:` section in docker-compose.yml
- Must derive which volumes are required based on enabled services
- Ansible must manage both directories and Docker volumes

### 9. Security Visibility

- File permissions are hidden inside Docker volume directories
- SELinux labels cannot be applied consistently
- Data locations are not transparent to users

## Decision

Standardize on **bind mounts exclusively** for all persistent data in Docker Compose deployments.

All persistent data will be stored under `./storage/{service}/`:

| Service | Bind Mount | Data Location |
| ---------- | ------------------------------------------ | -------------------------- |
| Tracker | `./storage/tracker/lib:/var/lib/torrust` | `./storage/tracker/lib` |
| Tracker | `./storage/tracker/log:/var/log/torrust` | `./storage/tracker/log` |
| Tracker | `./storage/tracker/etc:/etc/torrust` | `./storage/tracker/etc` |
| Caddy | `./storage/caddy/data:/data` | `./storage/caddy/data` |
| Caddy | `./storage/caddy/config:/config` | `./storage/caddy/config` |
| Caddy | `./storage/caddy/etc/Caddyfile:/etc/...` | `./storage/caddy/etc` |
| Grafana | `./storage/grafana/data:/var/lib/grafana` | `./storage/grafana/data` |
| Prometheus | `./storage/prometheus/etc:/etc/prometheus` | `./storage/prometheus/etc` |
| MySQL | `./storage/mysql/data:/var/lib/mysql` | `./storage/mysql/data` |

Mount options:

- `:ro` - Read-only for config files that shouldn't be modified
- `:Z` - SELinux private relabeling for writable data directories

## Consequences

### Positive

- **Simplified backup**: Single command `cp -r ./storage/ backup/` backs up everything
- **Easy restore**: Copy files back to `./storage/` to restore
- **Full observability**: All persistent data is visible at predictable paths
- **Consistent pattern**: Same approach for all services
- **Portable**: Data directory can be moved between hosts by copying
- **Easy debugging**: Direct file inspection without entering containers
- **Better development experience**: Reset state by deleting directories
- **Simpler deployment**: No top-level `volumes:` section needed in docker-compose.yml
- **Security visibility**: File permissions are visible and controllable

### Negative

- **Explicit directory creation**: Directories must be created with correct permissions before container start
- **Permission management**: Must ensure correct ownership for non-root containers (Grafana: 472:472, MySQL: 999:999)
- **SELinux handling**: Must apply `:Z` suffix for writable directories on SELinux systems
- **Additional Ansible playbooks**: Need playbooks to create directories with correct ownership

### Risks

- **Breaking change**: Existing deployments using named volumes will need migration
- **Permission errors**: Incorrect directory ownership will prevent containers from starting

### Mitigation

- Create new Ansible playbooks for Grafana and MySQL directory creation with correct ownership
- Document migration path for existing deployments
- E2E tests will verify correct permission handling

## Alternatives Considered

### 1. Named Volumes Only

**Rejected** because:

- Data is hidden in `/var/lib/docker/volumes/`
- Backup requires Docker-specific commands
- Inconsistent with our observability principles
- Users cannot easily access or inspect persistent data

### 2. Mixed Approach (Current State)

**Rejected** because:

- Inconsistency creates confusion and maintenance burden
- Different services have different storage patterns
- No single backup strategy works for all services
- Cognitive overhead for developers and operators

### 3. Docker Volume Plugins

**Rejected** because:

- Overkill for single-VM deployments
- Adds complexity and external dependencies
- Our deployment model is single-VM, not distributed
- Standard bind mounts meet all our requirements

## Related Decisions

- [Grafana Integration Pattern](./grafana-integration-pattern.md) - This ADR supersedes the volume recommendations in that decision
- [Configuration Directories as Secrets](./configuration-directories-as-secrets.md) - Related security considerations for data directories

## References

- [Docker Compose bind mounts documentation](https://docs.docker.com/compose/compose-file/07-volumes/)
- [SELinux and Docker](https://docs.docker.com/storage/bind-mounts/#configure-the-selinux-label)
- [Refactoring Plan: Docker Compose Topology Domain Model](../refactors/plans/docker-compose-topology-domain-model.md)
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ impl ImageBuilder {
.arg("inspect")
.arg(full_image_name)
.output()
.map(|output| output.status.success())
.unwrap_or(false)
.is_ok_and(|output| output.status.success())
}
}

Expand Down
3 changes: 1 addition & 2 deletions packages/linting/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ pub fn is_command_available(command: &str) -> bool {
Command::new("which")
.arg(command)
.output()
.map(|output| output.status.success())
.unwrap_or(false)
.is_ok_and(|output| output.status.success())
}

/// Install a tool using npm globally
Expand Down
10 changes: 4 additions & 6 deletions src/application/command_handlers/configure/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,8 @@ impl ConfigureCommandHandler {

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

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

if skip_firewall {
info!(
Expand Down
2 changes: 1 addition & 1 deletion src/domain/environment/state/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ mod tests {

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

#[test]
Expand Down
2 changes: 1 addition & 1 deletion src/presentation/views/progress/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ mod tests {

#[test]
fn it_should_format_seconds_correctly() {
let duration = Duration::from_millis(1000);
let duration = Duration::from_secs(1);
assert_eq!(format_duration(duration), "1.0s");

let duration = Duration::from_millis(2345);
Expand Down
10 changes: 5 additions & 5 deletions src/testing/e2e/containers/image_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl ContainerImageBuilder {
tag: "latest".to_string(),
dockerfile_path: None,
context_path: PathBuf::from("."),
build_timeout: Duration::from_secs(300),
build_timeout: Duration::from_mins(5),
}
}

Expand Down Expand Up @@ -433,7 +433,7 @@ mod tests {
assert_eq!(builder.tag(), "latest");
assert_eq!(builder.dockerfile_path(), None);
assert_eq!(builder.context_path(), &PathBuf::from("."));
assert_eq!(builder.build_timeout(), Duration::from_secs(300));
assert_eq!(builder.build_timeout(), Duration::from_mins(5));
}

#[test]
Expand Down Expand Up @@ -480,7 +480,7 @@ mod tests {

#[test]
fn it_should_configure_build_timeout() {
let timeout = Duration::from_secs(600);
let timeout = Duration::from_mins(10);
let builder = ContainerImageBuilder::new().with_build_timeout(timeout);

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

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

#[test]
Expand Down
8 changes: 4 additions & 4 deletions src/testing/e2e/containers/timeout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ pub struct ContainerTimeouts {
impl Default for ContainerTimeouts {
fn default() -> Self {
Self {
docker_build: Duration::from_secs(300), // 5 minutes
container_start: Duration::from_secs(60), // 1 minute
ssh_ready: Duration::from_secs(30), // 30 seconds
ssh_setup: Duration::from_secs(15), // 15 seconds
docker_build: Duration::from_mins(5), // 5 minutes
container_start: Duration::from_mins(1), // 1 minute
ssh_ready: Duration::from_secs(30), // 30 seconds
ssh_setup: Duration::from_secs(15), // 15 seconds
}
}
}
3 changes: 1 addition & 2 deletions src/testing/e2e/tasks/black_box/test_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,7 @@ impl E2eTestRunner {
pub fn run_services(&self) -> Result<()> {
// Check if run should be skipped (Docker not available in test container)
let skip_run = std::env::var("TORRUST_TD_SKIP_RUN_IN_CONTAINER")
.map(|v| v.to_lowercase() == "true")
.unwrap_or(false);
.is_ok_and(|v| v.to_lowercase() == "true");

if skip_run {
info!(
Expand Down
Loading