Skip to content

Commit

Permalink
provider/Azure: fixes:
Browse files Browse the repository at this point in the history
added wait on instance deletion for associated blob deletion.
added guarding Mutex for secgroup-rule-related concurrent operations.
added usage warning on secgroup rules.
  • Loading branch information
aznashwan committed Oct 29, 2015
1 parent ec2d22c commit 172d19d
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 29 deletions.
14 changes: 9 additions & 5 deletions builtin/providers/azure/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ type Client struct {

hostedServiceClient hostedservice.HostedServiceClient

secGroupClient networksecuritygroup.SecurityGroupClient

osImageClient osimage.OSImageClient

sqlClient sql.SQLDatabaseClient
Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand All @@ -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
}
12 changes: 6 additions & 6 deletions builtin/providers/azure/resource_azure_dns_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
56 changes: 51 additions & 5 deletions builtin/providers/azure/resource_azure_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -18,9 +19,11 @@ import (
)

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 {
Expand All @@ -44,6 +47,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,
Expand Down Expand Up @@ -234,6 +247,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,
Expand Down Expand Up @@ -560,16 +574,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)
Expand All @@ -594,6 +608,37 @@ func resourceAzureInstanceDelete(d *schema.ResourceData, meta interface{}) error
}
}

log.Printf("[INFO] Waiting for lease on instance '%s''s disk blob to be lifted.", name)

// in order to avoid `terraform taint`-like problems 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
}

// unfortunately, there's no better way of doing this except
// polling continuously until it goes away:
for {
exists, err := blobClient.BlobExists(
storageContainterName,
fmt.Sprintf(osDiskBlobNameFormat, name),
)
if err != nil {
return err
}

if !exists {
break
}

time.Sleep(3 * time.Second)
}

return nil
}

Expand Down Expand Up @@ -674,6 +719,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)
Expand Down
12 changes: 6 additions & 6 deletions builtin/providers/azure/resource_azure_local_network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 9 additions & 0 deletions builtin/providers/azure/resource_azure_security_group_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions builtin/providers/azure/resource_azure_virtual_network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 172d19d

Please sign in to comment.