diff --git a/builder/azure/arm/builder_acc_test.go b/builder/azure/arm/builder_acc_test.go index ddd98af57b8..046dcb3565c 100644 --- a/builder/azure/arm/builder_acc_test.go +++ b/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,108 @@ 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) + + if err != nil { + t.Fatalf("failed to create azure client: %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 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) + } + + 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) + + if err != nil { + t.Fatalf("failed to create azure client: %s", err) + } + + // validate temporary os blob was deleted + 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 +259,7 @@ const testBuilderAccManagedDiskWindows = ` }] } ` + const testBuilderAccManagedDiskWindowsBuildResourceGroup = ` { "variables": { @@ -287,6 +392,7 @@ const testBuilderAccManagedDiskLinux = ` }] } ` + const testBuilderAccManagedDiskLinuxDeviceLogin = ` { "variables": { @@ -379,6 +485,7 @@ const testBuilderAccBlobLinux = ` }] } ` + const testBuilderAccManagedDiskLinuxAzureCLI = ` { "builders": [{ @@ -388,7 +495,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", diff --git a/builder/azure/arm/step_deploy_template.go b/builder/azure/arm/step_deploy_template.go index 0ac029f6002..0390ff98541 100644 --- a/builder/azure/arm/step_deploy_template.go +++ b/builder/azure/arm/step_deploy_template.go @@ -6,6 +6,7 @@ import ( "fmt" "net/url" "strings" + "sync" "time" "github.com/hashicorp/packer-plugin-sdk/multistep" @@ -15,16 +16,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 +43,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 +61,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 +122,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) @@ -222,6 +238,8 @@ func (s *StepDeployTemplate) deleteDeploymentResources(ctx context.Context, depl return err } + resources := map[string]string{} + for deploymentOperations.NotDone() { deploymentOperation := deploymentOperations.Value() // Sometimes an empty operation is added to the list by Azure @@ -233,30 +251,47 @@ func (s *StepDeployTemplate) deleteDeploymentResources(ctx context.Context, depl resourceName := *deploymentOperation.Properties.TargetResource.ResourceName resourceType := *deploymentOperation.Properties.TargetResource.ResourceType - s.say(fmt.Sprintf(" -> %s : '%s'", resourceType, resourceName)) - - err = 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 != nil { - s.reportIfError(err, resourceName) - } + s.say(fmt.Sprintf("Adding to deletion queue -> %s : '%s'", resourceType, resourceName)) + resources[resourceType] = resourceName if err = deploymentOperations.Next(); err != nil { return err } } + var wg sync.WaitGroup + wg.Add(len(resources)) + + for resourceType, resourceName := range resources { + go func(resourceType, resourceName string) { + defer wg.Done() + retryConfig := retry.Config{ + Tries: 10, + RetryDelay: (&retry.Backoff{InitialBackoff: 10 * time.Second, MaxBackoff: 600 * time.Second, Multiplier: 2}).Linear, + } + + err = retryConfig.Run(ctx, func(ctx context.Context) error { + s.say(fmt.Sprintf("Attempting deletion -> %s : '%s'", resourceType, resourceName)) + err := deleteResource(ctx, s.client, + resourceType, + resourceName, + resourceGroupName) + if err != nil { + s.say(fmt.Sprintf("Error deleting resource. Will retry.\n"+ + "Name: %s\n"+ + "Error: %s\n", resourceName, err.Error())) + } + return err + }) + if err != nil { + s.reportIfError(err, resourceName) + } + }(resourceType, resourceName) + } + + s.say("Waiting for deletion of all resources...") + wg.Wait() + return nil } diff --git a/builder/azure/arm/step_deploy_template_test.go b/builder/azure/arm/step_deploy_template_test.go index e24daf0d10e..a29ef2a0c09 100644 --- a/builder/azure/arm/step_deploy_template_test.go +++ b/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 TestStepDeployTemplateCleanupShouldDeleteManagedOSImageInTemporaryResourceGroup(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 + }, + } +}