Skip to content

Commit

Permalink
Disable vhd delete when using managed disks
Browse files Browse the repository at this point in the history
Disabled logic for deleting vhd and storageaccount create/delete
when using managed disks. Prior, vhd delete logic was executed
for managed disks vm which led to a nil pointer exception. This
would disrupt the deletion process for all resources and lead to
remnant resources in Azure. Now, storage accounts are not created
for managed disks, and all resources are properly deleted whether
managed disks were used or not.
  • Loading branch information
rmweir authored and Craig Jellick committed Aug 24, 2019
1 parent e7645d0 commit 875f6c3
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 12 deletions.
7 changes: 5 additions & 2 deletions drivers/azure/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,8 +419,11 @@ func (d *Driver) Create() error {
d.ctx.PublicIPAddressID, d.ctx.SubnetID, d.ctx.NetworkSecurityGroupID, d.PrivateIPAddr); err != nil {
return err
}
if err := c.CreateStorageAccount(d.ctx, d.ResourceGroup, d.Location, storage.SkuName(d.StorageType)); err != nil {
return err
if !d.ManagedDisks {
// storage account is only necessary when using unmanaged disks
if err := c.CreateStorageAccount(d.ctx, d.ResourceGroup, d.Location, storage.SkuName(d.StorageType)); err != nil {
return err
}
}
if err := d.generateSSHKey(d.ctx); err != nil {
return err
Expand Down
29 changes: 19 additions & 10 deletions drivers/azure/azureutil/azureutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,9 +429,16 @@ func (a AzureClient) DeleteVirtualMachineIfExists(resourceGroup, name string) er
}

// Remove disk
if vmRef.VirtualMachineProperties != nil {
vhdURL := to.String(vmRef.VirtualMachineProperties.StorageProfile.OsDisk.Vhd.URI)
return a.removeOSDiskBlob(resourceGroup, name, vhdURL)
if vmProperties := vmRef.VirtualMachineProperties; vmProperties != nil {
// TODO: remove unattached managed disk, requires azure sdk upgrade
if managedDisk := vmProperties.StorageProfile.OsDisk.ManagedDisk; managedDisk != nil {
diskName := fmt.Sprintf("%s-os-disk", name)
log.Infof("Disk [%s] in resource group [%s] must be removed manually.", diskName, resourceGroup)
}
// if vhd is not nil then disk is unmanaged and disk blob should be removed
if vhd := vmProperties.StorageProfile.OsDisk.Vhd; vhd != nil {
return a.removeOSDiskBlob(resourceGroup, name, to.String(vhd.URI))
}
}
return nil
}
Expand Down Expand Up @@ -512,11 +519,7 @@ func (a AzureClient) CreateVirtualMachine(resourceGroup, name, location, size, a
return err
}

var (
osDiskBlobURL = osDiskStorageBlobURL(storageAccount, name)
sshKeyPath = fmt.Sprintf("/home/%s/.ssh/authorized_keys", username)
)
log.Debugf("OS disk blob will be placed at: %s", osDiskBlobURL)
sshKeyPath := fmt.Sprintf("/home/%s/.ssh/authorized_keys", username)
log.Debugf("SSH key will be placed at: %s", sshKeyPath)

var osProfile = &compute.OSProfile{
Expand Down Expand Up @@ -564,7 +567,7 @@ func (a AzureClient) CreateVirtualMachine(resourceGroup, name, location, size, a
Sku: to.StringPtr(img.sku),
Version: to.StringPtr(img.version),
},
OsDisk: getOSDisk(name, osDiskBlobURL, isManaged, storageType, diskSize),
OsDisk: getOSDisk(name, storageAccount, isManaged, storageType, diskSize),
},
},
}, nil)
Expand All @@ -573,7 +576,7 @@ func (a AzureClient) CreateVirtualMachine(resourceGroup, name, location, size, a

// GetOSDisk creates and returns pointer to a disk that is configured for either managed or unmanaged disks depending
// on setting.
func getOSDisk(name string, osDiskBlobURL string, isManaged bool, storageType string, diskSize int32) *compute.OSDisk {
func getOSDisk(name string, account *storage.AccountProperties, isManaged bool, storageType string, diskSize int32) *compute.OSDisk {
var osdisk *compute.OSDisk
if isManaged {
osdisk = &compute.OSDisk{
Expand All @@ -586,6 +589,8 @@ func getOSDisk(name string, osDiskBlobURL string, isManaged bool, storageType st
DiskSizeGB: to.Int32Ptr(diskSize),
}
} else {
osDiskBlobURL := osDiskStorageBlobURL(account, name)
log.Debugf("OS disk blob will be placed at: %s", osDiskBlobURL)
osdisk = &compute.OSDisk{
Name: to.StringPtr(fmt.Sprintf(fmtOSDiskResourceName, name)),
Caching: compute.ReadWrite,
Expand Down Expand Up @@ -840,6 +845,10 @@ func checkResourceExistsFromError(err error) (bool, error) {
// osDiskStorageBlobURL gives the full url of the VHD blob where the OS disk for
// the given VM should be stored.
func osDiskStorageBlobURL(account *storage.AccountProperties, vmName string) string {
if account == nil {
return ""
}

containerURL := osDiskStorageContainerURL(account, vmName) // has trailing slash
blobName := fmt.Sprintf(fmtOSDiskBlobName, vmName)
return containerURL + blobName
Expand Down

0 comments on commit 875f6c3

Please sign in to comment.