diff --git a/dracclient/client.py b/dracclient/client.py index 1057d44..34d4aec 100644 --- a/dracclient/client.py +++ b/dracclient/client.py @@ -1043,32 +1043,33 @@ class DRACClient(object): def change_physical_disk_state(self, mode, controllers_to_physical_disk_ids=None): - """Convert disks RAID status and return a list of controller IDs + """Convert disks RAID status - Builds a list of controller ids that have had disks converted to the - specified RAID status by: - - Examining all the disks in the system and filtering out any that are - not attached to a RAID/BOSS controller. - - Inspect the controllers' disks to see if there are any that need to - be converted, if so convert them. If a disk is already in the desired - status the disk is ignored. Also check for failed or unknown disk - statuses and raise an exception where appropriate. - - Return a list of controller IDs for controllers whom have had any of - their disks converted, and whether a reboot is required. + This method intelligently converts the requested physical disks from + RAID to JBOD or vice versa. It does this by only converting the + disks that are not already in the correct state. - The caller typically should then create a config job for the list of - controllers returned to finalize the RAID configuration. - - :param mode: constants.RaidStatus enumeration used to determine what - raid status to check for. + :param mode: constants.RaidStatus enumeration that indicates the mode + to change the disks to. :param controllers_to_physical_disk_ids: Dictionary of controllers and - corresponding disk ids we are inspecting and creating jobs for - when needed. - :returns: a dict containing the following key/values: + corresponding disk ids to convert to the requested mode. + :returns: a dictionary containing: + - conversion_results, a dictionary that maps controller ids + to the conversion results for that controller. The + conversion results are a dict that contains: + - The is_commit_required key with the value always set to + True indicating that a config job must be created to + complete disk conversion. + - The is_reboot_required key with a RebootRequired + enumerated value indicating whether the server must be + rebooted to complete disk conversion. + Also contained in the main dict are the following key/values, + which are deprecated, should not be used, and will be removed + in a future release: - is_reboot_required, a boolean stating whether a reboot is - required or not. + required or not. - commit_required_ids, a list of controller ids that will - need to commit their pending RAID changes via a config job. + need to commit their pending RAID changes via a config job. :raises: DRACOperationFailed on error reported back by the DRAC and the exception message does not contain NOT_SUPPORTED_MSG constant. :raises: Exception on unknown error. diff --git a/dracclient/resources/raid.py b/dracclient/resources/raid.py index 058e3e3..eab2351 100644 --- a/dracclient/resources/raid.py +++ b/dracclient/resources/raid.py @@ -12,6 +12,7 @@ # under the License. import collections +import copy import logging from dracclient import constants @@ -557,15 +558,17 @@ class RAIDManagement(object): :param mode: constants.RaidStatus enumeration used to determine what raid status to check for. :param physical_disks: all physical disks - :param controllers_to_physical_disk_ids: Dictionary of controllers - we are inspecting and creating jobs for when needed. If - needed modify this dict so that only drives that need to - be changed to RAID or JBOD are in the list of disk keys - for corresponding controller. + :param controllers_to_physical_disk_ids: Dictionary of controllers and + corresponding disk ids to convert to the requested mode. + :returns: a dictionary mapping controller FQDDs to the list of + physical disks that need to be converted for that controller. :raises: ValueError: Exception message will list failed drives and drives whose state cannot be changed at this time, drive state is not "ready" or "non-RAID". """ + controllers_to_physical_disk_ids = copy.deepcopy( + controllers_to_physical_disk_ids) + p_disk_id_to_status = {} for physical_disk in physical_disks: p_disk_id_to_status[physical_disk.id] = physical_disk.raid_status @@ -624,34 +627,37 @@ class RAIDManagement(object): raise ValueError(error_msg) + return controllers_to_physical_disk_ids + def change_physical_disk_state(self, mode, controllers_to_physical_disk_ids=None): - """Convert disks RAID status and return a list of controller IDs + """Convert disks RAID status - Builds a list of controller ids that have had disks converted to the - specified RAID status by: - - Examining all the disks in the system and filtering out any that are - not attached to a RAID/BOSS controller. - - Inspect the controllers' disks to see if there are any that need to - be converted, if so convert them. If a disk is already in the desired - status the disk is ignored. Also check for failed or unknown disk - statuses and raise an exception where appropriate. - - Return a list of controller IDs for controllers whom have had any of - their disks converted, and whether a reboot is required. + This method intelligently converts the requested physical disks from + RAID to JBOD or vice versa. It does this by only converting the + disks that are not already in the correct state. - The caller typically should then create a config job for the list of - controllers returned to finalize the RAID configuration. - - :param mode: constants.RaidStatus enumeration used to determine what - raid status to check for. + :param mode: constants.RaidStatus enumeration that indicates the mode + to change the disks to. :param controllers_to_physical_disk_ids: Dictionary of controllers and - corresponding disk ids we are inspecting and creating jobs for - when needed. - :returns: a dict containing the following key/values: + corresponding disk ids to convert to the requested mode. + :returns: a dictionary containing: + - conversion_results, a dictionary that maps controller ids + to the conversion results for that controller. The + conversion results are a dict that contains: + - The is_commit_required key with the value always set to + True indicating that a config job must be created to + complete disk conversion. + - The is_reboot_required key with a RebootRequired + enumerated value indicating whether the server must be + rebooted to complete disk conversion. + Also contained in the main dict are the following key/values, + which are deprecated, should not be used, and will be removed + in a future release: - is_reboot_required, a boolean stating whether a reboot is - required or not. + required or not. - commit_required_ids, a list of controller ids that will - need to commit their pending RAID changes via a config job. + need to commit their pending RAID changes via a config job. :raises: DRACOperationFailed on error reported back by the DRAC and the exception message does not contain NOT_SUPPORTED_MSG constant. :raises: Exception on unknown error. @@ -678,13 +684,14 @@ class RAIDManagement(object): Raise exception if there are any failed drives or drives not in status 'ready' or 'non-RAID' ''' - self._check_disks_status(mode, physical_disks, - controllers_to_physical_disk_ids) + final_ctls_to_phys_disk_ids = self._check_disks_status( + mode, physical_disks, controllers_to_physical_disk_ids) is_reboot_required = False controllers = [] + controllers_to_results = {} for controller, physical_disk_ids \ - in controllers_to_physical_disk_ids.items(): + in final_ctls_to_phys_disk_ids.items(): if physical_disk_ids: LOG.debug("Converting the following disks to {} on RAID " "controller {}: {}".format( @@ -697,22 +704,39 @@ class RAIDManagement(object): if constants.NOT_SUPPORTED_MSG in str(ex): LOG.debug("Controller {} does not support " "JBOD mode".format(controller)) - pass + controllers_to_results[controller] = \ + utils.build_return_dict( + doc=None, + resource_uri=None, + is_commit_required_value=False, + is_reboot_required_value=constants. + RebootRequired.false) else: raise else: - if conversion_results: - reboot_true = constants.RebootRequired.true - reboot_optional = constants.RebootRequired.optional - _is_reboot_required = \ - conversion_results["is_reboot_required"] - is_reboot_required = is_reboot_required \ - or (_is_reboot_required - in [reboot_true, reboot_optional]) - if conversion_results["is_commit_required"]: - controllers.append(controller) + controllers_to_results[controller] = conversion_results - return {'is_reboot_required': is_reboot_required, + # Remove the code below when is_reboot_required and + # commit_required_ids are deprecated + reboot_true = constants.RebootRequired.true + reboot_optional = constants.RebootRequired.optional + _is_reboot_required = \ + conversion_results["is_reboot_required"] + is_reboot_required = is_reboot_required \ + or (_is_reboot_required + in [reboot_true, reboot_optional]) + controllers.append(controller) + else: + controllers_to_results[controller] = \ + utils.build_return_dict( + doc=None, + resource_uri=None, + is_commit_required_value=False, + is_reboot_required_value=constants. + RebootRequired.false) + + return {'conversion_results': controllers_to_results, + 'is_reboot_required': is_reboot_required, 'commit_required_ids': controllers} def is_realtime_supported(self, raid_controller_fqdd): diff --git a/dracclient/tests/test_raid.py b/dracclient/tests/test_raid.py index a93a77a..f582792 100644 --- a/dracclient/tests/test_raid.py +++ b/dracclient/tests/test_raid.py @@ -991,13 +991,13 @@ class ClientRAIDManagementTestCase(base.BaseTest): raid_mgt._check_disks_status, mode, physical_disks, - self.controllers_to_physical_disk_ids.copy()) + self.controllers_to_physical_disk_ids) mode = constants.RaidStatus.jbod self.assertRaises(ValueError, raid_mgt._check_disks_status, mode, physical_disks, - self.controllers_to_physical_disk_ids.copy()) + self.controllers_to_physical_disk_ids) def test_check_disks_status_fail(self, mock_requests): mode = constants.RaidStatus.raid @@ -1009,13 +1009,13 @@ class ClientRAIDManagementTestCase(base.BaseTest): raid_mgt._check_disks_status, mode, physical_disks, - self.controllers_to_physical_disk_ids.copy()) + self.controllers_to_physical_disk_ids) mode = constants.RaidStatus.jbod self.assertRaises(ValueError, raid_mgt._check_disks_status, mode, physical_disks, - self.controllers_to_physical_disk_ids.copy()) + self.controllers_to_physical_disk_ids) def test_check_disks_status_no_change(self, mock_requests): raid_mgt = self.drac_client._raid_mgmt @@ -1023,11 +1023,8 @@ class ClientRAIDManagementTestCase(base.BaseTest): physical_disks = [self.disk_1, self.disk_2, self.disk_3, self.disk_4] - raid_cntl_to_phys_disk_ids = (self.controllers_to_physical_disk_ids. - copy()) - - raid_mgt._check_disks_status(mode, physical_disks, - raid_cntl_to_phys_disk_ids) + raid_cntl_to_phys_disk_ids = raid_mgt._check_disks_status( + mode, physical_disks, self.controllers_to_physical_disk_ids) raid_len = len(raid_cntl_to_phys_disk_ids['RAID.Integrated.1-1']) self.assertEqual(raid_len, 0) @@ -1037,10 +1034,8 @@ class ClientRAIDManagementTestCase(base.BaseTest): physical_disks = [disk_1_non_raid, disk_2_non_raid, self.disk_3, self.disk_4] - jbod_cntl_to_phys_disk_ids = (self.controllers_to_physical_disk_ids. - copy()) - raid_mgt._check_disks_status(mode, physical_disks, - jbod_cntl_to_phys_disk_ids) + jbod_cntl_to_phys_disk_ids = raid_mgt._check_disks_status( + mode, physical_disks, self.controllers_to_physical_disk_ids) jbod_len = len(jbod_cntl_to_phys_disk_ids['RAID.Integrated.1-1']) self.assertEqual(jbod_len, 0) @@ -1049,11 +1044,8 @@ class ClientRAIDManagementTestCase(base.BaseTest): mode = constants.RaidStatus.jbod physical_disks = [self.disk_1, self.disk_2, self.disk_3, self.disk_4] - jbod_cntl_to_phys_disk_ids = (self.controllers_to_physical_disk_ids. - copy()) - - raid_mgt._check_disks_status(mode, physical_disks, - jbod_cntl_to_phys_disk_ids) + jbod_cntl_to_phys_disk_ids = raid_mgt._check_disks_status( + mode, physical_disks, self.controllers_to_physical_disk_ids) jbod_len = len(jbod_cntl_to_phys_disk_ids['RAID.Integrated.1-1']) self.assertEqual(jbod_len, 2) @@ -1062,10 +1054,8 @@ class ClientRAIDManagementTestCase(base.BaseTest): disk_2_non_raid = self.disk_2._replace(raid_status='non-RAID') physical_disks = [disk_1_non_raid, disk_2_non_raid, self.disk_3, self.disk_4] - raid_cntl_to_phys_disk_ids = (self.controllers_to_physical_disk_ids. - copy()) - raid_mgt._check_disks_status(mode, physical_disks, - raid_cntl_to_phys_disk_ids) + raid_cntl_to_phys_disk_ids = raid_mgt._check_disks_status( + mode, physical_disks, self.controllers_to_physical_disk_ids) raid_len = len(raid_cntl_to_phys_disk_ids['RAID.Integrated.1-1']) self.assertEqual(raid_len, 2) @@ -1080,13 +1070,13 @@ class ClientRAIDManagementTestCase(base.BaseTest): raid_mgt._check_disks_status, mode, physical_disks, - self.controllers_to_physical_disk_ids.copy()) + self.controllers_to_physical_disk_ids) mode = constants.RaidStatus.jbod self.assertRaises(ValueError, raid_mgt._check_disks_status, mode, physical_disks, - self.controllers_to_physical_disk_ids.copy()) + self.controllers_to_physical_disk_ids) @mock.patch.object(dracclient.client.WSManClient, 'wait_until_idrac_is_ready', spec_set=True, @@ -1102,15 +1092,21 @@ class ClientRAIDManagementTestCase(base.BaseTest): mock_requests.post( 'https://1.2.3.4:443/wsman', text=test_utils.RAIDEnumerations[uris.DCIM_PhysicalDiskView]['ok']) - mock_convert_physical_disks.return_value = {'is_commit_required': True, - 'is_reboot_required': - constants.RebootRequired - .true} - cntl_to_phys_d_ids = self.controllers_to_physical_disk_ids + cvt_phys_disks_return_value = {'is_commit_required': True, + 'is_reboot_required': constants. + RebootRequired.true} + mock_convert_physical_disks.return_value = cvt_phys_disks_return_value + + expected_return_value = {'RAID.Integrated.1-1': + cvt_phys_disks_return_value, + 'AHCI.Integrated.1-1': + cvt_phys_disks_return_value} results = self.drac_client.change_physical_disk_state( - mode, cntl_to_phys_d_ids) + mode, self.controllers_to_physical_disk_ids) self.assertTrue(results["is_reboot_required"]) self.assertEqual(len(results["commit_required_ids"]), 2) + self.assertEqual(results['conversion_results'], + expected_return_value) @mock.patch.object(dracclient.resources.raid.RAIDManagement, 'list_physical_disks', spec_set=True, @@ -1128,38 +1124,44 @@ class ClientRAIDManagementTestCase(base.BaseTest): physical_disks = [disk_1_non_raid, disk_2_non_raid, self.disk_3, self.disk_4] mock_list_physical_disks.return_value = physical_disks - mock_convert_physical_disks.return_value = {'is_commit_required': True, - 'is_reboot_required': - constants.RebootRequired - .true} - cntl_to_phys_d_ids = self.controllers_to_physical_disk_ids + boss_return_value = {'is_commit_required': False, + 'is_reboot_required': + constants.RebootRequired.false} + raid_return_value = {'is_commit_required': True, + 'is_reboot_required': + constants.RebootRequired.true} + mock_convert_physical_disks.return_value = raid_return_value + results = self.drac_client.change_physical_disk_state( - mode, cntl_to_phys_d_ids) + mode, self.controllers_to_physical_disk_ids) self.assertTrue(results["is_reboot_required"]) self.assertEqual(len(results["commit_required_ids"]), 1) + self.assertEqual(len(results['conversion_results']), 2) + self.assertEqual(results['conversion_results']['AHCI.Integrated.1-1'], + boss_return_value) + self.assertEqual(results['conversion_results']['RAID.Integrated.1-1'], + raid_return_value) @mock.patch.object(dracclient.resources.raid.RAIDManagement, 'list_physical_disks', spec_set=True, autospec=True) - @mock.patch.object(dracclient.resources.raid.RAIDManagement, - 'convert_physical_disks', spec_set=True, - autospec=True) def test_change_physical_disk_state_none( self, mock_requests, - mock_convert_physical_disks, mock_list_physical_disks): mode = constants.RaidStatus.raid physical_disks = [self.disk_1, self.disk_2, self.disk_3, self.disk_4] - mock_convert_physical_disks.return_value = {'is_commit_required': True, - 'is_reboot_required': - constants.RebootRequired - .true} mock_list_physical_disks.return_value = physical_disks - cntl_to_phys_d_ids = self.controllers_to_physical_disk_ids + expected_return_value = {'is_commit_required': False, + 'is_reboot_required': + constants.RebootRequired.false} results = self.drac_client.change_physical_disk_state( - mode, cntl_to_phys_d_ids) + mode, self.controllers_to_physical_disk_ids) self.assertFalse(results["is_reboot_required"]) self.assertEqual(len(results["commit_required_ids"]), 0) + self.assertEqual(results['conversion_results']['RAID.Integrated.1-1'], + expected_return_value) + self.assertEqual(results['conversion_results']['AHCI.Integrated.1-1'], + expected_return_value) @mock.patch.object(dracclient.resources.raid.RAIDManagement, 'list_physical_disks', spec_set=True, @@ -1179,11 +1181,17 @@ class ClientRAIDManagementTestCase(base.BaseTest): physical_disks = [disk_1_non_raid, disk_2_non_raid, self.disk_3, self.disk_4] mock_list_physical_disks.return_value = physical_disks - cntl_to_phys_d_ids = self.controllers_to_physical_disk_ids + expected_return_value = {'is_commit_required': False, + 'is_reboot_required': + constants.RebootRequired.false} results = self.drac_client.change_physical_disk_state( - mode, cntl_to_phys_d_ids) + mode, self.controllers_to_physical_disk_ids) self.assertFalse(results["is_reboot_required"]) self.assertEqual(len(results["commit_required_ids"]), 0) + self.assertEqual(results['conversion_results']['RAID.Integrated.1-1'], + expected_return_value) + self.assertEqual(results['conversion_results']['AHCI.Integrated.1-1'], + expected_return_value) @mock.patch.object(dracclient.resources.raid.RAIDManagement, 'list_physical_disks', spec_set=True, @@ -1203,12 +1211,12 @@ class ClientRAIDManagementTestCase(base.BaseTest): physical_disks = [disk_1_non_raid, disk_2_non_raid, self.disk_3, self.disk_4] mock_list_physical_disks.return_value = physical_disks - cntl_to_phys_d_ids = self.controllers_to_physical_disk_ids self.assertRaisesRegexp( exceptions.DRACOperationFailed, "OTHER_MESSAGE", self.drac_client.change_physical_disk_state, - mode, cntl_to_phys_d_ids) + mode, + self.controllers_to_physical_disk_ids) @mock.patch.object(dracclient.resources.raid.RAIDManagement, 'list_physical_disks', spec_set=True, @@ -1227,11 +1235,12 @@ class ClientRAIDManagementTestCase(base.BaseTest): physical_disks = [disk_1_non_raid, disk_2_non_raid, self.disk_3, self.disk_4] mock_list_physical_disks.return_value = physical_disks - cntl_to_phys_d_ids = self.controllers_to_physical_disk_ids self.assertRaisesRegexp( - Exception, "SOMETHING_BAD_HAPPENED", + Exception, + "SOMETHING_BAD_HAPPENED", self.drac_client.change_physical_disk_state, - mode, cntl_to_phys_d_ids) + mode, + self.controllers_to_physical_disk_ids) @mock.patch.object(dracclient.client.WSManClient, 'wait_until_idrac_is_ready', spec_set=True, @@ -1285,45 +1294,6 @@ class ClientRAIDManagementTestCase(base.BaseTest): self.assertFalse(results["is_reboot_required"]) self.assertEqual(len(results["commit_required_ids"]), 0) - @mock.patch.object(dracclient.client.WSManClient, - 'wait_until_idrac_is_ready', spec_set=True, - autospec=True) - @mock.patch.object(dracclient.resources.raid.RAIDManagement, - 'list_physical_disks', spec_set=True, - autospec=True) - @mock.patch.object(dracclient.resources.raid.RAIDManagement, - 'convert_physical_disks', spec_set=True, - autospec=True) - def test_change_physical_disk_state_conversion_return_values( - self, mock_requests, - mock_convert_physical_disks, - mock_list_physical_disks, - mock_wait_until_idrac_is_ready): - mock_requests.post( - 'https://1.2.3.4:443/wsman', - text=test_utils.RAIDEnumerations[uris.DCIM_ControllerView]['ok']) - mode = constants.RaidStatus.jbod - physical_disks = [self.disk_1, self.disk_2, self.disk_3, self.disk_4] - '''Test all logic branches for 100% coverage, it is unlikely - convert_physical_disks() will return empty dict but we do check - for this case in change_physical_disk_state()''' - mock_convert_physical_disks.return_value = {} - mock_list_physical_disks.return_value = physical_disks - results = self.drac_client.change_physical_disk_state(mode) - self.assertFalse(results["is_reboot_required"]) - self.assertEqual(len(results["commit_required_ids"]), 0) - '''Where convert_physical_disks() does not require a commit after - executing, unlikely case but provides 100% code coverage of all - logic branches.''' - mock_convert_physical_disks.return_value = {'is_commit_required': - False, - 'is_reboot_required': - constants.RebootRequired - .false} - results = self.drac_client.change_physical_disk_state(mode) - self.assertFalse(results["is_reboot_required"]) - self.assertEqual(len(results["commit_required_ids"]), 0) - @mock.patch.object(dracclient.client.WSManClient, 'wait_until_idrac_is_ready', spec_set=True, autospec=True)