docs: [#288] add ADR for bind mount standardization #289
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull request overview
This PR introduces an ADR that records the decision to standardize on bind mounts for all Docker Compose volumes, providing rationale and context for the upcoming topology/domain-model refactor. It also registers the ADR in the central ADR index.
Changes:
- Added
docs/decisions/bind-mount-standardization.mddocumenting the bind-mount-only strategy, its 9 key motivations, consequences (positive, negative, risks, mitigations), and rejected alternatives. - Updated
docs/decisions/README.mdto include the new ADR in the decision index with status, date, link, and a concise summary.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| docs/decisions/bind-mount-standardization.md | New ADR capturing the bind-mount standardization decision, reasons, alternatives, consequences, and links to related ADRs and the Docker Compose topology refactoring plan. |
| docs/decisions/README.md | ADR index updated to register the new bind-mount ADR with correct status, date, link, and summary. |
josecelano
force-pushed
the
288-adr-bind-mount-standardization
branch
from
1cee6a0 to
1e9b3ba
Compare
Create ADR documenting the decision to use bind mounts exclusively for all Docker Compose volume mounts. Key points: - 9 reasons documented (observability, backup, restore, consistency, portability, debugging, dev experience, architecture, security) - 3 alternatives rejected (named volumes only, mixed approach, plugins) - Consequences documented (positive and negative) - References refactoring plan This ADR establishes the foundation for Phase 0 of the Docker Compose topology refactoring. Closes #288
josecelano
force-pushed
the
288-adr-bind-mount-standardization
branch
from
1e9b3ba to
4134459
Compare
Member
Author
|
ACK 4134459 |
josecelano
added a commit
that referenced
this pull request
Jan 25, 2026
…gregate (P2.1, P2.2) d51857b docs: [#296] add Phase 3 port topology to epic and create draft issue (Jose Celano) cf3b6f3 feat: [#296] add network descriptions as inline comments in docker-compose.yml (Jose Celano) a5071c9 docs: [#296] move network security docs from template to Rust module (Jose Celano) cee9de8 docs: [#296] update documentation with Phase 2 PR #297 completion (Jose Celano) f9de701 refactor: [#296] create DockerComposeTopology aggregate and derive networks (Jose Celano) Pull request description: # Phase 2: Create DockerComposeTopology Aggregate **Issue**: #296 **Epic**: #287 (Docker Compose Topology Domain Model Refactoring) ## Summary This PR implements Phase 2 of the Docker Compose Topology refactoring, introducing the `DockerComposeTopology` aggregate and establishing the single source of truth pattern for network derivation. ## Changes ### P2.1: Service Enum and Aggregate - **`Service` enum** (`src/domain/topology/service.rs`): Type-safe service identification with 5 variants: - `Tracker` - Core BitTorrent tracker - `MySQL` - Database service - `Prometheus` - Metrics collection - `Grafana` - Visualization - `Caddy` - TLS proxy - **`DockerComposeTopology` aggregate** (`src/domain/topology/aggregate.rs`): - Collects all `ServiceTopology` entries - Derives `required_networks()` from service configurations - Ensures deterministic ordering (alphabetical by name) - Enforces invariants: no orphan networks - **`ServiceTopology` struct**: Links services to their network assignments ### P2.2: Network Derivation in Context and Template - **`NetworkDefinition` type** (`context/network_definition.rs`): Template-friendly network representation - **Context updates** (`context/mod.rs`, `context/builder.rs`): - Added `required_networks` field to `DockerComposeContext` - Added `derive_required_networks()` method in builder - Networks collected from all enabled services, deduplicated, sorted - **Template update** (`docker-compose.yml.tera`): - Replaced 4 conditional network blocks with single `required_networks` loop - Before: `{%- if mysql %}\n database_network:\n driver: bridge\n{%- endif %}` - After: `{%- for net in required_networks %}\n {{ net.name }}:\n driver: {{ net.driver }}\n{%- endfor %}` ## Test Coverage - 14 new tests for `DockerComposeTopology` aggregate - 12 new tests for `Service` enum - 5 new tests for `NetworkDefinition` - 12 new tests for `required_networks` derivation in context - All 389 unit tests pass - All E2E tests pass (infrastructure lifecycle + deployment workflow) ## Behavioral Equivalence The generated `docker-compose.yml` output is functionally identical before and after this change. The only difference is implementation: networks are now derived from service configurations rather than duplicated in template conditionals. ## Files Changed | File | Change | |------|--------| | `src/domain/topology/service.rs` | **NEW** - Service enum | | `src/domain/topology/aggregate.rs` | **NEW** - DockerComposeTopology aggregate | | `src/domain/topology/mod.rs` | Modified - exports new types | | `src/domain/mod.rs` | Modified - re-exports new types | | `context/network_definition.rs` | **NEW** - NetworkDefinition type | | `context/mod.rs` | Modified - required_networks field + tests | | `context/builder.rs` | Modified - derive_required_networks() | | `docker-compose.yml.tera` | Modified - uses required_networks loop | ## Progress Update After this PR: - ✅ ADR-01: Bind Mount Standardization (PR #289) - ✅ BUG-01: Remove invalid Grafana template branch (PR #291) - ✅ Phase 0: Bind mounts (PR #293) - ✅ Phase 1: Network domain types (PR #295) - ✅ **Phase 2: DockerComposeTopology aggregate** (this PR) - ⏳ Phase 3: Service volumes with bind mount type (next) - ⏳ Phase 4: Integration in builder pipeline (final) ACKs for top commit: josecelano: ACK d51857b Tree-SHA512: f8faa90b6211e491c9e9be05d762c12d1cac6ed5e0bb671877ec007b129784f2bad1dacbf16b37c6c77f6a35024f4ada8dbf23b5a8fa260f52b511a55005538c
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds an Architectural Decision Record (ADR) documenting the decision to standardize on bind mounts for all Docker Compose volumes.
Related Issues
Changes
New ADR:
docs/decisions/bind-mount-standardization.md:Zsuffix)Updated ADR Index:
docs/decisions/README.mdKey Points from ADR
Reasons for Bind Mounts
Rejected Alternatives
Checklist