azurerm_management_lock - Add post-create/delete polling to tolerate RP propagation

[magodo]

This PR adds post-create/delete polling to mitigate issues caused by slow RP propagation, e.g. a GET after creation returned 404 when the GET hits a different Azure regional endpoint.

There are currently a couple of (random) test failures in the daily test suite are failing due to this, e.g.:

=== RUN   TestAccManagementLock_resourceGroupReadOnlyBasic
=== PAUSE TestAccManagementLock_resourceGroupReadOnlyBasic
=== CONT  TestAccManagementLock_resourceGroupReadOnlyBasic
    testcase.go:113: Step 1/2 error: Error running apply: exit status 1
        Error: Provider produced inconsistent result after apply
        When applying changes to azurerm_management_lock.test, provider
        "provider[\"registry.terraform.io/hashicorp/azurerm\"]" produced an
        unexpected new value: Root resource was present, but now absent.
        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

Test

💤  TF_ACC=1 go test -v -timeout=20h -run='TestAccManagementLock_' ./internal/services/resource
=== RUN   TestAccManagementLock_resourceGroupReadOnlyBasic
=== PAUSE TestAccManagementLock_resourceGroupReadOnlyBasic
=== RUN   TestAccManagementLock_requiresImport
=== PAUSE TestAccManagementLock_requiresImport
=== RUN   TestAccManagementLock_resourceGroupReadOnlyComplete
=== PAUSE TestAccManagementLock_resourceGroupReadOnlyComplete
=== RUN   TestAccManagementLock_resourceGroupCanNotDeleteBasic
=== PAUSE TestAccManagementLock_resourceGroupCanNotDeleteBasic
=== RUN   TestAccManagementLock_resourceGroupCanNotDeleteComplete
=== PAUSE TestAccManagementLock_resourceGroupCanNotDeleteComplete
=== RUN   TestAccManagementLock_publicIPReadOnlyBasic
=== PAUSE TestAccManagementLock_publicIPReadOnlyBasic
=== RUN   TestAccManagementLock_publicIPCanNotDeleteBasic
=== PAUSE TestAccManagementLock_publicIPCanNotDeleteBasic
=== RUN   TestAccManagementLock_subscriptionReadOnlyBasic
    management_lock_resource_test.go:130: `TF_ACC_SUBSCRIPTION_PARALLEL_LOCK` isn't specified - skipping since this test can't be run in Parallel
--- SKIP: TestAccManagementLock_subscriptionReadOnlyBasic (0.00s)
=== RUN   TestAccManagementLock_subscriptionCanNotDeleteBasic
    management_lock_resource_test.go:150: `TF_ACC_SUBSCRIPTION_PARALLEL_LOCK` isn't specified - skipping since this test can't be run in Parallel
--- SKIP: TestAccManagementLock_subscriptionCanNotDeleteBasic (0.00s)
=== CONT  TestAccManagementLock_resourceGroupReadOnlyBasic
=== CONT  TestAccManagementLock_resourceGroupCanNotDeleteComplete
=== CONT  TestAccManagementLock_publicIPCanNotDeleteBasic
=== CONT  TestAccManagementLock_resourceGroupReadOnlyComplete
=== CONT  TestAccManagementLock_resourceGroupCanNotDeleteBasic
=== CONT  TestAccManagementLock_publicIPReadOnlyBasic
=== CONT  TestAccManagementLock_requiresImport
--- PASS: TestAccManagementLock_resourceGroupCanNotDeleteBasic (288.93s)
--- PASS: TestAccManagementLock_publicIPReadOnlyBasic (322.27s)
--- PASS: TestAccManagementLock_resourceGroupReadOnlyComplete (352.41s)
--- PASS: TestAccManagementLock_resourceGroupCanNotDeleteComplete (353.01s)
--- PASS: TestAccManagementLock_requiresImport (353.93s)
--- PASS: TestAccManagementLock_resourceGroupReadOnlyBasic (356.80s)
--- PASS: TestAccManagementLock_publicIPCanNotDeleteBasic (395.21s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/resource      395.238s

[stephybun]

Thanks @magodo. There are two issues that should be fixed up but once that's done this should be good to go. Thanks!

[stephybun]

We should be getting the deadline which will give us the remaining time for the create and not using the update timeout which was removed above?

Suggested change

	stateConf := &pluginsdk.StateChangeConf{
	Target: []string{
	"OK",
	},
	Refresh: managementLockStateRefreshFunc(ctx, client, id),
	MinTimeout: 10 * time.Second,
	ContinuousTargetOccurence: 12,
	Timeout: d.Timeout(pluginsdk.TimeoutUpdate),
	}
	if _, err := stateConf.WaitForStateContext(ctx); err != nil {
	return fmt.Errorf("waiting for %s to finish create replication", id)
	}
	deadline, ok := ctx.Deadline()
	if !ok {
	return fmt.Errorf("internal-error: context was missing a deadline")
	}
	stateConf := &pluginsdk.StateChangeConf{
	Target: []string{
	"OK",
	},
	Refresh: managementLockStateRefreshFunc(ctx, client, id),
	MinTimeout: 10 * time.Second,
	ContinuousTargetOccurence: 12,
	Timeout: time.Until(deadline),
	}
	if _, err := stateConf.WaitForStateContext(ctx); err != nil {
	return fmt.Errorf("waiting for %s to finish create replication", id)
	}

[stephybun]

Same here

[magodo]

@stephybun Thank you for the quick review! I've applied the changes you've pointed out, please take another look.

[stephybun]

Thanks @magodo! LGTM 👍

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.