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

azurerm_log_analytics_workspace: allow changing sku between CapacityReservation and PerGB2018 #22597

Merged
merged 3 commits into from
Aug 7, 2023

Conversation

ziyeqf
Copy link
Contributor

@ziyeqf ziyeqf commented Jul 19, 2023

fix #12177

changing between CapacityReservation and PerGB2018 does not force new resource to be created, but it will lead to a 31-days commitment period during which it could not be changed to lower pricing tier. https://learn.microsoft.com/en-us/azure/azure-monitor/logs/cost-logs#commitment-tiers)

test

❯ tftest loganalytics TestAccLogAnalyticsWorkspace    
=== RUN   TestAccLogAnalyticsWorkspace_basic
=== PAUSE TestAccLogAnalyticsWorkspace_basic
=== RUN   TestAccLogAnalyticsWorkspace_requiresImport
=== PAUSE TestAccLogAnalyticsWorkspace_requiresImport
=== RUN   TestAccLogAnalyticsWorkspace_complete
=== PAUSE TestAccLogAnalyticsWorkspace_complete
=== RUN   TestAccLogAnalyticsWorkspace_freeTier
    log_analytics_workspace_resource_test.go:95: `TestAccLogAnalyticsWorkspace_freeTier` due to subscription pricing tier configuration (e.g. Pricing tier doesn't match the subscription's billing model.)
--- SKIP: TestAccLogAnalyticsWorkspace_freeTier (0.00s)
=== RUN   TestAccLogAnalyticsWorkspace_withDefaultSku
=== PAUSE TestAccLogAnalyticsWorkspace_withDefaultSku
=== RUN   TestAccLogAnalyticsWorkspace_withVolumeCap
=== PAUSE TestAccLogAnalyticsWorkspace_withVolumeCap
=== RUN   TestAccLogAnalyticsWorkspace_removeVolumeCap
=== PAUSE TestAccLogAnalyticsWorkspace_removeVolumeCap
=== RUN   TestAccLogAnalyticsWorkspace_withInternetIngestionEnabled
=== PAUSE TestAccLogAnalyticsWorkspace_withInternetIngestionEnabled
=== RUN   TestAccLogAnalyticsWorkspace_withInternetQueryEnabled
=== PAUSE TestAccLogAnalyticsWorkspace_withInternetQueryEnabled
=== RUN   TestAccLogAnalyticsWorkspace_withCapacityReservation
=== PAUSE TestAccLogAnalyticsWorkspace_withCapacityReservation
=== RUN   TestAccLogAnalyticsWorkspace_negativeOne
=== PAUSE TestAccLogAnalyticsWorkspace_negativeOne
=== RUN   TestAccLogAnalyticsWorkspace_cmkForQueryForced
=== PAUSE TestAccLogAnalyticsWorkspace_cmkForQueryForced
=== RUN   TestAccLogAnalyticsWorkspace_ToggleAllowOnlyResourcePermission
=== PAUSE TestAccLogAnalyticsWorkspace_ToggleAllowOnlyResourcePermission
=== RUN   TestAccLogAnalyticsWorkspace_ToggleDisableLocalAuth
=== PAUSE TestAccLogAnalyticsWorkspace_ToggleDisableLocalAuth
=== RUN   TestAccLogAnalyticsWorkspace_updateSku
=== PAUSE TestAccLogAnalyticsWorkspace_updateSku
=== CONT  TestAccLogAnalyticsWorkspace_basic
=== CONT  TestAccLogAnalyticsWorkspace_withInternetQueryEnabled
=== CONT  TestAccLogAnalyticsWorkspace_ToggleAllowOnlyResourcePermission
=== CONT  TestAccLogAnalyticsWorkspace_withVolumeCap
=== CONT  TestAccLogAnalyticsWorkspace_negativeOne
=== CONT  TestAccLogAnalyticsWorkspace_requiresImport
=== CONT  TestAccLogAnalyticsWorkspace_withDefaultSku
=== CONT  TestAccLogAnalyticsWorkspace_complete
=== NAME  TestAccLogAnalyticsWorkspace_withInternetQueryEnabled
    testcase.go:113: Error running init: exit status 1
        
        Error: Failed to install provider
        
        Error while installing hashicorp/azuread v2.8.0: read tcp
        198.18.0.1:52300->198.18.0.44:443: read: operation timed out
        
