Merge "Add realtime support to drive conversion"

dracclient/client.py

@@ -1043,28 +1043,29 @@ 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
+        This method intelligently converts the requested physical disks from
-        specified RAID status by:
+        RAID to JBOD or vice versa.  It does this by only converting the
-        - Examining all the disks in the system and filtering out any that are
+        disks that are not already in the correct state.
-          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.
 
-        The caller typically should then create a config job for the list of
+        :param mode: constants.RaidStatus enumeration that indicates the mode
-        controllers returned to finalize the RAID configuration.
+                     to change the disks to.
-
-        :param mode: constants.RaidStatus enumeration used to determine what
-                     raid status to check for.
         :param controllers_to_physical_disk_ids: Dictionary of controllers and
-               corresponding disk ids we are inspecting and creating jobs for
+               corresponding disk ids to convert to the requested mode.
-               when needed.
+        :returns: a dictionary containing:
-        :returns: a dict containing the following key/values:
+                  - 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.
                   - commit_required_ids, a list of controller ids that will

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
+        :param controllers_to_physical_disk_ids: Dictionary of controllers and
-                     we are inspecting and creating jobs for when needed. If
+               corresponding disk ids to convert to the requested mode.
-                     needed modify this dict so that only drives that need to
+        :returns: a dictionary mapping controller FQDDs to the list of
-                     be changed to RAID or JBOD are in the list of disk keys
+                  physical disks that need to be converted for that controller.
-                     for corresponding 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,30 +627,33 @@ 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
+        This method intelligently converts the requested physical disks from
-        specified RAID status by:
+        RAID to JBOD or vice versa.  It does this by only converting the
-        - Examining all the disks in the system and filtering out any that are
+        disks that are not already in the correct state.
-          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.
 
-        The caller typically should then create a config job for the list of
+        :param mode: constants.RaidStatus enumeration that indicates the mode
-        controllers returned to finalize the RAID configuration.
+                     to change the disks to.
-
-        :param mode: constants.RaidStatus enumeration used to determine what
-                     raid status to check for.
         :param controllers_to_physical_disk_ids: Dictionary of controllers and
-               corresponding disk ids we are inspecting and creating jobs for
+               corresponding disk ids to convert to the requested mode.
-               when needed.
+        :returns: a dictionary containing:
-        :returns: a dict containing the following key/values:
+                  - 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.
                   - commit_required_ids, a list of controller ids that will
@@ -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,
+        final_ctls_to_phys_disk_ids = self._check_disks_status(
-                                 controllers_to_physical_disk_ids)
+                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,11 +704,20 @@ 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:
+                    controllers_to_results[controller] = conversion_results
+
+                    # 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 = \
@@ -709,10 +725,18 @@ class RAIDManagement(object):
                     is_reboot_required = is_reboot_required \
                         or (_is_reboot_required
                             in [reboot_true, reboot_optional])
-                        if conversion_results["is_commit_required"]:
                     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 {'is_reboot_required': is_reboot_required,
+        return {'conversion_results': controllers_to_results,
+                'is_reboot_required': is_reboot_required,
                 'commit_required_ids': controllers}
 
     def is_realtime_supported(self, raid_controller_fqdd):

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.
+        raid_cntl_to_phys_disk_ids = raid_mgt._check_disks_status(
-                                      copy())
+            mode,  physical_disks, self.controllers_to_physical_disk_ids)
-
-        raid_mgt._check_disks_status(mode,  physical_disks,
-                                     raid_cntl_to_phys_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.
+        jbod_cntl_to_phys_disk_ids = raid_mgt._check_disks_status(
-                                      copy())
+            mode,  physical_disks, self.controllers_to_physical_disk_ids)
-        raid_mgt._check_disks_status(mode,  physical_disks,
-                                     jbod_cntl_to_phys_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.
+        jbod_cntl_to_phys_disk_ids = raid_mgt._check_disks_status(
-                                      copy())
+            mode, physical_disks, self.controllers_to_physical_disk_ids)
-
-        raid_mgt._check_disks_status(mode, physical_disks,
-                                     jbod_cntl_to_phys_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.
+        raid_cntl_to_phys_disk_ids = raid_mgt._check_disks_status(
-                                      copy())
+            mode,  physical_disks, self.controllers_to_physical_disk_ids)
-        raid_mgt._check_disks_status(mode,  physical_disks,
-                                     raid_cntl_to_phys_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,
+        cvt_phys_disks_return_value = {'is_commit_required': True,
-                                                    'is_reboot_required':
+                                       'is_reboot_required': constants.
-                                                    constants.RebootRequired
+                                       RebootRequired.true}
-                                                    .true}
+        mock_convert_physical_disks.return_value = cvt_phys_disks_return_value
-        cntl_to_phys_d_ids = self.controllers_to_physical_disk_ids
+
+        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,
+        boss_return_value = {'is_commit_required': False,
                              'is_reboot_required':
-                                                    constants.RebootRequired
+                             constants.RebootRequired.false}
-                                                    .true}
+        raid_return_value = {'is_commit_required': True,
-        cntl_to_phys_d_ids = self.controllers_to_physical_disk_ids
+                             '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)