Merge CI stability improvements from pr-225

- Extended test timeout from 30m to 60m
- Added 90-minute job timeouts to GitHub Actions
- Enhanced CloudStack simulator setup with better logging
- Added retry logic for data center deployment
- Improved error handling and diagnostics

This addresses the 21 failing CI checks in PR #225.

Author: sidshas03

.github/actions/setup-cloudstack/action.yml

@@ -43,18 +43,30 @@ runs:
       run: |
         echo "Starting Cloudstack health check"
         T=0
-        until [ $T -gt 40 ]  || curl -sfL http://localhost:8080 --output /dev/null
+        MAX_WAIT=40
+        SLEEP_INTERVAL=30
+        
+        echo "Will wait up to $((MAX_WAIT * SLEEP_INTERVAL / 60)) minutes for CloudStack to be ready"
+        
+        until [ $T -gt $MAX_WAIT ] || curl -sfL http://localhost:8080 --output /dev/null
         do
-            echo "Waiting for Cloudstack to be ready..."
+            echo "Waiting for Cloudstack to be ready... (attempt $((T+1))/$((MAX_WAIT+1)), elapsed: $((T * SLEEP_INTERVAL / 60))m $((T * SLEEP_INTERVAL % 60))s)"
             ((T+=1))
-            sleep 30
+            sleep $SLEEP_INTERVAL
         done
 
         # After loop, check if Cloudstack is up
         if ! curl -sfSL http://localhost:8080 --output /dev/null; then
-          echo "Cloudstack did not become ready in time"
+          echo "Cloudstack did not become ready in time after $((MAX_WAIT * SLEEP_INTERVAL / 60)) minutes"
+          echo "Checking CloudStack container status:"
+          docker ps -a | grep cloudstack || echo "No CloudStack containers found"
+          echo "Checking CloudStack logs:"
+          docker logs $(docker ps -q --filter "ancestor=apache/cloudstack-simulator") --tail 50 || echo "Could not retrieve logs"
+          echo "Testing connectivity:"
           curl -v http://localhost:8080 || true
           exit 22
+        else
+          echo "CloudStack is ready after $((T * SLEEP_INTERVAL / 60))m $((T * SLEEP_INTERVAL % 60))s"
         fi
     - name: Setting up Cloudstack
       id: setup-cloudstack
@@ -64,8 +76,23 @@ runs:
         set -euo pipefail
 
         echo "Deploying Data Center..."
-        docker exec $(docker container ls --format=json -l | jq -r .ID) \
-          python /root/tools/marvin/marvin/deployDataCenter.py -i /root/setup/dev/advanced.cfg
+        # Retry data center deployment up to 3 times
+        for attempt in 1 2 3; do
+          echo "Data center deployment attempt $attempt/3"
+          if docker exec $(docker container ls --format=json -l | jq -r .ID) \
+            python /root/tools/marvin/marvin/deployDataCenter.py -i /root/setup/dev/advanced.cfg; then
+            echo "Data center deployment successful on attempt $attempt"
+            break
+          else
+            echo "Data center deployment failed on attempt $attempt"
+            if [ $attempt -eq 3 ]; then
+              echo "All data center deployment attempts failed"
+              exit 1
+            fi
+            echo "Waiting 30 seconds before retry..."
+            sleep 30
+          fi
+        done
 
         # Get the container ID of the running simulator
         CONTAINER_ID=$(docker ps --filter "ancestor=apache/cloudstack-simulator:${{ matrix.cloudstack-version }}" --format "{{.ID}}" | head -n1)

.github/workflows/acceptance.yml

@@ -47,6 +47,7 @@ jobs:
     name: Terraform ${{ matrix.terraform-version }} with Cloudstack ${{ matrix.cloudstack-version }}
     needs: [prepare-matrix]
     runs-on: ubuntu-latest
+    timeout-minutes: 90
     steps:
       - uses: actions/checkout@v4
       - name: Set up Go
@@ -86,6 +87,7 @@ jobs:
     name: OpenTofu ${{ matrix.opentofu-version }} with Cloudstack ${{ matrix.cloudstack-version }}
     needs: [prepare-matrix]
     runs-on: ubuntu-latest
+    timeout-minutes: 90
     steps:
       - uses: actions/checkout@v4
       - name: Set up Go

GNUmakefile

@@ -31,7 +31,7 @@ test: fmtcheck
 		xargs -t -n4 go test $(TESTARGS) -timeout=30s -parallel=4
 
 testacc: fmtcheck
