Merge pull request #9559 from hashicorp/fix_9482

builder/azure-arm: Remove duplicate resource deletion steps

builder/azure/arm/builder.go

@@ -221,8 +221,6 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			NewStepSnapshotDataDisks(azureClient, ui, &b.config),
 			NewStepCaptureImage(azureClient, ui),
 			NewStepPublishToSharedImageGallery(azureClient, ui, &b.config),
-			NewStepDeleteResourceGroup(azureClient, ui),
-			NewStepDeleteOSDisk(azureClient, ui),
 			NewStepDeleteAdditionalDisks(azureClient, ui),
 		}
 	} else if b.config.OSType == constants.Target_Windows {
@@ -264,8 +262,6 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 			NewStepSnapshotDataDisks(azureClient, ui, &b.config),
 			NewStepCaptureImage(azureClient, ui),
 			NewStepPublishToSharedImageGallery(azureClient, ui, &b.config),
-			NewStepDeleteResourceGroup(azureClient, ui),
-			NewStepDeleteOSDisk(azureClient, ui),
 			NewStepDeleteAdditionalDisks(azureClient, ui),
 		)
 	} else {

builder/azure/arm/step_create_resource_group.go

@@ -111,27 +111,30 @@ func (s *StepCreateResourceGroup) Cleanup(state multistep.StateBag) {
 	if state.Get(constants.ArmIsExistingResourceGroup).(bool) {
 		ui.Say("\nThe resource group was not created by Packer, not deleting ...")
 		return
-	} else {
+	}
-		ui.Say("\nCleanup requested, deleting resource group ...")
 
-		var resourceGroupName = state.Get(constants.ArmResourceGroupName).(string)
+	ctx := context.TODO()
-		ctx := context.TODO()
+	resourceGroupName := state.Get(constants.ArmResourceGroupName).(string)
-		f, err := s.client.GroupsClient.Delete(ctx, resourceGroupName)
+	if exists, err := s.exists(ctx, resourceGroupName); !exists || err != nil {
-		if err == nil {
+		return
-			if state.Get(constants.ArmAsyncResourceGroupDelete).(bool) {
+	}
-				s.say(fmt.Sprintf("\n Not waiting for Resource Group delete as requested by user. Resource Group Name is %s", resourceGroupName))
+
-			} else {
+	ui.Say("\nCleanup requested, deleting resource group ...")
-				err = f.WaitForCompletionRef(ctx, s.client.GroupsClient.Client)
+	f, err := s.client.GroupsClient.Delete(ctx, resourceGroupName)
-			}
+	if err == nil {
-		}
+		if state.Get(constants.ArmAsyncResourceGroupDelete).(bool) {
-		if err != nil {
+			s.say(fmt.Sprintf("\n Not waiting for Resource Group delete as requested by user. Resource Group Name is %s", resourceGroupName))
-			ui.Error(fmt.Sprintf("Error deleting resource group.  Please delete it manually.\n\n"+
+		} else {
-				"Name: %s\n"+
+			err = f.WaitForCompletionRef(ctx, s.client.GroupsClient.Client)
-				"Error: %s", resourceGroupName, err))
-			return
-		}
-		if !state.Get(constants.ArmAsyncResourceGroupDelete).(bool) {
-			ui.Say("Resource group has been deleted.")
 		}
 	}
+	if err != nil {
+		ui.Error(fmt.Sprintf("Error deleting resource group.  Please delete it manually.\n\n"+
+			"Name: %s\n"+
+			"Error: %s", resourceGroupName, err))
+		return
+	}
+	if !state.Get(constants.ArmAsyncResourceGroupDelete).(bool) {
+		ui.Say("Resource group has been deleted.")
+	}
 }

builder/azure/arm/step_delete_additional_disks.go

@@ -35,8 +35,13 @@ func NewStepDeleteAdditionalDisks(client *AzureClient, ui packer.Ui) *StepDelete
 
 func (s *StepDeleteAdditionalDisk) deleteBlob(storageContainerName string, blobName string) error {
 	blob := s.client.BlobStorageClient.GetContainerReference(storageContainerName).GetBlobReference(blobName)
-	err := blob.Delete(nil)
+	_, err := blob.BreakLease(nil)
+	if err != nil && !strings.Contains(err.Error(), "LeaseNotPresentWithLeaseOperation") {
+		s.say(s.client.LastError.Error())
+		return err
+	}
 
+	err = blob.Delete(nil)
 	if err != nil {
 		s.say(s.client.LastError.Error())
 	}
@@ -80,25 +85,26 @@ func (s *StepDeleteAdditionalDisk) Run(ctx context.Context, state multistep.Stat
 				s.say("Failed to delete the managed Additional Disk!")
 				return processStepResult(err, s.error, state)
 			}
+			continue
+		}
+
+		u, err := url.Parse(additionaldisk)
+		if err != nil {
+			s.say("Failed to parse the Additional Disk's VHD URI!")
+			return processStepResult(err, s.error, state)
+		}
+
+		xs := strings.Split(u.Path, "/")
+		if len(xs) < 3 {
+			err = errors.New("Failed to parse Additional Disk's VHD URI!")
 		} else {
-			u, err := url.Parse(additionaldisk)
+			var storageAccountName = xs[1]
-			if err != nil {
+			var blobName = strings.Join(xs[2:], "/")
-				s.say("Failed to parse the Additional Disk's VHD URI!")
-				return processStepResult(err, s.error, state)
-			}
 
-			xs := strings.Split(u.Path, "/")
+			err = s.delete(storageAccountName, blobName)
-			if len(xs) < 3 {
+		}
-				err = errors.New("Failed to parse Additional Disk's VHD URI!")
+		if err != nil {
-			} else {
+			return processStepResult(err, s.error, state)
-				var storageAccountName = xs[1]
-				var blobName = strings.Join(xs[2:], "/")
-
-				err = s.delete(storageAccountName, blobName)
-			}
-			if err != nil {
-				return processStepResult(err, s.error, state)
-			}
 		}
 	}
 	return multistep.ActionContinue

builder/azure/arm/step_delete_os_disk.go

@@ -1,99 +0,0 @@
-package arm
-
-import (
-	"context"
-	"errors"
-	"fmt"
-	"net/url"
-	"strings"
-
-	"github.com/hashicorp/packer/builder/azure/common/constants"
-
-	"github.com/hashicorp/packer/helper/multistep"
-	"github.com/hashicorp/packer/packer"
-)
-
-type StepDeleteOSDisk struct {
-	client        *AzureClient
-	delete        func(string, string) error
-	deleteManaged func(context.Context, string, string) error
-	say           func(message string)
-	error         func(e error)
-}
-
-func NewStepDeleteOSDisk(client *AzureClient, ui packer.Ui) *StepDeleteOSDisk {
-	var step = &StepDeleteOSDisk{
-		client: client,
-		say:    func(message string) { ui.Say(message) },
-		error:  func(e error) { ui.Error(e.Error()) },
-	}
-
-	step.delete = step.deleteBlob
-	step.deleteManaged = step.deleteManagedDisk
-	return step
-}
-
-func (s *StepDeleteOSDisk) deleteBlob(storageContainerName string, blobName string) error {
-	blob := s.client.BlobStorageClient.GetContainerReference(storageContainerName).GetBlobReference(blobName)
-	err := blob.Delete(nil)
-
-	if err != nil {
-		s.say(s.client.LastError.Error())
-	}
-	return err
-}
-
-func (s *StepDeleteOSDisk) deleteManagedDisk(ctx context.Context, resourceGroupName string, imageName string) error {
-	xs := strings.Split(imageName, "/")
-	diskName := xs[len(xs)-1]
-	f, err := s.client.DisksClient.Delete(ctx, resourceGroupName, diskName)
-	if err == nil {
-		err = f.WaitForCompletionRef(ctx, s.client.DisksClient.Client)
-	}
-	return err
-}
-
-func (s *StepDeleteOSDisk) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
-	s.say("Deleting the temporary OS disk ...")
-
-	var osDisk = state.Get(constants.ArmOSDiskVhd).(string)
-	var isManagedDisk = state.Get(constants.ArmIsManagedImage).(bool)
-	var isExistingResourceGroup = state.Get(constants.ArmIsExistingResourceGroup).(bool)
-	var resourceGroupName = state.Get(constants.ArmResourceGroupName).(string)
-
-	if isManagedDisk && !isExistingResourceGroup {
-		s.say(fmt.Sprintf(" -> OS Disk : skipping, managed disk was used..."))
-		return multistep.ActionContinue
-	}
-
-	s.say(fmt.Sprintf(" -> OS Disk : '%s'", osDisk))
-
-	var err error
-	if isManagedDisk {
-		err = s.deleteManaged(ctx, resourceGroupName, osDisk)
-		if err != nil {
-			s.say("Failed to delete the managed OS Disk!")
-			return processStepResult(err, s.error, state)
-		}
-		return multistep.ActionContinue
-	}
-	u, err := url.Parse(osDisk)
-	if err != nil {
-		s.say("Failed to parse the OS Disk's VHD URI!")
-		return processStepResult(err, s.error, state)
-	}
-
-	xs := strings.Split(u.Path, "/")
-	if len(xs) < 3 {
-		err = errors.New("Failed to parse OS Disk's VHD URI!")
-	} else {
-		var storageAccountName = xs[1]
-		var blobName = strings.Join(xs[2:], "/")
-
-		err = s.delete(storageAccountName, blobName)
-	}
-	return processStepResult(err, s.error, state)
-}
-
-func (*StepDeleteOSDisk) Cleanup(multistep.StateBag) {
-}

builder/azure/arm/step_delete_os_disk_test.go

@@ -1,227 +0,0 @@
-package arm
-
-import (
-	"context"
-	"errors"
-	"fmt"
-	"testing"
-
-	"github.com/hashicorp/packer/builder/azure/common/constants"
-	"github.com/hashicorp/packer/helper/multistep"
-)
-
-func TestStepDeleteOSDiskShouldFailIfGetFails(t *testing.T) {
-	var testSubject = &StepDeleteOSDisk{
-		delete:        func(string, string) error { return fmt.Errorf("!! Unit Test FAIL !!") },
-		deleteManaged: func(context.Context, string, string) error { return nil },
-		say:           func(message string) {},
-		error:         func(e error) {},
-	}
-
-	stateBag := DeleteTestStateBagStepDeleteOSDisk("http://storage.blob.core.windows.net/images/pkrvm_os.vhd")
-
-	var result = testSubject.Run(context.Background(), stateBag)
-	if result != multistep.ActionHalt {
-		t.Fatalf("Expected the step to return 'ActionHalt', but got '%d'.", result)
-	}
-
-	if _, ok := stateBag.GetOk(constants.Error); ok == false {
-		t.Fatalf("Expected the step to set stateBag['%s'], but it was not.", constants.Error)
-	}
-}
-
-func TestStepDeleteOSDiskShouldPassIfGetPasses(t *testing.T) {
-	var testSubject = &StepDeleteOSDisk{
-		delete: func(string, string) error { return nil },
-		say:    func(message string) {},
-		error:  func(e error) {},
-	}
-
-	stateBag := DeleteTestStateBagStepDeleteOSDisk("http://storage.blob.core.windows.net/images/pkrvm_os.vhd")
-
-	var result = testSubject.Run(context.Background(), stateBag)
-	if result != multistep.ActionContinue {
-		t.Fatalf("Expected the step to return 'ActionContinue', but got '%d'.", result)
-	}
-
-	if _, ok := stateBag.GetOk(constants.Error); ok == true {
-		t.Fatalf("Expected the step to not set stateBag['%s'], but it was.", constants.Error)
-	}
-}
-
-func TestStepDeleteOSDiskShouldTakeStepArgumentsFromStateBag(t *testing.T) {
-	var actualStorageContainerName string
-	var actualBlobName string
-
-	var testSubject = &StepDeleteOSDisk{
-		delete: func(storageContainerName string, blobName string) error {
-			actualStorageContainerName = storageContainerName
-			actualBlobName = blobName
-			return nil
-		},
-		say:   func(message string) {},
-		error: func(e error) {},
-	}
-
-	stateBag := DeleteTestStateBagStepDeleteOSDisk("http://storage.blob.core.windows.net/images/pkrvm_os.vhd")
-	var result = testSubject.Run(context.Background(), stateBag)
-
-	if result != multistep.ActionContinue {
-		t.Fatalf("Expected the step to return 'ActionContinue', but got '%d'.", result)
-	}
-
-	if actualStorageContainerName != "images" {
-		t.Fatalf("Expected the storage container name to be 'images', but found '%s'.", actualStorageContainerName)
-	}
-
-	if actualBlobName != "pkrvm_os.vhd" {
-		t.Fatalf("Expected the blob name to be 'pkrvm_os.vhd', but found '%s'.", actualBlobName)
-	}
-}
-
-func TestStepDeleteOSDiskShouldHandleComplexStorageContainerNames(t *testing.T) {
-	var actualStorageContainerName string
-	var actualBlobName string
-
-	var testSubject = &StepDeleteOSDisk{
-		delete: func(storageContainerName string, blobName string) error {
-			actualStorageContainerName = storageContainerName
-			actualBlobName = blobName
-			return nil
-		},
-		say:   func(message string) {},
-		error: func(e error) {},
-	}
-
-	stateBag := DeleteTestStateBagStepDeleteOSDisk("http://storage.blob.core.windows.net/abc/def/pkrvm_os.vhd")
-	testSubject.Run(context.Background(), stateBag)
-
-	if actualStorageContainerName != "abc" {
-		t.Fatalf("Expected the storage container name to be 'abc/def', but found '%s'.", actualStorageContainerName)
-	}
-
-	if actualBlobName != "def/pkrvm_os.vhd" {
-		t.Fatalf("Expected the blob name to be 'pkrvm_os.vhd', but found '%s'.", actualBlobName)
-	}
-}
-
-func TestStepDeleteOSDiskShouldFailIfVHDNameCannotBeURLParsed(t *testing.T) {
-	var testSubject = &StepDeleteOSDisk{
-		delete:        func(string, string) error { return nil },
-		say:           func(message string) {},
-		error:         func(e error) {},
-		deleteManaged: func(context.Context, string, string) error { return nil },
-	}
-
-	// Invalid URL per https://golang.org/src/net/url/url_test.go
-	stateBag := DeleteTestStateBagStepDeleteOSDisk("http://[fe80::1%en0]/")
-
-	var result = testSubject.Run(context.Background(), stateBag)
-	if result != multistep.ActionHalt {
-		t.Fatalf("Expected the step to return 'ActionHalt', but got '%v'.", result)
-	}
-
-	if _, ok := stateBag.GetOk(constants.Error); ok == false {
-		t.Fatalf("Expected the step to not stateBag['%s'], but it was.", constants.Error)
-	}
-}
-func TestStepDeleteOSDiskShouldFailIfVHDNameIsTooShort(t *testing.T) {
-	var testSubject = &StepDeleteOSDisk{
-		delete:        func(string, string) error { return nil },
-		say:           func(message string) {},
-		error:         func(e error) {},
-		deleteManaged: func(context.Context, string, string) error { return nil },
-	}
-
-	stateBag := DeleteTestStateBagStepDeleteOSDisk("storage.blob.core.windows.net/abc")
-
-	var result = testSubject.Run(context.Background(), stateBag)
-	if result != multistep.ActionHalt {
-		t.Fatalf("Expected the step to return 'ActionHalt', but got '%d'.", result)
-	}
-
-	if _, ok := stateBag.GetOk(constants.Error); ok == false {
-		t.Fatalf("Expected the step to not stateBag['%s'], but it was.", constants.Error)
-	}
-}
-
-func TestStepDeleteOSDiskShouldPassIfManagedDiskInTempResourceGroup(t *testing.T) {
-	var testSubject = &StepDeleteOSDisk{
-		delete: func(string, string) error { return nil },
-		say:    func(message string) {},
-		error:  func(e error) {},
-	}
-
-	stateBag := new(multistep.BasicStateBag)
-	stateBag.Put(constants.ArmOSDiskVhd, "subscriptions/123-456-789/resourceGroups/existingresourcegroup/providers/Microsoft.Compute/disks/osdisk")
-	stateBag.Put(constants.ArmIsManagedImage, true)
-	stateBag.Put(constants.ArmIsExistingResourceGroup, false)
-	stateBag.Put(constants.ArmResourceGroupName, "testgroup")
-
-	var result = testSubject.Run(context.Background(), stateBag)
-	if result != multistep.ActionContinue {
-		t.Fatalf("Expected the step to return 'ActionContinue', but got '%d'.", result)
-	}
-
-	if _, ok := stateBag.GetOk(constants.Error); ok == true {
-		t.Fatalf("Expected the step to not set stateBag['%s'], but it was.", constants.Error)
-	}
-}
-
-func TestStepDeleteOSDiskShouldFailIfManagedDiskInExistingResourceGroupFailsToDelete(t *testing.T) {
-	var testSubject = &StepDeleteOSDisk{
-		delete:        func(string, string) error { return nil },
-		say:           func(message string) {},
-		error:         func(e error) {},
-		deleteManaged: func(context.Context, string, string) error { return errors.New("UNIT TEST FAIL!") },
-	}
-
-	stateBag := new(multistep.BasicStateBag)
-	stateBag.Put(constants.ArmOSDiskVhd, "subscriptions/123-456-789/resourceGroups/existingresourcegroup/providers/Microsoft.Compute/disks/osdisk")
-	stateBag.Put(constants.ArmIsManagedImage, true)
-	stateBag.Put(constants.ArmIsExistingResourceGroup, true)
-	stateBag.Put(constants.ArmResourceGroupName, "testgroup")
-
-	var result = testSubject.Run(context.Background(), stateBag)
-	if result != multistep.ActionHalt {
-		t.Fatalf("Expected the step to return 'ActionHalt', but got '%d'.", result)
-	}
-
-	if _, ok := stateBag.GetOk(constants.Error); ok == false {
-		t.Fatalf("Expected the step to not stateBag['%s'], but it was.", constants.Error)
-	}
-}
-
-func TestStepDeleteOSDiskShouldFailIfManagedDiskInExistingResourceGroupIsDeleted(t *testing.T) {
-	var testSubject = &StepDeleteOSDisk{
-		delete:        func(string, string) error { return nil },
-		say:           func(message string) {},
-		error:         func(e error) {},
-		deleteManaged: func(context.Context, string, string) error { return nil },
-	}
-
-	stateBag := new(multistep.BasicStateBag)
-	stateBag.Put(constants.ArmOSDiskVhd, "subscriptions/123-456-789/resourceGroups/existingresourcegroup/providers/Microsoft.Compute/disks/osdisk")
-	stateBag.Put(constants.ArmIsManagedImage, true)
-	stateBag.Put(constants.ArmIsExistingResourceGroup, true)
-	stateBag.Put(constants.ArmResourceGroupName, "testgroup")
-
-	var result = testSubject.Run(context.Background(), stateBag)
-	if result != multistep.ActionContinue {
-		t.Fatalf("Expected the step to return 'ActionContinue', but got '%d'.", result)
-	}
-
-	if _, ok := stateBag.GetOk(constants.Error); ok == true {
-		t.Fatalf("Expected the step to not set stateBag['%s'], but it was.", constants.Error)
-	}
-}
-
-func DeleteTestStateBagStepDeleteOSDisk(osDiskVhd string) multistep.StateBag {
-	stateBag := new(multistep.BasicStateBag)
-	stateBag.Put(constants.ArmOSDiskVhd, osDiskVhd)
-	stateBag.Put(constants.ArmIsManagedImage, false)
-	stateBag.Put(constants.ArmIsExistingResourceGroup, false)
-	stateBag.Put(constants.ArmResourceGroupName, "testgroup")
-
-	return stateBag
-}

