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_batch_pool - Fixes a bug in settings_json of extension block #24976

Merged
merged 8 commits into from
Feb 23, 2024

Conversation

liuwuliuyun
Copy link
Contributor

Related Resource
azurerm_batch_pool
Issue Description
This issue is raised by a customer that if specify settings_json = {} in azurerm_batch_pool. The extension will never be attached to the created batch pool.

Root Cause 1
The very first reason of this issue is that the value this property should be encapsulated with jsonencode function, so the correct usage should be settings_json = jsonencode({}).
Solution for 1
Add proper description for settings_json and protected_settings in doc.

Root Cause 2
The second reason above parsing error is not shown when user runs terraform apply is that the parsing error is not thrown in the resource creat function
Solution for 2
Add error handling logic to throw parsing error when creating the resource.

After above code change, there will be a plan diff after apply. In order to solve this new issue, I dig a little deeper in the read function.

Root Cause 3
In the read function, pluginsdk.FlattenJsonToString will transfer empty map to empty string "" rather than {}.
Solution for 3

  • Use json.Unmarshel to replace pluginsdk.FlattenJsonToString will have correct transformation.
  • Add a new testcase TestAccBatchPool_extensionsWithEmptySettings
GOROOT=C:\Program Files\Go #gosetup
GOPATH=C:\Users\yunliu1\go #gosetup
"C:\Program Files\Go\bin\go.exe" test -c -o C:\Users\yunliu1\AppData\Local\JetBrains\GoLand2023.3\tmp\GoLand\___TestAccBatchPool_extensionsWithEmptySettings_in_github_com_hashicorp_terraform_provider_azurerm_internal_services_batch.test.exe github.com/hashicorp/terraform-provider-azurerm/internal/services/batch #gosetup
"C:\Program Files\Go\bin\go.exe" tool test2json -t C:\Users\yunliu1\AppData\Local\JetBrains\GoLand2023.3\tmp\GoLand\___TestAccBatchPool_extensionsWithEmptySettings_in_github_com_hashicorp_terraform_provider_azurerm_internal_services_batch.test.exe -test.v -test.paniconexit0 -test.run ^\QTestAccBatchPool_extensionsWithEmptySettings\E$ #gosetup
=== RUN   TestAccBatchPool_extensionsWithEmptySettings
=== PAUSE TestAccBatchPool_extensionsWithEmptySettings
=== CONT  TestAccBatchPool_extensionsWithEmptySettings
--- PASS: TestAccBatchPool_extensionsWithEmptySettings (333.10s)
PASS


Process finished with the exit code 0

Copy link
Member

@catriona-m catriona-m left a comment

Choose a reason for hiding this comment

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

Hi @liuwuliuyun thanks for submitting this. It looks like there is a test failure that could be related to this:

