More tweaks to improve performance

builtin/providers/cloudstack/provider.go

@@ -36,7 +36,7 @@ func Provider() terraform.ResourceProvider {
 			"timeout": &schema.Schema{
 				Type:        schema.TypeInt,
 				Required:    true,
-				DefaultFunc: schema.EnvDefaultFunc("CLOUDSTACK_TIMEOUT", 300),
+				DefaultFunc: schema.EnvDefaultFunc("CLOUDSTACK_TIMEOUT", 900),
 			},
 		},
 

builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go

@@ -301,22 +301,18 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 	// If this is a managed firewall, add all unknown rules into a single dummy rule
 	managed := d.Get("managed").(bool)
 	if managed && len(ruleMap) > 0 {
-		// Add all UUIDs to a uuids map
-		uuids := make(map[string]interface{}, len(ruleMap))
 		for uuid := range ruleMap {
-			uuids[uuid] = uuid
+			// Make a dummy rule to hold the unknown UUID
-		}
-
-		// Make a dummy rule to hold all unknown UUIDs
 			rule := map[string]interface{}{
-			"source_cidr": "N/A",
+				"source_cidr": uuid,
-			"protocol":    "N/A",
+				"protocol":    uuid,
-			"uuids":       ruleMap,
+				"uuids":       map[string]interface{}{uuid: uuid},
 			}
 
 			// Add the dummy rule to the rules set
 			rules.Add(rule)
 		}
