Skip to content

[Compute] Make dependent resources of VM/VMSS support edge zone #18708

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.

Sign up for GitHub

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

Merged
jiasli merged 4 commits into from
Jul 2, 2021
Merged

[Compute] Make dependent resources of VM/VMSS support edge zone #18708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aa53906
Make dependent resources of VM/VMSS support edge zone
zhoxing-ms Jun 30, 2021
ae44877
Add check for edge zone
zhoxing-ms Jul 1, 2021
39c57d7
Add support for public IP
zhoxing-ms Jul 1, 2021
913bde3
Refactor help for edge zone
zhoxing-ms Jul 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/azure-cli/azure/cli/command_modules/vm/_params.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ def load_arguments(self, _):
c.argument('backend_port', help='When creating a new load balancer, backend port to open with NAT rules (Defaults to 22 on Linux and 3389 on Windows). When creating an application gateway, the backend port to use for the backend HTTP settings.', type=int)
c.argument('load_balancer', help='Name to use when creating a new load balancer (default) or referencing an existing one. Can also reference an existing load balancer by ID or specify "" for none.', options_list=['--load-balancer', '--lb'])
c.argument('load_balancer_sku', resource_type=ResourceType.MGMT_NETWORK, min_api='2017-08-01', options_list=['--lb-sku'], arg_type=get_enum_type(LoadBalancerSkuName),
help="Sku of the Load Balancer to create. Default to 'Standard' when single placement group is turned off; otherwise, default to 'Basic'")
help="Sku of the Load Balancer to create. Default to 'Standard' when single placement group is turned off; otherwise, default to 'Basic'. The public IP is supported to be created on edge zone only when it is 'Standard'")
c.argument('nat_pool_name', help='Name to use for the NAT pool when creating a new load balancer.', options_list=['--lb-nat-pool-name', '--nat-pool-name'])