------- Stdout: -------
=== RUN   TestAccBatchPool_basic
=== PAUSE TestAccBatchPool_basic
=== CONT  TestAccBatchPool_basic
    testcase.go:113: Step 1/2 error: Error running apply: exit status 1
        Error: creating Pool (Subscription: "*******"
        Resource Group Name: "acctestRG-batch-240222144545372017"
        Batch Account Name: "testaccbatcha3jut"
        Pool Name: "testaccpoola3jut"): unexpected status 400 with error: MissingRequiredProperty: A required property was not specified in the request body.
        RequestId:cb7a6301-4666-4f48-84e9-190c96db8615
        Time:2024-02-22T14:47:02.3650524Z
          with azurerm_batch_pool.test,
          on terraform_plugin_test.tf line 32, in resource "azurerm_batch_pool" "test":
          32: resource "azurerm_batch_pool" "test" {
--- FAIL: TestAccBatchPool_basic (141.73s)
FAIL

Once that is resolved, we can take another look. Thanks!

@liuwuliuyun
Copy link
Contributor Author

liuwuliuyun commented Feb 23, 2024

Thanks @catriona-m for your review. I changed the code related to expanding vnet config. Here are the results from the latest test run. All those failed tests are identical to the failed tests from main branch.

image

Copy link
Member

@catriona-m catriona-m left a comment

Choose a reason for hiding this comment

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

Hi @liuwuliuyun - thanks for making the requested changes! LGTM!

@catriona-m catriona-m merged commit 1812720 into hashicorp:main Feb 23, 2024
33 checks passed
@github-actions github-actions bot added this to the v3.94.0 milestone Feb 23, 2024
catriona-m added a commit that referenced this pull request Feb 23, 2024
rizkybiz pushed a commit to rizkybiz/terraform-provider-azurerm that referenced this pull request Feb 29, 2024
…block (hashicorp#24976)

* throw error when parsing failed for `extension`

* fix empty settings_json in read function

* add new testcase for empty settings_json

* Update batch_pool_resource.go

* optimize expand vnetConfig flow

* Update batch_pool.html.markdown

---------

Co-authored-by: kt <kt@katbyte.me>
Co-authored-by: catriona-m <86247157+catriona-m@users.noreply.github.com>
rizkybiz pushed a commit to rizkybiz/terraform-provider-azurerm that referenced this pull request Feb 29, 2024
lemeurherve pushed a commit to jenkins-infra/azure that referenced this pull request Mar 11, 2024
<Actions>
<action
id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8">
        <h3>Bump Terraform `azurerm` provider version</h3>
<details
id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24">
            <summary>Update Terraform lock file</summary>
<p>changes detected:&#xA;&#x9;&#34;hashicorp/azurerm&#34; updated from
&#34;3.93.0&#34; to &#34;3.94.0&#34; in file
&#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>3.94.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.94.0&#xA;FEATURES:&#xA;&#xA;*
**New Resource**: `azurerm_kubernetes_fleet_update_run`
([#24813](https://github.com/hashicorp/terraform-provider-azurerm/issues/24813))&#xA;&#xA;ENHANCEMENTS:&#xA;&#xA;*
dependencies: updating to `v0.20240228.1142829` of
`github.com/hashicorp/go-azure-sdk`
([#25081](hashicorp/terraform-provider-azurerm#25081
`servicefabric`: updating to use the transport layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest`
([#25002](hashicorp/terraform-provider-azurerm#25002
`springcloud`: updating to API Version `2024-01-01-preview`
([#24937](hashicorp/terraform-provider-azurerm#24937
`securitycenter`: updating to use the transport layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest`
([#25081](hashicorp/terraform-provider-azurerm#25081
Data Source: `azurerm_storage_table_entities` - support for `select`
([#24987](hashicorp/terraform-provider-azurerm#24987
Data Source: `azurerm_netapp_volume` - support for the
`smb_access_based_enumeration` and `smb_non_browsable` properties
([#24514](hashicorp/terraform-provider-azurerm#24514
`azurerm_cosmosdb_account` - add support for the `minimal_tls_version`
property
([#24966](hashicorp/terraform-provider-azurerm#24966
`azurerm_federated_identity_credential` - the federated credentials can
now be changed without creating a new resource
([#25003](hashicorp/terraform-provider-azurerm#25003
`azurerm_kubernetes_cluster` - support for the
`current_kubernetes_version` property
([#25079](hashicorp/terraform-provider-azurerm#25079
`azurerm_kubernetes_cluster` - private DNS is now allowed for the
`web_app_routing` property
([#25038](hashicorp/terraform-provider-azurerm#25038
`azurerm_kubernetes_cluster` - migration between different
`outbound_type`s is now allowed
([#25021](hashicorp/terraform-provider-azurerm#25021
`azurerm_mssql_database` - support for the `recovery_point_id` and
`restore_long_term_retention_backup_id` properties
([#24904](hashicorp/terraform-provider-azurerm#24904
`azurerm_linux_virtual_machine` - support for the
`automatic_upgrade_enabled`, `disk_controller_type`,
`os_image_notification`, `treat_failure_as_deployment_failure_enabled`,
and `vm_agent_platform_updates_enabled`properties
([#23394](hashicorp/terraform-provider-azurerm#23394
`azurerm_nginx_deployment` - support for the `automatic_upgrade_channel`
property
([#24867](hashicorp/terraform-provider-azurerm#24867
`azurerm_netapp_volume` - support for the `smb_access_based_enumeration`
and `smb_non_browsable` properties
([#24514](hashicorp/terraform-provider-azurerm#24514
`azurerm_netapp_pool` - support for the `encryption_type` property
([#24993](hashicorp/terraform-provider-azurerm#24993
`azurerm_role_definition` - upgrade to the API version
`2022-05-01-preview`
([#25008](hashicorp/terraform-provider-azurerm#25008
`azurerm_redis_cache` - allow AAD auth for all SKUs
([#25006](hashicorp/terraform-provider-azurerm#25006
`azurerm_sql_managed_instance` - support for the
`zone_redundant_enabled` property
([#25089](hashicorp/terraform-provider-azurerm#25089
`azurerm_spring_cloud_gateway` - support for the
`application_performance_monitoring_ids` property
([#24919](hashicorp/terraform-provider-azurerm#24919
`azurerm_spring_cloud_configuration_service` - support for the
`refresh_interval_in_seconds` property
([#25009](hashicorp/terraform-provider-azurerm#25009
`azurerm_synapse_workspace` - support for using the
`user_assigned_identity_id` property within the `customer_managed_key`
block
([#25027](hashicorp/terraform-provider-azurerm#25027
`azurerm_windows_virtual_machine` - support for the
`automatic_upgrade_enabled`, `disk_controller_type`,
`os_image_notification`, `treat_failure_as_deployment_failure_enabled`,
and `vm_agent_platform_updates_enabled`properties
([#23394](https://github.com/hashicorp/terraform-provider-azurerm/issues/23394))&#xA;&#xA;BUG
FIXES:&#xA;&#xA;* `azurerm_api_management_notification_recipient_email`
- fixing an issue where response pages weren&#39;t iterated over
correctly
([#25055](hashicorp/terraform-provider-azurerm#25055
`azurerm_api_management_notification_recipient_user` - fixing an issue
where response pages weren&#39;t iterated over correctly
([#25055](hashicorp/terraform-provider-azurerm#25055
`azurerm_batch_pool` - fix setting the `extension.settings_json`
property
([#24976](hashicorp/terraform-provider-azurerm#24976
`azurerm_key_vault_key` - `expiration_date` can be updated if newer date
is ahead
([#25000](hashicorp/terraform-provider-azurerm#25000
`azurerm_pim_active_role_assignment` - fix an isue where the resource
would disappear or fail to import after 45 days
([#24524](hashicorp/terraform-provider-azurerm#24524
`azurerm_pim_eligible_role_assignment` - fix an isue where the resource
would disappear or fail to import after 45 days
([#24524](hashicorp/terraform-provider-azurerm#24524
`azurerm_recovery_services_vault` - validate that
`use_system_assigned_identity` and `user_assigned_identity_id` cannot be
set at the same time
([#24091](hashicorp/terraform-provider-azurerm#24091
`azurerm_recovery_vaults` will now create properly with
`SystemAssigned,UserAssigned` identity
([#24978](hashicorp/terraform-provider-azurerm#24978
`azurerm_subscription` - fixing an issue where response pages
weren&#39;t iterated over correctly
([#25055](https://github.com/hashicorp/terraform-provider-azurerm/issues/25055))&#xA;&#xA;&#xA;</pre>
            </details>
            <details>
                <summary>3.95.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.95.0&#xA;FEATURES:&#xA;&#xA;*
New Resource: `azurerm_container_app_custom_domain`
([#24421](hashicorp/terraform-provider-azurerm#24421
New Resource:
`azurerm_data_protection_backup_instance_kubernetes_cluster`
([#24940](hashicorp/terraform-provider-azurerm#24940
New Resource: `azurerm_static_web_app`
([#25117](hashicorp/terraform-provider-azurerm#25117
New resource: `azurerm_static_web_app_custom_domain`
([#25117](hashicorp/terraform-provider-azurerm#25117
New resource:
`azurerm_system_center_virtual_machine_manager_availability_set`
([#24975](hashicorp/terraform-provider-azurerm#24975
New Resource: `azurerm_workloads_sap_three_tier_virtual_instance`
([#24384](hashicorp/terraform-provider-azurerm#24384
New Resource: `azurerm_workloads_sap_single_node_virtual_instance`
([#24331](https://github.com/hashicorp/terraform-provider-azurerm/issues/24331))&#xA;&#xA;ENHANCEMENTS:&#xA;&#xA;*
`dependencies`: updating to v0.20240229.1102109 of
`github.com/hashicorp/go-azure-sdk`
([#25102](hashicorp/terraform-provider-azurerm#25102
`monitor`: updating to use the transport layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest`
[GH-#25102]&#xA;* `network`: updating to API Version `2023-09-01`
([#25095](hashicorp/terraform-provider-azurerm#25095
`azurerm_data_factory_integration_runtime_managed` - support for the
`credential_name` property
([#25033](hashicorp/terraform-provider-azurerm#25033
`azurerm_linux_function_app` - support for the `description` property in
the `ip_restriction` block
([#24527](hashicorp/terraform-provider-azurerm#24527
`azurerm_linux_function_app` - support for the
`ip_restriction_default_action` and `scm_ip_restriction_default_action`
properties
([#25131](hashicorp/terraform-provider-azurerm#25131
`azurerm_linux_function_app_slot` - support for the `description`
property in the `ip_restriction` block
([#24527](hashicorp/terraform-provider-azurerm#24527
`azurerm_linux_function_app_slot` - support for the
`ip_restriction_default_action` and `scm_ip_restriction_default_action`
properties
([#25131](hashicorp/terraform-provider-azurerm#25131
`azurerm_linux_web_app` - support for the `description` property in the
`ip_restriction` block
([#24527](hashicorp/terraform-provider-azurerm#24527
`azurerm_linux_web_app` - support for the
`ip_restriction_default_action` and `scm_ip_restriction_default_action`
properties
([#25131](hashicorp/terraform-provider-azurerm#25131
`azurerm_linux_web_app_slot` - support for the `description` property in
the `ip_restriction` block
([#24527](hashicorp/terraform-provider-azurerm#24527
`azurerm_linux_web_app_slot` - support for the
`ip_restriction_default_action` and `scm_ip_restriction_default_action`
properties
([#25131](hashicorp/terraform-provider-azurerm#25131
`azurerm_mysql_flexible_server` - setting the `storage.size_gb` property
to a smaller value now forces a new resource to be created
([#25074](hashicorp/terraform-provider-azurerm#25074
`azurerm_orbital_contact_profile` - changing the `channels` property no
longer creates a new resource
([#25129](hashicorp/terraform-provider-azurerm#25129
`azurerm_private_dns_resolver_inbound_endpoint` - the
`private_ip_address` property is no longer required when
`private_ip_allocation_method` is `Dynamic`
([#25035](hashicorp/terraform-provider-azurerm#25035
`stream_analytics_output_blob` - support for the `blob_write_mode`
property
([#25127](hashicorp/terraform-provider-azurerm#25127
`azurerm_windows_function_app` - support for the `description` property
in the `ip_restriction` block
([#24527](hashicorp/terraform-provider-azurerm#24527
`azurerm_windows_function_app` - support for the
`ip_restriction_default_action` and `scm_ip_restriction_default_action`
properties
([#25131](hashicorp/terraform-provider-azurerm#25131
`azurerm_windows_function_app_slot` - support for the `description`
property in the `ip_restriction` block
([#24527](hashicorp/terraform-provider-azurerm#24527
`azurerm_windows_function_app_slot` - support for the
`ip_restriction_default_action` and `scm_ip_restriction_default_action`
properties
([#25131](hashicorp/terraform-provider-azurerm#25131
`azurerm_windows_web_app` - support for the `description` property in
the `ip_restriction` block
([#24527](hashicorp/terraform-provider-azurerm#24527
`azurerm_windows_web_app` - support for the
`ip_restriction_default_action` and `scm_ip_restriction_default_action`
properties
([#25131](hashicorp/terraform-provider-azurerm#25131
`azurerm_windows_web_app_slot` - support for the `description` property
in the `ip_restriction` block
([#24527](hashicorp/terraform-provider-azurerm#24527
`azurerm_windows_web_app_slot` - support for the
`ip_restriction_default_action` and `scm_ip_restriction_default_action`
properties
([#25131](https://github.com/hashicorp/terraform-provider-azurerm/issues/25131))&#xA;&#xA;BUG
FIXES:&#xA;&#xA;* Data Source: `azurerm_function_app_host_keys` -
correctly set `event_grid_extension_key` by searching for the renamed
property in the API response
([#25108](hashicorp/terraform-provider-azurerm#25108
`azurerm_app_service_public_certificate` - fix issue where certificate
information was not being set correctly in the read
([#24943](hashicorp/terraform-provider-azurerm#24943
`azurerm_container_registry` - prevent recreation of the resource when
the `georeplication.tags` are updated
([#24994](hashicorp/terraform-provider-azurerm#24994
`azurerm_firewall_policy_rule_collection_group` - fix issue where the
client subscription ID was used to construct the `firewall_policy_id`
([#25145](hashicorp/terraform-provider-azurerm#25145
`azurerm_function_app_hybrid_connection` - fix issue where
`SendKeyValue` was not populated in the API payload
([#23761](hashicorp/terraform-provider-azurerm#23761
`azurerm_orbital_contact_profile` - fix creation of the resource when
`event_hub_uri` is not specified
([#25128](hashicorp/terraform-provider-azurerm#25128
`azurerm_recovery_services_vault` - prevent a panic when `immutability`
is updated
([#25132](hashicorp/terraform-provider-azurerm#25132
`azurerm_storage_account` - fix issue where the queue encryption key
type was set as the table encryption key type
([#25046](hashicorp/terraform-provider-azurerm#25046
`azurerm_web_app_hybrid_connection` - fix issue where `SendKeyValue` was
not populated in the API payload
([#23761](hashicorp/terraform-provider-azurerm#23761
`azurerm_mssql_database` - fix incorrect error due to typo when using
`restore_long_term_retention_backup_id`
([#25180](https://github.com/hashicorp/terraform-provider-azurerm/issues/25180))&#xA;&#xA;DEPRECATIONS:&#xA;&#xA;*
Deprecated Resource: `azurerm_static_site`
([#25117](hashicorp/terraform-provider-azurerm#25117
Deprecated Resource: `azurerm_static_site_custom_domain`
([#25117](hashicorp/terraform-provider-azurerm#25117
`azurerm_kubernetes_fleet_manager` - the `hub_profile` property has been
deprecated
([#25010](https://github.com/hashicorp/terraform-provider-azurerm/issues/25010))&#xA;&#xA;&#xA;</pre>
            </details>
        </details>
<a
href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/40/">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) <60776566+jenkins-infra-bot@users.noreply.github.com>
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 Apr 21, 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