refactor: improve E2E test infrastructure with better error handling and modular cleanup

- Extract cleanup functionality into dedicated module
- Add pre-flight cleanup to handle lingering resources from interrupted tests
- Replace generic Box<dyn Error> with concrete EmergencyDestroyError type
- Improve E2E test documentation with comprehensive error information
- Add proper error handling traits and better resource conflict detection
- Restructure imports for better separation of concerns

Author: josecelano

src/bin/e2e_tests.rs

@@ -7,9 +7,10 @@ use tracing::{error, info};
 // Import E2E testing infrastructure
 use torrust_tracker_deploy::e2e::environment::TestEnvironment;
 use torrust_tracker_deploy::e2e::tasks::{
+    cleanup_infrastructure::cleanup_infrastructure,
     configure_infrastructure::configure_infrastructure,
-    provision_infrastructure::{cleanup_infrastructure, provision_infrastructure},
-    validate_deployment::validate_deployment,
+    preflight_cleanup::cleanup_lingering_resources,
+    provision_infrastructure::provision_infrastructure, validate_deployment::validate_deployment,
 };
 use torrust_tracker_deploy::logging::{self, LogFormat};
 
@@ -34,8 +35,26 @@ struct Cli {
     log_format: LogFormat,
 }
 
+/// Main entry point for E2E tests.
+///
+/// Runs the full deployment workflow: provision infrastructure, configure services,
+/// validate deployment, and cleanup resources.
+///
+/// # Errors
+///
+/// Returns an error if:
+/// - Pre-flight cleanup fails
+/// - Infrastructure provisioning fails  
+/// - Service configuration fails
+/// - Deployment validation fails
+/// - Resource cleanup fails (when enabled)
+///
+/// # Panics
+///
+/// May panic during the match statement if unexpected error combinations occur
+/// that are not handled by the current error handling logic.
 #[tokio::main]
-async fn main() -> Result<()> {
+pub async fn main() -> Result<()> {
     let cli = Cli::parse();
 
     // Initialize logging based on the chosen format
@@ -50,6 +69,9 @@ async fn main() -> Result<()> {
 
     let env = TestEnvironment::new(cli.keep, cli.templates_dir)?;
 
+    // Perform pre-flight cleanup to remove any lingering resources from interrupted tests
+    cleanup_lingering_resources(&env)?;
+
     let test_start = Instant::now();
 
     let deployment_result = run_full_deployment_test(&env).await;

src/command_wrappers/opentofu/mod.rs

@@ -7,6 +7,38 @@ pub mod json_parser;
 pub use client::{InstanceInfo, OpenTofuClient, OpenTofuError};
 pub use json_parser::ParseError;
 
+/// Errors that can occur during emergency destroy operations
+#[derive(Debug)]
+pub enum EmergencyDestroyError {
+    /// Command execution failed (e.g., tofu binary not found)
+    CommandExecution { source: std::io::Error },
+
+    /// `OpenTofu` destroy operation failed with error output
+    DestroyFailed { stderr: String },
+}
+
+impl std::fmt::Display for EmergencyDestroyError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::CommandExecution { source } => {
+                write!(f, "Failed to execute OpenTofu destroy command: {source}")
+            }
+            Self::DestroyFailed { stderr } => {
+                write!(f, "OpenTofu destroy failed: {stderr}")
+            }
+        }
+    }
+}
+
+impl std::error::Error for EmergencyDestroyError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match self {
+            Self::CommandExecution { source } => Some(source),
+            Self::DestroyFailed { .. } => None,
+        }
+    }
+}
+
 /// Emergency destroy operation for cleanup scenarios
 ///
 /// This function performs a destructive `OpenTofu` destroy operation without prompting.
@@ -19,13 +51,13 @@ pub use json_parser::ParseError;
 ///
 /// # Returns
 ///
-/// * `Result<(), Box<dyn std::error::Error>>` - Success or error from the destroy operation
+/// * `Result<(), EmergencyDestroyError>` - Success or concrete error from the destroy operation
 ///
 /// # Errors
 ///
 /// Returns an error if the `OpenTofu` destroy command fails or if there are issues
 /// with command execution.
