Skip to content

Commit

Permalink
r/dedicated_host: fixing comments from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
tombuildsstuff committed Jan 24, 2020
1 parent 4cf7757 commit 6ba7bf8
Show file tree
Hide file tree
Showing 10 changed files with 370 additions and 109 deletions.
12 changes: 6 additions & 6 deletions azurerm/internal/services/compute/data_source_dedicated_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ func dataSourceArmDedicatedHost() *schema.Resource {
ValidateFunc: validateDedicatedHostName(),
},

"location": azure.SchemaLocationForDataSource(),

"resource_group_name": azure.SchemaResourceGroupNameForDataSource(),

"host_group_name": {
"dedicated_host_group_name": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validateDedicatedHostGroupName(),
},

"resource_group_name": azure.SchemaResourceGroupNameForDataSource(),

"location": azure.SchemaLocationForDataSource(),

"tags": tags.SchemaDataSource(),
},
}
Expand Down Expand Up @@ -66,7 +66,7 @@ func dataSourceArmDedicatedHostRead(d *schema.ResourceData, meta interface{}) er
if location := resp.Location; location != nil {
d.Set("location", azure.NormalizeLocation(*location))
}
d.Set("host_group_name", hostGroupName)
d.Set("dedicated_host_group_name", hostGroupName)

return tags.FlattenAndSet(d, resp.Tags)
}
33 changes: 33 additions & 0 deletions azurerm/internal/services/compute/parse/dedicated_host_group.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package parse

import (
"fmt"

"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
)

type DedicatedHostGroupId struct {
ResourceGroup string
Name string
}

