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.

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

[Backup] az backup restore restore-disks: Add support for disk restore in edge-zone backups and support for TWN + TNW CRR #28150

Merged
merged 11 commits into from
Jan 22, 2024
Merged

[Backup] az backup restore restore-disks: Add support for disk restore in edge-zone backups and support for TWN + TNW CRR #28150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
87587b7
edge zones support, untested
Dec 13, 2023
9212bbf
Edge zones: tested + added error messages
Jan 8, 2024
a0c9b3a
fixed style, added paired region for twn and tnw
Jan 8, 2024
a485348
Making the three-state-flag of the flag logical
Jan 8, 2024
eed8d79
fixing bug with the three-state-flag
Jan 11, 2024
20063ad
Merge branch 'dev' into users/zubair/edgezones
Jan 12, 2024
c186c18
Upgrade requirements to latest SDK
Jan 15, 2024
217ba88
Upgraded code for new SDK, fixed some tests
Jan 15, 2024
e46be3b
All tests passing
Jan 15, 2024
b291bf3
fix style
Jan 15, 2024
3fe1bb8
fix style
Jan 16, 2024
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
1 change: 1 addition & 0 deletions src/azure-cli/azure/cli/command_modules/backup/_params.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ def load_arguments(self, _):
c.argument('target_subnet_name', help='Name of the subnet in which the target VM should be created, in the case of Alternate Location restore a new VM')
c.argument('target_subscription_id', help='ID of the subscription to which the resource should be restored')
c.argument('storage_account_resource_group', help='Name of the resource group which contains the storage account. Default value will be same as --resource-group if not specified.')
c.argument('restore_to_edge_zone', arg_type=get_three_state_flag(), help='Switch parameter to indicate edge zone VM restore. This parameter can\'t be used in cross region and cross subscription restore scenarios.')

with self.argument_context('backup restore restore-azurefileshare') as c:
c.argument('resolve_conflict', resolve_conflict_type)
Expand Down
172 changes: 104 additions & 68 deletions src/azure-cli/azure/cli/command_modules/backup/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,73 +47,77 @@
logger = get_logger(__name__)