-pub fn emergency_destroy<P: AsRef<Path>>(working_dir: P) -> Result<(), Box<dyn std::error::Error>> {
+pub fn emergency_destroy<P: AsRef<Path>>(working_dir: P) -> Result<(), EmergencyDestroyError> {
     use std::process::Command;
 
     tracing::debug!(
@@ -36,14 +68,15 @@ pub fn emergency_destroy<P: AsRef<Path>>(working_dir: P) -> Result<(), Box<dyn s
     let output = Command::new("tofu")
         .args(["destroy", "-auto-approve"])
         .current_dir(&working_dir)
-        .output()?;
+        .output()
+        .map_err(|source| EmergencyDestroyError::CommandExecution { source })?;
 
     if output.status.success() {
         tracing::debug!("Emergency destroy: `OpenTofu` destroy completed successfully");
         Ok(())
     } else {
-        let stderr = String::from_utf8_lossy(&output.stderr);
+        let stderr = String::from_utf8_lossy(&output.stderr).to_string();
         tracing::error!("Emergency destroy: `OpenTofu` destroy failed: {stderr}");
-        Err(format!("`OpenTofu` destroy failed: {stderr}").into())
+        Err(EmergencyDestroyError::DestroyFailed { stderr })
     }
 }

src/e2e/tasks/cleanup_infrastructure.rs

@@ -0,0 +1,55 @@
+use tracing::{info, warn};
+
+use crate::e2e::environment::TestEnvironment;
+
+/// Clean up test infrastructure
+///
+/// This function destroys the test infrastructure using `OpenTofu`.
+/// If `keep_env` is set in the environment configuration, the cleanup
+/// is skipped and the environment is preserved.
+///
+/// # Arguments
+///
+/// * `env` - The test environment containing configuration and services
+///
+/// # Behavior
+///
+/// - If `env.config.keep_env` is `true`, logs a message and returns without cleanup
+/// - Otherwise, attempts to destroy infrastructure using `OpenTofu`
+/// - Logs success or failure appropriately
+/// - Does not return errors - failures are logged as warnings
+pub fn cleanup_infrastructure(env: &TestEnvironment) {
+    if env.config.keep_env {
+        info!(
+            operation = "cleanup",
+            action = "keep_environment",
+            instance = "torrust-vm",
+            connect_command = "lxc exec torrust-vm -- /bin/bash",
+            "Keeping test environment as requested"
+        );
+        return;
+    }
+
+    info!(operation = "cleanup", "Cleaning up test environment");
+
+    // Destroy infrastructure using OpenTofuClient
+    let result = env
+        .services
+        .opentofu_client
+        .destroy(true) // auto_approve = true
+        .map_err(anyhow::Error::from);
+
+    match result {
+        Ok(_) => info!(
+            operation = "cleanup",
+            status = "success",
+            "Test environment cleaned up successfully"
+        ),
+        Err(e) => warn!(
+            operation = "cleanup",
+            status = "failed",
+            error = %e,
+            "Cleanup failed"
+        ),
+    }
+}

src/e2e/tasks/mod.rs

@@ -1,5 +1,7 @@
 pub mod clean_and_prepare_templates;
+pub mod cleanup_infrastructure;
 pub mod configure_infrastructure;
+pub mod preflight_cleanup;
 pub mod provision_infrastructure;
 pub mod setup_ssh_key;
 pub mod validate_deployment;

src/e2e/tasks/preflight_cleanup.rs

@@ -0,0 +1,126 @@
+//! Pre-flight cleanup module for E2E tests
+//!
+//! This module provides functionality to clean up any lingering resources
+//! from previous test runs that may have been interrupted before cleanup.
+
+use std::fmt;
+use tracing::{info, warn};
+
+use crate::command_wrappers::opentofu::{self, EmergencyDestroyError};
+use crate::e2e::environment::TestEnvironment;
+
+/// Errors that can occur during pre-flight cleanup operations
+#[derive(Debug)]
+pub enum PreflightCleanupError {
+    /// Emergency destroy operation failed
+    EmergencyDestroyFailed { source: EmergencyDestroyError },
+
+    /// Resource conflicts detected that would prevent new test runs
+    ResourceConflicts { details: String },
+}
+
+impl fmt::Display for PreflightCleanupError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::EmergencyDestroyFailed { source } => {
+                write!(f, "Emergency destroy operation failed: {source}")
+            }
+            Self::ResourceConflicts { details } => {
+                write!(
+                    f,
+                    "Resource conflicts detected that would prevent new test runs: {details}"
+                )
+            }
+        }
+    }
+}
+
+impl std::error::Error for PreflightCleanupError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match self {
+            Self::EmergencyDestroyFailed { source } => Some(source),
+            Self::ResourceConflicts { .. } => None,
+        }
+    }
+}
+
+/// Performs pre-flight cleanup of any lingering test resources
+///
+/// This function attempts to clean up any resources that might remain from
+/// previous interrupted test runs. It's designed to be safe to run even when
+/// no resources exist.
+///
+/// # Arguments
+///
+/// * `env` - The test environment containing configuration and services
+///
+/// # Returns
+///
+/// Returns `Ok(())` if cleanup succeeds or if there were no resources to clean up.
+///
+/// # Errors
+///
+/// Returns an error if cleanup fails and resources are still present that would
+/// prevent new test runs from starting successfully.
+///
+/// # Examples
+///
+/// ```no_run
+/// use torrust_tracker_deploy::e2e::{environment::TestEnvironment, tasks::preflight_cleanup};
+///
+/// # fn example() -> Result<(), Box<dyn std::error::Error>> {
+/// let env = TestEnvironment::new(false, "./templates")?;
+/// preflight_cleanup::cleanup_lingering_resources(&env)?;
+/// # Ok(())
+/// # }
+/// ```
+pub fn cleanup_lingering_resources(env: &TestEnvironment) -> Result<(), PreflightCleanupError> {
+    info!(
+        operation = "preflight_cleanup",
+        "Starting pre-flight cleanup of any lingering test resources"
+    );
+
+    // Attempt to destroy any existing OpenTofu infrastructure
+    let tofu_dir = env.config.build_dir.join(&env.config.opentofu_subfolder);
+
+    if !tofu_dir.exists() {
+        info!(
+            operation = "preflight_cleanup",
+            status = "clean",
+            "No OpenTofu directory found, skipping cleanup"
+        );
+        return Ok(());
+    }
+
+    // Use emergency_destroy which is designed to handle cases where resources may not exist
+    match opentofu::emergency_destroy(&tofu_dir) {
+        Ok(()) => {
+            info!(
+                operation = "preflight_cleanup",
+                status = "success",
+                "Pre-flight cleanup completed successfully"
+            );
+        }
+        Err(e) => {
+            // Log as warning rather than error since this is pre-flight cleanup
+            // and resources may legitimately not exist
+            warn!(
+                operation = "preflight_cleanup",
+                status = "partial_failure",
+                error = %e,
+                "Pre-flight cleanup encountered issues (this may be normal if no resources existed)"
+            );
+
+            // Don't return an error for pre-flight cleanup failures unless they indicate
+            // actual resource conflicts that would prevent new test runs
+            let error_message = e.to_string().to_lowercase();
+            if error_message.contains("already exists") || error_message.contains("in use") {
+                return Err(PreflightCleanupError::ResourceConflicts {
+                    details: e.to_string(),
+                });
+            }
+            return Err(PreflightCleanupError::EmergencyDestroyFailed { source: e });
+        }
+    }
+    Ok(())
+}

