From c724492410f7d0815f9bb993424d3e701bf3fe20 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 26 Jan 2026 17:37:14 +0000 Subject: [PATCH 01/12] docs: add code quality analysis for release handler --- docs/analysis/release-handler-code-quality.md | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 docs/analysis/release-handler-code-quality.md diff --git a/docs/analysis/release-handler-code-quality.md b/docs/analysis/release-handler-code-quality.md new file mode 100644 index 00000000..c53c1810 --- /dev/null +++ b/docs/analysis/release-handler-code-quality.md @@ -0,0 +1,184 @@ +# Code Quality Analysis: Release Command Handler + +**File**: [src/application/command_handlers/release/handler.rs](../../src/application/command_handlers/release/handler.rs) +**Date**: 2026-01-26 +**Lines of Code**: 994 + +## Executive Summary + +The `ReleaseCommandHandler` implements a release workflow for deploying software to configured environments. While it demonstrates good architecture principles (DDD, state machine pattern, comprehensive documentation), there are several maintainability and readability concerns worth addressing. + +## Critical Issues + +### 1. Severe Code Duplication (DRY Violation) + +The file contains **massive repetitive patterns** across 10+ methods. Each step method follows the same structure: + +```rust +fn step_X(environment, instance_ip) -> StepResult<...> { + let current_step = ReleaseStep::X; + + // Optional: Check if feature is configured + if environment.context().user_inputs.feature().is_none() { + info!(..., status = "skipped", "..."); + return Ok(()); + } + + let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); + + SomeStep::new(ansible_client).execute().map_err(|e| { + (ReleaseCommandHandlerError::SomeVariant(e.to_string()), current_step) + })?; + + info!(..., "... completed successfully"); + Ok(()) +} +``` + +**Impact**: Adding a new step requires copying ~30 lines and modifying a few values. This pattern appears **~15 times**. + +### 2. Unused Parameters (`_instance_ip`) + +Multiple methods accept `instance_ip: IpAddr` but prefix it with `_` to suppress unused warnings: + +| Method | Uses `instance_ip`? | +| --------------------------------------- | -------------------------- | +| `create_tracker_storage` | No (`_instance_ip`) | +| `init_tracker_database` | No (`_instance_ip`) | +| `create_prometheus_storage` | No (`_instance_ip`) | +| `create_grafana_storage` | No (`_instance_ip`) | +| `create_mysql_storage` | No (`_instance_ip`) | +| `deploy_prometheus_config_to_remote` | No (`_instance_ip`) | +| `deploy_caddy_config_to_remote` | No (`_instance_ip`) | +| `deploy_grafana_provisioning_to_remote` | No (`_instance_ip`) | +| `deploy_tracker_config_to_remote` | No (`_instance_ip`) | +| `deploy_compose_files_to_remote` | **Yes** (only for logging) | + +**Impact**: 9 out of 10 methods don't use `instance_ip` for logic. This suggests either: + +- Future functionality that was never implemented +- An interface design that doesn't match actual needs +- Copy-paste from a method that did need it + +### 3. Inconsistent Error Mapping + +Different errors use different construction patterns: + +```rust +// Pattern 1: String conversion only +ReleaseCommandHandlerError::TemplateRendering(e.to_string()) + +// Pattern 2: String + Source (loses type info) +ReleaseCommandHandlerError::Deployment { + message: e.to_string(), + source: Box::new(e), +} + +// Pattern 3: Typed source preserved +ReleaseCommandHandlerError::DeploymentFailed { + message: e.to_string(), + source: e, // DeployComposeFilesStepError +} +``` + +**Impact**: Inconsistent error handling makes debugging harder and violates the project's error handling guidelines. + +## Moderate Issues + +### 4. Method Length and Cognitive Load + +| Method | Lines | Complexity | +| -------------------------- | ----- | ---------------------------------- | +| `execute_release_workflow` | ~50 | 15 sequential steps | +| `execute` | ~60 | State transitions + error handling | + +The `execute_release_workflow` method is essentially a flat list of 15 steps, making it hard to understand the logical groupings. + +### 5. Magic String Duplication + +The string `"ansible"` appears as a path suffix **11 times**: + +```rust +environment.build_dir().join("ansible") +``` + +### 6. Clippy Attribute Proliferation + +Many methods require `#[allow(clippy::result_large_err)]` due to the `StepResult` tuple design: + +```rust +#[allow(clippy::result_large_err)] +#[allow(clippy::result_large_err, clippy::unused_self)] +``` + +This suggests the error type design may need reconsideration. + +### 7. Inconsistent Static vs Instance Methods + +| Method Type | Methods | +| ----------------- | ------------------------------------------------------------------------------------------- | +| `Self::` (static) | `create_tracker_storage`, `init_tracker_database`, `render_*_templates`, `create_*_storage` | +| `&self` | `deploy_*_to_remote`, `render_docker_compose_templates` | + +Some `&self` methods don't actually use `self` (hence `clippy::unused_self`). The inconsistency makes the API confusing. + +## Positive Aspects + +1. **Excellent Documentation**: Every method has comprehensive doc comments with `# Errors` sections +2. **Tracing Integration**: Good use of structured logging with `tracing::info!` +3. **Type-State Pattern**: Proper use of `Environment` → `Environment` → `Environment` +4. **Failure Context**: Good failure tracking with `ReleaseFailureContext` and trace file generation +5. **Instrumentation**: Proper use of `#[instrument]` for tracing spans + +## Recommendations + +### Priority 1: Extract Common Step Execution Pattern + +Create a generic step executor that handles the repetitive boilerplate: + +```rust +// Conceptual approach +fn execute_step( + &self, + step: ReleaseStep, + condition: impl Fn(&Environment) -> bool, + skip_message: &str, + step_factory: impl Fn() -> S, + error_mapper: impl Fn(S::Error) -> ReleaseCommandHandlerError, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> +where + S: Step +``` + +### Priority 2: Group Related Steps + +Consider grouping the 15 steps into logical phases: + +1. **Storage Phase**: Create all storage directories +2. **Render Phase**: Render all templates +3. **Deploy Phase**: Deploy all configurations + +### Priority 3: Remove Unused Parameters + +Either remove `instance_ip` from methods that don't use it, or document why it's there for future use. + +### Priority 4: Standardize Error Construction + +Use a consistent pattern—preferably preserving the source error with `#[source]` for proper error chain debugging. + +### Priority 5: Extract Constants + +```rust +const ANSIBLE_DIR: &str = "ansible"; +``` + +## Metrics Summary + +| Metric | Value | Assessment | +| --------------------- | --------- | ------------------------- | +| Lines of Code | 994 | High for a single handler | +| Methods | ~20 | Reasonable | +| Duplicated Patterns | ~15 | High | +| Unused Parameters | 9 methods | Problematic | +| `#[allow]` Attributes | 12+ | Code smell | +| Doc Coverage | ~100% | Excellent | From 8330d50a197ea5c136827f28c5aa695fe376ad21 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 26 Jan 2026 17:41:57 +0000 Subject: [PATCH 02/12] refactor: group release workflow steps by service --- .../command_handlers/release/handler.rs | 183 ++++++++++++++---- 1 file changed, 150 insertions(+), 33 deletions(-) diff --git a/src/application/command_handlers/release/handler.rs b/src/application/command_handlers/release/handler.rs index 1b25b649..37a6a9cd 100644 --- a/src/application/command_handlers/release/handler.rs +++ b/src/application/command_handlers/release/handler.rs @@ -172,11 +172,14 @@ impl ReleaseCommandHandler { /// Execute the release workflow with step tracking /// - /// This method orchestrates the complete release workflow: - /// 1. Create tracker storage directories - /// 2. Initialize tracker `SQLite` database - /// 3. Render Docker Compose templates to the build directory - /// 4. Deploy compose files to the remote host via Ansible + /// This method orchestrates the complete release workflow, organized by service: + /// + /// 1. **Tracker**: Storage creation, database init, config rendering, deployment + /// 2. **Prometheus**: Storage creation, config rendering, deployment (if enabled) + /// 3. **Grafana**: Storage creation, provisioning rendering, deployment (if enabled) + /// 4. **`MySQL`**: Storage creation (if enabled) + /// 5. **Caddy**: Config rendering, deployment (if HTTPS enabled) + /// 6. **Docker Compose**: Template rendering, deployment /// /// If an error occurs, it returns both the error and the step that was being /// executed, enabling accurate failure context generation. @@ -194,56 +197,170 @@ impl ReleaseCommandHandler { environment: &Environment, instance_ip: IpAddr, ) -> StepResult, ReleaseCommandHandlerError, ReleaseStep> { - // Step 1: Create tracker storage directories - Self::create_tracker_storage(environment, instance_ip)?; + // Tracker service steps + self.release_tracker_service(environment, instance_ip)?; - // Step 2: Initialize tracker database - Self::init_tracker_database(environment, instance_ip)?; + // Prometheus service steps (if enabled) + self.release_prometheus_service(environment, instance_ip)?; - // Step 3: Render tracker configuration templates - let tracker_build_dir = Self::render_tracker_templates(environment)?; + // Grafana service steps (if enabled) + self.release_grafana_service(environment, instance_ip)?; + + // MySQL service steps (if enabled) + Self::release_mysql_service(environment, instance_ip)?; + + // Caddy service steps (if HTTPS enabled) + self.release_caddy_service(environment, instance_ip)?; + + // Docker Compose deployment + self.release_docker_compose(environment, instance_ip) + .await?; + + let released = environment.clone().released(); + + Ok(released) + } - // Step 4: Deploy tracker configuration to remote + // ========================================================================= + // Service-level release orchestration + // ========================================================================= + + /// Release the Tracker service + /// + /// Executes all steps required to release the Tracker: + /// 1. Create storage directories + /// 2. Initialize database + /// 3. Render configuration templates + /// 4. Deploy configuration to remote + /// + /// # Errors + /// + /// Returns a tuple of (error, step) if any tracker step fails + #[allow(clippy::result_large_err, clippy::unused_self)] + fn release_tracker_service( + &self, + environment: &Environment, + instance_ip: IpAddr, + ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + Self::create_tracker_storage(environment, instance_ip)?; + Self::init_tracker_database(environment, instance_ip)?; + let tracker_build_dir = Self::render_tracker_templates(environment)?; self.deploy_tracker_config_to_remote(environment, &tracker_build_dir, instance_ip)?; + Ok(()) + } - // Step 5: Create Prometheus storage directories (if enabled) + /// Release the Prometheus service (if enabled) + /// + /// Executes all steps required to release Prometheus: + /// 1. Create storage directories + /// 2. Render configuration templates + /// 3. Deploy configuration to remote + /// + /// If Prometheus is not configured, all steps are skipped. + /// + /// # Errors + /// + /// Returns a tuple of (error, step) if any Prometheus step fails + #[allow(clippy::result_large_err, clippy::unused_self)] + fn release_prometheus_service( + &self, + environment: &Environment, + instance_ip: IpAddr, + ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { Self::create_prometheus_storage(environment, instance_ip)?; - - // Step 6: Render Prometheus configuration templates (if enabled) Self::render_prometheus_templates(environment)?; - - // Step 7: Deploy Prometheus configuration to remote (if enabled) self.deploy_prometheus_config_to_remote(environment, instance_ip)?; + Ok(()) + } - // Step 8: Create Grafana storage directories (if enabled) + /// Release the Grafana service (if enabled) + /// + /// Executes all steps required to release Grafana: + /// 1. Create storage directories + /// 2. Render provisioning templates + /// 3. Deploy provisioning to remote + /// + /// If Grafana is not configured, all steps are skipped. + /// + /// # Errors + /// + /// Returns a tuple of (error, step) if any Grafana step fails + #[allow(clippy::result_large_err, clippy::unused_self)] + fn release_grafana_service( + &self, + environment: &Environment, + instance_ip: IpAddr, + ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { Self::create_grafana_storage(environment, instance_ip)?; - - // Step 9: Render Grafana provisioning templates (if enabled) Self::render_grafana_templates(environment)?; - - // Step 10: Deploy Grafana provisioning to remote (if enabled) self.deploy_grafana_provisioning_to_remote(environment, instance_ip)?; + Ok(()) + } - // Step 11: Create MySQL storage directories (if enabled) + /// Release the `MySQL` service (if enabled) + /// + /// Executes all steps required to release `MySQL`: + /// 1. Create storage directories + /// + /// If `MySQL` is not configured as the tracker database, this step is skipped. + /// + /// # Errors + /// + /// Returns a tuple of (error, step) if `MySQL` storage creation fails + #[allow(clippy::result_large_err)] + fn release_mysql_service( + environment: &Environment, + instance_ip: IpAddr, + ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { Self::create_mysql_storage(environment, instance_ip)?; + Ok(()) + } - // Step 12: Render Caddy configuration templates (if HTTPS enabled) + /// Release the Caddy service (if HTTPS enabled) + /// + /// Executes all steps required to release Caddy: + /// 1. Render configuration templates + /// 2. Deploy configuration to remote + /// + /// If HTTPS is not configured, all steps are skipped. + /// + /// # Errors + /// + /// Returns a tuple of (error, step) if any Caddy step fails + #[allow(clippy::result_large_err, clippy::unused_self)] + fn release_caddy_service( + &self, + environment: &Environment, + instance_ip: IpAddr, + ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { Self::render_caddy_templates(environment)?; - - // Step 13: Deploy Caddy configuration to remote (if HTTPS enabled) self.deploy_caddy_config_to_remote(environment, instance_ip)?; + Ok(()) + } - // Step 14: Render Docker Compose templates + /// Release Docker Compose configuration + /// + /// Executes all steps required to deploy Docker Compose: + /// 1. Render Docker Compose templates + /// 2. Deploy compose files to remote + /// + /// # Errors + /// + /// Returns a tuple of (error, step) if any Docker Compose step fails + async fn release_docker_compose( + &self, + environment: &Environment, + instance_ip: IpAddr, + ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let compose_build_dir = self.render_docker_compose_templates(environment).await?; - - // Step 15: Deploy compose files to remote self.deploy_compose_files_to_remote(environment, &compose_build_dir, instance_ip)?; - - let released = environment.clone().released(); - - Ok(released) + Ok(()) } + // ========================================================================= + // Individual step implementations + // ========================================================================= + /// Create tracker storage directories on the remote host /// /// # Errors From 36dd7233ebcf6688e4a85badd6c32f1b2c0e499a Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 26 Jan 2026 17:48:53 +0000 Subject: [PATCH 03/12] refactor: extract AnsibleClient construction helper --- .../command_handlers/release/handler.rs | 66 ++++++++----------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/src/application/command_handlers/release/handler.rs b/src/application/command_handlers/release/handler.rs index 37a6a9cd..8fc00a40 100644 --- a/src/application/command_handlers/release/handler.rs +++ b/src/application/command_handlers/release/handler.rs @@ -361,6 +361,13 @@ impl ReleaseCommandHandler { // Individual step implementations // ========================================================================= + /// Create an Ansible client configured for the environment's build directory + /// + /// This is a helper method to reduce duplication across step implementations. + fn ansible_client(environment: &Environment) -> Arc { + Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))) + } + /// Create tracker storage directories on the remote host /// /// # Errors @@ -373,9 +380,7 @@ impl ReleaseCommandHandler { ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::CreateTrackerStorage; - let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); - - CreateTrackerStorageStep::new(ansible_client) + CreateTrackerStorageStep::new(Self::ansible_client(environment)) .execute() .map_err(|e| { ( @@ -405,9 +410,7 @@ impl ReleaseCommandHandler { ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::InitTrackerDatabase; - let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); - - InitTrackerDatabaseStep::new(ansible_client) + InitTrackerDatabaseStep::new(Self::ansible_client(environment)) .execute() .map_err(|e| { ( @@ -533,9 +536,7 @@ impl ReleaseCommandHandler { return Ok(()); } - let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); - - CreatePrometheusStorageStep::new(ansible_client) + CreatePrometheusStorageStep::new(Self::ansible_client(environment)) .execute() .map_err(|e| { ( @@ -579,9 +580,7 @@ impl ReleaseCommandHandler { return Ok(()); } - let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); - - CreateGrafanaStorageStep::new(ansible_client) + CreateGrafanaStorageStep::new(Self::ansible_client(environment)) .execute() .map_err(|e| { ( @@ -625,9 +624,7 @@ impl ReleaseCommandHandler { return Ok(()); } - let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); - - CreateMysqlStorageStep::new(ansible_client) + CreateMysqlStorageStep::new(Self::ansible_client(environment)) .execute() .map_err(|e| { ( @@ -677,9 +674,7 @@ impl ReleaseCommandHandler { return Ok(()); } - let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); - - DeployPrometheusConfigStep::new(ansible_client) + DeployPrometheusConfigStep::new(Self::ansible_client(environment)) .execute() .map_err(|e| { ( @@ -831,9 +826,7 @@ impl ReleaseCommandHandler { return Ok(()); } - let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); - - DeployCaddyConfigStep::new(ansible_client) + DeployCaddyConfigStep::new(Self::ansible_client(environment)) .execute() .map_err(|e| { ( @@ -889,9 +882,7 @@ impl ReleaseCommandHandler { return Ok(()); } - let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); - - DeployGrafanaProvisioningStep::new(ansible_client) + DeployGrafanaProvisioningStep::new(Self::ansible_client(environment)) .execute() .map_err(|e| { ( @@ -929,19 +920,20 @@ impl ReleaseCommandHandler { ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::DeployTrackerConfigToRemote; - let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); - - DeployTrackerConfigStep::new(ansible_client, tracker_build_dir.to_path_buf()) - .execute() - .map_err(|e| { - ( - ReleaseCommandHandlerError::Deployment { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; + DeployTrackerConfigStep::new( + Self::ansible_client(environment), + tracker_build_dir.to_path_buf(), + ) + .execute() + .map_err(|e| { + ( + ReleaseCommandHandlerError::Deployment { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; info!( command = "release", From a7b2a17f9a63df9f500bc74f68d8cad4042352f1 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 26 Jan 2026 17:51:50 +0000 Subject: [PATCH 04/12] refactor: remove unused instance_ip parameters from step methods --- .../command_handlers/release/handler.rs | 46 ++++++------------- 1 file changed, 15 insertions(+), 31 deletions(-) diff --git a/src/application/command_handlers/release/handler.rs b/src/application/command_handlers/release/handler.rs index 8fc00a40..81ddb791 100644 --- a/src/application/command_handlers/release/handler.rs +++ b/src/application/command_handlers/release/handler.rs @@ -187,7 +187,7 @@ impl ReleaseCommandHandler { /// # Arguments /// /// * `environment` - The environment in Releasing state - /// * `instance_ip` - The validated instance IP address (precondition checked by caller) + /// * `instance_ip` - The validated instance IP address (used for Docker Compose deployment logging) /// /// # Errors /// @@ -198,19 +198,19 @@ impl ReleaseCommandHandler { instance_ip: IpAddr, ) -> StepResult, ReleaseCommandHandlerError, ReleaseStep> { // Tracker service steps - self.release_tracker_service(environment, instance_ip)?; + self.release_tracker_service(environment)?; // Prometheus service steps (if enabled) - self.release_prometheus_service(environment, instance_ip)?; + self.release_prometheus_service(environment)?; // Grafana service steps (if enabled) - self.release_grafana_service(environment, instance_ip)?; + self.release_grafana_service(environment)?; // MySQL service steps (if enabled) - Self::release_mysql_service(environment, instance_ip)?; + Self::release_mysql_service(environment)?; // Caddy service steps (if HTTPS enabled) - self.release_caddy_service(environment, instance_ip)?; + self.release_caddy_service(environment)?; // Docker Compose deployment self.release_docker_compose(environment, instance_ip) @@ -240,12 +240,11 @@ impl ReleaseCommandHandler { fn release_tracker_service( &self, environment: &Environment, - instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - Self::create_tracker_storage(environment, instance_ip)?; - Self::init_tracker_database(environment, instance_ip)?; + Self::create_tracker_storage(environment)?; + Self::init_tracker_database(environment)?; let tracker_build_dir = Self::render_tracker_templates(environment)?; - self.deploy_tracker_config_to_remote(environment, &tracker_build_dir, instance_ip)?; + self.deploy_tracker_config_to_remote(environment, &tracker_build_dir)?; Ok(()) } @@ -265,11 +264,10 @@ impl ReleaseCommandHandler { fn release_prometheus_service( &self, environment: &Environment, - instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - Self::create_prometheus_storage(environment, instance_ip)?; + Self::create_prometheus_storage(environment)?; Self::render_prometheus_templates(environment)?; - self.deploy_prometheus_config_to_remote(environment, instance_ip)?; + self.deploy_prometheus_config_to_remote(environment)?; Ok(()) } @@ -289,11 +287,10 @@ impl ReleaseCommandHandler { fn release_grafana_service( &self, environment: &Environment, - instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - Self::create_grafana_storage(environment, instance_ip)?; + Self::create_grafana_storage(environment)?; Self::render_grafana_templates(environment)?; - self.deploy_grafana_provisioning_to_remote(environment, instance_ip)?; + self.deploy_grafana_provisioning_to_remote(environment)?; Ok(()) } @@ -310,9 +307,8 @@ impl ReleaseCommandHandler { #[allow(clippy::result_large_err)] fn release_mysql_service( environment: &Environment, - instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - Self::create_mysql_storage(environment, instance_ip)?; + Self::create_mysql_storage(environment)?; Ok(()) } @@ -331,10 +327,9 @@ impl ReleaseCommandHandler { fn release_caddy_service( &self, environment: &Environment, - instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { Self::render_caddy_templates(environment)?; - self.deploy_caddy_config_to_remote(environment, instance_ip)?; + self.deploy_caddy_config_to_remote(environment)?; Ok(()) } @@ -376,7 +371,6 @@ impl ReleaseCommandHandler { #[allow(clippy::result_large_err)] fn create_tracker_storage( environment: &Environment, - _instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::CreateTrackerStorage; @@ -406,7 +400,6 @@ impl ReleaseCommandHandler { #[allow(clippy::result_large_err)] fn init_tracker_database( environment: &Environment, - _instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::InitTrackerDatabase; @@ -521,7 +514,6 @@ impl ReleaseCommandHandler { #[allow(clippy::result_large_err)] fn create_prometheus_storage( environment: &Environment, - _instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::CreatePrometheusStorage; @@ -565,7 +557,6 @@ impl ReleaseCommandHandler { #[allow(clippy::result_large_err)] fn create_grafana_storage( environment: &Environment, - _instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::CreateGrafanaStorage; @@ -609,7 +600,6 @@ impl ReleaseCommandHandler { #[allow(clippy::result_large_err)] fn create_mysql_storage( environment: &Environment, - _instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::CreateMysqlStorage; @@ -650,7 +640,6 @@ impl ReleaseCommandHandler { /// # Arguments /// /// * `environment` - The environment in Releasing state - /// * `instance_ip` - The target instance IP address /// /// # Errors /// @@ -659,7 +648,6 @@ impl ReleaseCommandHandler { fn deploy_prometheus_config_to_remote( &self, environment: &Environment, - _instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::DeployPrometheusConfigToRemote; @@ -811,7 +799,6 @@ impl ReleaseCommandHandler { fn deploy_caddy_config_to_remote( &self, environment: &Environment, - _instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::DeployCaddyConfigToRemote; @@ -856,7 +843,6 @@ impl ReleaseCommandHandler { fn deploy_grafana_provisioning_to_remote( &self, environment: &Environment, - _instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::DeployGrafanaProvisioning; @@ -906,7 +892,6 @@ impl ReleaseCommandHandler { /// /// * `environment` - The environment in Releasing state /// * `tracker_build_dir` - Path to the rendered tracker configuration - /// * `instance_ip` - The target instance IP address /// /// # Errors /// @@ -916,7 +901,6 @@ impl ReleaseCommandHandler { &self, environment: &Environment, tracker_build_dir: &Path, - _instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::DeployTrackerConfigToRemote; From f87fa2cae5fb68a14babb988da1ea5091978a1fb Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 26 Jan 2026 18:02:38 +0000 Subject: [PATCH 05/12] refactor: convert unused self methods to associated functions --- .../command_handlers/release/handler.rs | 52 +++++++------------ 1 file changed, 20 insertions(+), 32 deletions(-) diff --git a/src/application/command_handlers/release/handler.rs b/src/application/command_handlers/release/handler.rs index 81ddb791..0421cf3a 100644 --- a/src/application/command_handlers/release/handler.rs +++ b/src/application/command_handlers/release/handler.rs @@ -198,23 +198,22 @@ impl ReleaseCommandHandler { instance_ip: IpAddr, ) -> StepResult, ReleaseCommandHandlerError, ReleaseStep> { // Tracker service steps - self.release_tracker_service(environment)?; + Self::release_tracker_service(environment)?; // Prometheus service steps (if enabled) - self.release_prometheus_service(environment)?; + Self::release_prometheus_service(environment)?; // Grafana service steps (if enabled) - self.release_grafana_service(environment)?; + Self::release_grafana_service(environment)?; // MySQL service steps (if enabled) Self::release_mysql_service(environment)?; // Caddy service steps (if HTTPS enabled) - self.release_caddy_service(environment)?; + Self::release_caddy_service(environment)?; // Docker Compose deployment - self.release_docker_compose(environment, instance_ip) - .await?; + Self::release_docker_compose(environment, instance_ip).await?; let released = environment.clone().released(); @@ -236,15 +235,14 @@ impl ReleaseCommandHandler { /// # Errors /// /// Returns a tuple of (error, step) if any tracker step fails - #[allow(clippy::result_large_err, clippy::unused_self)] + #[allow(clippy::result_large_err)] fn release_tracker_service( - &self, environment: &Environment, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { Self::create_tracker_storage(environment)?; Self::init_tracker_database(environment)?; let tracker_build_dir = Self::render_tracker_templates(environment)?; - self.deploy_tracker_config_to_remote(environment, &tracker_build_dir)?; + Self::deploy_tracker_config_to_remote(environment, &tracker_build_dir)?; Ok(()) } @@ -260,14 +258,13 @@ impl ReleaseCommandHandler { /// # Errors /// /// Returns a tuple of (error, step) if any Prometheus step fails - #[allow(clippy::result_large_err, clippy::unused_self)] + #[allow(clippy::result_large_err)] fn release_prometheus_service( - &self, environment: &Environment, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { Self::create_prometheus_storage(environment)?; Self::render_prometheus_templates(environment)?; - self.deploy_prometheus_config_to_remote(environment)?; + Self::deploy_prometheus_config_to_remote(environment)?; Ok(()) } @@ -283,14 +280,13 @@ impl ReleaseCommandHandler { /// # Errors /// /// Returns a tuple of (error, step) if any Grafana step fails - #[allow(clippy::result_large_err, clippy::unused_self)] + #[allow(clippy::result_large_err)] fn release_grafana_service( - &self, environment: &Environment, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { Self::create_grafana_storage(environment)?; Self::render_grafana_templates(environment)?; - self.deploy_grafana_provisioning_to_remote(environment)?; + Self::deploy_grafana_provisioning_to_remote(environment)?; Ok(()) } @@ -323,13 +319,12 @@ impl ReleaseCommandHandler { /// # Errors /// /// Returns a tuple of (error, step) if any Caddy step fails - #[allow(clippy::result_large_err, clippy::unused_self)] + #[allow(clippy::result_large_err)] fn release_caddy_service( - &self, environment: &Environment, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { Self::render_caddy_templates(environment)?; - self.deploy_caddy_config_to_remote(environment)?; + Self::deploy_caddy_config_to_remote(environment)?; Ok(()) } @@ -343,12 +338,11 @@ impl ReleaseCommandHandler { /// /// Returns a tuple of (error, step) if any Docker Compose step fails async fn release_docker_compose( - &self, environment: &Environment, instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let compose_build_dir = self.render_docker_compose_templates(environment).await?; - self.deploy_compose_files_to_remote(environment, &compose_build_dir, instance_ip)?; + let compose_build_dir = Self::render_docker_compose_templates(environment).await?; + Self::deploy_compose_files_to_remote(environment, &compose_build_dir, instance_ip)?; Ok(()) } @@ -644,9 +638,8 @@ impl ReleaseCommandHandler { /// # Errors /// /// Returns a tuple of (error, `ReleaseStep::DeployPrometheusConfigToRemote`) if deployment fails - #[allow(clippy::result_large_err, clippy::unused_self)] + #[allow(clippy::result_large_err)] fn deploy_prometheus_config_to_remote( - &self, environment: &Environment, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::DeployPrometheusConfigToRemote; @@ -795,9 +788,8 @@ impl ReleaseCommandHandler { /// # Errors /// /// Returns a tuple of (error, `ReleaseStep::DeployCaddyConfigToRemote`) if deployment fails - #[allow(clippy::result_large_err, clippy::unused_self)] + #[allow(clippy::result_large_err)] fn deploy_caddy_config_to_remote( - &self, environment: &Environment, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::DeployCaddyConfigToRemote; @@ -839,9 +831,8 @@ impl ReleaseCommandHandler { /// # Errors /// /// Returns a tuple of (error, `ReleaseStep::DeployGrafanaProvisioning`) if deployment fails - #[allow(clippy::result_large_err, clippy::unused_self)] + #[allow(clippy::result_large_err)] fn deploy_grafana_provisioning_to_remote( - &self, environment: &Environment, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::DeployGrafanaProvisioning; @@ -896,9 +887,8 @@ impl ReleaseCommandHandler { /// # Errors /// /// Returns a tuple of (error, `ReleaseStep::DeployTrackerConfigToRemote`) if deployment fails - #[allow(clippy::result_large_err, clippy::unused_self)] + #[allow(clippy::result_large_err)] fn deploy_tracker_config_to_remote( - &self, environment: &Environment, tracker_build_dir: &Path, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { @@ -934,7 +924,6 @@ impl ReleaseCommandHandler { /// /// Returns a tuple of (error, `ReleaseStep::RenderDockerComposeTemplates`) if rendering fails async fn render_docker_compose_templates( - &self, environment: &Environment, ) -> StepResult { let current_step = ReleaseStep::RenderDockerComposeTemplates; @@ -973,9 +962,8 @@ impl ReleaseCommandHandler { /// # Errors /// /// Returns a tuple of (error, `ReleaseStep::DeployComposeFilesToRemote`) if deployment fails - #[allow(clippy::result_large_err, clippy::unused_self)] + #[allow(clippy::result_large_err)] fn deploy_compose_files_to_remote( - &self, environment: &Environment, compose_build_dir: &Path, instance_ip: IpAddr, From d5f95e1dc9a4aa6bbe56ebf432ec47b35468060d Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 26 Jan 2026 18:27:49 +0000 Subject: [PATCH 06/12] refactor: normalize error types to include source for error chaining - Convert all step-related error variants from tuple String to struct with message and source fields using BoxedStepError (Box) - Add module-level documentation explaining the boxed error design decision - Replace generic Deployment/DeploymentFailed variants with specific: TrackerConfigDeployment, GrafanaProvisioningDeployment, PrometheusConfigDeployment, ComposeFilesDeployment - Update Traceable impl, error_kind(), and help() for new variants - Update handler.rs to construct errors with both message and source - Update tests to use struct variant syntax with helper function Trade-off: Using Box loses pattern matching on source type, but enables uniform error handling for 15+ heterogeneous step error types and preserves error messages for debugging. --- .../command_handlers/release/errors.rs | 388 ++++++++++++++---- .../command_handlers/release/handler.rs | 71 +++- 2 files changed, 362 insertions(+), 97 deletions(-) diff --git a/src/application/command_handlers/release/errors.rs b/src/application/command_handlers/release/errors.rs index c39719ff..14381898 100644 --- a/src/application/command_handlers/release/errors.rs +++ b/src/application/command_handlers/release/errors.rs @@ -1,9 +1,35 @@ //! Error types for the Release command handler +//! +//! # Design Decision: Boxed Error Sources +//! +//! All step-related error variants use `Box` +//! as the source type rather than concrete step error types. This design choice +//! was made because: +//! +//! 1. **Many heterogeneous step types**: The release workflow involves 15+ steps, +//! each with its own error type (`CommandError`, `TrackerProjectGeneratorError`, +//! `DeployComposeFilesStepError`, etc.). Using concrete types would require +//! either a massive wrapper enum or many separate error variants. +//! +//! 2. **Extensibility**: New steps can be added without modifying the error enum. +//! +//! 3. **Uniform error handling**: All step errors can be handled uniformly with +//! the `with_step` helper pattern. +//! +//! **Trade-off**: We lose the ability to pattern match on the concrete source type +//! and `Traceable::trace_source()` returns `None` for boxed errors. However, the +//! error message is preserved via `to_string()`, and the trace file captures full +//! context for debugging. +//! +//! **Preferred pattern**: In cases where there are fewer, well-defined error sources, +//! prefer using concrete types with `#[source]` for better type safety and traceability. -use crate::application::steps::application::DeployComposeFilesStepError; use crate::domain::environment::state::StateTypeError; use crate::shared::error::{ErrorKind, Traceable}; +/// Type alias for boxed step errors to reduce verbosity +pub type BoxedStepError = Box; + /// Comprehensive error type for the `ReleaseCommandHandler` /// /// This error type captures all possible failures that can occur during @@ -37,51 +63,113 @@ pub enum ReleaseCommandHandlerError { StatePersistence(#[from] crate::domain::environment::repository::RepositoryError), /// Template rendering failed - #[error("Template rendering failed: {0}")] - TemplateRendering(String), + #[error("Template rendering failed: {message}")] + TemplateRendering { + /// Description of the rendering failure + message: String, + /// The underlying error from the template step + #[source] + source: BoxedStepError, + }, /// Tracker storage directory creation failed - #[error("Tracker storage creation failed: {0}")] - TrackerStorageCreation(String), + #[error("Tracker storage creation failed: {message}")] + TrackerStorageCreation { + /// Description of the failure + message: String, + /// The underlying error from the storage creation step + #[source] + source: BoxedStepError, + }, /// Tracker database initialization failed - #[error("Tracker database initialization failed: {0}")] - TrackerDatabaseInit(String), + #[error("Tracker database initialization failed: {message}")] + TrackerDatabaseInit { + /// Description of the failure + message: String, + /// The underlying error from the database init step + #[source] + source: BoxedStepError, + }, /// Prometheus storage directory creation failed - #[error("Prometheus storage creation failed: {0}")] - PrometheusStorageCreation(String), + #[error("Prometheus storage creation failed: {message}")] + PrometheusStorageCreation { + /// Description of the failure + message: String, + /// The underlying error from the storage creation step + #[source] + source: BoxedStepError, + }, /// Grafana storage directory creation failed - #[error("Grafana storage creation failed: {0}")] - GrafanaStorageCreation(String), + #[error("Grafana storage creation failed: {message}")] + GrafanaStorageCreation { + /// Description of the failure + message: String, + /// The underlying error from the storage creation step + #[source] + source: BoxedStepError, + }, /// `MySQL` storage directory creation failed - #[error("MySQL storage creation failed: {0}")] - MysqlStorageCreation(String), + #[error("MySQL storage creation failed: {message}")] + MysqlStorageCreation { + /// Description of the failure + message: String, + /// The underlying error from the storage creation step + #[source] + source: BoxedStepError, + }, /// Caddy configuration deployment failed - #[error("Caddy configuration deployment failed: {0}")] - CaddyConfigDeployment(String), + #[error("Caddy configuration deployment failed: {message}")] + CaddyConfigDeployment { + /// Description of the failure + message: String, + /// The underlying error from the deployment step + #[source] + source: BoxedStepError, + }, + + /// Tracker configuration deployment failed + #[error("Tracker configuration deployment failed: {message}")] + TrackerConfigDeployment { + /// Description of the failure + message: String, + /// The underlying error from the deployment step + #[source] + source: BoxedStepError, + }, - /// General deployment operation failed - #[error("Deployment failed: {message}")] - Deployment { - /// The error message + /// Grafana provisioning deployment failed + #[error("Grafana provisioning deployment failed: {message}")] + GrafanaProvisioningDeployment { + /// Description of the failure message: String, - /// The underlying error source + /// The underlying error from the deployment step #[source] - source: Box, + source: BoxedStepError, }, - /// Deployment to remote host failed - #[error("Deployment to remote host failed: {message}")] - DeploymentFailed { + /// Prometheus configuration deployment failed + #[error("Prometheus configuration deployment failed: {message}")] + PrometheusConfigDeployment { /// Description of the failure message: String, - /// The underlying deployment step error + /// The underlying error from the deployment step #[source] - source: DeployComposeFilesStepError, + source: BoxedStepError, + }, + + /// Docker Compose files deployment failed + #[error("Docker Compose deployment failed: {message}")] + ComposeFilesDeployment { + /// Description of the failure + message: String, + /// The underlying error from the deployment step + #[source] + source: BoxedStepError, }, /// Release operation failed @@ -109,33 +197,48 @@ impl Traceable for ReleaseCommandHandlerError { Self::StatePersistence(e) => { format!("ReleaseCommandHandlerError: Failed to persist environment state - {e}") } - Self::TemplateRendering(message) => { + Self::TemplateRendering { message, .. } => { format!("ReleaseCommandHandlerError: Template rendering failed - {message}") } - Self::TrackerStorageCreation(message) => { + Self::TrackerStorageCreation { message, .. } => { format!("ReleaseCommandHandlerError: Tracker storage creation failed - {message}") } - Self::TrackerDatabaseInit(message) => { + Self::TrackerDatabaseInit { message, .. } => { format!("ReleaseCommandHandlerError: Tracker database initialization failed - {message}") } - Self::PrometheusStorageCreation(message) => { + Self::PrometheusStorageCreation { message, .. } => { format!( "ReleaseCommandHandlerError: Prometheus storage creation failed - {message}" ) } - Self::GrafanaStorageCreation(message) => { + Self::GrafanaStorageCreation { message, .. } => { format!("ReleaseCommandHandlerError: Grafana storage creation failed - {message}") } - Self::MysqlStorageCreation(message) => { + Self::MysqlStorageCreation { message, .. } => { format!("ReleaseCommandHandlerError: MySQL storage creation failed - {message}") } - Self::CaddyConfigDeployment(message) => { + Self::CaddyConfigDeployment { message, .. } => { format!( "ReleaseCommandHandlerError: Caddy configuration deployment failed - {message}" ) } - Self::Deployment { message, .. } | Self::DeploymentFailed { message, .. } => { - format!("ReleaseCommandHandlerError: Deployment failed - {message}") + Self::TrackerConfigDeployment { message, .. } => { + format!( + "ReleaseCommandHandlerError: Tracker configuration deployment failed - {message}" + ) + } + Self::GrafanaProvisioningDeployment { message, .. } => { + format!( + "ReleaseCommandHandlerError: Grafana provisioning deployment failed - {message}" + ) + } + Self::PrometheusConfigDeployment { message, .. } => { + format!( + "ReleaseCommandHandlerError: Prometheus configuration deployment failed - {message}" + ) + } + Self::ComposeFilesDeployment { message, .. } => { + format!("ReleaseCommandHandlerError: Docker Compose deployment failed - {message}") } Self::ReleaseOperationFailed { name, message } => { format!( @@ -146,21 +249,25 @@ impl Traceable for ReleaseCommandHandlerError { } fn trace_source(&self) -> Option<&dyn Traceable> { + // Box doesn't implement Traceable, so we return None for all + // step-related errors. The error message is preserved via `to_string()` + // and the trace file captures full context for debugging. match self { - // Box doesn't implement Traceable - Self::DeploymentFailed { source, .. } => Some(source), - Self::Deployment { .. } - | Self::StatePersistence(_) - | Self::EnvironmentNotFound { .. } + Self::EnvironmentNotFound { .. } | Self::MissingInstanceIp { .. } | Self::InvalidState(_) - | Self::TemplateRendering(_) - | Self::TrackerStorageCreation(_) - | Self::TrackerDatabaseInit(_) - | Self::PrometheusStorageCreation(_) - | Self::GrafanaStorageCreation(_) - | Self::MysqlStorageCreation(_) - | Self::CaddyConfigDeployment(_) + | Self::StatePersistence(_) + | Self::TemplateRendering { .. } + | Self::TrackerStorageCreation { .. } + | Self::TrackerDatabaseInit { .. } + | Self::PrometheusStorageCreation { .. } + | Self::GrafanaStorageCreation { .. } + | Self::MysqlStorageCreation { .. } + | Self::CaddyConfigDeployment { .. } + | Self::TrackerConfigDeployment { .. } + | Self::GrafanaProvisioningDeployment { .. } + | Self::PrometheusConfigDeployment { .. } + | Self::ComposeFilesDeployment { .. } | Self::ReleaseOperationFailed { .. } => None, } } @@ -171,17 +278,18 @@ impl Traceable for ReleaseCommandHandlerError { | Self::MissingInstanceIp { .. } | Self::InvalidState(_) => ErrorKind::Configuration, Self::StatePersistence(_) => ErrorKind::StatePersistence, - Self::TemplateRendering(_) - | Self::TrackerStorageCreation(_) - | Self::TrackerDatabaseInit(_) - | Self::PrometheusStorageCreation(_) - | Self::GrafanaStorageCreation(_) - | Self::MysqlStorageCreation(_) - | Self::CaddyConfigDeployment(_) => ErrorKind::TemplateRendering, - Self::Deployment { .. } | Self::ReleaseOperationFailed { .. } => { - ErrorKind::InfrastructureOperation - } - Self::DeploymentFailed { source, .. } => source.error_kind(), + Self::TemplateRendering { .. } => ErrorKind::TemplateRendering, + Self::TrackerStorageCreation { .. } + | Self::TrackerDatabaseInit { .. } + | Self::PrometheusStorageCreation { .. } + | Self::GrafanaStorageCreation { .. } + | Self::MysqlStorageCreation { .. } + | Self::CaddyConfigDeployment { .. } + | Self::TrackerConfigDeployment { .. } + | Self::GrafanaProvisioningDeployment { .. } + | Self::PrometheusConfigDeployment { .. } + | Self::ComposeFilesDeployment { .. } + | Self::ReleaseOperationFailed { .. } => ErrorKind::InfrastructureOperation, } } } @@ -285,7 +393,7 @@ State files are stored in: data// If the problem persists, report it with full system details." } - Self::TemplateRendering(_) => { + Self::TemplateRendering { .. } => { "Template Rendering Failed - Troubleshooting: 1. Check that template files exist in the templates directory @@ -301,7 +409,7 @@ Common causes: For more information, see docs/user-guide/commands.md" } - Self::TrackerStorageCreation(_) => { + Self::TrackerStorageCreation { .. } => { "Tracker Storage Creation Failed - Troubleshooting: 1. Verify the target instance is reachable: @@ -325,7 +433,7 @@ Common causes: For more information, see docs/user-guide/commands.md" } - Self::TrackerDatabaseInit(_) => { + Self::TrackerDatabaseInit { .. } => { "Tracker Database Initialization Failed - Troubleshooting: 1. Verify the tracker storage directories were created: @@ -350,7 +458,7 @@ Common causes: For more information, see docs/user-guide/commands.md" } - Self::PrometheusStorageCreation(_) => { + Self::PrometheusStorageCreation { .. } => { "Prometheus Storage Creation Failed - Troubleshooting: 1. Verify the target instance is reachable: @@ -374,7 +482,7 @@ Common causes: For more information, see docs/user-guide/commands.md" } - Self::GrafanaStorageCreation(_) => { + Self::GrafanaStorageCreation { .. } => { "Grafana Storage Creation Failed - Troubleshooting: 1. Verify the target instance is reachable: @@ -398,7 +506,7 @@ Common causes: For more information, see docs/user-guide/commands.md" } - Self::MysqlStorageCreation(_) => { + Self::MysqlStorageCreation { .. } => { "MySQL Storage Creation Failed - Troubleshooting: 1. Verify the target instance is reachable: @@ -422,7 +530,7 @@ Common causes: For more information, see docs/user-guide/commands.md" } - Self::CaddyConfigDeployment(_) => { + Self::CaddyConfigDeployment { .. } => { "Caddy Configuration Deployment Failed - Troubleshooting: 1. Verify the target instance is reachable: @@ -448,10 +556,89 @@ Common causes: For more information, see docs/user-guide/commands.md" } - Self::Deployment { .. } => { - "Deployment Failed - Troubleshooting: + Self::TrackerConfigDeployment { .. } => { + "Tracker Configuration Deployment Failed - Troubleshooting: + +1. Verify the target instance is reachable: + ssh @ + +2. Check that the tracker configuration was generated in the build directory: + ls build//tracker/ + +3. Verify the Ansible playbook exists: + ls templates/ansible/deploy-tracker-config.yml + +4. Check that the instance has sufficient disk space: + df -h + +5. Review the error message above for specific details + +Common causes: +- Configuration files not generated +- 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::GrafanaProvisioningDeployment { .. } => { + "Grafana Provisioning Deployment Failed - Troubleshooting: + +1. Verify the target instance is reachable: + ssh @ + +2. Check that the Grafana provisioning files were generated: + ls build//grafana/ + +3. Verify the Ansible playbook exists: + ls templates/ansible/deploy-grafana-provisioning.yml + +4. Check that the instance has sufficient disk space: + df -h + +5. Review the error message above for specific details + +Common causes: +- Provisioning files not generated +- 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::PrometheusConfigDeployment { .. } => { + "Prometheus Configuration Deployment Failed - Troubleshooting: -1. Verify the build directory exists and contains expected files +1. Verify the target instance is reachable: + ssh @ + +2. Check that the Prometheus configuration was generated: + ls build//prometheus/ + +3. Verify the Ansible playbook exists: + ls templates/ansible/deploy-prometheus-config.yml + +4. Check that the instance has sufficient disk space: + df -h + +5. Review the error message above for specific details + +Common causes: +- Configuration files not generated +- 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::ComposeFilesDeployment { .. } => { + "Docker Compose Deployment Failed - Troubleshooting: + +1. Verify the build directory exists and contains expected files: + ls build//docker-compose/ 2. Check that the target instance is reachable: ssh @ @@ -471,7 +658,6 @@ Common causes: For more information, see docs/user-guide/commands.md" } - Self::DeploymentFailed { source, .. } => source.help(), Self::ReleaseOperationFailed { .. } => { "Release Operation Failed - Troubleshooting: @@ -503,6 +689,12 @@ mod tests { use super::*; use crate::domain::environment::repository::RepositoryError; use crate::domain::environment::state::StateTypeError; + use std::io; + + /// Helper function to create a boxed error for testing + fn make_boxed_error(msg: &str) -> BoxedStepError { + Box::new(io::Error::other(msg)) + } #[test] fn it_should_provide_help_for_environment_not_found() { @@ -538,7 +730,10 @@ mod tests { #[test] fn it_should_provide_help_for_template_rendering() { - let error = ReleaseCommandHandlerError::TemplateRendering("Test error".to_string()); + let error = ReleaseCommandHandlerError::TemplateRendering { + message: "Test error".to_string(), + source: make_boxed_error("underlying error"), + }; let help = error.help(); assert!(help.contains("Template Rendering")); @@ -582,18 +777,49 @@ mod tests { actual: "created".to_string(), }), 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 { + ReleaseCommandHandlerError::TemplateRendering { + message: "test".to_string(), + source: make_boxed_error("test"), + }, + ReleaseCommandHandlerError::TrackerStorageCreation { + message: "test".to_string(), + source: make_boxed_error("test"), + }, + ReleaseCommandHandlerError::TrackerDatabaseInit { + message: "test".to_string(), + source: make_boxed_error("test"), + }, + ReleaseCommandHandlerError::PrometheusStorageCreation { + message: "test".to_string(), + source: make_boxed_error("test"), + }, + ReleaseCommandHandlerError::GrafanaStorageCreation { + message: "test".to_string(), + source: make_boxed_error("test"), + }, + ReleaseCommandHandlerError::MysqlStorageCreation { + message: "test".to_string(), + source: make_boxed_error("test"), + }, + ReleaseCommandHandlerError::CaddyConfigDeployment { + message: "test".to_string(), + source: make_boxed_error("test"), + }, + ReleaseCommandHandlerError::TrackerConfigDeployment { + message: "test".to_string(), + source: make_boxed_error("test"), + }, + ReleaseCommandHandlerError::GrafanaProvisioningDeployment { + message: "test".to_string(), + source: make_boxed_error("test"), + }, + ReleaseCommandHandlerError::PrometheusConfigDeployment { + message: "test".to_string(), + source: make_boxed_error("test"), + }, + ReleaseCommandHandlerError::ComposeFilesDeployment { message: "test".to_string(), - source: DeployComposeFilesStepError::ComposeBuildDirNotFound { - path: "/tmp/test".to_string(), - }, + source: make_boxed_error("test"), }, ReleaseCommandHandlerError::ReleaseOperationFailed { name: "test".to_string(), diff --git a/src/application/command_handlers/release/handler.rs b/src/application/command_handlers/release/handler.rs index 0421cf3a..383db908 100644 --- a/src/application/command_handlers/release/handler.rs +++ b/src/application/command_handlers/release/handler.rs @@ -372,7 +372,10 @@ impl ReleaseCommandHandler { .execute() .map_err(|e| { ( - ReleaseCommandHandlerError::TrackerStorageCreation(e.to_string()), + ReleaseCommandHandlerError::TrackerStorageCreation { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -401,7 +404,10 @@ impl ReleaseCommandHandler { .execute() .map_err(|e| { ( - ReleaseCommandHandlerError::TrackerDatabaseInit(e.to_string()), + ReleaseCommandHandlerError::TrackerDatabaseInit { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -435,7 +441,10 @@ impl ReleaseCommandHandler { let tracker_build_dir = step.execute().map_err(|e| { ( - ReleaseCommandHandlerError::TemplateRendering(e.to_string()), + ReleaseCommandHandlerError::TemplateRendering { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -483,7 +492,10 @@ impl ReleaseCommandHandler { step.execute().map_err(|e| { ( - ReleaseCommandHandlerError::TemplateRendering(e.to_string()), + ReleaseCommandHandlerError::TemplateRendering { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -526,7 +538,10 @@ impl ReleaseCommandHandler { .execute() .map_err(|e| { ( - ReleaseCommandHandlerError::PrometheusStorageCreation(e.to_string()), + ReleaseCommandHandlerError::PrometheusStorageCreation { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -569,7 +584,10 @@ impl ReleaseCommandHandler { .execute() .map_err(|e| { ( - ReleaseCommandHandlerError::GrafanaStorageCreation(e.to_string()), + ReleaseCommandHandlerError::GrafanaStorageCreation { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -612,7 +630,10 @@ impl ReleaseCommandHandler { .execute() .map_err(|e| { ( - ReleaseCommandHandlerError::MysqlStorageCreation(e.to_string()), + ReleaseCommandHandlerError::MysqlStorageCreation { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -659,7 +680,10 @@ impl ReleaseCommandHandler { .execute() .map_err(|e| { ( - ReleaseCommandHandlerError::TemplateRendering(e.to_string()), + ReleaseCommandHandlerError::PrometheusConfigDeployment { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -718,7 +742,10 @@ impl ReleaseCommandHandler { step.execute().map_err(|e| { ( - ReleaseCommandHandlerError::TemplateRendering(e.to_string()), + ReleaseCommandHandlerError::TemplateRendering { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -766,7 +793,10 @@ impl ReleaseCommandHandler { step.execute().map_err(|e| { ( - ReleaseCommandHandlerError::TemplateRendering(e.to_string()), + ReleaseCommandHandlerError::TemplateRendering { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -809,7 +839,10 @@ impl ReleaseCommandHandler { .execute() .map_err(|e| { ( - ReleaseCommandHandlerError::CaddyConfigDeployment(e.to_string()), + ReleaseCommandHandlerError::CaddyConfigDeployment { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -863,7 +896,10 @@ impl ReleaseCommandHandler { .execute() .map_err(|e| { ( - ReleaseCommandHandlerError::TemplateRendering(e.to_string()), + ReleaseCommandHandlerError::GrafanaProvisioningDeployment { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -901,7 +937,7 @@ impl ReleaseCommandHandler { .execute() .map_err(|e| { ( - ReleaseCommandHandlerError::Deployment { + ReleaseCommandHandlerError::TrackerConfigDeployment { message: e.to_string(), source: Box::new(e), }, @@ -937,7 +973,10 @@ impl ReleaseCommandHandler { let compose_build_dir = step.execute().await.map_err(|e| { ( - ReleaseCommandHandlerError::TemplateRendering(e.to_string()), + ReleaseCommandHandlerError::TemplateRendering { + message: e.to_string(), + source: Box::new(e), + }, current_step, ) })?; @@ -975,9 +1014,9 @@ impl ReleaseCommandHandler { step.execute().map_err(|e| { ( - ReleaseCommandHandlerError::DeploymentFailed { + ReleaseCommandHandlerError::ComposeFilesDeployment { message: e.to_string(), - source: e, + source: Box::new(e), }, current_step, ) From c03113865688028e93a93348c0644fa57a63a8d4 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 26 Jan 2026 18:59:28 +0000 Subject: [PATCH 07/12] refactor: extract release steps into service-specific modules Reorganized the release command handler by extracting step implementations into a dedicated steps/ subdirectory organized by service: - steps/tracker.rs: tracker service release steps - steps/prometheus.rs: prometheus service steps (optional) - steps/grafana.rs: grafana service steps (optional) - steps/mysql.rs: mysql service steps (optional) - steps/caddy.rs: caddy service steps (HTTPS) - steps/compose.rs: docker compose deployment (async) - steps/common.rs: shared utilities (ansible_client helper) Each module only exposes its release() function publicly, with individual step implementations remaining private. This improves code organization and encapsulation while reducing handler.rs from 1114 to 291 lines (~74%). --- .../command_handlers/release/handler.rs | 838 +----------------- .../command_handlers/release/mod.rs | 7 + .../command_handlers/release/steps/caddy.rs | 137 +++ .../command_handlers/release/steps/common.rs | 14 + .../command_handlers/release/steps/compose.rs | 115 +++ .../command_handlers/release/steps/grafana.rs | 210 +++++ .../command_handlers/release/steps/mod.rs | 13 + .../command_handlers/release/steps/mysql.rs | 79 ++ .../release/steps/prometheus.rs | 188 ++++ .../command_handlers/release/steps/tracker.rs | 184 ++++ 10 files changed, 955 insertions(+), 830 deletions(-) create mode 100644 src/application/command_handlers/release/steps/caddy.rs create mode 100644 src/application/command_handlers/release/steps/common.rs create mode 100644 src/application/command_handlers/release/steps/compose.rs create mode 100644 src/application/command_handlers/release/steps/grafana.rs create mode 100644 src/application/command_handlers/release/steps/mod.rs create mode 100644 src/application/command_handlers/release/steps/mysql.rs create mode 100644 src/application/command_handlers/release/steps/prometheus.rs create mode 100644 src/application/command_handlers/release/steps/tracker.rs diff --git a/src/application/command_handlers/release/handler.rs b/src/application/command_handlers/release/handler.rs index 383db908..08a9a408 100644 --- a/src/application/command_handlers/release/handler.rs +++ b/src/application/command_handlers/release/handler.rs @@ -1,30 +1,16 @@ //! Release command handler implementation use std::net::IpAddr; -use std::path::{Path, PathBuf}; use std::sync::Arc; use tracing::{error, info, instrument}; use super::errors::ReleaseCommandHandlerError; -use crate::adapters::ansible::AnsibleClient; +use super::steps; use crate::application::command_handlers::common::StepResult; -use crate::application::steps::{ - application::{ - CreateGrafanaStorageStep, CreateMysqlStorageStep, CreatePrometheusStorageStep, - CreateTrackerStorageStep, DeployCaddyConfigStep, DeployGrafanaProvisioningStep, - DeployPrometheusConfigStep, DeployTrackerConfigStep, InitTrackerDatabaseStep, - }, - rendering::{ - RenderCaddyTemplatesStep, RenderGrafanaTemplatesStep, RenderPrometheusTemplatesStep, - RenderTrackerTemplatesStep, - }, - DeployComposeFilesStep, RenderDockerComposeTemplatesStep, -}; use crate::domain::environment::repository::{EnvironmentRepository, TypedEnvironmentRepository}; use crate::domain::environment::state::{ReleaseFailureContext, ReleaseStep}; use crate::domain::environment::{Configured, Environment, Released, Releasing}; -use crate::domain::template::TemplateManager; use crate::domain::EnvironmentName; use crate::shared::error::Traceable; @@ -198,22 +184,22 @@ impl ReleaseCommandHandler { instance_ip: IpAddr, ) -> StepResult, ReleaseCommandHandlerError, ReleaseStep> { // Tracker service steps - Self::release_tracker_service(environment)?; + steps::tracker::release(environment)?; // Prometheus service steps (if enabled) - Self::release_prometheus_service(environment)?; + steps::prometheus::release(environment)?; // Grafana service steps (if enabled) - Self::release_grafana_service(environment)?; + steps::grafana::release(environment)?; // MySQL service steps (if enabled) - Self::release_mysql_service(environment)?; + steps::mysql::release(environment)?; // Caddy service steps (if HTTPS enabled) - Self::release_caddy_service(environment)?; + steps::caddy::release(environment)?; // Docker Compose deployment - Self::release_docker_compose(environment, instance_ip).await?; + steps::compose::release(environment, instance_ip).await?; let released = environment.clone().released(); @@ -221,817 +207,9 @@ impl ReleaseCommandHandler { } // ========================================================================= - // Service-level release orchestration + // Helper methods // ========================================================================= - /// Release the Tracker service - /// - /// Executes all steps required to release the Tracker: - /// 1. Create storage directories - /// 2. Initialize database - /// 3. Render configuration templates - /// 4. Deploy configuration to remote - /// - /// # Errors - /// - /// Returns a tuple of (error, step) if any tracker step fails - #[allow(clippy::result_large_err)] - fn release_tracker_service( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - Self::create_tracker_storage(environment)?; - Self::init_tracker_database(environment)?; - let tracker_build_dir = Self::render_tracker_templates(environment)?; - Self::deploy_tracker_config_to_remote(environment, &tracker_build_dir)?; - Ok(()) - } - - /// Release the Prometheus service (if enabled) - /// - /// Executes all steps required to release Prometheus: - /// 1. Create storage directories - /// 2. Render configuration templates - /// 3. Deploy configuration to remote - /// - /// If Prometheus is not configured, all steps are skipped. - /// - /// # Errors - /// - /// Returns a tuple of (error, step) if any Prometheus step fails - #[allow(clippy::result_large_err)] - fn release_prometheus_service( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - Self::create_prometheus_storage(environment)?; - Self::render_prometheus_templates(environment)?; - Self::deploy_prometheus_config_to_remote(environment)?; - Ok(()) - } - - /// Release the Grafana service (if enabled) - /// - /// Executes all steps required to release Grafana: - /// 1. Create storage directories - /// 2. Render provisioning templates - /// 3. Deploy provisioning to remote - /// - /// If Grafana is not configured, all steps are skipped. - /// - /// # Errors - /// - /// Returns a tuple of (error, step) if any Grafana step fails - #[allow(clippy::result_large_err)] - fn release_grafana_service( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - Self::create_grafana_storage(environment)?; - Self::render_grafana_templates(environment)?; - Self::deploy_grafana_provisioning_to_remote(environment)?; - Ok(()) - } - - /// Release the `MySQL` service (if enabled) - /// - /// Executes all steps required to release `MySQL`: - /// 1. Create storage directories - /// - /// If `MySQL` is not configured as the tracker database, this step is skipped. - /// - /// # Errors - /// - /// Returns a tuple of (error, step) if `MySQL` storage creation fails - #[allow(clippy::result_large_err)] - fn release_mysql_service( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - Self::create_mysql_storage(environment)?; - Ok(()) - } - - /// Release the Caddy service (if HTTPS enabled) - /// - /// Executes all steps required to release Caddy: - /// 1. Render configuration templates - /// 2. Deploy configuration to remote - /// - /// If HTTPS is not configured, all steps are skipped. - /// - /// # Errors - /// - /// Returns a tuple of (error, step) if any Caddy step fails - #[allow(clippy::result_large_err)] - fn release_caddy_service( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - Self::render_caddy_templates(environment)?; - Self::deploy_caddy_config_to_remote(environment)?; - Ok(()) - } - - /// Release Docker Compose configuration - /// - /// Executes all steps required to deploy Docker Compose: - /// 1. Render Docker Compose templates - /// 2. Deploy compose files to remote - /// - /// # Errors - /// - /// Returns a tuple of (error, step) if any Docker Compose step fails - async fn release_docker_compose( - environment: &Environment, - instance_ip: IpAddr, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let compose_build_dir = Self::render_docker_compose_templates(environment).await?; - Self::deploy_compose_files_to_remote(environment, &compose_build_dir, instance_ip)?; - Ok(()) - } - - // ========================================================================= - // Individual step implementations - // ========================================================================= - - /// Create an Ansible client configured for the environment's build directory - /// - /// This is a helper method to reduce duplication across step implementations. - fn ansible_client(environment: &Environment) -> Arc { - Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))) - } - - /// Create tracker storage directories on the remote host - /// - /// # Errors - /// - /// Returns a tuple of (error, `ReleaseStep::CreateTrackerStorage`) if creation fails - #[allow(clippy::result_large_err)] - fn create_tracker_storage( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let current_step = ReleaseStep::CreateTrackerStorage; - - CreateTrackerStorageStep::new(Self::ansible_client(environment)) - .execute() - .map_err(|e| { - ( - ReleaseCommandHandlerError::TrackerStorageCreation { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - step = %current_step, - "Tracker storage directories created successfully" - ); - - Ok(()) - } - - /// Initialize tracker database on the remote host - /// - /// # Errors - /// - /// Returns a tuple of (error, `ReleaseStep::InitTrackerDatabase`) if initialization fails - #[allow(clippy::result_large_err)] - fn init_tracker_database( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let current_step = ReleaseStep::InitTrackerDatabase; - - InitTrackerDatabaseStep::new(Self::ansible_client(environment)) - .execute() - .map_err(|e| { - ( - ReleaseCommandHandlerError::TrackerDatabaseInit { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - step = %current_step, - "Tracker database initialized successfully" - ); - - Ok(()) - } - - /// Render Tracker configuration templates to the build directory - /// - /// # Errors - /// - /// Returns a tuple of (error, `ReleaseStep::RenderTrackerTemplates`) if rendering fails - #[allow(clippy::result_large_err)] - fn render_tracker_templates( - environment: &Environment, - ) -> StepResult { - let current_step = ReleaseStep::RenderTrackerTemplates; - - let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); - let step = RenderTrackerTemplatesStep::new( - Arc::new(environment.clone()), - template_manager, - environment.build_dir().clone(), - ); - - let tracker_build_dir = step.execute().map_err(|e| { - ( - ReleaseCommandHandlerError::TemplateRendering { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - tracker_build_dir = %tracker_build_dir.display(), - "Tracker configuration templates rendered successfully" - ); - - Ok(tracker_build_dir) - } - - /// Render Prometheus configuration templates to the build directory (if enabled) - /// - /// This step is optional and only executes if Prometheus is configured in the environment. - /// If Prometheus is not configured, the step is skipped without error. - /// - /// # Errors - /// - /// Returns a tuple of (error, `ReleaseStep::RenderPrometheusTemplates`) if rendering fails - #[allow(clippy::result_large_err)] - fn render_prometheus_templates( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let current_step = ReleaseStep::RenderPrometheusTemplates; - - // Check if Prometheus is configured - if environment.context().user_inputs.prometheus().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "Prometheus not configured - skipping template rendering" - ); - return Ok(()); - } - - let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); - let step = RenderPrometheusTemplatesStep::new( - Arc::new(environment.clone()), - template_manager, - environment.build_dir().clone(), - ); - - step.execute().map_err(|e| { - ( - ReleaseCommandHandlerError::TemplateRendering { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - step = %current_step, - "Prometheus configuration templates rendered successfully" - ); - - Ok(()) - } - - /// Create Prometheus storage directories on the remote host (if enabled) - /// - /// This step is optional and only executes if Prometheus is configured in the environment. - /// If Prometheus is not configured, the step is skipped without error. - /// - /// # Errors - /// - /// Returns a tuple of (error, `ReleaseStep::CreatePrometheusStorage`) if creation fails - #[allow(clippy::result_large_err)] - fn create_prometheus_storage( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let current_step = ReleaseStep::CreatePrometheusStorage; - - // Check if Prometheus is configured - if environment.context().user_inputs.prometheus().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "Prometheus not configured - skipping storage creation" - ); - return Ok(()); - } - - CreatePrometheusStorageStep::new(Self::ansible_client(environment)) - .execute() - .map_err(|e| { - ( - ReleaseCommandHandlerError::PrometheusStorageCreation { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - step = %current_step, - "Prometheus storage directories created successfully" - ); - - 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, - ) -> 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(()); - } - - CreateGrafanaStorageStep::new(Self::ansible_client(environment)) - .execute() - .map_err(|e| { - ( - ReleaseCommandHandlerError::GrafanaStorageCreation { - message: e.to_string(), - source: Box::new(e), - }, - 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, - ) -> 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(()); - } - - CreateMysqlStorageStep::new(Self::ansible_client(environment)) - .execute() - .map_err(|e| { - ( - ReleaseCommandHandlerError::MysqlStorageCreation { - message: e.to_string(), - source: Box::new(e), - }, - 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. - /// If Prometheus is not configured, the step is skipped without error. - /// - /// # Arguments - /// - /// * `environment` - The environment in Releasing state - /// - /// # Errors - /// - /// Returns a tuple of (error, `ReleaseStep::DeployPrometheusConfigToRemote`) if deployment fails - #[allow(clippy::result_large_err)] - fn deploy_prometheus_config_to_remote( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let current_step = ReleaseStep::DeployPrometheusConfigToRemote; - - // Check if Prometheus is configured - if environment.context().user_inputs.prometheus().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "Prometheus not configured - skipping config deployment" - ); - return Ok(()); - } - - DeployPrometheusConfigStep::new(Self::ansible_client(environment)) - .execute() - .map_err(|e| { - ( - ReleaseCommandHandlerError::PrometheusConfigDeployment { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - step = %current_step, - "Prometheus configuration deployed successfully" - ); - - Ok(()) - } - - /// Render Grafana provisioning templates (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::RenderGrafanaTemplates`) if rendering fails - #[allow(clippy::result_large_err)] - fn render_grafana_templates( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let current_step = ReleaseStep::RenderGrafanaTemplates; - - // 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 provisioning template rendering" - ); - return Ok(()); - } - - // Check if Prometheus is configured (required for datasource) - if environment.context().user_inputs.prometheus().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "Prometheus not configured - skipping Grafana provisioning (datasource requires Prometheus)" - ); - return Ok(()); - } - - let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); - let step = RenderGrafanaTemplatesStep::new( - Arc::new(environment.clone()), - template_manager, - environment.build_dir().clone(), - ); - - step.execute().map_err(|e| { - ( - ReleaseCommandHandlerError::TemplateRendering { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - step = %current_step, - "Grafana provisioning templates rendered successfully" - ); - - Ok(()) - } - - /// Render Caddy configuration templates (if HTTPS enabled) - /// - /// This step is optional and only executes if HTTPS is configured in the environment. - /// If HTTPS is not configured, the step is skipped without error. - /// - /// # Errors - /// - /// Returns a tuple of (error, `ReleaseStep::RenderCaddyTemplates`) if rendering fails - #[allow(clippy::result_large_err)] - fn render_caddy_templates( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let current_step = ReleaseStep::RenderCaddyTemplates; - - // Check if HTTPS is configured - if environment.context().user_inputs.https().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "HTTPS not configured - skipping Caddy template rendering" - ); - return Ok(()); - } - - let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); - let step = RenderCaddyTemplatesStep::new( - Arc::new(environment.clone()), - template_manager, - environment.build_dir().clone(), - ); - - step.execute().map_err(|e| { - ( - ReleaseCommandHandlerError::TemplateRendering { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - step = %current_step, - "Caddy configuration templates rendered successfully" - ); - - Ok(()) - } - - /// Deploy Caddy configuration to the remote host (if HTTPS enabled) - /// - /// This step is optional and only executes if HTTPS is configured in the environment. - /// If HTTPS is not configured, the step is skipped without error. - /// - /// # Errors - /// - /// Returns a tuple of (error, `ReleaseStep::DeployCaddyConfigToRemote`) if deployment fails - #[allow(clippy::result_large_err)] - fn deploy_caddy_config_to_remote( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let current_step = ReleaseStep::DeployCaddyConfigToRemote; - - // Check if HTTPS is configured - if environment.context().user_inputs.https().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "HTTPS not configured - skipping Caddy config deployment" - ); - return Ok(()); - } - - DeployCaddyConfigStep::new(Self::ansible_client(environment)) - .execute() - .map_err(|e| { - ( - ReleaseCommandHandlerError::CaddyConfigDeployment { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - step = %current_step, - "Caddy configuration deployed to remote successfully" - ); - - Ok(()) - } - - /// Deploy Grafana provisioning configuration to 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::DeployGrafanaProvisioning`) if deployment fails - #[allow(clippy::result_large_err)] - fn deploy_grafana_provisioning_to_remote( - environment: &Environment, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let current_step = ReleaseStep::DeployGrafanaProvisioning; - - // 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 provisioning deployment" - ); - return Ok(()); - } - - // Check if Prometheus is configured (required for datasource) - if environment.context().user_inputs.prometheus().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "Prometheus not configured - skipping Grafana provisioning deployment" - ); - return Ok(()); - } - - DeployGrafanaProvisioningStep::new(Self::ansible_client(environment)) - .execute() - .map_err(|e| { - ( - ReleaseCommandHandlerError::GrafanaProvisioningDeployment { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - step = %current_step, - "Grafana provisioning configuration deployed successfully" - ); - - Ok(()) - } - - /// Deploy tracker configuration to the remote host via Ansible - /// - /// # Arguments - /// - /// * `environment` - The environment in Releasing state - /// * `tracker_build_dir` - Path to the rendered tracker configuration - /// - /// # Errors - /// - /// Returns a tuple of (error, `ReleaseStep::DeployTrackerConfigToRemote`) if deployment fails - #[allow(clippy::result_large_err)] - fn deploy_tracker_config_to_remote( - environment: &Environment, - tracker_build_dir: &Path, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let current_step = ReleaseStep::DeployTrackerConfigToRemote; - - DeployTrackerConfigStep::new( - Self::ansible_client(environment), - tracker_build_dir.to_path_buf(), - ) - .execute() - .map_err(|e| { - ( - ReleaseCommandHandlerError::TrackerConfigDeployment { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - step = %current_step, - "Tracker configuration deployed successfully" - ); - - Ok(()) - } - - /// Render Docker Compose templates to the build directory - /// - /// # Errors - /// - /// Returns a tuple of (error, `ReleaseStep::RenderDockerComposeTemplates`) if rendering fails - async fn render_docker_compose_templates( - environment: &Environment, - ) -> StepResult { - let current_step = ReleaseStep::RenderDockerComposeTemplates; - - let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); - let step = RenderDockerComposeTemplatesStep::new( - Arc::new(environment.clone()), - template_manager, - environment.build_dir().clone(), - ); - - let compose_build_dir = step.execute().await.map_err(|e| { - ( - ReleaseCommandHandlerError::TemplateRendering { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - compose_build_dir = %compose_build_dir.display(), - "Docker Compose templates rendered successfully" - ); - - Ok(compose_build_dir) - } - - /// Deploy compose files to the remote host via Ansible - /// - /// # Arguments - /// - /// * `environment` - The environment in Releasing state - /// * `compose_build_dir` - Path to the rendered compose files - /// * `instance_ip` - The target instance IP address - /// - /// # Errors - /// - /// Returns a tuple of (error, `ReleaseStep::DeployComposeFilesToRemote`) if deployment fails - #[allow(clippy::result_large_err)] - fn deploy_compose_files_to_remote( - environment: &Environment, - compose_build_dir: &Path, - instance_ip: IpAddr, - ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { - let current_step = ReleaseStep::DeployComposeFilesToRemote; - - let ansible_client = Arc::new(AnsibleClient::new(environment.ansible_build_dir())); - let step = DeployComposeFilesStep::new(ansible_client, compose_build_dir.to_path_buf()); - - step.execute().map_err(|e| { - ( - ReleaseCommandHandlerError::ComposeFilesDeployment { - message: e.to_string(), - source: Box::new(e), - }, - current_step, - ) - })?; - - info!( - command = "release", - compose_build_dir = %compose_build_dir.display(), - instance_ip = %instance_ip, - "Compose files deployed to remote host successfully" - ); - - Ok(()) - } - /// Build failure context for a release error and generate trace file /// /// This helper method builds structured error context including the failed step, diff --git a/src/application/command_handlers/release/mod.rs b/src/application/command_handlers/release/mod.rs index 2b56b592..80ae6eb9 100644 --- a/src/application/command_handlers/release/mod.rs +++ b/src/application/command_handlers/release/mod.rs @@ -19,6 +19,12 @@ //! - **Explicit State Transitions**: Type-safe state machine for environment lifecycle //! - **Explicit Errors**: All errors implement `.help()` with actionable guidance //! +//! ## Module Organization +//! +//! - `handler.rs` - Core handler with `execute()`, state transitions, workflow orchestration +//! - `errors.rs` - Error types for release operations +//! - `steps/` - Service-specific step implementations (tracker, prometheus, etc.) +//! //! ## Release Workflow //! //! The command handler orchestrates a multi-step workflow: @@ -40,6 +46,7 @@ pub mod errors; pub mod handler; +mod steps; #[cfg(test)] mod tests; diff --git a/src/application/command_handlers/release/steps/caddy.rs b/src/application/command_handlers/release/steps/caddy.rs new file mode 100644 index 00000000..64140d8e --- /dev/null +++ b/src/application/command_handlers/release/steps/caddy.rs @@ -0,0 +1,137 @@ +//! Caddy service release steps +//! +//! This module contains all steps required to release the Caddy service: +//! - Configuration template rendering +//! - Configuration deployment to remote +//! +//! All steps are optional and only execute if HTTPS is configured. + +use std::sync::Arc; + +use tracing::info; + +use super::common::ansible_client; +use crate::application::command_handlers::common::StepResult; +use crate::application::command_handlers::release::errors::ReleaseCommandHandlerError; +use crate::application::steps::application::DeployCaddyConfigStep; +use crate::application::steps::rendering::RenderCaddyTemplatesStep; +use crate::domain::environment::state::ReleaseStep; +use crate::domain::environment::{Environment, Releasing}; +use crate::domain::template::TemplateManager; + +/// Release the Caddy service (if HTTPS enabled) +/// +/// Executes all steps required to release Caddy: +/// 1. Render configuration templates +/// 2. Deploy configuration to remote +/// +/// If HTTPS is not configured, all steps are skipped. +/// +/// # Errors +/// +/// Returns a tuple of (error, step) if any Caddy step fails +#[allow(clippy::result_large_err)] +pub fn release( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + render_templates(environment)?; + deploy_config_to_remote(environment)?; + Ok(()) +} + +/// Render Caddy configuration templates (if HTTPS enabled) +/// +/// This step is optional and only executes if HTTPS is configured in the environment. +/// If HTTPS is not configured, the step is skipped without error. +/// +/// # Errors +/// +/// Returns a tuple of (error, `ReleaseStep::RenderCaddyTemplates`) if rendering fails +#[allow(clippy::result_large_err)] +fn render_templates( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::RenderCaddyTemplates; + + // Check if HTTPS is configured + if environment.context().user_inputs.https().is_none() { + info!( + command = "release", + step = %current_step, + status = "skipped", + "HTTPS not configured - skipping Caddy template rendering" + ); + return Ok(()); + } + + let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); + let step = RenderCaddyTemplatesStep::new( + Arc::new(environment.clone()), + template_manager, + environment.build_dir().clone(), + ); + + step.execute().map_err(|e| { + ( + ReleaseCommandHandlerError::TemplateRendering { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "Caddy configuration templates rendered successfully" + ); + + Ok(()) +} + +/// Deploy Caddy configuration to the remote host (if HTTPS enabled) +/// +/// This step is optional and only executes if HTTPS is configured in the environment. +/// If HTTPS is not configured, the step is skipped without error. +/// +/// # Errors +/// +/// Returns a tuple of (error, `ReleaseStep::DeployCaddyConfigToRemote`) if deployment fails +#[allow(clippy::result_large_err)] +fn deploy_config_to_remote( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::DeployCaddyConfigToRemote; + + // Check if HTTPS is configured + if environment.context().user_inputs.https().is_none() { + info!( + command = "release", + step = %current_step, + status = "skipped", + "HTTPS not configured - skipping Caddy config deployment" + ); + return Ok(()); + } + + DeployCaddyConfigStep::new(ansible_client(environment)) + .execute() + .map_err(|e| { + ( + ReleaseCommandHandlerError::CaddyConfigDeployment { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "Caddy configuration deployed to remote successfully" + ); + + Ok(()) +} diff --git a/src/application/command_handlers/release/steps/common.rs b/src/application/command_handlers/release/steps/common.rs new file mode 100644 index 00000000..3d3eac75 --- /dev/null +++ b/src/application/command_handlers/release/steps/common.rs @@ -0,0 +1,14 @@ +//! Common utilities shared across release steps + +use std::sync::Arc; + +use crate::adapters::ansible::AnsibleClient; +use crate::domain::environment::{Environment, Releasing}; + +/// Create an Ansible client configured for the environment's build directory +/// +/// This is a helper function to reduce duplication across step implementations. +#[must_use] +pub fn ansible_client(environment: &Environment) -> Arc { + Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))) +} diff --git a/src/application/command_handlers/release/steps/compose.rs b/src/application/command_handlers/release/steps/compose.rs new file mode 100644 index 00000000..1cf5b4ac --- /dev/null +++ b/src/application/command_handlers/release/steps/compose.rs @@ -0,0 +1,115 @@ +//! Docker Compose release steps +//! +//! This module contains all steps required to deploy Docker Compose: +//! - Template rendering +//! - Compose files deployment to remote + +use std::net::IpAddr; +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use tracing::info; + +use crate::adapters::ansible::AnsibleClient; +use crate::application::command_handlers::common::StepResult; +use crate::application::command_handlers::release::errors::ReleaseCommandHandlerError; +use crate::application::steps::{DeployComposeFilesStep, RenderDockerComposeTemplatesStep}; +use crate::domain::environment::state::ReleaseStep; +use crate::domain::environment::{Environment, Releasing}; +use crate::domain::template::TemplateManager; + +/// Release Docker Compose configuration +/// +/// Executes all steps required to deploy Docker Compose: +/// 1. Render Docker Compose templates +/// 2. Deploy compose files to remote +/// +/// # Errors +/// +/// Returns a tuple of (error, step) if any Docker Compose step fails +pub async fn release( + environment: &Environment, + instance_ip: IpAddr, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let compose_build_dir = render_templates(environment).await?; + deploy_files_to_remote(environment, &compose_build_dir, instance_ip)?; + Ok(()) +} + +/// Render Docker Compose templates to the build directory +/// +/// # Errors +/// +/// Returns a tuple of (error, `ReleaseStep::RenderDockerComposeTemplates`) if rendering fails +async fn render_templates( + environment: &Environment, +) -> StepResult { + let current_step = ReleaseStep::RenderDockerComposeTemplates; + + let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); + let step = RenderDockerComposeTemplatesStep::new( + Arc::new(environment.clone()), + template_manager, + environment.build_dir().clone(), + ); + + let compose_build_dir = step.execute().await.map_err(|e| { + ( + ReleaseCommandHandlerError::TemplateRendering { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + compose_build_dir = %compose_build_dir.display(), + "Docker Compose templates rendered successfully" + ); + + Ok(compose_build_dir) +} + +/// Deploy compose files to the remote host via Ansible +/// +/// # Arguments +/// +/// * `environment` - The environment in Releasing state +/// * `compose_build_dir` - Path to the rendered compose files +/// * `instance_ip` - The target instance IP address +/// +/// # Errors +/// +/// Returns a tuple of (error, `ReleaseStep::DeployComposeFilesToRemote`) if deployment fails +#[allow(clippy::result_large_err)] +fn deploy_files_to_remote( + environment: &Environment, + compose_build_dir: &Path, + instance_ip: IpAddr, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::DeployComposeFilesToRemote; + + let ansible_client = Arc::new(AnsibleClient::new(environment.ansible_build_dir())); + let step = DeployComposeFilesStep::new(ansible_client, compose_build_dir.to_path_buf()); + + step.execute().map_err(|e| { + ( + ReleaseCommandHandlerError::ComposeFilesDeployment { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + compose_build_dir = %compose_build_dir.display(), + instance_ip = %instance_ip, + "Compose files deployed to remote host successfully" + ); + + Ok(()) +} diff --git a/src/application/command_handlers/release/steps/grafana.rs b/src/application/command_handlers/release/steps/grafana.rs new file mode 100644 index 00000000..ffe9beb8 --- /dev/null +++ b/src/application/command_handlers/release/steps/grafana.rs @@ -0,0 +1,210 @@ +//! Grafana service release steps +//! +//! This module contains all steps required to release the Grafana service: +//! - Storage directory creation +//! - Provisioning template rendering +//! - Provisioning deployment to remote +//! +//! All steps are optional and only execute if Grafana is configured. + +use std::sync::Arc; + +use tracing::info; + +use super::common::ansible_client; +use crate::application::command_handlers::common::StepResult; +use crate::application::command_handlers::release::errors::ReleaseCommandHandlerError; +use crate::application::steps::application::{ + CreateGrafanaStorageStep, DeployGrafanaProvisioningStep, +}; +use crate::application::steps::rendering::RenderGrafanaTemplatesStep; +use crate::domain::environment::state::ReleaseStep; +use crate::domain::environment::{Environment, Releasing}; +use crate::domain::template::TemplateManager; + +/// Release the Grafana service (if enabled) +/// +/// Executes all steps required to release Grafana: +/// 1. Create storage directories +/// 2. Render provisioning templates +/// 3. Deploy provisioning to remote +/// +/// If Grafana is not configured, all steps are skipped. +/// +/// # Errors +/// +/// Returns a tuple of (error, step) if any Grafana step fails +#[allow(clippy::result_large_err)] +pub fn release( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + create_storage(environment)?; + render_templates(environment)?; + deploy_provisioning_to_remote(environment)?; + 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_storage( + environment: &Environment, +) -> 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(()); + } + + CreateGrafanaStorageStep::new(ansible_client(environment)) + .execute() + .map_err(|e| { + ( + ReleaseCommandHandlerError::GrafanaStorageCreation { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "Grafana storage directories created successfully" + ); + + Ok(()) +} + +/// Render Grafana provisioning templates (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::RenderGrafanaTemplates`) if rendering fails +#[allow(clippy::result_large_err)] +fn render_templates( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::RenderGrafanaTemplates; + + // 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 provisioning template rendering" + ); + return Ok(()); + } + + // Check if Prometheus is configured (required for datasource) + if environment.context().user_inputs.prometheus().is_none() { + info!( + command = "release", + step = %current_step, + status = "skipped", + "Prometheus not configured - skipping Grafana provisioning (datasource requires Prometheus)" + ); + return Ok(()); + } + + let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); + let step = RenderGrafanaTemplatesStep::new( + Arc::new(environment.clone()), + template_manager, + environment.build_dir().clone(), + ); + + step.execute().map_err(|e| { + ( + ReleaseCommandHandlerError::TemplateRendering { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "Grafana provisioning templates rendered successfully" + ); + + Ok(()) +} + +/// Deploy Grafana provisioning configuration to 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::DeployGrafanaProvisioning`) if deployment fails +#[allow(clippy::result_large_err)] +fn deploy_provisioning_to_remote( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::DeployGrafanaProvisioning; + + // 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 provisioning deployment" + ); + return Ok(()); + } + + // Check if Prometheus is configured (required for datasource) + if environment.context().user_inputs.prometheus().is_none() { + info!( + command = "release", + step = %current_step, + status = "skipped", + "Prometheus not configured - skipping Grafana provisioning deployment" + ); + return Ok(()); + } + + DeployGrafanaProvisioningStep::new(ansible_client(environment)) + .execute() + .map_err(|e| { + ( + ReleaseCommandHandlerError::GrafanaProvisioningDeployment { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "Grafana provisioning configuration deployed successfully" + ); + + Ok(()) +} diff --git a/src/application/command_handlers/release/steps/mod.rs b/src/application/command_handlers/release/steps/mod.rs new file mode 100644 index 00000000..d9ec95da --- /dev/null +++ b/src/application/command_handlers/release/steps/mod.rs @@ -0,0 +1,13 @@ +//! Release step implementations organized by service +//! +//! This module contains the individual step implementations for the release workflow, +//! organized by the service they operate on. Each submodule provides functions that +//! wrap the underlying step structs with error mapping and logging. + +pub mod caddy; +pub mod common; +pub mod compose; +pub mod grafana; +pub mod mysql; +pub mod prometheus; +pub mod tracker; diff --git a/src/application/command_handlers/release/steps/mysql.rs b/src/application/command_handlers/release/steps/mysql.rs new file mode 100644 index 00000000..2af335e7 --- /dev/null +++ b/src/application/command_handlers/release/steps/mysql.rs @@ -0,0 +1,79 @@ +//! `MySQL` service release steps +//! +//! This module contains all steps required to release the `MySQL` service: +//! - Storage directory creation +//! +//! All steps are optional and only execute if `MySQL` is configured as the tracker database. + +use tracing::info; + +use super::common::ansible_client; +use crate::application::command_handlers::common::StepResult; +use crate::application::command_handlers::release::errors::ReleaseCommandHandlerError; +use crate::application::steps::application::CreateMysqlStorageStep; +use crate::domain::environment::state::ReleaseStep; +use crate::domain::environment::{Environment, Releasing}; + +/// Release the `MySQL` service (if enabled) +/// +/// Executes all steps required to release `MySQL`: +/// 1. Create `MySQL` storage directories +/// +/// If `MySQL` is not configured as the tracker database, this step is skipped. +/// +/// # Errors +/// +/// Returns a tuple of (error, step) if `MySQL` storage creation fails +#[allow(clippy::result_large_err)] +pub fn release( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + create_storage(environment)?; + 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_storage( + environment: &Environment, +) -> 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(()); + } + + CreateMysqlStorageStep::new(ansible_client(environment)) + .execute() + .map_err(|e| { + ( + ReleaseCommandHandlerError::MysqlStorageCreation { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "MySQL storage directories created successfully" + ); + + Ok(()) +} diff --git a/src/application/command_handlers/release/steps/prometheus.rs b/src/application/command_handlers/release/steps/prometheus.rs new file mode 100644 index 00000000..a780b829 --- /dev/null +++ b/src/application/command_handlers/release/steps/prometheus.rs @@ -0,0 +1,188 @@ +//! Prometheus service release steps +//! +//! This module contains all steps required to release the Prometheus service: +//! - Storage directory creation +//! - Configuration template rendering +//! - Configuration deployment to remote +//! +//! All steps are optional and only execute if Prometheus is configured. + +use std::sync::Arc; + +use tracing::info; + +use super::common::ansible_client; +use crate::application::command_handlers::common::StepResult; +use crate::application::command_handlers::release::errors::ReleaseCommandHandlerError; +use crate::application::steps::application::{ + CreatePrometheusStorageStep, DeployPrometheusConfigStep, +}; +use crate::application::steps::rendering::RenderPrometheusTemplatesStep; +use crate::domain::environment::state::ReleaseStep; +use crate::domain::environment::{Environment, Releasing}; +use crate::domain::template::TemplateManager; + +/// Release the Prometheus service (if enabled) +/// +/// Executes all steps required to release Prometheus: +/// 1. Create storage directories +/// 2. Render configuration templates +/// 3. Deploy configuration to remote +/// +/// If Prometheus is not configured, all steps are skipped. +/// +/// # Errors +/// +/// Returns a tuple of (error, step) if any Prometheus step fails +#[allow(clippy::result_large_err)] +pub fn release( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + create_storage(environment)?; + render_templates(environment)?; + deploy_config_to_remote(environment)?; + Ok(()) +} + +/// Create Prometheus storage directories on the remote host (if enabled) +/// +/// This step is optional and only executes if Prometheus is configured in the environment. +/// If Prometheus is not configured, the step is skipped without error. +/// +/// # Errors +/// +/// Returns a tuple of (error, `ReleaseStep::CreatePrometheusStorage`) if creation fails +#[allow(clippy::result_large_err)] +fn create_storage( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::CreatePrometheusStorage; + + // Check if Prometheus is configured + if environment.context().user_inputs.prometheus().is_none() { + info!( + command = "release", + step = %current_step, + status = "skipped", + "Prometheus not configured - skipping storage creation" + ); + return Ok(()); + } + + CreatePrometheusStorageStep::new(ansible_client(environment)) + .execute() + .map_err(|e| { + ( + ReleaseCommandHandlerError::PrometheusStorageCreation { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "Prometheus storage directories created successfully" + ); + + Ok(()) +} + +/// Render Prometheus configuration templates to the build directory (if enabled) +/// +/// This step is optional and only executes if Prometheus is configured in the environment. +/// If Prometheus is not configured, the step is skipped without error. +/// +/// # Errors +/// +/// Returns a tuple of (error, `ReleaseStep::RenderPrometheusTemplates`) if rendering fails +#[allow(clippy::result_large_err)] +fn render_templates( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::RenderPrometheusTemplates; + + // Check if Prometheus is configured + if environment.context().user_inputs.prometheus().is_none() { + info!( + command = "release", + step = %current_step, + status = "skipped", + "Prometheus not configured - skipping template rendering" + ); + return Ok(()); + } + + let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); + let step = RenderPrometheusTemplatesStep::new( + Arc::new(environment.clone()), + template_manager, + environment.build_dir().clone(), + ); + + step.execute().map_err(|e| { + ( + ReleaseCommandHandlerError::TemplateRendering { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "Prometheus configuration templates rendered 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. +/// If Prometheus is not configured, the step is skipped without error. +/// +/// # Errors +/// +/// Returns a tuple of (error, `ReleaseStep::DeployPrometheusConfigToRemote`) if deployment fails +#[allow(clippy::result_large_err)] +fn deploy_config_to_remote( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::DeployPrometheusConfigToRemote; + + // Check if Prometheus is configured + if environment.context().user_inputs.prometheus().is_none() { + info!( + command = "release", + step = %current_step, + status = "skipped", + "Prometheus not configured - skipping config deployment" + ); + return Ok(()); + } + + DeployPrometheusConfigStep::new(ansible_client(environment)) + .execute() + .map_err(|e| { + ( + ReleaseCommandHandlerError::PrometheusConfigDeployment { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "Prometheus configuration deployed successfully" + ); + + Ok(()) +} diff --git a/src/application/command_handlers/release/steps/tracker.rs b/src/application/command_handlers/release/steps/tracker.rs new file mode 100644 index 00000000..4ec54848 --- /dev/null +++ b/src/application/command_handlers/release/steps/tracker.rs @@ -0,0 +1,184 @@ +//! Tracker service release steps +//! +//! This module contains all steps required to release the Tracker service: +//! - Storage directory creation +//! - Database initialization +//! - Configuration template rendering +//! - Configuration deployment to remote + +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use tracing::info; + +use super::common::ansible_client; +use crate::application::command_handlers::common::StepResult; +use crate::application::command_handlers::release::errors::ReleaseCommandHandlerError; +use crate::application::steps::application::{ + CreateTrackerStorageStep, DeployTrackerConfigStep, InitTrackerDatabaseStep, +}; +use crate::application::steps::rendering::RenderTrackerTemplatesStep; +use crate::domain::environment::state::ReleaseStep; +use crate::domain::environment::{Environment, Releasing}; +use crate::domain::template::TemplateManager; + +/// Release the Tracker service +/// +/// Executes all steps required to release the Tracker: +/// 1. Create storage directories +/// 2. Initialize database +/// 3. Render configuration templates +/// 4. Deploy configuration to remote +/// +/// # Errors +/// +/// Returns a tuple of (error, step) if any tracker step fails +#[allow(clippy::result_large_err)] +pub fn release( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + create_storage(environment)?; + init_database(environment)?; + let tracker_build_dir = render_templates(environment)?; + deploy_config_to_remote(environment, &tracker_build_dir)?; + Ok(()) +} + +/// Create tracker storage directories on the remote host +/// +/// # Errors +/// +/// Returns a tuple of (error, `ReleaseStep::CreateTrackerStorage`) if creation fails +#[allow(clippy::result_large_err)] +fn create_storage( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::CreateTrackerStorage; + + CreateTrackerStorageStep::new(ansible_client(environment)) + .execute() + .map_err(|e| { + ( + ReleaseCommandHandlerError::TrackerStorageCreation { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "Tracker storage directories created successfully" + ); + + Ok(()) +} + +/// Initialize tracker database on the remote host +/// +/// # Errors +/// +/// Returns a tuple of (error, `ReleaseStep::InitTrackerDatabase`) if initialization fails +#[allow(clippy::result_large_err)] +fn init_database( + environment: &Environment, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::InitTrackerDatabase; + + InitTrackerDatabaseStep::new(ansible_client(environment)) + .execute() + .map_err(|e| { + ( + ReleaseCommandHandlerError::TrackerDatabaseInit { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "Tracker database initialized successfully" + ); + + Ok(()) +} + +/// Render Tracker configuration templates to the build directory +/// +/// # Errors +/// +/// Returns a tuple of (error, `ReleaseStep::RenderTrackerTemplates`) if rendering fails +#[allow(clippy::result_large_err)] +fn render_templates( + environment: &Environment, +) -> StepResult { + let current_step = ReleaseStep::RenderTrackerTemplates; + + let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); + let step = RenderTrackerTemplatesStep::new( + Arc::new(environment.clone()), + template_manager, + environment.build_dir().clone(), + ); + + let tracker_build_dir = step.execute().map_err(|e| { + ( + ReleaseCommandHandlerError::TemplateRendering { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + tracker_build_dir = %tracker_build_dir.display(), + "Tracker configuration templates rendered successfully" + ); + + Ok(tracker_build_dir) +} + +/// Deploy tracker configuration to the remote host via Ansible +/// +/// # Arguments +/// +/// * `environment` - The environment in Releasing state +/// * `tracker_build_dir` - Path to the rendered tracker configuration +/// +/// # Errors +/// +/// Returns a tuple of (error, `ReleaseStep::DeployTrackerConfigToRemote`) if deployment fails +#[allow(clippy::result_large_err)] +fn deploy_config_to_remote( + environment: &Environment, + tracker_build_dir: &Path, +) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + let current_step = ReleaseStep::DeployTrackerConfigToRemote; + + DeployTrackerConfigStep::new(ansible_client(environment), tracker_build_dir.to_path_buf()) + .execute() + .map_err(|e| { + ( + ReleaseCommandHandlerError::TrackerConfigDeployment { + message: e.to_string(), + source: Box::new(e), + }, + current_step, + ) + })?; + + info!( + command = "release", + step = %current_step, + "Tracker configuration deployed successfully" + ); + + Ok(()) +} From 806ea87ff6a718b113bca170915ce5b51d636933 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 26 Jan 2026 19:09:09 +0000 Subject: [PATCH 08/12] refactor: extract release workflow to dedicated module Moved execute_release_workflow to a new workflow.rs module, separating: - handler.rs: State management, persistence, error handling - workflow.rs: Orchestrates step execution in correct order This makes the workflow a stateless free function that coordinates step calls, while the handler focuses on state transitions and persistence. --- .../command_handlers/release/handler.rs | 59 +----------------- .../command_handlers/release/mod.rs | 2 + .../command_handlers/release/workflow.rs | 61 +++++++++++++++++++ 3 files changed, 65 insertions(+), 57 deletions(-) create mode 100644 src/application/command_handlers/release/workflow.rs diff --git a/src/application/command_handlers/release/handler.rs b/src/application/command_handlers/release/handler.rs index 08a9a408..e9169456 100644 --- a/src/application/command_handlers/release/handler.rs +++ b/src/application/command_handlers/release/handler.rs @@ -1,13 +1,11 @@ //! Release command handler implementation -use std::net::IpAddr; use std::sync::Arc; use tracing::{error, info, instrument}; use super::errors::ReleaseCommandHandlerError; -use super::steps; -use crate::application::command_handlers::common::StepResult; +use super::workflow; use crate::domain::environment::repository::{EnvironmentRepository, TypedEnvironmentRepository}; use crate::domain::environment::state::{ReleaseFailureContext, ReleaseStep}; use crate::domain::environment::{Configured, Environment, Released, Releasing}; @@ -120,10 +118,7 @@ impl ReleaseCommandHandler { "Releasing state persisted. Executing release steps." ); - match self - .execute_release_workflow(&releasing_env, instance_ip) - .await - { + match workflow::execute(&releasing_env, instance_ip).await { Ok(released) => { info!( command = "release", @@ -156,56 +151,6 @@ impl ReleaseCommandHandler { } } - /// Execute the release workflow with step tracking - /// - /// This method orchestrates the complete release workflow, organized by service: - /// - /// 1. **Tracker**: Storage creation, database init, config rendering, deployment - /// 2. **Prometheus**: Storage creation, config rendering, deployment (if enabled) - /// 3. **Grafana**: Storage creation, provisioning rendering, deployment (if enabled) - /// 4. **`MySQL`**: Storage creation (if enabled) - /// 5. **Caddy**: Config rendering, deployment (if HTTPS enabled) - /// 6. **Docker Compose**: Template rendering, deployment - /// - /// If an error occurs, it returns both the error and the step that was being - /// executed, enabling accurate failure context generation. - /// - /// # Arguments - /// - /// * `environment` - The environment in Releasing state - /// * `instance_ip` - The validated instance IP address (used for Docker Compose deployment logging) - /// - /// # Errors - /// - /// Returns a tuple of (error, `current_step`) if any release step fails - async fn execute_release_workflow( - &self, - environment: &Environment, - instance_ip: IpAddr, - ) -> StepResult, ReleaseCommandHandlerError, ReleaseStep> { - // Tracker service steps - steps::tracker::release(environment)?; - - // Prometheus service steps (if enabled) - steps::prometheus::release(environment)?; - - // Grafana service steps (if enabled) - steps::grafana::release(environment)?; - - // MySQL service steps (if enabled) - steps::mysql::release(environment)?; - - // Caddy service steps (if HTTPS enabled) - steps::caddy::release(environment)?; - - // Docker Compose deployment - steps::compose::release(environment, instance_ip).await?; - - let released = environment.clone().released(); - - Ok(released) - } - // ========================================================================= // Helper methods // ========================================================================= diff --git a/src/application/command_handlers/release/mod.rs b/src/application/command_handlers/release/mod.rs index 80ae6eb9..4011d303 100644 --- a/src/application/command_handlers/release/mod.rs +++ b/src/application/command_handlers/release/mod.rs @@ -22,6 +22,7 @@ //! ## Module Organization //! //! - `handler.rs` - Core handler with `execute()`, state transitions, workflow orchestration +//! - `workflow.rs` - Release workflow orchestration (step coordination) //! - `errors.rs` - Error types for release operations //! - `steps/` - Service-specific step implementations (tracker, prometheus, etc.) //! @@ -47,6 +48,7 @@ pub mod errors; pub mod handler; mod steps; +mod workflow; #[cfg(test)] mod tests; diff --git a/src/application/command_handlers/release/workflow.rs b/src/application/command_handlers/release/workflow.rs new file mode 100644 index 00000000..9fbc9cc6 --- /dev/null +++ b/src/application/command_handlers/release/workflow.rs @@ -0,0 +1,61 @@ +//! Release workflow orchestration +//! +//! This module orchestrates the complete release workflow by coordinating +//! all service-specific release steps in the correct order. + +use std::net::IpAddr; + +use super::errors::ReleaseCommandHandlerError; +use super::steps; +use crate::application::command_handlers::common::StepResult; +use crate::domain::environment::state::ReleaseStep; +use crate::domain::environment::{Environment, Released, Releasing}; + +/// Execute the release workflow with step tracking +/// +/// This function orchestrates the complete release workflow, organized by service: +/// +/// 1. **Tracker**: Storage creation, database init, config rendering, deployment +/// 2. **Prometheus**: Storage creation, config rendering, deployment (if enabled) +/// 3. **Grafana**: Storage creation, provisioning rendering, deployment (if enabled) +/// 4. **`MySQL`**: Storage creation (if enabled) +/// 5. **Caddy**: Config rendering, deployment (if HTTPS enabled) +/// 6. **Docker Compose**: Template rendering, deployment +/// +/// If an error occurs, it returns both the error and the step that was being +/// executed, enabling accurate failure context generation. +/// +/// # Arguments +/// +/// * `environment` - The environment in Releasing state +/// * `instance_ip` - The validated instance IP address (used for Docker Compose deployment logging) +/// +/// # Errors +/// +/// Returns a tuple of (error, `current_step`) if any release step fails +pub async fn execute( + environment: &Environment, + instance_ip: IpAddr, +) -> StepResult, ReleaseCommandHandlerError, ReleaseStep> { + // Tracker service steps + steps::tracker::release(environment)?; + + // Prometheus service steps (if enabled) + steps::prometheus::release(environment)?; + + // Grafana service steps (if enabled) + steps::grafana::release(environment)?; + + // MySQL service steps (if enabled) + steps::mysql::release(environment)?; + + // Caddy service steps (if HTTPS enabled) + steps::caddy::release(environment)?; + + // Docker Compose deployment + steps::compose::release(environment, instance_ip).await?; + + let released = environment.clone().released(); + + Ok(released) +} From da242e35a4bf635ef0d658464d487fe068aa0a04 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 26 Jan 2026 19:28:58 +0000 Subject: [PATCH 09/12] refactor: improve workflow module readability - Use explicit imports for step modules (tracker, prometheus, etc.) - Remove redundant inline comments since code is self-explanatory - Simplify doc comment to avoid duplicating individual module docs - Inline return value for conciseness --- .../command_handlers/release/workflow.rs | 52 +++++-------------- 1 file changed, 13 insertions(+), 39 deletions(-) diff --git a/src/application/command_handlers/release/workflow.rs b/src/application/command_handlers/release/workflow.rs index 9fbc9cc6..37849657 100644 --- a/src/application/command_handlers/release/workflow.rs +++ b/src/application/command_handlers/release/workflow.rs @@ -6,29 +6,16 @@ use std::net::IpAddr; use super::errors::ReleaseCommandHandlerError; -use super::steps; +use super::steps::{caddy, compose, grafana, mysql, prometheus, tracker}; use crate::application::command_handlers::common::StepResult; use crate::domain::environment::state::ReleaseStep; use crate::domain::environment::{Environment, Released, Releasing}; -/// Execute the release workflow with step tracking +/// Execute the release workflow /// -/// This function orchestrates the complete release workflow, organized by service: -/// -/// 1. **Tracker**: Storage creation, database init, config rendering, deployment -/// 2. **Prometheus**: Storage creation, config rendering, deployment (if enabled) -/// 3. **Grafana**: Storage creation, provisioning rendering, deployment (if enabled) -/// 4. **`MySQL`**: Storage creation (if enabled) -/// 5. **Caddy**: Config rendering, deployment (if HTTPS enabled) -/// 6. **Docker Compose**: Template rendering, deployment -/// -/// If an error occurs, it returns both the error and the step that was being -/// executed, enabling accurate failure context generation. -/// -/// # Arguments -/// -/// * `environment` - The environment in Releasing state -/// * `instance_ip` - The validated instance IP address (used for Docker Compose deployment logging) +/// Orchestrates the complete release workflow by calling each service's +/// release steps in the correct order. Each service module handles its +/// own conditional logic (e.g., skipping if not enabled). /// /// # Errors /// @@ -37,25 +24,12 @@ pub async fn execute( environment: &Environment, instance_ip: IpAddr, ) -> StepResult, ReleaseCommandHandlerError, ReleaseStep> { - // Tracker service steps - steps::tracker::release(environment)?; - - // Prometheus service steps (if enabled) - steps::prometheus::release(environment)?; - - // Grafana service steps (if enabled) - steps::grafana::release(environment)?; - - // MySQL service steps (if enabled) - steps::mysql::release(environment)?; - - // Caddy service steps (if HTTPS enabled) - steps::caddy::release(environment)?; - - // Docker Compose deployment - steps::compose::release(environment, instance_ip).await?; - - let released = environment.clone().released(); - - Ok(released) + tracker::release(environment)?; + prometheus::release(environment)?; + grafana::release(environment)?; + mysql::release(environment)?; + caddy::release(environment)?; + compose::release(environment, instance_ip).await?; + + Ok(environment.clone().released()) } From fabe39972c9b586988156889890b95db8765e5df Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 26 Jan 2026 19:38:55 +0000 Subject: [PATCH 10/12] refactor: remove instance_ip parameter from release workflow The instance_ip is now obtained from the environment object within each step, rather than being passed as a parameter. This simplifies the function signatures and reduces coupling. - Remove instance_ip param from workflow::execute() - Remove instance_ip param from compose::release() and deploy_files_to_remote() - Update compose logging to use environment.instance_ip() - Keep instance_ip validation in handler for early error detection --- src/application/command_handlers/release/handler.rs | 3 ++- src/application/command_handlers/release/steps/compose.rs | 8 ++------ src/application/command_handlers/release/workflow.rs | 5 +---- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/application/command_handlers/release/handler.rs b/src/application/command_handlers/release/handler.rs index e9169456..9d671c5a 100644 --- a/src/application/command_handlers/release/handler.rs +++ b/src/application/command_handlers/release/handler.rs @@ -90,6 +90,7 @@ impl ReleaseCommandHandler { ) -> Result, ReleaseCommandHandlerError> { let environment = self.load_configured_environment(env_name)?; + // Validate instance IP exists before proceeding (fail early) let instance_ip = environment.instance_ip().ok_or_else(|| { ReleaseCommandHandlerError::MissingInstanceIp { name: env_name.to_string(), @@ -118,7 +119,7 @@ impl ReleaseCommandHandler { "Releasing state persisted. Executing release steps." ); - match workflow::execute(&releasing_env, instance_ip).await { + match workflow::execute(&releasing_env).await { Ok(released) => { info!( command = "release", diff --git a/src/application/command_handlers/release/steps/compose.rs b/src/application/command_handlers/release/steps/compose.rs index 1cf5b4ac..a3dea447 100644 --- a/src/application/command_handlers/release/steps/compose.rs +++ b/src/application/command_handlers/release/steps/compose.rs @@ -4,7 +4,6 @@ //! - Template rendering //! - Compose files deployment to remote -use std::net::IpAddr; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -29,10 +28,9 @@ use crate::domain::template::TemplateManager; /// Returns a tuple of (error, step) if any Docker Compose step fails pub async fn release( environment: &Environment, - instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let compose_build_dir = render_templates(environment).await?; - deploy_files_to_remote(environment, &compose_build_dir, instance_ip)?; + deploy_files_to_remote(environment, &compose_build_dir)?; Ok(()) } @@ -78,7 +76,6 @@ async fn render_templates( /// /// * `environment` - The environment in Releasing state /// * `compose_build_dir` - Path to the rendered compose files -/// * `instance_ip` - The target instance IP address /// /// # Errors /// @@ -87,7 +84,6 @@ async fn render_templates( fn deploy_files_to_remote( environment: &Environment, compose_build_dir: &Path, - instance_ip: IpAddr, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::DeployComposeFilesToRemote; @@ -107,7 +103,7 @@ fn deploy_files_to_remote( info!( command = "release", compose_build_dir = %compose_build_dir.display(), - instance_ip = %instance_ip, + instance_ip = ?environment.instance_ip(), "Compose files deployed to remote host successfully" ); diff --git a/src/application/command_handlers/release/workflow.rs b/src/application/command_handlers/release/workflow.rs index 37849657..9d4c2345 100644 --- a/src/application/command_handlers/release/workflow.rs +++ b/src/application/command_handlers/release/workflow.rs @@ -3,8 +3,6 @@ //! This module orchestrates the complete release workflow by coordinating //! all service-specific release steps in the correct order. -use std::net::IpAddr; - use super::errors::ReleaseCommandHandlerError; use super::steps::{caddy, compose, grafana, mysql, prometheus, tracker}; use crate::application::command_handlers::common::StepResult; @@ -22,14 +20,13 @@ use crate::domain::environment::{Environment, Released, Releasing}; /// Returns a tuple of (error, `current_step`) if any release step fails pub async fn execute( environment: &Environment, - instance_ip: IpAddr, ) -> StepResult, ReleaseCommandHandlerError, ReleaseStep> { tracker::release(environment)?; prometheus::release(environment)?; grafana::release(environment)?; mysql::release(environment)?; caddy::release(environment)?; - compose::release(environment, instance_ip).await?; + compose::release(environment).await?; Ok(environment.clone().released()) } From 629bed5f056c56a2ee9f810d71c61f8848ccf72e Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 26 Jan 2026 20:03:36 +0000 Subject: [PATCH 11/12] refactor: move service availability checks to release() functions Move the conditional checks for optional services (MySQL, Caddy, Prometheus, Grafana) from individual step functions to the main release() function in each module. This eliminates repetitive checks in every step and makes the control flow clearer. - mysql.rs: Check uses_mysql() once in release() - caddy.rs: Check https().is_none() once in release() - prometheus.rs: Check prometheus().is_none() once in release() - grafana.rs: Check grafana().is_none() in release(), with secondary prometheus check after storage creation (since only provisioning requires Prometheus for datasource) --- .../command_handlers/release/steps/caddy.rs | 43 +++----- .../command_handlers/release/steps/grafana.rs | 99 ++++++------------- .../command_handlers/release/steps/mysql.rs | 29 +++--- .../release/steps/prometheus.rs | 59 +++-------- 4 files changed, 70 insertions(+), 160 deletions(-) diff --git a/src/application/command_handlers/release/steps/caddy.rs b/src/application/command_handlers/release/steps/caddy.rs index 64140d8e..e06e86da 100644 --- a/src/application/command_handlers/release/steps/caddy.rs +++ b/src/application/command_handlers/release/steps/caddy.rs @@ -34,15 +34,23 @@ use crate::domain::template::TemplateManager; pub fn release( environment: &Environment, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + // Check if HTTPS is configured + if environment.context().user_inputs.https().is_none() { + info!( + command = "release", + service = "caddy", + status = "skipped", + "HTTPS not configured - skipping all Caddy steps" + ); + return Ok(()); + } + render_templates(environment)?; deploy_config_to_remote(environment)?; Ok(()) } -/// Render Caddy configuration templates (if HTTPS enabled) -/// -/// This step is optional and only executes if HTTPS is configured in the environment. -/// If HTTPS is not configured, the step is skipped without error. +/// Render Caddy configuration templates /// /// # Errors /// @@ -53,17 +61,6 @@ fn render_templates( ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::RenderCaddyTemplates; - // Check if HTTPS is configured - if environment.context().user_inputs.https().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "HTTPS not configured - skipping Caddy template rendering" - ); - return Ok(()); - } - let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); let step = RenderCaddyTemplatesStep::new( Arc::new(environment.clone()), @@ -90,10 +87,7 @@ fn render_templates( Ok(()) } -/// Deploy Caddy configuration to the remote host (if HTTPS enabled) -/// -/// This step is optional and only executes if HTTPS is configured in the environment. -/// If HTTPS is not configured, the step is skipped without error. +/// Deploy Caddy configuration to the remote host /// /// # Errors /// @@ -104,17 +98,6 @@ fn deploy_config_to_remote( ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::DeployCaddyConfigToRemote; - // Check if HTTPS is configured - if environment.context().user_inputs.https().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "HTTPS not configured - skipping Caddy config deployment" - ); - return Ok(()); - } - DeployCaddyConfigStep::new(ansible_client(environment)) .execute() .map_err(|e| { diff --git a/src/application/command_handlers/release/steps/grafana.rs b/src/application/command_handlers/release/steps/grafana.rs index ffe9beb8..38a9ebdd 100644 --- a/src/application/command_handlers/release/steps/grafana.rs +++ b/src/application/command_handlers/release/steps/grafana.rs @@ -6,6 +6,7 @@ //! - Provisioning deployment to remote //! //! All steps are optional and only execute if Grafana is configured. +//! Provisioning steps additionally require Prometheus for datasource configuration. use std::sync::Arc; @@ -26,10 +27,11 @@ use crate::domain::template::TemplateManager; /// /// Executes all steps required to release Grafana: /// 1. Create storage directories -/// 2. Render provisioning templates -/// 3. Deploy provisioning to remote +/// 2. Render provisioning templates (requires Prometheus) +/// 3. Deploy provisioning to remote (requires Prometheus) /// /// If Grafana is not configured, all steps are skipped. +/// Provisioning steps are skipped if Prometheus is not configured. /// /// # Errors /// @@ -38,16 +40,36 @@ use crate::domain::template::TemplateManager; pub fn release( environment: &Environment, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + // Check if Grafana is configured + if environment.context().user_inputs.grafana().is_none() { + info!( + command = "release", + service = "grafana", + status = "skipped", + "Grafana not configured - skipping all Grafana steps" + ); + return Ok(()); + } + create_storage(environment)?; + + // Provisioning requires Prometheus for datasource configuration + if environment.context().user_inputs.prometheus().is_none() { + info!( + command = "release", + service = "grafana", + status = "partial", + "Prometheus not configured - skipping Grafana provisioning (datasource requires Prometheus)" + ); + return Ok(()); + } + render_templates(environment)?; deploy_provisioning_to_remote(environment)?; 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. +/// Create Grafana storage directories on the remote host /// /// # Errors /// @@ -58,17 +80,6 @@ fn create_storage( ) -> 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(()); - } - CreateGrafanaStorageStep::new(ansible_client(environment)) .execute() .map_err(|e| { @@ -90,10 +101,7 @@ fn create_storage( Ok(()) } -/// Render Grafana provisioning templates (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. +/// Render Grafana provisioning templates /// /// # Errors /// @@ -104,28 +112,6 @@ fn render_templates( ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::RenderGrafanaTemplates; - // 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 provisioning template rendering" - ); - return Ok(()); - } - - // Check if Prometheus is configured (required for datasource) - if environment.context().user_inputs.prometheus().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "Prometheus not configured - skipping Grafana provisioning (datasource requires Prometheus)" - ); - return Ok(()); - } - let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); let step = RenderGrafanaTemplatesStep::new( Arc::new(environment.clone()), @@ -152,10 +138,7 @@ fn render_templates( Ok(()) } -/// Deploy Grafana provisioning configuration to 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. +/// Deploy Grafana provisioning configuration to the remote host /// /// # Errors /// @@ -166,28 +149,6 @@ fn deploy_provisioning_to_remote( ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::DeployGrafanaProvisioning; - // 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 provisioning deployment" - ); - return Ok(()); - } - - // Check if Prometheus is configured (required for datasource) - if environment.context().user_inputs.prometheus().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "Prometheus not configured - skipping Grafana provisioning deployment" - ); - return Ok(()); - } - DeployGrafanaProvisioningStep::new(ansible_client(environment)) .execute() .map_err(|e| { diff --git a/src/application/command_handlers/release/steps/mysql.rs b/src/application/command_handlers/release/steps/mysql.rs index 2af335e7..bfdfad5a 100644 --- a/src/application/command_handlers/release/steps/mysql.rs +++ b/src/application/command_handlers/release/steps/mysql.rs @@ -19,7 +19,7 @@ use crate::domain::environment::{Environment, Releasing}; /// Executes all steps required to release `MySQL`: /// 1. Create `MySQL` storage directories /// -/// If `MySQL` is not configured as the tracker database, this step is skipped. +/// If `MySQL` is not configured as the tracker database, all steps are skipped. /// /// # Errors /// @@ -28,14 +28,22 @@ use crate::domain::environment::{Environment, Releasing}; pub fn release( environment: &Environment, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + // Check if MySQL is configured (via tracker database driver) + if !environment.context().user_inputs.tracker().uses_mysql() { + info!( + command = "release", + service = "mysql", + status = "skipped", + "MySQL not configured - skipping all MySQL steps" + ); + return Ok(()); + } + create_storage(environment)?; 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. +/// Create `MySQL` storage directories on the remote host /// /// # Errors /// @@ -46,17 +54,6 @@ fn create_storage( ) -> 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(()); - } - CreateMysqlStorageStep::new(ansible_client(environment)) .execute() .map_err(|e| { diff --git a/src/application/command_handlers/release/steps/prometheus.rs b/src/application/command_handlers/release/steps/prometheus.rs index a780b829..a18f6a63 100644 --- a/src/application/command_handlers/release/steps/prometheus.rs +++ b/src/application/command_handlers/release/steps/prometheus.rs @@ -38,16 +38,24 @@ use crate::domain::template::TemplateManager; pub fn release( environment: &Environment, ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { + // Check if Prometheus is configured + if environment.context().user_inputs.prometheus().is_none() { + info!( + command = "release", + service = "prometheus", + status = "skipped", + "Prometheus not configured - skipping all Prometheus steps" + ); + return Ok(()); + } + create_storage(environment)?; render_templates(environment)?; deploy_config_to_remote(environment)?; Ok(()) } -/// Create Prometheus storage directories on the remote host (if enabled) -/// -/// This step is optional and only executes if Prometheus is configured in the environment. -/// If Prometheus is not configured, the step is skipped without error. +/// Create Prometheus storage directories on the remote host /// /// # Errors /// @@ -58,17 +66,6 @@ fn create_storage( ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::CreatePrometheusStorage; - // Check if Prometheus is configured - if environment.context().user_inputs.prometheus().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "Prometheus not configured - skipping storage creation" - ); - return Ok(()); - } - CreatePrometheusStorageStep::new(ansible_client(environment)) .execute() .map_err(|e| { @@ -90,10 +87,7 @@ fn create_storage( Ok(()) } -/// Render Prometheus configuration templates to the build directory (if enabled) -/// -/// This step is optional and only executes if Prometheus is configured in the environment. -/// If Prometheus is not configured, the step is skipped without error. +/// Render Prometheus configuration templates to the build directory /// /// # Errors /// @@ -104,17 +98,6 @@ fn render_templates( ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::RenderPrometheusTemplates; - // Check if Prometheus is configured - if environment.context().user_inputs.prometheus().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "Prometheus not configured - skipping template rendering" - ); - return Ok(()); - } - let template_manager = Arc::new(TemplateManager::new(environment.templates_dir())); let step = RenderPrometheusTemplatesStep::new( Arc::new(environment.clone()), @@ -141,10 +124,7 @@ fn render_templates( 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. -/// If Prometheus is not configured, the step is skipped without error. +/// Deploy Prometheus configuration to the remote host via Ansible /// /// # Errors /// @@ -155,17 +135,6 @@ fn deploy_config_to_remote( ) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> { let current_step = ReleaseStep::DeployPrometheusConfigToRemote; - // Check if Prometheus is configured - if environment.context().user_inputs.prometheus().is_none() { - info!( - command = "release", - step = %current_step, - status = "skipped", - "Prometheus not configured - skipping config deployment" - ); - return Ok(()); - } - DeployPrometheusConfigStep::new(ansible_client(environment)) .execute() .map_err(|e| { From 45588ce8c4c4643ce98e0cd18c78cb47e7466047 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 26 Jan 2026 20:47:02 +0000 Subject: [PATCH 12/12] chore: remove release handler analysis document The analysis document was used during the refactoring process and is no longer needed now that the improvements have been implemented. --- docs/analysis/release-handler-code-quality.md | 184 ------------------ 1 file changed, 184 deletions(-) delete mode 100644 docs/analysis/release-handler-code-quality.md diff --git a/docs/analysis/release-handler-code-quality.md b/docs/analysis/release-handler-code-quality.md deleted file mode 100644 index c53c1810..00000000 --- a/docs/analysis/release-handler-code-quality.md +++ /dev/null @@ -1,184 +0,0 @@ -# Code Quality Analysis: Release Command Handler - -**File**: [src/application/command_handlers/release/handler.rs](../../src/application/command_handlers/release/handler.rs) -**Date**: 2026-01-26 -**Lines of Code**: 994 - -## Executive Summary - -The `ReleaseCommandHandler` implements a release workflow for deploying software to configured environments. While it demonstrates good architecture principles (DDD, state machine pattern, comprehensive documentation), there are several maintainability and readability concerns worth addressing. - -## Critical Issues - -### 1. Severe Code Duplication (DRY Violation) - -The file contains **massive repetitive patterns** across 10+ methods. Each step method follows the same structure: - -```rust -fn step_X(environment, instance_ip) -> StepResult<...> { - let current_step = ReleaseStep::X; - - // Optional: Check if feature is configured - if environment.context().user_inputs.feature().is_none() { - info!(..., status = "skipped", "..."); - return Ok(()); - } - - let ansible_client = Arc::new(AnsibleClient::new(environment.build_dir().join("ansible"))); - - SomeStep::new(ansible_client).execute().map_err(|e| { - (ReleaseCommandHandlerError::SomeVariant(e.to_string()), current_step) - })?; - - info!(..., "... completed successfully"); - Ok(()) -} -``` - -**Impact**: Adding a new step requires copying ~30 lines and modifying a few values. This pattern appears **~15 times**. - -### 2. Unused Parameters (`_instance_ip`) - -Multiple methods accept `instance_ip: IpAddr` but prefix it with `_` to suppress unused warnings: - -| Method | Uses `instance_ip`? | -| --------------------------------------- | -------------------------- | -| `create_tracker_storage` | No (`_instance_ip`) | -| `init_tracker_database` | No (`_instance_ip`) | -| `create_prometheus_storage` | No (`_instance_ip`) | -| `create_grafana_storage` | No (`_instance_ip`) | -| `create_mysql_storage` | No (`_instance_ip`) | -| `deploy_prometheus_config_to_remote` | No (`_instance_ip`) | -| `deploy_caddy_config_to_remote` | No (`_instance_ip`) | -| `deploy_grafana_provisioning_to_remote` | No (`_instance_ip`) | -| `deploy_tracker_config_to_remote` | No (`_instance_ip`) | -| `deploy_compose_files_to_remote` | **Yes** (only for logging) | - -**Impact**: 9 out of 10 methods don't use `instance_ip` for logic. This suggests either: - -- Future functionality that was never implemented -- An interface design that doesn't match actual needs -- Copy-paste from a method that did need it - -### 3. Inconsistent Error Mapping - -Different errors use different construction patterns: - -```rust -// Pattern 1: String conversion only -ReleaseCommandHandlerError::TemplateRendering(e.to_string()) - -// Pattern 2: String + Source (loses type info) -ReleaseCommandHandlerError::Deployment { - message: e.to_string(), - source: Box::new(e), -} - -// Pattern 3: Typed source preserved -ReleaseCommandHandlerError::DeploymentFailed { - message: e.to_string(), - source: e, // DeployComposeFilesStepError -} -``` - -**Impact**: Inconsistent error handling makes debugging harder and violates the project's error handling guidelines. - -## Moderate Issues - -### 4. Method Length and Cognitive Load - -| Method | Lines | Complexity | -| -------------------------- | ----- | ---------------------------------- | -| `execute_release_workflow` | ~50 | 15 sequential steps | -| `execute` | ~60 | State transitions + error handling | - -The `execute_release_workflow` method is essentially a flat list of 15 steps, making it hard to understand the logical groupings. - -### 5. Magic String Duplication - -The string `"ansible"` appears as a path suffix **11 times**: - -```rust -environment.build_dir().join("ansible") -``` - -### 6. Clippy Attribute Proliferation - -Many methods require `#[allow(clippy::result_large_err)]` due to the `StepResult` tuple design: - -```rust -#[allow(clippy::result_large_err)] -#[allow(clippy::result_large_err, clippy::unused_self)] -``` - -This suggests the error type design may need reconsideration. - -### 7. Inconsistent Static vs Instance Methods - -| Method Type | Methods | -| ----------------- | ------------------------------------------------------------------------------------------- | -| `Self::` (static) | `create_tracker_storage`, `init_tracker_database`, `render_*_templates`, `create_*_storage` | -| `&self` | `deploy_*_to_remote`, `render_docker_compose_templates` | - -Some `&self` methods don't actually use `self` (hence `clippy::unused_self`). The inconsistency makes the API confusing. - -## Positive Aspects - -1. **Excellent Documentation**: Every method has comprehensive doc comments with `# Errors` sections -2. **Tracing Integration**: Good use of structured logging with `tracing::info!` -3. **Type-State Pattern**: Proper use of `Environment` → `Environment` → `Environment` -4. **Failure Context**: Good failure tracking with `ReleaseFailureContext` and trace file generation -5. **Instrumentation**: Proper use of `#[instrument]` for tracing spans - -## Recommendations - -### Priority 1: Extract Common Step Execution Pattern - -Create a generic step executor that handles the repetitive boilerplate: - -```rust -// Conceptual approach -fn execute_step( - &self, - step: ReleaseStep, - condition: impl Fn(&Environment) -> bool, - skip_message: &str, - step_factory: impl Fn() -> S, - error_mapper: impl Fn(S::Error) -> ReleaseCommandHandlerError, -) -> StepResult<(), ReleaseCommandHandlerError, ReleaseStep> -where - S: Step -``` - -### Priority 2: Group Related Steps - -Consider grouping the 15 steps into logical phases: - -1. **Storage Phase**: Create all storage directories -2. **Render Phase**: Render all templates -3. **Deploy Phase**: Deploy all configurations - -### Priority 3: Remove Unused Parameters - -Either remove `instance_ip` from methods that don't use it, or document why it's there for future use. - -### Priority 4: Standardize Error Construction - -Use a consistent pattern—preferably preserving the source error with `#[source]` for proper error chain debugging. - -### Priority 5: Extract Constants - -```rust -const ANSIBLE_DIR: &str = "ansible"; -``` - -## Metrics Summary - -| Metric | Value | Assessment | -| --------------------- | --------- | ------------------------- | -| Lines of Code | 994 | High for a single handler | -| Methods | ~20 | Reasonable | -| Duplicated Patterns | ~15 | High | -| Unused Parameters | 9 methods | Problematic | -| `#[allow]` Attributes | 12+ | Code smell | -| Doc Coverage | ~100% | Excellent |