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

loadbalancer: refactoring to use hashicorp/go-azure-sdk #24291

Merged
merged 2 commits into from Jan 26, 2024

Conversation

sinbai
Copy link
Contributor

@sinbai sinbai commented Dec 20, 2023

  • Upgrade loadbalancer to use hashicorp/go-azure-sdk and upgrade the API version to 2023-06-01.
    (Because this PR is upgraded from track1 SDK to pandora SDK, the upgraded SDK and API version are combined into one PR. )

  • Remove the computed and add default value Tcp for property protocol of resource azurerm_lb_probe
    (Because the following error is caused by using Pandora SDK, the fix and the SDK upgrade are merged into one PR. )

    This is because some test cases of resource azurerm_lb_probe failed with the following error after upgrading SDK:

    { "error":{ "code":"InvalidRequestFormat", "message":"Cannot parse the request.", "details":[ { "code":"InvalidJson", "message":"Error converting value \"\" to type 'Microsoft.WindowsAzure.Networking.Nrp.Frontend.Contract.Csm.Public.Protocol'. Path 'properties.probes[0].properties.protocol', line 1, position 1257." } ] } }

The above error occurs when the property protocol is not specified in tf config. In this case, in the API request, the protocol is set to string empty, so the API returns the above error.

After verifying the API behavior, it was found that the default value of the protocol is Tcp. So, removing the computed and add default value for protocol to fix the tests failure. See test results below for more details.

  1. sku = "Basic"
    PUT /subscriptions/xxx/resourceGroups/acctestRG-lb-test/providers/Microsoft.Network/loadBalancers/arm-test-loadbalancer-1222

Request:
{
   ...
      "probes":[
         {
            "name":"probe-1222",
            "properties":{
               "intervalInSeconds":15,
               "numberOfProbes":2,
               "port":80,
               "probeThreshold":1
            }
         }
      ]
   },
   "sku":{
      "name":"Basic",
      "tier":"Regional"
   },
   "tags":{
      
   }
}
Response:
{
  "name": "arm-test-loadbalancer-1222",
    ...
    "probes": [
      {
        "name": "probe-1222",
        "id": "/subscriptions/xxx/resourceGroups/acctestRG-lb-test/providers/Microsoft.Network/loadBalancers/arm-test-loadbalancer-1222/probes/probe-1222",
        "etag": "W/\"600e0103-42ed-4bce-bce9-8f9964409bed\"",
        "properties": {
          "provisioningState": "Succeeded",
          "protocol": "Tcp",
          "port": 80,
          "intervalInSeconds": 15,
          "numberOfProbes": 2,
          "probeThreshold": 1
        },
        "type": "Microsoft.Network/loadBalancers/probes"
      }
    ],
    "inboundNatRules": [],
    "inboundNatPools": []
  },
  "sku": {
    "name": "Basic",
    "tier": "Regional"
  }
}

  1. sku = "Standard"
    PUT /subscriptions/xxx/resourceGroups/acctestRG-lb-1222-5/providers/Microsoft.Network/loadBalancers/acctest-loadbalancer-1222-2
Request:	
{
	     ...
	      "probes":[
	         {
	            "name":"probe-1222",
	            "properties":{
	               "intervalInSeconds":15,
	               "numberOfProbes":2,
	               "port":20,
	               "probeThreshold":1
	            }
	         }
	      ]
	   },
	   "sku":{
	      "name":"Standard",
	      "tier":"Regional"
	   },
	   "tags":{
	      
	   }
	}
Response:
{
	  "name": "acctest-loadbalancer-1222-2",
	   ...
	    "probes": [
	      {
	        "name": "probe-1222",
	        "id": "/subscriptions/xxx/resourceGroups/acctestRG-lb-1222-5/providers/Microsoft.Network/loadBalancers/acctest-loadbalancer-1222-2/probes/probe-1222",
	        "etag": "W/\"ed047950-1768-4aa5-a265-ae4a4f9b2c41\"",
	        "properties": {
	          "provisioningState": "Succeeded",
	          "protocol": "Tcp",
	          "port": 20,
	          "intervalInSeconds": 15,
	          "numberOfProbes": 2,
	          "probeThreshold": 1
	        },
	        "type": "Microsoft.Network/loadBalancers/probes"
	      }
	    ],
	    "inboundNatRules": [],
	    "outboundRules": [],
	    "inboundNatPools": []
	  },
	  "sku": {
	    "name": "Standard",
	    "tier": "Regional"
	  }
	}
  1. sku = "Standard"
    PUT /subscriptions/xxx/resourceGroups/acctestRG-lb-1222-5/providers/Microsoft.Network/loadBalancers/acctestlb-1222-3
