[Network] Migrate network to track2 SDK #16245

jsntcy · 2020-12-11T09:28:29Z

Description
Here are main changes for Network migration to track2:

LRO operations are renamed to begin_xxx_xxx, for example in track1, the name is create_or_update; while in track2, the name is begin_create_or_update.
For generic update of LRO, set setter_name to begin_create_or_update.
In track2, SDK parameters are not flattened, so we need pass object parameter instead of flattened parameters to SDK.
In track2, use HttpResponseError instead of CloudError.
Some model names are changed, for example, ipv4 to I_PV4.
Fix an issue related to LRO generic update when using --no-wait.
Fix some tests due to changes in track2.
Run live tests and re-record tests.

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.

This checklist is used to make sure that common guidelines for a pull request are followed.

The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.

yonzhan · 2020-12-11T10:51:07Z

Network

jsntcy · 2020-12-18T01:37:17Z

/azp run

azure-pipelines · 2020-12-18T01:37:34Z

Azure Pipelines successfully started running 2 pipeline(s).

jiasli · 2020-12-18T03:00:50Z

src/azure-cli-core/azure/cli/core/commands/__init__.py

@@ -172,7 +172,7 @@ def _resolve_model(self):

        doc_string = doc_string.replace('\r', '').replace('\n', ' ')
        doc_string = re.sub(' +', ' ', doc_string)
-        model_name_regex = re.compile(r':return: (.*that returns )?(?P<model>[a-zA-Z]*)')
+        model_name_regex = re.compile(r':return: (?:.*?that returns (?:either )?)?(?P<model>[a-zA-Z]*)')


This is dark magic. Consider adding more comments.

Good catch. Originally I added comments, it seems when I resolved conflicts manually after rebasing dev, the comments was removed accidentally. I'll add comments here to explain why this change happened.

Could add more annotation or example? The regex is not clear.

kairu-ms · 2020-12-18T03:08:27Z

src/azure-cli/azure/cli/command_modules/network/custom.py

@@ -2904,7 +2915,7 @@ def create_private_endpoint(cmd, resource_group_name, private_endpoint_name, sub
    else:
        private_endpoint.private_link_service_connections = [pls_connection]

-    return client.create_or_update(resource_group_name, private_endpoint_name, private_endpoint)
+    return client.begin_create_or_update(resource_group_name, private_endpoint_name, private_endpoint)


Do we need support no_wait for this long running operation?

Yes, usually LRO need support no_wait, but actually some of our code doesn't follow this guideline.
In this PR, we just focus on migration and keep the same behavior as before.
For improvement, we can do it in future PRs.

jsntcy · 2020-12-18T06:02:58Z

src/azure-cli/azure/cli/command_modules/network/commands.py

@@ -472,6 +472,7 @@ def _make_singular(value):
            g.command('delete', delete_network_resource_property_entry('application_gateways', subresource), supports_no_wait=True)
            g.custom_command('create', 'create_ag_{}'.format(_make_singular(subresource)), supports_no_wait=True, validator=create_validator)
            g.generic_update_command('update', command_type=network_ag_sdk, supports_no_wait=True,
+                                     setter_name='begin_create_or_update',


If the setter name is create_or_update (track1), we don't need provide it.

jsntcy · 2020-12-18T06:03:38Z

src/azure-cli/azure/cli/command_modules/network/commands.py

@@ -564,7 +568,7 @@ def _make_singular(value):
                            client_factory=cf_app_gateway_waf_policy,
                            min_api='2018-12-01') as g:
        g.custom_command('create', 'create_ag_waf_policy')
-        g.command('delete', 'delete')
+        g.command('delete', 'begin_delete')


For LRO, the name is changed from xxx to begin_xxx.

jsntcy · 2020-12-18T06:10:24Z

src/azure-cli/azure/cli/command_modules/network/custom.py


    # if VNETs specified, have to create the protection plan and then add the VNETs
    plan_id = LongRunningOperation(cmd.cli_ctx)(
-        ddos_client.create_or_update(resource_group_name, ddos_plan_name, location=location, tags=tags)).id
+        ddos_client.begin_create_or_update(resource_group_name, ddos_plan_name, parameters=ddos_protection_plan)).id


