Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New resources azurerm_capacity_reservation_group and azurerm_capacity_reservation #16464

Merged
merged 6 commits into from Jun 8, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
169 changes: 169 additions & 0 deletions internal/services/compute/capacity_reservation_group_resource.go
@@ -0,0 +1,169 @@
package compute

import (
"fmt"
"log"
"time"

"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-11-01/compute"
"github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema"
"github.com/hashicorp/go-azure-helpers/resourcemanager/location"
"github.com/hashicorp/go-azure-helpers/resourcemanager/zones"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-provider-azurerm/helpers/azure"
"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/compute/parse"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/compute/validate"
"github.com/hashicorp/terraform-provider-azurerm/internal/tags"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/timeouts"
"github.com/hashicorp/terraform-provider-azurerm/utils"
)

func resourceCapacityReservationGroup() *pluginsdk.Resource {
return &pluginsdk.Resource{
Create: resourceCapacityReservationGroupCreate,
Read: resourceCapacityReservationGroupRead,
Update: resourceCapacityReservationGroupUpdate,
Delete: resourceCapacityReservationGroupDelete,

Timeouts: &pluginsdk.ResourceTimeout{
Create: pluginsdk.DefaultTimeout(30 * time.Minute),
Read: pluginsdk.DefaultTimeout(5 * time.Minute),
Update: pluginsdk.DefaultTimeout(30 * time.Minute),
Delete: pluginsdk.DefaultTimeout(30 * time.Minute),
},

Importer: pluginsdk.ImporterValidatingResourceId(func(id string) error {
_, err := parse.CapacityReservationGroupID(id)
return err
}),

Schema: map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validate.CapacityReservationGroupName(),
},

"resource_group_name": azure.SchemaResourceGroupName(),

"location": azure.SchemaLocation(),

"zones": commonschema.ZonesMultipleOptionalForceNew(),

"tags": tags.Schema(),
},
}
}

func resourceCapacityReservationGroupCreate(d *pluginsdk.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).Compute.CapacityReservationGroupsClient
subscriptionId := meta.(*clients.Client).Account.SubscriptionId
ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d)
defer cancel()

name := d.Get("name").(string)
resourceGroup := d.Get("resource_group_name").(string)
id := parse.NewCapacityReservationGroupID(subscriptionId, resourceGroup, name)
existing, err := client.Get(ctx, id.ResourceGroup, id.Name, "")
if err != nil {
if !utils.ResponseWasNotFound(existing.Response) {
return fmt.Errorf("checking for existing %s: %+v", id, err)
}
}
if !utils.ResponseWasNotFound(existing.Response) {
return tf.ImportAsExistsError("azurerm_capacity_reservation_group", id.ID())
}

parameters := compute.CapacityReservationGroup{
Name: utils.String(name),
Location: utils.String(location.Normalize(d.Get("location").(string))),
Tags: tags.Expand(d.Get("tags").(map[string]interface{})),
}

zones := zones.Expand(d.Get("zones").(*schema.Set).List())
if len(zones) > 0 {
parameters.Zones = &zones
}
Comment on lines +88 to +91
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be using the standard Expand functions for Zones

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

different from the other resource in this change, in line 87 (before the new commit), already used the standard zones.Expand


if _, err := client.CreateOrUpdate(ctx, resourceGroup, name, parameters); err != nil {
return fmt.Errorf("creating %s: %+v", id, err)
}

d.SetId(id.ID())
return resourceCapacityReservationGroupRead(d, meta)
}

func resourceCapacityReservationGroupRead(d *pluginsdk.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).Compute.CapacityReservationGroupsClient
ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
defer cancel()

id, err := parse.CapacityReservationGroupID(d.Id())
if err != nil {
return err
}

resp, err := client.Get(ctx, id.ResourceGroup, id.Name, "")
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
log.Printf("[INFO] %s was not found - removing from state", *id)
d.SetId("")
return nil
}
return fmt.Errorf("retrieving %s: %+v", id, err)
}

d.Set("name", resp.Name)
d.Set("resource_group_name", id.ResourceGroup)
d.Set("location", location.NormalizeNilable(resp.Location))
d.Set("zones", utils.FlattenStringSlice(resp.Zones))
return tags.FlattenAndSet(d, resp.Tags)
}

func resourceCapacityReservationGroupUpdate(d *pluginsdk.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).Compute.CapacityReservationGroupsClient
ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d)
defer cancel()

id, err := parse.CapacityReservationGroupID(d.Id())
if err != nil {
return err
}

parameters := compute.CapacityReservationGroupUpdate{}

if d.HasChange("tags") {
parameters.Tags = tags.Expand(d.Get("tags").(map[string]interface{}))
}

if _, err := client.Update(ctx, id.ResourceGroup, id.Name, parameters); err != nil {
return fmt.Errorf("updating %s: %+v", id, err)
}
return resourceCapacityReservationGroupRead(d, meta)
}

func resourceCapacityReservationGroupDelete(d *pluginsdk.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).Compute.CapacityReservationGroupsClient
ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d)
defer cancel()

id, err := parse.CapacityReservationGroupID(d.Id())
if err != nil {
return err
}

