From c8c9bbb22ad0458709c26f06a3830ef1542b966c Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Mon, 14 May 2018 20:06:23 -0700 Subject: [PATCH 1/3] Async delete Resource Group --- builder/azure/arm/builder.go | 1 + builder/azure/arm/config.go | 3 +++ builder/azure/arm/step_create_resource_group.go | 12 +++++++++--- builder/azure/arm/step_delete_resource_group.go | 8 +++++++- builder/azure/common/constants/stateBag.go | 1 + website/source/docs/builders/azure.html.md | 4 ++++ 6 files changed, 25 insertions(+), 4 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index 5603f0e52..e59f0f84b 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -344,6 +344,7 @@ func (b *Builder) configureStateBag(stateBag multistep.StateBag) { stateBag.Put(constants.ArmIsManagedImage, b.config.isManagedImage()) stateBag.Put(constants.ArmManagedImageResourceGroupName, b.config.ManagedImageResourceGroupName) stateBag.Put(constants.ArmManagedImageName, b.config.ManagedImageName) + stateBag.Put(constants.ArmAsyncRGDelete, b.config.AsyncRGDelete) } // Parameters that are only known at runtime after querying Azure. diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index de3a0784e..ded561a58 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -151,6 +151,9 @@ type Config struct { Comm communicator.Config `mapstructure:",squash"` ctx *interpolate.Context + + //Cleanup + AsyncRGDelete bool `mapstructure:"async_resourcegroup_delete"` } type keyVaultCertificate struct { diff --git a/builder/azure/arm/step_create_resource_group.go b/builder/azure/arm/step_create_resource_group.go index 9262f868a..8036adb36 100644 --- a/builder/azure/arm/step_create_resource_group.go +++ b/builder/azure/arm/step_create_resource_group.go @@ -118,14 +118,20 @@ func (s *StepCreateResourceGroup) Cleanup(state multistep.StateBag) { ctx := context.TODO() f, err := s.client.GroupsClient.Delete(ctx, resourceGroupName) if err == nil { - err = f.WaitForCompletion(ctx, s.client.GroupsClient.Client) + if state.Get(constants.ArmAsyncRGDelete).(bool) { + s.say(fmt.Sprintf("\n Not waiting for Resource Group delete as requested by user. Resource Group Name is %s", resourceGroupName)) + } else { + err = f.WaitForCompletion(ctx, s.client.GroupsClient.Client) + } } if err != nil { ui.Error(fmt.Sprintf("Error deleting resource group. Please delete it manually.\n\n"+ "Name: %s\n"+ "Error: %s", resourceGroupName, err)) + return + } + if !state.Get(constants.ArmAsyncRGDelete).(bool) { + ui.Say("Resource group has been deleted.") } - - ui.Say("Resource group has been deleted.") } } diff --git a/builder/azure/arm/step_delete_resource_group.go b/builder/azure/arm/step_delete_resource_group.go index 714f43c1b..e6445ce1d 100644 --- a/builder/azure/arm/step_delete_resource_group.go +++ b/builder/azure/arm/step_delete_resource_group.go @@ -53,7 +53,13 @@ func (s *StepDeleteResourceGroup) deleteResourceGroup(ctx context.Context, state s.say("\nThe resource group was created by Packer, deleting ...") f, err := s.client.GroupsClient.Delete(ctx, resourceGroupName) if err == nil { - f.WaitForCompletion(ctx, s.client.GroupsClient.Client) + if state.Get(constants.ArmAsyncRGDelete).(bool) { + // No need to wait for the complition for delete if request is Accepted + s.say(fmt.Sprintf("\nResource Group is being deleted, not waiting for deletion due to config. Resource Group Name '%s'", resourceGroupName)) + } else { + f.WaitForCompletion(ctx, s.client.GroupsClient.Client) + } + } if err != nil { diff --git a/builder/azure/common/constants/stateBag.go b/builder/azure/common/constants/stateBag.go index 3576f6820..a90c47e16 100644 --- a/builder/azure/common/constants/stateBag.go +++ b/builder/azure/common/constants/stateBag.go @@ -35,4 +35,5 @@ const ( ArmManagedImageResourceGroupName string = "arm.ManagedImageResourceGroupName" ArmManagedImageLocation string = "arm.ManagedImageLocation" ArmManagedImageName string = "arm.ManagedImageName" + ArmAsyncRGDelete string = "arm.AsyncRGDelete" ) diff --git a/website/source/docs/builders/azure.html.md b/website/source/docs/builders/azure.html.md index 9019e035e..23f104269 100644 --- a/website/source/docs/builders/azure.html.md +++ b/website/source/docs/builders/azure.html.md @@ -221,6 +221,10 @@ Providing `temp_resource_group_name` or `location` in combination with `build_re CLI example `azure vm sizes -l westus` +- `async_resourcegroup_delete` (boolean) If you want packer to delete the temporary resource group asynchronously. Its a boolean value + and defaults to false. **Important** Setting this true would mean that your builds are faster however this is a very + small chance that the temporary resource group is not deleted by Azure. + ## Basic Example Here is a basic example for Azure. From e1b18d594a604545e874bc416b3e194106cdf4af Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Tue, 15 May 2018 11:41:26 -0700 Subject: [PATCH 2/3] Updates based on PR feedback --- builder/azure/arm/builder.go | 2 +- builder/azure/arm/builder_test.go | 1 + builder/azure/arm/config.go | 2 +- builder/azure/arm/config_test.go | 67 +++++++++++++++++++ .../azure/arm/step_create_resource_group.go | 4 +- .../azure/arm/step_delete_resource_group.go | 2 +- builder/azure/common/constants/stateBag.go | 2 +- website/source/docs/builders/azure.html.md | 5 +- 8 files changed, 76 insertions(+), 9 deletions(-) diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index e59f0f84b..c21e95114 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -344,7 +344,7 @@ func (b *Builder) configureStateBag(stateBag multistep.StateBag) { stateBag.Put(constants.ArmIsManagedImage, b.config.isManagedImage()) stateBag.Put(constants.ArmManagedImageResourceGroupName, b.config.ManagedImageResourceGroupName) stateBag.Put(constants.ArmManagedImageName, b.config.ManagedImageName) - stateBag.Put(constants.ArmAsyncRGDelete, b.config.AsyncRGDelete) + stateBag.Put(constants.ArmAsyncResourceGroupDelete, b.config.AsyncResourceGroupDelete) } // Parameters that are only known at runtime after querying Azure. diff --git a/builder/azure/arm/builder_test.go b/builder/azure/arm/builder_test.go index 2db475a02..99855333d 100644 --- a/builder/azure/arm/builder_test.go +++ b/builder/azure/arm/builder_test.go @@ -25,6 +25,7 @@ func TestStateBagShouldBePopulatedExpectedValues(t *testing.T) { constants.ArmStorageAccountName, constants.ArmVirtualMachineCaptureParameters, constants.ArmPublicIPAddressName, + constants.ArmAsyncResourceGroupDelete, } for _, v := range expectedStateBagKeys { diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index ded561a58..2dfb69809 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -153,7 +153,7 @@ type Config struct { ctx *interpolate.Context //Cleanup - AsyncRGDelete bool `mapstructure:"async_resourcegroup_delete"` + AsyncResourceGroupDelete bool `mapstructure:"async_resourcegroup_delete"` } type keyVaultCertificate struct { diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go index 909ce2c3a..8e8bd3d68 100644 --- a/builder/azure/arm/config_test.go +++ b/builder/azure/arm/config_test.go @@ -1264,6 +1264,73 @@ func TestConfigShouldAllowTempNameOverrides(t *testing.T) { } } +func TestConfigShouldAllowAsyncResourceGroupOverride(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "os_type": "linux", + "managed_image_name": "ignore", + "managed_image_resource_group_name": "ignore", + "async_resourcegroup_delete": "true", + } + + c, _, err := newConfig(config, getPackerConfiguration()) + if err != nil { + t.Errorf("newConfig failed with %q", err) + } + + if c.AsyncResourceGroupDelete != true { + t.Errorf("expected async_resourcegroup_delete to be %q, but got %t", "async_resourcegroup_delete", c.AsyncResourceGroupDelete) + } +} +func TestConfigShouldAllowAsyncResourceGroupOverrideNoValue(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "os_type": "linux", + "managed_image_name": "ignore", + "managed_image_resource_group_name": "ignore", + } + + c, _, err := newConfig(config, getPackerConfiguration()) + if err != nil { + t.Errorf("newConfig failed with %q", err) + } + + if c.AsyncResourceGroupDelete != false { + t.Errorf("expected async_resourcegroup_delete to be %q, but got %t", "async_resourcegroup_delete", c.AsyncResourceGroupDelete) + } +} +func TestConfigShouldAllowAsyncResourceGroupOverrideBadValue(t *testing.T) { + config := map[string]interface{}{ + "image_offer": "ignore", + "image_publisher": "ignore", + "image_sku": "ignore", + "location": "ignore", + "subscription_id": "ignore", + "communicator": "none", + "os_type": "linux", + "managed_image_name": "ignore", + "managed_image_resource_group_name": "ignore", + "async_resourcegroup_delete": "asdasda", + } + + c, _, err := newConfig(config, getPackerConfiguration()) + if err != nil && c == nil { + t.Log("newConfig failed which is expected ", err) + + } + +} + func getArmBuilderConfiguration() map[string]string { m := make(map[string]string) for _, v := range requiredConfigValues { diff --git a/builder/azure/arm/step_create_resource_group.go b/builder/azure/arm/step_create_resource_group.go index 8036adb36..f835933db 100644 --- a/builder/azure/arm/step_create_resource_group.go +++ b/builder/azure/arm/step_create_resource_group.go @@ -118,7 +118,7 @@ func (s *StepCreateResourceGroup) Cleanup(state multistep.StateBag) { ctx := context.TODO() f, err := s.client.GroupsClient.Delete(ctx, resourceGroupName) if err == nil { - if state.Get(constants.ArmAsyncRGDelete).(bool) { + if state.Get(constants.ArmAsyncResourceGroupDelete).(bool) { s.say(fmt.Sprintf("\n Not waiting for Resource Group delete as requested by user. Resource Group Name is %s", resourceGroupName)) } else { err = f.WaitForCompletion(ctx, s.client.GroupsClient.Client) @@ -130,7 +130,7 @@ func (s *StepCreateResourceGroup) Cleanup(state multistep.StateBag) { "Error: %s", resourceGroupName, err)) return } - if !state.Get(constants.ArmAsyncRGDelete).(bool) { + if !state.Get(constants.ArmAsyncResourceGroupDelete).(bool) { ui.Say("Resource group has been deleted.") } } diff --git a/builder/azure/arm/step_delete_resource_group.go b/builder/azure/arm/step_delete_resource_group.go index e6445ce1d..56d2f15c5 100644 --- a/builder/azure/arm/step_delete_resource_group.go +++ b/builder/azure/arm/step_delete_resource_group.go @@ -53,7 +53,7 @@ func (s *StepDeleteResourceGroup) deleteResourceGroup(ctx context.Context, state s.say("\nThe resource group was created by Packer, deleting ...") f, err := s.client.GroupsClient.Delete(ctx, resourceGroupName) if err == nil { - if state.Get(constants.ArmAsyncRGDelete).(bool) { + if state.Get(constants.ArmAsyncResourceGroupDelete).(bool) { // No need to wait for the complition for delete if request is Accepted s.say(fmt.Sprintf("\nResource Group is being deleted, not waiting for deletion due to config. Resource Group Name '%s'", resourceGroupName)) } else { diff --git a/builder/azure/common/constants/stateBag.go b/builder/azure/common/constants/stateBag.go index a90c47e16..e1fe8a66b 100644 --- a/builder/azure/common/constants/stateBag.go +++ b/builder/azure/common/constants/stateBag.go @@ -35,5 +35,5 @@ const ( ArmManagedImageResourceGroupName string = "arm.ManagedImageResourceGroupName" ArmManagedImageLocation string = "arm.ManagedImageLocation" ArmManagedImageName string = "arm.ManagedImageName" - ArmAsyncRGDelete string = "arm.AsyncRGDelete" + ArmAsyncResourceGroupDelete string = "arm.AsyncResourceGroupDelete" ) diff --git a/website/source/docs/builders/azure.html.md b/website/source/docs/builders/azure.html.md index 23f104269..99538700e 100644 --- a/website/source/docs/builders/azure.html.md +++ b/website/source/docs/builders/azure.html.md @@ -221,9 +221,8 @@ Providing `temp_resource_group_name` or `location` in combination with `build_re CLI example `azure vm sizes -l westus` -- `async_resourcegroup_delete` (boolean) If you want packer to delete the temporary resource group asynchronously. Its a boolean value - and defaults to false. **Important** Setting this true would mean that your builds are faster however this is a very - small chance that the temporary resource group is not deleted by Azure. +- `async_resourcegroup_delete` (boolean) If you want packer to delete the temporary resource group asynchronously. It's a boolean value + and defaults to false. **Important** Setting this true means that your builds are faster, any failed deletes are not reported. ## Basic Example From 784c7973c21dee556334af170e92e345843c689f Mon Sep 17 00:00:00 2001 From: Hariharan Jayaraman Date: Tue, 15 May 2018 11:45:24 -0700 Subject: [PATCH 3/3] minor updates to docs --- website/source/docs/builders/azure.html.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/source/docs/builders/azure.html.md b/website/source/docs/builders/azure.html.md index 99538700e..e697a3e42 100644 --- a/website/source/docs/builders/azure.html.md +++ b/website/source/docs/builders/azure.html.md @@ -221,8 +221,8 @@ Providing `temp_resource_group_name` or `location` in combination with `build_re CLI example `azure vm sizes -l westus` -- `async_resourcegroup_delete` (boolean) If you want packer to delete the temporary resource group asynchronously. It's a boolean value - and defaults to false. **Important** Setting this true means that your builds are faster, any failed deletes are not reported. +- `async_resourcegroup_delete` (boolean) If you want packer to delete the temporary resource group asynchronously set this value. It's a boolean value + and defaults to false. **Important** Setting this true means that your builds are faster, however any failed deletes are not reported. ## Basic Example