with self.argument_context('vmss create', min_api='2017-03-30', arg_group='Network') as c:
Expand Down Expand Up @@ -819,7 +819,8 @@ def load_arguments(self, _):
c.argument('public_ip_address_dns_name', help='Globally unique DNS name for a newly created public IP.')
if self.supported_api_version(min_api='2017-08-01', resource_type=ResourceType.MGMT_NETWORK):
PublicIPAddressSkuName = self.get_models('PublicIPAddressSkuName', resource_type=ResourceType.MGMT_NETWORK)
c.argument('public_ip_sku', help='Public IP SKU. It is set to Basic by default.', default=None, arg_type=get_enum_type(PublicIPAddressSkuName))
c.argument('public_ip_sku', help='Public IP SKU. It is set to Basic by default. The public IP is supported to be created on edge zone only when it is \'Standard\'',
default=None, arg_type=get_enum_type(PublicIPAddressSkuName))
c.argument('nic_delete_option', nargs='+', min_api='2021-03-01',
help='Specify what happens to the network interface when the VM is deleted. Use a singular '
'value to apply on all resources, or use <Name>=<Value> to configure '
Expand Down
42 changes: 36 additions & 6 deletions src/azure-cli/azure/cli/command_modules/vm/_template_builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def build_output_deployment_resource(key, property_name, property_provider, prop
return deployment


def build_storage_account_resource(_, name, location, tags, sku):
def build_storage_account_resource(_, name, location, tags, sku, edge_zone=None):
storage_account = {
'type': 'Microsoft.Storage/storageAccounts',
'name': name,
Expand All @@ -75,10 +75,16 @@ def build_storage_account_resource(_, name, location, tags, sku):
'dependsOn': [],
'properties': {'accountType': sku}
}

if edge_zone:
storage_account['apiVersion'] = '2021-04-01'
storage_account['extendedLocation'] = edge_zone

return storage_account


def build_public_ip_resource(cmd, name, location, tags, address_allocation, dns_name, sku, zone, count=None):
def build_public_ip_resource(cmd, name, location, tags, address_allocation, dns_name, sku, zone, count=None,
edge_zone=None):
public_ip_properties = {'publicIPAllocationMethod': address_allocation}

if dns_name:
Expand Down Expand Up @@ -109,12 +115,18 @@ def build_public_ip_resource(cmd, name, location, tags, address_allocation, dns_

if sku and cmd.supported_api_version(ResourceType.MGMT_NETWORK, min_api='2017-08-01'):
public_ip['sku'] = {'name': sku}

# The edge zones are only built out using Standard SKU Public IPs
if edge_zone and sku.lower() == 'standard':
public_ip['apiVersion'] = '2021-02-01'
public_ip['extendedLocation'] = edge_zone

return public_ip


def build_nic_resource(_, name, location, tags, vm_name, subnet_id, private_ip_address=None,
nsg_id=None, public_ip_id=None, application_security_groups=None, accelerated_networking=None,
count=None):
count=None, edge_zone=None):
private_ip_allocation = 'Static' if private_ip_address else 'Dynamic'
ip_config_properties = {
'privateIPAllocationMethod': private_ip_allocation,
Expand Down Expand Up @@ -172,6 +184,10 @@ def build_nic_resource(_, name, location, tags, vm_name, subnet_id, private_ip_a
'count': count
}

if edge_zone:
nic['extendedLocation'] = edge_zone
nic['apiVersion'] = '2021-02-01'
Copy link
Contributor

@Juliehzl Juliehzl Jul 2, 2021

Choose a reason for hiding this comment

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

hard-coded api version?

Copy link
Contributor

@Juliehzl Juliehzl Jul 2, 2021

Choose a reason for hiding this comment

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

should we get specified api version from resource type and check if the api version is supported?

Copy link
Contributor Author

@zhoxing-ms zhoxing-ms Jul 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Good question! I've thought about this, and my personal opinion is:

Before, the api-versions of dependent resources on are fixed and independent, so it might be better to keep the api-versions which support edge zone independent.
Otherwise, there will be several problems:

  1. If the api-version of dependent resources is upgraded, the VM related tests need to be recorded, which increases the coupling.
  2. The api-version used by dependent resources may tend to be stable and fixed. I think this is the reason why the previous api-version used by dependent resources is the lowest version. code link
  3. If other profiles also need to support edge zone in the future, if the api-versions of their dependent resources have not been upgraded in that profile, but we need to give higher priority to support VM and VMSS, then we can only bump version for all the dependent resources together. This workload is very heavy and it is not conducive to split.

In addition, similar situations have been dealt with in this way before: code link

As for why not add the check for api-version, it is because --edge-zone has the api-version control min_api='2020-12-01' code link

That’s just my two cents, do you have any ideas or suggestions for this?

Copy link
Contributor

@Juliehzl Juliehzl Jul 2, 2021

Choose a reason for hiding this comment

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

LGTM then


return nic


Expand Down Expand Up @@ -213,7 +229,7 @@ def build_nsg_resource(_, name, location, tags, nsg_rule):


def build_vnet_resource(_, name, location, tags, vnet_prefix=None, subnet=None,
subnet_prefix=None, dns_servers=None):
subnet_prefix=None, dns_servers=None, edge_zone=None):
vnet = {
'name': name,
'type': 'Microsoft.Network/virtualNetworks',
Expand All @@ -236,6 +252,10 @@ def build_vnet_resource(_, name, location, tags, vnet_prefix=None, subnet=None,
'addressPrefix': subnet_prefix
}
}]
if edge_zone:
vnet['extendedLocation'] = edge_zone
vnet['apiVersion'] = '2021-02-01'

return vnet


Expand Down Expand Up @@ -675,7 +695,7 @@ def _ag_subresource_id(_type, name):

def build_load_balancer_resource(cmd, name, location, tags, backend_pool_name, nat_pool_name,
backend_port, frontend_ip_name, public_ip_id, subnet_id, private_ip_address,
private_ip_allocation, sku, instance_count, disable_overprovision):
private_ip_allocation, sku, instance_count, disable_overprovision, edge_zone=None):
lb_id = "resourceId('Microsoft.Network/loadBalancers', '{}')".format(name)

frontend_ip_config = _build_frontend_ip_config(frontend_ip_name, public_ip_id,
Expand Down Expand Up @@ -736,10 +756,15 @@ def build_load_balancer_resource(cmd, name, location, tags, backend_pool_name, n
"idleTimeoutInMinutes": 5,
}
}]

if edge_zone:
lb['apiVersion'] = '2021-02-01'
lb['extendedLocation'] = edge_zone

return lb


def build_vmss_storage_account_pool_resource(_, loop_name, location, tags, storage_sku):
def build_vmss_storage_account_pool_resource(_, loop_name, location, tags, storage_sku, edge_zone=None):

storage_resource = {
'type': 'Microsoft.Storage/storageAccounts',
Expand All @@ -755,6 +780,11 @@ def build_vmss_storage_account_pool_resource(_, loop_name, location, tags, stora
'accountType': storage_sku
}
}

if edge_zone:
storage_resource['apiVersion'] = '2021-04-01'
storage_resource['extendedLocation'] = edge_zone

return storage_resource


