Merge pull request #6 from terraform-providers/fix-deadlock

Fixing the Deadlock issue
hashicorp · Jun 19, 2017 · 94568d9 · 94568d9
2 parents 9f21693 + bcb5c86
commit 94568d9
Show file tree

Hide file tree

Showing 10 changed files with 345 additions and 58 deletions.
diff --git a/azurerm/locks.go b/azurerm/locks.go
@@ -1,12 +1,24 @@
 package azurerm
 
-func azureRMUnlockMultiple(names *[]string) {
+// handle the case of using the same name for different kinds of resources
+func azureRMLockByName(name string, resourceType string) {
+	updatedName := resourceType + "." + name
+	armMutexKV.Lock(updatedName)
+}
+
+func azureRMLockMultipleByName(names *[]string, resourceType string) {
 	for _, name := range *names {
-		armMutexKV.Unlock(name)
+		azureRMLockByName(name, resourceType)
 	}
 }
-func azureRMLockMultiple(names *[]string) {
+
+func azureRMUnlockByName(name string, resourceType string) {
+	updatedName := resourceType + "." + name
+	armMutexKV.Unlock(updatedName)
+}
+
+func azureRMUnlockMultipleByName(names *[]string, resourceType string) {
 	for _, name := range *names {
-		armMutexKV.Lock(name)
+		azureRMUnlockByName(name, resourceType)
 	}
 }
diff --git a/azurerm/resource_arm_network_interface_card.go b/azurerm/resource_arm_network_interface_card.go
@@ -178,8 +178,8 @@ func resourceArmNetworkInterfaceCreate(d *schema.ResourceData, meta interface{})
 			return err
 		}
 
-		armMutexKV.Lock(networkSecurityGroupName)
-		defer armMutexKV.Unlock(networkSecurityGroupName)
+		azureRMLockByName(networkSecurityGroupName, networkSecurityGroupResourceName)
+		defer azureRMUnlockByName(networkSecurityGroupName, networkSecurityGroupResourceName)
 	}
 
 	dns, hasDns := d.GetOk("dns_servers")
@@ -205,13 +205,16 @@ func resourceArmNetworkInterfaceCreate(d *schema.ResourceData, meta interface{})
 		properties.DNSSettings = &ifaceDnsSettings
 	}
 
-	ipConfigs, namesToLock, sgErr := expandAzureRmNetworkInterfaceIpConfigurations(d)
+	ipConfigs, subnetnToLock, vnnToLock, sgErr := expandAzureRmNetworkInterfaceIpConfigurations(d)
 	if sgErr != nil {
 		return fmt.Errorf("Error Building list of Network Interface IP Configurations: %s", sgErr)
 	}
 
-	azureRMLockMultiple(namesToLock)
-	defer azureRMUnlockMultiple(namesToLock)
+	azureRMLockMultipleByName(subnetnToLock, subnetResourceName)
+	defer azureRMUnlockMultipleByName(subnetnToLock, subnetResourceName)
+
+	azureRMLockMultipleByName(vnnToLock, virtualNetworkResourceName)
+	defer azureRMUnlockMultipleByName(vnnToLock, virtualNetworkResourceName)
 
 	if len(ipConfigs) > 0 {
 		properties.IPConfigurations = &ipConfigs
@@ -327,12 +330,13 @@ func resourceArmNetworkInterfaceDelete(d *schema.ResourceData, meta interface{})
 			return err
 		}
 
-		armMutexKV.Lock(networkSecurityGroupName)
-		defer armMutexKV.Unlock(networkSecurityGroupName)
+		azureRMLockByName(networkSecurityGroupName, networkSecurityGroupResourceName)
+		defer azureRMUnlockByName(networkSecurityGroupName, networkSecurityGroupResourceName)
 	}
 
 	configs := d.Get("ip_configuration").(*schema.Set).List()
-	namesToLock := make([]string, 0)
+	subnetNamesToLock := make([]string, 0)
+	virtualNetworkNamesToLock := make([]string, 0)
 
 	for _, configRaw := range configs {
 		data := configRaw.(map[string]interface{})
@@ -343,13 +347,17 @@ func resourceArmNetworkInterfaceDelete(d *schema.ResourceData, meta interface{})
 			return err
 		}
 		subnetName := subnetId.Path["subnets"]