func DedicatedHostGroupID(input string) (*DedicatedHostGroupId, error) {
id, err := azure.ParseAzureResourceID(input)
if err != nil {
return nil, fmt.Errorf("[ERROR] Unable to parse Dedicated Host Group ID %q: %+v", input, err)
}

server := DedicatedHostGroupId{
ResourceGroup: id.ResourceGroup,
}

if server.Name, err = id.PopSegment("hostGroups"); err != nil {
return nil, err
}

if err := id.ValidateNoEmptySegments(input); err != nil {
return nil, err
}

return &server, nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package parse

import (
"testing"
)

func TestDedicatedHostGroupID(t *testing.T) {
testData := []struct {
Name string
Input string
Error bool
Expect *DedicatedHostGroupId
}{
{
Name: "Empty",
Input: "",
Error: true,
},
{
Name: "No Resource Groups Segment",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000",
Error: true,
},
{
Name: "No Resource Groups Value",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/",
Error: true,
},
{
Name: "Resource Group ID",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/foo/",
Error: true,
},
{
Name: "Missing Host Group Value",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/resGroup1/providers/Microsoft.Compute/hostGroups/",
Error: true,
},
{
Name: "Host Group ID",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/resGroup1/providers/Microsoft.Compute/hostGroups/group1",
Error: false,
Expect: &DedicatedHostGroupId{
ResourceGroup: "resGroup1",
Name: "group1",
},
},
{
Name: "Wrong Casing",
Input: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/resGroup1/providers/Microsoft.Compute/HostGroups/group1",
Error: true,
},
}

for _, v := range testData {
t.Logf("[DEBUG] Testing %q", v.Name)

actual, err := DedicatedHostGroupID(v.Input)
if err != nil {
if v.Error {
continue
}

t.Fatalf("Expected a value but got an error: %s", err)
}

if actual.Name != v.Expect.Name {
t.Fatalf("Expected %q but got %q for Name", v.Expect.Name, actual.Name)
}

if actual.ResourceGroup != v.Expect.ResourceGroup {
t.Fatalf("Expected %q but got %q for Resource Group", v.Expect.ResourceGroup, actual.ResourceGroup)
}
}
}
67 changes: 44 additions & 23 deletions azurerm/internal/services/compute/resource_arm_dedicated_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/compute/validate"

"github.com/hashicorp/go-azure-helpers/response"

Expand Down Expand Up @@ -54,13 +55,11 @@ func resourceArmDedicatedHost() *schema.Resource {

"location": azure.SchemaLocation(),

"resource_group_name": azure.SchemaResourceGroupName(),

"host_group_name": {
"dedicated_host_group_id": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validateDedicatedHostGroupName(),
ValidateFunc: validate.DedicatedHostGroupID,
},

"sku_name": {
Expand Down Expand Up @@ -108,8 +107,13 @@ func resourceArmDedicatedHostCreate(d *schema.ResourceData, meta interface{}) er
defer cancel()

name := d.Get("name").(string)
resourceGroupName := d.Get("resource_group_name").(string)
hostGroupName := d.Get("host_group_name").(string)
dedicatedHostGroupId, err := parse.DedicatedHostGroupID(d.Get("dedicated_host_group_id").(string))
if err != nil {
return err
}

resourceGroupName := dedicatedHostGroupId.ResourceGroup
hostGroupName := dedicatedHostGroupId.Name

if features.ShouldResourcesBeImported() && d.IsNewResource() {
existing, err := client.Get(ctx, resourceGroupName, hostGroupName, name, "")
Expand All @@ -128,15 +132,13 @@ func resourceArmDedicatedHostCreate(d *schema.ResourceData, meta interface{}) er
DedicatedHostProperties: &compute.DedicatedHostProperties{
AutoReplaceOnFailure: utils.Bool(d.Get("auto_replace_on_failure").(bool)),
LicenseType: compute.DedicatedHostLicenseTypes(d.Get("license_type").(string)),
PlatformFaultDomain: utils.Int32(int32(d.Get("platform_fault_domain").(int))),
},
Sku: &compute.Sku{
Name: utils.String(d.Get("sku_name").(string)),
},
Tags: tags.Expand(d.Get("tags").(map[string]interface{})),
}
if platformFaultDomain, ok := d.GetOk("platform_fault_domain"); ok {
parameters.DedicatedHostProperties.PlatformFaultDomain = utils.Int32(int32(platformFaultDomain.(int)))
}

future, err := client.CreateOrUpdate(ctx, resourceGroupName, hostGroupName, name, parameters)
if err != nil {
Expand All @@ -151,15 +153,16 @@ func resourceArmDedicatedHostCreate(d *schema.ResourceData, meta interface{}) er
return fmt.Errorf("Error retrieving Dedicated Host %q (Host Group Name %q / Resource Group %q): %+v", name, hostGroupName, resourceGroupName, err)
}
if resp.ID == nil {
return fmt.Errorf("Cannot read Dedicated Host %q (Host Group Name %q / Resource Group %q) ID", name, hostGroupName, resourceGroupName)
return fmt.Errorf("Cannot read ID for Dedicated Host %q (Host Group Name %q / Resource Group %q)", name, hostGroupName, resourceGroupName)
}
d.SetId(*resp.ID)

return resourceArmDedicatedHostRead(d, meta)
}

func resourceArmDedicatedHostRead(d *schema.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).Compute.DedicatedHostsClient
groupsClient := meta.(*clients.Client).Compute.DedicatedHostGroupsClient
hostsClient := meta.(*clients.Client).Compute.DedicatedHostsClient
ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
defer cancel()

Expand All @@ -168,27 +171,44 @@ func resourceArmDedicatedHostRead(d *schema.ResourceData, meta interface{}) erro
return err
}

resp, err := client.Get(ctx, id.ResourceGroup, id.HostGroup, id.Name, "")
group, err := groupsClient.Get(ctx, id.ResourceGroup, id.HostGroup)
if err != nil {
if utils.ResponseWasNotFound(group.Response) {
log.Printf("[INFO] Parent Dedicated Host Group %q does not exist - removing from state", d.Id())
d.SetId("")
return nil
}

return fmt.Errorf("Error retrieving Dedicated Host Group %q (Resource Group %q): %+v", id.HostGroup, id.ResourceGroup, err)
}

resp, err := hostsClient.Get(ctx, id.ResourceGroup, id.HostGroup, id.Name, "")
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
log.Printf("[INFO] Dedicated Host %q does not exist - removing from state", d.Id())
d.SetId("")
return nil
}
return fmt.Errorf("Error reading Dedicated Host %q (Host Group Name %q / Resource Group %q): %+v", id.Name, id.HostGroup, id.ResourceGroup, err)

return fmt.Errorf("Error retrieving Dedicated Host %q (Host Group Name %q / Resource Group %q): %+v", id.Name, id.HostGroup, id.ResourceGroup, err)
}

