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

provider: adding a cache around the Resource Provider Registration #21695

Merged
merged 9 commits into from
May 9, 2023

Conversation

tombuildsstuff
Copy link
Contributor

@tombuildsstuff tombuildsstuff commented May 8, 2023

This PR supersedes #20050 and hashicorp/go-azure-helpers#158 and introduces a cache for the list of Resource Providers cached during provider startup.

As mentioned in #20050, the providers API can take 6-8s to respond - as such there's performance benefits to caching this, in theory. Whilst that PR looks reasonable, we actually need to remove this logic from hashicorp/go-azure-helpers (since it can't be migrated across to using hashicorp/go-azure-sdk without adding a circular reference - and this is also specific to Resource Manager API's).

However since Terraform relaunches the provider binary multiple times - that is during a terraform apply the binary will be launched once to compute the plan and then another time post-plan confirmation - meaning that the providers API will still be called multiple times per terraform run, however this'll be limited to one time per provider binary launch.

As such this PR:

  1. Updates the Resource Provider Registration SDK to using hashicorp/go-azure-sdk (meaning we can remove this logic from hashicorp/go-azure-helpers).
  2. Updates the API Version used to register Resource Providers to 2022-09-01.
  3. Adds Custom Pollers to ensure the Resource Providers are fully registered at provider launch (by polling on the registrationState field).

Supersedes #20050
Supersedes hashicorp/go-azure-helpers#158

* provider: Resource Provider Registration now uses API Version [GH-21695]
* provider: fixing a bug where we would invoke but not poll for the Registration State during automatic Resource Provider Registration [GH-21695]

…API Version `2022-09-01`

This PR supersedes both #20050 and
hashicorp/go-azure-helpers#158 by refactoring this to use a cache
and then migrating this across to `hashicorp/go-azure-sdk`, since this needs to be migrated away
from using `Azure/azure-sdk-for-go` anyway.
@tombuildsstuff
Copy link
Contributor Author

tombuildsstuff commented May 8, 2023

@manicminer @jackofallops I've moved this logic fully into the Provider, but part of me's wondering if this belongs within `hashicorp/go-azure-sdk - WDYT?

@jackofallops
Copy link
Member

@manicminer @jackofallops I've moved this logic fully into the Provider, but part of me's wondering if this belongs within `hashicorp/go-azure-sdk - WDYT?

Given the potential re-use in stack etc, possibly worth it in the longer term? But for now I think it's fine here?

Copy link
Member

@jackofallops jackofallops left a comment

Choose a reason for hiding this comment

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

Thanks @tombuildsstuff - LGTM 👍

Copy link
Contributor

@manicminer manicminer left a comment

Choose a reason for hiding this comment

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

I agree this is probably best in the provider, and personally I'd likely just copy it into azurestack as needed and leave it at that.

internal/resourceproviders/cache.go Show resolved Hide resolved
@tombuildsstuff
Copy link
Contributor Author

Tests look good 👍

@tombuildsstuff tombuildsstuff merged commit 6e8559d into main May 9, 2023
@tombuildsstuff tombuildsstuff deleted the f/resource-provider-registration-caching branch May 9, 2023 14:17
tombuildsstuff added a commit that referenced this pull request May 9, 2023
@github-actions
Copy link

This functionality has been released in v3.56.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants