clean up azure temporary managed os disk

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(" -> %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)

builder/azure/arm/step_deploy_template_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/packer-plugin-sdk/multistep"
+	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
 	"github.com/hashicorp/packer/builder/azure/common/constants"
 )
 
@@ -108,11 +109,97 @@ func TestStepDeployTemplateDeleteImageShouldFailWithInvalidImage(t *testing.T) {
 	}
 }
 
+func TestStepDeployTemplateCleanupShouldDeleteManagedOSImageInExistingResourceGroup(t *testing.T) {
+	var deleteDiskCounter = 0
+	var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter)
+
+	stateBag := createTestStateBagStepDeployTemplate()
+	stateBag.Put(constants.ArmIsManagedImage, true)
+	stateBag.Put(constants.ArmIsExistingResourceGroup, true)
+	stateBag.Put(constants.ArmIsResourceGroupCreated, true)
+	stateBag.Put("ui", packersdk.TestUi(t))
+
+	testSubject.Cleanup(stateBag)
+	if deleteDiskCounter != 1 {
+		t.Fatalf("Expected DeployTemplate Cleanup to invoke deleteDisk 1 time, but invoked %d times", deleteDiskCounter)
+	}
+}
+
+func TestStepDeployTemplateCleanupShouldNotDeleteManagedOSImageInTemporaryResourceGroup(t *testing.T) {
+	var deleteDiskCounter = 0
+	var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter)
+
+	stateBag := createTestStateBagStepDeployTemplate()
+	stateBag.Put(constants.ArmIsManagedImage, true)
+	stateBag.Put(constants.ArmIsExistingResourceGroup, false)
+	stateBag.Put(constants.ArmIsResourceGroupCreated, true)
+	stateBag.Put("ui", packersdk.TestUi(t))
+
+	testSubject.Cleanup(stateBag)
+	if deleteDiskCounter != 1 {
+		t.Fatalf("Expected DeployTemplate Cleanup to invoke deleteDisk 1 times, but invoked %d times", deleteDiskCounter)
+	}
+}
+
+func TestStepDeployTemplateCleanupShouldDeleteVHDOSImageInExistingResourceGroup(t *testing.T) {
+	var deleteDiskCounter = 0
+	var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter)
+
+	stateBag := createTestStateBagStepDeployTemplate()
+	stateBag.Put(constants.ArmIsManagedImage, false)
+	stateBag.Put(constants.ArmIsExistingResourceGroup, true)
+	stateBag.Put(constants.ArmIsResourceGroupCreated, true)
+	stateBag.Put("ui", packersdk.TestUi(t))
+
+	testSubject.Cleanup(stateBag)
+	if deleteDiskCounter != 1 {
+		t.Fatalf("Expected DeployTemplate Cleanup to invoke deleteDisk 1 time, but invoked %d times", deleteDiskCounter)
+	}
+}
+
+func TestStepDeployTemplateCleanupShouldVHDOSImageInTemporaryResourceGroup(t *testing.T) {
+	var deleteDiskCounter = 0
+	var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter)
+
+	stateBag := createTestStateBagStepDeployTemplate()
+	stateBag.Put(constants.ArmIsManagedImage, false)
+	stateBag.Put(constants.ArmIsExistingResourceGroup, false)
+	stateBag.Put(constants.ArmIsResourceGroupCreated, true)
+	stateBag.Put("ui", packersdk.TestUi(t))
+
+	testSubject.Cleanup(stateBag)
+	if deleteDiskCounter != 1 {
+		t.Fatalf("Expected DeployTemplate Cleanup to invoke deleteDisk 1 times, but invoked %d times", deleteDiskCounter)
+	}
+}
+
 func createTestStateBagStepDeployTemplate() multistep.StateBag {
 	stateBag := new(multistep.BasicStateBag)
 
 	stateBag.Put(constants.ArmDeploymentName, "Unit Test: DeploymentName")
 	stateBag.Put(constants.ArmResourceGroupName, "Unit Test: ResourceGroupName")
+	stateBag.Put(constants.ArmComputeName, "Unit Test: ComputeName")
 
 	return stateBag
 }
+
+func createTestStepDeployTemplateDeleteOSImage(deleteDiskCounter *int) *StepDeployTemplate {
+	return &StepDeployTemplate{
+		deploy: func(context.Context, string, string) error { return nil },
+		say:    func(message string) {},
+		error:  func(e error) {},
+		deleteDisk: func(ctx context.Context, imageType string, imageName string, resourceGroupName string) error {
+			*deleteDiskCounter++
+			return nil
+		},
+		disk: func(ctx context.Context, resourceGroupName, computeName string) (string, string, error) {
+			return "Microsoft.Compute/disks", "", nil
+		},
+		delete: func(ctx context.Context, deploymentName, resourceGroupName string) error {
+			return nil
+		},
+		deleteDeployment: func(ctx context.Context, state multistep.StateBag) error {
+			return nil
+		},
+	}
+}