+		subnetNamesToLock = append(subnetNamesToLock, subnetName)
+
 		virtualNetworkName := subnetId.Path["virtualNetworks"]
-		namesToLock = append(namesToLock, subnetName)
-		namesToLock = append(namesToLock, virtualNetworkName)
+		virtualNetworkNamesToLock = append(virtualNetworkNamesToLock, virtualNetworkName)
 	}
 
-	azureRMLockMultiple(&namesToLock)
-	defer azureRMUnlockMultiple(&namesToLock)
+	azureRMLockMultipleByName(&subnetNamesToLock, subnetResourceName)
+	defer azureRMUnlockMultipleByName(&subnetNamesToLock, subnetResourceName)
+
+	azureRMLockMultipleByName(&virtualNetworkNamesToLock, virtualNetworkResourceName)
+	defer azureRMUnlockMultipleByName(&virtualNetworkNamesToLock, virtualNetworkResourceName)
 
 	_, error := ifaceClient.Delete(resGroup, name, make(chan struct{}))
 	err = <-error
@@ -398,10 +406,11 @@ func validateNetworkInterfacePrivateIpAddressAllocation(v interface{}, k string)
 	return
 }
 
-func expandAzureRmNetworkInterfaceIpConfigurations(d *schema.ResourceData) ([]network.InterfaceIPConfiguration, *[]string, error) {
+func expandAzureRmNetworkInterfaceIpConfigurations(d *schema.ResourceData) ([]network.InterfaceIPConfiguration, *[]string, *[]string, error) {
 	configs := d.Get("ip_configuration").(*schema.Set).List()
 	ipConfigs := make([]network.InterfaceIPConfiguration, 0, len(configs))
-	namesToLock := make([]string, 0)
+	subnetNamesToLock := make([]string, 0)
+	virtualNetworkNamesToLock := make([]string, 0)
 
 	for _, configRaw := range configs {
 		data := configRaw.(map[string]interface{})
@@ -416,7 +425,7 @@ func expandAzureRmNetworkInterfaceIpConfigurations(d *schema.ResourceData) ([]ne
 		case "static":
 			allocationMethod = network.Static
 		default:
-			return []network.InterfaceIPConfiguration{}, nil, fmt.Errorf(
+			return []network.InterfaceIPConfiguration{}, nil, nil, fmt.Errorf(
 				"valid values for private_ip_allocation_method are 'dynamic' and 'static' - got '%s'",
 				private_ip_allocation_method)
 		}
@@ -430,12 +439,12 @@ func expandAzureRmNetworkInterfaceIpConfigurations(d *schema.ResourceData) ([]ne
 
 		subnetId, err := parseAzureResourceID(subnet_id)
 		if err != nil {
-			return []network.InterfaceIPConfiguration{}, nil, err
+			return []network.InterfaceIPConfiguration{}, nil, nil, err
 		}
 		subnetName := subnetId.Path["subnets"]
 		virtualNetworkName := subnetId.Path["virtualNetworks"]
-		namesToLock = append(namesToLock, subnetName)
-		namesToLock = append(namesToLock, virtualNetworkName)
+		subnetNamesToLock = append(subnetNamesToLock, subnetName)
+		virtualNetworkNamesToLock = append(virtualNetworkNamesToLock, virtualNetworkName)
 
 		if v := data["private_ip_address"].(string); v != "" {
 			properties.PrivateIPAddress = &v
@@ -486,5 +495,5 @@ func expandAzureRmNetworkInterfaceIpConfigurations(d *schema.ResourceData) ([]ne
 		ipConfigs = append(ipConfigs, ipConfig)
 	}
 
-	return ipConfigs, &namesToLock, nil
+	return ipConfigs, &subnetNamesToLock, &virtualNetworkNamesToLock, nil
 }
diff --git a/azurerm/resource_arm_network_interface_card_test.go b/azurerm/resource_arm_network_interface_card_test.go
@@ -116,6 +116,23 @@ func TestAccAzureRMNetworkInterface_withTags(t *testing.T) {
 	})
 }
 
+func TestAccAzureRMNetworkInterface_bug7986(t *testing.T) {
+	rInt := acctest.RandInt()
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testCheckAzureRMNetworkInterfaceDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAzureRMNetworkInterface_bug7986(rInt),
+				Check: resource.ComposeTestCheckFunc(
+					testCheckAzureRMNetworkInterfaceExists("azurerm_network_interface.test"),
+				),
+			},
+		},
+	})
+}
+
 func testCheckAzureRMNetworkInterfaceExists(name string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		// Ensure we have enough information in state to look up in API
@@ -475,3 +492,111 @@ resource "azurerm_network_interface" "test2" {
 }
 `, rInt)
 }
+
+func testAccAzureRMNetworkInterface_bug7986(rInt int) string {
+	return fmt.Sprintf(`
+resource "azurerm_resource_group" "test" {
+  name     = "acctest-%d"
+  location = "West Europe"
+}
+
+resource "azurerm_network_security_group" "test" {
+  name                = "acctest-%d-nsg"
+  location            = "${azurerm_resource_group.test.location}"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+
+  security_rule {
+    name                       = "test123"
+    priority                   = 100
+    direction                  = "Inbound"
+    access                     = "Allow"
+    protocol                   = "Tcp"
+    source_port_range          = "*"
+    destination_port_range     = "*"
+    source_address_prefix      = "*"
+    destination_address_prefix = "*"
+  }
+
+  tags {
+    environment = "Production"
+  }
+}
+
+resource "azurerm_network_security_rule" "test" {
+  name                        = "test"
+  priority                    = 100
+  direction                   = "Outbound"
+  access                      = "Allow"
+  protocol                    = "Tcp"
+  source_port_range           = "*"
+  destination_port_range      = "*"
+  source_address_prefix       = "*"
+  destination_address_prefix  = "*"
+  resource_group_name         = "${azurerm_resource_group.test.name}"
+  network_security_group_name = "${azurerm_network_security_group.test.name}"
+}
+
+resource "azurerm_public_ip" "test" {
+  name                         = "acctest-%d-pip"
+  location                     = "${azurerm_resource_group.test.location}"
+  resource_group_name          = "${azurerm_resource_group.test.name}"
+  public_ip_address_allocation = "Dynamic"
+
+  tags {
+    environment = "Production"
+  }
+}
+
+resource "azurerm_virtual_network" "test" {
+  name                = "acctest-%d-vn"
+  address_space       = ["10.0.0.0/16"]
+  resource_group_name = "${azurerm_resource_group.test.name}"
+  location            = "${azurerm_resource_group.test.location}"
+
+  subnet {
+    name           = "second"
+    address_prefix = "10.0.3.0/24"
+    security_group = "${azurerm_network_security_group.test.id}"
+  }
+}
+
+resource "azurerm_subnet" "test" {
+  name                 = "first"
+  resource_group_name  = "${azurerm_resource_group.test.name}"
+  virtual_network_name = "${azurerm_virtual_network.test.name}"
+  address_prefix       = "10.0.2.0/24"
+}
+
+resource "azurerm_network_interface" "test1" {
+  name                = "acctest-%d-nic1"
+  location            = "${azurerm_resource_group.test.location}"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+
+  ip_configuration {
+    name                          = "testconfiguration1"
+    subnet_id                     = "${azurerm_subnet.test.id}"
+    private_ip_address_allocation = "dynamic"
+  }
+
+  tags {
+    environment = "staging"
+  }
+}
+
+resource "azurerm_network_interface" "test2" {
+  name                = "acctest-%d-nic2"
+  location            = "${azurerm_resource_group.test.location}"
+  resource_group_name = "${azurerm_resource_group.test.name}"
+
+  ip_configuration {
+    name                          = "testconfiguration1"
+    subnet_id                     = "${azurerm_subnet.test.id}"
+    private_ip_address_allocation = "dynamic"
+  }
+
+  tags {
+    environment = "staging"
+  }
+}
+`, rInt, rInt, rInt, rInt, rInt, rInt)
+}
diff --git a/azurerm/resource_arm_network_security_group.go b/azurerm/resource_arm_network_security_group.go
@@ -13,6 +13,8 @@ import (
 	"github.com/hashicorp/terraform/helper/schema"
 )
 
+var networkSecurityGroupResourceName = "azurerm_network_security_group"
+
 func resourceArmNetworkSecurityGroup() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceArmNetworkSecurityGroupCreate,
@@ -137,6 +139,9 @@ func resourceArmNetworkSecurityGroupCreate(d *schema.ResourceData, meta interfac
 		return fmt.Errorf("Error Building list of Network Security Group Rules: %s", sgErr)
 	}
 
+	azureRMLockByName(name, networkSecurityGroupResourceName)
+	defer azureRMUnlockByName(name, networkSecurityGroupResourceName)
+
 	sg := network.SecurityGroup{
 		Name:     &name,
 		Location: &location,

diff --git a/azurerm/resource_arm_network_security_rule.go b/azurerm/resource_arm_network_security_rule.go
@@ -120,8 +120,8 @@ func resourceArmNetworkSecurityRuleCreate(d *schema.ResourceData, meta interface
 	direction := d.Get("direction").(string)
 	protocol := d.Get("protocol").(string)
 
-	armMutexKV.Lock(nsgName)
-	defer armMutexKV.Unlock(nsgName)
+	azureRMLockByName(nsgName, networkSecurityGroupResourceName)
+	defer azureRMUnlockByName(nsgName, networkSecurityGroupResourceName)
 
 	properties := network.SecurityRulePropertiesFormat{
 		SourcePortRange:          &source_port_range,
@@ -155,8 +155,7 @@ func resourceArmNetworkSecurityRuleCreate(d *schema.ResourceData, meta interface
 		return err
 	}
 	if read.ID == nil {
-		return fmt.Errorf("Cannot read Security Group Rule %s/%s (resource group %s) ID",
-			nsgName, name, resGroup)
+		return fmt.Errorf("Cannot read Security Group Rule %s/%s (resource group %s) ID", nsgName, name, resGroup)
 	}
 
 	d.SetId(*read.ID)
@@ -211,8 +210,8 @@ func resourceArmNetworkSecurityRuleDelete(d *schema.ResourceData, meta interface
 	nsgName := id.Path["networkSecurityGroups"]
 	sgRuleName := id.Path["securityRules"]
 
-	armMutexKV.Lock(nsgName)
-	defer armMutexKV.Unlock(nsgName)
+	azureRMLockByName(nsgName, networkSecurityGroupResourceName)
+	defer azureRMUnlockByName(nsgName, networkSecurityGroupResourceName)
 
 	_, error := secRuleClient.Delete(resGroup, nsgName, sgRuleName, make(chan struct{}))
 	err = <-error

diff --git a/azurerm/resource_arm_route.go b/azurerm/resource_arm_route.go
@@ -72,8 +72,8 @@ func resourceArmRouteCreate(d *schema.ResourceData, meta interface{}) error {
 	addressPrefix := d.Get("address_prefix").(string)
 	nextHopType := d.Get("next_hop_type").(string)
 
-	armMutexKV.Lock(rtName)
-	defer armMutexKV.Unlock(rtName)
+	azureRMLockByName(rtName, routeTableResourceName)
+	defer azureRMUnlockByName(rtName, routeTableResourceName)
 
 	properties := network.RoutePropertiesFormat{
 		AddressPrefix: &addressPrefix,
@@ -153,8 +153,8 @@ func resourceArmRouteDelete(d *schema.ResourceData, meta interface{}) error {
 	rtName := id.Path["routeTables"]
 	routeName := id.Path["routes"]
 
-	armMutexKV.Lock(rtName)
-	defer armMutexKV.Unlock(rtName)
+	azureRMLockByName(rtName, routeTableResourceName)
+	defer azureRMUnlockByName(rtName, routeTableResourceName)
 
 	_, error := routesClient.Delete(resGroup, rtName, routeName, make(chan struct{}))
 	err = <-error

diff --git a/azurerm/resource_arm_route_table.go b/azurerm/resource_arm_route_table.go
@@ -12,6 +12,8 @@ import (
 	"github.com/hashicorp/terraform/helper/schema"
 )
 
+var routeTableResourceName = "azurerm_route_table"
+
 func resourceArmRouteTable() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceArmRouteTableCreate,