clean up azure temporary managed os disk

builder/azure/arm/builder_acc_test.go

@@ -20,12 +20,18 @@ package arm
 //   go test -v -timeout 90m -run TestBuilderAcc_.*
 
 import (
-	"testing"
-
+	"bytes"
+	"context"
+	"errors"
 	"fmt"
 	"os"
+	"strings"
+	"testing"
 
+	"github.com/Azure/go-autorest/autorest"
+	"github.com/Azure/go-autorest/autorest/azure/auth"
 	builderT "github.com/hashicorp/packer-plugin-sdk/acctest"
+	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
 )
 
 const DeviceLoginAcceptanceTest = "DEVICELOGIN_TEST"
@@ -96,10 +102,15 @@ func TestBuilderAcc_ManagedDisk_Linux_AzureCLI(t *testing.T) {
 		return
 	}
 
+	var b Builder
 	builderT.Test(t, builderT.TestCase{
-		PreCheck: func() { testAccPreCheck(t) },
-		Builder:  &Builder{},
+		PreCheck: func() { testAuthPreCheck(t) },
+		Builder:  &b,
 		Template: testBuilderAccManagedDiskLinuxAzureCLI,
+		Check: func([]packersdk.Artifact) error {
+			checkTemporaryGroupDeleted(t, &b)
+			return nil
+		},
 	})
 }
 
@@ -112,15 +123,100 @@ func TestBuilderAcc_Blob_Windows(t *testing.T) {
 }
 
 func TestBuilderAcc_Blob_Linux(t *testing.T) {
+	var b Builder
 	builderT.Test(t, builderT.TestCase{
-		PreCheck: func() { testAccPreCheck(t) },
-		Builder:  &Builder{},
+		PreCheck: func() { testAuthPreCheck(t) },
+		Builder:  &b,
 		Template: testBuilderAccBlobLinux,
+		Check: func([]packersdk.Artifact) error {
+			checkUnmanagedVHDDeleted(t, &b)
+			return nil
+		},
 	})
 }
 
 func testAccPreCheck(*testing.T) {}
 
