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.

Sign up for GitHub

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

Jump to bottom

azurerm_managed_disk - support for PremiumV2_LRS #17671

Merged
merged 19 commits into from
Sep 22, 2022
Merged

azurerm_managed_disk - support for PremiumV2_LRS #17671

merged 19 commits into from
Sep 22, 2022

Conversation

neil-yechenwei
Copy link
Contributor

@neil-yechenwei neil-yechenwei commented Jul 19, 2022 •
edited
Loading

This PR includes two changes.

  1. Upgrade API version to 2022-03-02 for Disks.
  2. Support new storage account type PremiumV2_LRS for azurerm_managed_disk. This feature only exists in API version 2022-03-02.

@katbyte katbyte self-assigned this Jul 20, 2022
@tombuildsstuff tombuildsstuff mentioned this pull request Aug 2, 2022
Closed
@katbyte katbyte added this to the Blocked milestone Aug 9, 2022
@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Aug 11, 2022 •
edited
Loading

@katbyte , Updated this PR to use Pandora SDK for Disks.

@neil-yechenwei neil-yechenwei changed the title azurerm_managed_disk - support PremiumV2_LRS azurerm_managed_disk - support for PremiumV2_LRS Aug 15, 2022
@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Aug 23, 2022 •
edited
Loading

Note: The failed TCs in below screenshot are also failed in Teamcity Nightly Run due to same error. So they aren't related with this PR.
image
image
image

katbyte
katbyte requested changes Aug 30, 2022
View reviewed changes
Copy link
Collaborator

@katbyte katbyte left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM aside from test failures:
image

the resource group not deleting is likely due to a delete call finishing to early. failing in main or not we should be fixing them as we are doing an SDK upgrade where its exceptionally important to test

@neil-yechenwei
Copy link
Contributor Author

neil-yechenwei commented Aug 30, 2022 •
edited
Loading

@katbyte , please running TCs in specific region like west europe in teamcity because not all settings work for all regions.

I just now fixed and reran all related TCs and they all passed on Teamcity. Is it able to be merged?
image

@neil-yechenwei neil-yechenwei force-pushed the manageddiskstorageaccounttypev2 branch 2 times, most recently from f0a8283 to 676fab0 Compare August 31, 2022 02:36
@neil-yechenwei neil-yechenwei force-pushed the manageddiskstorageaccounttypev2 branch from e6df6d0 to aec84e1 Compare August 31, 2022 03:32
@neil-yechenwei neil-yechenwei force-pushed the manageddiskstorageaccounttypev2 branch from a00a69e to 1da64e8 Compare August 31, 2022 06:02
@neil-yechenwei neil-yechenwei requested a review from katbyte August 31, 2022 08:11
@myc2h6o
Copy link
Contributor

myc2h6o commented Sep 9, 2022

I happened to be fixing the test failures in compute service. For specific test TestAccManagedDisk_storageAccountType that needs to be run in west europe, maybe @neil-yechenwei you can try a similar approach as #18331, setting data.Locations.Primary = "westeurope" in the test config. And one of the merge conflict reasons might be #18303 which fixes TestAccManagedDisk_import as well, could you help check if that's the case?

@neil-yechenwei
Copy link
Contributor Author

@myc2h6o , I've updated code. Thanks for your reminder.

@myc2h6o myc2h6o mentioned this pull request Sep 16, 2022
Merged
katbyte
katbyte approved these changes Sep 21, 2022
View reviewed changes
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

LGTM 🌻

@neil-yechenwei
Copy link
Contributor Author

@katbyte , Kindly ping. Is this PR able to be merged?

@katbyte katbyte merged commit 68873d5 into hashicorp:main Sep 22, 2022
katbyte added a commit that referenced this pull request Sep 22, 2022
@katbyte
CHANGELOG #17671
43b0931
@katbyte katbyte modified the milestones: Blocked, v3.24.0 Sep 24, 2022
@github-actions
Copy link

This functionality has been released in v3.24.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!

@github-actions
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 Oct 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@katbyte katbyte katbyte approved these changes

Assignees

@katbyte katbyte

Labels
dependencies documentation size/XL
Projects
None yet
Milestone
v3.24.0
Development

Successfully merging this pull request may close these issues.
3 participants
@neil-yechenwei @myc2h6o @katbyte