From 79b0071cbe225a17c754c2841c1cc820c5ba2b0d Mon Sep 17 00:00:00 2001 From: pipeline Date: Tue, 2 Jun 2026 05:37:28 -0500 Subject: [PATCH] feat(storage_pool): support flexible num_data_slices for Gen2 pools Replace the hardcoded TwoPlusTwo=2 / EightPlusTwo=8 data-slice mapping in StoragePool.create() with verbatim pass-through of numDataSlices and numProtectionSlices, falling back to the protection_scheme presets when no explicit slice count is given. check_create_params() now validates numDataSlices as a positive integer and requires one of numDataSlices or protectionScheme. Backward compatible with existing protection_scheme usage. Tests: tests/gen2/test_storage_pool.py (U-001..U-007). FEAT-23070 --- PyPowerFlex/objects/gen2/storage_pool.py | 20 +++- tests/gen2/test_storage_pool.py | 113 +++++++++++++++++++++++ 2 files changed, 130 insertions(+), 3 deletions(-) diff --git a/PyPowerFlex/objects/gen2/storage_pool.py b/PyPowerFlex/objects/gen2/storage_pool.py index a2d7886..e7bf4da 100644 --- a/PyPowerFlex/objects/gen2/storage_pool.py +++ b/PyPowerFlex/objects/gen2/storage_pool.py @@ -61,11 +61,22 @@ def get_by_name(self, protion_domain_id, name): def check_create_params(self, sp): """Check create parameters.""" - required_fields = ["name", "protectionDomainId", "deviceGroupId", "protectionScheme"] + required_fields = ["name", "protectionDomainId", "deviceGroupId"] missing_fields = [field for field in required_fields if field not in sp] if missing_fields: - msg = ("name, protection_domain_id, device_group_id, protection_scheme are required " + msg = ("name, protection_domain_id, device_group_id are required " + "for creating a storage pool.") + raise exceptions.InvalidInput(msg) + + num_data_slices = sp.get("numDataSlices") + if num_data_slices is not None: + if (not isinstance(num_data_slices, int) or isinstance(num_data_slices, bool) + or num_data_slices <= 0): + msg = "num_data_slices must be a positive integer." + raise exceptions.InvalidInput(msg) + elif sp.get("protectionScheme") is None: + msg = ("Either num_data_slices or protection_scheme must be specified " "for creating a storage pool.") raise exceptions.InvalidInput(msg) @@ -95,7 +106,10 @@ def create(self, sp): if "compressionMethod" in sp: params["compressionMethod"] = sp["compressionMethod"] - if sp["protectionScheme"] == "TwoPlusTwo": + if sp.get("numDataSlices") is not None: + params["numDataSlices"] = sp["numDataSlices"] + params["numProtectionSlices"] = sp.get("numProtectionSlices", 2) + elif sp.get("protectionScheme") == "TwoPlusTwo": params["numDataSlices"] = 2 params["numProtectionSlices"] = 2 else: diff --git a/tests/gen2/test_storage_pool.py b/tests/gen2/test_storage_pool.py index fcbdbee..fc642fb 100644 --- a/tests/gen2/test_storage_pool.py +++ b/tests/gen2/test_storage_pool.py @@ -17,6 +17,8 @@ # pylint: disable=invalid-name,too-many-public-methods +from unittest import mock + from PyPowerFlex import exceptions from tests.common import PyPowerFlexTestCase @@ -470,3 +472,114 @@ def test_storage_pool_rename_bad_status(self): self.client.storage_pool.rename, self.fake_sp_id, name='new_name') + + # ------------------------------------------------------------------ + # Flexible data slices (FEAT-23070) — added by TDD Writer (Stage 07) + # ------------------------------------------------------------------ + + def _capture_create_payload(self, sp): + """Call create() while capturing the payload sent to _create_entity.""" + captured = {} + + def fake_create_entity(params=None): + captured.update(params or {}) + return {'id': self.fake_sp_id} + + with mock.patch.object(self.client.storage_pool, '_create_entity', + side_effect=fake_create_entity), \ + mock.patch.object(self.client.storage_pool, 'update', + return_value=(False, {'id': self.fake_sp_id})): + self.client.storage_pool.create(sp) + return captured + + def test_storage_pool_create_with_num_data_slices(self): + """U-001: explicit num_data_slices is sent verbatim in the create payload.""" + sp = { + 'name': self.fake_sp_name, + 'protectionDomainId': self.fake_pd_id, + 'deviceGroupId': '1', + 'numDataSlices': 4, + 'physicalSizeGB': 10, + 'compressionMethod': 'None', + } + captured = self._capture_create_payload(sp) + self.assertEqual(captured.get('numDataSlices'), 4) + self.assertEqual(captured.get('numProtectionSlices'), 2) + + def test_storage_pool_create_with_explicit_protection_slices(self): + """U-004: explicit numProtectionSlices is honored.""" + sp = { + 'name': self.fake_sp_name, + 'protectionDomainId': self.fake_pd_id, + 'deviceGroupId': '1', + 'numDataSlices': 6, + 'numProtectionSlices': 3, + 'physicalSizeGB': 10, + } + captured = self._capture_create_payload(sp) + self.assertEqual(captured.get('numDataSlices'), 6) + self.assertEqual(captured.get('numProtectionSlices'), 3) + + def test_storage_pool_create_preset_twoplustwo(self): + """U-002: backward-compat preset TwoPlusTwo -> 2/2.""" + sp = { + 'name': self.fake_sp_name, + 'protectionDomainId': self.fake_pd_id, + 'deviceGroupId': '1', + 'protectionScheme': 'TwoPlusTwo', + 'physicalSizeGB': 10, + } + captured = self._capture_create_payload(sp) + self.assertEqual(captured.get('numDataSlices'), 2) + self.assertEqual(captured.get('numProtectionSlices'), 2) + + def test_storage_pool_create_preset_eightplustwo(self): + """U-003: backward-compat preset EightPlusTwo -> 8/2.""" + sp = { + 'name': self.fake_sp_name, + 'protectionDomainId': self.fake_pd_id, + 'deviceGroupId': '1', + 'protectionScheme': 'EightPlusTwo', + 'physicalSizeGB': 10, + } + captured = self._capture_create_payload(sp) + self.assertEqual(captured.get('numDataSlices'), 8) + self.assertEqual(captured.get('numProtectionSlices'), 2) + + def test_check_create_params_invalid_num_data_slices(self): + """U-005: num_data_slices <= 0 raises InvalidInput.""" + sp = { + 'name': self.fake_sp_name, + 'protectionDomainId': self.fake_pd_id, + 'deviceGroupId': '1', + 'protectionScheme': 'TwoPlusTwo', + 'numDataSlices': 0, + 'useAllAvailableCapacity': True, + } + self.assertRaises(exceptions.InvalidInput, + self.client.storage_pool.check_create_params, + sp) + + def test_check_create_params_requires_slices_or_scheme(self): + """U-006: neither num_data_slices nor protectionScheme raises InvalidInput.""" + sp = { + 'name': self.fake_sp_name, + 'protectionDomainId': self.fake_pd_id, + 'deviceGroupId': '1', + 'useAllAvailableCapacity': True, + } + self.assertRaises(exceptions.InvalidInput, + self.client.storage_pool.check_create_params, + sp) + + def test_check_create_params_num_data_slices_only(self): + """U-007: num_data_slices without protectionScheme is valid (one-of).""" + sp = { + 'name': self.fake_sp_name, + 'protectionDomainId': self.fake_pd_id, + 'deviceGroupId': '1', + 'numDataSlices': 4, + 'useAllAvailableCapacity': True, + } + # Should not raise once one-of validation is implemented. + self.client.storage_pool.check_create_params(sp)