In track2, SDK parameters are not flattened, so we need pass object parameter instead of flattened parameters to SDK.

jsntcy · 2020-12-18T06:16:29Z

src/azure-cli/azure/cli/command_modules/network/custom.py

@@ -2807,7 +2818,7 @@ def assign_express_route_port_identity(cmd, resource_group_name, express_route_p
    ports = client.get(resource_group_name, express_route_port_name)

    ManagedServiceIdentity, ManagedServiceIdentityUserAssignedIdentitiesValue = \
-        cmd.get_models('ManagedServiceIdentity', 'ManagedServiceIdentityUserAssignedIdentitiesValue')
+        cmd.get_models('ManagedServiceIdentity', 'Components1Jq1T4ISchemasManagedserviceidentityPropertiesUserassignedidentitiesAdditionalproperties')  # pylint: disable=line-too-long


Components1Jq1T4ISchemasManagedserviceidentityPropertiesUserassignedidentitiesAdditionalproperties [](start = 50, length = 98)

It's an autorest/swagger issue, and will fix it future PR once the SDK is fixed.

jsntcy · 2020-12-18T06:19:01Z

src/azure-cli/azure/cli/command_modules/network/custom.py

    # legacy implementation
-    return _legacy_generate_vpn_client(client, resource_group_name, virtual_network_gateway_name, params, polling=ARMPolling(lro_options={'final-state-via': 'location'}))
+    return client.begin_generatevpnclientpackage(resource_group_name, virtual_network_gateway_name, params)


Use SDK method directly now and no need to write own method to call rest api.

jsntcy · 2020-12-18T06:34:20Z

src/azure-cli/azure/cli/command_modules/network/_params.py

@@ -899,7 +899,7 @@ def load_arguments(self, _):
        c.argument('floating_ip', help='Enable floating IP.', arg_type=get_three_state_flag())
        c.argument('idle_timeout', help='Idle timeout in minutes.', type=int)
        c.argument('protocol', help='Network transport protocol.', arg_type=get_enum_type(TransportProtocol))
-        c.argument('private_ip_address_version', min_api='2019-04-01', help='The private IP address version to use.', default=IPVersion.ipv4.value if IPVersion else '')
+        c.argument('private_ip_address_version', min_api='2019-04-01', help='The private IP address version to use.', default=IPVersion.I_PV4.value if IPVersion else '')


name change by track2 SDK.

jsntcy · 2020-12-18T06:38:42Z

scripts/sdk_process/patch_models.py

@@ -234,7 +234,8 @@ def find_autorest_generated_folder(module_prefix="azure.mgmt"):
    track2_packages = [
        'azure.mgmt.keyvault',
        'azure.mgmt.storage',
-        'azure.mgmt.compute'
+        'azure.mgmt.compute',
+        'azure.mgmt.network'


Add package here to pass CI.

kairu-ms · 2020-12-18T06:46:55Z

src/azure-cli/azure/cli/command_modules/network/custom.py

    vnet_gateway = VirtualNetworkGateway(
        gateway_type=gateway_type, vpn_type=vpn_type, vpn_gateway_generation=vpn_gateway_generation, location=location,
-        tags=tags, sku=VirtualNetworkGatewaySku(name=sku, tier=sku), active_active=active_active, ip_configurations=[],
+        tags=tags, sku=VirtualNetworkGatewaySku(name=sku, tier=sku), active=active, ip_configurations=[],


Why property name changed? Is it normal for Track 2 SDK?

No, most are not changed. I think it's due to track2 codegen logic change.

kairu-ms · 2020-12-18T07:03:54Z

src/azure-cli/azure/cli/command_modules/network/tests/latest/test_network_commands.py

+        # self.cmd('network application-gateway show -g {rg} -n {gateway}',
+        #          checks=self.check('length(backendHttpSettingsCollection[1].trustedRootCertificates)', 2))
+        # self.cmd('network application-gateway http-settings update -g {rg} --gateway-name {gateway} -n {settings} --no-wait')


Why mark these lines as comments?

Good catch, comment out accidentally and will uncomment and record this test.

jiasli · 2021-02-01T13:19:02Z

src/azure-cli/azure/cli/command_modules/network/custom.py

@@ -6008,15 +6036,15 @@ def create_vnet_peering(cmd, resource_group_name, virtual_network_name, virtual_
        use_remote_gateways=use_remote_gateways)
    aux_subscription = parse_resource_id(remote_virtual_network)['subscription']
    ncf = network_client_factory(cmd.cli_ctx, aux_subscriptions=[aux_subscription])


aux_subscriptions will stop working for Track 2 SDKs due to SDK limitation Azure/azure-sdk-for-python#8313.

jsntcy self-assigned this Dec 11, 2020

yonzhan requested review from kairu-ms, msyyc and haroldrandom December 11, 2020 10:51

yonzhan added this to the S180 milestone Dec 11, 2020

yonzhan requested review from Juliehzl and jiasli December 11, 2020 10:53

jsntcy force-pushed the migrate-network-track2-final branch from bf64ff0 to 46d0a72 Compare December 14, 2020 11:20

jsntcy marked this pull request as ready for review December 15, 2020 10:48

jsntcy requested review from bim-msft, evelyn-ys, fengzhou-msft, houk-ms, qwordy, yungezz and zhoxing-ms as code owners December 15, 2020 10:48

[Network] Migrate network to track2 SDK

0da192d

jsntcy force-pushed the migrate-network-track2-final branch from c3917d5 to 0da192d Compare December 17, 2020 07:40

jiasli reviewed Dec 18, 2020

View reviewed changes

kairu-ms reviewed Dec 18, 2020

View reviewed changes

msyyc requested a review from kairu-ms December 18, 2020 03:21

Add comments to regular expression change

20c7f60

jsntcy commented Dec 18, 2020

View reviewed changes

Remove useless argument

8fbf915

kairu-ms reviewed Dec 18, 2020

View reviewed changes

Uncomment test

e68f95c

kairu-ms approved these changes Dec 18, 2020

View reviewed changes

yonzhan merged commit e4906ce into Azure:dev Dec 20, 2020

jiasli reviewed Feb 1, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Network] Migrate network to track2 SDK #16245

[Network] Migrate network to track2 SDK #16245

jsntcy commented Dec 11, 2020 •

edited

Loading

yonzhan commented Dec 11, 2020

jsntcy commented Dec 18, 2020

azure-pipelines bot commented Dec 18, 2020

jiasli Dec 18, 2020 •

edited

Loading

jsntcy Dec 18, 2020

msyyc Dec 18, 2020

kairu-ms Dec 18, 2020

jsntcy Dec 18, 2020

jsntcy Dec 18, 2020

jsntcy Dec 18, 2020

jsntcy Dec 18, 2020

jsntcy Dec 18, 2020

jsntcy Dec 18, 2020

jsntcy Dec 18, 2020

jsntcy Dec 18, 2020

kairu-ms Dec 18, 2020

jsntcy Dec 18, 2020

kairu-ms Dec 18, 2020

jsntcy Dec 18, 2020

jiasli Feb 1, 2021

[Network] Migrate network to track2 SDK #16245

[Network] Migrate network to track2 SDK #16245

Conversation

jsntcy commented Dec 11, 2020 • edited Loading

yonzhan commented Dec 11, 2020

jsntcy commented Dec 18, 2020

azure-pipelines bot commented Dec 18, 2020

jiasli Dec 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsntcy commented Dec 11, 2020 •

edited

Loading

jiasli Dec 18, 2020 •

edited

Loading