builder/azure/arm/step_delete_resource_group.go

@@ -1,153 +0,0 @@
-package arm
-
-import (
-	"context"
-	"fmt"
-	"time"
-
-	"github.com/hashicorp/packer/builder/azure/common/constants"
-	"github.com/hashicorp/packer/common/retry"
-	"github.com/hashicorp/packer/helper/multistep"
-	"github.com/hashicorp/packer/packer"
-)
-
-const (
-	maxResourcesToDelete = 50
-)
-
-type StepDeleteResourceGroup struct {
-	client *AzureClient
-	delete func(ctx context.Context, state multistep.StateBag, resourceGroupName string) error
-	say    func(message string)
-	error  func(e error)
-}
-
-func NewStepDeleteResourceGroup(client *AzureClient, ui packer.Ui) *StepDeleteResourceGroup {
-	var step = &StepDeleteResourceGroup{
-		client: client,
-		say:    func(message string) { ui.Say(message) },
-		error:  func(e error) { ui.Error(e.Error()) },
-	}
-
-	step.delete = step.deleteResourceGroup
-	return step
-}
-
-func (s *StepDeleteResourceGroup) deleteResourceGroup(ctx context.Context, state multistep.StateBag, resourceGroupName string) error {
-	var err error
-	if state.Get(constants.ArmIsExistingResourceGroup).(bool) {
-		s.say("\nThe resource group was not created by Packer, only deleting individual resources ...")
-		var deploymentName = state.Get(constants.ArmDeploymentName).(string)
-		err = s.deleteDeploymentResources(ctx, deploymentName, resourceGroupName)
-		if err != nil {
-			return err
-		}
-
-		if keyVaultDeploymentName, ok := state.GetOk(constants.ArmKeyVaultDeploymentName); ok {
-			// Only delete if custom keyvault was not provided.
-			if exists := state.Get(constants.ArmIsExistingKeyVault).(bool); !exists {
-				s.say("\n Deleting the keyvault deployment because it was created by Packer...")
-				err = s.deleteDeploymentResources(ctx, keyVaultDeploymentName.(string), resourceGroupName)
-				if err != nil {
-					return err
-				}
-			}
-		}
-
-		return nil
-	} else {
-		s.say("\nThe resource group was created by Packer, deleting ...")
-		f, err := s.client.GroupsClient.Delete(ctx, resourceGroupName)
-		if err == nil {
-			if state.Get(constants.ArmAsyncResourceGroupDelete).(bool) {
-				// No need to wait for the completion for delete if request is Accepted
-				s.say(fmt.Sprintf("\nResource Group is being deleted, not waiting for deletion due to config. Resource Group Name '%s'", resourceGroupName))
-			} else {
-				f.WaitForCompletionRef(ctx, s.client.GroupsClient.Client)
-			}
-
-		}
-
-		if err != nil {
-			s.say(s.client.LastError.Error())
-		}
-		return err
-	}
-}
-
-func (s *StepDeleteResourceGroup) deleteDeploymentResources(ctx context.Context, deploymentName, resourceGroupName string) error {
-	maxResources := int32(maxResourcesToDelete)
-
-	deploymentOperations, err := s.client.DeploymentOperationsClient.ListComplete(ctx, resourceGroupName, deploymentName, &maxResources)
-	if err != nil {
-		s.reportIfError(err, resourceGroupName)
-		return err
-	}
-
-	for deploymentOperations.NotDone() {
-		deploymentOperation := deploymentOperations.Value()
-		// Sometimes an empty operation is added to the list by Azure
-		if deploymentOperation.Properties.TargetResource == nil {
-			deploymentOperations.Next()
-			continue
-		}
-
-		resourceName := *deploymentOperation.Properties.TargetResource.ResourceName
-		resourceType := *deploymentOperation.Properties.TargetResource.ResourceType
-
-		s.say(fmt.Sprintf(" -> %s : '%s'",
-			resourceType,
-			resourceName))
-
-		retry.Config{
-			Tries:      10,
-			RetryDelay: (&retry.Backoff{InitialBackoff: 10 * time.Second, MaxBackoff: 600 * time.Second, Multiplier: 2}).Linear,
-		}.Run(ctx, func(ctx context.Context) error {
-			err := deleteResource(ctx, s.client,
-				resourceType,
-				resourceName,
-				resourceGroupName)
-			if err != nil {
-				s.reportIfError(err, resourceName)
-			}
-			return nil
-		})
-
-		if err = deploymentOperations.Next(); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
-func (s *StepDeleteResourceGroup) reportIfError(err error, resourceName string) {
-	if err != nil {
-		s.say(fmt.Sprintf("Error deleting resource. Please delete manually.\n\n"+
-			"Name: %s\n"+
-			"Error: %s", resourceName, err.Error()))
-		s.error(err)
-	}
-}
-
-func (s *StepDeleteResourceGroup) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
-	s.say("Deleting resource group ...")
-
-	var resourceGroupName = state.Get(constants.ArmResourceGroupName).(string)
-	s.say(fmt.Sprintf(" -> ResourceGroupName : '%s'", resourceGroupName))
-
-	err := s.delete(ctx, state, resourceGroupName)
-	if err != nil {
-		state.Put(constants.Error, err)
-		s.error(err)
-
-		return multistep.ActionHalt
-	}
-
-	state.Put(constants.ArmIsResourceGroupCreated, false)
-
-	return multistep.ActionContinue
-}
-
-func (*StepDeleteResourceGroup) Cleanup(multistep.StateBag) {
-}

builder/azure/arm/step_delete_resource_group_test.go

@@ -1,78 +0,0 @@
-package arm
-
-import (
-	"context"
-	"fmt"
-	"testing"
-
-	"github.com/hashicorp/packer/builder/azure/common/constants"
-	"github.com/hashicorp/packer/helper/multistep"
-)
-
-func TestStepDeleteResourceGroupShouldFailIfDeleteFails(t *testing.T) {
-	var testSubject = &StepDeleteResourceGroup{
-		delete: func(context.Context, multistep.StateBag, string) error { return fmt.Errorf("!! Unit Test FAIL !!") },
-		say:    func(message string) {},
-		error:  func(e error) {},
-	}
-
-	stateBag := DeleteTestStateBagStepDeleteResourceGroup()
-
-	var result = testSubject.Run(context.Background(), stateBag)
-	if result != multistep.ActionHalt {
-		t.Fatalf("Expected the step to return 'ActionHalt', but got '%d'.", result)
-	}
-
-	if _, ok := stateBag.GetOk(constants.Error); ok == false {
-		t.Fatalf("Expected the step to set stateBag['%s'], but it was not.", constants.Error)
-	}
-}
-
-func TestStepDeleteResourceGroupShouldPassIfDeletePasses(t *testing.T) {
-	var testSubject = &StepDeleteResourceGroup{
-		delete: func(context.Context, multistep.StateBag, string) error { return nil },
-		say:    func(message string) {},
-		error:  func(e error) {},
-	}
-
-	stateBag := DeleteTestStateBagStepDeleteResourceGroup()
-
-	var result = testSubject.Run(context.Background(), stateBag)
-	if result != multistep.ActionContinue {
-		t.Fatalf("Expected the step to return 'ActionContinue', but got '%d'.", result)
-	}
-
-	if _, ok := stateBag.GetOk(constants.Error); ok == true {
-		t.Fatalf("Expected the step to not set stateBag['%s'], but it was.", constants.Error)
-	}
-}
-
-func TestStepDeleteResourceGroupShouldDeleteStateBagArmResourceGroupCreated(t *testing.T) {
-	var testSubject = &StepDeleteResourceGroup{
-		delete: func(context.Context, multistep.StateBag, string) error {
-			return nil
-		},
-		say:   func(message string) {},
-		error: func(e error) {},
-	}
-
-	stateBag := DeleteTestStateBagStepDeleteResourceGroup()
-	testSubject.Run(context.Background(), stateBag)
-
-	value, ok := stateBag.GetOk(constants.ArmIsResourceGroupCreated)
-	if !ok {
-		t.Fatal("Expected the resource bag value arm.IsResourceGroupCreated to exist")
-	}
-
-	if value.(bool) {
-		t.Fatalf("Expected arm.IsResourceGroupCreated to be false, but got %q", value)
-	}
-}
-
-func DeleteTestStateBagStepDeleteResourceGroup() multistep.StateBag {
-	stateBag := new(multistep.BasicStateBag)
-	stateBag.Put(constants.ArmResourceGroupName, "Unit Test: ResourceGroupName")
-	stateBag.Put(constants.ArmIsResourceGroupCreated, "Unit Test: IsResourceGroupCreated")
-
-	return stateBag
-}

builder/azure/arm/step_deploy_template.go

@@ -42,6 +42,109 @@ func NewStepDeployTemplate(client *AzureClient, ui packer.Ui, config *Config, de
 	return step
 }
 
+func (s *StepDeployTemplate) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
+	s.say("Deploying deployment template ...")
+
+	var resourceGroupName = state.Get(constants.ArmResourceGroupName).(string)
+	s.say(fmt.Sprintf(" -> ResourceGroupName : '%s'", resourceGroupName))
+	s.say(fmt.Sprintf(" -> DeploymentName    : '%s'", s.name))
+
+	return processStepResult(
+		s.deploy(ctx, resourceGroupName, s.name),
+		s.error, state)
+}
+
+func (s *StepDeployTemplate) Cleanup(state multistep.StateBag) {
+	defer func() {
+		err := s.deleteTemplate(context.Background(), state)
+		if err != nil {
+			s.say(s.client.LastError.Error())
+		}
+	}()
+
+	//Only clean up if this was an existing resource group and the resource group
+	//is marked as created
+	existingResourceGroup := state.Get(constants.ArmIsExistingResourceGroup).(bool)
+	resourceGroupCreated := state.Get(constants.ArmIsResourceGroupCreated).(bool)
+	if !existingResourceGroup || !resourceGroupCreated {
+		return
+	}
+
+	ui := state.Get("ui").(packer.Ui)
+	ui.Say("\nThe resource group was not created by Packer, deleting individual resources ...")
+
+	deploymentName := s.name
+	resourceGroupName := state.Get(constants.ArmResourceGroupName).(string)
+
+	// Get image disk details before deleting the image; otherwise we won't be able to
+	// delete the disk as the image request will return a 404
+	computeName := state.Get(constants.ArmComputeName).(string)
+	imageType, imageName, err := s.disk(context.TODO(), resourceGroupName, computeName)
+
+	if err != nil && !strings.Contains(err.Error(), "ResourceNotFound") {
+		ui.Error(fmt.Sprintf("Could not retrieve OS Image details: %s", err))
+	}
+
+	ui.Say(" -> Deployment Resources within: " + deploymentName)
+	if deploymentName != "" {
+		maxResources := int32(50)
+		deploymentOperations, err := s.client.DeploymentOperationsClient.ListComplete(context.TODO(), resourceGroupName, deploymentName, &maxResources)
+		if err != nil {
+			ui.Error(fmt.Sprintf("Error deleting resources.  Please delete them manually.\n\n"+
+				"Name: %s\n"+
+				"Error: %s", resourceGroupName, err))
+		}
+
+		for deploymentOperations.NotDone() {
+			deploymentOperation := deploymentOperations.Value()
+			// Sometimes an empty operation is added to the list by Azure
+			if deploymentOperation.Properties.TargetResource == nil {
+				if err := deploymentOperations.Next(); err != nil {
+					ui.Error(fmt.Sprintf("Error moving to to next deployment operation ...\n\n"+
+						"Name: %s\n"+
+						"Error: %s", resourceGroupName, err))
+					break
+				}
+				continue
+			}
+
+			ui.Say(fmt.Sprintf(" -> %s : '%s'",
+				*deploymentOperation.Properties.TargetResource.ResourceType,
+				*deploymentOperation.Properties.TargetResource.ResourceName))
+
+			err = s.delete(context.TODO(), s.client,
+				*deploymentOperation.Properties.TargetResource.ResourceType,
+				*deploymentOperation.Properties.TargetResource.ResourceName,
+				resourceGroupName)
+			if err != nil {
+				ui.Error(fmt.Sprintf("Error deleting resource.  Please delete manually.\n\n"+
+					"Name: %s\n"+
+					"Error: %s", *deploymentOperation.Properties.TargetResource.ResourceName, err))
+			}
+
+			if err = deploymentOperations.Next(); err != nil {
+				ui.Error(fmt.Sprintf("Error deleting resources.  Please delete them manually.\n\n"+
+					"Name: %s\n"+
+					"Error: %s", resourceGroupName, err))
+				break
+			}
+		}
+
+		// The disk is not defined as an operation in the template so it has to be deleted separately
+		if imageType == "" && imageName == "" {
+			return
+		}
+
+		ui.Say(fmt.Sprintf(" -> %s : '%s'", imageType, imageName))
+		err = s.deleteDisk(context.TODO(), imageType, imageName, resourceGroupName)
+		if err != nil {
+			ui.Error(fmt.Sprintf("Error deleting resource.  Please delete manually.\n\n"+
+				"Name: %s\n"+
+				"Error: %s", imageName, err))
+		}
+	}
+}
+
 func (s *StepDeployTemplate) deployTemplate(ctx context.Context, resourceGroupName string, deploymentName string) error {
 	deployment, err := s.factory(s.config)
 	if err != nil {
@@ -62,48 +165,32 @@ func (s *StepDeployTemplate) deleteTemplate(ctx context.Context, state multistep
 	var resourceGroupName = state.Get(constants.ArmResourceGroupName).(string)
 	var deploymentName = s.name
 	ui := state.Get("ui").(packer.Ui)
-	ui.Say(fmt.Sprintf("Removing the created Deployment object: '%s'", deploymentName))
 
+	ui.Say(fmt.Sprintf("Removing the created Deployment object: '%s'", deploymentName))
 	f, err := s.client.DeploymentsClient.Delete(ctx, resourceGroupName, deploymentName)
 	if err == nil {
 		err = f.WaitForCompletionRef(ctx, s.client.DeploymentsClient.Client)
 	}
-	if err != nil {
-		s.say(s.client.LastError.Error())
-	}
 
 	return err
 }
 
-func (s *StepDeployTemplate) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction {
-	s.say("Deploying deployment template ...")
-
-	var resourceGroupName = state.Get(constants.ArmResourceGroupName).(string)
-
-	s.say(fmt.Sprintf(" -> ResourceGroupName : '%s'", resourceGroupName))
-	s.say(fmt.Sprintf(" -> DeploymentName    : '%s'", s.name))
-
-	return processStepResult(
-		s.deploy(ctx, resourceGroupName, s.name),
-		s.error, state)
-}
-
 func (s *StepDeployTemplate) getImageDetails(ctx context.Context, resourceGroupName string, computeName string) (string, string, error) {
 	//We can't depend on constants.ArmOSDiskVhd being set
-	var imageName string
+	var imageName, imageType string
-	var imageType string
 	vm, err := s.client.VirtualMachinesClient.Get(ctx, resourceGroupName, computeName, "")
 	if err != nil {
 		return imageName, imageType, err
-	} else {
-		if vm.StorageProfile.OsDisk.Vhd != nil {
-			imageType = "image"
-			imageName = *vm.StorageProfile.OsDisk.Vhd.URI
-		} else {
-			imageType = "Microsoft.Compute/disks"
-			imageName = *vm.StorageProfile.OsDisk.ManagedDisk.ID
-		}
 	}
+
+	if vm.StorageProfile.OsDisk.Vhd != nil {
+		imageType = "image"
+		imageName = *vm.StorageProfile.OsDisk.Vhd.URI
+	} else {
+		imageType = "Microsoft.Compute/disks"
+		imageName = *vm.StorageProfile.OsDisk.ManagedDisk.ID
+	}
+
 	return imageType, imageName, nil
 }
 
@@ -158,6 +245,7 @@ func (s *StepDeployTemplate) deleteImage(ctx context.Context, imageType string,
 		}
 		return err
 	}
+
 	// VHD image
 	u, err := url.Parse(imageName)
 	if err != nil {
@@ -171,75 +259,11 @@ func (s *StepDeployTemplate) deleteImage(ctx context.Context, imageType string,
 	var blobName = strings.Join(xs[2:], "/")
 
 	blob := s.client.BlobStorageClient.GetContainerReference(storageAccountName).GetBlobReference(blobName)
-	err = blob.Delete(nil)
+	_, err = blob.BreakLease(nil)
-	return err
+	if err != nil && !strings.Contains(err.Error(), "LeaseNotPresentWithLeaseOperation") {
-}
+		s.say(s.client.LastError.Error())
-
+		return err
-func (s *StepDeployTemplate) Cleanup(state multistep.StateBag) {
+	}
-	defer s.deleteTemplate(context.Background(), state)
+
-
+	return blob.Delete(nil)
-	//Only clean up if this was an existing resource group and the resource group
-	//is marked as created
-	var existingResourceGroup = state.Get(constants.ArmIsExistingResourceGroup).(bool)
-	var resourceGroupCreated = state.Get(constants.ArmIsResourceGroupCreated).(bool)
-	if !existingResourceGroup || !resourceGroupCreated {
-		return
-	}
-	ui := state.Get("ui").(packer.Ui)
-	ui.Say("\nThe resource group was not created by Packer, deleting individual resources ...")
-
-	var resourceGroupName = state.Get(constants.ArmResourceGroupName).(string)
-	var computeName = state.Get(constants.ArmComputeName).(string)
-	var deploymentName = s.name
-	imageType, imageName, err := s.disk(context.TODO(), resourceGroupName, computeName)
-	if err != nil {
-		ui.Error("Could not retrieve OS Image details")
-	}
-
-	ui.Say(" -> Deployment Resources within: " + deploymentName)
-	if deploymentName != "" {
-		maxResources := int32(50)
-		deploymentOperations, err := s.client.DeploymentOperationsClient.ListComplete(context.TODO(), resourceGroupName, deploymentName, &maxResources)
-		if err != nil {
-			ui.Error(fmt.Sprintf("Error deleting resources.  Please delete them manually.\n\n"+
-				"Name: %s\n"+
-				"Error: %s", resourceGroupName, err))
-		}
-		for deploymentOperations.NotDone() {
-			deploymentOperation := deploymentOperations.Value()
-			// Sometimes an empty operation is added to the list by Azure
-			if deploymentOperation.Properties.TargetResource == nil {
-				deploymentOperations.Next()
-				continue
-			}
-			ui.Say(fmt.Sprintf(" -> %s : '%s'",
-				*deploymentOperation.Properties.TargetResource.ResourceType,
-				*deploymentOperation.Properties.TargetResource.ResourceName))
-			err = s.delete(context.TODO(), s.client,
-				*deploymentOperation.Properties.TargetResource.ResourceType,
-				*deploymentOperation.Properties.TargetResource.ResourceName,
-				resourceGroupName)
-			if err != nil {
-				ui.Error(fmt.Sprintf("Error deleting resource.  Please delete manually.\n\n"+
-					"Name: %s\n"+
-					"Error: %s", *deploymentOperation.Properties.TargetResource.ResourceName, err))
-			}
-			if err = deploymentOperations.Next(); err != nil {
-				ui.Error(fmt.Sprintf("Error deleting resources.  Please delete them manually.\n\n"+
-					"Name: %s\n"+
-					"Error: %s", resourceGroupName, err))
-				break
-			}
-		}
-
-		// The disk is not defined as an operation in the template so has to be
-		// deleted separately
-		ui.Say(fmt.Sprintf(" -> %s : '%s'", imageType, imageName))
-		err = s.deleteDisk(context.TODO(), imageType, imageName, resourceGroupName)
-		if err != nil {
-			ui.Error(fmt.Sprintf("Error deleting resource.  Please delete manually.\n\n"+
-				"Name: %s\n"+
-				"Error: %s", imageName, err))
-		}
-	}
 }