--- PASS: TestAccLogAnalyticsWorkspace_requiresImport (146.76s)
=== CONT  TestAccLogAnalyticsWorkspace_updateSku
=== CONT  TestAccLogAnalyticsWorkspace_withInternetIngestionEnabled
--- FAIL: TestAccLogAnalyticsWorkspace_withInternetQueryEnabled (148.63s)
--- PASS: TestAccLogAnalyticsWorkspace_withDefaultSku (166.74s)
=== CONT  TestAccLogAnalyticsWorkspace_ToggleDisableLocalAuth
--- PASS: TestAccLogAnalyticsWorkspace_complete (173.74s)
=== CONT  TestAccLogAnalyticsWorkspace_cmkForQueryForced
--- PASS: TestAccLogAnalyticsWorkspace_negativeOne (214.52s)
=== CONT  TestAccLogAnalyticsWorkspace_withCapacityReservation
--- PASS: TestAccLogAnalyticsWorkspace_basic (229.89s)
=== CONT  TestAccLogAnalyticsWorkspace_removeVolumeCap
--- PASS: TestAccLogAnalyticsWorkspace_withVolumeCap (250.41s)
--- PASS: TestAccLogAnalyticsWorkspace_ToggleAllowOnlyResourcePermission (316.67s)
--- PASS: TestAccLogAnalyticsWorkspace_updateSku (183.77s)
--- PASS: TestAccLogAnalyticsWorkspace_withInternetIngestionEnabled (186.86s)
--- PASS: TestAccLogAnalyticsWorkspace_ToggleDisableLocalAuth (177.66s)
--- PASS: TestAccLogAnalyticsWorkspace_cmkForQueryForced (176.09s)
--- PASS: TestAccLogAnalyticsWorkspace_withCapacityReservation (209.43s)
--- PASS: TestAccLogAnalyticsWorkspace_removeVolumeCap (216.31s)
FAIL
FAIL    github.com/hashicorp/terraform-provider-azurerm/internal/services/loganalytics  446.273s
FAIL

rerun the failed one

❯ tftest loganalytics TestAccLogAnalyticsWorkspace_withInternetQueryEnabled
=== RUN   TestAccLogAnalyticsWorkspace_withInternetQueryEnabled
=== PAUSE TestAccLogAnalyticsWorkspace_withInternetQueryEnabled
=== CONT  TestAccLogAnalyticsWorkspace_withInternetQueryEnabled
--- PASS: TestAccLogAnalyticsWorkspace_withInternetQueryEnabled (144.83s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/loganalytics  144.886s

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.

Thanks @ziyeqf, this is off to a good start, however we'll need to avoid using d.GetRawConfig(). If you can update this then we'll circle back for re-review and testing. Thanks!

capacityReservationLevel, ok := d.GetOk(propName)
if ok {
// read from raw config as it's an optional + computed property, GetOk() will read value from state.
capacityReservationLevel := d.GetRawConfig().AsValueMap()[propName]
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not use GetRawConfig as this will not translate to terraform-plugin-framework

@@ -45,6 +45,8 @@ The following arguments are supported:

~> **NOTE:** A new pricing model took effect on `2018-04-03`, which requires the SKU `PerGB2018`. If you're provisioned resources before this date you have the option of remaining with the previous Pricing SKU and using the other SKUs defined above. More information about [the Pricing SKUs is available at the following URI](https://aka.ms/PricingTierWarning).

~> **NOTE:** Changing `sku` forces a new Log Analytics Workspace to be created, except changing between `PerGB2018` and `CapacityReservation`, but changing it to `CapacityReservation` or changing `reservation_capacity_in_gb_per_day` to a higher tier will lead to a 31-days commitment period, during which the sku could not be changed to a lower one. More information about [the commitment perioud](https://learn.microsoft.com/en-us/azure/azure-monitor/logs/cost-logs#commitment-tiers)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
~> **NOTE:** Changing `sku` forces a new Log Analytics Workspace to be created, except changing between `PerGB2018` and `CapacityReservation`, but changing it to `CapacityReservation` or changing `reservation_capacity_in_gb_per_day` to a higher tier will lead to a 31-days commitment period, during which the sku could not be changed to a lower one. More information about [the commitment perioud](https://learn.microsoft.com/en-us/azure/azure-monitor/logs/cost-logs#commitment-tiers)
~> **NOTE:** Changing `sku` forces a new Log Analytics Workspace to be created, except when changing between `PerGB2018` and `CapacityReservation`. However, changing `sku` to `CapacityReservation` or changing `reservation_capacity_in_gb_per_day` to a higher tier will lead to a 31-days commitment period, during which the SKU cannot be changed to a lower one. Please refer to [official documentation](https://learn.microsoft.com/en-us/azure/azure-monitor/logs/cost-logs#commitment-tiers) for further information.

ziyeqf added 2 commits July 20, 2023 10:10
Signed-off-by: ziyeqf <[email protected]>
Signed-off-by: ziyeqf <[email protected]>
@ziyeqf
Copy link
Contributor Author

ziyeqf commented Jul 20, 2023

we might need to note reservation_capacity_in_gb_per_day is not computed yet anymore in changelog?

@ziyeqf ziyeqf requested a review from manicminer August 1, 2023 03:03
@katbyte
Copy link
Collaborator

katbyte commented Aug 1, 2023

@ziyeqf - what do you mean by reservation_capacity_in_gb_per_day is not computed yet ?

@ziyeqf
Copy link
Contributor Author

ziyeqf commented Aug 2, 2023

@ziyeqf - what do you mean by reservation_capacity_in_gb_per_day is not computed yet ?

oh, I mean it's not computed anymore.

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ziyeqf
Copy link
Contributor Author

ziyeqf commented Aug 7, 2023

kindly ping for merge :)

@stephybun stephybun merged commit a03cc3f into hashicorp:main Aug 7, 2023
@github-actions github-actions bot added this to the v3.69.0 milestone Aug 7, 2023
@ziyeqf ziyeqf deleted the issue/log_workspace_sku branch August 7, 2023 14:11
stephybun added a commit that referenced this pull request Aug 7, 2023
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 18, 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.

azurerm_log_analytics_workspace: on change of "sku", terraform incorrectly wants to rebuild workspace
5 participants