new resource `azurerm_stack_hci_storage_path` #26509

teowa · 2024-07-01T05:57:28Z

Community Note

Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

Swagger: https://github.com/Azure/azure-rest-api-specs/blob/81a4ee5a83ae38620c0e1404793caffe005d26e4/specification/azurestackhci/resource-manager/Microsoft.AzureStackHCI/stable/2024-01-01/storageContainers.json

Azure doc: https://learn.microsoft.com/en-us/azure-stack/hci/manage/create-storage-path?tabs=azurecli

Acctest depends on a custom location generated by HCI deployment.

PR Checklist

I have followed the guidelines in our Contributing Documentation.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Changes to existing Resource / Data Source

I have added an explanation of what my changes do and why I'd like you to include them (This may be covered by linking to an issue above, but may benefit from additional explanation).
I have written new tests for my resource or datasource changes & updated any relevent documentation.
I have successfully run tests with my changes locally. If not, please provide details on testing challenges that prevented you running the tests.
(For changes that include a state migration only). I have manually tested the migration path between relevant versions of the provider.

Testing

My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

-> % ARM_TEST_LOCATION=australiaeast ARM_TEST_STACK_HCI_CUSTOM_LOCATION_ID=/subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/acctest-hci-0701/providers/Microsoft.ExtendedLocation/customLocations/customlocation0701 make acctests SERVICE="azurestackhci" TESTARGS="-parallel 5 -run TestAccStackHCIStoragePath_" TESTTIMEOUT=2h
==> Checking that code complies with gofmt requirements...
==> Checking that Custom Timeouts are used...
==> Checking that acceptance test packages are used...
TF_ACC=1 go test -v ./internal/services/azurestackhci -parallel 5 -run TestAccStackHCIStoragePath_ -timeout 2h -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccStackHCIStoragePath_basic
=== PAUSE TestAccStackHCIStoragePath_basic
=== RUN   TestAccStackHCIStoragePath_update
=== PAUSE TestAccStackHCIStoragePath_update
=== RUN   TestAccStackHCIStoragePath_requiresImport
=== PAUSE TestAccStackHCIStoragePath_requiresImport
=== RUN   TestAccStackHCIStoragePath_complete
=== PAUSE TestAccStackHCIStoragePath_complete
=== CONT  TestAccStackHCIStoragePath_basic
=== CONT  TestAccStackHCIStoragePath_requiresImport
=== CONT  TestAccStackHCIStoragePath_complete
=== CONT  TestAccStackHCIStoragePath_update
--- PASS: TestAccStackHCIStoragePath_basic (239.65s)
--- PASS: TestAccStackHCIStoragePath_requiresImport (297.76s)
--- PASS: TestAccStackHCIStoragePath_complete (298.90s)
--- PASS: TestAccStackHCIStoragePath_update (364.97s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/azurestackhci 365.021s

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

new resource azurerm_stack_hci_storage_path [GH-00000]

This is a (please select all that apply):

Bug Fix
New Feature (ie adding a service, resource, or data source)
Enhancement
Breaking Change

Related Issue(s)

Fixes #0000

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

ms-zhenhua

Hi @teowa,
Thanks for this PR - I've taken a look through and left some comments inline. If we can fix those up, this should be good to go 👍

ms-zhenhua · 2024-07-03T07:19:16Z

internal/services/azurestackhci/stack_hci_storage_path_resource.go

+			Type:         pluginsdk.TypeString,
+			Required:     true,
+			ForceNew:     true,
+			ValidateFunc: validation.StringIsNotEmpty,


does the name have max length and special characters limitation?

ms-zhenhua · 2024-07-03T07:45:17Z

internal/services/azurestackhci/stack_hci_storage_path_resource.go

+				},
+			}
+
+			future, err := client.CreateOrUpdate(ctx, id, payload)


why not use CreateOrUpdateThenPoll ?

the resource might exist if poll fail, so here we setID before poll.

why not put metadata.SetID(id) before or after CreateOrUpdateThenPoll ?

The resource creation might fail before the instance is created (e.g., a same name resource already exists, or name property invalid), in this case the ID should not be saved.

both creation failure and poll failure are failure, why SetID for one but not for the other? We should set ID after CreateOrUpdateThenPoll to keep consistent with other resources

for this resource, creation failure will not create the instance, but poll failure will result in the resource created but with a failed status, if we save the ID for poll failure, it will save effort for users to manually import or delete the failed instance. This is similar to the case in #25646 (comment).

ms-zhenhua · 2024-07-03T07:49:23Z

internal/services/azurestackhci/stack_hci_storage_path_resource_test.go

+		t.Skipf("skip the test as one or more of below environment variables are not specified: %q", customLocationIdEnv)
+	}
+
+	data.ResourceTest(t, r, []acceptance.TestStep{


please also test update tags.

…m into stackhci_storage_path

teowa · 2024-07-03T13:47:53Z

--- PASS: TestAccStackHCIStoragePath_basic (243.20s)
--- PASS: TestAccStackHCIStoragePath_requiresImport (300.56s)
--- PASS: TestAccStackHCIStoragePath_complete (300.75s)
--- PASS: TestAccStackHCIStoragePath_update (621.03s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/azurestackhci 621.075s

teowa added 2 commits June 28, 2024 09:50

new resource stack_hci_storage_path

83b411c

add doc

f42a963

teowa marked this pull request as draft July 1, 2024 05:57

github-actions bot added documentation service/azure-stack-hci size/XL labels Jul 1, 2024

ms-zhenhua suggested changes Jul 3, 2024

View reviewed changes

github-actions bot added the waiting-response label Jul 3, 2024

add name validation; update test

4ab5ae1

github-actions bot added waiting-response and removed waiting-response labels Jul 3, 2024

Merge branch 'main' of github.com:hashicorp/terraform-provider-azurer…

57721f4

…m into stackhci_storage_path

github-actions bot removed the waiting-response label Jul 3, 2024

github-actions bot added the waiting-response label Jul 4, 2024

setID after poll

8be6a95

github-actions bot removed the waiting-response label Jul 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new resource `azurerm_stack_hci_storage_path` #26509

new resource `azurerm_stack_hci_storage_path` #26509

teowa commented Jul 1, 2024

ms-zhenhua left a comment

ms-zhenhua Jul 3, 2024

teowa Jul 3, 2024

ms-zhenhua Jul 3, 2024

teowa Jul 3, 2024

ms-zhenhua Jul 4, 2024

teowa Jul 4, 2024

ms-zhenhua Jul 4, 2024

teowa Jul 4, 2024

ms-zhenhua Jul 3, 2024

teowa Jul 3, 2024

teowa commented Jul 3, 2024

new resource azurerm_stack_hci_storage_path #26509

Are you sure you want to change the base?

new resource azurerm_stack_hci_storage_path #26509

Conversation

teowa commented Jul 1, 2024

Community Note

Description

PR Checklist

Changes to existing Resource / Data Source

Testing

Change Log

Related Issue(s)

ms-zhenhua left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teowa commented Jul 3, 2024

new resource `azurerm_stack_hci_storage_path` #26509

new resource `azurerm_stack_hci_storage_path` #26509