Expand Down
18 changes: 9 additions & 9 deletions src/azure-cli/azure/cli/command_modules/vm/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ def create_vm(cmd, vm_name, resource_group_name, image=None, size='Standard_DS1_
hash_string(vm_id, length=14, force_lower=True))
vm_dependencies.append('Microsoft.Storage/storageAccounts/{}'.format(storage_account))
master_template.add_resource(build_storage_account_resource(cmd, storage_account, location,
tags, storage_sku))
tags, storage_sku, edge_zone))

nic_name = None
if nic_type == 'new':
Expand Down Expand Up @@ -836,8 +836,8 @@ def create_vm(cmd, vm_name, resource_group_name, image=None, size='Standard_DS1_
if not vnet_exists:
vnet_name = vnet_name or '{}VNET'.format(vm_name)
nic_dependencies.append('Microsoft.Network/virtualNetworks/{}'.format(vnet_name))
master_template.add_resource(build_vnet_resource(
cmd, vnet_name, location, tags, vnet_address_prefix, subnet, subnet_address_prefix))
master_template.add_resource(build_vnet_resource(cmd, vnet_name, location, tags, vnet_address_prefix,
subnet, subnet_address_prefix, edge_zone=edge_zone))

if nsg_type == 'new':
if nsg_rule is None:
Expand All @@ -856,7 +856,7 @@ def create_vm(cmd, vm_name, resource_group_name, image=None, size='Standard_DS1_
master_template.add_resource(build_public_ip_resource(cmd, public_ip_address, location, tags,
public_ip_address_allocation,
public_ip_address_dns_name,
public_ip_sku, zone, count))
public_ip_sku, zone, count, edge_zone))

subnet_id = subnet if is_valid_resource_id(subnet) else \
'{}/virtualNetworks/{}/subnets/{}'.format(network_id_template, vnet_name, subnet)
Expand Down Expand Up @@ -895,7 +895,7 @@ def create_vm(cmd, vm_name, resource_group_name, image=None, size='Standard_DS1_
nic_resource = build_nic_resource(
cmd, nic_name, location, tags, vm_name, subnet_id, private_ip_address, nsg_id,
public_ip_address_id, application_security_groups, accelerated_networking=accelerated_networking,
count=count)
count=count, edge_zone=edge_zone)
nic_resource['dependsOn'] = nic_dependencies
master_template.add_resource(nic_resource)
else:
Expand Down Expand Up @@ -2513,7 +2513,7 @@ def create_vmss(cmd, vmss_name, resource_group_name, image=None,
subnet = subnet or '{}Subnet'.format(vmss_name)
vmss_dependencies.append('Microsoft.Network/virtualNetworks/{}'.format(vnet_name))
vnet = build_vnet_resource(
cmd, vnet_name, location, tags, vnet_address_prefix, subnet, subnet_address_prefix)
cmd, vnet_name, location, tags, vnet_address_prefix, subnet, subnet_address_prefix, edge_zone=edge_zone)
if app_gateway_type:
vnet['properties']['subnets'].append({
'name': 'appGwSubnet',
Expand Down Expand Up @@ -2556,7 +2556,7 @@ def _get_public_ip_address_allocation(value, sku):
master_template.add_resource(build_public_ip_resource(
cmd, public_ip_address, location, tags,
_get_public_ip_address_allocation(public_ip_address_allocation, load_balancer_sku),
public_ip_address_dns_name, load_balancer_sku, zones))
public_ip_address_dns_name, load_balancer_sku, zones, edge_zone=edge_zone))
public_ip_address_id = '{}/publicIPAddresses/{}'.format(network_id_template,
public_ip_address)

Expand All @@ -2569,7 +2569,7 @@ def _get_public_ip_address_allocation(value, sku):
cmd, load_balancer, location, tags, backend_pool_name, nat_pool_name, backend_port,
'loadBalancerFrontEnd', public_ip_address_id, subnet_id, private_ip_address='',
private_ip_allocation='Dynamic', sku=load_balancer_sku, instance_count=instance_count,
disable_overprovision=disable_overprovision)
disable_overprovision=disable_overprovision, edge_zone=edge_zone)
lb_resource['dependsOn'] = lb_dependencies
master_template.add_resource(lb_resource)

Expand Down Expand Up @@ -2615,7 +2615,7 @@ def _get_public_ip_address_allocation(value, sku):
# create storage accounts if needed for unmanaged disk storage
if storage_profile == StorageProfile.SAPirImage:
master_template.add_resource(build_vmss_storage_account_pool_resource(
cmd, 'storageLoop', location, tags, storage_sku))
cmd, 'storageLoop', location, tags, storage_sku, edge_zone))
master_template.add_variable('storageAccountNames', [
'{}{}'.format(naming_prefix, x) for x in range(5)
])
Expand Down
Loading