-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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_table
: support plan
property
#24341
Conversation
Related to issue #16889 |
@@ -41,9 +41,15 @@ The following arguments are supported: | |||
|
|||
* `workspace_id` - (Required) The object ID of the Log Analytics Workspace that contains the table. | |||
|
|||
* `retention_in_days` - (Required) The table's retention in days. Possible values are either 7 (Free Tier only) or range between 30 and 730. | |||
* `plan` - (Optional) Specify the system how to handle and charge the logs ingested to the table. Possible values are `Analytics` and `Basic. Defaults to `Analytics`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basic
is missing the closing backtick `
|
||
if string(tables.TablePlanEnumBasic) == rd.Get("plan").(string) { | ||
if _, ok := rd.GetOk("retention_in_days"); ok { | ||
return fmt.Errorf("cannot set retention_in_days as it is immutabl on Basic plan") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
immutabl -> immutable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of "immutable" should we use the wording is "Retention is fixed at eight days".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zioproto thanks for your feedback. I'm using immutable
because I'm not sure if "8 days" would change to something else in the future. If changes might occur, TF may not be able to modify it in time, thus giving users wrong prompts. Of course, I'd be happy to put "8 days" in the message if the service team could confirm that this number won't change in the future. Could you help confirm this? Thanks in advance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The context is that the error messages should give information that helps the customer to find the documentation searching on the web.
The problem here is that a customer searching on Google "retention_in_days as it is immutable on Basic Plan" will not find anything in the Azure documentation. Instead searching "retention_in_days fixed at 8 days" will return the correct Azure documentation page where this is explained.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation. The code has been updated.
|
||
-> **Note:** `retention_in_days` will revert back to the value of azurerm_log_analytics_workspace retention_in_days when a azurerm_log_analytics_workspace_table is deleted. | ||
|
||
-> **Note:** The `retention_in_days` cannot be specified when the `plan` is `Basic` because it is immutable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"because the retention is fixed to 8 days" would give a better information to the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it is safe to mention the 8 days. The product is GA and this value is mentioned multiple times in the documentation:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation. The code has been updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aside from 1 comment LGTM 🍰
<Actions> <action id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>"hashicorp/azurerm" updated from "3.85.0" to "3.86.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.86.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.86.0
FEATURES:

* New Data Source: `azurerm_dashboard_grafana` ([#24243](hashicorp/terraform-provider-azurerm#24243 New Resource: `azurerm_log_analytics_workspace_table` ([#24229](hashicorp/terraform-provider-azurerm#24229 New Resource: `azurerm_automation_powershell72_module` ([#23980](hashicorp/terraform-provider-azurerm#23980 New Resource: `azurerm_data_factory_credential_user_managed_identity` ([#24307](https://github.com/hashicorp/terraform-provider-azurerm/issues/24307))

ENHANCEMENTS:

* dependencies: updating to `v0.20231215.1114251` of `hashicorp/go-azure-sdk` ([#24251](hashicorp/terraform-provider-azurerm#24251 dependencies: `azurerm_spring_cloud_api_portal` - update to use `hashicorp/go-azure-sdk` ([#24321](hashicorp/terraform-provider-azurerm#24321 Data Source: `azurerm_kusto_cluster` - now exports the `identity` block ([#24314](hashicorp/terraform-provider-azurerm#24314 `azurerm_data_protection_backup_policy_postgresql` - support for the `time_zone` property ([#24312](hashicorp/terraform-provider-azurerm#24312 `azurerm_data_protection_backup_policy_disk` - support for the `time_zone` property ([#24312](hashicorp/terraform-provider-azurerm#24312 `azurerm_key_vault_managed_hardware_security_module` -the `tags` property can now be updated ([#24333](hashicorp/terraform-provider-azurerm#24333 `azurerm_logic_app_standard` - support for the `site_config.0.public_network_access_enabled` property ([#24257](hashicorp/terraform-provider-azurerm#24257 `azurerm_log_analytics_workspace_table` - support for the `plan` property ([#24341](hashicorp/terraform-provider-azurerm#24341 `azurerm_linux_web_app` - support the value `20-lts` for the `node_version` property ([#24289](hashicorp/terraform-provider-azurerm#24289 `azurerm_recovery_services_vault` - support creation with immutability set to locked ([#23806](hashicorp/terraform-provider-azurerm#23806 `azurerm_spring_cloud_service` - support for the `sku_tier` property ([#24103](https://github.com/hashicorp/terraform-provider-azurerm/issues/24103))

BUG FIXES:

* Data Source: `azurerm_role_definition` - correctly export the `role_definition_id` attribute ([#24320](hashicorp/terraform-provider-azurerm#24320 `azurerm_bot_service` - fixing a bug where `public_network_access_enabled` was always set to `true` ([#24255](hashicorp/terraform-provider-azurerm#24255 `azurerm_bot_service_azure_bot` - `tags` can now be updated ([#24332](hashicorp/terraform-provider-azurerm#24332 `azurerm_cosmosdb_account` - fix validation for the `ip_range_filter` property ([#24306](hashicorp/terraform-provider-azurerm#24306 `azurerm_linux_virtual_machine` - the `additional_capabilities.0.ultra_ssd_enabled` can now be changed during the update ([#24274](hashicorp/terraform-provider-azurerm#24274 `azurerm_logic_app_standard` - update the default value of `version` from `~3` which is no longer supported to `~4` ([#24134](hashicorp/terraform-provider-azurerm#24134 `azurerm_logic_app_standard` - fix a crash when setting the default `version` 4.0 flag ([#24322](hashicorp/terraform-provider-azurerm#24322 `azurerm_iothub_device_update_account` - changing the `sku` property now creates a new resource ([#24324](hashicorp/terraform-provider-azurerm#24324 `azurerm_iothub` - prevent an inconsistant value after an apply ([#24326](hashicorp/terraform-provider-azurerm#24326 `azurerm_orchestrated_virtual_machine_scale_set` - correctly update the resource when hotpatch is enabled ([#24335](hashicorp/terraform-provider-azurerm#24335 `azurerm_windows_virtual_machine` - the `additional_capabilities.0.ultra_ssd_enabled` can now be changed during the update ([#24274](hashicorp/terraform-provider-azurerm#24274 `azurerm_scheduled_query_rules_alert` - changing the `data_source_id` now creates a new resource ([#24327](hashicorp/terraform-provider-azurerm#24327 `azurerm_scheduled_query_rules_log` - changing the `data_source_id` now creates a new resource ([#24327](https://github.com/hashicorp/terraform-provider-azurerm/issues/24327))


</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/terraform-jobs/job/azure/job/main/985/">Jenkins pipeline link</a> </action> </Actions> --- <table> <tr> <td width="77"> <img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli logo" width="50" height="50"> </td> <td> <p> Created automatically by <a href="https://www.updatecli.io/">Updatecli</a> </p> <details><summary>Options:</summary> <br /> <p>Most of Updatecli configuration is done via <a href="https://www.updatecli.io/docs/prologue/quick-start/">its manifest(s)</a>.</p> <ul> <li>If you close this pull request, Updatecli will automatically reopen it, the next time it runs.</li> <li>If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.</li> </ul> <p> Feel free to report any issues at <a href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br /> If you find this tool useful, do not hesitate to star <a href="https://github.com/updatecli/updatecli/stargazers">our GitHub repository</a> as a sign of appreciation, and/or to tell us directly on our <a href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>! </p> </details> </td> </tr> </table> Co-authored-by: Jenkins Infra Bot (updatecli) <[email protected]>
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. |
Swagger: https://github.com/Azure/azure-rest-api-specs/blob/d1eb3c20113d1018f25a8d97fdfa5f8bb5c659ea/specification/operationalinsights/resource-manager/Microsoft.OperationalInsights/stable/2022-10-01/Tables.json#L730
Change
retention_in_days
tooptional
because API does not accept setting this value inBasic
plan.There is no test case for updating
plan
since changing theplan
is limited to once a week.Test results:
PASS: TestAccLogAnalyticsWorkspaceTable_plan (267.83s)
PASS: TestAccLogAnalyticsWorkspaceTable_updateTableRetention (253.40s)
Fix #16889