+func testAuthPreCheck(t *testing.T) {
+	_, err := auth.NewAuthorizerFromEnvironment()
+	if err != nil {
+		t.Fatalf("failed to auth to azure: %s", err)
+	}
+}
+
+func checkTemporaryGroupDeleted(t *testing.T, b *Builder) {
+	ui := testUi()
+
+	spnCloud, spnKeyVault, err := b.getServicePrincipalTokens(ui.Say)
+	if err != nil {
+		t.Fatalf("failed getting azure tokens: %s", err)
+	}
+
+	ui.Message("Creating test Azure Resource Manager (ARM) client ...")
+	azureClient, err := NewAzureClient(
+		b.config.ClientConfig.SubscriptionID,
+		b.config.SharedGalleryDestination.SigDestinationSubscription,
+		b.config.ResourceGroupName,
+		b.config.StorageAccount,
+		b.config.ClientConfig.CloudEnvironment(),
+		b.config.SharedGalleryTimeout,
+		b.config.PollingDurationTimeout,
+		spnCloud,
+		spnKeyVault)
+
+	// Validate resource group has been deleted
+	_, err = azureClient.GroupsClient.Get(context.Background(), b.config.tmpResourceGroupName)
+	if err == nil || !resourceNotFound(err) {
+		t.Fatalf("failed validating resource group deletion: %s", err)
+	}
+}
+
+func checkUnmanagedVHDDeleted(t *testing.T, b *Builder) {
+	ui := testUi()
+
+	spnCloud, spnKeyVault, err := b.getServicePrincipalTokens(ui.Say)
+	if err != nil {
+		t.Fatalf("failed getting azure tokens: %s", err)
+	}
+
+	ui.Message("Creating test Azure Resource Manager (ARM) client ...")
+	azureClient, err := NewAzureClient(
+		b.config.ClientConfig.SubscriptionID,
+		b.config.SharedGalleryDestination.SigDestinationSubscription,
+		b.config.ResourceGroupName,
+		b.config.StorageAccount,
+		b.config.ClientConfig.CloudEnvironment(),
+		b.config.SharedGalleryTimeout,
+		b.config.PollingDurationTimeout,
+		spnCloud,
+		spnKeyVault)
+
+	blob := azureClient.BlobStorageClient.GetContainerReference("images").GetBlobReference(b.config.tmpOSDiskName)
+	_, err = blob.BreakLease(nil)
+	if err != nil && !strings.Contains(err.Error(), "BlobNotFound") {
+		t.Fatalf("failed validating deletion of unmanaged vhd: %s", err)
+	}
+
+	// Validate resource group has been deleted
+	_, err = azureClient.GroupsClient.Get(context.Background(), b.config.tmpResourceGroupName)
+	if err == nil || !resourceNotFound(err) {
+		t.Fatalf("failed validating resource group deletion: %s", err)
+	}
+}
+
+func resourceNotFound(err error) bool {
+	derr := autorest.DetailedError{}
+	return errors.As(err, &derr) && derr.StatusCode == 404
+}
+
+func testUi() *packersdk.BasicUi {
+	return &packersdk.BasicUi{
+		Reader:      new(bytes.Buffer),
+		Writer:      new(bytes.Buffer),
+		ErrorWriter: new(bytes.Buffer),
+	}
+}
+
 const testBuilderAccManagedDiskWindows = `
 {
 	"variables": {
@@ -155,6 +251,7 @@ const testBuilderAccManagedDiskWindows = `
 	}]
 }
 `
+
 const testBuilderAccManagedDiskWindowsBuildResourceGroup = `
 {
 	"variables": {
@@ -287,6 +384,7 @@ const testBuilderAccManagedDiskLinux = `
 	}]
 }
 `
+
 const testBuilderAccManagedDiskLinuxDeviceLogin = `
 {
 	"variables": {
@@ -365,7 +463,7 @@ const testBuilderAccBlobLinux = `
 	  "subscription_id": "{{user ` + "`subscription_id`" + `}}",
 
 	  "storage_account": "{{user ` + "`storage_account`" + `}}",
-	  "resource_group_name": "packer-acceptance-test",
+	  "resource_group_name": "ace-vault-image",
 	  "capture_container_name": "test",
 	  "capture_name_prefix": "testBuilderAccBlobLinux",
 
@@ -374,11 +472,12 @@ const testBuilderAccBlobLinux = `
 	  "image_offer": "UbuntuServer",
 	  "image_sku": "16.04-LTS",
 
-	  "location": "South Central US",
+	  "location": "Eastus2",
 	  "vm_size": "Standard_DS2_v2"
 	}]
 }
 `
+
 const testBuilderAccManagedDiskLinuxAzureCLI = `
 {
 	"builders": [{
@@ -388,7 +487,8 @@ const testBuilderAccManagedDiskLinuxAzureCLI = `
 
 	  "managed_image_resource_group_name": "packer-acceptance-test",
 	  "managed_image_name": "testBuilderAccManagedDiskLinuxAzureCLI-{{timestamp}}",
-
+	  "temp_resource_group_name": "packer-acceptance-test-managed-cli",
+
 	  "os_type": "Linux",
 	  "image_publisher": "Canonical",
 	  "image_offer": "UbuntuServer",

builder/azure/arm/step_deploy_template.go

@@ -15,16 +15,17 @@ import (
 )
 
 type StepDeployTemplate struct {
-	client     *AzureClient
-	deploy     func(ctx context.Context, resourceGroupName string, deploymentName string) error
-	delete     func(ctx context.Context, deploymentName, resourceGroupName string) error
-	disk       func(ctx context.Context, resourceGroupName string, computeName string) (string, string, error)
-	deleteDisk func(ctx context.Context, imageType string, imageName string, resourceGroupName string) error
-	say        func(message string)
-	error      func(e error)
-	config     *Config
-	factory    templateFactoryFunc
-	name       string
+	client           *AzureClient
+	deploy           func(ctx context.Context, resourceGroupName string, deploymentName string) error
+	delete           func(ctx context.Context, deploymentName, resourceGroupName string) error
+	disk             func(ctx context.Context, resourceGroupName string, computeName string) (string, string, error)
+	deleteDisk       func(ctx context.Context, imageType string, imageName string, resourceGroupName string) error
+	deleteDeployment func(ctx context.Context, state multistep.StateBag) error
+	say              func(message string)
+	error            func(e error)
+	config           *Config
+	factory          templateFactoryFunc
+	name             string
 }
 
 func NewStepDeployTemplate(client *AzureClient, ui packersdk.Ui, config *Config, deploymentName string, factory templateFactoryFunc) *StepDeployTemplate {
@@ -41,6 +42,7 @@ func NewStepDeployTemplate(client *AzureClient, ui packersdk.Ui, config *Config,
 	step.delete = step.deleteDeploymentResources
 	step.disk = step.getImageDetails
 	step.deleteDisk = step.deleteImage
+	step.deleteDeployment = step.deleteDeploymentObject
 	return step
 }
 
@@ -58,32 +60,45 @@ func (s *StepDeployTemplate) Run(ctx context.Context, state multistep.StateBag)
 
 func (s *StepDeployTemplate) Cleanup(state multistep.StateBag) {
 	defer func() {
-		err := s.deleteTemplate(context.Background(), state)
+		err := s.deleteDeployment(context.Background(), state)
 		if err != nil {
-			s.say(s.client.LastError.Error())
+			s.say(err.Error())
 		}
 	}()
 
-	// Only clean up if this is an existing resource group that has been verified to exist.
-	// ArmIsResourceGroupCreated is set in step_create_resource_group to true, when Packer has verified that the resource group exists.
-	// ArmIsExistingResourceGroup is set to true when build_resource_group is set in the Packer configuration.
-	existingResourceGroup := state.Get(constants.ArmIsExistingResourceGroup).(bool)
-	resourceGroupCreated := state.Get(constants.ArmIsResourceGroupCreated).(bool)
-	if !existingResourceGroup || !resourceGroupCreated {
-		return
-	}
-
 	ui := state.Get("ui").(packersdk.Ui)
-	ui.Say("\nThe resource group was not created by Packer, deleting individual resources ...")
+	ui.Say("\nDeleting individual resources ...")
 
 	deploymentName := s.name
 	resourceGroupName := state.Get(constants.ArmResourceGroupName).(string)
-	err := s.deleteDeploymentResources(context.TODO(), deploymentName, resourceGroupName)
+	// 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))
+	}
+	err = s.delete(context.TODO(), deploymentName, resourceGroupName)
 	if err != nil {
 		s.reportIfError(err, resourceGroupName)
 	}
 
-	NewStepDeleteAdditionalDisks(s.client, ui).Run(context.TODO(), state)
+	// The disk was not found on the VM, this is an error.
+	if imageType == "" && imageName == "" {
+		ui.Error(fmt.Sprintf("Failed to find temporary OS disk on VM.  Please delete manually.\n\n"+
+			"VM Name: %s\n"+
+			"Error: %s", computeName, err))
+		return
+	}
+
+	ui.Say(fmt.Sprintf(" Deleting -> %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 {
@@ -106,7 +121,7 @@ func (s *StepDeployTemplate) deployTemplate(ctx context.Context, resourceGroupNa
 	return err
 }
 
-func (s *StepDeployTemplate) deleteTemplate(ctx context.Context, state multistep.StateBag) error {
+func (s *StepDeployTemplate) deleteDeploymentObject(ctx context.Context, state multistep.StateBag) error {
 	deploymentName := s.name
 	resourceGroupName := state.Get(constants.ArmResourceGroupName).(string)
 	ui := state.Get("ui").(packersdk.Ui)