diff --git a/builder/azure/arm/builder.go b/builder/azure/arm/builder.go index e6993cfb5..3874cdb05 100644 --- a/builder/azure/arm/builder.go +++ b/builder/azure/arm/builder.go @@ -287,7 +287,16 @@ func (b *Builder) configureStateBag(stateBag multistep.StateBag) { stateBag.Put(constants.ArmLocation, b.config.Location) stateBag.Put(constants.ArmNicName, DefaultNicName) stateBag.Put(constants.ArmPublicIPAddressName, DefaultPublicIPAddressName) - stateBag.Put(constants.ArmResourceGroupName, b.config.tmpResourceGroupName) + if b.config.TempResourceGroupName != "" && b.config.BuildResourceGroupName != "" { + stateBag.Put(constants.ArmDoubleResourceGroupNameSet, true) + } + if b.config.tmpResourceGroupName != "" { + stateBag.Put(constants.ArmResourceGroupName, b.config.tmpResourceGroupName) + stateBag.Put(constants.ArmIsExistingResourceGroup, false) + } else { + stateBag.Put(constants.ArmResourceGroupName, b.config.BuildResourceGroupName) + stateBag.Put(constants.ArmIsExistingResourceGroup, true) + } stateBag.Put(constants.ArmStorageAccountName, b.config.StorageAccount) stateBag.Put(constants.ArmIsManagedImage, b.config.isManagedImage()) diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index 3fc4f13b9..fd6bd9edb 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -83,6 +83,7 @@ type Config struct { StorageAccount string `mapstructure:"storage_account"` TempComputeName string `mapstructure:"temp_compute_name"` TempResourceGroupName string `mapstructure:"temp_resource_group_name"` + BuildResourceGroupName string `mapstructure:"build_resource_group_name"` storageAccountBlobEndpoint string CloudEnvironmentName string `mapstructure:"cloud_environment_name"` cloudEnvironment *azure.Environment @@ -129,7 +130,13 @@ type keyVaultCertificate struct { } func (c *Config) toVMID() string { - return fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/virtualMachines/%s", c.SubscriptionID, c.tmpResourceGroupName, c.tmpComputeName) + var resourceGroupName string + if c.tmpResourceGroupName != "" { + resourceGroupName = c.tmpResourceGroupName + } else { + resourceGroupName = c.BuildResourceGroupName + } + return fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/virtualMachines/%s", c.SubscriptionID, resourceGroupName, c.tmpComputeName) } func (c *Config) isManagedImage() bool { @@ -328,9 +335,10 @@ func setRuntimeValues(c *Config) { c.tmpComputeName = c.TempComputeName } c.tmpDeploymentName = tempName.DeploymentName - if c.TempResourceGroupName == "" { + // Only set tmpResourceGroupName if no name has been specified + if c.TempResourceGroupName == "" && c.BuildResourceGroupName == "" { c.tmpResourceGroupName = tempName.ResourceGroupName - } else { + } else if c.TempResourceGroupName != "" && c.BuildResourceGroupName == "" { c.tmpResourceGroupName = c.TempResourceGroupName } c.tmpOSDiskName = tempName.OSDiskName diff --git a/builder/azure/arm/step_create_resource_group.go b/builder/azure/arm/step_create_resource_group.go index 745748669..934cb3225 100644 --- a/builder/azure/arm/step_create_resource_group.go +++ b/builder/azure/arm/step_create_resource_group.go @@ -1,6 +1,7 @@ package arm import ( + "errors" "fmt" "github.com/Azure/azure-sdk-for-go/arm/resources/resources" @@ -51,32 +52,48 @@ func (s *StepCreateResourceGroup) doesResourceGroupExist(resourceGroupName strin } func (s *StepCreateResourceGroup) Run(state multistep.StateBag) multistep.StepAction { - s.say("Creating resource group ...") + var doubleResource, ok = state.GetOk(constants.ArmDoubleResourceGroupNameSet) + if ok && doubleResource.(bool) { + err := errors.New("You have filled in both temp_resource_group_name and build_resource_group_name. Please choose one.") + return processStepResult(err, s.error, state) + } var resourceGroupName = state.Get(constants.ArmResourceGroupName).(string) var location = state.Get(constants.ArmLocation).(string) var tags = state.Get(constants.ArmTags).(*map[string]*string) - s.say(fmt.Sprintf(" -> ResourceGroupName : '%s'", resourceGroupName)) - s.say(fmt.Sprintf(" -> Location : '%s'", location)) - s.say(fmt.Sprintf(" -> Tags :")) - for k, v := range *tags { - s.say(fmt.Sprintf(" ->> %s : %s", k, *v)) - } - exists, err := s.exists(resourceGroupName) if err != nil { - s.say(s.client.LastError.Error()) + return processStepResult(err, s.error, state) } - state.Put(constants.ArmIsExistingResourceGroup, exists) + configThinksExists := state.Get(constants.ArmIsExistingResourceGroup).(bool) + if exists != configThinksExists { + if configThinksExists { + err = errors.New("The resource group you want to use does not exist yet. Please use temp_resource_group_name to create a temporary resource group.") + } else { + err = errors.New("A resource group with that name already exists. Please use build_resource_group_name to use an existing resource group.") + } + return processStepResult(err, s.error, state) + } + // If the resource group exists, we may not have permissions to update it so we don't. if !exists { + s.say("Creating resource group ...") + + s.say(fmt.Sprintf(" -> ResourceGroupName : '%s'", resourceGroupName)) + s.say(fmt.Sprintf(" -> Location : '%s'", location)) + s.say(fmt.Sprintf(" -> Tags :")) + for k, v := range *tags { + s.say(fmt.Sprintf(" ->> %s : %s", k, *v)) + } err = s.create(resourceGroupName, location, tags) if err == nil { state.Put(constants.ArmIsResourceGroupCreated, true) } } else { - // Mark the resource group as created to deal with later checks + s.say("Using existing resource group ...") + s.say(fmt.Sprintf(" -> ResourceGroupName : '%s'", resourceGroupName)) + s.say(fmt.Sprintf(" -> Location : '%s'", location)) state.Put(constants.ArmIsResourceGroupCreated, true) } diff --git a/builder/azure/arm/step_create_resource_group_test.go b/builder/azure/arm/step_create_resource_group_test.go index 47a4a01e6..afd47cfef 100644 --- a/builder/azure/arm/step_create_resource_group_test.go +++ b/builder/azure/arm/step_create_resource_group_test.go @@ -1,6 +1,7 @@ package arm import ( + "errors" "fmt" "testing" @@ -8,6 +9,33 @@ import ( "github.com/mitchellh/multistep" ) +func TestStepCreateResourceGroupShouldFailIfBothGroupNames(t *testing.T) { + stateBag := new(multistep.BasicStateBag) + + stateBag.Put(constants.ArmDoubleResourceGroupNameSet, true) + + value := "Unit Test: Tags" + tags := map[string]*string{ + "tag01": &value, + } + + stateBag.Put(constants.ArmTags, &tags) + var testSubject = &StepCreateResourceGroup{ + create: func(string, string, *map[string]*string) error { return nil }, + say: func(message string) {}, + error: func(e error) {}, + exists: func(string) (bool, error) { return false, nil }, + } + var result = testSubject.Run(stateBag) + if result != multistep.ActionHalt { + t.Fatalf("Expected the step to return 'ActionHalt', but got '%d'.", result) + } + + if _, ok := stateBag.GetOk(constants.Error); ok == false { + t.Fatalf("Expected the step to set stateBag['%s'], but it was not.", constants.Error) + } +} + func TestStepCreateResourceGroupShouldFailIfCreateFails(t *testing.T) { var testSubject = &StepCreateResourceGroup{ create: func(string, string, *map[string]*string) error { return fmt.Errorf("!! Unit Test FAIL !!") }, @@ -28,6 +56,26 @@ func TestStepCreateResourceGroupShouldFailIfCreateFails(t *testing.T) { } } +func TestStepCreateResourceGroupShouldFailIfExistsFails(t *testing.T) { + var testSubject = &StepCreateResourceGroup{ + create: func(string, string, *map[string]*string) error { return nil }, + say: func(message string) {}, + error: func(e error) {}, + exists: func(string) (bool, error) { return false, errors.New("FAIL") }, + } + + stateBag := createTestStateBagStepCreateResourceGroup() + + var result = testSubject.Run(stateBag) + if result != multistep.ActionHalt { + t.Fatalf("Expected the step to return 'ActionHalt', but got '%d'.", result) + } + + if _, ok := stateBag.GetOk(constants.Error); ok == false { + t.Fatalf("Expected the step to set stateBag['%s'], but it was not.", constants.Error) + } +} + func TestStepCreateResourceGroupShouldPassIfCreatePasses(t *testing.T) { var testSubject = &StepCreateResourceGroup{ create: func(string, string, *map[string]*string) error { return nil }, @@ -94,7 +142,7 @@ func TestStepCreateResourceGroupShouldTakeStepArgumentsFromStateBag(t *testing.T } } -func TestStepCreateResourceGroupMarkAsNonExistingIfDoesntExist(t *testing.T) { +func TestStepCreateResourceGroupMarkShouldFailIfTryingExistingButDoesntExist(t *testing.T) { var testSubject = &StepCreateResourceGroup{ create: func(string, string, *map[string]*string) error { return fmt.Errorf("!! Unit Test FAIL !!") }, say: func(message string) {}, @@ -102,18 +150,19 @@ func TestStepCreateResourceGroupMarkAsNonExistingIfDoesntExist(t *testing.T) { exists: func(string) (bool, error) { return false, nil }, } - stateBag := createTestStateBagStepCreateResourceGroup() + stateBag := createTestExistingStateBagStepCreateResourceGroup() - if _, ok := stateBag.GetOk(constants.ArmIsExistingResourceGroup); ok == true { - t.Fatalf("Expected 'constants.ArmIsExistingResourceGroup' not to be set, but it was") + var result = testSubject.Run(stateBag) + if result != multistep.ActionHalt { + t.Fatalf("Expected the step to return 'ActionHalt', but got '%d'.", result) } - testSubject.Run(stateBag) - if stateBag.Get(constants.ArmIsExistingResourceGroup).(bool) == true { - t.Fatalf("Expected 'constants.ArmIsExistingResourceGroup' to be set to false, but it wasn't.") + + if _, ok := stateBag.GetOk(constants.Error); ok == false { + t.Fatalf("Expected the step to set stateBag['%s'], but it was not.", constants.Error) } } -func TestStepCreateResourceGroupMarkAsExistingIfExists(t *testing.T) { +func TestStepCreateResourceGroupMarkShouldFailIfTryingTempButExist(t *testing.T) { var testSubject = &StepCreateResourceGroup{ create: func(string, string, *map[string]*string) error { return fmt.Errorf("!! Unit Test FAIL !!") }, say: func(message string) {}, @@ -123,12 +172,13 @@ func TestStepCreateResourceGroupMarkAsExistingIfExists(t *testing.T) { stateBag := createTestStateBagStepCreateResourceGroup() - if _, ok := stateBag.GetOk(constants.ArmIsExistingResourceGroup); ok == true { - t.Fatalf("Expected 'constants.ArmIsExistingResourceGroup' not to be set, but it was") + var result = testSubject.Run(stateBag) + if result != multistep.ActionHalt { + t.Fatalf("Expected the step to return 'ActionHalt', but got '%d'.", result) } - testSubject.Run(stateBag) - if stateBag.Get(constants.ArmIsExistingResourceGroup).(bool) == false { - t.Fatalf("Expected 'constants.ArmIsExistingResourceGroup' to be set to true, but it wasn't.") + + if _, ok := stateBag.GetOk(constants.Error); ok == false { + t.Fatalf("Expected the step to set stateBag['%s'], but it was not.", constants.Error) } } @@ -137,6 +187,23 @@ func createTestStateBagStepCreateResourceGroup() multistep.StateBag { stateBag.Put(constants.ArmLocation, "Unit Test: Location") stateBag.Put(constants.ArmResourceGroupName, "Unit Test: ResourceGroupName") + stateBag.Put(constants.ArmIsExistingResourceGroup, false) + + value := "Unit Test: Tags" + tags := map[string]*string{ + "tag01": &value, + } + + stateBag.Put(constants.ArmTags, &tags) + return stateBag +} + +func createTestExistingStateBagStepCreateResourceGroup() multistep.StateBag { + stateBag := new(multistep.BasicStateBag) + + stateBag.Put(constants.ArmLocation, "Unit Test: Location") + stateBag.Put(constants.ArmResourceGroupName, "Unit Test: ResourceGroupName") + stateBag.Put(constants.ArmIsExistingResourceGroup, true) value := "Unit Test: Tags" tags := map[string]*string{ diff --git a/builder/azure/arm/step_delete_os_disk.go b/builder/azure/arm/step_delete_os_disk.go index 6c56ac96d..af5e2fa9b 100644 --- a/builder/azure/arm/step_delete_os_disk.go +++ b/builder/azure/arm/step_delete_os_disk.go @@ -42,7 +42,6 @@ func (s *StepDeleteOSDisk) deleteBlob(storageContainerName string, blobName stri } func (s *StepDeleteOSDisk) deleteManagedDisk(resourceGroupName string, imageName string) error { - s.say("In the deleting part") xs := strings.Split(imageName, "/") diskName := xs[len(xs)-1] _, errChan := s.client.DisksClient.Delete(resourceGroupName, diskName, nil) @@ -70,15 +69,14 @@ func (s *StepDeleteOSDisk) Run(state multistep.StateBag) multistep.StepAction { err = s.deleteManaged(resourceGroupName, osDisk) if err != nil { s.say("Failed to delete the managed OS Disk!") - return multistep.ActionHalt + return processStepResult(err, s.error, state) } - s.say("After deleting") return multistep.ActionContinue } u, err := url.Parse(osDisk) if err != nil { s.say("Failed to parse the OS Disk's VHD URI!") - return multistep.ActionHalt + return processStepResult(err, s.error, state) } xs := strings.Split(u.Path, "/") diff --git a/builder/azure/arm/step_delete_os_disk_test.go b/builder/azure/arm/step_delete_os_disk_test.go index e19f0e0ba..7fdd7468e 100644 --- a/builder/azure/arm/step_delete_os_disk_test.go +++ b/builder/azure/arm/step_delete_os_disk_test.go @@ -1,6 +1,7 @@ package arm import ( + "errors" "fmt" "testing" @@ -103,6 +104,77 @@ func TestStepDeleteOSDiskShouldHandleComplexStorageContainerNames(t *testing.T) } } +func TestStepDeleteOSDiskShouldPassIfManagedDiskInTempResourceGroup(t *testing.T) { + var testSubject = &StepDeleteOSDisk{ + delete: func(string, string) error { return nil }, + say: func(message string) {}, + error: func(e error) {}, + } + + stateBag := new(multistep.BasicStateBag) + stateBag.Put(constants.ArmOSDiskVhd, "subscriptions/123-456-789/resourceGroups/existingresourcegroup/providers/Microsoft.Compute/disks/osdisk") + stateBag.Put(constants.ArmIsManagedImage, true) + stateBag.Put(constants.ArmIsExistingResourceGroup, false) + stateBag.Put(constants.ArmResourceGroupName, "testgroup") + + var result = testSubject.Run(stateBag) + if result != multistep.ActionContinue { + t.Fatalf("Expected the step to return 'ActionContinue', but got '%d'.", result) + } + + if _, ok := stateBag.GetOk(constants.Error); ok == true { + t.Fatalf("Expected the step to not set stateBag['%s'], but it was.", constants.Error) + } +} + +func TestStepDeleteOSDiskShouldFailIfManagedDiskInExistingResourceGroupFailsToDelete(t *testing.T) { + var testSubject = &StepDeleteOSDisk{ + delete: func(string, string) error { return nil }, + say: func(message string) {}, + error: func(e error) {}, + deleteManaged: func(string, string) error { return errors.New("UNIT TEST FAIL!") }, + } + + stateBag := new(multistep.BasicStateBag) + stateBag.Put(constants.ArmOSDiskVhd, "subscriptions/123-456-789/resourceGroups/existingresourcegroup/providers/Microsoft.Compute/disks/osdisk") + stateBag.Put(constants.ArmIsManagedImage, true) + stateBag.Put(constants.ArmIsExistingResourceGroup, true) + stateBag.Put(constants.ArmResourceGroupName, "testgroup") + + var result = testSubject.Run(stateBag) + if result != multistep.ActionHalt { + t.Fatalf("Expected the step to return 'ActionHalt', but got '%d'.", result) + } + + if _, ok := stateBag.GetOk(constants.Error); ok == false { + t.Fatalf("Expected the step to not stateBag['%s'], but it was.", constants.Error) + } +} + +func TestStepDeleteOSDiskShouldFailIfManagedDiskInExistingResourceGroupIsDeleted(t *testing.T) { + var testSubject = &StepDeleteOSDisk{ + delete: func(string, string) error { return nil }, + say: func(message string) {}, + error: func(e error) {}, + deleteManaged: func(string, string) error { return nil }, + } + + stateBag := new(multistep.BasicStateBag) + stateBag.Put(constants.ArmOSDiskVhd, "subscriptions/123-456-789/resourceGroups/existingresourcegroup/providers/Microsoft.Compute/disks/osdisk") + stateBag.Put(constants.ArmIsManagedImage, true) + stateBag.Put(constants.ArmIsExistingResourceGroup, true) + stateBag.Put(constants.ArmResourceGroupName, "testgroup") + + var result = testSubject.Run(stateBag) + if result != multistep.ActionContinue { + t.Fatalf("Expected the step to return 'ActionContinue', but got '%d'.", result) + } + + if _, ok := stateBag.GetOk(constants.Error); ok == true { + t.Fatalf("Expected the step to not set stateBag['%s'], but it was.", constants.Error) + } +} + func DeleteTestStateBagStepDeleteOSDisk(osDiskVhd string) multistep.StateBag { stateBag := new(multistep.BasicStateBag) stateBag.Put(constants.ArmOSDiskVhd, osDiskVhd) diff --git a/builder/azure/common/constants/stateBag.go b/builder/azure/common/constants/stateBag.go index 1df94a8f0..6d5a93068 100644 --- a/builder/azure/common/constants/stateBag.go +++ b/builder/azure/common/constants/stateBag.go @@ -23,6 +23,7 @@ const ( ArmPublicIPAddressName string = "arm.PublicIPAddressName" ArmResourceGroupName string = "arm.ResourceGroupName" ArmIsResourceGroupCreated string = "arm.IsResourceGroupCreated" + ArmDoubleResourceGroupNameSet string = "arm.DoubleResourceGroupNameSet" ArmStorageAccountName string = "arm.StorageAccountName" ArmTags string = "arm.Tags" ArmVirtualMachineCaptureParameters string = "arm.VirtualMachineCaptureParameters"