Merge pull request #4051 from svanharmelen/f-reduce-api-calls

provider/cloudstack: improve performance for all firewall related resources
hashicorp · Nov 25, 2015 · ca2a7ee · ca2a7ee
2 parents 164fb7c + a3eae45
commit ca2a7ee
Show file tree

Hide file tree

Showing 3 changed files with 132 additions and 159 deletions.
diff --git a/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go b/builtin/providers/cloudstack/resource_cloudstack_egress_firewall.go
@@ -203,6 +203,22 @@ func resourceCloudStackEgressFirewallCreateRule(
 func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 
+	// Get all the rules from the running environment
+	p := cs.Firewall.NewListEgressFirewallRulesParams()
+	p.SetNetworkid(d.Id())
+	p.SetListall(true)
+
+	l, err := cs.Firewall.ListEgressFirewallRules(p)
+	if err != nil {
+		return err
+	}
+
+	// Make a map of all the rules so we can easily find a rule
+	ruleMap := make(map[string]*cloudstack.EgressFirewallRule, l.Count)
+	for _, r := range l.EgressFirewallRules {
+		ruleMap[r.Id] = r
+	}
+
 	// Create an empty schema.Set to hold all rules
 	rules := &schema.Set{
 		F: resourceCloudStackEgressFirewallRuleHash,
@@ -221,17 +237,15 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 				}
 
 				// Get the rule
-				r, count, err := cs.Firewall.GetEgressFirewallRuleByID(id.(string))
-				// If the count == 0, there is no object found for this ID
-				if err != nil {
-					if count == 0 {
-						delete(uuids, "icmp")
-						continue
-					}
-
-					return err
+				r, ok := ruleMap[id.(string)]
+				if !ok {
+					delete(uuids, "icmp")
+					continue
 				}
 
+				// Delete the known rule so only unknown rules remain in the ruleMap
+				delete(ruleMap, id.(string))
+
 				// Update the values
 				rule["source_cidr"] = r.Cidrlist
 				rule["protocol"] = r.Protocol
@@ -259,16 +273,15 @@ func resourceCloudStackEgressFirewallRead(d *schema.ResourceData, meta interface
 						}
 
 						// Get the rule
-						r, count, err := cs.Firewall.GetEgressFirewallRuleByID(id.(string))
-						if err != nil {
-							if count == 0 {
-								delete(uuids, port.(string))
-								continue
-							}
-
-							return err
+						r, ok := ruleMap[id.(string)]
+						if !ok {
+							delete(uuids, port.(string))
+							continue
 						}
 
+						// Delete the known rule so only unknown rules remain in the ruleMap
+						delete(ruleMap, id.(string))
+
 						// Update the values
 						rule["source_cidr"] = r.Cidrlist
 						rule["protocol"] = r.Protocol
@@ -287,43 +300,22 @@ 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 {
-		// Get all the rules from the running environment
-		p := cs.Firewall.NewListEgressFirewallRulesParams()
-		p.SetNetworkid(d.Id())
-		p.SetListall(true)
-
-		r, err := cs.Firewall.ListEgressFirewallRules(p)
-		if err != nil {
-			return err
-		}
-
-		// Add all UUIDs to the uuids map
-		uuids := make(map[string]interface{}, len(r.EgressFirewallRules))
-		for _, r := range r.EgressFirewallRules {
-			uuids[r.Id] = r.Id
+	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
 		}
 
-		// Delete all expected UUIDs from the uuids map
-		for _, rule := range rules.List() {
-			rule := rule.(map[string]interface{})
-
-			for _, id := range rule["uuids"].(map[string]interface{}) {
-				delete(uuids, id.(string))
-			}
+		// Make a dummy rule to hold all unknown UUIDs
+		rule := map[string]interface{}{
+			"source_cidr": "N/A",
+			"protocol":    "N/A",
+			"uuids":       ruleMap,
 		}
 
-		if len(uuids) > 0 {
-			// Make a dummy rule to hold all unknown UUIDs
-			rule := map[string]interface{}{
-				"source_cidr": "N/A",
-				"protocol":    "N/A",
-				"uuids":       uuids,
-			}
-
-			// Add the dummy rule to the rules set
-			rules.Add(rule)
-		}
+		// Add the dummy rule to the rules set
+		rules.Add(rule)
 	}
 
 	if rules.Len() > 0 {

diff --git a/builtin/providers/cloudstack/resource_cloudstack_firewall.go b/builtin/providers/cloudstack/resource_cloudstack_firewall.go
@@ -203,6 +203,22 @@ func resourceCloudStackFirewallCreateRule(
 func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) error {
 	cs := meta.(*cloudstack.CloudStackClient)
 
+	// Get all the rules from the running environment
+	p := cs.Firewall.NewListFirewallRulesParams()
+	p.SetIpaddressid(d.Id())
+	p.SetListall(true)
+
+	l, err := cs.Firewall.ListFirewallRules(p)
+	if err != nil {
+		return err
+	}
+
+	// Make a map of all the rules so we can easily find a rule
+	ruleMap := make(map[string]*cloudstack.FirewallRule, l.Count)
+	for _, r := range l.FirewallRules {
+		ruleMap[r.Id] = r
+	}
+
 	// Create an empty schema.Set to hold all rules
 	rules := &schema.Set{
 		F: resourceCloudStackFirewallRuleHash,
@@ -221,17 +237,15 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
 				}
 
 				// Get the rule
-				r, count, err := cs.Firewall.GetFirewallRuleByID(id.(string))
-				// If the count == 0, there is no object found for this ID
-				if err != nil {
-					if count == 0 {
-						delete(uuids, "icmp")
-						continue
-					}
-
-					return err
+				r, ok := ruleMap[id.(string)]
+				if !ok {
+					delete(uuids, "icmp")
+					continue
 				}
 
+				// Delete the known rule so only unknown rules remain in the ruleMap
+				delete(ruleMap, id.(string))
+
 				// Update the values
 				rule["source_cidr"] = r.Cidrlist
 				rule["protocol"] = r.Protocol
@@ -259,16 +273,15 @@ func resourceCloudStackFirewallRead(d *schema.ResourceData, meta interface{}) er
 						}
 
 						// Get the rule
-						r, count, err := cs.Firewall.GetFirewallRuleByID(id.(string))
-						if err != nil {
-							if count == 0 {
-								delete(uuids, port.(string))
-								continue
-							}
-
-							return err
+						r, ok := ruleMap[id.(string)]
+						if !ok {
+							delete(uuids, port.(string))
+							continue
 						}
 
+						// Delete the known rule so only unknown rules remain in the ruleMap
+						delete(ruleMap, id.(string))
+
 						// Update the values
 						rule["source_cidr"] = r.Cidrlist
 						rule["protocol"] = r.Protocol
@@ -287,43 +300,22 @@ 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 {
-		// Get all the rules from the running environment
-		p := cs.Firewall.NewListFirewallRulesParams()
-		p.SetIpaddressid(d.Id())
-		p.SetListall(true)
-
-		r, err := cs.Firewall.ListFirewallRules(p)
-		if err != nil {
-			return err
-		}
-
-		// Add all UUIDs to the uuids map
-		uuids := make(map[string]interface{}, len(r.FirewallRules))
-		for _, r := range r.FirewallRules {
-			uuids[r.Id] = r.Id
+	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
 		}
 
-		// Delete all expected UUIDs from the uuids map
-		for _, rule := range rules.List() {
-			rule := rule.(map[string]interface{})
-
-			for _, id := range rule["uuids"].(map[string]interface{}) {
-				delete(uuids, id.(string))
-			}
+		// Make a dummy rule to hold all unknown UUIDs
+		rule := map[string]interface{}{
+			"source_cidr": "N/A",
+			"protocol":    "N/A",
+			"uuids":       uuids,
 		}
 
-		if len(uuids) > 0 {
-			// Make a dummy rule to hold all unknown UUIDs
-			rule := map[string]interface{}{
-				"source_cidr": "N/A",
-				"protocol":    "N/A",
-				"uuids":       uuids,
-			}
-
-			// Add the dummy rule to the rules set
-			rules.Add(rule)
-		}
+		// Add the dummy rule to the rules set
+		rules.Add(rule)
 	}
 
 	if rules.Len() > 0 {