Skip to content

fix: [#290] remove dead Grafana-without-Prometheus template branch#291

Merged
josecelano merged 1 commit into
mainfrom
290-remove-invalid-grafana-without-prometheus-template-branch
Jan 24, 2026
Merged

fix: [#290] remove dead Grafana-without-Prometheus template branch#291
josecelano merged 1 commit into
mainfrom
290-remove-invalid-grafana-without-prometheus-template-branch

Conversation

@josecelano

Copy link
Copy Markdown
Member

Summary

Removes dead code from the Docker Compose template that handled an impossible configuration state (Grafana enabled without Prometheus).

Related Issues

Changes

  • Removed unreachable {%- else %} branch in Grafana depends_on section
  • The domain layer already validates that Grafana requires Prometheus (GrafanaRequiresPrometheus error in src/domain/environment/user_inputs.rs)
  • The template's conditional branch handling "Grafana without Prometheus" could never be executed

Before

    depends_on:
{%- if prometheus %}
      prometheus:
        condition: service_healthy
{%- else %}
      - tracker
{%- endif %}

After

    depends_on:
      prometheus:
        condition: service_healthy

Verification

  • Domain validation confirms this configuration is impossible
  • Pre-commit checks pass
  • E2E tests pass

@josecelano josecelano self-assigned this Jan 24, 2026
@josecelano

Copy link
Copy Markdown
Member Author

ACK 5da1fff

@josecelano
josecelano merged commit 9399dd2 into main Jan 24, 2026
44 checks passed
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Remove invalid "Grafana without Prometheus" template branch

1 participant