feat: Add DestroyCommand in Application Layer with E2E Integration#14
Conversation
Co-authored-by: josecelano <58816+josecelano@users.noreply.github.com>
E2E Test Enhancement Request for DestroyCommand@copilot The DestroyCommand implementation looks excellent! However, we need to enhance the E2E testing to properly exercise the new destroy functionality. Required Changes to
|
- Create run_destroy_command task in virtual_machine module - Add update_from_destroyed method to TestContext - Add run_full_destroy_test function to e2e_tests_full.rs - Replace legacy cleanup_test_infrastructure with DestroyCommand - Handle keep_env flag in run_destroy_command - Add comprehensive error handling for destroy operations Co-authored-by: josecelano <58816+josecelano@users.noreply.github.com>
Implemented as requested. Added
All tests pass and linters are clean. Commit: b593725 |
🎉 Excellent Work on DestroyCommand Implementation!@copilot - Outstanding job implementing the DestroyCommand! The implementation is comprehensive, well-tested, and follows all the project patterns perfectly. The E2E testing enhancements are particularly impressive. After thorough review and testing, I've identified three specific improvements that would make this production-ready: 🔧 Problem #1: Repetitive Pattern Matching in E2E TestsFile: Current Code (lines 82-95): let destroyed_env = match test_context.environment.clone() {
AnyEnvironmentState::Created(env) => destroy_command.execute(env),
AnyEnvironmentState::Provisioning(env) => destroy_command.execute(env),
AnyEnvironmentState::Provisioned(env) => destroy_command.execute(env),
AnyEnvironmentState::Configuring(env) => destroy_command.execute(env),
AnyEnvironmentState::Configured(env) => destroy_command.execute(env),
AnyEnvironmentState::Releasing(env) => destroy_command.execute(env),
AnyEnvironmentState::Released(env) => destroy_command.execute(env),
AnyEnvironmentState::Running(env) => destroy_command.execute(env),
AnyEnvironmentState::ConfigureFailed(env) => destroy_command.execute(env),
AnyEnvironmentState::ReleaseFailed(env) => destroy_command.execute(env),
AnyEnvironmentState::RunFailed(env) => destroy_command.execute(env),
AnyEnvironmentState::Destroyed(env) => {
info!("Environment is already in Destroyed state");
Ok(env)
}
}Suggested Fix: let destroyed_env = match test_context.environment.clone() {
AnyEnvironmentState::Destroyed(env) => {
info!("Environment is already in Destroyed state");
Ok(env)
}
_ => destroy_command.execute(env),
}
|
Problem #1: Simplify pattern matching in run_destroy_command - Reorder match arms to check Destroyed state first - More readable and maintainable code structure Problem #2: Add state-based infrastructure logic (simplified) - OpenTofu destroy is idempotent, so always call it - No need for complex state checking Problem #3: Implement state file cleanup per Issue #11 - Add cleanup_state_files method to remove data/ and build/ dirs - Only cleanup after successful infrastructure destruction - Add StateCleanupFailed error variant with proper error handling - Include path information in error messages for debugging Co-authored-by: josecelano <58816+josecelano@users.noreply.github.com>
All three improvements implemented:
All tests pass and linters are clean. Commit: [short hash will be in next message] |
|
ACK b9b29b3 |
Overview
Implements the
DestroyCommandin the Application Layer to orchestrate infrastructure destruction workflows. This command follows the established DDD patterns used byProvisionCommandandConfigureCommand, providing a clean, testable interface for tearing down deployed environments.This PR also includes full E2E test integration to properly exercise the destroy functionality in the test suite.
Implementation Details
Architecture
The implementation follows the three-level architecture pattern:
DestroyCommandorchestrates the complete destruction workflowDestroyInfrastructureStepexecutes OpenTofu destroy operationsOpenTofuClient.destroy()methodKey Components
DestroyCommand(src/application/commands/destroy.rs)Environment<S>in any stateEnvironment<Destroyed>on successdata/andbuild/directories after successful destruction (per Issue Add DestroyCommand in Application Layer #11)DestroyInfrastructureStep(src/application/steps/infrastructure/destroy.rs)run_destroy_command(src/testing/e2e/tasks/virtual_machine/run_destroy_command.rs)DestroyCommand::execute()keep_envflag for debuggingError Handling
Traceabletrait for error chain trackingState File Cleanup
The DestroyCommand automatically cleans up environment state files after successful infrastructure destruction:
data/directory (contains state and metadata)build/directory (contains OpenTofu and Ansible configurations)Usage Example
E2E Test Integration
New E2E Components
run_full_destroy_testfunction ine2e_tests_full.rsrun_full_deployment_testbut for destructionupdate_from_destroyedmethod inTestContextupdate_from_provisioned()andupdate_from_configured()E2E Test Workflow
The E2E test suite now includes the complete lifecycle:
The destroy operation is executed using the new
DestroyCommandinstead of the legacy direct OpenTofu client calls, ensuring proper state management and error handling throughout the test lifecycle.Testing
Quality Assurance
Files Changed
Created:
src/application/commands/destroy.rs(280 lines) - includes state cleanup logicsrc/application/steps/infrastructure/destroy.rs(128 lines)src/testing/e2e/tasks/virtual_machine/run_destroy_command.rs(170 lines)Modified:
src/application/commands/mod.rssrc/application/steps/infrastructure/mod.rssrc/application/steps/mod.rssrc/bin/e2e_tests_full.rssrc/testing/e2e/context.rssrc/testing/e2e/tasks/virtual_machine/mod.rsRelated Issues
Next Steps
This implementation provides the core command functionality with E2E integration. Subsequent PRs will:
Original prompt
This section details on the original issue you should resolve
<issue_title>Add DestroyCommand in Application Layer</issue_title>
<issue_description># Add DestroyCommand in Application Layer
Issue Type: Sub-issue (9.1)
Parent Epic: #9 (
epic-app-layer-destroy-command.md)Related Roadmap: Section 1.2
Priority: High
Estimated Effort: 6-8 hours
📋 Issue Overview
Create the
DestroyCommandstruct and implementation insrc/application/commands/destroy/with complete infrastructure teardown functionality.This is the core subtask that implements the business logic for destroying deployed environments. It follows the DDD Application Layer patterns established by existing commands like
ProvisionCommandandConfigCommand.🎯 Goals
DestroyCommandfollowing existing command patternsthiserrorintegration📦 Scope
Core Implementation
ProvisionCommand,ConfigCommand)src/bin/e2e_tests_full.rsfor immediate testingInfrastructure Integration
We already have the infrastructure teardown logic in
src/testing/e2e/tasks/virtual_machine/cleanup_infrastructure.rsthat calls:This subtask combines command creation with infrastructure teardown because they're closely coupled and we need the full functionality to test it properly.
🏗️ Technical Design
Command Structure
Follow the established DDD Application Layer patterns:
Error Handling
Define proper error types with
thiserror:Integration Points
OpenTofuClientfor infrastructure destructionsrc/bin/e2e_tests_full.rsfor immediate testing📋 Acceptance Criteria
DestroyCommandexists insrc/application/commands/destroy/thiserrorsrc/bin/e2e_tests_full.rsto test the feature locally🧪 Testing Strategy
Unit Tests
Integration Testing
E2E Testing
e2e_tests_full.rs🔗 Dependencies
src/testing/e2e/tasks/virtual_machine/cleanup_infrastructure.rs📝 Implementation Notes
Design Considerations
Fixes #11
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.