fix: [#326] require --output-dir flag to prevent artifact conflicts#327
Merged
josecelano merged 17 commits intomainfrom Feb 10, 2026
Merged
fix: [#326] require --output-dir flag to prevent artifact conflicts#327josecelano merged 17 commits intomainfrom
josecelano merged 17 commits intomainfrom
Conversation
…always-required IP
…ervice - Created src/application/services/rendering/ module structure - Moved and renamed AnsibleTemplateService -> AnsibleTemplateRenderingService - Updated all consumers (provision, register, render handlers + errors) - Fixed pre-existing async test issues in render handler - Removed trivial test that violated clippy rules - All tests pass, clippy clean Part of the template rendering services extraction refactoring plan.
This commit implements Phase 1 of the template rendering service extraction plan. Four simple rendering services have been created: - OpenTofuTemplateRenderingService: Renders OpenTofu infrastructure templates - TrackerTemplateRenderingService: Renders tracker.toml configuration - PrometheusTemplateRenderingService: Renders prometheus.yml (conditional) - GrafanaTemplateRenderingService: Renders Grafana provisioning (conditional) All services follow the established pattern: - Factory methods (from_paths or from_params) with explicit domain types - Thin error wrappers around infrastructure layer errors - Comprehensive tracing for observability - Conditional logic clearly expressed (Prometheus/Grafana) Additionally, fixed 26 clippy pedantic errors across multiple files including: - Documentation formatting (`OpenTofu` backticks) - Match arm consolidation (merged 9 identical Configuration error patterns) - Static method conversion (parse_ip_address) - Format argument inlining (uninlined_format_args) - Unnecessary Result wraps removed (show_success) - Long function allow annotation (router dispatch) Tests: All 2181 unit tests pass. Relates to refactoring plan: docs/refactors/plans/extract-template-rendering-services.md
…vices This commit implements Phase 2 of the template rendering service extraction plan. Three complex rendering services have been created: - DockerComposeTemplateRenderingService: Renders Docker Compose configuration with complex context building (~130 lines of duplication removed): * Database variant selection (SQLite vs MySQL) * Topology computation (enabled services) * Optional service configuration (Prometheus, Grafana, Backup, Caddy) * Grafana environment context * MySQL setup configuration - CaddyTemplateRenderingService: Renders Caddy TLS proxy configuration with automatic TLS service extraction (~40 lines of duplication removed): * Builds CaddyContext from HTTPS config * Extracts TLS-enabled services (Tracker API, HTTP Trackers, Health Check, Grafana) * Returns Option<PathBuf> (None if no HTTPS or no TLS services) - BackupTemplateRenderingService: Renders backup configuration with database config conversion (~30 lines of duplication removed): * Converts domain DatabaseConfig to backup format * Builds BackupContext with schedule * Returns Option<PathBuf> (None if backup not configured) All services follow the established pattern: - Factory methods (from_paths) with explicit dependencies - Static methods for pure logic that doesn't need instance state - Thin error wrappers around infrastructure layer errors - Comprehensive tracing for observability - Conditional rendering where appropriate Tests: All 2190 unit tests pass (9 new tests added). Relates to refactoring plan: docs/refactors/plans/extract-template-rendering-services.md
…ing services - Replace render_opentofu_templates() with OpenTofuTemplateRenderingService - Replace render_ansible_templates() with direct AnsibleTemplateRenderingService call - Add render calls for 6 remaining templates (DockerCompose, Tracker, Prometheus, Grafana, Caddy, Backup) - Remove helper methods that are now in services - Remove unused imports (TemplateManager, TofuProjectGenerator) - Update render_all_templates() to orchestrate all 8 service calls inline Impact: Handler reduced from 503 lines with duplicated rendering logic to ~380 lines of pure orchestration Code Reduction: ~150 lines removed from handler (now in services) Testing: All 2190 tests passing
…dering services - Updated 6 Step files to use services instead of owning TemplateManager: * RenderTrackerTemplatesStep * RenderPrometheusTemplatesStep * RenderGrafanaTemplatesStep * RenderDockerComposeTemplatesStep * RenderCaddyTemplatesStep * RenderBackupTemplatesStep - Steps now accept templates_dir PathBuf instead of Arc<TemplateManager> - Removed 6 TemplateManager imports from release handler step files - Updated all Step constructors in release handler - Removed helper methods from Steps (build_tracker_config, apply_*_config, etc.) - All helper logic now encapsulated in services - Error types changed to XxxTemplateRenderingServiceError - Updated all test files to use new constructor signature - Total: 12 files changed, 112 insertions(+), 438 deletions(-) - Net reduction: 326 lines removed
…as completed - Updated progress tracking: all 5 phases completed (100%) - Marked all implementation checklists as completed - Added commit hashes for each phase: * Phase 0 (3ecf94b): Rendering module + Ansible service relocation * Phase 1 (d217e14): 4 simple services * Phase 2 (901113e): 3 complex services * Phase 3 (3e14bea): Render handler refactored * Phase 4 (463e793): Steps refactored - Updated timeline with actual completion date - Added results section documenting ~750 lines removed - Changed status from Planning to Completed
BREAKING CHANGE: render command now requires --output-dir parameter
Problem
-------
The render command wrote artifacts to build/{env}/ which would be silently
overwritten by the provision command. This caused data loss and no record
of preview renders.
Example failure scenario:
1. User creates environment
2. User renders with test IP → artifacts in build/env/ (LOST)
3. User provisions → overwrites render output (DATA LOSS)
Solution
--------
Require --output-dir flag to separate preview artifacts from deployment
artifacts:
- Render writes to user-specified directory (e.g., ./preview/)
- Provision writes to build/{env}/ (deployment mode)
- No conflicts, no data loss, clear separation of concerns
Changes
-------
CLI:
- Added 'output_dir: PathBuf' field to Render command (required)
- Added 'force: bool' flag to overwrite existing output directory
- Removed short option '-f' from force flag (conflict with env_file)
Application Layer:
- Updated execute() signature: added output_dir and force parameters
- Added validate_output_directory() to check existence and create if needed
- render_all_templates() now writes to user-specified directory
- Moved output directory validation after input validation (fail-fast)
Error Handling:
- Added OutputDirectoryExists error with --force suggestion
- Added OutputDirectoryCreationFailed error with actionable message
Tests:
- Updated ProcessRunner methods to accept output_dir parameter
- All E2E tests use temporary output directories
- Unit tests use tempfile for proper isolation
- Idempotency test renamed to it_should_fail_when_output_directory_already_exists
Documentation:
- Updated user guide with --output-dir in ALL examples
- Updated console commands reference with new syntax
- Updated manual E2E test procedures to use preview directories
- Updated comparison logic (preview vs provision directories)
- Added "Why Output Directory is Required" section
Why This Design
---------------
1. **No Ambiguity**: render = preview (user location), provision = deployment (build/)
2. **No Data Loss**: User-specified location preserved after provision
3. **Multiple Renders**: Can render with different IPs to different directories
4. **Clear Intent**: Explicit output location shows preview nature
Migration Guide
---------------
Old command (broken):
render --env-name my-env --instance-ip 192.168.1.100
New command (fixed):
render --env-name my-env --instance-ip 192.168.1.100 --output-dir ./preview-my-env
Related: #326
- Fix validate handler test by creating dynamic config with absolute paths - Mark flaky Ansible installation test as ignored with documentation
- Move entry from active-refactorings.md to completed-refactorings.md - Delete completed plan document from plans/ - Refactoring completed with 5 phases (commit 463e793)
- Add comprehensive skill for render command usage - Covers manual deployment, template verification, and contributor workflows - Includes fast create→render iteration for template development - Update AGENTS.md to include new skill in available skills table
Member
Author
|
ACK 80172a2 |
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
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.
Feature: Render Command with Required Output Directory
The
rendercommand generates deployment artifacts (OpenTofu, Ansible, Docker Compose, configuration files) without provisioning infrastructure. This enables artifact preview, manual deployment workflows, and fast template verification for contributors.What It Does
Generates all deployment artifacts for a Torrust Tracker environment:
Key characteristic: Read-only operation - never modifies environment state or provisions infrastructure.
How to Use
Basic Usage
Parameters
--env-nameor--env-file- Input source (mutually exclusive)--instance-ip- Target instance IP (required for Ansible inventory)--output-dir- Output directory for artifacts (required)--force- Overwrite existing output directoryBenefits
For Manual Deployment
For Preview and Validation
For Contributors and Template Development
create → render (2 sec)vscreate → provision → configure (10 min)Example workflow for contributors:
Why --output-dir is Required
The
--output-dirparameter is mandatory to prevent data loss and ambiguity:The Problem Without It
If render used
build/{env}/(the provision command's directory):The Solution
Require explicit output directory:
build/{env}/Design principle: render = preview (user-specified location), provision = deployment (build/)
Use Cases
Documentation
docs/user-guide/commands/render.md(complete command reference with examples)docs/console-commands.md(updated with required --output-dir)docs/e2e-testing/manual-testing.md(updated procedures).github/skills/render-tracker-artifacts/skill.md(comprehensive workflows for AI agents)Related