diff --git a/builder/azure/arm/azure_client.go b/builder/azure/arm/azure_client.go index ce36643a..3b453166 100644 --- a/builder/azure/arm/azure_client.go +++ b/builder/azure/arm/azure_client.go @@ -77,7 +77,7 @@ func errorCapture(client *AzureClient) client.ResponseMiddleware { } // Returns an Azure Client used for the Azure Resource Manager -func NewAzureClient(ctx context.Context, isVHDBuild bool, cloud *environments.Environment, sharedGalleryTimeout time.Duration, pollingDuration time.Duration, authOptions commonclient.AzureAuthOptions) (*AzureClient, error) { +func NewAzureClient(ctx context.Context, storageAccountName string, cloud *environments.Environment, sharedGalleryTimeout time.Duration, pollingDuration time.Duration, authOptions commonclient.AzureAuthOptions) (*AzureClient, error) { var azureClient = &AzureClient{} @@ -230,13 +230,12 @@ func NewAzureClient(ctx context.Context, isVHDBuild bool, cloud *environments.En azureClient.GalleryImagesClient = *galleryImagesClient // We only need the Blob Client to delete the OS VHD during VHD builds - if isVHDBuild { + if storageAccountName != "" { storageAccountAuthorizer, err := commonclient.BuildStorageAuthorizer(ctx, authOptions, *cloud) if err != nil { return nil, err } - - blobClient, err := giovanniBlobStorageSDK.NewWithBaseUri(cloud.Storage.Name()) + blobClient, err := giovanniBlobStorageSDK.NewWithBaseUri(fmt.Sprintf("https://%s.blob.core.windows.net", storageAccountName)) if err != nil { return nil, err } diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index c95de4b0..609a3fb7 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -100,11 +100,9 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) } ui.Message("Creating Azure Resource Manager (ARM) client ...") - // For VHD Builds we need to enable the Blob Azure Storage client - configureBlobClient := (b.config.ResourceGroupName != "" || b.config.StorageAccount != "") azureClient, err := NewAzureClient( ctx, - configureBlobClient, + b.config.StorageAccount, b.config.ClientConfig.CloudEnvironment(), b.config.SharedGalleryTimeout, b.config.PollingDurationTimeout, @@ -474,8 +472,6 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook) if b.config.isPublishToSIG() { return b.sharedImageArtifact(stateData) } - ui.Say(b.config.storageAccountBlobEndpoint) - ui.Say(fmt.Sprintf("%d", len(b.config.AdditionalDiskSize))) return NewArtifact( b.stateBag.Get(constants.ArmBuildVMInternalId).(string), b.config.CaptureNamePrefix, diff --git a/builder/azure/arm/step_deploy_template.go b/builder/azure/arm/step_deploy_template.go index 762b525d..7492adb2 100644 --- a/builder/azure/arm/step_deploy_template.go +++ b/builder/azure/arm/step_deploy_template.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "log" "net/url" "strings" "sync" @@ -30,21 +31,28 @@ type DeploymentTemplateType int const ( VirtualMachineTemplate DeploymentTemplateType = iota KeyVaultTemplate + VMResourceType = "Microsoft.Compute/virtualMachines" + NetworkInterfaceResourceType = "Microsoft.Network/networkInterfaces" + KeyVaultResourceType = "Microsoft.KeyVault/vaults" ) type StepDeployTemplate struct { - client *AzureClient - deploy func(ctx context.Context, subscriptionId string, resourceGroupName string, deploymentName string) error - delete func(ctx context.Context, subscriptionId, deploymentName, resourceGroupName string) error - disk func(ctx context.Context, subscriptionId string, resourceGroupName string, computeName string) (string, string, error) - deleteDisk func(ctx context.Context, imageName string, resourceGroupName string, isManagedDisk bool, subscriptionId string, storageAccountName 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 - templateType DeploymentTemplateType + client *AzureClient + deploy func(ctx context.Context, subscriptionId string, resourceGroupName string, deploymentName string) error + deleteDetatchedResources func(ctx context.Context, subscriptionId string, resourceGroupName string, resources map[string]string) error + getDisk func(ctx context.Context, subscriptionId string, resourceGroupName string, computeName string) (string, string, error) + deleteDisk func(ctx context.Context, imageName string, resourceGroupName string, isManagedDisk bool, subscriptionId string, storageAccountName string) error + deleteVM func(ctx context.Context, virtualMachineId virtualmachines.VirtualMachineId) error + deleteNic func(ctx context.Context, networkInterfacesId commonids.NetworkInterfaceId) error + deleteDeployment func(ctx context.Context, state multistep.StateBag) error + deleteKV func(ctx context.Context, id commonids.KeyVaultId) error + listDeploymentOps func(ctx context.Context, id deploymentoperations.ResourceGroupDeploymentId) ([]deploymentoperations.DeploymentOperation, error) + say func(message string) + error func(e error) + config *Config + factory templateFactoryFunc + name string + templateType DeploymentTemplateType } func NewStepDeployTemplate(client *AzureClient, ui packersdk.Ui, config *Config, deploymentName string, factory templateFactoryFunc, templateType DeploymentTemplateType) *StepDeployTemplate { @@ -59,10 +67,14 @@ func NewStepDeployTemplate(client *AzureClient, ui packersdk.Ui, config *Config, } step.deploy = step.deployTemplate - step.delete = step.deleteDeploymentResources - step.disk = step.getImageDetails + step.getDisk = step.getImageDetails step.deleteDisk = step.deleteImage + step.deleteDetatchedResources = step.deleteDetatchedResourcesWithQueue step.deleteDeployment = step.deleteDeploymentObject + step.deleteNic = step.deleteNetworkInterface + step.deleteVM = step.deleteVirtualMachine + step.deleteKV = step.deleteKeyVault + step.listDeploymentOps = step.listDeploymentOperations return step } @@ -93,61 +105,134 @@ func (s *StepDeployTemplate) Cleanup(state multistep.StateBag) { deploymentName := s.name resourceGroupName := state.Get(constants.ArmResourceGroupName).(string) subscriptionId := state.Get(constants.ArmSubscription).(string) + deploymentOpsId := deploymentoperations.ResourceGroupDeploymentId{ + DeploymentName: deploymentName, + ResourceGroupName: resourceGroupName, + SubscriptionId: subscriptionId, + } + // If this is the KeyVault Deployment, delete the KeyVault if s.templateType == KeyVaultTemplate { ui.Say("\nDeleting KeyVault created during build") - err := s.delete(ctx, subscriptionId, deploymentName, resourceGroupName) + deploymentOperations, err := s.listDeploymentOps(ctx, deploymentOpsId) if err != nil { - s.reportIfError(err, resourceGroupName) + ui.Error(fmt.Sprintf("Could not retrieve deployment operations: %s\n to get the KeyVault, please manually delete it ", err)) + return } + for _, deploymentOperation := range deploymentOperations { + // Sometimes an empty operation is added to the list by Azure + if deploymentOperation.Properties.TargetResource == nil { + continue + } + resourceName := *deploymentOperation.Properties.TargetResource.ResourceName + resourceType := *deploymentOperation.Properties.TargetResource.ResourceType + + if resourceType == KeyVaultResourceType { + kvID := commonids.KeyVaultId{ + VaultName: resourceName, + ResourceGroupName: resourceGroupName, + SubscriptionId: subscriptionId, + } + err := s.deleteKV(ctx, kvID) + if err != nil { + s.reportResourceDeletionFailure(err, resourceGroupName) + } + return + } + } + return + } + // Otherwise delete the Virtual Machine + ui.Say("\nDeleting Virtual Machine deployment and its attached resources...") + // 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) + isManagedDisk := state.Get(constants.ArmIsManagedImage).(bool) + isSIGImage := state.Get(constants.ArmIsSIGImage).(bool) + armStorageAccountName := state.Get(constants.ArmStorageAccountName).(string) + imageType, imageName, err := s.getDisk(ctx, subscriptionId, resourceGroupName, computeName) + if err != nil { + ui.Error(fmt.Sprintf("Could not retrieve OS Image details: %s", err)) + } - } else { - ui.Say("\nDeleting Virtual Machine deployment and its attached resources...") - // 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) - isManagedDisk := state.Get(constants.ArmIsManagedImage).(bool) - isSIGImage := state.Get(constants.ArmIsSIGImage).(bool) - armStorageAccountName := state.Get(constants.ArmStorageAccountName).(string) - imageType, imageName, err := s.disk(ctx, subscriptionId, resourceGroupName, computeName) - if err != nil { - ui.Error(fmt.Sprintf("Could not retrieve OS Image details: %s", err)) + deploymentOperations, err := s.listDeploymentOps(ctx, deploymentOpsId) + if err != nil { + ui.Error(fmt.Sprintf("Could not retrieve deployment operations: %s\n Virtual Machine %s, and its please manually delete it and its ascoiated resources", err, computeName)) + return + } + resources := map[string]string{} + var vmID *virtualmachines.VirtualMachineId + var networkInterfaceID *commonids.NetworkInterfaceId + for _, deploymentOperation := range deploymentOperations { + // Sometimes an empty operation is added to the list by Azure + if deploymentOperation.Properties.TargetResource == nil { + continue + } + resourceName := *deploymentOperation.Properties.TargetResource.ResourceName + resourceType := *deploymentOperation.Properties.TargetResource.ResourceType + + // Grab the Virtual Machine and Network ID resource names, and save them into Azure Resource IDs to be used later + // We always want to delete the VM first, then the NIC, even if the ListDeployment endpoint doesn't return resources sorted in the order we want to delete them + switch resourceType { + case "Microsoft.Compute/virtualMachines": + vmIDDeref := virtualmachines.NewVirtualMachineID(subscriptionId, resourceGroupName, resourceName) + vmID = &vmIDDeref + case "Microsoft.Network/networkInterfaces": + networkInterfaceIDDeref := commonids.NewNetworkInterfaceID(subscriptionId, resourceGroupName, resourceName) + networkInterfaceID = &networkInterfaceIDDeref + default: + resources[resourceType] = resourceName } - err = s.delete(ctx, subscriptionId, deploymentName, resourceGroupName) + } + + if vmID != nil { + + err := s.deleteVM(ctx, *vmID) if err != nil { - s.reportIfError(err, resourceGroupName) + return } - // 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)) + } + if networkInterfaceID != nil { + err := s.deleteNic(ctx, *networkInterfaceID) + if err != nil { return } - if !state.Get(constants.ArmKeepOSDisk).(bool) { - ui.Say(fmt.Sprintf(" Deleting -> %s : '%s'", imageType, imageName)) - err = s.deleteDisk(ctx, imageName, resourceGroupName, (isManagedDisk || isSIGImage), subscriptionId, armStorageAccountName) - if err != nil { - ui.Error(fmt.Sprintf("Error deleting resource. Please delete manually.\n\n"+ - "Name: %s\n"+ - "Error: %s", imageName, err)) - } + } + err = s.deleteDetatchedResources(ctx, subscriptionId, resourceGroupName, resources) + if err != nil { + s.reportResourceDeletionFailure(err, resourceGroupName) + } + // 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 + } + if !state.Get(constants.ArmKeepOSDisk).(bool) { + err = s.deleteDisk(ctx, imageName, resourceGroupName, (isManagedDisk || isSIGImage), subscriptionId, armStorageAccountName) + if err != nil { + s.reportResourceDeletionFailure(err, imageName) + } else { - ui.Say(fmt.Sprintf("Skipping deletion -> %s : '%s' since 'keep_os_disk' is set to true", imageType, imageName)) + ui.Say(fmt.Sprintf("Deleted -> %s : '%s'", imageType, imageName)) } - var dataDisks []string - if disks := state.Get(constants.ArmAdditionalDiskVhds); disks != nil { - dataDisks = disks.([]string) - } - for i, additionaldisk := range dataDisks { - s.say(fmt.Sprintf(" Deleting Additional Disk -> %d: '%s'", i+1, additionaldisk)) + } else { + ui.Say(fmt.Sprintf("Skipping deletion -> %s : '%s' since 'keep_os_disk' is set to true", imageType, imageName)) + } + var dataDisks []string + if disks := state.Get(constants.ArmAdditionalDiskVhds); disks != nil { + dataDisks = disks.([]string) + } + for i, additionaldisk := range dataDisks { + err := s.deleteImage(ctx, additionaldisk, resourceGroupName, (isManagedDisk || isSIGImage), subscriptionId, armStorageAccountName) + if err == nil { + s.say(fmt.Sprintf("Deleted Additional Disk -> %d: '%s'", i+1, additionaldisk)) - err := s.deleteImage(ctx, additionaldisk, resourceGroupName, (isManagedDisk || isSIGImage), subscriptionId, armStorageAccountName) - if err != nil { - s.say("Failed to delete the managed Additional Disk!") - } + } else { + s.reportResourceDeletionFailure(err, additionaldisk) } - } + } func (s *StepDeployTemplate) deployTemplate(ctx context.Context, subscriptionId string, resourceGroupName string, deploymentName string) error { @@ -184,16 +269,11 @@ func (s *StepDeployTemplate) deleteDeploymentObject(ctx context.Context, state m } func (s *StepDeployTemplate) getImageDetails(ctx context.Context, subscriptionId string, resourceGroupName string, computeName string) (string, string, error) { - //TODO is this still true? - //We can't depend on constants.ArmOSDiskVhd being set var imageName, imageType string pollingContext, cancel := context.WithTimeout(ctx, s.client.PollingDuration) defer cancel() vmID := virtualmachines.NewVirtualMachineID(subscriptionId, resourceGroupName, computeName) vm, err := s.client.VirtualMachinesClient.Get(pollingContext, vmID, virtualmachines.DefaultGetOperationOptions()) - if err != nil { - return imageName, imageType, err - } if err != nil { s.say(s.client.LastError.Error()) return "", "", err @@ -217,41 +297,6 @@ func (s *StepDeployTemplate) getImageDetails(ctx context.Context, subscriptionId return imageType, imageName, nil } -func deleteResource(ctx context.Context, client *AzureClient, subscriptionId string, resourceType string, resourceName string, resourceGroupName string) error { - - pollingContext, cancel := context.WithTimeout(ctx, client.PollingDuration) - defer cancel() - - switch resourceType { - case "Microsoft.Compute/virtualMachines": - vmID := virtualmachines.NewVirtualMachineID(subscriptionId, resourceGroupName, resourceName) - if err := client.VirtualMachinesClient.DeleteThenPoll(pollingContext, vmID, virtualmachines.DefaultDeleteOperationOptions()); err != nil { - return err - } - case "Microsoft.KeyVault/vaults": - id := commonids.NewKeyVaultID(subscriptionId, resourceGroupName, resourceName) - _, err := client.VaultsClient.Delete(pollingContext, id) - return err - case "Microsoft.Network/networkInterfaces": - interfaceID := commonids.NewNetworkInterfaceID(subscriptionId, resourceGroupName, resourceName) - err := client.NetworkMetaClient.NetworkInterfaces.DeleteThenPoll(pollingContext, interfaceID) - return err - case "Microsoft.Network/virtualNetworks": - vnetID := commonids.NewVirtualNetworkID(subscriptionId, resourceGroupName, resourceName) - err := client.NetworkMetaClient.VirtualNetworks.DeleteThenPoll(pollingContext, vnetID) - return err - case "Microsoft.Network/networkSecurityGroups": - secGroupId := networksecuritygroups.NewNetworkSecurityGroupID(subscriptionId, resourceGroupName, resourceName) - err := client.NetworkMetaClient.NetworkSecurityGroups.DeleteThenPoll(pollingContext, secGroupId) - return err - case "Microsoft.Network/publicIPAddresses": - ipID := commonids.NewPublicIPAddressID(subscriptionId, resourceGroupName, resourceName) - err := client.NetworkMetaClient.PublicIPAddresses.DeleteThenPoll(pollingContext, ipID) - return err - } - return nil -} - // TODO Let's split this into two separate methods // deleteVHD and deleteManagedDisk, and then just check in Cleanup which function to call func (s *StepDeployTemplate) deleteImage(ctx context.Context, imageName string, resourceGroupName string, isManagedDisk bool, subscriptionId string, storageAccountName string) error { @@ -279,82 +324,125 @@ func (s *StepDeployTemplate) deleteImage(ctx context.Context, imageName string, if len(xs) < 3 { return errors.New("Unable to parse path of image " + imageName) } - _, err = s.client.GiovanniBlobClient.Delete(pollingContext, storageAccountName, blobName, giovanniBlobStorageSDK.DeleteInput{}) + _, err = s.client.GiovanniBlobClient.Delete(pollingContext, "images", blobName, giovanniBlobStorageSDK.DeleteInput{}) return err } -func (s *StepDeployTemplate) deleteDeploymentResources(ctx context.Context, subscriptionId, deploymentName, resourceGroupName string) error { +func (s *StepDeployTemplate) retryDeletion(ctx context.Context, resourceType string, resourceName string, deleteResourceFunction func() error) error { + log.Printf("[INFO] Attempting deletion -> %s : %s", resourceType, resourceName) + retryConfig := retry.Config{ + Tries: 5, + RetryDelay: (&retry.Backoff{ + InitialBackoff: 3 * time.Second, + MaxBackoff: 15 * time.Second, + Multiplier: 1.5, + }).Linear, + } + err := retryConfig.Run(ctx, func(ctx context.Context) error { + err := deleteResourceFunction() + if err != nil { + log.Printf("[INFO] Couldn't delete resource %s.%s, will retry", resourceType, resourceName) + } + return err + }) + if err != nil { + s.reportResourceDeletionFailure(err, resourceName) + } else { + s.say(fmt.Sprintf("Deleted -> %s : '%s'", resourceType, resourceName)) + } + return err + +} + +func (s *StepDeployTemplate) deleteVirtualMachine(ctx context.Context, vmID virtualmachines.VirtualMachineId) error { + pollingContext, cancel := context.WithTimeout(ctx, s.client.PollingDuration) + defer cancel() + err := s.retryDeletion(pollingContext, vmID.VirtualMachineName, VMResourceType, func() error { + return s.client.VirtualMachinesClient.DeleteThenPoll(ctx, vmID, virtualmachines.DefaultDeleteOperationOptions()) + }) + return err +} + +func (s *StepDeployTemplate) deleteKeyVault(ctx context.Context, id commonids.KeyVaultId) error { + pollingContext, cancel := context.WithTimeout(ctx, s.client.PollingDuration) + defer cancel() + err := s.retryDeletion(pollingContext, id.VaultName, KeyVaultResourceType, func() error { + _, err := s.client.VaultsClient.Delete(pollingContext, id) + return err + }) + return err +} + +func (s *StepDeployTemplate) deleteNetworkInterface(ctx context.Context, id commonids.NetworkInterfaceId) error { + pollingContext, cancel := context.WithTimeout(ctx, s.client.PollingDuration) + defer cancel() + err := s.retryDeletion(pollingContext, id.NetworkInterfaceName, NetworkInterfaceResourceType, func() error { + return s.client.NetworkMetaClient.NetworkInterfaces.DeleteThenPoll(pollingContext, id) + }) + return err +} + +func (s *StepDeployTemplate) listDeploymentOperations(ctx context.Context, id deploymentoperations.ResourceGroupDeploymentId) ([]deploymentoperations.DeploymentOperation, error) { var maxResources int64 = 50 options := deploymentoperations.DefaultListOperationOptions() options.Top = &maxResources - id := deploymentoperations.NewResourceGroupDeploymentID(subscriptionId, resourceGroupName, deploymentName) pollingContext, cancel := context.WithTimeout(ctx, s.client.PollingDuration) defer cancel() deploymentOperations, err := s.client.DeploymentOperationsClient.ListComplete(pollingContext, id, options) if err != nil { - s.reportIfError(err, resourceGroupName) - return err - } - - resources := map[string]string{} - - for _, deploymentOperation := range deploymentOperations.Items { - // Sometimes an empty operation is added to the list by Azure - if deploymentOperation.Properties.TargetResource == nil { - continue - } - - resourceName := *deploymentOperation.Properties.TargetResource.ResourceName - resourceType := *deploymentOperation.Properties.TargetResource.ResourceType - - s.say(fmt.Sprintf("Adding to deletion queue -> %s : '%s'", resourceType, resourceName)) - resources[resourceType] = resourceName - + return nil, err } + return deploymentOperations.Items, nil +} +// This function is called to delete the resources remaining in the deployment after we delete the Virtual Machine and the deleteNic +// These resources before the VM and the NIC results in errors +func (s *StepDeployTemplate) deleteDetatchedResourcesWithQueue(ctx context.Context, subscriptionId string, resourceGroupName string, resources map[string]string) error { 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: 5 * time.Second, MaxBackoff: 60 * time.Second, Multiplier: 1.5}).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, + s.retryDeletion(ctx, resourceName, resourceType, func() error { + return deleteResource(ctx, s.client, subscriptionId, resourceType, resourceName, resourceGroupName) - if err != nil { - s.say(fmt.Sprintf("Couldn't delete %s resource. Will retry.\n"+ - "Name: %s", - resourceType, resourceName)) - } - return err }) - if err != nil { - s.reportIfError(err, resourceName) - } + }(resourceType, resourceName) } - s.say("Waiting for deletion of all resources...") wg.Wait() 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) +func deleteResource(ctx context.Context, client *AzureClient, subscriptionId string, resourceType string, resourceName string, resourceGroupName string) error { + pollingContext, cancel := context.WithTimeout(ctx, client.PollingDuration) + defer cancel() + + var err error + switch resourceType { + case "Microsoft.Network/virtualNetworks": + vnetID := commonids.NewVirtualNetworkID(subscriptionId, resourceGroupName, resourceName) + err = client.NetworkMetaClient.VirtualNetworks.DeleteThenPoll(pollingContext, vnetID) + case "Microsoft.Network/networkSecurityGroups": + secGroupId := networksecuritygroups.NewNetworkSecurityGroupID(subscriptionId, resourceGroupName, resourceName) + err = client.NetworkMetaClient.NetworkSecurityGroups.DeleteThenPoll(pollingContext, secGroupId) + case "Microsoft.Network/publicIPAddresses": + ipID := commonids.NewPublicIPAddressID(subscriptionId, resourceGroupName, resourceName) + err = client.NetworkMetaClient.PublicIPAddresses.DeleteThenPoll(pollingContext, ipID) } + return err +} + +func (s *StepDeployTemplate) reportResourceDeletionFailure(err error, resourceName string) { + s.say(fmt.Sprintf("Error deleting resource. Please delete manually.\n\n"+ + "Name: %s\n"+ + "Error: %s", resourceName, err.Error())) + s.error(err) } diff --git a/builder/azure/arm/step_deploy_template_test.go b/builder/azure/arm/step_deploy_template_test.go index 7f266869..a0cbdc92 100644 --- a/builder/azure/arm/step_deploy_template_test.go +++ b/builder/azure/arm/step_deploy_template_test.go @@ -9,6 +9,11 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/go-azure-helpers/resourcemanager/commonids" + "github.com/hashicorp/go-azure-sdk/resource-manager/compute/2022-03-01/virtualmachines" + "github.com/hashicorp/go-azure-sdk/resource-manager/resources/2022-09-01/deploymentoperations" + "github.com/hashicorp/packer-plugin-azure/builder/azure/common" "github.com/hashicorp/packer-plugin-azure/builder/azure/common/constants" "github.com/hashicorp/packer-plugin-sdk/multistep" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" @@ -126,8 +131,10 @@ func TestStepDeployTemplateDeleteImageShouldFailWithInvalidImage(t *testing.T) { } func TestStepDeployTemplateCleanupShouldDeleteManagedOSImageInExistingResourceGroup(t *testing.T) { - var deleteDiskCounter = 0 - var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter, VirtualMachineTemplate) + mockTrackers := mockTrackers{ + deleteDiskCounter: common.IntPtr(0), + } + var testSubject = createTestStepDeployTemplateDeleteOSImage(t, &mockTrackers, VirtualMachineTemplate, virtualMachineDeploymentOperations()) stateBag := createTestStateBagStepDeployTemplate() stateBag.Put(constants.ArmIsManagedImage, true) @@ -138,14 +145,19 @@ func TestStepDeployTemplateCleanupShouldDeleteManagedOSImageInExistingResourceGr 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) + if *mockTrackers.deleteDiskCounter != 1 { + t.Fatalf("Expected DeployTemplate Cleanup to invoke deleteDisk 1 time, but invoked %d times", *mockTrackers.deleteDiskCounter) + } + if mockTrackers.vmDeleteCalled == nil || *mockTrackers.vmDeleteCalled == false { + t.Fatalf("Expected DeployTemplate cleanup to delete call deleteVM to delete the VirtualMachine but didn't") } } func TestStepDeployTemplateCleanupShouldDeleteManagedOSImageInTemporaryResourceGroup(t *testing.T) { - var deleteDiskCounter = 0 - var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter, VirtualMachineTemplate) + mockTrackers := mockTrackers{ + deleteDiskCounter: common.IntPtr(0), + } + var testSubject = createTestStepDeployTemplateDeleteOSImage(t, &mockTrackers, VirtualMachineTemplate, virtualMachineDeploymentOperations()) stateBag := createTestStateBagStepDeployTemplate() stateBag.Put(constants.ArmIsManagedImage, true) @@ -156,14 +168,19 @@ func TestStepDeployTemplateCleanupShouldDeleteManagedOSImageInTemporaryResourceG 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) + if *mockTrackers.deleteDiskCounter != 1 { + t.Fatalf("Expected DeployTemplate Cleanup to invoke deleteDisk 1 time, but invoked %d times", *mockTrackers.deleteDiskCounter) + } + if mockTrackers.vmDeleteCalled == nil || *mockTrackers.vmDeleteCalled == false { + t.Fatalf("Expected DeployTemplate cleanup to delete call deleteVM to delete the VirtualMachine but didn't") } } func TestStepDeployTemplateCleanupShouldDeleteVHDOSImageInExistingResourceGroup(t *testing.T) { - var deleteDiskCounter = 0 - var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter, VirtualMachineTemplate) + mockTrackers := mockTrackers{ + deleteDiskCounter: common.IntPtr(0), + } + var testSubject = createTestStepDeployTemplateDeleteOSImage(t, &mockTrackers, VirtualMachineTemplate, virtualMachineDeploymentOperations()) stateBag := createTestStateBagStepDeployTemplate() stateBag.Put(constants.ArmIsManagedImage, false) @@ -174,14 +191,19 @@ func TestStepDeployTemplateCleanupShouldDeleteVHDOSImageInExistingResourceGroup( 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) + if *mockTrackers.deleteDiskCounter != 1 { + t.Fatalf("Expected DeployTemplate Cleanup to invoke deleteDisk 1 time, but invoked %d times", *mockTrackers.deleteDiskCounter) + } + if mockTrackers.vmDeleteCalled == nil || *mockTrackers.vmDeleteCalled == false { + t.Fatalf("Expected DeployTemplate cleanup to delete call deleteVM to delete the VirtualMachine but didn't") } } func TestStepDeployTemplateCleanupShouldVHDOSImageInTemporaryResourceGroup(t *testing.T) { - var deleteDiskCounter = 0 - var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter, VirtualMachineTemplate) + mockTrackers := mockTrackers{ + deleteDiskCounter: common.IntPtr(0), + } + var testSubject = createTestStepDeployTemplateDeleteOSImage(t, &mockTrackers, VirtualMachineTemplate, virtualMachineDeploymentOperations()) stateBag := createTestStateBagStepDeployTemplate() stateBag.Put(constants.ArmIsManagedImage, false) @@ -192,14 +214,66 @@ func TestStepDeployTemplateCleanupShouldVHDOSImageInTemporaryResourceGroup(t *te 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) + if *mockTrackers.deleteDiskCounter != 1 { + t.Fatalf("Expected DeployTemplate Cleanup to invoke deleteDisk 1 times, but invoked %d times", *mockTrackers.deleteDiskCounter) + } + if mockTrackers.vmDeleteCalled == nil || *mockTrackers.vmDeleteCalled == false { + t.Fatalf("Expected DeployTemplate cleanup to delete call deleteVM to delete the VirtualMachine but didn't") } } +func TestStepDeployTemplateCleanupShouldDeleteVirtualMachineAndNetworkResourcesInOrderToAvoidConflicts(t *testing.T) { + mockTrackers := mockTrackers{ + actualNetworkResources: nil, + } + var testSubject = createTestStepDeployTemplateDeleteOSImage(t, &mockTrackers, VirtualMachineTemplate, virtualMachineDeploymentOperations()) + testSubject.listDeploymentOps = func(ctx context.Context, id deploymentoperations.ResourceGroupDeploymentId) ([]deploymentoperations.DeploymentOperation, error) { + return virtualMachineWithNetworkingDeploymentOperations(), nil + } + testSubject.deleteDetatchedResources = func(ctx context.Context, subscriptionId, resourceGroupName string, resources map[string]string) error { + if mockTrackers.vmDeleteCalled == nil || *mockTrackers.vmDeleteCalled == false { + t.Fatal("deleteNetworkResources called before deleting VM, this will lead to deletion conflicts") + } + if mockTrackers.deleteNicCalled == nil || *mockTrackers.deleteNicCalled == false { + t.Fatal("deleteNetworkResources called before deleting NIC, this will lead to deletion conflicts") + } + + if len(resources) != 0 { + mockTrackers.actualNetworkResources = &resources + } + return nil + } -func TestStepDeployTemplateCleanupShouldNotDeleteDiskForKeyVaultDeployments(t *testing.T) { - var deleteDiskCounter = 0 - var testSubject = createTestStepDeployTemplateDeleteOSImage(&deleteDiskCounter, KeyVaultTemplate) + stateBag := createTestStateBagStepDeployTemplate() + stateBag.Put(constants.ArmIsManagedImage, false) + stateBag.Put(constants.ArmIsSIGImage, false) + stateBag.Put(constants.ArmIsExistingResourceGroup, true) + stateBag.Put(constants.ArmIsResourceGroupCreated, true) + stateBag.Put(constants.ArmKeepOSDisk, false) + stateBag.Put("ui", packersdk.TestUi(t)) + + testSubject.Cleanup(stateBag) + if mockTrackers.vmDeleteCalled == nil || *mockTrackers.vmDeleteCalled == false { + t.Fatalf("Expected DeployTemplate cleanup to call deleteVM to delete the VirtualMachine but it didn't") + } + if mockTrackers.deleteNicCalled == nil || *mockTrackers.deleteNicCalled == false { + t.Fatalf("Expected DeployTemplate cleanup to call delete network interface but it didn't") + } + if mockTrackers.actualNetworkResources == nil { + t.Fatalf("Expected DeployTemplate to call delete network resources but it didn't") + } else { + expectedResources := map[string]string{"Microsoft.Network/publicIPAddresses": "ip", "Microsoft.Network/virtualNetworks": "vnet"} + if diff := cmp.Diff(expectedResources, *mockTrackers.actualNetworkResources); diff != "" { + t.Fatalf("Unexpected difference in expected parameter deleteNetworkResources.resources %s", diff) + } + } +} +func TestStepDeployTemplateCleanupShouldDeleteKeyVault(t *testing.T) { + mockTrackers := mockTrackers{ + keyVaultDeleteCalled: common.BoolPtr(false), + } + // This step lacks any methods not required to delete the key vault + // As such it validates that during the deletion of the key vault, no other endpoints are called, such as delete Disk + var testSubject = createTestStepDeployTemplateKeyVault(&mockTrackers, KeyVaultTemplate, keyVaultDeploymentOperations()) stateBag := createTestStateBagStepDeployTemplate() stateBag.Put(constants.ArmIsManagedImage, false) @@ -210,8 +284,8 @@ func TestStepDeployTemplateCleanupShouldNotDeleteDiskForKeyVaultDeployments(t *t stateBag.Put("ui", packersdk.TestUi(t)) testSubject.Cleanup(stateBag) - if deleteDiskCounter != 0 { - t.Fatalf("Expected DeployTemplate Cleanup to not invoke deleteDisk, but invoked %d times", deleteDiskCounter) + if mockTrackers.keyVaultDeleteCalled == nil || *mockTrackers.keyVaultDeleteCalled == false { + t.Fatalf("Expected DeployTemplate cleanup to delete call deleteKV to delete the KeyVault but didn't") } } @@ -227,19 +301,129 @@ func createTestStateBagStepDeployTemplate() multistep.StateBag { return stateBag } -func createTestStepDeployTemplateDeleteOSImage(deleteDiskCounter *int, templateType DeploymentTemplateType) *StepDeployTemplate { +func virtualMachineDeploymentOperations() []deploymentoperations.DeploymentOperation { + return []deploymentoperations.DeploymentOperation{ + { + Properties: &deploymentoperations.DeploymentOperationProperties{ + TargetResource: &deploymentoperations.TargetResource{ + ResourceName: common.StringPtr("virtualmachine"), + ResourceType: common.StringPtr("Microsoft.Compute/virtualMachines"), + }, + }, + }, + } +} + +func virtualMachineWithNetworkingDeploymentOperations() []deploymentoperations.DeploymentOperation { + return []deploymentoperations.DeploymentOperation{ + { + Properties: &deploymentoperations.DeploymentOperationProperties{ + TargetResource: &deploymentoperations.TargetResource{ + ResourceName: common.StringPtr("virtualmachine"), + ResourceType: common.StringPtr("Microsoft.Compute/virtualMachines"), + }, + }, + }, + { + Properties: &deploymentoperations.DeploymentOperationProperties{ + TargetResource: &deploymentoperations.TargetResource{ + ResourceName: common.StringPtr("coolnic"), + ResourceType: common.StringPtr("Microsoft.Network/networkInterfaces"), + }, + }, + }, + { + Properties: &deploymentoperations.DeploymentOperationProperties{ + TargetResource: &deploymentoperations.TargetResource{ + ResourceName: common.StringPtr("vnet"), + ResourceType: common.StringPtr("Microsoft.Network/virtualNetworks"), + }, + }, + }, + { + Properties: &deploymentoperations.DeploymentOperationProperties{ + TargetResource: &deploymentoperations.TargetResource{ + ResourceName: common.StringPtr("ip"), + ResourceType: common.StringPtr("Microsoft.Network/publicIPAddresses"), + }, + }, + }, + } +} + +func keyVaultDeploymentOperations() []deploymentoperations.DeploymentOperation { + return []deploymentoperations.DeploymentOperation{ + { + Properties: &deploymentoperations.DeploymentOperationProperties{ + TargetResource: &deploymentoperations.TargetResource{ + ResourceName: common.StringPtr("vault3-mojave"), + ResourceType: common.StringPtr("Microsoft.KeyVault/vaults"), + }, + }, + }, + } +} + +type mockTrackers struct { + deleteDiskCounter *int + deleteNicCalled *bool + keyVaultDeleteCalled *bool + vmDeleteCalled *bool + actualNetworkResources *map[string]string +} + +func createTestStepDeployTemplateDeleteOSImage(t *testing.T, trackers *mockTrackers, templateType DeploymentTemplateType, deploymentOperations []deploymentoperations.DeploymentOperation) *StepDeployTemplate { + if trackers.deleteDiskCounter == nil { + trackers.deleteDiskCounter = common.IntPtr(0) + } return &StepDeployTemplate{ deploy: func(context.Context, string, string, string) error { return nil }, say: func(message string) {}, error: func(e error) {}, deleteDisk: func(ctx context.Context, imageName string, resourceGroupName string, isManagedDisk bool, subscriptionId string, storageAccountName string) error { - *deleteDiskCounter++ + *trackers.deleteDiskCounter++ return nil }, - disk: func(ctx context.Context, subscriptionId, resourceGroupName, computeName string) (string, string, error) { + getDisk: func(ctx context.Context, subscriptionId, resourceGroupName, computeName string) (string, string, error) { return "Microsoft.Compute/disks", "", nil }, - delete: func(ctx context.Context, subscriptionId, deploymentName, resourceGroupName string) error { + deleteNic: func(ctx context.Context, networkInterfacesId commonids.NetworkInterfaceId) error { + if trackers.vmDeleteCalled == nil || *trackers.vmDeleteCalled == false { + t.Fatal("Unexpectedly deleteNic before deleting VM") + } + trackers.deleteNicCalled = common.BoolPtr(true) + return nil + }, + deleteDetatchedResources: func(ctx context.Context, subscriptionId string, resourceGroupName string, resources map[string]string) error { + if len(resources) != 0 { + trackers.actualNetworkResources = &resources + } + return nil + }, + deleteDeployment: func(ctx context.Context, state multistep.StateBag) error { + return nil + }, + listDeploymentOps: func(ctx context.Context, id deploymentoperations.ResourceGroupDeploymentId) ([]deploymentoperations.DeploymentOperation, error) { + return deploymentOperations, nil + }, + deleteVM: func(ctx context.Context, virtualMachineId virtualmachines.VirtualMachineId) error { + trackers.vmDeleteCalled = common.BoolPtr(true) + return nil + }, + templateType: templateType, + } +} + +func createTestStepDeployTemplateKeyVault(trackers *mockTrackers, templateType DeploymentTemplateType, deploymentOperations []deploymentoperations.DeploymentOperation) *StepDeployTemplate { + return &StepDeployTemplate{ + deploy: func(context.Context, string, string, string) error { return nil }, + say: func(message string) {}, + error: func(e error) {}, + listDeploymentOps: func(ctx context.Context, id deploymentoperations.ResourceGroupDeploymentId) ([]deploymentoperations.DeploymentOperation, error) { + return deploymentOperations, nil + }, + deleteKV: func(ctx context.Context, id commonids.KeyVaultId) error { + trackers.keyVaultDeleteCalled = common.BoolPtr(true) return nil }, deleteDeployment: func(ctx context.Context, state multistep.StateBag) error { diff --git a/go.mod b/go.mod index 8446094b..deb10553 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/hashicorp/go-azure-sdk/resource-manager v0.20240411.1145857 github.com/hashicorp/go-azure-sdk/sdk v0.20240411.1145857 github.com/mitchellh/go-homedir v1.1.0 - github.com/tombuildsstuff/giovanni v0.25.3 + github.com/tombuildsstuff/giovanni v0.26.1 ) require ( diff --git a/go.sum b/go.sum index 89e8c0e3..02e3540d 100644 --- a/go.sum +++ b/go.sum @@ -163,12 +163,8 @@ github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/go-azure-helpers v0.66.2 h1:+Pzuo7pdKl0hBXXr5ymmhs4Q40tHAo2nAvHq4WgSjx8= github.com/hashicorp/go-azure-helpers v0.66.2/go.mod h1:kJxXrFtJKJdOEqvad8pllAe7dhP4DbN8J6sqFZe47+4= -github.com/hashicorp/go-azure-sdk/resource-manager v0.20240327.1161949 h1:6kiYPtSO7l08UshpjOkeBvqDOIOdRXhYFMAYRdosLoo= -github.com/hashicorp/go-azure-sdk/resource-manager v0.20240327.1161949/go.mod h1:6emA77HGPRTEV9n0VLpVfzsvinp6iDBqsf/W1C+eRhY= github.com/hashicorp/go-azure-sdk/resource-manager v0.20240411.1145857 h1:pgbiUHdg4QGd2Seg48fsWVNS2ydMoS5ZzjYyVGbDndk= github.com/hashicorp/go-azure-sdk/resource-manager v0.20240411.1145857/go.mod h1:MpsjFWjpJGEFSenIPkWORZrvO7ZAA46bNZKu1HQrTaY= -github.com/hashicorp/go-azure-sdk/sdk v0.20240327.1161949 h1:Iu3U8RUGKQNza6MSFyTNjgasDaEcpehqr0Rn4ruBHBk= -github.com/hashicorp/go-azure-sdk/sdk v0.20240327.1161949/go.mod h1:POOjeoqNp+mvlLBuibJTziUAkBZ7FxXGeGestwemL/w= github.com/hashicorp/go-azure-sdk/sdk v0.20240411.1145857 h1:zc6BJzd1u6fvQ1zjwwWMMnhAKE/EYsL2DKBM5Yl1Iko= github.com/hashicorp/go-azure-sdk/sdk v0.20240411.1145857/go.mod h1:POOjeoqNp+mvlLBuibJTziUAkBZ7FxXGeGestwemL/w= github.com/hashicorp/go-cleanhttp v0.5.0/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= @@ -375,8 +371,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -github.com/tombuildsstuff/giovanni v0.25.3 h1:sqUXiKvU4tmmGA3+YXZe87j4ngu7DkwijFO/ex8a4Hk= -github.com/tombuildsstuff/giovanni v0.25.3/go.mod h1:s7xbU2lN5Iz9MBglmDDv9p2QPbn6x3UkJBtpCfUerLs= +github.com/tombuildsstuff/giovanni v0.26.1 h1:RZgnpyIHtgw0GXYpw3xttNk35obJNoI1hztCZsh/Djo= +github.com/tombuildsstuff/giovanni v0.26.1/go.mod h1:s7xbU2lN5Iz9MBglmDDv9p2QPbn6x3UkJBtpCfUerLs= github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM= github.com/ugorji/go v1.2.6/go.mod h1:anCg0y61KIhDlPZmnH+so+RQbysYVyDko0IMgJv0Nn0= github.com/ugorji/go/codec v1.2.6 h1:7kbGefxLoDBuYXOms4yD7223OpNMMPNPZxXk5TvFcyQ=