From a290f389b3da0ec91f09648ba42ab350ae9d9240 Mon Sep 17 00:00:00 2001 From: p3rf Team Date: Mon, 12 Feb 2024 11:23:21 -0800 Subject: [PATCH] Refactored azure virtual machine to use azure disk strategies and enable disk provisioning benchmark for Azure PiperOrigin-RevId: 606310865 --- CHANGES.next.md | 2 + .../provision_disk_benchmark.py | 16 +- .../providers/azure/azure_disk.py | 17 +- .../providers/azure/azure_disk_strategies.py | 205 +++++++++++++++++- .../providers/azure/azure_virtual_machine.py | 81 +++---- perfkitbenchmarker/providers/azure/flags.py | 2 +- .../providers/gcp/gce_disk_strategies.py | 8 +- tests/benchmark_spec_test.py | 2 +- tests/disk_metadata_test.py | 4 +- tests/scratch_disk_test.py | 6 + 10 files changed, 277 insertions(+), 66 deletions(-) diff --git a/CHANGES.next.md b/CHANGES.next.md index 3062fae272..4d5c2bad26 100644 --- a/CHANGES.next.md +++ b/CHANGES.next.md @@ -418,3 +418,5 @@ create and attach time. - Reduce duplicate code in MaintenanceEventTrigger.AppendSamples(). - Attach "run_number" label to "LM Total Time" sample. +- Refactored `azure_virtual_machine.py` to use `azure_disk_strategies.py` and + Enabled disk provision benchmark for Azure. diff --git a/perfkitbenchmarker/linux_benchmarks/provisioning_benchmarks/provision_disk_benchmark.py b/perfkitbenchmarker/linux_benchmarks/provisioning_benchmarks/provision_disk_benchmark.py index 65d0cbde6b..d8400b6125 100644 --- a/perfkitbenchmarker/linux_benchmarks/provisioning_benchmarks/provision_disk_benchmark.py +++ b/perfkitbenchmarker/linux_benchmarks/provisioning_benchmarks/provision_disk_benchmark.py @@ -27,14 +27,16 @@ from perfkitbenchmarker import benchmark_spec from perfkitbenchmarker import configs from perfkitbenchmarker import disk +from perfkitbenchmarker import errors from perfkitbenchmarker import linux_virtual_machine from perfkitbenchmarker import sample +from perfkitbenchmarker.providers.azure import flags as azure_flags from perfkitbenchmarker.providers.gcp import flags as gcp_flags + FLAGS = flags.FLAGS BENCHMARK_NAME = 'provision_disk' - BENCHMARK_CONFIG = """ provision_disk: description: > @@ -45,10 +47,16 @@ GCP: machine_type: n2-standard-2 zone: us-central1-c + Azure: + machine_type: Standard_D2s_v5 + zone: eastus2-2 disk_spec: GCP: disk_type: pd-ssd disk_size: 10 + Azure: + disk_type: PremiumV2_LRS + disk_size: 10 """ @@ -70,6 +78,10 @@ def CheckPrerequisites(benchmark_config): raise ValueError( 'gcp_create_disks_with_vm must be set to false for GCP' ) + if FLAGS.cloud == 'Azure' and azure_flags.AZURE_ATTACH_DISK_WITH_CREATE.value: + raise errors.Setup.InvalidFlagConfigurationError( + 'azure_attach_disk_with_create must be set to false for Azure' + ) def _WaitUntilAttached(vm, dsk) -> None: @@ -93,7 +105,7 @@ def Run(bm_spec: benchmark_spec.BenchmarkSpec) -> List[sample.Sample]: 'after VM creation.' ) samples = [] - disk_details = vm.create_disk_strategy.pd_disk_groups + disk_details = vm.create_disk_strategy.remote_disk_groups for disk_group in disk_details: for disk_details in disk_group: total_time = 0 diff --git a/perfkitbenchmarker/providers/azure/azure_disk.py b/perfkitbenchmarker/providers/azure/azure_disk.py index 9d79b5d261..c26bde69f5 100644 --- a/perfkitbenchmarker/providers/azure/azure_disk.py +++ b/perfkitbenchmarker/providers/azure/azure_disk.py @@ -38,13 +38,26 @@ MAX_DRIVE_SUFFIX_LENGTH = 2 # Last allowable device is /dev/sdzz. -PREMIUM_STORAGE = 'Premium_LRS' PREMIUM_STORAGE_V2 = 'PremiumV2_LRS' +PREMIUM_STORAGE = 'Premium_LRS' +PREMIUM_ZRS = 'Premium_ZRS' +STANDARD_SSD_LRS = 'StandardSSD_LRS' +STANDARD_SSD_ZRS = 'StandardSSD_ZRS' STANDARD_DISK = 'Standard_LRS' ULTRA_STORAGE = 'UltraSSD_LRS' -PREMIUM_ZRS = 'Premium_ZRS' +# https://learn.microsoft.com/en-us/rest/api/compute/disks/list?view=rest-compute-2023-10-02&tabs=HTTP#diskstorageaccounttypes +AZURE_REMOTE_DISK_TYPES = [ + PREMIUM_STORAGE, + PREMIUM_STORAGE_V2, + STANDARD_SSD_LRS, + STANDARD_SSD_ZRS, + STANDARD_DISK, + ULTRA_STORAGE, + PREMIUM_ZRS, +] + HOST_CACHING = 'host_caching' AZURE = 'Azure' diff --git a/perfkitbenchmarker/providers/azure/azure_disk_strategies.py b/perfkitbenchmarker/providers/azure/azure_disk_strategies.py index 87522fed3b..6c435fdf76 100644 --- a/perfkitbenchmarker/providers/azure/azure_disk_strategies.py +++ b/perfkitbenchmarker/providers/azure/azure_disk_strategies.py @@ -18,20 +18,221 @@ """ from typing import Any + from absl import flags +from perfkitbenchmarker import disk from perfkitbenchmarker import disk_strategies +from perfkitbenchmarker import errors from perfkitbenchmarker.providers.azure import azure_disk FLAGS = flags.FLAGS +virtual_machine = Any # pylint: disable=invalid-name -virtual_machine = Any # pylint: disable=invalid-name +def GetCreateDiskStrategy( + vm: 'virtual_machine.BaseVirtualMachine', + disk_spec: disk.BaseDiskSpec, + disk_count: int, +) -> disk_strategies.CreateDiskStrategy: + """Returns the strategies to create disks for the disk type.""" + if disk_spec and disk_count > 0: + if disk_spec.disk_type == disk.LOCAL: + return AzureCreateLocalSSDDiskStrategy(vm, disk_spec, disk_count) + elif disk_spec.disk_type in azure_disk.AZURE_REMOTE_DISK_TYPES: + return AzureCreateRemoteDiskStrategy(vm, disk_spec, disk_count) + return AzureCreateNonResourceDiskStrategy(vm, disk_spec, disk_count) + + +class AzureCreateDiskStrategy(disk_strategies.CreateDiskStrategy): + """Same as CreateDiskStrategy, but with Base Disk spec.""" + + disk_spec: disk.BaseDiskSpec + + +class AzureCreateOSDiskStrategy(AzureCreateDiskStrategy): + """Strategies to create OS disk. + """ + + def __init__( + self, + vm: 'virtual_machine.BaseVirtualMachine', + disk_spec: disk.BaseDiskSpec, + disk_count: int, + ): + super().__init__(vm, disk_spec, disk_count) + self.vm = vm + self.disk_count = disk_count + if disk_spec is None: + disk_spec = disk.BaseDiskSpec('azure_os_disk') + disk_spec.disk_type = vm.boot_disk_type or vm.storage_account.storage_type + if vm.boot_disk_size: + disk_spec.disk_size = vm.boot_disk_size + self.disk = azure_disk.AzureDisk( + self.disk_spec, self.vm, None, is_image=True + ) + self.disk_spec = disk_spec + + def GetCreationCommand(self) -> dict[str, Any]: + dic = {} + disk_type_cmd = [] + disk_size_cmd = [] + if self.disk_spec.disk_type: + disk_type_cmd = ['--storage-sku', f'os={self.disk.disk_type}'] + if self.disk and self.disk.disk_size: + disk_size_cmd = ['--os-disk-size-gb', str(self.disk.disk_size)] + dic['create-disk'] = disk_type_cmd+disk_size_cmd + return dic + + +class AzureCreateRemoteDiskStrategy(AzureCreateDiskStrategy): + """Strategies to create remote disks. + """ + + def __init__( + self, + vm: 'virtual_machine.BaseVirtualMachine', + disk_spec: disk.BaseDiskSpec, + disk_count: int, + ): + super().__init__(vm, disk_spec, disk_count) + self.remote_disk_groups = [] + self.vm = vm + for _, disk_spec in enumerate(self.disk_specs): + disks = [] + for _ in range(disk_spec.num_striped_disks): + disk_number = ( + self.vm.remote_disk_counter + + 1 + + self.vm.max_local_disks + ) + self.vm.remote_disk_counter += 1 + lun = next(self.vm.lun_counter) + data_disk = azure_disk.AzureDisk(disk_spec, self.vm, lun) + # TODO(user) Clean code to avoid using + # disk_number as it is used for windows only + data_disk.disk_number = disk_number + disks.append(data_disk) + self.remote_disk_groups.append(disks) + + def DiskCreatedOnVMCreation(self) -> bool: + """Returns whether the disk is created on VM creation.""" + return False + + def GetSetupDiskStrategy(self) -> disk_strategies.SetUpDiskStrategy: + """Returns the SetUpDiskStrategy for the disk.""" + return AzureSetUpRemoteDiskStrategy(self.vm, self.disk_specs) + + +class AzureCreateLocalSSDDiskStrategy(AzureCreateDiskStrategy): + """Strategies to create local disks. + """ + + def DiskCreatedOnVMCreation(self) -> bool: + """Returns whether the disk is created on VM creation.""" + return True + + def GetSetupDiskStrategy(self) -> disk_strategies.SetUpDiskStrategy: + """Returns the SetUpDiskStrategy for the disk.""" + return AzureSetUpLocalSSDDiskStrategy(self.vm, self.disk_spec) + + +class AzureCreateNonResourceDiskStrategy( + disk_strategies.EmptyCreateDiskStrategy +): + """Strategies to create non remote and non local disks like RAM, SMB.""" + + def DiskCreatedOnVMCreation(self) -> bool: + # This have to be set to False due to _CreateScratchDiskFromDisks in + # virtual_machine.py. + return False + + def GetSetupDiskStrategy(self) -> disk_strategies.SetUpDiskStrategy: + """Returns the SetUpDiskStrategy for the disk.""" + if not self.disk_spec: + return disk_strategies.EmptySetupDiskStrategy(self.vm, self.disk_spec) + + if self.disk_spec.disk_type == disk.RAM: + return disk_strategies.SetUpRamDiskStrategy(self.vm, self.disk_spec) + + elif self.disk_spec.disk_type == disk.SMB: + return disk_strategies.SetUpSMBDiskStrategy(self.vm, self.disk_spec) + + return disk_strategies.EmptySetupDiskStrategy(self.vm, self.disk_spec) + + +class AzureSetUpLocalSSDDiskStrategy(disk_strategies.SetUpDiskStrategy): + """Strategies to setup local disk. + """ + + def __init__( + self, + vm: 'virtual_machine.BaseVirtualMachine', + disk_spec: disk.BaseDiskSpec, + ): + super().__init__(vm, disk_spec) + self.vm = vm + self.disk_spec = disk_spec + + def SetUpDisk( + self + ): + disks = [] + + for _ in range(self.disk_spec.num_striped_disks): + disk_number = self.vm.local_disk_counter + 1 + self.vm.local_disk_counter += 1 + if ( + self.vm.local_disk_counter + > self.vm.max_local_disks + ): + raise errors.Error('Not enough local disks.') + lun = next(self.vm.lun_counter) + data_disk = azure_disk.AzureDisk(self.disk_spec, self.vm, lun) + data_disk.disk_number = disk_number + disks.append(data_disk) + if len(disks) > 1: + # If the disk_spec called for a striped disk, create one. + scratch_disk = disk.StripedDisk(self.disk_spec, disks) + else: + scratch_disk = disks[0] + AzurePrepareScratchDiskStrategy().PrepareScratchDisk( + self.vm, scratch_disk, self.disk_spec + ) + + +class AzureSetUpRemoteDiskStrategy(disk_strategies.SetUpDiskStrategy): + """Strategies to setup remote disk. + """ + + def __init__( + self, + vm: 'virtual_machine.BaseVirtualMachine', + disk_specs: list[disk.BaseDiskSpec], + ): + super().__init__(vm, disk_specs[0]) + self.vm = vm + self.disk_specs = disk_specs + + def SetUpDisk(self): + for disk_spec_id, disk_spec in enumerate(self.disk_specs): + disk_group = self.vm.create_disk_strategy.remote_disk_groups[disk_spec_id] + if len(disk_group) > 1: + # If the disk_spec called for a striped disk, create one. + scratch_disk = disk.StripedDisk(disk_spec, disk_group) + else: + scratch_disk = disk_group[0] + if not self.vm.create_disk_strategy.DiskCreatedOnVMCreation(): + scratch_disk.Create() + scratch_disk.Attach(self.vm) + AzurePrepareScratchDiskStrategy().PrepareScratchDisk( + self.vm, scratch_disk, disk_spec + ) class AzurePrepareScratchDiskStrategy( disk_strategies.PrepareScratchDiskStrategy ): - """Strategies to prepare scratch disk on AWS.""" + """Strategies to prepare scratch disk on Azure.""" def GetLocalSSDNames(self) -> list[str]: # This is only for Ls-Series local ssd diff --git a/perfkitbenchmarker/providers/azure/azure_virtual_machine.py b/perfkitbenchmarker/providers/azure/azure_virtual_machine.py index 4095fd2649..9a45dcde4a 100644 --- a/perfkitbenchmarker/providers/azure/azure_virtual_machine.py +++ b/perfkitbenchmarker/providers/azure/azure_virtual_machine.py @@ -53,11 +53,10 @@ from perfkitbenchmarker.providers.azure import azure_disk_strategies from perfkitbenchmarker.providers.azure import azure_network from perfkitbenchmarker.providers.azure import util -from six.moves import range import yaml FLAGS = flags.FLAGS -NUM_LOCAL_VOLUMES = { +NUM_LOCAL_VOLUMES: dict[str, int] = { 'Standard_L8s_v2': 1, 'Standard_L16s_v2': 2, 'Standard_L32s_v2': 4, @@ -667,10 +666,12 @@ def __init__(self, vm_spec): self.network = azure_network.AzureNetwork.GetNetwork(self) self.firewall = azure_network.AzureFirewall.GetFirewall() if azure_disk.HasTempDrive(self.machine_type): - self.max_local_disks = NUM_LOCAL_VOLUMES.get(self.machine_type, 1) + self.max_local_disks = NUM_LOCAL_VOLUMES.get( + self.machine_type, 1 + ) else: self.max_local_disks = 0 - self._lun_counter = itertools.count() + self.lun_counter = itertools.count() self._deleted = False self.resource_group = azure_network.GetResourceGroup() @@ -731,14 +732,13 @@ def __init__(self, vm_spec): self.low_priority_status_code = None self.spot_early_termination = False self.ultra_ssd_enabled = False - - disk_spec = disk.BaseDiskSpec('azure_os_disk') - disk_spec.disk_type = ( - vm_spec.boot_disk_type or self.storage_account.storage_type + self.boot_disk_size = vm_spec.boot_disk_size + self.boot_disk_type = vm_spec.boot_disk_type + self.create_os_disk_strategy = ( + azure_disk_strategies.AzureCreateOSDiskStrategy( + self, disk.BaseDiskSpec('azure_os_disk'), 1 + ) ) - if vm_spec.boot_disk_size: - disk_spec.disk_size = vm_spec.boot_disk_size - self.os_disk = azure_disk.AzureDisk(disk_spec, self, None, is_image=True) @property @classmethod @@ -782,10 +782,10 @@ def _RequiresUltraDisk(self): def _Create(self): """See base class.""" assert self.network is not None - disk_size_args = [] - if self.os_disk.disk_size: - disk_size_args = ['--os-disk-size-gb', str(self.os_disk.disk_size)] + os_disk_args = self.create_os_disk_strategy.GetCreationCommand()[ + 'create-disk' + ] confidential_args = [] if self.machine_type_is_confidential: confidential_args = [ @@ -819,12 +819,10 @@ def _Create(self): self.machine_type, '--admin-username', self.user_name, - '--storage-sku', - f'os={self.os_disk.disk_type}', '--name', self.name, ] - + disk_size_args + + os_disk_args + confidential_args + self.resource_group.args + self.nic.args @@ -1012,15 +1010,17 @@ def _PostCreate(self): + self.resource_group.args ) response = json.loads(stdout) - self.os_disk.name = response['storageProfile']['osDisk']['name'] - self.os_disk.created = True + self.create_os_disk_strategy.disk.name = response['storageProfile'][ + 'osDisk' + ]['name'] + self.create_os_disk_strategy.disk.created = True vm_util.IssueCommand( [ azure.AZURE_PATH, 'disk', 'update', '--name', - self.os_disk.name, + self.create_os_disk_strategy.disk.name, '--set', util.GetTagsJson(self.resource_group.timeout_minutes), ] @@ -1030,37 +1030,14 @@ def _PostCreate(self): if self.public_ip: self.ip_address = self.public_ip.GetIPAddress() - def CreateScratchDisk(self, _, disk_spec): - """Create a VM's scratch disk. + def SetupAllScratchDisks(self): + """Set up all scratch disks of the current VM.""" + self.create_disk_strategy.GetSetupDiskStrategy().SetUpDisk() - Args: - disk_spec: virtual_machine.BaseDiskSpec object of the disk. - - Raises: - CreationError: If an SMB disk is listed but the SMB service not created. - """ - disks = [] - - for _ in range(disk_spec.num_striped_disks): - if disk_spec.disk_type == disk.LOCAL: - # Local disk numbers start at 1 (0 is the system disk). - disk_number = self.local_disk_counter + 1 - self.local_disk_counter += 1 - if self.local_disk_counter > self.max_local_disks: - raise errors.Error('Not enough local disks.') - else: - # Remote disk numbers start at 1 + max_local disks (0 is the system disk - # and local disks occupy [1, max_local_disks]). - disk_number = self.remote_disk_counter + 1 + self.max_local_disks - self.remote_disk_counter += 1 - lun = next(self._lun_counter) - data_disk = azure_disk.AzureDisk(disk_spec, self, lun) - data_disk.disk_number = disk_number - disks.append(data_disk) - - scratch_disk = self._CreateScratchDiskFromDisks(disk_spec, disks) - azure_disk_strategies.AzurePrepareScratchDiskStrategy().PrepareScratchDisk( - self, scratch_disk, disk_spec + def SetDiskSpec(self, disk_spec, disk_count): + """Sets Disk Specs of the current VM. Calls before the VM is created.""" + self.create_disk_strategy = azure_disk_strategies.GetCreateDiskStrategy( + self, disk_spec, disk_count ) def InstallCli(self): @@ -1114,8 +1091,8 @@ def GetResourceMetadata(self): assert self.network is not None result = super(AzureVirtualMachine, self).GetResourceMetadata() result['accelerated_networking'] = self.nic.accelerated_networking - result['boot_disk_type'] = self.os_disk.disk_type - result['boot_disk_size'] = self.os_disk.disk_size + result['boot_disk_type'] = self.create_os_disk_strategy.disk.disk_type + result['boot_disk_size'] = self.create_os_disk_strategy.disk.disk_size if self.network.placement_group: result['placement_group_strategy'] = self.network.placement_group.strategy else: diff --git a/perfkitbenchmarker/providers/azure/flags.py b/perfkitbenchmarker/providers/azure/flags.py index 41f2142b54..a5a9858c41 100644 --- a/perfkitbenchmarker/providers/azure/flags.py +++ b/perfkitbenchmarker/providers/azure/flags.py @@ -151,7 +151,7 @@ 'The Runtime Interface used when interacting with Synapse.', ) flags.DEFINE_string('query_timeout', '600', 'Query timeout in seconds.') -flags.DEFINE_boolean( +AZURE_ATTACH_DISK_WITH_CREATE = flags.DEFINE_boolean( 'azure_attach_disk_with_create', True, 'Whether to create PD disks at VM creation time. Defaults to True.', diff --git a/perfkitbenchmarker/providers/gcp/gce_disk_strategies.py b/perfkitbenchmarker/providers/gcp/gce_disk_strategies.py index 3a549de26c..7504f5fbcc 100644 --- a/perfkitbenchmarker/providers/gcp/gce_disk_strategies.py +++ b/perfkitbenchmarker/providers/gcp/gce_disk_strategies.py @@ -55,7 +55,7 @@ class CreatePDDiskStrategy(GCPCreateDiskStrategy): def __init__(self, vm: Any, disk_spec: disk.BaseDiskSpec, disk_count: int): super().__init__(vm, disk_spec, disk_count) - self.pd_disk_groups = [] + self.remote_disk_groups = [] for disk_spec_id, disk_spec in enumerate(self.disk_specs): disks = [] for i in range(disk_spec.num_striped_disks): @@ -73,7 +73,7 @@ def __init__(self, vm: Any, disk_spec: disk.BaseDiskSpec, disk_count: int): data_disk.interface = gce_disk.SCSI vm.remote_disk_counter += 1 disks.append(data_disk) - self.pd_disk_groups.append(disks) + self.remote_disk_groups.append(disks) def DiskCreatedOnVMCreation(self) -> bool: """Returns whether the disk is created on VM creation.""" @@ -100,7 +100,7 @@ def GetCreationCommand(self) -> dict[str, Any]: create_disks = [] dic = {} - for disk_group in self.pd_disk_groups: + for disk_group in self.remote_disk_groups: for pd_disk in disk_group: create_disks.append(pd_disk.GetCreateFlags()) if create_disks: @@ -224,7 +224,7 @@ def __init__(self, vm, disk_specs: list[gce_disk.GceDiskSpec]): def SetUpDisk(self): # disk spec is not used here. for disk_spec_id, disk_spec in enumerate(self.disk_specs): - disk_group = self.vm.create_disk_strategy.pd_disk_groups[disk_spec_id] + disk_group = self.vm.create_disk_strategy.remote_disk_groups[disk_spec_id] # Create the disk if it is not created on create if not self.vm.create_disk_strategy.DiskCreatedOnVMCreation(): for pd_disk in disk_group: diff --git a/tests/benchmark_spec_test.py b/tests/benchmark_spec_test.py index 95cf9b4601..bfa87c36c9 100644 --- a/tests/benchmark_spec_test.py +++ b/tests/benchmark_spec_test.py @@ -274,7 +274,7 @@ def testValidConfigWithDiskSpec(self): vms = spec.vm_groups['default'] self.assertEqual(len(vms), 2) for vm in vms: - self.assertEqual(len(vm.create_disk_strategy.pd_disk_groups), 3) + self.assertEqual(len(vm.create_disk_strategy.remote_disk_groups), 3) self.assertTrue( all(disk_spec.disk_size == 75 for disk_spec in vm.disk_specs) ) diff --git a/tests/disk_metadata_test.py b/tests/disk_metadata_test.py index 1b11d998a8..65645a3baa 100644 --- a/tests/disk_metadata_test.py +++ b/tests/disk_metadata_test.py @@ -148,8 +148,8 @@ def DoAzureDiskTest( azure_disk.AzureDisk.Create = mock.Mock() azure_disk.AzureDisk.Attach = mock.Mock() vm.StripeDisks = mock.Mock() - vm.CreateScratchDisk(0, disk_spec) - + vm.SetDiskSpec(disk_spec, 1) + vm.SetupAllScratchDisks() expected = { disk.MEDIA: goal_media, disk.REPLICATION: goal_replication, diff --git a/tests/scratch_disk_test.py b/tests/scratch_disk_test.py index eb0545da7c..bd1c0b86f9 100644 --- a/tests/scratch_disk_test.py +++ b/tests/scratch_disk_test.py @@ -178,6 +178,12 @@ def _CreateVm(self): def _GetDiskClass(self): return azure_disk.AzureDisk + def GetDiskSpec(self, mount_point): + test_disk = disk.BaseDiskSpec(_COMPONENT, mount_point=mount_point) + test_disk.disk_type = azure_disk.STANDARD_DISK + test_disk.disk_size = 10 + return test_disk + class GceScratchDiskTest(ScratchDiskTestMixin, unittest.TestCase):