d.Set("name", resp.Name)
d.Set("resource_group_name", id.ResourceGroup)
d.Set("dedicated_host_group_id", group.ID)

if location := resp.Location; location != nil {
d.Set("location", azure.NormalizeLocation(*location))
}
d.Set("host_group_name", id.HostGroup)
d.Set("sku_name", resp.Sku.Name)
if props := resp.DedicatedHostProperties; props != nil {
d.Set("platform_fault_domain", props.PlatformFaultDomain)
d.Set("auto_replace_on_failure", props.AutoReplaceOnFailure)
d.Set("license_type", props.LicenseType)

platformFaultDomain := 0
if props.PlatformFaultDomain != nil {
platformFaultDomain = int(*props.PlatformFaultDomain)
}
d.Set("platform_fault_domain", platformFaultDomain)
}

return tags.FlattenAndSet(d, resp.Tags)
Expand All @@ -199,9 +219,10 @@ func resourceArmDedicatedHostUpdate(d *schema.ResourceData, meta interface{}) er
ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d)
defer cancel()

name := d.Get("name").(string)
resourceGroupName := d.Get("resource_group_name").(string)
hostGroupName := d.Get("host_group_name").(string)
id, err := parse.DedicatedHostID(d.Id())
if err != nil {
return err
}

parameters := compute.DedicatedHostUpdate{
DedicatedHostProperties: &compute.DedicatedHostProperties{
Expand All @@ -211,12 +232,12 @@ func resourceArmDedicatedHostUpdate(d *schema.ResourceData, meta interface{}) er
Tags: tags.Expand(d.Get("tags").(map[string]interface{})),
}

future, err := client.Update(ctx, resourceGroupName, hostGroupName, name, parameters)
future, err := client.Update(ctx, id.ResourceGroup, id.HostGroup, id.Name, parameters)
if err != nil {
return fmt.Errorf("Error updating Dedicated Host %q (Host Group Name %q / Resource Group %q): %+v", name, hostGroupName, resourceGroupName, err)
return fmt.Errorf("Error updating Dedicated Host %q (Host Group Name %q / Resource Group %q): %+v", id.Name, id.HostGroup, id.ResourceGroup, err)
}
if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
return fmt.Errorf("Error waiting for update of Dedicated Host %q (Host Group Name %q / Resource Group %q): %+v", name, hostGroupName, resourceGroupName, err)
return fmt.Errorf("Error waiting for update of Dedicated Host %q (Host Group Name %q / Resource Group %q): %+v", id.Name, id.HostGroup, id.ResourceGroup, err)
}

return resourceArmDedicatedHostRead(d, meta)
Expand Down Expand Up @@ -273,7 +294,7 @@ func dedicatedHostDeletedRefreshFunc(ctx context.Context, client *compute.Dedica
if utils.ResponseWasNotFound(res.Response) {
return "NotFound", "NotFound", nil
}
return nil, "", fmt.Errorf("Error issuing read request in dedicatedHostDeletedRefreshFunc: %+v", err)
return nil, "", fmt.Errorf("Error polling to check if the Host Group has been deleted: %+v", err)
}

return res, "Exists", nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ func TestAccDataSourceAzureRMDedicatedHost_basic(t *testing.T) {
{
Config: testAccDataSourceDedicatedHost_basic(data),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrSet(data.ResourceName, "id"),
resource.TestCheckResourceAttrSet(data.ResourceName, "location"),
resource.TestCheckResourceAttrSet(data.ResourceName, "tags.%"),
),
},
},
Expand All @@ -32,9 +33,9 @@ func testAccDataSourceDedicatedHost_basic(data acceptance.TestData) string {
%s
data "azurerm_dedicated_host" "test" {
name = azurerm_dedicated_host.test.name
resource_group_name = azurerm_dedicated_host.test.resource_group_name
host_group_name = azurerm_dedicated_host.test.host_group_name
name = azurerm_dedicated_host.test.name
dedicated_host_group_name = azurerm_dedicated_host_group.test.name
resource_group_name = azurerm_dedicated_host.test.resource_group_name
}
`, config)
}
Loading

0 comments on commit 6ba7bf8

Please sign in to comment.