Merge #25: feat: [#24] Improve UX by adding automatic waiting to deployment commands

ad974e0 feat: [#24] improve UX by adding automatic waiting to deployment commands (Jose Celano)
826f054 docs: [#24] update copilot instructions with sudo cache management info (Jose Celano)

Pull request description:

  ## Summary

  This PR resolves issue #24 by implementing automatic waiting logic in deployment commands to prevent premature execution failures and improve user experience.

  ## Problem Solved

  Previously, the three-step deployment workflow would fail if commands were run too quickly:
  ```bash
  make infra-apply ENVIRONMENT=local    # Would succeed
  make app-deploy ENVIRONMENT=local     # Would fail if run immediately
  make app-health-check ENVIRONMENT=local # Would fail due to services not ready
  ```

  Users had to manually wait and guess timing, leading to poor UX and unreliable deployments.

  ## Solution Implemented

  **Solution 1**: Default automatic waiting with `SKIP_WAIT` option for advanced users.

  ### Key Features

  - ✅ **Automatic VM readiness waiting** in `make infra-apply`
  - ✅ **Automatic service health waiting** in `make app-deploy`
  - ✅ **SKIP_WAIT parameter** for advanced users (`SKIP_WAIT=true`)
  - ✅ **Clear progress feedback** during operations
  - ✅ **Bug fix**: Resolved OpenTofu working directory issues
  - ✅ **Comprehensive testing**: Manual + E2E validation

  ### Changes Made

  #### Infrastructure Layer
  - **shell-utils.sh**: Added `wait_for_vm_ip()` and `wait_for_cloud_init_completion()` functions
  - **provision-infrastructure.sh**: Automatic VM readiness waiting + bug fixes
  - **deploy-app.sh**: Enhanced service health waiting with better logging

  #### User Interface
  - **Makefile**: Added `SKIP_WAIT` parameter support for `infra-apply` and `app-deploy`
  - **Documentation**: Updated guides with new workflow and sudo cache management

  #### Testing & Quality
  - **test-e2e.sh**: Updated to use centralized waiting functions
  - **CI validation**: All tests pass, including comprehensive E2E testing

  ## Testing Results

  ### ✅ Manual Testing
  ```bash
  make infra-apply ENVIRONMENT=local    # ✅ Waits for VM + cloud-init
  make app-deploy ENVIRONMENT=local     # ✅ Waits for service health
  make app-health-check ENVIRONMENT=local # ✅ 100% success rate
  ```

  ### ✅ E2E Testing
  - **Total test time**: 2m 40s
  - **Health checks**: 14/14 passed (100%)
  - **Smoke tests**: 0 failures
  - **Clean workflow**: Infrastructure → Application → Health → Cleanup

  ### ✅ Advanced User Workflow
  ```bash
  make infra-apply ENVIRONMENT=local SKIP_WAIT=true    # ✅ Fast execution
  make app-deploy ENVIRONMENT=local SKIP_WAIT=true     # ✅ No waiting
  ```

  ## Impact

  ### 🎯 User Experience
  - **Eliminates manual timing guesswork**
  - **Provides clear progress feedback**
  - **Maintains advanced user flexibility**
  - **Fixes confusing warning messages**

  ### 🏗️ Technical Benefits
  - **Robust deployment workflow**
  - **Better error handling and logging**
  - **Centralized waiting logic**
  - **Improved twelve-factor compliance**

  ## Documentation

  Updated documentation includes:
  - **Contributor guide**: Sudo cache management, workflow improvements
  - **Cloud deployment guide**: Updated with new parameters
  - **Testing coverage**: Both manual and automated scenarios

  ## Breaking Changes

  **None** - This is a pure UX improvement that maintains backward compatibility.

  ## Related Issues

  - Closes #24
  - Improves twelve-factor deployment workflow
  - Enhances contributor experience

  ## Review Notes

  - All CI tests pass ✅
  - E2E testing validates complete workflow ✅
  - Manual testing confirms UX improvements ✅
  - Documentation updated ✅
  - No breaking changes ✅

  The three-step deployment workflow now works seamlessly without manual intervention, while preserving advanced user capabilities.

ACKs for top commit:
  josecelano:
    ACK ad974e0

Tree-SHA512: 1995d568b3c742f7de91a85aeb8f43dc9cbd97910f681a13fb74978d19a925586805c2100e3f4cb64ec6be6a1121502c42a0cb0dc0919a2551f80d6cd6ae8b4e

Author: josecelano

.github/copilot-instructions.md

@@ -314,9 +314,11 @@ The twelve-factor **Build, Release, Run** stages apply to the application deploy
 
 #### Branch Naming
 
-- **Format**: `{issue-number}-{short-description}`
-- **Examples**: `42-add-mysql-support`, `15-fix-ssl-renewal`
+- **Format**: `{issue-number}-{short-description-following-github-conventions}`
+- **GitHub conventions**: Use lowercase, separate words with hyphens, descriptive but concise
+- **Examples**: `42-add-mysql-support`, `15-fix-ssl-renewal`, `24-improve-ux-add-automatic-waiting-to-infra-apply-and-app-deploy-commands`
 - Always start with the GitHub issue number
+- Follow GitHub's recommended branch naming: lowercase, hyphens for word separation, descriptive of the change
 
 #### Commit Messages
 

Makefile

@@ -71,7 +71,12 @@ infra-plan: ## Plan infrastructure changes
 infra-apply: ## Provision infrastructure (platform setup)
 	@echo "Provisioning infrastructure for $(ENVIRONMENT)..."
 	@echo "⚠️  This command may prompt for your password for sudo operations"
-	$(SCRIPTS_DIR)/provision-infrastructure.sh $(ENVIRONMENT) apply
+	@if [ "$(SKIP_WAIT)" = "true" ]; then \
+		echo "⚠️  SKIP_WAIT=true - Infrastructure will not wait for full readiness"; \
+	else \
+		echo "ℹ️  Infrastructure will wait for full readiness (use SKIP_WAIT=true to skip)"; \
+	fi
+	SKIP_WAIT=$(SKIP_WAIT) $(SCRIPTS_DIR)/provision-infrastructure.sh $(ENVIRONMENT) apply
 
 infra-destroy: ## Destroy infrastructure
 	@echo "Destroying infrastructure for $(ENVIRONMENT)..."
@@ -116,7 +121,12 @@ infra-test-local: ## Run local-only infrastructure tests (requires virtualizatio
 
 app-deploy: ## Deploy application (Twelve-Factor Build + Release + Run stages)
 	@echo "Deploying application for $(ENVIRONMENT)..."
-	$(SCRIPTS_DIR)/deploy-app.sh $(ENVIRONMENT)
+	@if [ "$(SKIP_WAIT)" = "true" ]; then \
+		echo "⚠️  SKIP_WAIT=true - Application will not wait for service readiness"; \
+	else \
+		echo "ℹ️  Application will wait for service readiness (use SKIP_WAIT=true to skip)"; \
+	fi
+	SKIP_WAIT=$(SKIP_WAIT) $(SCRIPTS_DIR)/deploy-app.sh $(ENVIRONMENT)
 
 app-redeploy: ## Redeploy application without infrastructure changes
 	@echo "Redeploying application for $(ENVIRONMENT)..."

docs/guides/cloud-deployment-guide.md

@@ -66,10 +66,16 @@ make infra-config-local
 
 ```bash
 # Test deployment locally with KVM
+# Commands wait for full readiness by default
 make infra-apply ENVIRONMENT=local
 make app-deploy ENVIRONMENT=local
 make app-health-check
 
+# Advanced users: Skip waiting for faster execution
+make infra-apply ENVIRONMENT=local SKIP_WAIT=true
+make app-deploy ENVIRONMENT=local SKIP_WAIT=true
+make app-health-check
+
 # Access the local instance via SSH
 make vm-ssh
 
@@ -247,6 +253,41 @@ should be tested in a staging environment first.
 
 ## Detailed Deployment Process
 
+### ✅ Improved User Experience (Automatic Waiting)
+
+**Issue #24 - Enhanced Workflow**: The deployment commands now wait for full readiness by
+default, providing a much better user experience:
+
+**Previous workflow problems**:
+
+- Commands completed before systems were actually ready
+- Users had to manually wait between steps without clear indicators
+- Following commands often failed if run too quickly
+
+**✅ Current improved workflow**:
+
+```bash
+# Each command waits for full readiness by default
+make infra-apply ENVIRONMENT=local    # Waits for VM IP + cloud-init completion
+make app-deploy ENVIRONMENT=local     # Waits for all services to be healthy
+make app-health-check                 # Validates everything is working
+```
+
+**Key improvements**:
+
+- ✅ **Clear progress indicators**: You see exactly what's happening during waits
+- ✅ **Automatic readiness detection**: Commands complete when actually ready for next step
+- ✅ **Reliable workflow**: No more timing-related failures between commands
+- ✅ **Backwards compatibility**: Use `SKIP_WAIT=true` for original fast behavior
+
+**Advanced usage** (for CI/automation):
+
+```bash
+# Skip waiting for faster execution (original behavior)
+make infra-apply ENVIRONMENT=local SKIP_WAIT=true
+make app-deploy ENVIRONMENT=local SKIP_WAIT=true
+```
+
 ### Infrastructure Deployment
 
 The infrastructure deployment creates and configures the VM:
@@ -262,6 +303,8 @@ make infra-apply ENVIRONMENT=production
 # 4. Sets up torrust user with SSH access
 # 5. Configures firewall rules
 # 6. Creates persistent data volume
+# 7. ✅ NEW: Waits for VM IP assignment and cloud-init completion
+# 8. ✅ NEW: Ensures infrastructure is ready for next step
 ```
 
 ### Application Deployment
@@ -284,6 +327,8 @@ make app-deploy ENVIRONMENT=production
 #    - Grafana dashboards
 # 5. Configures automated maintenance tasks
 # 6. Validates all service health
+# 7. ✅ NEW: Waits for all services to be healthy and ready
+# 8. ✅ NEW: Ensures deployment is complete before finishing
 ```
 
 ### Health Validation

infrastructure/scripts/deploy-app.sh

@@ -14,6 +14,7 @@ TERRAFORM_DIR="${PROJECT_ROOT}/infrastructure/terraform"
 ENVIRONMENT="${1:-local}"
 VM_IP="${2:-}"
 SKIP_HEALTH_CHECK="${SKIP_HEALTH_CHECK:-false}"
+SKIP_WAIT="${SKIP_WAIT:-false}"  # New parameter for skipping waiting
 ENABLE_HTTPS="${ENABLE_SSL:-true}"   # Enable HTTPS with self-signed certificates by default
 
 # Source shared shell utilities
@@ -707,23 +708,27 @@ wait_for_services() {
 setup_backup_automation() {
     local vm_ip="$1"
 
+    log_info "   Checking backup automation configuration..."
+
     # Load environment variables from the generated .env file
     if [[ -f "${PROJECT_ROOT}/application/storage/compose/.env" ]]; then
         # shellcheck source=/dev/null
         source "${PROJECT_ROOT}/application/storage/compose/.env"
+        log_info "   ✅ Loaded environment configuration"
     else
-        log_warning "Environment file not found, using defaults"
+        log_warning "   ⚠️  Environment file not found, using defaults"
     fi
 
     # Check if backup automation is enabled
     if [[ "${ENABLE_DB_BACKUPS:-false}" != "true" ]]; then
-        log_info "Database backup automation disabled (ENABLE_DB_BACKUPS=false)"
+        log_info "   ⏹️  Database backup automation disabled (ENABLE_DB_BACKUPS=${ENABLE_DB_BACKUPS:-false})"
         return 0
     fi
 
-    log_info "Setting up automated database backups..."
+    log_info "   ✅ Database backup automation enabled - proceeding with setup..."
 
     # Create backup directory and set permissions
+    log_info "   ⏳ Creating backup directory and setting permissions..."
     vm_exec "${vm_ip}" "
         # Create backup directory if it doesn't exist
         sudo mkdir -p /var/lib/torrust/mysql/backups
@@ -734,8 +739,10 @@ setup_backup_automation() {
         # Set appropriate permissions
         chmod 755 /var/lib/torrust/mysql/backups
     " "Setting up backup directory"
+    log_info "   ✅ Backup directory setup completed"
 
     # Install crontab entry for automated backups
+    log_info "   ⏳ Installing MySQL backup cron job..."
     vm_exec "${vm_ip}" "
         cd /home/torrust/github/torrust/torrust-tracker-demo
         
@@ -752,8 +759,10 @@ setup_backup_automation() {
         echo 'Current crontab entries:'
         crontab -l || echo 'No crontab entries found'
     " "Installing MySQL backup cron job"
+    log_info "   ✅ Cron job installation completed"
 
     # Test backup script functionality
+    log_info "   ⏳ Validating backup script functionality..."
     vm_exec "${vm_ip}" "
         cd /home/torrust/github/torrust/torrust-tracker-demo/application
         
@@ -775,8 +784,9 @@ setup_backup_automation() {
             echo '✅ Fixed backup script permissions'
         fi
     " "Validating backup script"
+    log_info "   ✅ Backup script validation completed"
 
-    log_success "Database backup automation configured successfully"
+    log_success "   🎉 Database backup automation configured successfully"
     log_info "Backup schedule: Daily at 3:00 AM"
     log_info "Backup location: /var/lib/torrust/mysql/backups"
     log_info "Retention period: ${BACKUP_RETENTION_DAYS:-7} days"
@@ -817,19 +827,32 @@ run_stage() {
         docker compose --env-file /var/lib/torrust/compose/.env up -d
     " "Starting application services"
 
-    # Wait for services to initialize
-    wait_for_services "${vm_ip}"
+    # Wait for services to initialize (unless skipped)
+    if [[ "${SKIP_WAIT}" != "true" ]]; then
+        log_info "⏳ Waiting for application services to be healthy..."
+        log_info "   (Use SKIP_WAIT=true to skip this waiting)"
+        wait_for_services "${vm_ip}"
+        log_success "🎉 All application services are healthy and ready!"
+    else
+        log_warning "⚠️  Skipping wait for service health checks (SKIP_WAIT=true)"
+        log_info "   Note: Services may not be ready immediately"
+    fi
 
     # Setup HTTPS with self-signed certificates (if enabled)
     if [[ "${ENABLE_HTTPS}" == "true" ]]; then
+        log_info "⏳ Setting up HTTPS certificates..."
         log_info "HTTPS certificates already generated - services should be running with HTTPS..."
-        log_success "HTTPS setup completed"
+        log_success "✅ HTTPS setup completed"
+    else
+        log_info "⏹️  HTTPS setup skipped (ENABLE_HTTPS=${ENABLE_HTTPS})"
     fi
 
     # Setup database backup automation (if enabled)
+    log_info "⏳ Setting up database backup automation..."
     setup_backup_automation "${vm_ip}"
+    log_success "✅ Database backup automation completed"
 
-    log_success "Run stage completed"
+    log_success "🎉 Run stage completed successfully"
 }
 
 # Validate deployment (Health checks)
@@ -1018,10 +1041,14 @@ main() {
     test_ssh_connection "${vm_ip}"
     wait_for_system_ready "${vm_ip}"
     release_stage "${vm_ip}"
-    run_stage "${vm_ip}"
+    run_stage "${vm_ip}"  # This already includes waiting for services
 
     if [[ "${SKIP_HEALTH_CHECK}" != "true" ]]; then
+        log_info "⏳ Running deployment validation..."
         validate_deployment "${vm_ip}"
+        log_success "✅ Deployment validation completed"
+    else
+        log_warning "⚠️  Skipping deployment validation (SKIP_HEALTH_CHECK=true)"
     fi
 
     show_connection_info "${vm_ip}"
@@ -1040,6 +1067,7 @@ Arguments:
 
 Environment Variables:
     SKIP_HEALTH_CHECK    Skip health check validation (true/false, default: false)
+    SKIP_WAIT           Skip waiting for services to be ready (true/false, default: false)
 
 Examples:
     $0 local                    # Deploy to local environment (get IP from Terraform)

infrastructure/scripts/provision-infrastructure.sh

@@ -11,11 +11,10 @@ PROJECT_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)"
 TERRAFORM_DIR="${PROJECT_ROOT}/infrastructure/terraform"
 
 # Default values
-# Configuration
-SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
-PROJECT_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)"
 ENVIRONMENT="${1:-local}"
 ACTION="${2:-apply}"
+SKIP_WAIT="${SKIP_WAIT:-false}"  # New parameter for skipping waiting
+SKIP_WAIT="${SKIP_WAIT:-false}"  # New parameter for skipping waiting
 
 # Source shared shell utilities
 # shellcheck source=../../scripts/shell-utils.sh
@@ -115,9 +114,32 @@ provision_infrastructure() {
 
         tofu apply -auto-approve -var-file="local.tfvars"
 
+        # Wait for infrastructure to be fully ready (unless skipped)
+        if [[ "${SKIP_WAIT}" != "true" ]]; then
+            log_info "⏳ Waiting for infrastructure to be fully ready..."
+            log_info "   (Use SKIP_WAIT=true to skip this waiting)"
+            
+            # Wait for VM IP assignment
+            if ! wait_for_vm_ip "${ENVIRONMENT}" "${PROJECT_ROOT}"; then
+                log_error "Failed to wait for VM IP assignment"
+                exit 1
+            fi
+            
+            # Wait for cloud-init completion  
+            if ! wait_for_cloud_init_completion "${ENVIRONMENT}"; then
+                log_error "Failed to wait for cloud-init completion"
+                exit 1
+            fi
+            
+            log_success "🎉 Infrastructure is fully ready for application deployment!"
+        else
+            log_warning "⚠️  Skipping wait for infrastructure readiness (SKIP_WAIT=true)"
+            log_info "   Note: You may need to wait before running app-deploy"
+        fi
+
         # Get VM IP and display connection info
         local vm_ip
-        vm_ip=$(tofu output -raw vm_ip 2>/dev/null || echo "")
+        vm_ip=$(cd "${TERRAFORM_DIR}" && tofu output -raw vm_ip 2>/dev/null || echo "")
 
         if [[ -n "${vm_ip}" ]]; then
             log_success "Infrastructure provisioned successfully"