Request:
{
	      ...
	      "probes":[
	         {
	            "name":"probe-1223",
	            "properties":{
	               "intervalInSeconds":15,
	               "numberOfProbes":2,
	               "port":20,
	               "probeThreshold":1
	            }
	         }
	      ]
	   },
	   "sku":{
	      "name":"Gateway",
	      "tier":"Regional"
	   },
	   "tags":{
	      
	   }
	}
Response:
{
	     ...
	    "probes": [
	      {
	        "name": "probe-1223",
	        "id": "/subscriptions/xxx/resourceGroups/acctestRG-lb-1222-5/providers/Microsoft.Network/loadBalancers/acctestlb-1222-3/probes/probe-1223",
	        "etag": "W/\"5b43fd7a-3416-4d67-81bd-eb7a86938b03\"",
	        "properties": {
	          "provisioningState": "Updating",
	          "protocol": "Tcp",
	          "port": 20,
	          "intervalInSeconds": 15,
	          "numberOfProbes": 2,
	          "probeThreshold": 1
	        },
	        "type": "Microsoft.Network/loadBalancers/probes"
	      }
	    ],
	    "inboundNatRules": [],
	    "outboundRules": [],
	    "inboundNatPools": []
	  },
	  "sku": {
	    "name": "Gateway",
	    "tier": "Regional"
	  }
}

image

The above two failing tests have been fixed in the PR.

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks for this @sinbai. Would you mind rebasing this? (due to the changes we made in the go-azure-sdk module structure)

@sinbai
Copy link
Contributor Author

sinbai commented Jan 26, 2024

Thanks for this @sinbai. Would you mind rebasing this? (due to the changes we made in the go-azure-sdk module structure)

Thank you for your reminder @stephybun , already rebased.

Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks @sinbai LGTM 👍

