diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 3b01a5bfa..90bbcc413 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -222,7 +222,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), - NewStepDeleteAdditionalDisks(azureClient, ui), } } else if b.config.OSType == constants.Target_Windows { steps = []multistep.Step{ @@ -263,7 +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), - NewStepDeleteAdditionalDisks(azureClient, ui), ) } else { return nil, fmt.Errorf("Builder does not support the os_type '%s'", b.config.OSType) diff --git a/builder/azure/arm/builder_acc_test.go b/builder/azure/arm/builder_acc_test.go index 1e63f5ef9..1837bf5b0 100644 --- a/builder/azure/arm/builder_acc_test.go +++ b/builder/azure/arm/builder_acc_test.go @@ -46,6 +46,14 @@ func TestBuilderAcc_ManagedDisk_Windows_Build_Resource_Group(t *testing.T) { }) } +func TestBuilderAcc_ManagedDisk_Windows_Build_Resource_Group_Additional_Disk(t *testing.T) { + builderT.Test(t, builderT.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Builder: &Builder{}, + Template: testBuilderAccManagedDiskWindowsBuildResourceGroupAdditionalDisk, + }) +} + func TestBuilderAcc_ManagedDisk_Windows_DeviceLogin(t *testing.T) { if os.Getenv(DeviceLoginAcceptanceTest) == "" { t.Skip(fmt.Sprintf( @@ -169,6 +177,42 @@ const testBuilderAccManagedDiskWindowsBuildResourceGroup = ` } ` +const testBuilderAccManagedDiskWindowsBuildResourceGroupAdditionalDisk = ` +{ + "variables": { + "client_id": "{{env ` + "`ARM_CLIENT_ID`" + `}}", + "client_secret": "{{env ` + "`ARM_CLIENT_SECRET`" + `}}", + "subscription_id": "{{env ` + "`ARM_SUBSCRIPTION_ID`" + `}}" + }, + "builders": [{ + "type": "test", + + "client_id": "{{user ` + "`client_id`" + `}}", + "client_secret": "{{user ` + "`client_secret`" + `}}", + "subscription_id": "{{user ` + "`subscription_id`" + `}}", + + "build_resource_group_name" : "packer-acceptance-test", + "managed_image_resource_group_name": "packer-acceptance-test", + "managed_image_name": "testBuilderAccManagedDiskWindowsBuildResourceGroupAdditionDisk-{{timestamp}}", + + "os_type": "Windows", + "image_publisher": "MicrosoftWindowsServer", + "image_offer": "WindowsServer", + "image_sku": "2012-R2-Datacenter", + + "communicator": "winrm", + "winrm_use_ssl": "true", + "winrm_insecure": "true", + "winrm_timeout": "3m", + "winrm_username": "packer", + "async_resourcegroup_delete": "true", + + "vm_size": "Standard_DS2_v2", + "disk_additional_size": [10,15] + }] +} +` + const testBuilderAccManagedDiskWindowsDeviceLogin = ` { "variables": { diff --git a/builder/azure/arm/step_deploy_template.go b/builder/azure/arm/step_deploy_template.go index 003514e70..04a5295f4 100644 --- a/builder/azure/arm/step_deploy_template.go +++ b/builder/azure/arm/step_deploy_template.go @@ -6,8 +6,10 @@ import ( "fmt" "net/url" "strings" + "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" ) @@ -15,7 +17,7 @@ import ( type StepDeployTemplate struct { client *AzureClient deploy func(ctx context.Context, resourceGroupName string, deploymentName string) error - delete func(ctx context.Context, client *AzureClient, resourceType string, resourceName string, resourceGroupName 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) @@ -36,7 +38,7 @@ func NewStepDeployTemplate(client *AzureClient, ui packer.Ui, config *Config, de } step.deploy = step.deployTemplate - step.delete = deleteResource + step.delete = step.deleteDeploymentResources step.disk = step.getImageDetails step.deleteDisk = step.deleteImage return step @@ -62,8 +64,9 @@ func (s *StepDeployTemplate) Cleanup(state multistep.StateBag) { } }() - //Only clean up if this was an existing resource group and the resource group - //is marked as created + // 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 { @@ -75,74 +78,12 @@ func (s *StepDeployTemplate) Cleanup(state multistep.StateBag) { 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)) + err := s.deleteDeploymentResources(context.TODO(), deploymentName, resourceGroupName) + if err != nil { + s.reportIfError(err, resourceGroupName) } - 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)) - } - } + NewStepDeleteAdditionalDisks(s.client, ui).Run(context.TODO(), state) } func (s *StepDeployTemplate) deployTemplate(ctx context.Context, resourceGroupName string, deploymentName string) error { @@ -152,27 +93,31 @@ func (s *StepDeployTemplate) deployTemplate(ctx context.Context, resourceGroupNa } f, err := s.client.DeploymentsClient.CreateOrUpdate(ctx, resourceGroupName, deploymentName, *deployment) - if err == nil { - err = f.WaitForCompletionRef(ctx, s.client.DeploymentsClient.Client) - } if err != nil { s.say(s.client.LastError.Error()) + return err } + + err = f.WaitForCompletionRef(ctx, s.client.DeploymentsClient.Client) + if err == nil { + s.say(s.client.LastError.Error()) + } + return err } func (s *StepDeployTemplate) deleteTemplate(ctx context.Context, state multistep.StateBag) error { - var resourceGroupName = state.Get(constants.ArmResourceGroupName).(string) - var deploymentName = s.name + deploymentName := s.name + resourceGroupName := state.Get(constants.ArmResourceGroupName).(string) ui := state.Get("ui").(packer.Ui) 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 { + return err } - return err + return f.WaitForCompletionRef(ctx, s.client.DeploymentsClient.Client) } func (s *StepDeployTemplate) getImageDetails(ctx context.Context, resourceGroupName string, computeName string) (string, string, error) { @@ -186,11 +131,12 @@ func (s *StepDeployTemplate) getImageDetails(ctx context.Context, resourceGroupN 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 } + imageType = "Microsoft.Compute/disks" + imageName = *vm.StorageProfile.OsDisk.ManagedDisk.ID + return imageType, imageName, nil } @@ -267,3 +213,55 @@ func (s *StepDeployTemplate) deleteImage(ctx context.Context, imageType string, return blob.Delete(nil) } + +func (s *StepDeployTemplate) deleteDeploymentResources(ctx context.Context, deploymentName, resourceGroupName string) error { + var maxResources int32 = 50 + 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 *StepDeployTemplate) 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) + } +}