provider/Azure: various bugfixes

builtin/providers/azure/config.go

@@ -35,8 +35,6 @@ type Client struct {
 
 	hostedServiceClient hostedservice.HostedServiceClient
 
-	secGroupClient networksecuritygroup.SecurityGroupClient
-
 	osImageClient osimage.OSImageClient
 
 	sqlClient sql.SQLDatabaseClient
@@ -52,7 +50,11 @@ type Client struct {
 	// unfortunately; because of how Azure's network API works; doing networking operations
 	// concurrently is very hazardous, and we need a mutex to guard the VirtualNetworkClient.
 	vnetClient virtualnetwork.VirtualNetworkClient
-	mutex      *sync.Mutex
+	vnetMutex  *sync.Mutex
+
+	// same as the above for security group rule operations:
+	secGroupClient networksecuritygroup.SecurityGroupClient
+	secGroupMutex  *sync.Mutex
 }
 
 // getStorageClientForStorageService is helper method which returns the
@@ -106,14 +108,15 @@ func (c *Config) NewClientFromSettingsData() (*Client, error) {
 		affinityGroupClient:  affinitygroup.NewClient(mc),
 		hostedServiceClient:  hostedservice.NewClient(mc),
 		secGroupClient:       networksecuritygroup.NewClient(mc),
+		secGroupMutex:        &sync.Mutex{},
 		osImageClient:        osimage.NewClient(mc),
 		sqlClient:            sql.NewClient(mc),
 		storageServiceClient: storageservice.NewClient(mc),
 		vmClient:             virtualmachine.NewClient(mc),
 		vmDiskClient:         virtualmachinedisk.NewClient(mc),
 		vmImageClient:        virtualmachineimage.NewClient(mc),
 		vnetClient:           virtualnetwork.NewClient(mc),
-		mutex:                &sync.Mutex{},
+		vnetMutex:            &sync.Mutex{},
 	}, nil
 }
 
@@ -130,13 +133,14 @@ func (c *Config) NewClient() (*Client, error) {
 		affinityGroupClient:  affinitygroup.NewClient(mc),
 		hostedServiceClient:  hostedservice.NewClient(mc),
 		secGroupClient:       networksecuritygroup.NewClient(mc),
+		secGroupMutex:        &sync.Mutex{},
 		osImageClient:        osimage.NewClient(mc),
 		sqlClient:            sql.NewClient(mc),
 		storageServiceClient: storageservice.NewClient(mc),
 		vmClient:             virtualmachine.NewClient(mc),
 		vmDiskClient:         virtualmachinedisk.NewClient(mc),
 		vmImageClient:        virtualmachineimage.NewClient(mc),
 		vnetClient:           virtualnetwork.NewClient(mc),
-		mutex:                &sync.Mutex{},
+		vnetMutex:            &sync.Mutex{},
 	}, nil
 }

builtin/providers/azure/resource_azure_dns_server.go

@@ -43,8 +43,8 @@ func resourceAzureDnsServerCreate(d *schema.ResourceData, meta interface{}) erro
 	vnetClient := azureClient.vnetClient
 
 	log.Println("[INFO] Fetching current network configuration from Azure.")