+	}
 
 	if rules.Len() > 0 {
 		d.Set("rule", rules)

builtin/providers/cloudstack/resource_cloudstack_firewall.go

@@ -301,22 +301,18 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
 	// If this is a managed firewall, add all unknown rules into a single dummy rule
 	managed := d.Get("managed").(bool)
 	if managed && len(ruleMap) > 0 {
-		// Add all UUIDs to a uuids map
-		uuids := make(map[string]interface{}, len(ruleMap))
 		for uuid := range ruleMap {
-			uuids[uuid] = uuid
+			// Make a dummy rule to hold the unknown UUID
-		}
-
-		// Make a dummy rule to hold all unknown UUIDs
 			rule := map[string]interface{}{
-			"source_cidr": "N/A",
+				"source_cidr": uuid,
-			"protocol":    "N/A",
+				"protocol":    uuid,
-			"uuids":       uuids,
+				"uuids":       map[string]interface{}{uuid: uuid},
 			}
 
 			// Add the dummy rule to the rules set
 			rules.Add(rule)
 		}
+	}
 
 	if rules.Len() > 0 {
 		d.Set("rule", rules)

builtin/providers/cloudstack/resource_cloudstack_network_acl_rule.go

@@ -7,7 +7,10 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"sync"
+	"time"
 
+	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform/helper/hashcode"
 	"github.com/hashicorp/terraform/helper/schema"
 	"github.com/xanzy/go-cloudstack/cloudstack"
@@ -103,32 +106,72 @@ func resourceCloudStackNetworkACLRuleCreate(d *schema.ResourceData, meta interfa
 	d.SetId(d.Get("aclid").(string))
 
 	// Create all rules that are configured
-	if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 {
+	if nrs := d.Get("rule").(*schema.Set); nrs.Len() > 0 {
-
+		// Create an empty rule set to hold all newly created rules
-		// Create an empty schema.Set to hold all rules
 		rules := &schema.Set{
 			F: resourceCloudStackNetworkACLRuleHash,
 		}
 
-		for _, rule := range rs.List() {
+		err := resourceCloudStackNetworkACLRuleCreateRules(d, meta, rules, nrs)
-			// Create a single rule
-			err := resourceCloudStackNetworkACLRuleCreateRule(d, meta, rule.(map[string]interface{}))
 
 		// We need to update this first to preserve the correct state
-			rules.Add(rule)
 		d.Set("rule", rules)
 
 		if err != nil {
 			return err
 		}
 	}
-	}
 
 	return resourceCloudStackNetworkACLRuleRead(d, meta)
 }
 
+func resourceCloudStackNetworkACLRuleCreateRules(
+	d *schema.ResourceData,
+	meta interface{},
+	rules *schema.Set,
+	nrs *schema.Set) error {
+	var errs *multierror.Error
+
+	var wg sync.WaitGroup
+	wg.Add(nrs.Len())
+
+	sem := make(chan struct{}, 10)
+	for _, rule := range nrs.List() {
+		// Put in a tiny sleep here to avoid DoS'ing the API
+		time.Sleep(500 * time.Millisecond)
+
+		go func(rule map[string]interface{}) {
+			defer wg.Done()
+			sem <- struct{}{}
+
+			// Create a single rule
+			err := resourceCloudStackNetworkACLRuleCreateRule(d, meta, rule)
+
+			// If we have at least one UUID, we need to save the rule
+			if len(rule["uuids"].(map[string]interface{})) > 0 {
+				rules.Add(rule)
+			}
+
+			if err != nil {
+				errs = multierror.Append(errs, err)
+			}
+
+			<-sem
+		}(rule.(map[string]interface{}))
+	}
+
+	wg.Wait()
+
+	// We need to update this first to preserve the correct state
+	d.Set("rule", rules)
+
+	return errs.ErrorOrNil()
+}
+
 func resourceCloudStackNetworkACLRuleCreateRule(
-	d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error {
+	d *schema.ResourceData,
+	meta interface{},
+	rule map[string]interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 	uuids := rule["uuids"].(map[string]interface{})
 
@@ -188,8 +231,16 @@ func resourceCloudStackNetworkACLRuleCreateRule(
 				},
 			}
 
-			for _, port := range ps.List() {
+			// Define a regexp for parsing the port
 			re := regexp.MustCompile(`^(\d+)(?:-(\d+))?$`)
+
+			for _, port := range ps.List() {
+				if _, ok := uuids[port.(string)]; ok {
+					ports.Add(port)
+					rule["ports"] = ports
+					continue
+				}
+
 				m := re.FindStringSubmatch(port.(string))
 
 				startPort, err := strconv.Atoi(m[1])
@@ -354,21 +405,18 @@ func resourceCloudStackNetworkACLRuleRead(d *schema.ResourceData, meta interface
 	// If this is a managed firewall, add all unknown rules into a single dummy rule
 	managed := d.Get("managed").(bool)
 	if managed && len(ruleMap) > 0 {
-		// Add all UUIDs to a uuids map
-		uuids := make(map[string]interface{}, len(ruleMap))
 		for uuid := range ruleMap {
-			uuids[uuid] = uuid
+			// Make a dummy rule to hold the unknown UUID
-		}
-
 			rule := map[string]interface{}{
-			"source_cidr": "N/A",
+				"source_cidr": uuid,
-			"protocol":    "N/A",
+				"protocol":    uuid,
-			"uuids":       uuids,
+				"uuids":       map[string]interface{}{uuid: uuid},
 			}
 
 			// Add the dummy rule to the rules set
 			rules.Add(rule)
 		}
+	}
 
 	if rules.Len() > 0 {
 		d.Set("rule", rules)
@@ -391,26 +439,29 @@ func resourceCloudStackNetworkACLRuleUpdate(d *schema.ResourceData, meta interfa
 		ors := o.(*schema.Set).Difference(n.(*schema.Set))
 		nrs := n.(*schema.Set).Difference(o.(*schema.Set))
 
-		// Now first loop through all the old rules and delete any obsolete ones
+		// We need to start with a rule set containing all the rules we
-		for _, rule := range ors.List() {
+		// already have and want to keep. Any rules that are not deleted
-			// Delete the rule as it no longer exists in the config
+		// correctly and any newly created rules, will be added to this
-			err := resourceCloudStackNetworkACLRuleDeleteRule(d, meta, rule.(map[string]interface{}))
+		// set to make sure we end up in a consistent state
+		rules := o.(*schema.Set).Intersection(n.(*schema.Set))
+
+		// Now first loop through all the old rules and delete them
+		if ors.Len() > 0 {
+			err := resourceCloudStackNetworkACLRuleDeleteRules(d, meta, rules, ors)
+
+			// We need to update this first to preserve the correct state
+			d.Set("rule", rules)
+
 			if err != nil {
 				return err
 			}
 		}
 
-		// Make sure we save the state of the currently configured rules
+		// Then loop through all the new rules and create them
-		rules := o.(*schema.Set).Intersection(n.(*schema.Set))
+		if nrs.Len() > 0 {
-		d.Set("rule", rules)
+			err := resourceCloudStackNetworkACLRuleCreateRules(d, meta, rules, nrs)
-
-		// Then loop through all the currently configured rules and create the new ones
-		for _, rule := range nrs.List() {
-			// When successfully deleted, re-create it again if it still exists
-			err := resourceCloudStackNetworkACLRuleCreateRule(d, meta, rule.(map[string]interface{}))
 
 			// We need to update this first to preserve the correct state
-			rules.Add(rule)
 			d.Set("rule", rules)
 
 			if err != nil {
@@ -423,26 +474,71 @@ func resourceCloudStackNetworkACLRuleUpdate(d *schema.ResourceData, meta interfa
 }
 
 func resourceCloudStackNetworkACLRuleDelete(d *schema.ResourceData, meta interface{}) error {
+	// Create an empty rule set to hold all rules that where
+	// not deleted correctly
+	rules := &schema.Set{
+		F: resourceCloudStackNetworkACLRuleHash,
+	}
+
 	// Delete all rules
 	if rs := d.Get("rule").(*schema.Set); rs.Len() > 0 {
-		for _, rule := range rs.List() {
+		err := resourceCloudStackNetworkACLRuleDeleteRules(d, meta, rules, rs)
-			// Delete a single rule
-			err := resourceCloudStackNetworkACLRuleDeleteRule(d, meta, rule.(map[string]interface{}))
 
 		// We need to update this first to preserve the correct state
-			d.Set("rule", rs)
+		d.Set("rule", rules)
 
 		if err != nil {
 			return err
 		}
 	}
-	}
 
 	return nil
 }
 
+func resourceCloudStackNetworkACLRuleDeleteRules(
+	d *schema.ResourceData,
+	meta interface{},
+	rules *schema.Set,
+	ors *schema.Set) error {
+	var errs *multierror.Error
+
+	var wg sync.WaitGroup
+	wg.Add(ors.Len())
+
+	sem := make(chan struct{}, 10)
+	for _, rule := range ors.List() {
+		// Put a sleep here to avoid DoS'ing the API
+		time.Sleep(500 * time.Millisecond)
+
+		go func(rule map[string]interface{}) {
+			defer wg.Done()
+			sem <- struct{}{}
+
+			// Delete a single rule
+			err := resourceCloudStackNetworkACLRuleDeleteRule(d, meta, rule)
+
+			// If we have at least one UUID, we need to save the rule
+			if len(rule["uuids"].(map[string]interface{})) > 0 {
+				rules.Add(rule)
+			}
+
+			if err != nil {
+				errs = multierror.Append(errs, err)
+			}
+
+			<-sem
+		}(rule.(map[string]interface{}))
+	}
+
+	wg.Wait()
+
+	return errs.ErrorOrNil()
+}
+
 func resourceCloudStackNetworkACLRuleDeleteRule(
-	d *schema.ResourceData, meta interface{}, rule map[string]interface{}) error {
+	d *schema.ResourceData,
+	meta interface{},
+	rule map[string]interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 	uuids := rule["uuids"].(map[string]interface{})
 
@@ -463,6 +559,7 @@ func resourceCloudStackNetworkACLRuleDeleteRule(
 				"Invalid parameter id value=%s due to incorrect long value format, "+
 					"or entity does not exist", id.(string))) {
 				delete(uuids, k)
+				rule["uuids"] = uuids
 				continue
 			}
 
@@ -471,10 +568,8 @@ func resourceCloudStackNetworkACLRuleDeleteRule(
 
 		// Delete the UUID of this rule
 		delete(uuids, k)
-	}
-
-	// Update the UUIDs
 		rule["uuids"] = uuids
+	}
 
 	return nil
 }

builtin/providers/cloudstack/resource_cloudstack_port_forward.go

@@ -150,6 +150,22 @@ func resourceCloudStackPortForwardCreateForward(
 func resourceCloudStackPortForwardRead(d *schema.ResourceData, meta interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 
+	// Get all the forwards from the running environment
+	p := cs.Firewall.NewListPortForwardingRulesParams()
+	p.SetIpaddressid(d.Id())
+	p.SetListall(true)
+
+	l, err := cs.Firewall.ListPortForwardingRules(p)
+	if err != nil {
+		return err
+	}
+
+	// Make a map of all the forwards so we can easily find a forward
+	forwardMap := make(map[string]*cloudstack.PortForwardingRule, l.Count)
+	for _, f := range l.PortForwardingRules {
+		forwardMap[f.Id] = f
+	}
+
 	// Create an empty schema.Set to hold all forwards
 	forwards := &schema.Set{
 		F: resourceCloudStackPortForwardHash,
@@ -166,36 +182,34 @@ func resourceCloudStackPortForwardRead(d *schema.ResourceData, meta interface{})
 			}
 
 			// Get the forward
-			r, count, err := cs.Firewall.GetPortForwardingRuleByID(id.(string))
+			f, ok := forwardMap[id.(string)]
-			// If the count == 0, there is no object found for this ID
+			if !ok {
-			if err != nil {
-				if count == 0 {
 				forward["uuid"] = ""
 				continue
 			}
 
-				return err
+			// Delete the known rule so only unknown rules remain in the ruleMap
-			}
+			delete(forwardMap, id.(string))
 
-			privPort, err := strconv.Atoi(r.Privateport)
+			privPort, err := strconv.Atoi(f.Privateport)
 			if err != nil {
 				return err
 			}
 
-			pubPort, err := strconv.Atoi(r.Publicport)
+			pubPort, err := strconv.Atoi(f.Publicport)
 			if err != nil {
 				return err
 			}
 
 			// Update the values
-			forward["protocol"] = r.Protocol
+			forward["protocol"] = f.Protocol
 			forward["private_port"] = privPort
 			forward["public_port"] = pubPort
 
 			if isID(forward["virtual_machine"].(string)) {
-				forward["virtual_machine"] = r.Virtualmachineid
+				forward["virtual_machine"] = f.Virtualmachineid
 			} else {
-				forward["virtual_machine"] = r.Virtualmachinename
+				forward["virtual_machine"] = f.Virtualmachinename
 			}
 
 			forwards.Add(forward)
@@ -204,33 +218,11 @@ func resourceCloudStackPortForwardRead(d *schema.ResourceData, meta interface{})
 
 	// If this is a managed resource, add all unknown forwards to dummy forwards
 	managed := d.Get("managed").(bool)
-	if managed {
+	if managed && len(forwardMap) > 0 {
-		// Get all the forwards from the running environment
+		for uuid := range forwardMap {
-		p := cs.Firewall.NewListPortForwardingRulesParams()
-		p.SetIpaddressid(d.Id())
-		p.SetListall(true)
-
-		r, err := cs.Firewall.ListPortForwardingRules(p)
-		if err != nil {
-			return err
-		}
-
-		// Add all UUIDs to the uuids map
-		uuids := make(map[string]interface{}, len(r.PortForwardingRules))
-		for _, r := range r.PortForwardingRules {
-			uuids[r.Id] = r.Id
-		}
-
-		// Delete all expected UUIDs from the uuids map
-		for _, forward := range forwards.List() {
-			forward := forward.(map[string]interface{})
-			delete(uuids, forward["uuid"].(string))
-		}
-
-		for uuid := range uuids {
 			// Make a dummy forward to hold the unknown UUID
 			forward := map[string]interface{}{
-				"protocol":        "N/A",
+				"protocol":        uuid,
 				"private_port":    0,
 				"public_port":     0,
 				"virtual_machine": uuid,

builtin/providers/cloudstack/resources.go

@@ -10,7 +10,7 @@ import (
 	"github.com/xanzy/go-cloudstack/cloudstack"
 )
 
-// CloudStack uses a "special" ID of -1 to define an unlimited resource
+// UnlimitedResourceID is a "special" ID to define an unlimited resource
 const UnlimitedResourceID = "-1"
 
 type retrieveError struct {
@@ -135,8 +135,8 @@ func Retry(n int, f RetryFunc) (interface{}, error) {
 
 	for i := 0; i < n; i++ {
 		r, err := f()
-		if err == nil {
+		if err == nil || err == cloudstack.AsyncTimeoutErr {
-			return r, nil
+			return r, err
 		}
 
 		lastErr = err