# Mapping of workload type
secondary_region_map = {"ussecwest": "usseceast",
"usseceast": "ussecwest",
"usnateast": "usnatwest",
"usnatwest": "usnateast",
"swedencentral": "swedensouth",
"swedensouth": "swedencentral",
"norwaywest": "norwayeast",
"norwayeast": "norwaywest",
"germanynorth": "germanywestcentral",
"germanywestcentral": "germanynorth",
"westus3": "eastus",
"eastasia": "southeastasia",
"southeastasia": "eastasia",
"australiaeast": "australiasoutheast",
"australiasoutheast": "australiaeast",
"australiacentral": "australiacentral2",
"australiacentral2": "australiacentral",
"brazilsouth": "southcentralus",
"brazilsoutheast": "brazilsouth",
"canadacentral": "canadaeast",
"canadaeast": "canadacentral",
"chinanorth": "chinaeast",
"chinaeast": "chinanorth",
"chinanorth2": "chinaeast2",
"chinaeast2": "chinanorth2",
"chinanorth3": "chinaeast3",
"chinaeast3": "chinanorth3",
"northeurope": "westeurope",
"westeurope": "northeurope",
"francecentral": "francesouth",
"francesouth": "francecentral",
"germanycentral": "germanynortheast",
"germanynortheast": "germanycentral",
"centralindia": "southindia",
"southindia": "centralindia",
"westindia": "southindia",
"japaneast": "japanwest",
"japanwest": "japaneast",
"koreacentral": "koreasouth",
"koreasouth": "koreacentral",
"eastus": "westus",
"westus": "eastus",
"eastus2": "centralus",
"centralus": "eastus2",
"northcentralus": "southcentralus",
"southcentralus": "northcentralus",
"westus2": "westcentralus",
"westcentralus": "westus2",
"centraluseuap": "eastus2euap",
"eastus2euap": "centraluseuap",
"southafricanorth": "southafricawest",
"southafricawest": "southafricanorth",
"switzerlandnorth": "switzerlandwest",
"switzerlandwest": "switzerlandnorth",
"ukwest": "uksouth",
"uksouth": "ukwest",
"uaenorth": "uaecentral",
"uaecentral": "uaenorth",
"usdodeast": "usdodcentral",
"usdodcentral": "usdodeast",
"usgovarizona": "usgovtexas",
"usgovtexas": "usgovarizona",
"usgoviowa": "usgovvirginia",
"usgovvirginia": "usgovtexas",
"malaysiasouth": "japanwest",
"jioindiacentral": "jioindiawest",
"jioindiawest": "jioindiacentral"}
secondary_region_map = {
"australiacentral": "australiacentral2",
"australiacentral2": "australiacentral",
"australiaeast": "australiasoutheast",
"australiasoutheast": "australiaeast",
"brazilsouth": "southcentralus",
"brazilsoutheast": "brazilsouth",
"canadacentral": "canadaeast",
"canadaeast": "canadacentral",
"centralindia": "southindia",
"centralus": "eastus2",
"centraluseuap": "eastus2euap",
"chinaeast": "chinanorth",
"chinaeast2": "chinanorth2",
"chinaeast3": "chinanorth3",
"chinanorth": "chinaeast",
"chinanorth2": "chinaeast2",
"chinanorth3": "chinaeast3",
"eastasia": "southeastasia",
"eastus": "westus",
"eastus2": "centralus",
"eastus2euap": "centraluseuap",
"francecentral": "francesouth",
"francesouth": "francecentral",
"germanycentral": "germanynortheast",
"germanynorth": "germanywestcentral",
"germanynortheast": "germanycentral",
"germanywestcentral": "germanynorth",
"japaneast": "japanwest",
"japanwest": "japaneast",
"jioindiacentral": "jioindiawest",
"jioindiawest": "jioindiacentral",
"koreacentral": "koreasouth",
"koreasouth": "koreacentral",
"malaysiasouth": "japanwest",
"northcentralus": "southcentralus",
"northeurope": "westeurope",
"norwayeast": "norwaywest",
"norwaywest": "norwayeast",
"southafricanorth": "southafricawest",
"southafricawest": "southafricanorth",
"southcentralus": "northcentralus",
"southeastasia": "eastasia",
"southindia": "centralindia",
"swedencentral": "swedensouth",
"swedensouth": "swedencentral",
"switzerlandnorth": "switzerlandwest",
"switzerlandwest": "switzerlandnorth",
"taiwannorth": "taiwannorthwest",
"taiwannorthwest": "taiwannorth",
"uaecentral": "uaenorth",
"uaenorth": "uaecentral",
"uksouth": "ukwest",
"ukwest": "uksouth",
"usdodcentral": "usdodeast",
"usdodeast": "usdodcentral",
"usgovarizona": "usgovtexas",
"usgoviowa": "usgovvirginia",
"usgovtexas": "usgovarizona",
"usgovvirginia": "usgovtexas",
"usnateast": "usnatwest",
"usnatwest": "usnateast",
"usseceast": "ussecwest",
"ussecwest": "usseceast",
"westcentralus": "westus2",
"westeurope": "northeurope",
"westindia": "southindia",
"westus": "eastus",
"westus2": "westcentralus",
"westus3": "eastus"
}

