Update E2E full tests to use CreateCommandHandler and fix preflight cleanup

src/bin/e2e_tests_full.rs

@@ -55,17 +55,20 @@
 
 use anyhow::Result;
 use clap::Parser;
-use std::time::Instant;
+use std::sync::Arc;
+use std::time::{Duration, Instant};
 use tracing::{error, info};
 
 // Import E2E testing infrastructure
-use torrust_tracker_deployer_lib::adapters::ssh::{SshCredentials, DEFAULT_SSH_PORT};
-use torrust_tracker_deployer_lib::domain::{Environment, EnvironmentName};
+use torrust_tracker_deployer_lib::adapters::ssh::DEFAULT_SSH_PORT;
+use torrust_tracker_deployer_lib::infrastructure::persistence::repository_factory::RepositoryFactory;
 use torrust_tracker_deployer_lib::logging::{LogFormat, LogOutput, LoggingBuilder};
-use torrust_tracker_deployer_lib::shared::Username;
+use torrust_tracker_deployer_lib::shared::{Clock, SystemClock};
 use torrust_tracker_deployer_lib::testing::e2e::context::{TestContext, TestContextType};
 use torrust_tracker_deployer_lib::testing::e2e::tasks::{
+    preflight_cleanup::cleanup_previous_test_data,
     run_configure_command::run_configure_command,
+    run_create_command::run_create_command,
     run_test_command::run_test_command,
     virtual_machine::{
         preflight_cleanup::preflight_cleanup_previous_resources,
@@ -109,6 +112,7 @@ struct Cli {
 ///
 /// May panic during the match statement if unexpected error combinations occur
 /// that are not handled by the current error handling logic.
+#[allow(clippy::too_many_lines)]
 #[tokio::main]
 pub async fn main() -> Result<()> {
     let cli = Cli::parse();
@@ -127,29 +131,38 @@ pub async fn main() -> Result<()> {
         "Starting E2E tests"
     );
 
-    // Create environment entity for e2e-full testing
-    let environment_name = EnvironmentName::new("e2e-full".to_string())?;
-
     // Use absolute paths to project root for SSH keys to ensure they can be found by Ansible
     let project_root = std::env::current_dir().expect("Failed to get current directory");
     let ssh_private_key_path = project_root.join("fixtures/testing_rsa");
     let ssh_public_key_path = project_root.join("fixtures/testing_rsa.pub");
-    let ssh_user = Username::new("torrust").expect("Valid hardcoded username");
-    let ssh_credentials = SshCredentials::new(
-        ssh_private_key_path.clone(),
-        ssh_public_key_path.clone(),
-        ssh_user.clone(),
-    );
 
-    let ssh_port = DEFAULT_SSH_PORT;
-    let environment = Environment::new(environment_name, ssh_credentials, ssh_port);
+    // Cleanup any artifacts from previous test runs BEFORE creating the environment
+    // This prevents "environment already exists" errors from stale state
+    // We do this before CreateCommandHandler because it checks if environment exists in repository
+    cleanup_previous_test_data("e2e-full").map_err(|e| anyhow::anyhow!("{e}"))?;
+
+    // Create repository factory and clock for environment creation
+    let repository_factory = RepositoryFactory::new(Duration::from_secs(30));
+    let clock: Arc<dyn Clock> = Arc::new(SystemClock);
+
+    // Create environment via CreateCommandHandler
+    let environment = run_create_command(
+        &repository_factory,
+        clock,
+        "e2e-full",
+        ssh_private_key_path.to_string_lossy().to_string(),
+        ssh_public_key_path.to_string_lossy().to_string(),
+        "torrust",
+        DEFAULT_SSH_PORT,
+    )
+    .map_err(|e| anyhow::anyhow!("{e}"))?;
 
     let mut test_context =
         TestContext::from_environment(cli.keep, environment, TestContextType::VirtualMachine)?
             .init()?;
 
-    // Cleanup any artifacts from previous test runs that may have failed to clean up
-    // This ensures a clean slate before starting new tests
+    // Additional preflight cleanup for infrastructure (OpenTofu, LXD resources)
+    // This handles any lingering infrastructure from interrupted previous runs
     preflight_cleanup_previous_resources(&test_context)?;
 
     let test_start = Instant::now();

src/testing/e2e/tasks/container/preflight_cleanup.rs

@@ -8,7 +8,8 @@
 use crate::shared::command::CommandExecutor;
 use crate::testing::e2e::context::TestContext;
 use crate::testing::e2e::tasks::preflight_cleanup::{
-    cleanup_build_directory, cleanup_templates_directory, PreflightCleanupError,
+    cleanup_build_directory, cleanup_data_environment, cleanup_templates_directory,
+    PreflightCleanupError,
 };
 use tracing::{info, warn};
 
@@ -44,6 +45,9 @@ pub fn preflight_cleanup_previous_resources(
     // Clean the templates directory to ensure fresh embedded template extraction for E2E tests
     cleanup_templates_directory(env)?;
 
+    // Clean the data directory to ensure fresh environment state for E2E tests
+    cleanup_data_environment(env)?;
+
     // Clean up any hanging Docker containers from interrupted test runs
     cleanup_hanging_docker_containers(env);
 

src/testing/e2e/tasks/mod.rs

@@ -9,6 +9,7 @@
 //! The tasks are organized by deployment target:
 //!
 //! ### Infrastructure-agnostic tasks (can be used with both containers and VMs):
+//! - `run_create_command` - Environment creation using `CreateCommandHandler`
 //! - `run_configure_command` - Infrastructure configuration via Ansible and playbook execution
 //! - `run_configuration_validation` - Configuration validation and testing
 //! - `run_test_command` - Deployment validation and testing
@@ -30,5 +31,6 @@ pub mod container;
 pub mod preflight_cleanup;
 pub mod run_configuration_validation;
 pub mod run_configure_command;
+pub mod run_create_command;
 pub mod run_test_command;
 pub mod virtual_machine;

src/testing/e2e/tasks/preflight_cleanup.rs

@@ -48,6 +48,100 @@ impl std::error::Error for PreflightCleanupError {
     }
 }
 
+// TODO: Refactor TestContext to eliminate the need for this workaround function
+//
+// Current issue: TestContext requires an Environment, but we need to clean up data
+// directories BEFORE creating the Environment (because CreateCommandHandler checks
+// if the environment already exists in the repository).
+//
+// Proposed solutions:
+// 1. Make Environment optional in TestContext (TestContext { environment: Option<Environment> })
+// 2. Move Environment out of TestContext (preferred - better separation of concerns)
+//
+// The second option is better because:
+// - TestContext should manage test infrastructure (services, config, temp directories)
+// - Environment is a domain entity that represents deployment state
+// - Separating them provides clearer responsibilities and easier testing
+//
+// After refactoring, we could eliminate this standalone function and have all cleanup
+// go through a single preflight_cleanup_previous_resources() that doesn't require
+// a fully initialized TestContext with an Environment.
+
+/// Cleans the data directory for a specific environment name before `TestContext` creation
+///
+/// This helper function removes the `data/{environment_name}` directory to prevent
+/// "environment already exists" errors when `CreateCommandHandler` checks the repository.
+/// Unlike `cleanup_data_environment`, this function works without a `TestContext` and is
+/// intended to be called early in the test setup before environment creation.
+///
+/// # Safety
+///
+/// This function is only intended for E2E test environments and should never
+/// be called in production code paths. It's designed to provide test isolation
+/// by ensuring environments from previous test runs don't interfere.
+///
+/// # Arguments
+///
+/// * `environment_name` - The name of the environment to clean up
+///
+/// # Returns
+///
+/// Returns `Ok(())` if cleanup succeeds or if the directory doesn't exist.
+///
+/// # Errors
+///
+/// Returns a `PreflightCleanupError::ResourceConflicts` error if the data directory
+/// cannot be removed due to permission issues or file locks.
+pub fn cleanup_previous_test_data(environment_name: &str) -> Result<(), PreflightCleanupError> {
+    use std::path::Path;
+
+    let data_dir = Path::new("data").join(environment_name);
+
+    if !data_dir.exists() {
+        info!(
+            operation = "preflight_data_cleanup",
+            status = "clean",
+            path = %data_dir.display(),
+            "No previous data directory found, skipping cleanup"
+        );
+        return Ok(());
+    }
+
+    info!(
+        operation = "preflight_data_cleanup",
+        path = %data_dir.display(),
+        "Cleaning data directory from previous test run"
+    );
+
+    match std::fs::remove_dir_all(&data_dir) {
+        Ok(()) => {
+            info!(
+                operation = "preflight_data_cleanup",
+                status = "success",
+                path = %data_dir.display(),
+                "Data directory cleaned successfully"
+            );
+            Ok(())
+        }
+        Err(e) => {
+            warn!(
+                operation = "preflight_data_cleanup",
+                status = "failed",
+                path = %data_dir.display(),
+                error = %e,
+                "Failed to clean data directory"
+            );
+            Err(PreflightCleanupError::ResourceConflicts {
+                details: format!(
+                    "Failed to clean data directory '{}': {}",
+                    data_dir.display(),
+                    e
+                ),
+            })
+        }
+    }
+}
+
 /// Cleans the build directory to ensure fresh template state for E2E tests
 ///
 /// This function removes the build directory if it exists, ensuring that
@@ -192,3 +286,79 @@ pub fn cleanup_templates_directory(
         }
     }
 }
+
+/// Cleans the data directory for the test environment to ensure fresh state for E2E tests
+///
+/// This function removes the environment's data directory if it exists, ensuring that
+/// E2E tests start with a clean state and don't encounter conflicts with stale
+/// environment data from previous test runs. This prevents "environment already exists"
+/// errors and ensures proper test isolation.
+///
+/// # Safety
+///
+/// This function is only intended for E2E test environments and should never
+/// be called in production code paths. It's designed to provide test isolation
+/// by ensuring fresh environment state for each test run.
+///
+/// # Arguments
+///
+/// * `test_context` - The test context containing the environment configuration
+///
+/// # Returns
+///
+/// Returns `Ok(())` if cleanup succeeds or if the data directory doesn't exist.
+///
+/// # Errors
+///
+/// Returns a `PreflightCleanupError::ResourceConflicts` error if the data directory
+/// cannot be removed due to permission issues or file locks.
+pub fn cleanup_data_environment(test_context: &TestContext) -> Result<(), PreflightCleanupError> {
+    use std::path::Path;
+
+    // Construct the data directory path: data/{environment_name}
+    let data_dir = Path::new("data").join(test_context.environment.name().as_str());
+
+    if !data_dir.exists() {
+        info!(
+            operation = "data_directory_cleanup",
+            status = "clean",
+            path = %data_dir.display(),
+            "Data directory doesn't exist, skipping cleanup"
+        );
+        return Ok(());
+    }
+
+    info!(
+        operation = "data_directory_cleanup",
+        path = %data_dir.display(),
+        "Cleaning data directory for previous test environment"
+    );
+
+    match std::fs::remove_dir_all(&data_dir) {
+        Ok(()) => {
+            info!(
+                operation = "data_directory_cleanup",
+                status = "success",
+                path = %data_dir.display(),
+                "Data directory cleaned successfully"
+            );
+            Ok(())
+        }
+        Err(e) => {
+            warn!(
+                operation = "data_directory_cleanup",
+                status = "failed",
+                path = %data_dir.display(),
+                error = %e,
+                "Failed to clean data directory"
+            );
+            Err(PreflightCleanupError::ResourceConflicts {
+                details: format!(
+                    "Failed to clean data directory '{}': {}",
+                    data_dir.display(),
+                    e
+                ),
+            })
+        }
+    }
+}