From 42c52c42f51413171abf4752e6f6756c0d41038f Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 25 Jul 2025 18:14:10 +0100 Subject: [PATCH 1/9] refactor: [#14] move general utility functions from test-e2e.sh to shell-utils.sh - Extract general helper functions to scripts/shell-utils.sh for project-wide reuse: * test_http_endpoint() - HTTP endpoint testing with content validation * retry_with_timeout() - Configurable retry logic with custom parameters * time_operation() - Operation timing with automatic duration logging - Keep project-specific functions in tests/test-e2e.sh: * get_vm_ip() - Specific to torrust-tracker-demo VM name * ssh_to_vm() - Specific to torrust user and VM configuration - Benefits achieved: * Reduced code duplication across the project * Centralized common patterns for better maintainability * Standardized error handling and logging * Better separation of concerns between test logic and utilities - Quality assurance: * All code passes ShellCheck linting * End-to-end test passes (2m 42s execution time) * Functions maintain backward compatibility * Enhanced documentation and usage examples This refactoring eliminates duplicate code patterns while creating reusable utilities that can benefit other scripts in the infrastructure and application directories. --- scripts/shell-utils.sh | 100 ++++++++++++++++++++++++++++++++++++++++- tests/test-e2e.sh | 89 +++++++++++++++--------------------- 2 files changed, 133 insertions(+), 56 deletions(-) diff --git a/scripts/shell-utils.sh b/scripts/shell-utils.sh index 6b7a0b6..26219ac 100644 --- a/scripts/shell-utils.sh +++ b/scripts/shell-utils.sh @@ -14,6 +14,16 @@ # log_success "Operation completed successfully" # log_warning "This is a warning" # log_error "This is an error" +# +# # Use HTTP testing: +# result=$(test_http_endpoint "http://example.com" "expected content") +# if [[ "$result" == "success" ]]; then echo "Endpoint working"; fi +# +# # Use retry logic: +# retry_with_timeout "Testing connection" 5 2 "ping -c1 example.com >/dev/null" +# +# # Time operations: +# time_operation "Deployment" "make deploy" # Shared shell utilities - can be sourced multiple times safely export SHELL_UTILS_LOADED=1 @@ -212,6 +222,14 @@ ${BLUE}ENVIRONMENT VARIABLES:${NC} TRACE Set to 'true' to enable trace logging DRY_RUN Set to 'true' to show commands without executing +${BLUE}AVAILABLE FUNCTIONS:${NC} + Logging: log_info, log_success, log_warning, log_error, log_debug, log_trace + HTTP Testing: test_http_endpoint [expected_content] [timeout] + Retry Logic: retry_with_timeout + Timing: time_operation + Sudo Management: ensure_sudo_cached, run_with_sudo, clear_sudo_cache + Utilities: command_exists, safe_cd, execute_with_log, require_env_vars + ${BLUE}EXAMPLES:${NC} # Enable logging to file export SHELL_UTILS_LOG_FILE="/tmp/my-script.log" @@ -219,8 +237,16 @@ ${BLUE}EXAMPLES:${NC} # Enable debug mode export DEBUG=true - # Dry run mode - export DRY_RUN=true + # Test HTTP endpoint + if [[ \$(test_http_endpoint "https://example.com" "success") == "success" ]]; then + log_success "Endpoint is working" + fi + + # Retry with timeout + retry_with_timeout "Testing connection" 5 2 "ping -c1 example.com >/dev/null" + + # Time an operation + time_operation "Deployment" "make deploy" EOF } @@ -274,3 +300,73 @@ clear_sudo_cache() { sudo -k log_debug "Sudo credentials cache cleared" } + +# HTTP and Network Testing Functions + +# Test HTTP endpoints with optional content validation +test_http_endpoint() { + local url="$1" + local expected_content="$2" + local timeout="${3:-5}" + + local response + response=$(curl -f -s --max-time "${timeout}" "${url}" 2>/dev/null || echo "") + + if [[ -n "${expected_content}" ]] && echo "${response}" | grep -q "${expected_content}"; then + echo "success" + elif [[ -z "${expected_content}" ]] && [[ -n "${response}" ]]; then + echo "success" + else + echo "failed" + fi +} + +# Retry Logic and Timing Functions + +# Execute a command with retry logic and configurable parameters +retry_with_timeout() { + local description="$1" + local max_attempts="$2" + local sleep_interval="$3" + local test_command="$4" + + local attempt=1 + while [[ ${attempt} -le ${max_attempts} ]]; do + log_info "${description} (attempt ${attempt}/${max_attempts})..." + + if eval "${test_command}"; then + return 0 + fi + + if [[ ${attempt} -eq ${max_attempts} ]]; then + log_error "${description} failed after ${max_attempts} attempts" + return 1 + fi + + sleep "${sleep_interval}" + ((attempt++)) + done +} + +# Time an operation and log the duration +time_operation() { + local operation_name="$1" + local command="$2" + + local start_time + start_time=$(date +%s) + + if eval "${command}"; then + local end_time + end_time=$(date +%s) + local duration=$((end_time - start_time)) + log_success "${operation_name} completed successfully in ${duration} seconds" + return 0 + else + local end_time + end_time=$(date +%s) + local duration=$((end_time - start_time)) + log_error "${operation_name} failed after ${duration} seconds" + return 1 + fi +} diff --git a/tests/test-e2e.sh b/tests/test-e2e.sh index 1f8151b..e4400a7 100755 --- a/tests/test-e2e.sh +++ b/tests/test-e2e.sh @@ -32,6 +32,20 @@ log_section() { log "${BLUE}===============================================${NC}" } +# Helper function to get VM IP address from libvirt +get_vm_ip() { + virsh domifaddr torrust-tracker-demo 2>/dev/null | grep ipv4 | awk '{print $4}' | cut -d'/' -f1 || echo "" +} + +# Helper function for SSH connections with standard options +ssh_to_vm() { + local vm_ip="$1" + local command="$2" + local output_redirect="${3:->/dev/null 2>&1}" + + eval ssh -o ConnectTimeout=5 -o StrictHostKeyChecking=no torrust@"${vm_ip}" "\"${command}\"" "${output_redirect}" +} + # Track test start time TEST_START_TIME=$(date +%s) @@ -106,19 +120,11 @@ test_infrastructure_provisioning() { # Provision infrastructure (Step 2.3 from guide) log_info "Provisioning infrastructure..." - local start_time - start_time=$(date +%s) - - if ! make infra-apply ENVIRONMENT="${ENVIRONMENT}"; then + if ! time_operation "Infrastructure provisioning" "make infra-apply ENVIRONMENT=\"${ENVIRONMENT}\""; then log_error "Infrastructure provisioning failed" return 1 fi - local end_time - end_time=$(date +%s) - local duration=$((end_time - start_time)) - log_success "Infrastructure provisioned successfully in ${duration} seconds" - # Verify infrastructure (Step 2.4 from guide) log_info "Verifying infrastructure status..." if ! make infra-status ENVIRONMENT="${ENVIRONMENT}"; then @@ -149,10 +155,7 @@ test_application_deployment() { # Deploy application (Step 3.1 from guide) log_info "Deploying application using twelve-factor workflow..." - local start_time - start_time=$(date +%s) - - if ! make app-deploy ENVIRONMENT="${ENVIRONMENT}"; then + if ! time_operation "Application deployment" "make app-deploy ENVIRONMENT=\"${ENVIRONMENT}\""; then log_error "Application deployment failed" return 1 fi @@ -160,11 +163,6 @@ test_application_deployment() { # Note: app-deploy includes health validation via validate_deployment function log_info "Application deployment completed with built-in health validation" - local end_time - end_time=$(date +%s) - local duration=$((end_time - start_time)) - log_success "Application deployed successfully in ${duration} seconds" - return 0 } @@ -187,32 +185,21 @@ test_health_validation() { # Get VM IP for direct testing local vm_ip - vm_ip=$(virsh domifaddr torrust-tracker-demo 2>/dev/null | grep ipv4 | awk '{print $4}' | cut -d'/' -f1 || echo "") + vm_ip=$(get_vm_ip) if [[ -n "${vm_ip}" ]]; then log_info "Testing application endpoints on ${vm_ip}..." # Test tracker health endpoint (may take a moment to be ready) - local max_attempts=12 # 2 minutes - local attempt=1 - while [[ ${attempt} -le ${max_attempts} ]]; do - log_info "Testing health endpoint (attempt ${attempt}/${max_attempts})..." - # shellcheck disable=SC2034,SC2086 - if curl -f -s http://"${vm_ip}"/api/health_check >/dev/null 2>&1; then - log_success "Health endpoint responding" - break - fi - if [[ ${attempt} -eq ${max_attempts} ]]; then - log_warning "Health endpoint not responding after ${max_attempts} attempts" - else - sleep 10 - fi - ((attempt++)) - done + if retry_with_timeout "Testing health endpoint" 12 10 "test_http_endpoint \"http://${vm_ip}/api/health_check\" '\"status\":\"Ok\"' >/dev/null"; then # DevSkim: ignore DS137138 + log_success "Health endpoint responding" + else + log_warning "Health endpoint not responding after 12 attempts" + fi # Test if basic services are running log_info "Checking if Docker services are running..." - if ssh -o ConnectTimeout=5 -o StrictHostKeyChecking=no torrust@"${vm_ip}" "cd /home/torrust/github/torrust/torrust-tracker-demo/application && docker compose ps --services --filter status=running" 2>/dev/null | grep -q tracker; then + if ssh_to_vm "${vm_ip}" "cd /home/torrust/github/torrust/torrust-tracker-demo/application && docker compose ps --services --filter status=running" 2>/dev/null | grep -q tracker; then log_success "Tracker service is running" else log_warning "Tracker service may not be running yet" @@ -231,7 +218,7 @@ test_smoke_testing() { # Get VM IP for testing local vm_ip - vm_ip=$(virsh domifaddr torrust-tracker-demo 2>/dev/null | grep ipv4 | awk '{print $4}' | cut -d'/' -f1 || echo "") + vm_ip=$(get_vm_ip) if [[ -z "${vm_ip}" ]]; then log_error "VM IP not available - cannot run mandatory smoke tests" @@ -245,23 +232,19 @@ test_smoke_testing() { # Test 1: Health Check API (through nginx proxy on port 80) log_info "Testing health check API through nginx proxy..." - local health_response - health_response=$(curl -f -s http://"${vm_ip}":80/api/health_check 2>/dev/null || echo "") - if echo "${health_response}" | grep -q '"status":"Ok"'; then + if [[ $(test_http_endpoint "http://${vm_ip}:80/api/health_check" '"status":"Ok"') == "success" ]]; then # DevSkim: ignore DS137138 log_success "✓ Health check API working" else - log_error "✗ Health check API failed - Response: ${health_response}" + log_error "✗ Health check API failed" ((failed_tests++)) fi # Test 2: Statistics API (through nginx proxy on port 80) log_info "Testing statistics API through nginx proxy..." - local stats_response - stats_response=$(curl -f -s "http://${vm_ip}:80/api/v1/stats?token=local-dev-admin-token-12345" 2>/dev/null || echo "") # DevSkim: ignore DS137138 - if echo "${stats_response}" | grep -q '"torrents"'; then + if [[ $(test_http_endpoint "http://${vm_ip}:80/api/v1/stats?token=local-dev-admin-token-12345" '"torrents"') == "success" ]]; then # DevSkim: ignore DS137138 log_success "✓ Statistics API working" else - log_error "✗ Statistics API failed - Response: ${stats_response}" + log_error "✗ Statistics API failed" ((failed_tests++)) fi @@ -304,12 +287,10 @@ test_smoke_testing() { # Test 6: Direct tracker health check (port 1212) log_info "Testing direct tracker health check on port 1212..." - local direct_health - direct_health=$(curl -f -s http://"${vm_ip}":1212/api/health_check 2>/dev/null || echo "") - if echo "${direct_health}" | grep -q '"status":"Ok"'; then + if [[ $(test_http_endpoint "http://${vm_ip}:1212/api/health_check" '"status":"Ok"') == "success" ]]; then # DevSkim: ignore DS137138 log_success "✓ Direct tracker health check working" else - log_error "✗ Direct tracker health check failed - Response: ${direct_health}" + log_error "✗ Direct tracker health check failed" ((failed_tests++)) fi @@ -399,7 +380,7 @@ wait_for_vm_ip() { fi # Also check libvirt directly as fallback - vm_ip=$(virsh domifaddr torrust-tracker-demo 2>/dev/null | grep ipv4 | awk '{print $4}' | cut -d'/' -f1 || echo "") + vm_ip=$(get_vm_ip) if [[ -n "${vm_ip}" ]]; then log_success "VM IP assigned (via libvirt): ${vm_ip}" # Refresh terraform state to sync with actual VM state @@ -427,7 +408,7 @@ wait_for_cloud_init_to_finish() { local vm_ip="" # First get the VM IP - vm_ip=$(virsh domifaddr torrust-tracker-demo 2>/dev/null | grep ipv4 | awk '{print $4}' | cut -d'/' -f1 || echo "") + vm_ip=$(get_vm_ip) if [[ -z "${vm_ip}" ]]; then log_error "VM IP not available - cannot check readiness" return 1 @@ -439,7 +420,7 @@ wait_for_cloud_init_to_finish() { log_info "Checking cloud-init status (attempt ${attempt}/${max_attempts})..." # Check if SSH is available - if ! ssh -o ConnectTimeout=5 -o StrictHostKeyChecking=no torrust@"${vm_ip}" "echo 'SSH OK'" >/dev/null 2>&1; then + if ! ssh_to_vm "${vm_ip}" "echo 'SSH OK'"; then log_info "SSH not ready yet, waiting 10 seconds..." sleep 10 ((attempt++)) @@ -448,13 +429,13 @@ wait_for_cloud_init_to_finish() { # Check if cloud-init has finished local cloud_init_status - cloud_init_status=$(ssh -o ConnectTimeout=5 -o StrictHostKeyChecking=no torrust@"${vm_ip}" "cloud-init status" 2>/dev/null || echo "unknown") + cloud_init_status=$(ssh_to_vm "${vm_ip}" "cloud-init status" "2>/dev/null" || echo "unknown") if [[ "${cloud_init_status}" == *"done"* ]]; then log_success "Cloud-init completed successfully" # Check if Docker is available and working - if ssh -o ConnectTimeout=5 -o StrictHostKeyChecking=no torrust@"${vm_ip}" "docker --version && docker compose version" >/dev/null 2>&1; then + if ssh_to_vm "${vm_ip}" "docker --version && docker compose version"; then log_success "Docker is ready and available" log_success "VM is ready for application deployment" return 0 From 8ba68583659a17827f34cacf94f8ca07c0a754d9 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 28 Jul 2025 09:42:11 +0100 Subject: [PATCH 2/9] refactor: [#14] centralize log_section function and improve Docker Compose deployment - Move log_section function from duplicated implementations to scripts/shell-utils.sh - Remove duplicates from tests/test-e2e.sh, infrastructure/tests/test-ci.sh, infrastructure/tests/test-local.sh - Add vm_exec_with_timeout function to deploy-app.sh for robust long-running SSH commands - Split Docker Compose pull and up commands for better progress feedback and error isolation - Remove deprecated --include-deps flag from docker compose pull (now default behavior) - Add 10-minute timeout for Docker image pulls to prevent deployment hangs - Validate E2E and deployment workflows with refactored code - All CI tests and deployment health checks pass --- infrastructure/scripts/deploy-app.sh | 35 +++++++++++++++++++++++----- infrastructure/tests/test-ci.sh | 7 ------ infrastructure/tests/test-local.sh | 7 ------ scripts/shell-utils.sh | 9 +++++++ tests/test-e2e.sh | 7 ------ 5 files changed, 38 insertions(+), 27 deletions(-) diff --git a/infrastructure/scripts/deploy-app.sh b/infrastructure/scripts/deploy-app.sh index ef61532..12c275f 100755 --- a/infrastructure/scripts/deploy-app.sh +++ b/infrastructure/scripts/deploy-app.sh @@ -12,9 +12,6 @@ TERRAFORM_DIR="${PROJECT_ROOT}/infrastructure/terraform" # Default values ENVIRONMENT="${1:-local}" -# Get script configuration -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -PROJECT_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" VM_IP="${2:-}" SKIP_HEALTH_CHECK="${SKIP_HEALTH_CHECK:-false}" @@ -156,6 +153,24 @@ vm_exec() { fi } +# Execute command on VM via SSH with timeout +vm_exec_with_timeout() { + local vm_ip="$1" + local command="$2" + local timeout="${3:-300}" # Default 5 minutes + local description="${4:-}" + + if [[ -n "${description}" ]]; then + log_info "${description}" + fi + + # Use timeout command to limit execution time + if ! timeout "${timeout}" ssh -o StrictHostKeyChecking=no -o ConnectTimeout=30 torrust@"${vm_ip}" "${command}"; then + log_error "Failed to execute command on VM (timeout: ${timeout}s): ${command}" + exit 1 + fi +} + # RELEASE STAGE: Deploy application code and configuration release_stage() { local vm_ip="$1" @@ -382,12 +397,20 @@ run_stage() { fi " "Stopping existing services" - # Pull latest images and start services - vm_exec "${vm_ip}" " + # Pull latest images with timeout (10 minutes for large images) + log_info "Pulling Docker images (this may take several minutes for large images)..." + vm_exec_with_timeout "${vm_ip}" " cd /home/torrust/github/torrust/torrust-tracker-demo/application - # Pull latest images + # Pull images with progress output + echo 'Starting Docker image pull...' docker compose pull + echo 'Docker image pull completed' + " 600 "Pulling Docker images with 10-minute timeout" + + # Start services + vm_exec "${vm_ip}" " + cd /home/torrust/github/torrust/torrust-tracker-demo/application # Start services docker compose up -d diff --git a/infrastructure/tests/test-ci.sh b/infrastructure/tests/test-ci.sh index ef67253..29f926e 100755 --- a/infrastructure/tests/test-ci.sh +++ b/infrastructure/tests/test-ci.sh @@ -17,13 +17,6 @@ source "${PROJECT_ROOT}/scripts/shell-utils.sh" # Set log file for tee output export SHELL_UTILS_LOG_FILE="${TEST_LOG_FILE}" -log_section() { - log "" - log "${BLUE}===============================================${NC}" - log "${BLUE}$1${NC}" - log "${BLUE}===============================================${NC}" -} - # Initialize test log init_test_log() { init_log_file "${TEST_LOG_FILE}" "Torrust Tracker Demo - CI-Compatible Tests" diff --git a/infrastructure/tests/test-local.sh b/infrastructure/tests/test-local.sh index e26520e..78a6545 100755 --- a/infrastructure/tests/test-local.sh +++ b/infrastructure/tests/test-local.sh @@ -17,13 +17,6 @@ source "${PROJECT_ROOT}/scripts/shell-utils.sh" # Set log file for tee output export SHELL_UTILS_LOG_FILE="${TEST_LOG_FILE}" -log_section() { - log "" - log "${BLUE}===============================================${NC}" - log "${BLUE}$1${NC}" - log "${BLUE}===============================================${NC}" -} - # Initialize test log init_test_log() { init_log_file "${TEST_LOG_FILE}" "Torrust Tracker Demo - Local-Only Tests" diff --git a/scripts/shell-utils.sh b/scripts/shell-utils.sh index 26219ac..d7f6c61 100644 --- a/scripts/shell-utils.sh +++ b/scripts/shell-utils.sh @@ -14,6 +14,7 @@ # log_success "Operation completed successfully" # log_warning "This is a warning" # log_error "This is an error" +# log_section "Major Section Title" # # # Use HTTP testing: # result=$(test_http_endpoint "http://example.com" "expected content") @@ -78,6 +79,14 @@ log_trace() { fi } +# Section header logging - displays a prominent section separator +log_section() { + log "" + log "${BLUE}===============================================${NC}" + log "${BLUE}$1${NC}" + log "${BLUE}===============================================${NC}" +} + # Additional utility functions # Check if command exists diff --git a/tests/test-e2e.sh b/tests/test-e2e.sh index e4400a7..d1cf253 100755 --- a/tests/test-e2e.sh +++ b/tests/test-e2e.sh @@ -25,13 +25,6 @@ source "${PROJECT_ROOT}/scripts/shell-utils.sh" # Set log file for tee output export SHELL_UTILS_LOG_FILE="${TEST_LOG_FILE}" -log_section() { - log "" - log "${BLUE}===============================================${NC}" - log "${BLUE}$1${NC}" - log "${BLUE}===============================================${NC}" -} - # Helper function to get VM IP address from libvirt get_vm_ip() { virsh domifaddr torrust-tracker-demo 2>/dev/null | grep ipv4 | awk '{print $4}' | cut -d'/' -f1 || echo "" From 732a2976bf72bb1cde7e472dad3b1f58f042f1c2 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 28 Jul 2025 10:22:19 +0100 Subject: [PATCH 3/9] feat: [#14] improve cloud-init completion detection robustness - Replace Docker-based detection with multi-layered approach: 1. Primary: Official cloud-init status command 2. Secondary: Custom completion marker file 3. Tertiary: System service readiness checks - Add completion marker creation at end of cloud-init setup - Update E2E tests to use robust detection method - Update deployment scripts with same detection logic - Remove dependency on specific software for completion detection This makes cloud-init detection more reliable and future-proof, working regardless of command order or installed software. --- infrastructure/cloud-init/user-data.yaml.tpl | 5 ++ infrastructure/scripts/deploy-app.sh | 56 ++++++++++++-------- tests/test-e2e.sh | 35 ++++++++---- 3 files changed, 62 insertions(+), 34 deletions(-) diff --git a/infrastructure/cloud-init/user-data.yaml.tpl b/infrastructure/cloud-init/user-data.yaml.tpl index b34cb1c..8d812d7 100644 --- a/infrastructure/cloud-init/user-data.yaml.tpl +++ b/infrastructure/cloud-init/user-data.yaml.tpl @@ -191,6 +191,11 @@ runcmd: # Set up log rotation for Docker - systemctl restart docker + # Create completion marker for robust cloud-init status detection + # This file indicates that ALL cloud-init setup tasks have completed successfully + - echo "Cloud-init setup completed at $(date)" > /var/lib/cloud/torrust-setup-complete + - chmod 644 /var/lib/cloud/torrust-setup-complete + # Final message final_message: | Torrust Tracker Demo VM setup completed! diff --git a/infrastructure/scripts/deploy-app.sh b/infrastructure/scripts/deploy-app.sh index 12c275f..c855fea 100755 --- a/infrastructure/scripts/deploy-app.sh +++ b/infrastructure/scripts/deploy-app.sh @@ -73,39 +73,49 @@ test_ssh_connection() { exit 1 } -# Wait for cloud-init and Docker to be ready +# Wait for cloud-init to complete using robust detection method wait_for_system_ready() { local vm_ip="$1" local max_attempts=30 # 15 minutes (30 * 30 seconds) for cloud-init completion local attempt=1 - log_info "Waiting for system initialization (cloud-init and Docker) to complete..." + log_info "Waiting for cloud-init to complete using robust detection method..." while [[ ${attempt} -le ${max_attempts} ]]; do log_info "Checking system readiness (attempt ${attempt}/${max_attempts})..." - # Check if cloud-init is done + # Primary check: Official cloud-init status cloud_init_status=$(ssh -o StrictHostKeyChecking=no -o ConnectTimeout=10 "torrust@${vm_ip}" "cloud-init status" 2>/dev/null || echo "failed") if [[ "${cloud_init_status}" == *"done"* ]]; then log_info "Cloud-init completed: ${cloud_init_status}" - # Check if Docker is available - docker_available=$(ssh -o StrictHostKeyChecking=no -o ConnectTimeout=10 "torrust@${vm_ip}" "docker --version >/dev/null 2>&1 && echo 'available' || echo 'not-available'" 2>/dev/null || echo "not-available") + # Secondary check: Custom completion marker file + completion_marker_exists=$(ssh -o StrictHostKeyChecking=no -o ConnectTimeout=10 "torrust@${vm_ip}" "test -f /var/lib/cloud/torrust-setup-complete && echo 'exists' || echo 'not-exists'" 2>/dev/null || echo "not-exists") - if [[ "${docker_available}" == "available" ]]; then - # Check if Docker daemon is running - docker_running=$(ssh -o StrictHostKeyChecking=no -o ConnectTimeout=10 "torrust@${vm_ip}" "docker info >/dev/null 2>&1 && echo 'running' || echo 'not-running'" 2>/dev/null || echo "not-running") + if [[ "${completion_marker_exists}" == "exists" ]]; then + log_success "Setup completion marker found - all cloud-init tasks completed" + + # Tertiary check: Verify system services are ready (only if needed for deployment) + # Note: This check is deployment-specific, not cloud-init specific + systemd_ready=$(ssh -o StrictHostKeyChecking=no -o ConnectTimeout=10 "torrust@${vm_ip}" "systemctl is-system-running --quiet && echo 'ready' || echo 'not-ready'" 2>/dev/null || echo "not-ready") - if [[ "${docker_running}" == "running" ]]; then - log_success "System is ready: cloud-init done, Docker available and running" + if [[ "${systemd_ready}" == "ready" ]]; then + log_success "System is fully ready for application deployment" return 0 else - log_info "Docker installed but daemon not running yet, waiting..." + log_info "System services still starting up, waiting..." fi else - log_info "Docker not available yet, cloud-init may still be installing it..." + log_info "Setup completion marker not found yet, cloud-init tasks may still be running..." fi + elif [[ "${cloud_init_status}" == *"error"* ]]; then + log_error "Cloud-init failed with error status: ${cloud_init_status}" + + # Show detailed error information + detailed_status=$(ssh -o StrictHostKeyChecking=no -o ConnectTimeout=10 "torrust@${vm_ip}" "cloud-init status --long" 2>/dev/null || echo "unknown") + log_error "Detailed cloud-init status: ${detailed_status}" + return 1 else log_info "Cloud-init status: ${cloud_init_status}, waiting for completion..." fi @@ -116,22 +126,22 @@ wait_for_system_ready() { done log_error "Timeout waiting for system to be ready after ${max_attempts} attempts (15 minutes)" - log_error "Cloud-init may have failed or Docker installation encountered issues" + log_error "Cloud-init may have failed or system setup encountered issues" - # Show diagnostic information + # Show diagnostic information using robust detection methods vm_exec "${vm_ip}" " echo '=== System Diagnostic Information ===' echo 'Cloud-init status:' cloud-init status --long || echo 'cloud-init command failed' - echo '' - echo 'Docker version:' - docker --version || echo 'Docker not available' - echo '' - echo 'Docker service status:' - systemctl status docker || echo 'Docker service status unavailable' - echo '' - echo 'Recent cloud-init logs:' - tail -20 /var/log/cloud-init.log || echo 'Cloud-init logs unavailable' + echo + echo 'Setup completion marker:' + ls -la /var/lib/cloud/torrust-setup-complete 2>/dev/null || echo 'Completion marker not found' + echo + echo 'Cloud-init logs (last 20 lines):' + tail -20 /var/log/cloud-init.log 2>/dev/null || echo 'Cloud-init log not available' + echo + echo 'System service status:' + systemctl is-system-running || echo 'System status check failed' " "Dumping diagnostic information" exit 1 diff --git a/tests/test-e2e.sh b/tests/test-e2e.sh index d1cf253..55295e8 100755 --- a/tests/test-e2e.sh +++ b/tests/test-e2e.sh @@ -420,23 +420,36 @@ wait_for_cloud_init_to_finish() { continue fi - # Check if cloud-init has finished + # Primary check: Official cloud-init status local cloud_init_status cloud_init_status=$(ssh_to_vm "${vm_ip}" "cloud-init status" "2>/dev/null" || echo "unknown") if [[ "${cloud_init_status}" == *"done"* ]]; then - log_success "Cloud-init completed successfully" - - # Check if Docker is available and working - if ssh_to_vm "${vm_ip}" "docker --version && docker compose version"; then - log_success "Docker is ready and available" - log_success "VM is ready for application deployment" - return 0 + log_success "Cloud-init reports completion: ${cloud_init_status}" + + # Secondary check: Custom completion marker file + if ssh_to_vm "${vm_ip}" "test -f /var/lib/cloud/torrust-setup-complete"; then + log_success "Setup completion marker found" + + # Tertiary check: Verify critical services are available + # Note: This is not tied to specific software, just basic system readiness + if ssh_to_vm "${vm_ip}" "systemctl is-active docker >/dev/null 2>&1"; then + log_success "Critical services are active" + log_success "VM is ready for application deployment" + return 0 + else + log_info "Critical services not ready yet, waiting 10 seconds..." + fi else - log_info "Docker not ready yet, waiting 10 seconds..." + log_info "Setup completion marker not found yet, waiting 10 seconds..." fi elif [[ "${cloud_init_status}" == *"error"* ]]; then - log_error "Cloud-init failed with error status" + log_error "Cloud-init failed with error status: ${cloud_init_status}" + + # Try to get more detailed error information + local cloud_init_result + cloud_init_result=$(ssh_to_vm "${vm_ip}" "cloud-init status --long" "2>/dev/null" || echo "unknown") + log_error "Cloud-init detailed status: ${cloud_init_result}" return 1 else log_info "Cloud-init status: ${cloud_init_status}, waiting 10 seconds..." @@ -447,7 +460,7 @@ wait_for_cloud_init_to_finish() { done log_error "Timeout waiting for cloud-init to finish after $((max_attempts * 10)) seconds" - log_error "You can check manually with: ssh torrust@${vm_ip} 'cloud-init status'" + log_error "You can check manually with: ssh torrust@${vm_ip} 'cloud-init status --long'" return 1 } From 8c7591621034fbd6ff74581484be455eb429504f Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 28 Jul 2025 10:56:42 +0100 Subject: [PATCH 4/9] refactor: [#14] reorganize Makefile into infrastructure/application layers - Restructure Makefile into clear hierarchical layers (infra-, app-, vm-, test-, dev-) - Rename commands for better clarity: test -> test-e2e, test-syntax -> lint - Remove backward compatibility for legacy commands (apply, destroy, ssh, etc.) - Update help output with grouped commands by layer for better discoverability - Update all scripts, tests, and documentation to use new command structure - Implement twelve-factor app deployment workflow separation - Infrastructure provisioning: make infra-apply (platform setup) - Application deployment: make app-deploy (Build + Release + Run stages) - VM access and debugging: make vm-ssh, vm-console, vm-status - Testing: make test-e2e (comprehensive), make lint (syntax), make test-unit - Development workflows: make dev-deploy, make dev-test, make dev-clean Updated files: - Makefile: Complete restructure with new command organization - CI workflow: Updated to use make infra-test-ci - Documentation: Updated all references to use new command names - Test scripts: Updated to use new infra/app health check commands - E2E test: Updated to use make lint instead of make test-syntax This refactoring improves command discoverability, follows twelve-factor app principles, and provides clear separation between infrastructure and application concerns while maintaining all existing functionality. --- .github/copilot-instructions.md | 59 +++--- .github/workflows/testing.yml | 2 +- Makefile | 197 ++++++++++-------- README.md | 16 +- docs/guides/integration-testing-guide.md | 8 +- infrastructure/scripts/deploy-app.sh | 2 +- .../scripts/provision-infrastructure.sh | 2 +- infrastructure/tests/test-ci.sh | 6 +- infrastructure/tests/test-local.sh | 6 +- tests/README.md | 4 +- tests/test-e2e.sh | 12 +- 11 files changed, 165 insertions(+), 149 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 61e527a..d23eb17 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -132,30 +132,30 @@ cd torrust-tracker-demo make install-deps # 3. Setup SSH key for VMs -make setup-ssh-key +make infra-config-local # 4. Test twelve-factor deployment workflow locally make infra-apply # Provision infrastructure (platform setup) make app-deploy # Deploy application (Build + Release + Run stages) -make health-check # Validate deployment -make ssh # Connect to VM +make app-health-check # Validate deployment +make vm-ssh # Connect to VM make infra-destroy # Cleanup # 5. Run tests -make test # Full infrastructure test -make test-syntax # Syntax validation only +make test-e2e # Full infrastructure test +make lint # Syntax validation only ``` ### Main Commands #### Twelve-Factor Workflow (Recommended) -| Command | Purpose | -| ------------------- | ------------------------------------------------- | -| `make infra-apply` | Provision infrastructure (platform setup) | -| `make app-deploy` | Deploy application (Build + Release + Run stages) | -| `make app-redeploy` | Redeploy application (Release + Run stages only) | -| `make health-check` | Validate deployment health | +| Command | Purpose | +| ----------------------- | ------------------------------------------------- | +| `make infra-apply` | Provision infrastructure (platform setup) | +| `make app-deploy` | Deploy application (Build + Release + Run stages) | +| `make app-redeploy` | Redeploy application (Release + Run stages only) | +| `make app-health-check` | Validate deployment health | #### Infrastructure Management @@ -171,19 +171,18 @@ make test-syntax # Syntax validation only #### VM Access and Debugging -| Command | Purpose | -| ----------------- | --------------------------------- | -| `make ssh` | Connect to deployed VM | -| `make console` | Access VM console (text-based) | -| `make vm-console` | Access VM graphical console (GUI) | +| Command | Purpose | +| --------------------- | --------------------------------- | +| `make vm-ssh` | Connect to deployed VM | +| `make vm-console` | Access VM console (text-based) | +| `make vm-gui-console` | Access VM graphical console (GUI) | #### Testing and Validation -| Command | Purpose | -| ------------------ | --------------------------------------- | -| `make test` | Run complete infrastructure tests | -| `make test-syntax` | Run syntax validation only | -| `make lint` | Run all linting (alias for test-syntax) | +| Command | Purpose | +| ------------------ | --------------------------------- | +| `make test-e2e` | Run complete infrastructure tests | +| `make lint` | Run syntax validation only | #### Legacy Commands (Deprecated) @@ -245,7 +244,7 @@ The twelve-factor **Build, Release, Run** stages apply to the application deploy 1. **Initial Setup**: `make infra-apply` → `make app-deploy` 2. **Code Changes**: `make app-redeploy` (skips infrastructure) 3. **Infrastructure Changes**: `make infra-apply` → `make app-redeploy` -4. **Validation**: `make health-check` +4. **Validation**: `make app-health-check` 5. **Cleanup**: `make infra-destroy` ### Git Workflow @@ -408,8 +407,8 @@ The project implements intelligent sudo cache management to improve the user exp ```bash make install-deps # Install dependencies - make setup-ssh-key # Configure SSH access - make test-prereq # Verify setup + make infra-config-local # Configure SSH access + make infra-test-prereq # Verify setup ``` 3. **Install recommended VS Code extensions**: @@ -439,7 +438,7 @@ The project implements intelligent sudo cache management to improve the user exp ```bash make infra-apply # Deploy test VM make app-deploy # Deploy application - make ssh # Verify access + make vm-ssh # Verify access make infra-destroy # Clean up ``` @@ -448,7 +447,7 @@ The project implements intelligent sudo cache management to improve the user exp ### For Infrastructure Changes 1. **Local testing first**: Always test infrastructure changes locally -2. **Validate syntax**: Run `make test-syntax` before committing +2. **Validate syntax**: Run `make lint` before committing 3. **Document changes**: Update relevant documentation 4. **Test twelve-factor workflow**: Ensure both infrastructure provisioning and application deployment work ```bash @@ -482,7 +481,7 @@ Be mindful of the execution context for different types of commands. The project When executing commands on the remote VM, be aware of limitations with interactive sessions. -- **Problem**: The VS Code integrated terminal may not correctly handle commands that initiate a new interactive shell, such as `ssh torrust@` or `make ssh`. The connection may succeed, but subsequent commands sent to that shell may not execute as expected, and their output may not be captured. +- **Problem**: The VS Code integrated terminal may not correctly handle commands that initiate a new interactive shell, such as `ssh torrust@` or `make vm-ssh`. The connection may succeed, but subsequent commands sent to that shell may not execute as expected, and their output may not be captured. - **Solution**: Prefer executing commands non-interactively whenever possible. Instead of opening a persistent SSH session, pass the command directly to `ssh`. @@ -490,7 +489,7 @@ When executing commands on the remote VM, be aware of limitations with interacti ```bash # 1. Log in - make ssh + make vm-ssh # 2. Run command (this might fail or output won't be seen) df -H ``` @@ -569,7 +568,7 @@ This ensures that the command is executed and its output is returned to the prim **Pre-commit Testing Requirement**: ALWAYS run the CI test suite before committing any changes: ```bash -make test-ci +make infra-test-ci ``` This command runs all unit tests that don't require a virtual machine, including: @@ -581,7 +580,7 @@ This command runs all unit tests that don't require a virtual machine, including Only commit if all CI tests pass. If any tests fail, fix the issues before committing. -**Note**: End-to-end tests (`make test`) are excluded from pre-commit requirements due to their longer execution time (~5-8 minutes), but running them before pushing is strongly recommended for comprehensive validation. +**Note**: End-to-end tests (`make test-e2e`) are excluded from pre-commit requirements due to their longer execution time (~5-8 minutes), but running them before pushing is strongly recommended for comprehensive validation. **Best Practice**: Always ask "Would you like me to commit these changes?" before performing any git state-changing operations. diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index f185cb7..2da9daa 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -27,4 +27,4 @@ jobs: sudo ./install-opentofu.sh --install-method deb - name: Run CI test suite - run: make test-ci + run: make infra-test-ci diff --git a/Makefile b/Makefile index 21c3164..3f84681 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,12 @@ # Makefile for Torrust Tracker Demo - Twelve-Factor App Deployment -.PHONY: help install-deps lint test clean +.PHONY: help install-deps test-e2e lint test-unit clean .PHONY: infra-init infra-plan infra-apply infra-destroy infra-status infra-refresh-state -.PHONY: app-deploy app-redeploy health-check -.PHONY: ssh console vm-console -.PHONY: configure-local configure-production validate-config +.PHONY: infra-config-local infra-config-production infra-validate-config +.PHONY: infra-test-prereq infra-test-ci infra-test-local +.PHONY: app-deploy app-redeploy app-health-check +.PHONY: app-test-config app-test-containers app-test-services +.PHONY: vm-ssh vm-console vm-gui-console vm-clean-ssh vm-prepare-ssh vm-status +.PHONY: dev-setup dev-deploy dev-test dev-clean # Default variables VM_NAME ?= torrust-tracker-demo @@ -17,25 +20,31 @@ SCRIPTS_DIR = infrastructure/scripts help: ## Show this help message @echo "Torrust Tracker Demo - Twelve-Factor App Deployment" @echo "" - @echo "=== TWELVE-FACTOR DEPLOYMENT WORKFLOW ===" - @echo " 1. infra-apply - Provision infrastructure (platform setup)" - @echo " 2. app-deploy - Deploy application (Build + Release + Run stages)" - @echo " 3. health-check - Validate deployment" + @echo "🚀 QUICK DEVELOPMENT WORKFLOWS:" + @echo " dev-setup Complete development setup" + @echo " dev-deploy Full deployment workflow (infra + app)" + @echo " dev-test Quick validation (syntax + unit tests)" + @echo " dev-clean Complete cleanup" @echo "" - @echo "=== TESTING WORKFLOW ===" - @echo " 1. test-syntax - Fast syntax validation (30s)" - @echo " 2. test-unit - Unit tests without deployment (1-2min)" - @echo " 3. test-ci - CI-compatible tests (syntax + config + scripts)" - @echo " 4. test-local - Local-only tests (requires virtualization)" - @echo " 5. test - Full E2E test with deployment (5-8min)" + @echo "📋 INFRASTRUCTURE LAYER:" + @awk 'BEGIN {FS = ":.*?## "} /^infra-.*:.*?## / {printf " %-20s %s\n", $$1, $$2}' $(MAKEFILE_LIST) @echo "" - @echo "Available targets:" - @awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_-]+:.*?## / {printf " %-20s %s\n", $$1, $$2}' $(MAKEFILE_LIST) + @echo "🐳 APPLICATION LAYER:" + @awk 'BEGIN {FS = ":.*?## "} /^app-.*:.*?## / {printf " %-20s %s\n", $$1, $$2}' $(MAKEFILE_LIST) + @echo "" + @echo "🖥️ VM ACCESS:" + @awk 'BEGIN {FS = ":.*?## "} /^vm-.*:.*?## / {printf " %-20s %s\n", $$1, $$2}' $(MAKEFILE_LIST) + @echo "" + @echo "🧪 GLOBAL TESTING:" + @awk 'BEGIN {FS = ":.*?## "} /^test.*:.*?## / {printf " %-20s %s\n", $$1, $$2}' $(MAKEFILE_LIST) + @echo "" + @echo "⚙️ SYSTEM SETUP:" + @awk 'BEGIN {FS = ":.*?## "} /^(install-deps|clean).*:.*?## / {printf " %-20s %s\n", $$1, $$2}' $(MAKEFILE_LIST) @echo "" @echo "Examples:" + @echo " make dev-deploy ENVIRONMENT=local" @echo " make infra-apply ENVIRONMENT=local" @echo " make app-deploy ENVIRONMENT=local" - @echo " make health-check ENVIRONMENT=local" install-deps: ## Install required dependencies (Ubuntu/Debian) @echo "Installing dependencies..." @@ -46,7 +55,7 @@ install-deps: ## Install required dependencies (Ubuntu/Debian) @echo "Dependencies installed. Please log out and log back in for group changes to take effect." # ============================================================================= -# INFRASTRUCTURE PROVISIONING TARGETS (PLATFORM SETUP) +# INFRASTRUCTURE LAYER (PLATFORM SETUP & CONFIGURATION) # ============================================================================= infra-init: ## Initialize infrastructure (Terraform init) @@ -74,8 +83,32 @@ infra-refresh-state: ## Refresh Terraform state to detect IP changes @echo "Refreshing Terraform state..." @cd $(TERRAFORM_DIR) && tofu refresh +infra-config-local: ## Generate local environment configuration + @echo "Configuring local environment..." + $(SCRIPTS_DIR)/configure-env.sh local + +infra-config-production: ## Generate production environment configuration + @echo "Configuring production environment..." + $(SCRIPTS_DIR)/configure-env.sh production + +infra-validate-config: ## Validate configuration for all environments + @echo "Validating configuration..." + $(SCRIPTS_DIR)/validate-config.sh + +infra-test-prereq: ## Test system prerequisites for development + @echo "Testing prerequisites..." + $(INFRA_TESTS_DIR)/test-unit-infrastructure.sh vm-prereq + +infra-test-ci: ## Run infrastructure CI-compatible tests + @echo "Running infrastructure CI-compatible tests..." + $(INFRA_TESTS_DIR)/test-ci.sh + +infra-test-local: ## Run local-only infrastructure tests (requires virtualization) + @echo "Running local-only infrastructure tests..." + $(INFRA_TESTS_DIR)/test-local.sh + # ============================================================================= -# TWELVE-FACTOR APPLICATION TARGETS (BUILD + RELEASE + RUN STAGES) +# APPLICATION LAYER (BUILD + RELEASE + RUN STAGES) # ============================================================================= app-deploy: ## Deploy application (Twelve-Factor Build + Release + Run stages) @@ -86,15 +119,27 @@ app-redeploy: ## Redeploy application without infrastructure changes @echo "Redeploying application for $(ENVIRONMENT)..." $(SCRIPTS_DIR)/deploy-app.sh $(ENVIRONMENT) -health-check: ## Validate deployment health +app-health-check: ## Validate deployment health @echo "Running health check for $(ENVIRONMENT)..." $(SCRIPTS_DIR)/health-check.sh $(ENVIRONMENT) +app-test-config: ## Test application configuration + @echo "Testing application configuration..." + $(TESTS_DIR)/test-unit-application.sh config + +app-test-containers: ## Test application containers + @echo "Testing application containers..." + $(TESTS_DIR)/test-unit-application.sh containers + +app-test-services: ## Test application services + @echo "Testing application services..." + $(TESTS_DIR)/test-unit-application.sh services + # ============================================================================= # VM ACCESS AND DEBUGGING # ============================================================================= -ssh: ## SSH into the VM +vm-ssh: ## SSH into the VM @VM_IP=$$(cd $(TERRAFORM_DIR) && tofu output -raw vm_ip 2>/dev/null) && \ if [ -n "$$VM_IP" ] && [ "$$VM_IP" != "No IP assigned yet" ]; then \ echo "Connecting to VM: $$VM_IP"; \ @@ -104,47 +149,66 @@ ssh: ## SSH into the VM exit 1; \ fi -ssh-clean: ## Clean SSH known_hosts for VM (fixes host key verification warnings) +vm-clean-ssh: ## Clean SSH known_hosts for VM (fixes host key verification warnings) @echo "Cleaning SSH known_hosts for VM..." @$(SCRIPTS_DIR)/ssh-utils.sh clean -ssh-prepare: ## Clean SSH known_hosts and test connectivity +vm-prepare-ssh: ## Clean SSH known_hosts and test connectivity @echo "Preparing SSH connection to VM..." @$(SCRIPTS_DIR)/ssh-utils.sh prepare -console: ## Access VM console (text-based) +vm-console: ## Access VM console (text-based) @echo "Accessing VM console..." - @virsh console $(VM_NAME) || echo "VM console not accessible. Try 'make vm-console' for graphical console." + @virsh console $(VM_NAME) || echo "VM console not accessible. Try 'make vm-gui-console' for graphical console." -vm-console: ## Access VM graphical console (requires GUI) +vm-gui-console: ## Access VM graphical console (requires GUI) @echo "Opening graphical VM console..." @virt-viewer --connect qemu:///system $(VM_NAME) & +vm-status: ## Show detailed VM status + @echo "VM Status for $(VM_NAME):" + @echo "================================" + @virsh domstate $(VM_NAME) 2>/dev/null || echo "VM not found" + @virsh dominfo $(VM_NAME) 2>/dev/null | grep -E "(State|Memory|CPUs)" || true + @echo "" + @echo "Network Info:" + @virsh domifaddr $(VM_NAME) 2>/dev/null || echo "No network info available" + @echo "" + @echo "Terraform State:" + @cd $(TERRAFORM_DIR) && tofu output 2>/dev/null || echo "No Terraform state found" + # ============================================================================= -# CONFIGURATION MANAGEMENT +# QUICK DEVELOPMENT WORKFLOWS # ============================================================================= -configure-local: ## Generate local environment configuration - @echo "Configuring local environment..." - $(SCRIPTS_DIR)/configure-env.sh local +dev-setup: ## Complete development setup + @echo "Setting up development environment..." + @make install-deps -configure-production: ## Generate production environment configuration - @echo "Configuring production environment..." - $(SCRIPTS_DIR)/configure-env.sh production +dev-deploy: ## Full deployment workflow (infra + app) + @echo "Running full deployment workflow for $(ENVIRONMENT)..." + @make infra-apply ENVIRONMENT=$(ENVIRONMENT) + @make app-deploy ENVIRONMENT=$(ENVIRONMENT) + @make app-health-check ENVIRONMENT=$(ENVIRONMENT) + @echo "✅ Development deployment complete" -validate-config: ## Validate configuration for all environments - @echo "Validating configuration..." - $(SCRIPTS_DIR)/validate-config.sh +dev-test: ## Quick validation (syntax + unit tests) + @echo "Running quick validation tests..." + @make lint + @make test-unit + @echo "✅ Quick tests passed" + +dev-clean: ## Complete cleanup + @echo "Cleaning up development environment..." + @make infra-destroy ENVIRONMENT=$(ENVIRONMENT) || true + @make clean + @echo "✅ Development environment cleaned" # ============================================================================= -# TESTING AND QUALITY ASSURANCE +# GLOBAL TESTING AND QUALITY ASSURANCE # ============================================================================= -test-prereq: ## Test system prerequisites for development - @echo "Testing prerequisites..." - $(INFRA_TESTS_DIR)/test-unit-infrastructure.sh vm-prereq - -test: ## Run comprehensive end-to-end test (follows integration guide) +test-e2e: ## Run comprehensive end-to-end test (follows integration guide) @echo "Running comprehensive end-to-end test..." $(TESTS_DIR)/test-e2e.sh $(ENVIRONMENT) @@ -155,59 +219,12 @@ test-unit: ## Run unit tests (configuration, scripts, syntax) @echo "2. Infrastructure scripts validation..." $(INFRA_TESTS_DIR)/test-unit-scripts.sh -test-syntax: ## Run syntax validation only +lint: ## Run syntax validation only @echo "Running syntax validation..." ./scripts/lint.sh -test-ci: ## Run CI-compatible tests (syntax + config + scripts) - @echo "Running CI-compatible tests..." - $(INFRA_TESTS_DIR)/test-ci.sh - -test-local: ## Run local-only tests (requires virtualization) - @echo "Running local-only tests..." - $(INFRA_TESTS_DIR)/test-local.sh - -test-legacy: ## [DEPRECATED] Legacy test scripts have been removed - @echo "⚠️ DEPRECATED: Legacy test scripts have been removed" - @echo "Use 'make test-unit' for unit tests or 'make test' for E2E tests" - @exit 1 - -lint: test-syntax ## Run all linting (alias for test-syntax) - clean: ## Clean up temporary files and caches @echo "Cleaning up..." @rm -rf $(TERRAFORM_DIR)/.terraform @rm -f $(TERRAFORM_DIR)/terraform.tfstate.backup @echo "Clean completed" - -# ============================================================================= -# LEGACY COMPATIBILITY (DEPRECATED) -# ============================================================================= - -# These targets are maintained for backward compatibility but are deprecated -# Use the twelve-factor targets above instead - -init: infra-init ## [DEPRECATED] Use infra-init instead - @echo "⚠️ DEPRECATED: Use 'make infra-init' instead" - -plan: infra-plan ## [DEPRECATED] Use infra-plan instead - @echo "⚠️ DEPRECATED: Use 'make infra-plan' instead" - -apply: ## [DEPRECATED] Use infra-apply + app-deploy instead - @echo "⚠️ DEPRECATED: This target combines infrastructure and application deployment" - @echo " For twelve-factor compliance, use:" - @echo " 1. make infra-apply ENVIRONMENT=$(ENVIRONMENT)" - @echo " 2. make app-deploy ENVIRONMENT=$(ENVIRONMENT)" - @echo "" - @echo "Proceeding with legacy deployment..." - @make infra-apply ENVIRONMENT=$(ENVIRONMENT) - @make app-deploy ENVIRONMENT=$(ENVIRONMENT) - -destroy: infra-destroy ## [DEPRECATED] Use infra-destroy instead - @echo "⚠️ DEPRECATED: Use 'make infra-destroy' instead" - -status: infra-status ## [DEPRECATED] Use infra-status instead - @echo "⚠️ DEPRECATED: Use 'make infra-status' instead" - -refresh-state: infra-refresh-state ## [DEPRECATED] Use infra-refresh-state instead - @echo "⚠️ DEPRECATED: Use 'make infra-refresh-state' instead" diff --git a/README.md b/README.md index 15cf19d..b6a2f3a 100644 --- a/README.md +++ b/README.md @@ -78,14 +78,14 @@ make dev-setup # Log out and log back in for group permissions # 2. Configure SSH key -make setup-ssh-key +make infra-config-local # Edit infrastructure/terraform/local.tfvars with your SSH public key # 3. Deploy VM and application -make apply # Deploy VM -make ssh # Access VM +make infra-apply # Deploy VM +make vm-ssh # Access VM docker compose -f application/compose.yaml up -d # Deploy application -make destroy # Clean up +make infra-destroy # Clean up ``` ## 📚 Documentation @@ -142,10 +142,10 @@ The project uses automated linting to ensure code quality and consistency: # Run all linting checks make lint -# Run individual linters -make lint-yaml # YAML files (yamllint) -make lint-shell # Shell scripts (shellcheck) -make lint-markdown # Markdown files (markdownlint) +# Run individual linters (use script directly) +./scripts/lint.sh --yaml # YAML files (yamllint) +./scripts/lint.sh --shell # Shell scripts (shellcheck) +./scripts/lint.sh --markdown # Markdown files (markdownlint) ``` **Required tools:** diff --git a/docs/guides/integration-testing-guide.md b/docs/guides/integration-testing-guide.md index c4e7718..2eeccef 100644 --- a/docs/guides/integration-testing-guide.md +++ b/docs/guides/integration-testing-guide.md @@ -10,7 +10,7 @@ application concerns: 1. **Infrastructure Provisioning**: Setting up the platform (`make infra-apply`) 2. **Application Deployment**: Twelve-factor Build + Release + Run stages (`make app-deploy`) -3. **Validation**: Health checking (`make health-check`) +3. **Validation**: Health checking (`make app-health-check`) 4. **Cleanup**: Resource management (`make infra-destroy`) The new workflow separates infrastructure provisioning from application deployment, @@ -34,7 +34,7 @@ The automated test script (`tests/test-e2e.sh`) follows the same steps described - **Step 1**: Prerequisites validation - **Step 2**: Infrastructure provisioning (`make infra-apply`) - **Step 3**: Application deployment (`make app-deploy`) -- **Step 4**: Health validation (`make health-check`) +- **Step 4**: Health validation (`make app-health-check`) - **Step 5**: Smoke testing (basic functionality validation) - **Step 6**: Cleanup (`make infra-destroy`) @@ -184,7 +184,7 @@ Provisioning infrastructure for local... make infra-status ENVIRONMENT=local # [PROJECT_ROOT] Test SSH connectivity -make ssh +make vm-ssh # (type 'exit' to return) ``` @@ -260,7 +260,7 @@ make infra-status ENVIRONMENT=local ```bash # [PROJECT_ROOT] Run full health validation -time make health-check ENVIRONMENT=local +time make app-health-check ENVIRONMENT=local ``` **Expected Output**: diff --git a/infrastructure/scripts/deploy-app.sh b/infrastructure/scripts/deploy-app.sh index c855fea..2c63658 100755 --- a/infrastructure/scripts/deploy-app.sh +++ b/infrastructure/scripts/deploy-app.sh @@ -522,7 +522,7 @@ show_connection_info() { echo "Grafana: http://${vm_ip}:3100 (admin/admin)" # DevSkim: ignore DS137138 echo echo "=== NEXT STEPS ===" - echo "Health Check: make health-check ENVIRONMENT=${ENVIRONMENT}" + echo "Health Check: make app-health-check ENVIRONMENT=${ENVIRONMENT}" echo "View Logs: ssh torrust@${vm_ip} 'cd torrust-tracker-demo/application && docker compose logs'" echo "Stop Services: ssh torrust@${vm_ip} 'cd torrust-tracker-demo/application && docker compose down'" echo diff --git a/infrastructure/scripts/provision-infrastructure.sh b/infrastructure/scripts/provision-infrastructure.sh index 75a4a2b..2241e68 100755 --- a/infrastructure/scripts/provision-infrastructure.sh +++ b/infrastructure/scripts/provision-infrastructure.sh @@ -132,7 +132,7 @@ provision_infrastructure() { log_info "Next step: make app-deploy ENVIRONMENT=${ENVIRONMENT}" else log_warning "Infrastructure provisioned but VM IP not available yet" - log_info "Try: make status to check VM IP" + log_info "Try: make infra-status to check VM IP" fi ;; "destroy") diff --git a/infrastructure/tests/test-ci.sh b/infrastructure/tests/test-ci.sh index 29f926e..44ccd73 100755 --- a/infrastructure/tests/test-ci.sh +++ b/infrastructure/tests/test-ci.sh @@ -37,7 +37,7 @@ show_test_summary() { log "" log_info "Next steps for full validation:" log_info " 1. Run 'make test-local' on a system with virtualization" - log_info " 2. Run 'make test' for full end-to-end testing" + log_info " 2. Run 'make test-e2e' for full end-to-end testing" log "" log_info "Test log saved to: ${TEST_LOG_FILE}" } @@ -57,7 +57,7 @@ main() { # Test 1: Syntax validation (fast) log_section "TEST 1: SYNTAX VALIDATION" log_info "Running syntax validation..." - if ! make test-syntax; then + if ! make lint; then log_error "Syntax validation failed" exit 1 fi @@ -84,7 +84,7 @@ main() { # Test 4: Makefile validation log_section "TEST 4: MAKEFILE VALIDATION" log_info "Validating Makefile targets..." - if ! make validate-config 2>/dev/null; then + if ! make infra-validate-config 2>/dev/null; then log_warning "Makefile validation script not found (optional)" else log_success "Makefile validation passed" diff --git a/infrastructure/tests/test-local.sh b/infrastructure/tests/test-local.sh index 78a6545..fae1011 100755 --- a/infrastructure/tests/test-local.sh +++ b/infrastructure/tests/test-local.sh @@ -28,7 +28,7 @@ check_ci_environment() { if [ "${CI:-}" = "true" ] || [ "${GITHUB_ACTIONS:-}" = "true" ]; then log_error "Local-only tests detected CI environment" log_error "These tests require virtualization support and cannot run in CI" - log_error "Use 'make test-ci' for CI-compatible tests" + log_error "Use 'make infra-test-ci' for CI-compatible tests" exit 1 fi } @@ -77,7 +77,7 @@ show_test_summary() { log_info "Total local tests completed in ${duration} seconds" log_success "All local-only tests passed!" log "" - log_info "For full end-to-end testing, run: make test" + log_info "For full end-to-end testing, run: make test-e2e" log "" log_info "Test log saved to: ${TEST_LOG_FILE}" } @@ -115,7 +115,7 @@ main() { # Test 3: Optional - Quick infrastructure validation (without full deployment) log_section "INFRASTRUCTURE VALIDATION" log_info "Running infrastructure validation without deployment..." - if ! make test-prereq; then + if ! make infra-test-prereq; then log_warning "Infrastructure validation had warnings (this is usually OK)" else log_success "Infrastructure validation passed" diff --git a/tests/README.md b/tests/README.md index bb19aaa..bb3ee63 100644 --- a/tests/README.md +++ b/tests/README.md @@ -31,7 +31,7 @@ and application layers. 1. **Complete Deployment Workflow** (`test-e2e.sh`) - Infrastructure provisioning (`make infra-apply`) - Application deployment (`make app-deploy`) - - Health validation (`make health-check`) + - Health validation (`make app-health-check`) - Complete system integration - Duration: ~5-8 minutes @@ -112,7 +112,7 @@ The project tests orchestrate and validate the integration between the other lay 1. **Prerequisites Validation** - Validates system requirements 2. **Infrastructure Provisioning** - Deploys VM using `make infra-apply` 3. **Application Deployment** - Deploys tracker using `make app-deploy` -4. **Health Validation** - Validates all services using `make health-check` +4. **Health Validation** - Validates all services using `make app-health-check` 5. **Cleanup** - Destroys infrastructure using `make infra-destroy` **Output**: Generates detailed log at `/tmp/torrust-e2e-test.log` diff --git a/tests/test-e2e.sh b/tests/test-e2e.sh index 55295e8..a9021ac 100755 --- a/tests/test-e2e.sh +++ b/tests/test-e2e.sh @@ -6,7 +6,7 @@ # 1. Prerequisites validation # 2. Infrastructure provisioning (make infra-apply) # 3. Application deployment (make app-deploy) -# 4. Health validation (make health-check) +# 4. Health validation (make app-health-check) # 5. Cleanup (make infra-destroy) set -euo pipefail @@ -76,7 +76,7 @@ test_prerequisites() { cd "${PROJECT_ROOT}" - if ! make test-syntax; then + if ! make lint; then log_error "Prerequisites validation failed" return 1 fi @@ -168,7 +168,7 @@ test_health_validation() { # Run health check (Step 3.2 from guide) log_info "Running comprehensive health check..." - if ! make health-check ENVIRONMENT="${ENVIRONMENT}"; then + if ! make app-health-check ENVIRONMENT="${ENVIRONMENT}"; then log_error "Health check failed" return 1 fi @@ -515,7 +515,7 @@ run_e2e_test() { # Final result if [[ ${failed} -eq 0 ]]; then log_section "TEST RESULT: SUCCESS" - log_success "End-to-end twelve-factor deployment test passed!" + log_success "End-to-end test passed!" log_success "Total test time: ${minutes}m ${seconds}s" log_info "Test log: ${TEST_LOG_FILE}" return 0 @@ -551,10 +551,10 @@ Examples: SKIP_CONFIRMATION=true $0 local # Test without confirmation prompt Test Steps (following integration testing guide): - 1. Prerequisites validation (make test-syntax) + 1. Prerequisites validation (make lint) 2. Infrastructure provisioning (make infra-apply + VM readiness wait) 3. Application deployment (make app-deploy) - 4. Health validation (make health-check + endpoint testing) + 4. Health validation (make app-health-check + endpoint testing) 5. Smoke testing (mandatory tracker functionality validation) 6. Cleanup (make infra-destroy) From d3fe01e0f140153ac43cc4e81ab544d8bf88d4f7 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 28 Jul 2025 11:00:01 +0100 Subject: [PATCH 5/9] docs: [#14] fix table formatting in copilot instructions - Align table columns properly for Testing and Validation commands - Improve readability of command reference table --- .github/copilot-instructions.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d23eb17..b793644 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -179,10 +179,10 @@ make lint # Syntax validation only #### Testing and Validation -| Command | Purpose | -| ------------------ | --------------------------------- | -| `make test-e2e` | Run complete infrastructure tests | -| `make lint` | Run syntax validation only | +| Command | Purpose | +| --------------- | --------------------------------- | +| `make test-e2e` | Run complete infrastructure tests | +| `make lint` | Run syntax validation only | #### Legacy Commands (Deprecated) From db45c9431625f0a68bc1fa7d53031194d04b9ac9 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 28 Jul 2025 11:22:33 +0100 Subject: [PATCH 6/9] refactor: [#14] separate testing concerns across three-layer architecture - Refactor infrastructure/tests/test-ci.sh to only test infrastructure concerns - Remove global 'make lint' calls from infrastructure layer - Create new application/tests/test-ci.sh for application-only testing - Create new tests/test-ci.sh for project-wide orchestration - Update Makefile with proper layer separation (infra-test-ci, app-test-ci, test-ci) - Update GitHub workflow to use 'make test-ci' for complete validation - Fix application tests to be CI-friendly (optional environment files) - Add comprehensive documentation in .github/copilot-instructions.md - Document three-layer testing architecture with clear boundaries - Add AI assistant guidelines to prevent future violations Testing Hierarchy: - make test-ci: Project-wide orchestrator (global + infra + app) - make infra-test-ci: Infrastructure layer only - make app-test-ci: Application layer only Prevents mixing concerns like calling global commands from layer-specific tests. --- .github/copilot-instructions.md | 81 ++++++++++++++++++ .github/workflows/testing.yml | 2 +- Makefile | 26 +++++- application/tests/test-ci.sh | 72 ++++++++++++++++ application/tests/test-unit-application.sh | 22 ++++- .../copilot-instructions-testing-update.md | 45 ++++++++++ .../testing-layer-separation-summary.md | 82 +++++++++++++++++++ infrastructure/tests/test-ci.sh | 64 ++++++--------- tests/test-ci.sh | 77 +++++++++++++++++ 9 files changed, 424 insertions(+), 47 deletions(-) create mode 100755 application/tests/test-ci.sh create mode 100644 docs/refactoring/copilot-instructions-testing-update.md create mode 100644 docs/refactoring/testing-layer-separation-summary.md create mode 100755 tests/test-ci.sh diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b793644..57c3b63 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -348,6 +348,66 @@ The project includes a comprehensive linting script that validates all file type - **No secrets**: Never commit SSH keys, passwords, or tokens - **Documentation**: Update docs for any infrastructure changes +#### Three-Layer Testing Architecture + +**CRITICAL**: This project uses a strict three-layer testing architecture. +**Never mix concerns across layers**: + +**1. Project-Wide/Global Layer** (`tests/` folder) + +- **Command**: `make test-ci` +- **Scope**: Cross-cutting concerns that span all layers +- **Responsibilities**: + - Global syntax validation (`make lint` / `./scripts/lint.sh`) + - Project structure validation (`tests/test-unit-project.sh`) + - Makefile validation + - Orchestrates infrastructure and application layer tests + +**2. Infrastructure Layer** (`infrastructure/` folder) + +- **Command**: `make infra-test-ci` +- **Scope**: Infrastructure-specific validation ONLY +- **Responsibilities**: + - Terraform/OpenTofu syntax validation + - Cloud-init template validation + - Infrastructure script validation + - Infrastructure configuration validation +- **NEVER**: Call global commands like `make lint` from infrastructure tests + +**3. Application Layer** (`application/` folder) + +- **Command**: `make app-test-ci` +- **Scope**: Application-specific validation ONLY +- **Responsibilities**: + - Docker Compose syntax validation + - Application configuration validation + - Deployment script validation + - Grafana dashboard validation +- **NEVER**: Call global commands like `make lint` from application tests + +**Testing Hierarchy:** + +```text +make test-ci (Project-wide orchestrator) +├── Global concerns (syntax, structure, Makefile) +├── make infra-test-ci (Infrastructure layer only) +└── make app-test-ci (Application layer only) +``` + +**Common Mistake to Avoid:** + +- ❌ Adding `make lint` calls inside `infrastructure/tests/test-ci.sh` +- ❌ Testing application concerns from infrastructure tests +- ❌ Testing infrastructure concerns from application tests +- ✅ Keep each layer focused on its own responsibilities only +- ✅ Use `make test-ci` for complete validation that orchestrates all layers + +**GitHub Actions Integration:** + +- Uses `make test-ci` (not `make infra-test-ci`) to ensure all layers are tested +- Runs without virtualization (CI-compatible) +- Maintains separation of concerns for maintainable testing + #### End-to-End Smoke Testing For verifying the functionality of the tracker from an end-user's perspective (e.g., simulating announce/scrape requests), refer to the **Smoke Testing Guide**. This guide explains how to use the official `torrust-tracker-client` tools to perform black-box testing against a running tracker instance without needing a full BitTorrent client. @@ -466,6 +526,27 @@ When providing assistance: - Test infrastructure changes locally before suggesting them - Provide clear explanations and documentation - Consider the migration to Hetzner infrastructure in suggestions +- **CRITICAL**: Respect the three-layer testing architecture (see Testing Requirements above) + +#### Testing Layer Separation (CRITICAL) + +**NEVER** mix testing concerns across the three layers: + +- **Infrastructure tests** (`infrastructure/tests/`) should ONLY test infrastructure concerns +- **Application tests** (`application/tests/`) should ONLY test application concerns +- **Global tests** (`tests/`) handle cross-cutting concerns and orchestration + +**Common violations to avoid:** + +- ❌ Adding `make lint` calls in `infrastructure/tests/test-ci.sh` +- ❌ Testing Docker Compose from infrastructure layer +- ❌ Testing Terraform from application layer +- ❌ Any layer calling commands from other layers directly + +**Always use the proper orchestrator:** + +- Use `make test-ci` for complete testing (orchestrates all layers) +- Use layer-specific commands only when targeting that specific layer #### Command Execution Context diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 2da9daa..f185cb7 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -27,4 +27,4 @@ jobs: sudo ./install-opentofu.sh --install-method deb - name: Run CI test suite - run: make infra-test-ci + run: make test-ci diff --git a/Makefile b/Makefile index 3f84681..c5d46fb 100644 --- a/Makefile +++ b/Makefile @@ -35,8 +35,10 @@ help: ## Show this help message @echo "🖥️ VM ACCESS:" @awk 'BEGIN {FS = ":.*?## "} /^vm-.*:.*?## / {printf " %-20s %s\n", $$1, $$2}' $(MAKEFILE_LIST) @echo "" - @echo "🧪 GLOBAL TESTING:" + @echo "🧪 TESTING (3-LAYER ARCHITECTURE):" @awk 'BEGIN {FS = ":.*?## "} /^test.*:.*?## / {printf " %-20s %s\n", $$1, $$2}' $(MAKEFILE_LIST) + @awk 'BEGIN {FS = ":.*?## "} /^infra-test.*:.*?## / {printf " %-20s %s\n", $$1, $$2}' $(MAKEFILE_LIST) + @awk 'BEGIN {FS = ":.*?## "} /^app-test.*:.*?## / {printf " %-20s %s\n", $$1, $$2}' $(MAKEFILE_LIST) @echo "" @echo "⚙️ SYSTEM SETUP:" @awk 'BEGIN {FS = ":.*?## "} /^(install-deps|clean).*:.*?## / {printf " %-20s %s\n", $$1, $$2}' $(MAKEFILE_LIST) @@ -99,9 +101,10 @@ infra-test-prereq: ## Test system prerequisites for development @echo "Testing prerequisites..." $(INFRA_TESTS_DIR)/test-unit-infrastructure.sh vm-prereq -infra-test-ci: ## Run infrastructure CI-compatible tests - @echo "Running infrastructure CI-compatible tests..." - $(INFRA_TESTS_DIR)/test-ci.sh +infra-test-ci: ## Run infrastructure-only CI tests (no global concerns) + @echo "Running infrastructure-only CI tests..." + $(INFRA_TESTS_DIR)/test-unit-config.sh + $(INFRA_TESTS_DIR)/test-unit-scripts.sh infra-test-local: ## Run local-only infrastructure tests (requires virtualization) @echo "Running local-only infrastructure tests..." @@ -135,6 +138,10 @@ app-test-services: ## Test application services @echo "Testing application services..." $(TESTS_DIR)/test-unit-application.sh services +app-test-ci: ## Run application-only CI tests (no global concerns) + @echo "Running application-only CI tests..." + application/tests/test-ci.sh + # ============================================================================= # VM ACCESS AND DEBUGGING # ============================================================================= @@ -212,6 +219,16 @@ test-e2e: ## Run comprehensive end-to-end test (follows integration guide) @echo "Running comprehensive end-to-end test..." $(TESTS_DIR)/test-e2e.sh $(ENVIRONMENT) +test-ci: ## Run project-wide CI tests (global concerns) + @echo "Running project-wide CI tests..." + @echo "1. Global concerns (syntax, structure, Makefile)..." + tests/test-ci.sh + @echo "2. Infrastructure layer tests..." + @make infra-test-ci + @echo "3. Application layer tests..." + @make app-test-ci + @echo "✅ All CI tests passed!" + test-unit: ## Run unit tests (configuration, scripts, syntax) @echo "Running unit tests..." @echo "1. Configuration and syntax validation..." @@ -228,3 +245,4 @@ clean: ## Clean up temporary files and caches @rm -rf $(TERRAFORM_DIR)/.terraform @rm -f $(TERRAFORM_DIR)/terraform.tfstate.backup @echo "Clean completed" + diff --git a/application/tests/test-ci.sh b/application/tests/test-ci.sh new file mode 100755 index 0000000..2180416 --- /dev/null +++ b/application/tests/test-ci.sh @@ -0,0 +1,72 @@ +#!/bin/bash +# Application-only CI tests - Run application-specific tests that work in GitHub runners +# Focus: Docker Compose validation, application configuration, deployment scripts +# Scope: No virtualization, no global repo concerns, application layer only + +set -euo pipefail + +# Configuration +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +APPLICATION_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)" +PROJECT_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" +TEST_LOG_FILE="/tmp/torrust-application-ci-test.log" + +# Source shared shell utilities +# shellcheck source=../../scripts/shell-utils.sh +source "${PROJECT_ROOT}/scripts/shell-utils.sh" + +# Set log file for tee output +export SHELL_UTILS_LOG_FILE="${TEST_LOG_FILE}" + +# Initialize test log +init_test_log() { + init_log_file "${TEST_LOG_FILE}" "Application-only CI Tests" + log_info "Environment: CI (no deployment)" + log_info "Scope: Application layer only" + log_info "Application Root: ${APPLICATION_ROOT}" +} + +# Test execution summary +show_test_summary() { + local start_time=$1 + local end_time + local duration + end_time=$(date +%s) + duration=$((end_time - start_time)) + + log_section "APPLICATION CI TEST SUMMARY" + log_info "Application CI tests completed in ${duration} seconds" + log_success "All application-specific tests passed!" + log "" + log_info "Note: This only validates application layer concerns." + log_info "Run 'make test-ci' for complete project validation." + log "" + log_info "Test log saved to: ${TEST_LOG_FILE}" +} + +# Main test execution +main() { + local test_start_time + test_start_time=$(date +%s) + + init_test_log + + log_section "APPLICATION-ONLY CI TESTS" + log_info "Running application-specific tests for GitHub runners" + + cd "${PROJECT_ROOT}" + + # Test 1: Application unit tests + log_section "TEST 1: APPLICATION CONFIGURATION & CONTAINERS" + log_info "Running application unit tests..." + if ! "${APPLICATION_ROOT}/tests/test-unit-application.sh"; then + log_error "Application unit tests failed" + exit 1 + fi + log_success "Application unit tests passed" + + show_test_summary "${test_start_time}" +} + +# Run main function +main "$@" diff --git a/application/tests/test-unit-application.sh b/application/tests/test-unit-application.sh index 609c2d6..9c9355e 100755 --- a/application/tests/test-unit-application.sh +++ b/application/tests/test-unit-application.sh @@ -59,12 +59,19 @@ test_application_config() { local failed=0 local config_files=( - ".env.production" "compose.yaml" ) + # Optional files (for development/local testing) + local optional_files=( + ".env" + ".env.production" + ".env.local" + ) + cd "${APPLICATION_ROOT}" + # Check required configuration files for config_file in "${config_files[@]}"; do if [[ ! -f "${config_file}" ]]; then log_error "Required configuration file missing: ${config_file}" @@ -74,6 +81,19 @@ test_application_config() { fi done + # Check optional configuration files (info only, no failure) + local env_file_found=false + for config_file in "${optional_files[@]}"; do + if [[ -f "${config_file}" ]]; then + log_info "Found optional configuration file: ${config_file}" + env_file_found=true + fi + done + + if [[ "$env_file_found" = false ]]; then + log_info "No environment files found (normal for CI, generated during deployment)" + fi + # Test that configuration templates exist local template_dir="${APPLICATION_ROOT}/config/templates" if [[ -d "${template_dir}" ]]; then diff --git a/docs/refactoring/copilot-instructions-testing-update.md b/docs/refactoring/copilot-instructions-testing-update.md new file mode 100644 index 0000000..7e33e5b --- /dev/null +++ b/docs/refactoring/copilot-instructions-testing-update.md @@ -0,0 +1,45 @@ +# GitHub Copilot Instructions Update Summary + +## Changes Made + +Updated `.github/copilot-instructions.md` to include comprehensive guidance on the three-layer +testing architecture to prevent future violations of separation of concerns. + +### 1. New Section: "Three-Layer Testing Architecture" + +Added a detailed section under "Testing Requirements" that explains: + +- **Critical importance** of maintaining layer separation +- **Definition of each layer**: + - Project-Wide/Global Layer (`tests/` folder) + - Infrastructure Layer (`infrastructure/` folder) + - Application Layer (`application/` folder) +- **Specific responsibilities** for each layer +- **Clear hierarchy** showing orchestration relationships +- **Common mistakes to avoid** with explicit examples +- **GitHub Actions integration** guidance + +### 2. Enhanced AI Assistant Guidelines + +Added a specific "Testing Layer Separation (CRITICAL)" section in the AI assistants guidance that: + +- **Reinforces the architecture** with strong warnings +- **Lists common violations** that should never happen +- **Provides clear guidance** on proper command usage +- **Emphasizes orchestration** through `make test-ci` + +## Key Benefits + +1. **Prevents regression** - Future contributors and AI assistants will understand the architecture +2. **Clear guidance** - Explicit examples of what NOT to do +3. **Proper orchestration** - Emphasizes using `make test-ci` for complete testing +4. **Maintainable architecture** - Each layer stays focused on its responsibilities + +## Impact + +This documentation update ensures that the three-layer testing architecture violation we just +fixed will not happen again. Contributors and AI assistants now have clear, explicit guidance +on maintaining proper separation of concerns in the testing infrastructure. + +The documentation is strategically placed in both the general testing requirements and the AI +assistant-specific guidelines to ensure maximum visibility and adherence to the architecture. diff --git a/docs/refactoring/testing-layer-separation-summary.md b/docs/refactoring/testing-layer-separation-summary.md new file mode 100644 index 0000000..ad1cca8 --- /dev/null +++ b/docs/refactoring/testing-layer-separation-summary.md @@ -0,0 +1,82 @@ +# Testing Layer Separation - Summary + +## Problem + +The original `make infra-test-ci` command (which ran `infrastructure/tests/test-ci.sh`) was +mixing concerns across the three project layers: + +1. **Infrastructure layer** - `infrastructure/` folder +2. **Application layer** - `application/` folder +3. **Project/Global layer** - Cross-cutting concerns + +The `infrastructure/tests/test-ci.sh` script was calling `make lint` (a global command) and +performing other project-wide validations, violating the separation of concerns. + +## Solution + +Implemented proper three-layer separation for CI testing: + +### 1. Project-Wide CI Tests (`make test-ci`) + +- **Script**: `tests/test-ci.sh` +- **Purpose**: Global/cross-cutting concerns that span all layers +- **Responsibilities**: + - Global syntax validation (`./scripts/lint.sh`) + - Project structure validation + - Makefile validation + - Orchestrates infrastructure and application layer tests + +### 2. Infrastructure-Only CI Tests (`make infra-test-ci`) + +- **Script**: `infrastructure/tests/test-ci.sh` (refactored) +- **Purpose**: Infrastructure-specific validation only +- **Responsibilities**: + - Terraform/OpenTofu syntax validation + - Cloud-init template validation + - Infrastructure script validation + - Infrastructure configuration validation + +### 3. Application-Only CI Tests (`make app-test-ci`) + +- **Script**: `application/tests/test-ci.sh` (new) +- **Purpose**: Application-specific validation only +- **Responsibilities**: + - Docker Compose syntax validation + - Application configuration validation + - Deployment script validation + - Grafana dashboard validation + +## Testing Hierarchy + +```text +make test-ci (Project-wide orchestrator) +├── Global concerns (syntax, structure, Makefile) +├── make infra-test-ci (Infrastructure layer only) +└── make app-test-ci (Application layer only) +``` + +## GitHub Actions Integration + +Updated `.github/workflows/testing.yml` to use `make test-ci` instead of `make infra-test-ci`, +ensuring all three layers are properly tested in the correct order. + +## Benefits + +1. **Clear separation of concerns** - Each layer only tests its own responsibilities +2. **Parallel development** - Teams can work on different layers independently +3. **Focused testing** - Developers can test specific layers without running global tests +4. **Maintainable** - Changes to one layer don't affect tests for other layers +5. **CI efficiency** - Better organization of test output and failure isolation + +## Commands + +| Command | Purpose | Scope | +| -------------------- | ------------------------------ | ------------------------- | +| `make test-ci` | Complete CI validation | All layers (orchestrator) | +| `make infra-test-ci` | Infrastructure validation only | Infrastructure layer | +| `make app-test-ci` | Application validation only | Application layer | +| `make lint` | Global syntax validation | Project-wide | + +This change ensures that GitHub runners (which don't support virtualization) can properly +validate all project concerns without attempting infrastructure deployment while maintaining +clear architectural boundaries. diff --git a/infrastructure/tests/test-ci.sh b/infrastructure/tests/test-ci.sh index 44ccd73..1660b0a 100755 --- a/infrastructure/tests/test-ci.sh +++ b/infrastructure/tests/test-ci.sh @@ -1,14 +1,14 @@ #!/bin/bash -# CI-compatible tests - Run tests that work in GitHub runners -# Focus: Syntax validation, configuration validation, script unit tests -# Scope: No virtualization or infrastructure deployment required +# Infrastructure-only CI tests - Run infrastructure-specific tests that work in GitHub runners +# Focus: Infrastructure configuration validation, Terraform syntax, cloud-init templates +# Scope: No virtualization, no global repo concerns (like linting), infrastructure layer only set -euo pipefail # Configuration SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" PROJECT_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" -TEST_LOG_FILE="/tmp/torrust-ci-test.log" +TEST_LOG_FILE="/tmp/torrust-infrastructure-ci-test.log" # Source shared shell utilities # shellcheck source=../../scripts/shell-utils.sh @@ -19,8 +19,9 @@ export SHELL_UTILS_LOG_FILE="${TEST_LOG_FILE}" # Initialize test log init_test_log() { - init_log_file "${TEST_LOG_FILE}" "Torrust Tracker Demo - CI-Compatible Tests" + init_log_file "${TEST_LOG_FILE}" "Infrastructure-only CI Tests" log_info "Environment: CI (no virtualization)" + log_info "Scope: Infrastructure layer only" } # Test execution summary @@ -31,13 +32,12 @@ show_test_summary() { end_time=$(date +%s) duration=$((end_time - start_time)) - log_section "CI TEST SUMMARY" - log_info "Total CI tests completed in ${duration} seconds" - log_success "All CI-compatible tests passed!" + log_section "INFRASTRUCTURE CI TEST SUMMARY" + log_info "Infrastructure CI tests completed in ${duration} seconds" + log_success "All infrastructure-specific tests passed!" log "" - log_info "Next steps for full validation:" - log_info " 1. Run 'make test-local' on a system with virtualization" - log_info " 2. Run 'make test-e2e' for full end-to-end testing" + log_info "Note: This only validates infrastructure layer concerns." + log_info "Run 'make test-ci' for complete project validation." log "" log_info "Test log saved to: ${TEST_LOG_FILE}" } @@ -49,46 +49,28 @@ main() { init_test_log - log_section "TORRUST TRACKER DEMO - CI-COMPATIBLE TESTS" - log_info "Running tests suitable for GitHub runners (no virtualization)" + log_section "INFRASTRUCTURE-ONLY CI TESTS" + log_info "Running infrastructure-specific tests for GitHub runners" cd "${PROJECT_ROOT}" - # Test 1: Syntax validation (fast) - log_section "TEST 1: SYNTAX VALIDATION" - log_info "Running syntax validation..." - if ! make lint; then - log_error "Syntax validation failed" - exit 1 - fi - log_success "Syntax validation passed" - - # Test 2: Configuration validation - log_section "TEST 2: CONFIGURATION VALIDATION" - log_info "Running configuration validation..." + # Test 1: Infrastructure configuration validation + log_section "TEST 1: INFRASTRUCTURE CONFIGURATION" + log_info "Running infrastructure configuration validation..." if ! "${SCRIPT_DIR}/test-unit-config.sh"; then - log_error "Configuration validation failed" + log_error "Infrastructure configuration validation failed" exit 1 fi - log_success "Configuration validation passed" + log_success "Infrastructure configuration validation passed" - # Test 3: Script unit tests - log_section "TEST 3: SCRIPT UNIT TESTS" - log_info "Running script unit tests..." + # Test 2: Infrastructure script unit tests + log_section "TEST 2: INFRASTRUCTURE SCRIPTS" + log_info "Running infrastructure script validation..." if ! "${SCRIPT_DIR}/test-unit-scripts.sh"; then - log_error "Script unit tests failed" + log_error "Infrastructure script validation failed" exit 1 fi - log_success "Script unit tests passed" - - # Test 4: Makefile validation - log_section "TEST 4: MAKEFILE VALIDATION" - log_info "Validating Makefile targets..." - if ! make infra-validate-config 2>/dev/null; then - log_warning "Makefile validation script not found (optional)" - else - log_success "Makefile validation passed" - fi + log_success "Infrastructure script validation passed" show_test_summary "${test_start_time}" } diff --git a/tests/test-ci.sh b/tests/test-ci.sh new file mode 100755 index 0000000..217cb55 --- /dev/null +++ b/tests/test-ci.sh @@ -0,0 +1,77 @@ +#!/bin/bash +# Project-wide CI tests - Run global tests that work in GitHub runners +# Focus: Global syntax validation, Makefile validation, project structure +# Scope: No virtualization, cross-cutting concerns that span all layers + +set -euo pipefail + +# Configuration +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PROJECT_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)" +TEST_LOG_FILE="/tmp/torrust-project-ci-test.log" + +# Source shared shell utilities +# shellcheck source=../scripts/shell-utils.sh +source "${PROJECT_ROOT}/scripts/shell-utils.sh" + +# Set log file for tee output +export SHELL_UTILS_LOG_FILE="${TEST_LOG_FILE}" + +# Initialize test log +init_test_log() { + init_log_file "${TEST_LOG_FILE}" "Project-wide CI Tests" + log_info "Environment: CI (no deployment)" + log_info "Scope: Global/cross-cutting concerns" + log_info "Project Root: ${PROJECT_ROOT}" +} + +# Test execution summary +show_test_summary() { + local start_time=$1 + local end_time + local duration + end_time=$(date +%s) + duration=$((end_time - start_time)) + + log_section "PROJECT CI TEST SUMMARY" + log_info "Project-wide CI tests completed in ${duration} seconds" + log_success "All project-wide tests passed!" + log "" + log_info "Test log saved to: ${TEST_LOG_FILE}" +} + +# Main test execution +main() { + local test_start_time + test_start_time=$(date +%s) + + init_test_log + + log_section "PROJECT-WIDE CI TESTS" + log_info "Running global/cross-cutting tests for GitHub runners" + + cd "${PROJECT_ROOT}" + + # Test 1: Global syntax validation + log_section "TEST 1: GLOBAL SYNTAX VALIDATION" + log_info "Running global syntax validation (all file types)..." + if ! ./scripts/lint.sh; then + log_error "Global syntax validation failed" + exit 1 + fi + log_success "Global syntax validation passed" + + # Test 2: Project-wide unit tests + log_section "TEST 2: PROJECT STRUCTURE & MAKEFILE" + log_info "Running project-wide validation..." + if ! "${SCRIPT_DIR}/test-unit-project.sh"; then + log_error "Project-wide validation failed" + exit 1 + fi + log_success "Project-wide validation passed" + + show_test_summary "${test_start_time}" +} + +# Run main function +main "$@" From e5058469a8413e13c7754bb3e658728994dbddd5 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 28 Jul 2025 11:27:13 +0100 Subject: [PATCH 7/9] docs: add table of contents to copilot instructions Add comprehensive table of contents to .github/copilot-instructions.md for improved navigation through the contributor guide. The TOC includes: - Project overview and repository structure - Development workflow and command reference - Conventions, standards, and testing architecture - Getting started guides for different user types - AI assistant guidelines and restrictions Benefits: - Improves navigation for this comprehensive guide - Makes the document more accessible to contributors - Maintains visual consistency with emoji headings - Enhances professional appearance of documentation --- .github/copilot-instructions.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 57c3b63..a0eba1a 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1,5 +1,26 @@ # Torrust Tracker Demo - Contributor Guide +## Table of Contents + +- [🎯 Project Overview](#-project-overview) + - [Current Major Initiative](#current-major-initiative) +- [📁 Repository Structure](#-repository-structure) + - [Key Components](#key-components) +- [🛠️ Development Workflow](#️-development-workflow) + - [Quick Start for Contributors](#quick-start-for-contributors) + - [Main Commands](#main-commands) +- [📋 Conventions and Standards](#-conventions-and-standards) + - [Twelve-Factor App Principles](#twelve-factor-app-principles) + - [Git Workflow](#git-workflow) + - [Code Quality Standards](#code-quality-standards) + - [Testing Requirements](#testing-requirements) + - [Security Guidelines](#security-guidelines) +- [🚀 Getting Started](#-getting-started) + - [For New Contributors](#for-new-contributors) + - [For Infrastructure Changes](#for-infrastructure-changes) + - [For AI Assistants](#for-ai-assistants) +- [📖 Additional Resources](#-additional-resources) + ## 🎯 Project Overview **Torrust Tracker Demo** is the complete production deployment configuration for running a live [Torrust Tracker](https://github.com/torrust/torrust-tracker) instance. This repository provides: From efd79dab1e424a66b77be977d56ccb4cbc172b9a Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 28 Jul 2025 11:37:36 +0100 Subject: [PATCH 8/9] fix: [#14] add .gitkeep to preserve application/config/templates directory structure The GitHub Actions CI was failing because the application/config/templates directory wasn't being tracked by Git (empty directories are not tracked). The application unit tests expect this directory to exist. Fixes failing workflow: - application/tests/test-unit-application.sh checks for 'config' directory - Empty directories are not preserved during git checkout - Added .gitkeep to ensure directory structure is maintained This resolves the CI failure: 'Required application path missing: config' --- application/config/templates/.gitkeep | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 application/config/templates/.gitkeep diff --git a/application/config/templates/.gitkeep b/application/config/templates/.gitkeep new file mode 100644 index 0000000..8669a0d --- /dev/null +++ b/application/config/templates/.gitkeep @@ -0,0 +1,3 @@ +# This file ensures the templates directory is tracked by Git +# Template files for application configuration will be stored here +# as part of the twelve-factor app configuration management system From 0a36bbc6325f5cfe7b28abfe00c6da7f2d377471 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 28 Jul 2025 11:43:55 +0100 Subject: [PATCH 9/9] fix: [#14] correct configure-env test logic to eliminate CI warning The test was incorrectly expecting the configure-env.sh script to fail when called without parameters, but the script is designed to use sensible defaults ('local' environment). Changes: - Fixed test logic to expect success when script uses defaults - Replaced warning message with proper success validation - Added proper error handling when script actually fails This eliminates the CI warning: '[WARNING] Script should handle missing parameters gracefully' The script DOES handle missing parameters gracefully by using defaults, so the test should validate this behavior correctly. --- infrastructure/tests/scripts/test-configure-env.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/infrastructure/tests/scripts/test-configure-env.sh b/infrastructure/tests/scripts/test-configure-env.sh index 9ee4d20..0641415 100755 --- a/infrastructure/tests/scripts/test-configure-env.sh +++ b/infrastructure/tests/scripts/test-configure-env.sh @@ -63,11 +63,12 @@ test_configure_env_error_handling() { # Test with invalid environment names log_info "Testing invalid environment handling..." - # Test with empty parameters + # Test with empty parameters - script should succeed with defaults if "${SCRIPT_PATH}" >/dev/null 2>&1; then - log_warning "Script should handle missing parameters gracefully" + log_success "Script properly handles missing parameters by using defaults" else - log_info "Script properly handles missing parameters" + log_error "Script failed when it should use default parameters" + failed=$((failed + 1)) fi log_success "Configuration script error handling tests completed"