-	azureClient.mutex.Lock()
-	defer azureClient.mutex.Unlock()
+	azureClient.vnetMutex.Lock()
+	defer azureClient.vnetMutex.Unlock()
 	netConf, err := vnetClient.GetVirtualNetworkConfiguration()
 	if err != nil {
 		if management.IsResourceNotFoundError(err) {
@@ -124,8 +124,8 @@ func resourceAzureDnsServerUpdate(d *schema.ResourceData, meta interface{}) erro
 	if d.HasChange("dns_address") {
 		log.Println("[DEBUG] DNS server address has changes; updating it on Azure.")
 		log.Println("[INFO] Fetching current network configuration from Azure.")
-		azureClient.mutex.Lock()
-		defer azureClient.mutex.Unlock()
+		azureClient.vnetMutex.Lock()
+		defer azureClient.vnetMutex.Unlock()
 		netConf, err := vnetClient.GetVirtualNetworkConfiguration()
 		if err != nil {
 			return fmt.Errorf("Failed to get the current network configuration from Azure: %s", err)
@@ -198,8 +198,8 @@ func resourceAzureDnsServerDelete(d *schema.ResourceData, meta interface{}) erro
 	vnetClient := azureClient.vnetClient
 
 	log.Println("[INFO] Fetching current network configuration from Azure.")
-	azureClient.mutex.Lock()
-	defer azureClient.mutex.Unlock()
+	azureClient.vnetMutex.Lock()
+	defer azureClient.vnetMutex.Unlock()
 	netConf, err := vnetClient.GetVirtualNetworkConfiguration()
 	if err != nil {
 		return fmt.Errorf("Failed to get the current network configuration from Azure: %s", err)

builtin/providers/azure/resource_azure_instance.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"log"
 	"strings"
+	"time"
 
 	"github.com/Azure/azure-sdk-for-go/management"
 	"github.com/Azure/azure-sdk-for-go/management/hostedservice"
@@ -14,13 +15,16 @@ import (
 	"github.com/Azure/azure-sdk-for-go/management/virtualmachineimage"
 	"github.com/Azure/azure-sdk-for-go/management/vmutils"
 	"github.com/hashicorp/terraform/helper/hashcode"
+	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
 )
 
 const (
-	linux                = "Linux"
-	windows              = "Windows"
-	osDiskBlobStorageURL = "http://%s.blob.core.windows.net/vhds/%s.vhd"
+	linux                 = "Linux"
+	windows               = "Windows"
+	storageContainterName = "vhds"
+	osDiskBlobNameFormat  = "%s.vhd"
+	osDiskBlobStorageURL  = "http://%s.blob.core.windows.net/" + storageContainterName + "/" + osDiskBlobNameFormat
 )
 
 func resourceAzureInstance() *schema.Resource {
@@ -44,6 +48,16 @@ func resourceAzureInstance() *schema.Resource {
 				ForceNew: true,
 			},
 
+			// in order to prevent an unintentional delete of a containing
+			// hosted service in the case the same name are given to both the
+			// service and the instance despite their being created separately,
+			// we must maintain a flag to definitively denote whether this
+			// instance had a hosted service created for it or not:
+			"has_dedicated_service": &schema.Schema{
+				Type:     schema.TypeBool,
+				Computed: true,
+			},
+
 			"description": &schema.Schema{
 				Type:     schema.TypeString,
 				Optional: true,
@@ -234,6 +248,7 @@ func resourceAzureInstanceCreate(d *schema.ResourceData, meta interface{}) (err
 		// if not provided; just use the name of the instance to create a new one:
 		hostedServiceName = name
 		d.Set("hosted_service_name", hostedServiceName)
+		d.Set("has_dedicated_service", true)
 
 		p := hostedservice.CreateHostedServiceParameters{
 			ServiceName:    hostedServiceName,
@@ -560,16 +575,16 @@ func resourceAzureInstanceDelete(d *schema.ResourceData, meta interface{}) error
 	azureClient := meta.(*Client)
 	mc := azureClient.mgmtClient
 	vmClient := azureClient.vmClient
-	hostedServiceClient := azureClient.hostedServiceClient
 
 	name := d.Get("name").(string)
 	hostedServiceName := d.Get("hosted_service_name").(string)
 
 	log.Printf("[DEBUG] Deleting instance: %s", name)
 
 	// check if the instance had a hosted service created especially for it:
-	if name == hostedServiceName {
+	if d.Get("has_dedicated_service").(bool) {
 		// if so; we must delete the associated hosted service as well:
+		hostedServiceClient := azureClient.hostedServiceClient
 		req, err := hostedServiceClient.DeleteHostedService(name, true)
 		if err != nil {
 			return fmt.Errorf("Error deleting instance and hosted service %s: %s", name, err)
@@ -594,7 +609,35 @@ func resourceAzureInstanceDelete(d *schema.ResourceData, meta interface{}) error
 		}
 	}
 
-	return nil
+	log.Printf("[INFO] Waiting for the deletion of instance '%s''s disk blob.", name)
+
+	// in order to avoid `terraform taint`-like scenarios in which the instance
+	// is deleted and re-created so fast the previous storage blob which held
+	// the image doesn't manage to get deleted (despite it being in a
+	// 'deleting' state) and a lease conflict occurs over it, we must ensure
+	// the blob got completely deleted as well:
+	storName := d.Get("storage_service_name").(string)
+	blobClient, err := azureClient.getStorageServiceBlobClient(storName)
+	if err != nil {
+		return err
+	}
+
+	err = resource.Retry(5*time.Minute, func() error {
+		exists, err := blobClient.BlobExists(
+			storageContainterName, fmt.Sprintf(osDiskBlobNameFormat, name),
+		)
+		if err != nil {
+			return resource.RetryError{Err: err}
+		}
+
+		if exists {
+			return fmt.Errorf("Instance '%s''s disk storage blob still exists.", name)
+		}
+
+		return nil
+	})
+
+	return err
 }
 
 func resourceAzureEndpointHash(v interface{}) int {
@@ -674,6 +717,7 @@ func retrieveOSImageDetails(
 	label string,
 	name string,
 	storage string) (func(*virtualmachine.Role) error, string, []string, error) {
+
 	imgs, err := osImageClient.ListOSImages()
 	if err != nil {
 		return nil, "", nil, fmt.Errorf("Error retrieving image details: %s", err)

builtin/providers/azure/resource_azure_local_network.go

@@ -51,8 +51,8 @@ func resourceAzureLocalNetworkConnectionCreate(d *schema.ResourceData, meta inte
 	vnetClient := azureClient.vnetClient
 
 	log.Println("[INFO] Fetching current network configuration from Azure.")
-	azureClient.mutex.Lock()
-	defer azureClient.mutex.Unlock()
+	azureClient.vnetMutex.Lock()
+	defer azureClient.vnetMutex.Unlock()
 	netConf, err := vnetClient.GetVirtualNetworkConfiguration()
 	if err != nil {
 		if management.IsResourceNotFoundError(err) {
@@ -138,8 +138,8 @@ func resourceAzureLocalNetworkConnectionUpdate(d *schema.ResourceData, meta inte
 	vnetClient := azureClient.vnetClient
 
 	log.Println("[INFO] Fetching current network configuration from Azure.")
-	azureClient.mutex.Lock()
-	defer azureClient.mutex.Unlock()
+	azureClient.vnetMutex.Lock()
+	defer azureClient.vnetMutex.Unlock()
 	netConf, err := vnetClient.GetVirtualNetworkConfiguration()
 	if err != nil {
 		return fmt.Errorf("Failed to get the current network configuration from Azure: %s", err)
@@ -217,8 +217,8 @@ func resourceAzureLocalNetworkConnectionDelete(d *schema.ResourceData, meta inte
 	vnetClient := azureClient.vnetClient
 
 	log.Println("[INFO] Fetching current network configuration from Azure.")
-	azureClient.mutex.Lock()
-	defer azureClient.mutex.Unlock()
+	azureClient.vnetMutex.Lock()
+	defer azureClient.vnetMutex.Unlock()
 	netConf, err := vnetClient.GetVirtualNetworkConfiguration()
 	if err != nil {
 		return fmt.Errorf("Failed to get the current network configuration from Azure: %s", err)

builtin/providers/azure/resource_azure_security_group_rule.go

@@ -86,6 +86,9 @@ func resourceAzureSecurityGroupRuleCreate(d *schema.ResourceData, meta interface
 	mgmtClient := azureClient.mgmtClient
 	secGroupClient := azureClient.secGroupClient
 
+	azureClient.secGroupMutex.Lock()
+	defer azureClient.secGroupMutex.Unlock()
+
 	// create and configure the RuleResponse:
 	name := d.Get("name").(string)
 	rule := netsecgroup.RuleRequest{
@@ -183,6 +186,9 @@ func resourceAzureSecurityGroupRuleUpdate(d *schema.ResourceData, meta interface
 	mgmtClient := azureClient.mgmtClient
 	secGroupClient := azureClient.secGroupClient
 
+	azureClient.secGroupMutex.Lock()
+	defer azureClient.secGroupMutex.Unlock()
+
 	var found bool
 	name := d.Get("name").(string)
 	newRule := netsecgroup.RuleRequest{
@@ -262,6 +268,9 @@ func resourceAzureSecurityGroupRuleDelete(d *schema.ResourceData, meta interface
 	mgmtClient := azureClient.mgmtClient
 	secGroupClient := azureClient.secGroupClient
 
+	azureClient.secGroupMutex.Lock()
+	defer azureClient.secGroupMutex.Unlock()
+
 	name := d.Get("name").(string)
 	secGroupNames := d.Get("security_group_names").(*schema.Set).List()
 	for _, sg := range secGroupNames {

builtin/providers/azure/resource_azure_virtual_network.go

@@ -82,8 +82,8 @@ func resourceAzureVirtualNetworkCreate(d *schema.ResourceData, meta interface{})
 
 	// Lock the client just before we get the virtual network configuration and immediately
 	// set an defer to unlock the client again whenever this function exits
-	ac.mutex.Lock()
-	defer ac.mutex.Unlock()
+	ac.vnetMutex.Lock()
+	defer ac.vnetMutex.Unlock()
 
 	nc, err := vnetClient.GetVirtualNetworkConfiguration()
 	if err != nil {
@@ -181,8 +181,8 @@ func resourceAzureVirtualNetworkUpdate(d *schema.ResourceData, meta interface{})
 
 	// Lock the client just before we get the virtual network configuration and immediately
 	// set an defer to unlock the client again whenever this function exits
-	ac.mutex.Lock()
-	defer ac.mutex.Unlock()
+	ac.vnetMutex.Lock()
+	defer ac.vnetMutex.Unlock()
 
 	nc, err := vnetClient.GetVirtualNetworkConfiguration()
 	if err != nil {
@@ -227,8 +227,8 @@ func resourceAzureVirtualNetworkDelete(d *schema.ResourceData, meta interface{})
 
 	// Lock the client just before we get the virtual network configuration and immediately
 	// set an defer to unlock the client again whenever this function exits
-	ac.mutex.Lock()
-	defer ac.mutex.Unlock()
+	ac.vnetMutex.Lock()
+	defer ac.vnetMutex.Unlock()
 
 	nc, err := vnetClient.GetVirtualNetworkConfiguration()
 	if err != nil {

website/source/docs/providers/azure/r/security_group_rule.html.markdown

@@ -8,7 +8,19 @@ description: |-
 
 # azure\_security\_group\_rule
 
-Creates a new network security rule to be associated with a given security group.
+Creates a new network Security Group Rule to be associated with a number of
+given Security Groups.
+
+~> **NOTE on Security Group Rules**: for usability purposes; Terraform allows the
+addition of a single Security Group Rule to multiple Security Groups, despite
+it having to define each rule individually per Security Group on Azure. As a
+result; in the event that one of the Rules on one of the Groups is modified by
+external factors, Terraform cannot reason as to whether or not that change
+should be propagated to the others; let alone choose one changed Rule
+configuration over another in case of a conflic. As such; `terraform refresh`
+only checks that the rule is still defined for each of the specified
+`security_group_names`; ignoring the actual parameters of the Rule and **not**
+updating the state with regards to them.
 
 ## Example Usage