fabric_name = "Azure"
default_policy_name = "DefaultPolicy"
Expand Down Expand Up @@ -1274,6 +1278,32 @@ def _get_alr_restore_mode(target_vm_name, target_vnet_name, target_vnet_resource
""")


def _set_edge_zones_trigger_restore_properties(cmd, trigger_restore_properties, restore_to_edge_zone, recovery_point,
target_subscription, use_secondary_region, restore_mode):
# TODO: As the subscription we currently use does not have access to Edge Zones, no tests have been written for
# this. We have manually validated it, but tests should be added to validate all (successful + exceptional)
# cases as soon as is viable.
if restore_to_edge_zone is not None and restore_to_edge_zone:
# If CSR or CRR, error
if target_subscription != get_subscription_id(cmd.cli_ctx) or use_secondary_region:
raise InvalidArgumentValueError("The restore-to-edge-zone parameter can't be used for cross region "
"or cross subscription restore")
if recovery_point.properties.extended_location is None \
or recovery_point.properties.extended_location.name is None \
or recovery_point.properties.extended_location.name == "":
raise InvalidArgumentValueError("Please make sure that the recovery point belongs to an edge zone VM "
"and contains extended location")
trigger_restore_properties.extended_location = recovery_point.properties.extended_location

if restore_mode == "OriginalLocation":
if recovery_point.properties.extended_location is not None \
and recovery_point.properties.extended_location.name is not None \
and recovery_point.properties.extended_location.name != "":
trigger_restore_properties.extended_location = recovery_point.properties.extended_location

return trigger_restore_properties


# pylint: disable=too-many-locals
# pylint: disable=too-many-statements
def restore_disks(cmd, client, resource_group_name, vault_name, container_name, item_name, rp_name, storage_account,
Expand All @@ -1282,7 +1312,7 @@ def restore_disks(cmd, client, resource_group_name, vault_name, container_name,
rehydration_priority=None, disk_encryption_set_id=None, mi_system_assigned=None,
mi_user_assigned=None, target_zone=None, restore_mode='AlternateLocation', target_vm_name=None,
target_vnet_name=None, target_vnet_resource_group=None, target_subnet_name=None,
target_subscription_id=None, storage_account_resource_group=None):
target_subscription_id=None, storage_account_resource_group=None, restore_to_edge_zone=None):
vault = vaults_cf(cmd.cli_ctx).get(resource_group_name, vault_name)
vault_location = vault.location
vault_identity = vault.identity
Expand Down Expand Up @@ -1382,6 +1412,12 @@ def restore_disks(cmd, client, resource_group_name, vault_name, container_name,
recovery_point, target_zone, target_rg_id, _source_resource_id, restore_mode,
target_subscription, use_secondary_region)

# Edge zones-specific code. Not using existing set/get properties code as it is messy and prone to errors
trigger_restore_properties = _set_edge_zones_trigger_restore_properties(cmd, trigger_restore_properties,
restore_to_edge_zone,
recovery_point, target_subscription,
use_secondary_region, restore_mode)

trigger_restore_request = RestoreRequestResource(properties=trigger_restore_properties)

if use_secondary_region:
Expand Down
5 changes: 3 additions & 2 deletions src/azure-cli/azure/cli/command_modules/backup/custom_afs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ def enable_for_AzureFileShare(cmd, client, resource_group_name, vault_name, afs_
source_resource_id=storage_account.properties.container_id,
workload_type="AzureFileShare")
param = ProtectionContainerResource(properties=properties)
result = protection_containers_client.register(vault_name, resource_group_name, fabric_name,
storage_account.name, param, cls=helper.get_pipeline_response)
result = protection_containers_client.begin_register(vault_name, resource_group_name, fabric_name,
storage_account.name, param, polling=False,
cls=helper.get_pipeline_response).result()
helper.track_register_operation(cmd.cli_ctx, result, vault_name, resource_group_name, storage_account.name)

protectable_item = _get_protectable_item_for_afs(cmd.cli_ctx, vault_name, resource_group_name, afs_name,
Expand Down
4 changes: 2 additions & 2 deletions src/azure-cli/azure/cli/command_modules/backup/custom_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ def restore_disks(cmd, client, resource_group_name, vault_name, container_name,
rehydration_priority=None, disk_encryption_set_id=None, mi_system_assigned=None,
mi_user_assigned=None, target_zone=None, restore_mode='AlternateLocation', target_vm_name=None,
target_vnet_name=None, target_vnet_resource_group=None, target_subnet_name=None,
target_subscription_id=None, storage_account_resource_group=None):
target_subscription_id=None, storage_account_resource_group=None, restore_to_edge_zone=None):

if rehydration_duration < 10 or rehydration_duration > 30:
raise InvalidArgumentValueError('--rehydration-duration must have a value between 10 and 30 (both inclusive).')
Expand All @@ -437,7 +437,7 @@ def restore_disks(cmd, client, resource_group_name, vault_name, container_name,
rehydration_duration, rehydration_priority, disk_encryption_set_id,
mi_system_assigned, mi_user_assigned, target_zone, restore_mode, target_vm_name,
target_vnet_name, target_vnet_resource_group, target_subnet_name,
target_subscription_id, storage_account_resource_group)
target_subscription_id, storage_account_resource_group, restore_to_edge_zone)


def enable_for_azurefileshare(cmd, client, resource_group_name, vault_name, policy_name, storage_account,
Expand Down
8 changes: 4 additions & 4 deletions src/azure-cli/azure/cli/command_modules/backup/custom_wl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ def register_wl_container(cmd, client, vault_name, resource_group_name, workload
param = ProtectionContainerResource(properties=properties)

# Trigger register and wait for completion
result = client.register(vault_name, resource_group_name, fabric_name, container_name, param,
cls=cust_help.get_pipeline_response)
result = client.begin_register(vault_name, resource_group_name, fabric_name, container_name, param,
cls=cust_help.get_pipeline_response, polling=False).result()
Comment on lines +138 to +139
Copy link
Contributor

Choose a reason for hiding this comment

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

May I ask why you need to disable the logic of polling in the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The SDK was changed to return LROPoller instead, so I made the change to parse it correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, what I mean is what problems would there be if using polling?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can try to obtain results from LROPoller in the following ways

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

Line 4158 in 2caee14

return LongRunningOperation(cmd.cli_ctx)(validation_poller)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, what I mean is what problems would there be if using polling?

I'm not entirely sure, honestly. I'm just following the pattern we've been using previously in our code:

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

Line 1403 in 2caee14

trigger_restore_request, cls=cust_help.get_pipeline_response, polling=False).result()

Copy link
Contributor

@zhoxing-ms zhoxing-ms Jan 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Okay, you can keep the original code style for now. If you have a need of progress bar for long running operation, you can consider changing the code style to the one we recommend in the future.

return cust_help.track_register_operation(cmd.cli_ctx, result, vault_name, resource_group_name, container_name)


Expand Down Expand Up @@ -171,8 +171,8 @@ def re_register_wl_container(cmd, client, vault_name, resource_group_name, workl
source_resource_id=source_resource_id)
param = ProtectionContainerResource(properties=properties)
# Trigger register and wait for completion
result = client.register(vault_name, resource_group_name, fabric_name, container_name, param,
cls=cust_help.get_pipeline_response)
result = client.begin_register(vault_name, resource_group_name, fabric_name, container_name, param,
cls=cust_help.get_pipeline_response, polling=False).result()
return cust_help.track_register_operation(cmd.cli_ctx, result, vault_name, resource_group_name, container_name)


Expand Down
29 changes: 21 additions & 8 deletions src/azure-cli/azure/cli/command_modules/backup/tests/latest/preparers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
from azure.cli.testsdk.base import execute
# pylint: disable=line-too-long

from knack.log import get_logger
logger = get_logger(__name__)


class VaultPreparer(AbstractPreparer, SingleValueReplacer): # pylint: disable=too-many-instance-attributes
def __init__(self, name_prefix='clitest-vault', parameter_name='vault_name',
Expand Down Expand Up @@ -593,17 +596,27 @@ def _get_policy(self, **kwargs):
'decorator @AFSPolicyPreparer in front of this Item preparer.'
raise CliTestError(template)

def _delete_lock(self, lock):
lock_id = lock["id"]
try:
command_string = 'az lock delete --ids {}'.format(lock_id)
execute(self.cli_ctx, command_string)
except Exception:
raise CliTestError('Unable to delete the lock with ID {}, please delete it manually'.format(lock_id))

def _cleanup(self, resource_group, storage_account, vault, afs):
# Need to remove any resource locks on the Storage Account, and also manually delete the item
command_string = 'az lock list -g {}'.format(resource_group)
list_of_locks = execute(self.cli_ctx, command_string).get_output_in_json()
for lock in list_of_locks:
lock_id = lock["id"]
try:
command_string = 'az lock delete --ids {}'.format(lock_id)
execute(self.cli_ctx, command_string)
except Exception:
raise CliTestError('Unable to delete the lock with ID {}, please delete it manually'.format(lock_id))
self._delete_lock(lock)

# Cleaning up Storage account locks
command_string = 'az lock list -g {} --resource-name {} --resource-type {}'.format(
resource_group, storage_account, 'Microsoft.Storage/storageAccounts')
list_of_locks = execute(self.cli_ctx, command_string).get_output_in_json()
for lock in list_of_locks:
self._delete_lock(lock)

command_string = 'az backup protection disable'
command_string += ' -g {} -v {} --container-name {} --item-name {}'
Expand All @@ -612,15 +625,15 @@ def _cleanup(self, resource_group, storage_account, vault, afs):
try:
execute(self.cli_ctx, command_string)
except Exception:
print('Warning: Unable to unregister AFS item during AFS Item test cleanup.')
logger.warning('Warning: Unable to unregister AFS item during AFS Item test cleanup.')

command_string = 'az backup container unregister'
command_string += ' --vault-name {} --resource-group {} --container-name {} --backup-management-type AzureStorage --yes'
command_string = command_string.format(vault, resource_group, storage_account)
try:
execute(self.cli_ctx, command_string)
except Exception:
print('Warning: Unable to unregister storage container during AFS Item test cleanup.')
logger.warning('Warning: Unable to unregister storage container during AFS Item test cleanup.')


class AFSRPPreparer(AbstractPreparer, SingleValueReplacer):
Expand Down
Loading