From a604035a311e7160d6f5e8d7f93bec3306676d1c Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sat, 24 Jan 2026 21:12:26 +0000 Subject: [PATCH 1/2] docs: add draft issue for atomic ansible playbooks rule --- ...licit-rule-for-atomic-ansible-playbooks.md | 145 ++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 docs/issues/drafts/add-explicit-rule-for-atomic-ansible-playbooks.md diff --git a/docs/issues/drafts/add-explicit-rule-for-atomic-ansible-playbooks.md b/docs/issues/drafts/add-explicit-rule-for-atomic-ansible-playbooks.md new file mode 100644 index 00000000..6ff26a5c --- /dev/null +++ b/docs/issues/drafts/add-explicit-rule-for-atomic-ansible-playbooks.md @@ -0,0 +1,145 @@ +# Draft Issue: Add Explicit Rule for Atomic Ansible Playbooks + +## Problem + +During implementation of issue #292, the contributor (AI agent) initially added storage creation tasks directly into existing playbooks (`deploy-compose-files.yml` and `deploy-grafana-provisioning.yml`), violating the Single Responsibility Principle. + +The AGENTS.md has 20+ rules, but the atomic playbook principle was implicit rather than explicit. Rule #8 explains _how_ to register playbooks but doesn't state that each playbook should do exactly one thing. + +### Root Causes + +1. **Rule Density**: Too many rules to remember, easy to miss implicit conventions +2. **Pattern Recognition Failure**: Contributor saw existing playbooks and assumed adding tasks was acceptable +3. **Implicit vs Explicit**: The architecture principle wasn't stated as a clear rule + +## Example of the Mistake + +### Incorrect Approach (What Was Done Initially) + +```yaml +# deploy-compose-files.yml - WRONG: Adding unrelated task +- name: Deploy Docker Compose files + hosts: all + become: true + vars_files: + - variables.yml + + tasks: + - name: Create MySQL storage directory # WRONG - Different responsibility! + ansible.builtin.file: + path: "{{ deploy_dir }}/storage/mysql/data" + state: directory + owner: "999" + group: "999" + when: mysql_enabled | default(false) + + - name: Copy docker-compose files # Original task + # ... +``` + +### Correct Approach (After User Correction) + +```yaml +# create-mysql-storage.yml - RIGHT: Atomic playbook with single responsibility +- name: Create MySQL storage directory + hosts: all + become: true + vars_files: + - variables.yml + + tasks: + - name: Create MySQL data directory with correct ownership + ansible.builtin.file: + path: "{{ deploy_dir }}/storage/mysql/data" + state: directory + mode: "0755" + owner: "999" + group: "999" + when: mysql_enabled | default(false) +``` + +With conditional execution in Rust: + +```rust +// In release handler - conditional check happens in Rust, not Ansible +fn create_mysql_storage(environment: &Environment, ...) { + // Check if MySQL is configured (via tracker database driver) + if !environment.context().user_inputs.tracker().uses_mysql() { + info!("MySQL not configured - skipping storage creation"); + return Ok(()); + } + + // Run the atomic playbook + CreateMysqlStorageStep::new(ansible_client).execute() +} +``` + +## Proposed Solution + +### 1. Update Rule #8 in AGENTS.md + +Change from: + +```markdown +8. **When adding new Ansible playbooks**: Read `docs/contributing/templates/tera.md`... +``` + +To: + +```markdown +8. **When adding new Ansible playbooks**: Read `docs/contributing/templates/ansible.md` for the complete guide. + - **CRITICAL: One playbook = One responsibility** (Single Responsibility Principle) + - Each playbook should perform exactly ONE conceptual operation + - Conditional logic for service enablement belongs in Rust code, NOT in playbooks with `when:` clauses + - Static playbooks must be registered in `copy_static_templates()` +``` + +### 2. Add "Red Flags" Section to Ansible Guide + +In `docs/contributing/templates/ansible.md`: + +```markdown +## Red Flags (Stop and Reconsider) + +If you find yourself doing any of these, STOP and reconsider: + +- **Adding tasks to an existing playbook** → Create a new atomic playbook instead +- **Using `when:` for service enablement** → Move the condition to Rust code +- **Playbook names with "and"** (e.g., "create-storage-and-deploy-config.yml") → Split into separate playbooks +- **Multiple unrelated `ansible.builtin.file` tasks** → Each belongs in its own playbook + +**Correct pattern**: New feature = New atomic playbook + New Rust step with conditional check +``` + +### 3. Create ADR for Atomic Playbook Architecture + +Create `docs/decisions/atomic-ansible-playbooks.md` explaining: + +- Why we use atomic playbooks (testability, composability, clear failure points) +- How conditional execution works (Rust checks service enablement, then calls atomic playbook) +- The three-level architecture (Command → Step → Ansible playbook) + +### 4. Pre-Implementation Checklist + +Add to ansible guide: + +```markdown +## Before Adding Ansible Functionality Checklist + +- [ ] Am I adding tasks to an existing playbook? → **Create new atomic playbook** +- [ ] Does my playbook do more than one conceptual thing? → **Split it** +- [ ] Am I using `when:` for "should this service run"? → **Move to Rust conditional** +- [ ] Have I created a corresponding Rust step file? +- [ ] Have I registered the playbook in `copy_static_templates()`? +``` + +## Files to Modify + +1. `AGENTS.md` - Update Rule #8 +2. `docs/contributing/templates/ansible.md` - Add Red Flags section and checklist +3. Create `docs/decisions/atomic-ansible-playbooks.md` - ADR + +## Related + +- Issue #292 where this problem was discovered +- Existing pattern: `create-prometheus-storage.yml`, `create-tracker-storage.yml` From 3456d39a68adc23e8227a214cac029cf1e8895a2 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sat, 24 Jan 2026 21:45:30 +0000 Subject: [PATCH 2/2] refactor: [#292] convert Docker named volumes to bind mounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit converts four Docker named volumes to bind mounts under the ./storage/ directory hierarchy: - caddy_data → ./storage/caddy/data - caddy_config_vol → ./storage/caddy/config - grafana_data → ./storage/grafana/data - mysql_data → ./storage/mysql/data Implementation follows the atomic playbook principle: - Created create-grafana-storage.yml playbook (owner 472:472) - Created create-mysql-storage.yml playbook (owner 999:999) - Added corresponding Rust step files with conditional execution - Grafana storage only created when Grafana is enabled - MySQL storage only created when using MySQL driver This change prepares for the Docker Compose topology domain model refactoring by making all storage explicit and manageable. Closes #292 --- .../command_handlers/release/errors.rs | 72 +++++++++++ .../command_handlers/release/handler.rs | 116 +++++++++++++++-- .../application/create_grafana_storage.rs | 119 ++++++++++++++++++ .../steps/application/create_mysql_storage.rs | 119 ++++++++++++++++++ src/application/steps/application/mod.rs | 6 + .../environment/state/release_failed.rs | 6 + src/domain/tracker/config/mod.rs | 41 ++++++ .../template/renderer/project_generator.rs | 4 +- .../template/renderer/docker_compose.rs | 11 +- templates/ansible/create-grafana-storage.yml | 32 +++++ templates/ansible/create-mysql-storage.yml | 32 +++++ templates/ansible/variables.yml.tera | 1 + .../docker-compose/docker-compose.yml.tera | 27 +--- 13 files changed, 548 insertions(+), 38 deletions(-) create mode 100644 src/application/steps/application/create_grafana_storage.rs create mode 100644 src/application/steps/application/create_mysql_storage.rs create mode 100644 templates/ansible/create-grafana-storage.yml create mode 100644 templates/ansible/create-mysql-storage.yml diff --git a/src/application/command_handlers/release/errors.rs b/src/application/command_handlers/release/errors.rs index e0529a01..c39719ff 100644 --- a/src/application/command_handlers/release/errors.rs +++ b/src/application/command_handlers/release/errors.rs @@ -52,6 +52,14 @@ pub enum ReleaseCommandHandlerError { #[error("Prometheus storage creation failed: {0}")] PrometheusStorageCreation(String), + /// Grafana storage directory creation failed + #[error("Grafana storage creation failed: {0}")] + GrafanaStorageCreation(String), + + /// `MySQL` storage directory creation failed + #[error("MySQL storage creation failed: {0}")] + MysqlStorageCreation(String), + /// Caddy configuration deployment failed #[error("Caddy configuration deployment failed: {0}")] CaddyConfigDeployment(String), @@ -115,6 +123,12 @@ impl Traceable for ReleaseCommandHandlerError { "ReleaseCommandHandlerError: Prometheus storage creation failed - {message}" ) } + Self::GrafanaStorageCreation(message) => { + format!("ReleaseCommandHandlerError: Grafana storage creation failed - {message}") + } + Self::MysqlStorageCreation(message) => { + format!("ReleaseCommandHandlerError: MySQL storage creation failed - {message}") + } Self::CaddyConfigDeployment(message) => { format!( "ReleaseCommandHandlerError: Caddy configuration deployment failed - {message}" @@ -144,6 +158,8 @@ impl Traceable for ReleaseCommandHandlerError { | Self::TrackerStorageCreation(_) | Self::TrackerDatabaseInit(_) | Self::PrometheusStorageCreation(_) + | Self::GrafanaStorageCreation(_) + | Self::MysqlStorageCreation(_) | Self::CaddyConfigDeployment(_) | Self::ReleaseOperationFailed { .. } => None, } @@ -159,6 +175,8 @@ impl Traceable for ReleaseCommandHandlerError { | Self::TrackerStorageCreation(_) | Self::TrackerDatabaseInit(_) | Self::PrometheusStorageCreation(_) + | Self::GrafanaStorageCreation(_) + | Self::MysqlStorageCreation(_) | Self::CaddyConfigDeployment(_) => ErrorKind::TemplateRendering, Self::Deployment { .. } | Self::ReleaseOperationFailed { .. } => { ErrorKind::InfrastructureOperation @@ -354,6 +372,54 @@ Common causes: - Ansible playbook not found - Network connectivity issues +For more information, see docs/user-guide/commands.md" + } + Self::GrafanaStorageCreation(_) => { + "Grafana Storage Creation Failed - Troubleshooting: + +1. Verify the target instance is reachable: + ssh @ + +2. Check that the instance has sufficient disk space: + df -h + +3. Verify the Ansible playbook exists: + ls templates/ansible/create-grafana-storage.yml + +4. Check Ansible execution permissions + +5. Review the error message above for specific details + +Common causes: +- Insufficient disk space on target instance +- Permission denied on target directories +- Ansible playbook not found +- Network connectivity issues + +For more information, see docs/user-guide/commands.md" + } + Self::MysqlStorageCreation(_) => { + "MySQL Storage Creation Failed - Troubleshooting: + +1. Verify the target instance is reachable: + ssh @ + +2. Check that the instance has sufficient disk space: + df -h + +3. Verify the Ansible playbook exists: + ls templates/ansible/create-mysql-storage.yml + +4. Check Ansible execution permissions + +5. Review the error message above for specific details + +Common causes: +- Insufficient disk space on target instance +- Permission denied on target directories +- Ansible playbook not found +- Network connectivity issues + For more information, see docs/user-guide/commands.md" } Self::CaddyConfigDeployment(_) => { @@ -517,6 +583,12 @@ mod tests { }), ReleaseCommandHandlerError::StatePersistence(RepositoryError::NotFound), ReleaseCommandHandlerError::TemplateRendering("test".to_string()), + ReleaseCommandHandlerError::TrackerStorageCreation("test".to_string()), + ReleaseCommandHandlerError::TrackerDatabaseInit("test".to_string()), + ReleaseCommandHandlerError::PrometheusStorageCreation("test".to_string()), + ReleaseCommandHandlerError::GrafanaStorageCreation("test".to_string()), + ReleaseCommandHandlerError::MysqlStorageCreation("test".to_string()), + ReleaseCommandHandlerError::CaddyConfigDeployment("test".to_string()), ReleaseCommandHandlerError::DeploymentFailed { message: "test".to_string(), source: DeployComposeFilesStepError::ComposeBuildDirNotFound { diff --git a/src/application/command_handlers/release/handler.rs b/src/application/command_handlers/release/handler.rs index 30c5057d..1b25b649 100644 --- a/src/application/command_handlers/release/handler.rs +++ b/src/application/command_handlers/release/handler.rs @@ -11,9 +11,9 @@ use crate::adapters::ansible::AnsibleClient; use crate::application::command_handlers::common::StepResult; use crate::application::steps::{ application::{ - CreatePrometheusStorageStep, CreateTrackerStorageStep, DeployCaddyConfigStep, - DeployGrafanaProvisioningStep, DeployPrometheusConfigStep, DeployTrackerConfigStep, - InitTrackerDatabaseStep, + CreateGrafanaStorageStep, CreateMysqlStorageStep, CreatePrometheusStorageStep, + CreateTrackerStorageStep, DeployCaddyConfigStep, DeployGrafanaProvisioningStep, + DeployPrometheusConfigStep, DeployTrackerConfigStep, InitTrackerDatabaseStep, }, rendering::{ RenderCaddyTemplatesStep, RenderGrafanaTemplatesStep, RenderPrometheusTemplatesStep, @@ -215,22 +215,28 @@ impl ReleaseCommandHandler { // Step 7: Deploy Prometheus configuration to remote (if enabled) self.deploy_prometheus_config_to_remote(environment, instance_ip)?; - // Step 8: Render Grafana provisioning templates (if enabled) + // Step 8: Create Grafana storage directories (if enabled) + Self::create_grafana_storage(environment, instance_ip)?; + + // Step 9: Render Grafana provisioning templates (if enabled) Self::render_grafana_templates(environment)?; - // Step 9: Deploy Grafana provisioning to remote (if enabled) + // Step 10: Deploy Grafana provisioning to remote (if enabled) self.deploy_grafana_provisioning_to_remote(environment, instance_ip)?; - // Step 10: Render Caddy configuration templates (if HTTPS enabled) + // Step 11: Create MySQL storage directories (if enabled) + Self::create_mysql_storage(environment, instance_ip)?; + + // Step 12: Render Caddy configuration templates (if HTTPS enabled) Self::render_caddy_templates(environment)?; - // Step 11: Deploy Caddy configuration to remote (if HTTPS enabled) + // Step 13: Deploy Caddy configuration to remote (if HTTPS enabled) self.deploy_caddy_config_to_remote(environment, instance_ip)?; - // Step 12: Render Docker Compose templates + // Step 14: Render Docker Compose templates let compose_build_dir = self.render_docker_compose_templates(environment).await?; - // Step 13: Deploy compose files to remote + // Step 15: Deploy compose files to remote self.deploy_compose_files_to_remote(environment, &compose_build_dir, instance_ip)?; let released = environment.clone().released(); @@ -430,6 +436,98 @@ impl ReleaseCommandHandler { Ok(()) } + /// Create Grafana storage directories on the remote host (if enabled) + /// + /// This step is optional and only executes if Grafana is configured in the environment. + /// If Grafana is not configured, the step is skipped without error. + /// + /// # Errors + /// + /// Returns a tuple of (error, `ReleaseStep::CreateGrafanaStorage`) if creation fails + #[allow(clippy::result_large_err)] + fn create_grafana_storage( + environment: &Environment, + _instance_ip: IpAddr, + ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::CreateGrafanaStorage; + + // Check if Grafana is configured + if environment.context().user_inputs.grafana().is_none() { + info!( + command = "release", + step = %current_step, + status = "skipped", + "Grafana not configured - skipping storage creation" + ); + return Ok(()); + } + + let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); + + CreateGrafanaStorageStep::new(ansible_client) + .execute() + .map_err(|e| { + ( + ReleaseCommandHandlerError::GrafanaStorageCreation(e.to_string()), + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "Grafana storage directories created successfully" + ); + + Ok(()) + } + + /// Create `MySQL` storage directories on the remote host (if enabled) + /// + /// This step is optional and only executes if `MySQL` is configured as the tracker database. + /// If `MySQL` is not configured, the step is skipped without error. + /// + /// # Errors + /// + /// Returns a tuple of (error, `ReleaseStep::CreateMysqlStorage`) if creation fails + #[allow(clippy::result_large_err)] + fn create_mysql_storage( + environment: &Environment, + _instance_ip: IpAddr, + ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::CreateMysqlStorage; + + // Check if MySQL is configured (via tracker database driver) + if !environment.context().user_inputs.tracker().uses_mysql() { + info!( + command = "release", + step = %current_step, + status = "skipped", + "MySQL not configured - skipping storage creation" + ); + return Ok(()); + } + + let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); + + CreateMysqlStorageStep::new(ansible_client) + .execute() + .map_err(|e| { + ( + ReleaseCommandHandlerError::MysqlStorageCreation(e.to_string()), + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "MySQL storage directories created successfully" + ); + + Ok(()) + } + /// Deploy Prometheus configuration to the remote host via Ansible (if enabled) /// /// This step is optional and only executes if Prometheus is configured in the environment. diff --git a/src/application/steps/application/create_grafana_storage.rs b/src/application/steps/application/create_grafana_storage.rs new file mode 100644 index 00000000..2d983ade --- /dev/null +++ b/src/application/steps/application/create_grafana_storage.rs @@ -0,0 +1,119 @@ +//! Grafana storage directory creation step +//! +//! This module provides the `CreateGrafanaStorageStep` which handles creation +//! of the required directory structure for Grafana on remote hosts +//! via Ansible playbooks. This step ensures Grafana has the necessary +//! directories for persistent data storage. +//! +//! ## Key Features +//! +//! - Creates standardized directory structure for Grafana storage +//! - Sets appropriate ownership (472:472 for grafana user) +//! - Idempotent operation (safe to run multiple times) +//! - Only executes when Grafana is enabled in environment configuration +//! +//! ## Directory Structure +//! +//! The step creates the following directory hierarchy: +//! ```text +//! /opt/torrust/storage/grafana/ +//! └── data/ # Grafana persistent data (dashboards, sqlite db) +//! ``` +//! +//! ## Why Special Ownership? +//! +//! The Grafana container runs as user 472:472 (grafana). When using bind mounts +//! instead of named volumes, the host directory must be owned by this user/group +//! for the container to write data. Named volumes handle this automatically, +//! but bind mounts require explicit directory creation with correct permissions. +//! +//! See ADR: docs/decisions/bind-mount-standardization.md + +use std::sync::Arc; +use tracing::{info, instrument}; + +use crate::adapters::ansible::AnsibleClient; +use crate::shared::command::CommandError; + +/// Step that creates Grafana storage directories on a remote host via Ansible +/// +/// This step creates the necessary directory structure for Grafana, +/// ensuring all directories have correct ownership (472:472) and permissions. +pub struct CreateGrafanaStorageStep { + ansible_client: Arc, +} + +impl CreateGrafanaStorageStep { + /// Create a new Grafana storage directory creation step + /// + /// # Arguments + /// + /// * `ansible_client` - Ansible client for running playbooks + #[must_use] + pub fn new(ansible_client: Arc) -> Self { + Self { ansible_client } + } + + /// Execute the storage directory creation + /// + /// Runs the Ansible playbook that creates the Grafana storage directory structure. + /// + /// # Errors + /// + /// Returns `CommandError` if: + /// - Ansible playbook execution fails + /// - Directory creation fails on remote host + /// - Permission setting fails + #[instrument( + name = "create_grafana_storage", + skip_all, + fields(step_type = "system", component = "grafana", method = "ansible") + )] + pub fn execute(&self) -> Result<(), CommandError> { + info!( + step = "create_grafana_storage", + action = "create_directories", + "Creating Grafana storage directory structure" + ); + + match self + .ansible_client + .run_playbook("create-grafana-storage", &[]) + { + Ok(_) => { + info!( + step = "create_grafana_storage", + status = "success", + "Grafana storage directories created successfully" + ); + Ok(()) + } + Err(e) => { + tracing::error!( + step = "create_grafana_storage", + error = %e, + "Failed to create Grafana storage directories" + ); + Err(e) + } + } + } +} + +#[cfg(test)] +mod tests { + use tempfile::TempDir; + + use super::*; + + #[test] + fn it_should_create_grafana_storage_step() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let ansible_client = Arc::new(AnsibleClient::new(temp_dir.path().to_path_buf())); + + let step = CreateGrafanaStorageStep::new(ansible_client); + + // Step should be created successfully + assert!(!std::ptr::addr_of!(step).cast::<()>().is_null()); + } +} diff --git a/src/application/steps/application/create_mysql_storage.rs b/src/application/steps/application/create_mysql_storage.rs new file mode 100644 index 00000000..47fb854e --- /dev/null +++ b/src/application/steps/application/create_mysql_storage.rs @@ -0,0 +1,119 @@ +//! `MySQL` storage directory creation step +//! +//! This module provides the `CreateMysqlStorageStep` which handles creation +//! of the required directory structure for `MySQL` on remote hosts +//! via Ansible playbooks. This step ensures `MySQL` has the necessary +//! directories for persistent data storage. +//! +//! ## Key Features +//! +//! - Creates standardized directory structure for `MySQL` storage +//! - Sets appropriate ownership (999:999 for mysql user) +//! - Idempotent operation (safe to run multiple times) +//! - Only executes when `MySQL` is enabled in environment configuration +//! +//! ## Directory Structure +//! +//! The step creates the following directory hierarchy: +//! ```text +//! /opt/torrust/storage/mysql/ +//! └── data/ # MySQL persistent data (databases, tables) +//! ``` +//! +//! ## Why Special Ownership? +//! +//! The `MySQL` container runs as user 999:999 (mysql). When using bind mounts +//! instead of named volumes, the host directory must be owned by this user/group +//! for the container to write data. Named volumes handle this automatically, +//! but bind mounts require explicit directory creation with correct permissions. +//! +//! See ADR: docs/decisions/bind-mount-standardization.md + +use std::sync::Arc; +use tracing::{info, instrument}; + +use crate::adapters::ansible::AnsibleClient; +use crate::shared::command::CommandError; + +/// Step that creates `MySQL` storage directories on a remote host via Ansible +/// +/// This step creates the necessary directory structure for `MySQL`, +/// ensuring all directories have correct ownership (999:999) and permissions. +pub struct CreateMysqlStorageStep { + ansible_client: Arc, +} + +impl CreateMysqlStorageStep { + /// Create a new `MySQL` storage directory creation step + /// + /// # Arguments + /// + /// * `ansible_client` - Ansible client for running playbooks + #[must_use] + pub fn new(ansible_client: Arc) -> Self { + Self { ansible_client } + } + + /// Execute the storage directory creation + /// + /// Runs the Ansible playbook that creates the `MySQL` storage directory structure. + /// + /// # Errors + /// + /// Returns `CommandError` if: + /// - Ansible playbook execution fails + /// - Directory creation fails on remote host + /// - Permission setting fails + #[instrument( + name = "create_mysql_storage", + skip_all, + fields(step_type = "system", component = "mysql", method = "ansible") + )] + pub fn execute(&self) -> Result<(), CommandError> { + info!( + step = "create_mysql_storage", + action = "create_directories", + "Creating MySQL storage directory structure" + ); + + match self + .ansible_client + .run_playbook("create-mysql-storage", &[]) + { + Ok(_) => { + info!( + step = "create_mysql_storage", + status = "success", + "MySQL storage directories created successfully" + ); + Ok(()) + } + Err(e) => { + tracing::error!( + step = "create_mysql_storage", + error = %e, + "Failed to create MySQL storage directories" + ); + Err(e) + } + } + } +} + +#[cfg(test)] +mod tests { + use tempfile::TempDir; + + use super::*; + + #[test] + fn it_should_create_mysql_storage_step() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let ansible_client = Arc::new(AnsibleClient::new(temp_dir.path().to_path_buf())); + + let step = CreateMysqlStorageStep::new(ansible_client); + + // Step should be created successfully + assert!(!std::ptr::addr_of!(step).cast::<()>().is_null()); + } +} diff --git a/src/application/steps/application/mod.rs b/src/application/steps/application/mod.rs index a1602c32..6fd78aac 100644 --- a/src/application/steps/application/mod.rs +++ b/src/application/steps/application/mod.rs @@ -11,7 +11,9 @@ //! - `deploy_tracker_config` - Deploys tracker.toml configuration file to remote host //! - `create_prometheus_storage` - Creates Prometheus storage directory structure on remote host //! - `deploy_prometheus_config` - Deploys prometheus.yml configuration file to remote host +//! - `create_grafana_storage` - Creates Grafana storage directory structure on remote host //! - `deploy_grafana_provisioning` - Deploys Grafana provisioning files (datasources/dashboards) to remote host +//! - `create_mysql_storage` - Creates `MySQL` storage directory structure on remote host //! - `deploy_compose_files` - Deploys Docker Compose files to remote host via Ansible //! - `start_services` - Starts Docker Compose services via Ansible //! - `run` - Legacy run step (placeholder) @@ -29,6 +31,8 @@ //! software installation steps to provide complete deployment workflows //! from infrastructure provisioning to application operation. +pub mod create_grafana_storage; +pub mod create_mysql_storage; pub mod create_prometheus_storage; pub mod create_tracker_storage; pub mod deploy_caddy_config; @@ -40,6 +44,8 @@ pub mod init_tracker_database; pub mod run; pub mod start_services; +pub use create_grafana_storage::CreateGrafanaStorageStep; +pub use create_mysql_storage::CreateMysqlStorageStep; pub use create_prometheus_storage::CreatePrometheusStorageStep; pub use create_tracker_storage::CreateTrackerStorageStep; pub use deploy_caddy_config::DeployCaddyConfigStep; diff --git a/src/domain/environment/state/release_failed.rs b/src/domain/environment/state/release_failed.rs index 2e5593a9..b738a925 100644 --- a/src/domain/environment/state/release_failed.rs +++ b/src/domain/environment/state/release_failed.rs @@ -44,10 +44,14 @@ pub enum ReleaseStep { RenderPrometheusTemplates, /// Deploying Prometheus configuration to the remote host via Ansible DeployPrometheusConfigToRemote, + /// Creating Grafana storage directories on remote host + CreateGrafanaStorage, /// Rendering Grafana provisioning templates to the build directory RenderGrafanaTemplates, /// Deploying Grafana provisioning configuration to the remote host via Ansible DeployGrafanaProvisioning, + /// Creating `MySQL` storage directories on remote host + CreateMysqlStorage, /// Rendering Caddy configuration templates to the build directory (if HTTPS enabled) RenderCaddyTemplates, /// Deploying Caddy configuration to the remote host via Ansible (if HTTPS enabled) @@ -68,8 +72,10 @@ impl fmt::Display for ReleaseStep { Self::CreatePrometheusStorage => "Create Prometheus Storage", Self::RenderPrometheusTemplates => "Render Prometheus Templates", Self::DeployPrometheusConfigToRemote => "Deploy Prometheus Config to Remote", + Self::CreateGrafanaStorage => "Create Grafana Storage", Self::RenderGrafanaTemplates => "Render Grafana Templates", Self::DeployGrafanaProvisioning => "Deploy Grafana Provisioning", + Self::CreateMysqlStorage => "Create MySQL Storage", Self::RenderCaddyTemplates => "Render Caddy Templates", Self::DeployCaddyConfigToRemote => "Deploy Caddy Config to Remote", Self::RenderDockerComposeTemplates => "Render Docker Compose Templates", diff --git a/src/domain/tracker/config/mod.rs b/src/domain/tracker/config/mod.rs index cca91ae6..4c194dab 100644 --- a/src/domain/tracker/config/mod.rs +++ b/src/domain/tracker/config/mod.rs @@ -286,6 +286,47 @@ impl TrackerConfig { &self.health_check_api } + /// Returns whether the tracker is configured to use `MySQL` database. + /// + /// This is useful for determining if MySQL-related infrastructure + /// (like storage directories) needs to be created. + /// + /// # Examples + /// + /// ```rust + /// use torrust_tracker_deployer_lib::domain::tracker::{ + /// TrackerConfig, TrackerCoreConfig, DatabaseConfig, SqliteConfig, + /// UdpTrackerConfig, HttpTrackerConfig, HttpApiConfig, HealthCheckApiConfig + /// }; + /// + /// let tracker_config = TrackerConfig::new( + /// TrackerCoreConfig::new( + /// DatabaseConfig::Sqlite(SqliteConfig::new("tracker.db").unwrap()), + /// false, + /// ), + /// vec![UdpTrackerConfig::new("0.0.0.0:6969".parse().unwrap(), None).unwrap()], + /// vec![HttpTrackerConfig::new("0.0.0.0:7070".parse().unwrap(), None, false).unwrap()], + /// HttpApiConfig::new( + /// "0.0.0.0:1212".parse().unwrap(), + /// "MyAccessToken".to_string().into(), + /// None, + /// false, + /// ).expect("valid config"), + /// HealthCheckApiConfig::new( + /// "127.0.0.1:1313".parse().unwrap(), + /// None, + /// false, + /// ).expect("valid config"), + /// ).expect("valid config"); + /// + /// // SQLite config -> not MySQL + /// assert!(!tracker_config.uses_mysql()); + /// ``` + #[must_use] + pub fn uses_mysql(&self) -> bool { + matches!(self.core.database(), DatabaseConfig::Mysql(_)) + } + /// Checks for socket address conflicts /// /// Validates that no two services using the same protocol attempt to bind diff --git a/src/infrastructure/templating/ansible/template/renderer/project_generator.rs b/src/infrastructure/templating/ansible/template/renderer/project_generator.rs index 4de7095b..9ab87524 100644 --- a/src/infrastructure/templating/ansible/template/renderer/project_generator.rs +++ b/src/infrastructure/templating/ansible/template/renderer/project_generator.rs @@ -311,7 +311,9 @@ impl AnsibleProjectGenerator { "deploy-tracker-config.yml", "create-prometheus-storage.yml", "deploy-prometheus-config.yml", + "create-grafana-storage.yml", "deploy-grafana-provisioning.yml", + "create-mysql-storage.yml", "deploy-caddy-config.yml", "deploy-compose-files.yml", "run-compose-services.yml", @@ -322,7 +324,7 @@ impl AnsibleProjectGenerator { tracing::debug!( "Successfully copied {} static template files", - 18 // ansible.cfg + 17 playbooks + 20 // ansible.cfg + 19 playbooks ); Ok(()) diff --git a/src/infrastructure/templating/docker_compose/template/renderer/docker_compose.rs b/src/infrastructure/templating/docker_compose/template/renderer/docker_compose.rs index bec247c6..98ecb247 100644 --- a/src/infrastructure/templating/docker_compose/template/renderer/docker_compose.rs +++ b/src/infrastructure/templating/docker_compose/template/renderer/docker_compose.rs @@ -291,14 +291,15 @@ mod tests { ); assert!(content.contains("ping"), "Should use ping command"); - // Verify MySQL volume + // Verify MySQL volume uses bind mount (not named volume) assert!( - content.contains("mysql_data:"), - "Should have mysql_data volume definition" + content.contains("./storage/mysql/data:/var/lib/mysql"), + "Should use bind mount for mysql data" ); + // Named volumes section should NOT contain mysql_data (we use bind mounts now) assert!( - content.contains("driver: local"), - "Volume should use local driver" + !content.contains("mysql_data:"), + "Should NOT have named mysql_data volume (using bind mounts)" ); // Verify port is NOT exposed (security fix: https://github.com/torrust/torrust-tracker-deployer/issues/277) diff --git a/templates/ansible/create-grafana-storage.yml b/templates/ansible/create-grafana-storage.yml new file mode 100644 index 00000000..1dc10f3b --- /dev/null +++ b/templates/ansible/create-grafana-storage.yml @@ -0,0 +1,32 @@ +--- +# Create Grafana Storage Directory +# +# This playbook creates the Grafana data directory with correct ownership. +# Grafana container runs as user 472:472 (grafana), so the host directory +# must be owned by that user/group for the container to write data. +# +# This is required because we use bind mounts instead of named volumes. +# Docker named volumes automatically handle ownership, but bind mounts +# require explicit directory creation with correct permissions. +# +# Variables: +# - deploy_dir: Base deployment directory (e.g., /opt/torrust) +# - grafana_enabled: Whether Grafana is enabled (from variables.yml) +# +# See ADR: docs/decisions/bind-mount-standardization.md + +- name: Create Grafana storage directory + hosts: all + become: true + vars_files: + - variables.yml + + tasks: + - name: Create Grafana data directory with correct ownership + ansible.builtin.file: + path: "{{ deploy_dir }}/storage/grafana/data" + state: directory + mode: "0755" + owner: "472" + group: "472" + when: grafana_enabled | default(false) diff --git a/templates/ansible/create-mysql-storage.yml b/templates/ansible/create-mysql-storage.yml new file mode 100644 index 00000000..7609cf01 --- /dev/null +++ b/templates/ansible/create-mysql-storage.yml @@ -0,0 +1,32 @@ +--- +# Create MySQL Storage Directory +# +# This playbook creates the MySQL data directory with correct ownership. +# MySQL container runs as user 999:999 (mysql), so the host directory +# must be owned by that user/group for the container to write data. +# +# This is required because we use bind mounts instead of named volumes. +# Docker named volumes automatically handle ownership, but bind mounts +# require explicit directory creation with correct permissions. +# +# Variables: +# - deploy_dir: Base deployment directory (e.g., /opt/torrust) +# - mysql_enabled: Whether MySQL is enabled (from variables.yml) +# +# See ADR: docs/decisions/bind-mount-standardization.md + +- name: Create MySQL storage directory + hosts: all + become: true + vars_files: + - variables.yml + + tasks: + - name: Create MySQL data directory with correct ownership + ansible.builtin.file: + path: "{{ deploy_dir }}/storage/mysql/data" + state: directory + mode: "0755" + owner: "999" + group: "999" + when: mysql_enabled | default(false) diff --git a/templates/ansible/variables.yml.tera b/templates/ansible/variables.yml.tera index 02f57e23..42cd3274 100644 --- a/templates/ansible/variables.yml.tera +++ b/templates/ansible/variables.yml.tera @@ -14,6 +14,7 @@ deploy_dir: /opt/torrust # Service Enablement Flags grafana_enabled: {{ grafana_config is defined }} +mysql_enabled: {{ mysql_config is defined }} # Tracker Firewall Configuration {% if tracker_udp_ports is defined and tracker_udp_ports | length > 0 -%} diff --git a/templates/docker-compose/docker-compose.yml.tera b/templates/docker-compose/docker-compose.yml.tera index da0c7ddd..8e01293d 100644 --- a/templates/docker-compose/docker-compose.yml.tera +++ b/templates/docker-compose/docker-compose.yml.tera @@ -47,8 +47,8 @@ services: - "443:443/udp" # HTTP/3 (QUIC) volumes: - ./storage/caddy/etc/Caddyfile:/etc/caddy/Caddyfile:ro - - caddy_data:/data # TLS certificates (MUST persist!) - - caddy_config_vol:/config + - ./storage/caddy/data:/data # TLS certificates (MUST persist!) + - ./storage/caddy/config:/config networks: {%- for network in caddy.networks %} - {{ network }} @@ -145,7 +145,7 @@ services: - GF_SECURITY_ADMIN_USER=${GF_SECURITY_ADMIN_USER} - GF_SECURITY_ADMIN_PASSWORD=${GF_SECURITY_ADMIN_PASSWORD} volumes: - - grafana_data:/var/lib/grafana + - ./storage/grafana/data:/var/lib/grafana - ./storage/grafana/provisioning:/etc/grafana/provisioning:ro healthcheck: test: ["CMD", "wget", "--spider", "-q", "http://localhost:3000/api/health"] @@ -178,7 +178,7 @@ services: # - This prevents unauthorized external access to the database # See: https://github.com/torrust/torrust-tracker-deployer/issues/277 volumes: - - mysql_data:/var/lib/mysql + - ./storage/mysql/data:/var/lib/mysql command: --mysql-native-password=ON healthcheck: test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "-u", "root", "-p$$MYSQL_ROOT_PASSWORD"] @@ -232,22 +232,3 @@ networks: proxy_network: driver: bridge {%- endif %} - - -{%- if mysql or grafana or caddy %} -volumes: -{%- if mysql %} - mysql_data: - driver: local -{%- endif %} -{%- if grafana %} - grafana_data: - driver: local -{%- endif %} -{%- if caddy %} - caddy_data: - driver: local - caddy_config_vol: - driver: local -{%- endif %} -{% endif %}