-	TF_ACC=1 go test $(TEST) -v $(TESTARGS) -timeout 30m
+	TF_ACC=1 go test $(TEST) -v $(TESTARGS) -timeout 60m
 
 vet:
 	@echo "go vet ."

cloudstack/resource_cloudstack_egress_firewall.go

@@ -331,10 +331,12 @@ func createEgressFirewallRule(d *schema.ResourceData, meta interface{}, rule map
 			// by not setting startport and endport parameters
 			r, err := cs.Firewall.CreateEgressFirewallRule(p)
 			if err != nil {
-				return err
+				return fmt.Errorf("failed to create all-ports egress firewall rule: %w", err)
 			}
 			uuids["all"] = r.Id
 			rule["uuids"] = uuids
+			// Remove the ports field since we're creating an all-ports rule
+			delete(rule, "ports")
 		}
 	}
 
@@ -501,10 +503,50 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 					// Update the values
 					rule["protocol"] = r.Protocol
 					rule["cidr_list"] = cidrs
+					// Remove ports field for all-ports rules
+					delete(rule, "ports")
 					rules.Add(rule)
 				}
 			}
 
+			// Fallback: Check if any remaining rules in ruleMap match our expected all-ports pattern
+			// This handles cases where CloudStack might return all-ports rules in unexpected formats
+			if rule["protocol"].(string) != "icmp" && strings.ToLower(rule["protocol"].(string)) != "all" {
+				// Look for any remaining rules that might be our all-ports rule
+				for ruleID, r := range ruleMap {
+					// Get local CIDR set for comparison
+					localCidrSet, ok := rule["cidr_list"].(*schema.Set)
+					if !ok {
+						continue
+					}
+
+					if isAllPortsTCPUDP(r.Protocol, r.Startport, r.Endport) &&
+						strings.EqualFold(r.Protocol, rule["protocol"].(string)) &&
+						cidrSetsEqual(r.Cidrlist, localCidrSet) {
+						// This looks like our all-ports rule, add it to state
+						cidrs := &schema.Set{F: schema.HashString}
+						if r.Cidrlist != "" {
+							for _, cidr := range strings.Split(r.Cidrlist, ",") {
+								cidr = strings.TrimSpace(cidr)
+								if cidr != "" {
+									cidrs.Add(cidr)
+								}
+							}
+						}
+
+						rule["protocol"] = r.Protocol
+						rule["cidr_list"] = cidrs
+						// Remove ports field for all-ports rules
+						delete(rule, "ports")
+						rules.Add(rule)
+
+						// Remove from ruleMap so it's not processed again
+						delete(ruleMap, ruleID)
+						break
+					}
+				}
+			}
+
 			if strings.ToLower(rule["protocol"].(string)) == "all" {
 				id, ok := uuids["all"]
 				if !ok {
@@ -715,7 +757,7 @@ func verifyEgressFirewallParams(d *schema.ResourceData) error {
 
 	if !rules && !managed {
 		return fmt.Errorf(
-			"You must supply at least one 'rule' when not using the 'managed' firewall feature")
+			"you must supply at least one 'rule' when not using the 'managed' firewall feature")
 	}
 
 	return nil
@@ -731,11 +773,11 @@ func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]inte
 	if protocol == "icmp" {
 		if _, ok := rule["icmp_type"]; !ok {
 			return fmt.Errorf(
-				"Parameter icmp_type is a required parameter when using protocol 'icmp'")
+				"parameter icmp_type is a required parameter when using protocol 'icmp'")
 		}
 		if _, ok := rule["icmp_code"]; !ok {
 			return fmt.Errorf(
-				"Parameter icmp_code is a required parameter when using protocol 'icmp'")
+				"parameter icmp_code is a required parameter when using protocol 'icmp'")
 		}
 	} else if strings.ToLower(protocol) != "all" {
 		if ports, ok := rule["ports"].(*schema.Set); ok {
@@ -752,7 +794,7 @@ func verifyEgressFirewallRuleParams(d *schema.ResourceData, rule map[string]inte
 	} else if strings.ToLower(protocol) == "all" {
 		if ports, _ := rule["ports"].(*schema.Set); ports.Len() > 0 {
 			return fmt.Errorf(
-				"Parameter ports is not required when using protocol 'ALL'")
+				"parameter ports is not required when using protocol 'ALL'")
 		}
 	}