// It takes several seconds to sync the cache of reservations list in Capacity Reservation Group. Delete operation requires the list to be empty, and fails before the cache sync is completed.
// Retry the delete operation after a minute as a workaround. Issue is tracked by: https://github.com/Azure/azure-rest-api-specs/issues/18767
if _, err := client.Delete(ctx, id.ResourceGroup, id.Name); err != nil {
time.Sleep(1 * time.Minute)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i just noticed this - we should not be sleeping. is there something you can check/poll on instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@katbyte I tried get the reservations list in the reservation group but unfortunately it changes to empty very soon after the reservation is deleted. However, even if it's empty, the deletion still needs some additional seconds to work

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw the deletion in resource_group waits for 30 seconds in each polling, maybe we can wait for 30 seconds instead here as well? The internal cache is invisible from api but it shall be sync in less than 10 seconds

if len(nestedResourceIds) > 0 {
time.Sleep(30 * time.Second)
return pluginsdk.RetryableError(resourceGroupContainsItemsError(id.ResourceGroup, nestedResourceIds))

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is inside a retry function and acts as a delay between checking, it was a quick hack that was put in post 3.0 in 3.0.1 very quickly and should likely be changed/fixed to a state wait.

Could we instead put this inside a statewait where the retry and sleep logic is handled correctly? thanksa

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@katbyte yes it's better to use the common way. I've moved the deletion workaround into a stateRefresh function, can you take a look? Thanks!

if _, err := client.Delete(ctx, id.ResourceGroup, id.Name); err != nil {
return fmt.Errorf("deleting %s: %+v", id, err)
}
}

return nil
}
182 changes: 182 additions & 0 deletions internal/services/compute/capacity_reservation_group_resource_test.go
@@ -0,0 +1,182 @@
package compute_test

import (
"context"
"fmt"
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance"
"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance/check"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/compute/parse"
"github.com/hashicorp/terraform-provider-azurerm/utils"
)

type CapacityReservationGroupResource struct{}

func TestAccCapacityReservationGroup_basic(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_capacity_reservation_group", "test")
r := CapacityReservationGroupResource{}
data.ResourceTest(t, r, []resource.TestStep{
{
Config: r.basic(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func TestAccCapacityReservationGroup_requiresImport(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_capacity_reservation_group", "test")
r := CapacityReservationGroupResource{}
data.ResourceTest(t, r, []resource.TestStep{
{
Config: r.basic(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.RequiresImportErrorStep(r.requiresImport),
})
}

func TestAccCapacityReservationGroup_zones(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_capacity_reservation_group", "test")
r := CapacityReservationGroupResource{}
data.ResourceTest(t, r, []resource.TestStep{
{
Config: r.zones(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func TestAccCapacityReservationGroup_tags(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_capacity_reservation_group", "test")
r := CapacityReservationGroupResource{}
data.ResourceTest(t, r, []resource.TestStep{
{
Config: r.tags(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
{
Config: r.tagsUpdated(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func (r CapacityReservationGroupResource) Exists(ctx context.Context, client *clients.Client, state *terraform.InstanceState) (*bool, error) {
id, err := parse.CapacityReservationGroupID(state.ID)
if err != nil {
return nil, err
}

resp, err := client.Compute.CapacityReservationGroupsClient.Get(ctx, id.ResourceGroup, id.Name, "")
if err != nil {
if utils.ResponseWasNotFound(resp.Response) {
return utils.Bool(false), nil
}
return nil, fmt.Errorf("retrieving %s: %+v", id, err)
}

return utils.Bool(resp.ID != nil), nil
}

func (r CapacityReservationGroupResource) template(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {}
}

resource "azurerm_resource_group" "test" {
name = "acctest-compute-%d"
location = "%s"
}
`, data.RandomInteger, data.Locations.Primary)
}

func (r CapacityReservationGroupResource) basic(data acceptance.TestData) string {
template := r.template(data)
return fmt.Sprintf(`
%s

resource "azurerm_capacity_reservation_group" "test" {
name = "acctest-crg-%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
}
`, template, data.RandomInteger)
}

func (r CapacityReservationGroupResource) requiresImport(data acceptance.TestData) string {
config := r.basic(data)
return fmt.Sprintf(`
%s

resource "azurerm_capacity_reservation_group" "import" {
name = azurerm_capacity_reservation_group.test.name
resource_group_name = azurerm_capacity_reservation_group.test.resource_group_name
location = azurerm_capacity_reservation_group.test.location
}
`, config)
}

func (r CapacityReservationGroupResource) zones(data acceptance.TestData) string {
template := r.template(data)
return fmt.Sprintf(`
%s

resource "azurerm_capacity_reservation_group" "test" {
name = "acctest-ccrg-%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
zones = ["1", "2"]
}
`, template, data.RandomInteger)
}

func (r CapacityReservationGroupResource) tags(data acceptance.TestData) string {
template := r.template(data)
return fmt.Sprintf(`
%s

resource "azurerm_capacity_reservation_group" "test" {
name = "acctest-ccrg-%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
tags = {
ENV = "Test"
}
}
`, template, data.RandomInteger)
}

func (r CapacityReservationGroupResource) tagsUpdated(data acceptance.TestData) string {
template := r.template(data)
return fmt.Sprintf(`
%s

resource "azurerm_capacity_reservation_group" "test" {
name = "acctest-ccrg-%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
tags = {
ENV2 = "Test2"
}
}
`, template, data.RandomInteger)
}