src/e2e/tasks/provision_infrastructure.rs

@@ -1,7 +1,7 @@
 use anyhow::{Context, Result};
 use std::net::IpAddr;
 use std::sync::Arc;
-use tracing::{info, warn};
+use tracing::info;
 
 use crate::commands::ProvisionCommand;
 use crate::e2e::environment::TestEnvironment;
@@ -41,39 +41,3 @@ pub async fn provision_infrastructure(env: &TestEnvironment) -> Result<IpAddr> {
     // Return the IP from OpenTofu as it's our preferred source
     Ok(opentofu_instance_ip)
 }
-
-pub fn cleanup_infrastructure(env: &TestEnvironment) {
-    if env.config.keep_env {
-        info!(
-            operation = "cleanup",
-            action = "keep_environment",
-            instance = "torrust-vm",
-            connect_command = "lxc exec torrust-vm -- /bin/bash",
-            "Keeping test environment as requested"
-        );
-        return;
-    }
-
-    info!(operation = "cleanup", "Cleaning up test environment");
-
-    // Destroy infrastructure using OpenTofuClient
-    let result = env
-        .services
-        .opentofu_client
-        .destroy(true) // auto_approve = true
-        .map_err(anyhow::Error::from);
-
-    match result {
-        Ok(_) => info!(
-            operation = "cleanup",
-            status = "success",
-            "Test environment cleaned up successfully"
-        ),
-        Err(e) => warn!(
-            operation = "cleanup",
-            status = "failed",
-            error = %e,
-            "Cleanup failed"
-        ),
-    }
-}