-
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_monitor_diagnostic_setting
- remove explicitly disable log
in Update
#20611
azurerm_monitor_diagnostic_setting
- remove explicitly disable log
in Update
#20611
Conversation
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 this @teowa. Could we also add test cases where an enabled_log with category_group is updated from 2 to 1 l enabled_logs and the same for category? It would be good to confirm that this works for logs being removed as well as added. Thanks!
Hi @catriona-m , thanks for reviewing.
|
|
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.
Test failue:
------- Stdout: -------
=== RUN TestAccMonitorDiagnosticSetting_logAnalyticsWorkspace
=== PAUSE TestAccMonitorDiagnosticSetting_logAnalyticsWorkspace
=== CONT TestAccMonitorDiagnosticSetting_logAnalyticsWorkspace
testcase.go:110: Step 1/2 error: After applying this test step, the plan was not empty.
stdout:
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# azurerm_monitor_diagnostic_setting.test will be updated in-place
~ resource "azurerm_monitor_diagnostic_setting" "test" {
id = "/subscriptions/*******/resourceGroups/acctestRG-230223172423818172/providers/Microsoft.KeyVault/vaults/acctest23022317242381817|acctest-DS-230223172423818172"
+ log_analytics_destination_type = "AzureDiagnostics"
name = "acctest-DS-230223172423818172"
# (2 unchanged attributes hidden)
# (3 unchanged blocks hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccMonitorDiagnosticSetting_logAnalyticsWorkspace (335.14s)
FAIL
Refer #20203 for proposed resolution |
The failed test should be fixed in PR #20203 |
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, LGTM 🪵
This functionality has been released in v3.46.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! |
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. |
resolves #20554
A diagnostic setting must use either
category_group
orcategory
inlog
(alsoenabled_log
) block. The service API cannot accept diable a log withcategory_group
and enable a log withcategory
at the same time, see Azure/azure-rest-api-specs#22719By test, we don't need to explicitly disable the logs in update.
Some changes are to pass the acc test, relaetd to #20203