From d40e115ad783bfc2b5322dbb510e6dc0ebe31644 Mon Sep 17 00:00:00 2001 From: Christopher Boumenot Date: Mon, 9 May 2016 14:19:55 -0700 Subject: [PATCH] Only cleanup if necessary. (#3517) Do not delete the resource group as part of cleanup unless it exists. --- .../azure/arm/step_create_resource_group.go | 6 +++--- builder/azure/arm/step_delete_os_disk.go | 2 +- .../azure/arm/step_delete_resource_group.go | 7 +++++-- .../arm/step_delete_resource_group_test.go | 21 ++++++++----------- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/builder/azure/arm/step_create_resource_group.go b/builder/azure/arm/step_create_resource_group.go index cab2c7eb7..4d32c4091 100644 --- a/builder/azure/arm/step_create_resource_group.go +++ b/builder/azure/arm/step_create_resource_group.go @@ -7,8 +7,8 @@ import ( "fmt" "github.com/Azure/azure-sdk-for-go/arm/resources/resources" - "github.com/mitchellh/packer/builder/azure/common/constants" "github.com/mitchellh/multistep" + "github.com/mitchellh/packer/builder/azure/common/constants" "github.com/mitchellh/packer/packer" ) @@ -56,8 +56,8 @@ func (s *StepCreateResourceGroup) Run(state multistep.StateBag) multistep.StepAc } func (s *StepCreateResourceGroup) Cleanup(state multistep.StateBag) { - _, ok := state.GetOk(constants.ArmIsResourceGroupCreated) - if !ok { + isCreated, ok := state.GetOk(constants.ArmIsResourceGroupCreated) + if !ok || !isCreated.(bool) { return } diff --git a/builder/azure/arm/step_delete_os_disk.go b/builder/azure/arm/step_delete_os_disk.go index 2b7b59207..ba079cdbe 100644 --- a/builder/azure/arm/step_delete_os_disk.go +++ b/builder/azure/arm/step_delete_os_disk.go @@ -40,7 +40,7 @@ func (s *StepDeleteOSDisk) Run(state multistep.StateBag) multistep.StepAction { s.say("Deleting the temporary OS disk ...") var osDisk = state.Get(constants.ArmOSDiskVhd).(string) - s.say(fmt.Sprintf(" -> OS Disk : '%s'", osDisk)) + s.say(fmt.Sprintf(" -> OS Disk : '%s'", osDisk)) u, err := url.Parse(osDisk) if err != nil { diff --git a/builder/azure/arm/step_delete_resource_group.go b/builder/azure/arm/step_delete_resource_group.go index f74dedd3f..82b9adfb0 100644 --- a/builder/azure/arm/step_delete_resource_group.go +++ b/builder/azure/arm/step_delete_resource_group.go @@ -6,9 +6,9 @@ package arm import ( "fmt" + "github.com/mitchellh/multistep" "github.com/mitchellh/packer/builder/azure/common" "github.com/mitchellh/packer/builder/azure/common/constants" - "github.com/mitchellh/multistep" "github.com/mitchellh/packer/packer" ) @@ -46,7 +46,10 @@ func (s *StepDeleteResourceGroup) Run(state multistep.StateBag) multistep.StepAc func() bool { return common.IsStateCancelled(state) }, func(cancelCh <-chan struct{}) error { return s.delete(resourceGroupName, cancelCh) }) - return processInterruptibleResult(result, s.error, state) + stepAction := processInterruptibleResult(result, s.error, state) + state.Put(constants.ArmIsResourceGroupCreated, false) + + return stepAction } func (*StepDeleteResourceGroup) Cleanup(multistep.StateBag) { diff --git a/builder/azure/arm/step_delete_resource_group_test.go b/builder/azure/arm/step_delete_resource_group_test.go index 9398f9d30..bd6bf79cc 100644 --- a/builder/azure/arm/step_delete_resource_group_test.go +++ b/builder/azure/arm/step_delete_resource_group_test.go @@ -7,8 +7,8 @@ import ( "fmt" "testing" - "github.com/mitchellh/packer/builder/azure/common/constants" "github.com/mitchellh/multistep" + "github.com/mitchellh/packer/builder/azure/common/constants" ) func TestStepDeleteResourceGroupShouldFailIfDeleteFails(t *testing.T) { @@ -49,12 +49,9 @@ func TestStepDeleteResourceGroupShouldPassIfDeletePasses(t *testing.T) { } } -func TestStepDeleteResourceGroupShouldTakeStepArgumentsFromStateBag(t *testing.T) { - var actualResourceGroupName string - +func TestStepDeleteResourceGroupShouldDeleteStateBagArmResourceGroupCreated(t *testing.T) { var testSubject = &StepDeleteResourceGroup{ delete: func(resourceGroupName string, cancelCh <-chan struct{}) error { - actualResourceGroupName = resourceGroupName return nil }, say: func(message string) {}, @@ -62,22 +59,22 @@ func TestStepDeleteResourceGroupShouldTakeStepArgumentsFromStateBag(t *testing.T } stateBag := DeleteTestStateBagStepDeleteResourceGroup() - var result = testSubject.Run(stateBag) + testSubject.Run(stateBag) - if result != multistep.ActionContinue { - t.Fatalf("Expected the step to return 'ActionContinue', but got '%d'.", result) + value, ok := stateBag.GetOk(constants.ArmIsResourceGroupCreated) + if !ok { + t.Fatalf("Expected the resource bag value arm.IsResourceGroupCreated to exist") } - var expectedResourceGroupName = stateBag.Get(constants.ArmResourceGroupName).(string) - - if actualResourceGroupName != expectedResourceGroupName { - t.Fatalf("Expected the step to source 'constants.ArmResourceGroupName' from the state bag, but it did not.") + if value.(bool) { + t.Fatalf("Expected arm.IsResourceGroupCreated to be false, but got %q", value) } } func DeleteTestStateBagStepDeleteResourceGroup() multistep.StateBag { stateBag := new(multistep.BasicStateBag) stateBag.Put(constants.ArmResourceGroupName, "Unit Test: ResourceGroupName") + stateBag.Put(constants.ArmIsResourceGroupCreated, "Unit Test: IsResourceGroupCreated") return stateBag }