@stephybun stephybun merged commit bfe334a into hashicorp:main Jan 26, 2024
33 checks passed
@github-actions github-actions bot added this to the v3.90.0 milestone Jan 26, 2024
stephybun added a commit that referenced this pull request Jan 26, 2024
rizkybiz pushed a commit to rizkybiz/terraform-provider-azurerm that referenced this pull request Jan 30, 2024
rizkybiz pushed a commit to rizkybiz/terraform-provider-azurerm that referenced this pull request Jan 30, 2024
dduportal pushed a commit to jenkins-infra/azure that referenced this pull request Feb 5, 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.89.0&#34; to &#34;3.90.0&#34; in file
&#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>3.90.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.90.0&#xA;UPGRADE
NOTES:&#xA;&#xA;* provider - The provider will now automatically
register the `AppConfiguration`, `DataFactory`, and `SignalRService`
Resource Providers. When running Terraform with limited permissions,
note that you [must disable automatic Resource Provider Registration and
ensure that any Resource Providers Terraform requires are
registered]([XXX](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs#skip_provider_registration)).
([#24645](https://github.com/hashicorp/terraform-provider-azurerm/issues/24645))&#xA;
&#xA;FEATURES:&#xA;&#xA;* **New Data Source**:
`azurerm_nginx_configuration`
([#24642](https://github.com/hashicorp/terraform-provider-azurerm/issues/24642))&#xA;*
**New Data Source**: `azurerm_virtual_desktop_workspace`
([#24732](https://github.com/hashicorp/terraform-provider-azurerm/issues/24732))&#xA;*
**New Resource**: `azurerm_kubernetes_fleet_update_strategy`
([#24328](https://github.com/hashicorp/terraform-provider-azurerm/issues/24328))&#xA;*
**New Resource**: `azurerm_site_recovery_vmware_replicated_vm`
([#22477](https://github.com/hashicorp/terraform-provider-azurerm/issues/22477))&#xA;*
**New Resource**:
`azurerm_spring_cloud_new_relic_application_performance_monitoring`
([#24699](https://github.com/hashicorp/terraform-provider-azurerm/issues/24699))&#xA;&#xA;ENHANCEMENTS:&#xA;&#xA;*
provider: registering the Resource Provider `Microsoft.AppConfiguration`
([#24645](https://github.com/hashicorp/terraform-provider-azurerm/issues/24645))&#xA;*
provider: registering the Resource Provider `Microsoft.DataFactory`
([#24645](https://github.com/hashicorp/terraform-provider-azurerm/issues/24645))&#xA;*
provider: registering the Resource Provider `Microsoft.SignalRService`
([#24645](https://github.com/hashicorp/terraform-provider-azurerm/issues/24645))&#xA;*
provider: the Provider is now built using Go 1.21.6
([#24653](https://github.com/hashicorp/terraform-provider-azurerm/issues/24653))&#xA;*
dependencies: the dependency `github.com/hashicorp/go-azure-sdk` has
been split into multiple Go Modules - and as such will be referred to by
those paths going forwards
([#24636](https://github.com/hashicorp/terraform-provider-azurerm/issues/24636))&#xA;*
dependencies: updating to ``v0.20240201.1064937` of
`github.com/hashicorp/go-azure-sdk/resource-manager`
([#24738](https://github.com/hashicorp/terraform-provider-azurerm/issues/24738))&#xA;*
dependencies: updating to `v0.20240201.1064937` of
`github.com/hashicorp/go-azure-sdk/sdk`
([#24738](https://github.com/hashicorp/terraform-provider-azurerm/issues/24738))&#xA;*
`appservice`: update to `go-azure-sdk` and API version `2023-01-01`
([#24688](https://github.com/hashicorp/terraform-provider-azurerm/issues/24688))&#xA;*
`datafactory`: updating to use `tombuildsstuff/kermit`
([#24675](https://github.com/hashicorp/terraform-provider-azurerm/issues/24675))&#xA;*
`hdinsight`: refactoring to use
`github.com/hashicorp/go-azure-sdk/resource-manager`
([#24011](https://github.com/hashicorp/terraform-provider-azurerm/issues/24011))&#xA;*
`hdinsight`: updating to API Version `2021-06-01`
([#24011](https://github.com/hashicorp/terraform-provider-azurerm/issues/24011))&#xA;*
`loadbalancer`: updating to use `hashicorp/go-azure-sdk`
([#24291](https://github.com/hashicorp/terraform-provider-azurerm/issues/24291))&#xA;*
`nginx`: updating to API Version `2023-09-01`
([#24640](https://github.com/hashicorp/terraform-provider-azurerm/issues/24640))&#xA;*
`servicefabricmanagedcluster`: updating to use the base layer from
`hashicorp/go-azure-sdk` rather than `Azure/go-autorest`
([#24654](https://github.com/hashicorp/terraform-provider-azurerm/issues/24654))&#xA;*
`springcloud`: updating to use API Version `2023-11-01-preview`
([#24690](https://github.com/hashicorp/terraform-provider-azurerm/issues/24690))&#xA;*
`subscriptions`: refactoring to use `hashicorp/go-azure-sdk`
([#24663](https://github.com/hashicorp/terraform-provider-azurerm/issues/24663))&#xA;*
Data Source: `azurerm_stream_analytics_job` - support for User Assigned
Identities
([#24738](https://github.com/hashicorp/terraform-provider-azurerm/issues/24738))&#xA;*
`azurerm_cosmosdb_account` - support for the `gremlin_database` and
`tables_to_restore` properties
([#24627](https://github.com/hashicorp/terraform-provider-azurerm/issues/24627))&#xA;*
`azurerm_bot_channel_email` - support for the `magic_code` property
([#23129](https://github.com/hashicorp/terraform-provider-azurerm/issues/23129))&#xA;*
`azurerm_cosmosdb_account` - support for the `partition_merge_enabled`
property
([#24615](https://github.com/hashicorp/terraform-provider-azurerm/issues/24615))&#xA;*
`azurerm_mssql_managed_database` - support for the
`immutable_backups_enabled` property
([#24745](https://github.com/hashicorp/terraform-provider-azurerm/issues/24745))&#xA;*
`azurerm_mssql_database` - support for the `immutable_backups_enabled`
property
([#24745](https://github.com/hashicorp/terraform-provider-azurerm/issues/24745))&#xA;*
`azurerm_palo_alto_next_generation_firewall_virtual_hub_panorama` -
support for the `trusted_address_ranges` property
([#24459](https://github.com/hashicorp/terraform-provider-azurerm/issues/24459))&#xA;*
`azurerm_palo_alto_next_generation_firewall_virtual_network_local_rulestack`
- support for the `trusted_address_ranges` property
([#24459](https://github.com/hashicorp/terraform-provider-azurerm/issues/24459))&#xA;*
`azurerm_palo_alto_next_generation_firewall_virtual_network_panorama` -
support for the `trusted_address_ranges` property
([#24459](https://github.com/hashicorp/terraform-provider-azurerm/issues/24459))&#xA;*
`azurerm_servicebus_namespace` - updating to use API Version
`2022-10-01-preview`
([#24650](https://github.com/hashicorp/terraform-provider-azurerm/issues/24650))&#xA;*
`azurerm_spring_cloud_api_portal` - support for the
`api_try_out_enabled` property
([#24696](https://github.com/hashicorp/terraform-provider-azurerm/issues/24696))&#xA;*
`azurerm_spring_cloud_gateway` - support for the
`local_response_cache_per_route` and `local_response_cache_per_instance`
properties
([#24697](https://github.com/hashicorp/terraform-provider-azurerm/issues/24697))&#xA;*
`azurerm_stream_analytics_job` - support for User Assigned Identities
([#24738](https://github.com/hashicorp/terraform-provider-azurerm/issues/24738))&#xA;*
`azurerm_subscription` - refactoring to use `hashicorp/go-azure-sdk` to
set tags on the subscription
([#24734](https://github.com/hashicorp/terraform-provider-azurerm/issues/24734))&#xA;*
`azurerm_virtual_desktop_workspace` - correctly validate the `name`
property
([#24668](https://github.com/hashicorp/terraform-provider-azurerm/issues/24668))&#xA;&#xA;&#xA;BUG
FIXES:&#xA;&#xA;* provider: skip registration for resource providers
that are unavailable
([#24571](https://github.com/hashicorp/terraform-provider-azurerm/issues/24571))&#xA;*
`azurerm_app_configuration` - no longer require
`lifecycle_ignore_changes` for the `value` property when using a key
vault reference
([#24702](https://github.com/hashicorp/terraform-provider-azurerm/issues/24702))&#xA;*
`azurerm_app_service_managed_certificate` - fix casing issue in
`app_service_plan_id` by parsing insensitively
([#24664](https://github.com/hashicorp/terraform-provider-azurerm/issues/24664))&#xA;*
`azurerm_cognitive_deployment` - updates now include the `version`
property
([#24700](https://github.com/hashicorp/terraform-provider-azurerm/issues/24700))&#xA;*
`azurerm_dns_cname_record` - prevent casing issue in
`target_resource_id` by parsing the ID insensitively
([#24181](https://github.com/hashicorp/terraform-provider-azurerm/issues/24181))&#xA;*
`azurerm_mssql_managed_instance_failover_group` - prevent an issue when
trying to create a failover group with a managed instance from a
different subscription
([#24646](https://github.com/hashicorp/terraform-provider-azurerm/issues/24646))&#xA;*
`azurerm_storage_account` - conditionally update properties only when
needed
([#24669](https://github.com/hashicorp/terraform-provider-azurerm/issues/24669))&#xA;*
`azurerm_storage_account` - change update order for `access_tier`to
prevent errors when uploading blobs to the archive tier
([#22250](https://github.com/hashicorp/terraform-provider-azurerm/issues/22250))&#xA;&#xA;&#xA;</pre>
            </details>
        </details>
<a
href="https://infra.ci.jenkins.io/job/terraform-jobs/job/azure/job/main/1075/">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>
@sinbai sinbai deleted the loadbalancer/upgrade-sdk branch March 28, 2024 02:42
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 29, 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.

None yet

3 participants