From 986929bc99fa1ba58a6bf005ccabb629bd39a666 Mon Sep 17 00:00:00 2001 From: Simon Dodsley Date: Wed, 30 Apr 2025 11:42:43 -0400 Subject: [PATCH] [Pure Storage] Add capacity based backend QoS options Add new QoS spec parameters based on the capacity of the volume. New parameters are maxIOPS_per_GB and maxBWS_per_GB. If provied in the same QoS spec as the original max values for IOPS or BWS, the max values will take precedence. Capacity based QoS settings will be calculated and applied for the following actions: - create volume - create volume from snapshot - extend volume - clone volume - manage volume - retype volume QoS settings are not applied to volumes within volume groups as QoS for these is controlled at the volume group level. Implements: blueprint pure-per-gb-qos Change-Id: I79b5180ac48140f747761d0c04f7bd62d9995b43 --- cinder/tests/unit/volume/drivers/test_pure.py | 62 ++++++++++++----- cinder/volume/drivers/pure.py | 66 +++++++++++++++---- .../drivers/pure-storage-driver.rst | 15 +++++ .../pure_per_gb_qos-0b96279d615b81a1.yaml | 7 ++ 4 files changed, 122 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/pure_per_gb_qos-0b96279d615b81a1.yaml diff --git a/cinder/tests/unit/volume/drivers/test_pure.py b/cinder/tests/unit/volume/drivers/test_pure.py index 9f41ed76ab1..20586fa29c8 100644 --- a/cinder/tests/unit/volume/drivers/test_pure.py +++ b/cinder/tests/unit/volume/drivers/test_pure.py @@ -1198,10 +1198,14 @@ MPS_REFS = ValidResponse(200, None, 3, DotNotation(MANAGEABLE_PURE_SNAP_REFS[2])], {}) # unit for maxBWS is MB -QOS_IOPS_BWS = {"maxIOPS": "100", "maxBWS": "1"} -QOS_IOPS_BWS_2 = {"maxIOPS": "1000", "maxBWS": "10"} -QOS_INVALID = {"maxIOPS": "100", "maxBWS": str(512 * 1024 + 1)} -QOS_ZEROS = {"maxIOPS": "0", "maxBWS": "0"} +QOS_IOPS_BWS = {"maxIOPS": "100", "maxBWS": "1", + "maxIOPS_per_GB": "0", "maxBWS_per_GB": "0"} +QOS_IOPS_BWS_2 = {"maxIOPS": "1000", "maxBWS": "10", + "maxIOPS_per_GB": "0", "maxBWS_per_GB": "0"} +QOS_INVALID = {"maxIOPS": "100", "maxBWS": str(512 * 1024 + 1), + "maxIOPS_per_GB": "0", "maxBWS_per_GB": "0"} +QOS_ZEROS = {"maxIOPS": "0", "maxBWS": "0", + "maxIOPS_per_GB": "0", "maxBWS_per_GB": "0"} QOS_IOPS = {"maxIOPS": "100"} QOS_BWS = {"maxBWS": "1"} MAX_IOPS = 100000000 @@ -2126,11 +2130,14 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): @mock.patch(DRIVER_PATH + ".flasharray.VolumePost") @mock.patch(BASE_DRIVER_OBJ + "._add_to_group_if_needed") @mock.patch(BASE_DRIVER_OBJ + "._get_replication_type_from_vol_type") - def test_create_cloned_volume(self, mock_get_replication_type, + @mock.patch.object(volume_types, 'get_volume_type') + def test_create_cloned_volume(self, mock_get_volume_type, + mock_get_replication_type, mock_add_to_group, mock_fa): vol, vol_name = self.new_fake_vol(set_provider_id=False) src_vol, src_name = self.new_fake_vol() + mock_get_volume_type.return_value = vol.volume_type mock_data = self.array.flasharray.VolumePost(names=[vol_name], source= pure.flasharray. @@ -2172,7 +2179,9 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): @mock.patch(BASE_DRIVER_OBJ + "._get_qos_settings") @mock.patch(BASE_DRIVER_OBJ + ".set_qos") @mock.patch(DRIVER_PATH + ".flasharray.VolumePost") + @mock.patch.object(volume_types, 'get_volume_type') def test_create_cloned_volume_qos(self, qos_info, + mock_get_volume_type, mock_fa, mock_qos, mock_qos_specs): @@ -2184,6 +2193,7 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): vol, vol_name = self.new_fake_vol(set_provider_id=False) src_vol, src_name = self.new_fake_vol(spec={"size": 1}, type_qos_specs_id=qos.id) + mock_get_volume_type.return_value = vol.volume_type mock_data = self.array.flasharray.VolumePost(names=[vol_name], source= pure.flasharray. @@ -2194,10 +2204,16 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): self.mock_object(self.driver, '_get_volume_type_extra_spec', return_value={}) self.driver.create_cloned_volume(vol, src_vol) - self.driver.set_qos.assert_called_with(self.array, vol_name, qos) + self.driver.set_qos.assert_called_with(self.array, + vol_name, + vol["size"], + qos) @mock.patch(DRIVER_PATH + ".flasharray.VolumePost") - def test_create_cloned_volume_sync_rep(self, mock_fa): + @mock.patch.object(volume_types, 'get_volume_type') + def test_create_cloned_volume_sync_rep(self, + mock_get_volume_type, + mock_fa): repl_extra_specs = { 'replication_type': ' sync', 'replication_enabled': ' true', @@ -2206,6 +2222,7 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): type_extra_specs=repl_extra_specs) vol, vol_name = self.new_fake_vol(set_provider_id=False, type_extra_specs=repl_extra_specs) + mock_get_volume_type.return_value = vol.volume_type mock_data = self.array.flasharray.VolumePost(names=[vol_name], source=pure.flasharray. reference(name=src_name)) @@ -2222,11 +2239,14 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): @mock.patch(DRIVER_PATH + ".flasharray.VolumePost") @mock.patch(BASE_DRIVER_OBJ + "._add_to_group_if_needed") @mock.patch(BASE_DRIVER_OBJ + "._get_replication_type_from_vol_type") - def test_create_cloned_volume_and_extend(self, mock_get_replication_type, + @mock.patch.object(volume_types, 'get_volume_type') + def test_create_cloned_volume_and_extend(self, mock_get_volume_type, + mock_get_replication_type, mock_add_to_group, mock_fa, mock_extend): vol, vol_name = self.new_fake_vol(set_provider_id=False, spec={"size": 2}) + mock_get_volume_type.return_value = vol.volume_type src_vol, src_name = self.new_fake_vol() mock_get_replication_type.return_value = None mock_data = self.array.flasharray.VolumePost(names=[vol_name], @@ -2246,9 +2266,12 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): # Tests cloning a volume that is part of a consistency group @mock.patch(BASE_DRIVER_OBJ + "._add_to_group_if_needed") @mock.patch(BASE_DRIVER_OBJ + "._get_replication_type_from_vol_type") - def test_create_cloned_volume_with_cgroup(self, mock_get_replication_type, + @mock.patch.object(volume_types, 'get_volume_type') + def test_create_cloned_volume_with_cgroup(self, mock_get_volume_type, + mock_get_replication_type, mock_add_to_group): vol, vol_name = self.new_fake_vol(set_provider_id=False) + mock_get_volume_type.return_value = vol.volume_type group = fake_group.fake_group_obj(mock.MagicMock()) self.driver._get_volume_type_extra_spec = mock.Mock( return_value={}) @@ -2619,8 +2642,10 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): self.array.delete_connections.assert_not_called() @mock.patch(DRIVER_PATH + ".flasharray.VolumePatch") - def test_extend_volume(self, mock_fa): + @mock.patch.object(volume_types, 'get_volume_type') + def test_extend_volume(self, mock_get_volume_type, mock_fa): vol, vol_name = self.new_fake_vol(spec={"size": 1}) + mock_get_volume_type.return_value = vol.volume_type mock_data = self.flasharray.VolumePatch(provisioned=3 * units.Gi) self.driver.extend_volume(vol, 3) self.array.patch_volumes.\ @@ -4505,7 +4530,10 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): Reference(name=vol_name), name=vol_name, qos={'maxIOPS': 100, - 'maxBWS': 1048576}) + 'maxBWS': 1048576, + 'maxBWS': 1048576, + 'maxIOPS_per_GB': 0, + 'maxBWS_per_GB': 0}) mock_fa.return_value = mock_data mock_get_volume_type.return_value = vol.volume_type @@ -4517,9 +4545,11 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): with_default_protection= False, volume=mock_data) - self.driver.set_qos.assert_called_with(self.array, vol_name, + self.driver.set_qos.assert_called_with(self.array, vol_name, 1, {'maxIOPS': 100, - 'maxBWS': 1048576}) + 'maxBWS': 1048576, + 'maxIOPS_per_GB': 0, + 'maxBWS_per_GB': 0}) self.assertFalse(self.array.extend_volume.called) mock_add_to_group.assert_called_once_with(vol, vol_name) self.assert_error_propagates( @@ -4546,9 +4576,11 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): self.array.get_volumes.return_value = MPV self.driver.manage_existing(vol, volume_ref) - mock_qos.assert_called_with(self.array, vol_name, + mock_qos.assert_called_with(self.array, vol_name, 3, {'maxIOPS': 100, - 'maxBWS': 1048576}) + 'maxBWS': 1048576, + 'maxIOPS_per_GB': 0, + 'maxBWS_per_GB': 0}) @mock.patch(DRIVER_PATH + ".flasharray.VolumePatch") def test_retype_qos(self, mock_fa): diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py index 14079f439cb..bb425bafba0 100644 --- a/cinder/volume/drivers/pure.py +++ b/cinder/volume/drivers/pure.py @@ -228,7 +228,7 @@ class PureBaseVolumeDriver(san.SanDriver): """Performs volume management on Pure Storage FlashArray.""" SUPPORTS_ACTIVE_ACTIVE = True - PURE_QOS_KEYS = ['maxIOPS', 'maxBWS'] + PURE_QOS_KEYS = ['maxIOPS', 'maxBWS', 'maxIOPS_per_GB', 'maxBWS_per_GB'] # ThirdPartySystems wiki page CI_WIKI_NAME = "Pure_Storage_CI" @@ -346,7 +346,19 @@ class PureBaseVolumeDriver(san.SanDriver): target_array) @pure_driver_debug_trace - def set_qos(self, array, vol_name, qos): + def set_qos(self, array, vol_name, vol_size, qos): + # max_IOPS and max_BWS override the per GB IOPS and BW values if + # both are provided. If only a per GB value is provided then + # we must ensure, based on volume size, the IOPS or BW values + # do not exceed the maximum limits for these values allowed per + # volume. + if qos['maxIOPS'] == 0 and qos['maxIOPS_per_GB']: + qos['maxIOPS'] = min(MAX_IOPS, + int(qos['maxIOPS_per_GB']) * vol_size) + if qos['maxBWS'] == 0 and qos['maxBWS_per_GB']: + qos['maxBWS'] = min(MAX_BWS, + int(qos['maxBWS_per_GB']) * vol_size) + if qos['maxIOPS'] == 0 and qos['maxBWS'] == 0: array.patch_volumes(names=[vol_name], volume=flasharray.VolumePatch( @@ -444,6 +456,19 @@ class PureBaseVolumeDriver(san.SanDriver): @pure_driver_debug_trace def create_with_qos(self, array, vol_name, vol_size, qos): + # max_IOPS and max_BWS override the per GB IOPS and BW values if + # both are provided. Iif only a per GB value is provided then + # we must ensure, based on volume size, the IOPS or BW values + # do not exceed the maximum limits for these values allowed per + # volume. + gb_size = vol_size / units.Gi + if qos['maxIOPS'] == 0 and qos['maxIOPS_per_GB']: + qos['maxIOPS'] = min(MAX_IOPS, + int(qos['maxIOPS_per_GB']) * gb_size) + if qos['maxBWS'] == 0 and qos['maxBWS_per_GB']: + qos['maxBWS'] = min(MAX_BWS, + int(qos['maxBWS_per_GB']) * gb_size) + if self._array.safemode: if qos['maxIOPS'] == 0 and qos['maxBWS'] == 0: array.post_volumes(names=[vol_name], @@ -826,7 +851,7 @@ class PureBaseVolumeDriver(san.SanDriver): snapshot["volume_size"], volume["size"]) if qos is not None: - self.set_qos(current_array, vol_name, qos) + self.set_qos(current_array, vol_name, snapshot["volume_size"], qos) else: current_array.patch_volumes(names=[vol_name], volume=flasharray.VolumePatch( @@ -938,11 +963,15 @@ class PureBaseVolumeDriver(san.SanDriver): vol_name, src_vref["size"], volume["size"]) - # Check if the volume_type has QoS settings and if so - # apply them to the newly created volume - qos = self._get_qos_settings(volume.volume_type) - if qos: - self.set_qos(current_array, vol_name, qos) + type_id = volume.get('volume_type_id') + ctxt = context.get_admin_context() + if type_id is not None: + volume_type = volume_types.get_volume_type(ctxt, type_id) + # Check if the volume_type has QoS settings and if so + # apply them to the newly created volume + qos = self._get_qos_settings(volume_type) + if qos is not None: + self.set_qos(current_array, vol_name, volume["size"], qos) return self._setup_volume(current_array, volume, vol_name) @@ -1385,17 +1414,25 @@ class PureBaseVolumeDriver(san.SanDriver): return thin_provisioning @pure_driver_debug_trace - def extend_volume(self, volume, new_size): + def extend_volume(self, volume, new_size_gb): """Extend volume to new_size.""" # Get current array in case we have failed over via replication. current_array = self._get_current_array() vol_name = self._get_vol_name(volume) - new_size = new_size * units.Gi + new_size = new_size_gb * units.Gi current_array.patch_volumes(names=[vol_name], volume=flasharray.VolumePatch( provisioned=new_size)) + ctxt = context.get_admin_context() + type_id = volume.get('volume_type_id') + if type_id is not None: + volume_type = volume_types.get_volume_type(ctxt, type_id) + LOG.debug("QOS volume type: '%s'", volume_type) + qos = self._get_qos_settings(volume_type) + if qos is not None: + self.set_qos(current_array, vol_name, new_size, qos) def _add_volume_to_consistency_group(self, group, vol_name): pgroup_name = self._get_pgroup_name(group) @@ -1949,7 +1986,8 @@ class PureBaseVolumeDriver(san.SanDriver): qos = None qos = self._get_qos_settings(volume.volume_type) if qos: - self.set_qos(current_array, new_vol_name, qos) + vol_size = int(volume_data.provisioned / units.Gi) + self.set_qos(current_array, new_vol_name, vol_size, qos) volume.provider_id = new_vol_name async_enabled = self._enable_async_replication_if_needed(current_array, volume) @@ -2403,7 +2441,7 @@ class PureBaseVolumeDriver(san.SanDriver): if qos == {}: return None else: - # Check set vslues are within limits + # Check set values are within limits iops_qos = int(qos.get('maxIOPS', 0)) bw_qos = int(qos.get('maxBWS', 0)) * MIN_BWS if iops_qos != 0 and not (MIN_IOPS <= iops_qos <= MAX_IOPS): @@ -2419,6 +2457,8 @@ class PureBaseVolumeDriver(san.SanDriver): qos['maxIOPS'] = iops_qos qos['maxBWS'] = bw_qos + qos['maxIOPS_per_GB'] = int(qos.get('maxIOPS_per_GB', 0)) + qos['maxBWS_per_GB'] = int(qos.get('maxBWS_per_GB', 0)) * MIN_BWS return qos def _generate_purity_vol_name(self, volume): @@ -2769,7 +2809,7 @@ class PureBaseVolumeDriver(san.SanDriver): qos = self._get_qos_settings(new_type) vol_name = self._generate_purity_vol_name(volume) if qos is not None: - self.set_qos(current_array, vol_name, qos) + self.set_qos(current_array, vol_name, volume["size"], qos) else: current_array.patch_volumes(names=[vol_name], volume=flasharray.VolumePatch( diff --git a/doc/source/configuration/block-storage/drivers/pure-storage-driver.rst b/doc/source/configuration/block-storage/drivers/pure-storage-driver.rst index bc3c6265da3..700335e13e7 100644 --- a/doc/source/configuration/block-storage/drivers/pure-storage-driver.rst +++ b/doc/source/configuration/block-storage/drivers/pure-storage-driver.rst @@ -59,12 +59,27 @@ following capabilities in the OpenStack Block Storage API * **maxBWS** - Maximum bandwidth limit in MB/s. Range: 1 - 524288 (512GB/s) +* **maxIOPS_per_GB** - Maximum number of IOPs allowed for volume based on + capacity. Range: 100 - 100M + +* **maxBWS_per_GB** - Maximum bandwidth limit in MB/s based on capacity. + Range: 1 - 524288 (512GB/s) + +If both max and per_GB values are provided for a QoS type, the max value will +take precedence. + +If the calculated per_GB value for a volume based on capacity is greater +than the maximum allowed value, the maximum allowed values will be applied. + The qos keys above must be created and asscoiated to a volume type. For information on how to set the key-value pairs and associate them with a volume type see the `volume qos `_ section in the OpenStack Client command list. +QoS settings are not applied to any volume in a volume group as these are +controlled at the volume group level. + Configure OpenStack and Purity ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/releasenotes/notes/pure_per_gb_qos-0b96279d615b81a1.yaml b/releasenotes/notes/pure_per_gb_qos-0b96279d615b81a1.yaml new file mode 100644 index 00000000000..8f2db44af26 --- /dev/null +++ b/releasenotes/notes/pure_per_gb_qos-0b96279d615b81a1.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + [Pure Storage] Added new QoS spec parameters to support QoS per GB. + New spec options are ``maxIOPS_per_GB`` and ``maxBWS_per_GB``. If + either of these are provided with the equivalent ``max`` value, the + ``max`` value will take precedence.