DRAC: Fix a failure to create virtual disk bug

Certain RAID controllers (PERC H730P) require physical disks
to be switched from non-RAID (JBOD) mode to RAID mode to be
included in a virtual disk.  When this conversion happens,
the available free space on the physical disk is reduced due
to some space being allocated to RAID mode housekeeping.
If the user requests a virtual disk (a RAID 1 for example)
with a size close to the max size of the physical disks when
they are in JBOD mode, then creation of the virtual disk
following conversion of the physical disks from JBOD to RAID
mode will fail since there is not enough space due to the
space used by RAID mode housekeeping.
This patch works around this issue by recalculating the RAID
volume size after physical disk conversion has completed and
the free space on the converted drives is known.  Note that
this may result in a virtual disk that is slightly smaller
than the requested size, but still the max size that the
drives can support.

Change-Id: I720ab15e811f498aa15b88bfe8bb35fc49df292b
Story: 2007359
Task: 38912

ironic/drivers/modules/drac/raid.py

@@ -839,6 +839,40 @@ def _create_config_job(node, controller, reboot=False, realtime=False,
             'raid_config_parameters': raid_config_parameters}
 
 
+def _validate_volume_size(node, logical_disks):
+    new_physical_disks = list_physical_disks(node)
+    free_space_mb = {}
+    new_processed_volumes = []
+    for disk in new_physical_disks:
+        free_space_mb[disk] = disk.free_size_mb
+
+    for logical_disk in logical_disks:
+        selected_disks = [disk for disk in new_physical_disks
+                          if disk.id in logical_disk['physical_disks']]
+
+        spans_count = _calculate_spans(
+            logical_disk['raid_level'], len(selected_disks))
+
+        new_max_vol_size_mb = _max_volume_size_mb(
+            logical_disk['raid_level'],
+            selected_disks,
+            free_space_mb,
+            spans_count=spans_count)
+
+        if logical_disk['size_mb'] > new_max_vol_size_mb:
+            logical_disk['size_mb'] = new_max_vol_size_mb
+            LOG.info("Logical size does not match so calculating volume "
+                     "properties for current logical_disk")
+            _calculate_volume_props(
+                logical_disk, new_physical_disks, free_space_mb)
+            new_processed_volumes.append(logical_disk)
+
+    if new_processed_volumes:
+        return new_processed_volumes
+
+    return logical_disks
+
+
 def _commit_to_controllers(node, controllers, substep="completed"):
     """Commit changes to RAID controllers on the node.
 
@@ -931,6 +965,13 @@ def _create_virtual_disks(task, node):
     logical_disks_to_create = node.driver_internal_info[
         'logical_disks_to_create']
 
+    # Check valid properties attached to voiume after drives conversion
+    isVolValidationNeeded = node.driver_internal_info[
+        'volume_validation']
+    if isVolValidationNeeded:
+        logical_disks_to_create = _validate_volume_size(
+            node, logical_disks_to_create)
+
     controllers = list()
     for logical_disk in logical_disks_to_create:
         controller = dict()
@@ -1076,8 +1117,6 @@ class DracWSManRAID(base.RAIDInterface):
         driver_internal_info = node.driver_internal_info
         driver_internal_info[
             "logical_disks_to_create"] = logical_disks_to_create
-        node.driver_internal_info = driver_internal_info
-        node.save()
 
         commit_results = None
         if logical_disks_to_create:
@@ -1091,6 +1130,11 @@ class DracWSManRAID(base.RAIDInterface):
                 controllers_to_physical_disk_ids,
                 substep="create_virtual_disks")
 
+        volume_validation = True if commit_results else False
+        driver_internal_info['volume_validation'] = volume_validation
+        node.driver_internal_info = driver_internal_info
+        node.save()
+
         if commit_results:
             return commit_results
         else:

ironic/tests/unit/drivers/modules/drac/test_raid.py

@@ -727,6 +727,74 @@ class DracCreateRaidConfigurationHelpersTestCase(test_utils.BaseDracTest):
         self.assertEqual(expected_physical_disk_ids,
                          logical_disks[0]['physical_disks'])
 
+    @mock.patch.object(drac_common, 'get_drac_client', spec_set=True,
+                       autospec=True)
+    @mock.patch.object(drac_raid, 'list_physical_disks', autospec=True)
+    def test__validate_volume_size_requested_more_than_actual_size(
+            self, mock_list_physical_disks, mock_get_drac_client):
+        mock_client = mock.Mock()
+        mock_get_drac_client.return_value = mock_client
+        self.logical_disk = {
+            'physical_disks': [
+                'Disk.Bay.0:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.1:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.2:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.3:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.4:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.5:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.6:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.7:Enclosure.Internal.0-1:RAID.Integrated.1-1'],
+            'raid_level': '1+0', 'is_root_volume': True,
+            'size_mb': 102400000,
+            'controller': 'RAID.Integrated.1-1'}
+
+        self.logical_disks = [self.logical_disk.copy()]
+        self.target_raid_configuration = {'logical_disks': self.logical_disks}
+        self.node.target_raid_config = self.target_raid_configuration
+        self.node.save()
+
+        physical_disks = self._generate_physical_disks()
+        mock_list_physical_disks.return_value = physical_disks
+
+        processed_logical_disks = drac_raid._validate_volume_size(
+            self.node, self.node.target_raid_config['logical_disks'])
+
+        self.assertEqual(2287104, processed_logical_disks[0]['size_mb'])
+
+    @mock.patch.object(drac_common, 'get_drac_client', spec_set=True,
+                       autospec=True)
+    @mock.patch.object(drac_raid, 'list_physical_disks', autospec=True)
+    def test__validate_volume_size_requested_less_than_actual_size(
+            self, mock_list_physical_disks, mock_get_drac_client):
+        mock_client = mock.Mock()
+        mock_get_drac_client.return_value = mock_client
+        self.logical_disk = {
+            'physical_disks': [
+                'Disk.Bay.0:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.1:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.2:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.3:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.4:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.5:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.6:Enclosure.Internal.0-1:RAID.Integrated.1-1',
+                'Disk.Bay.7:Enclosure.Internal.0-1:RAID.Integrated.1-1'],
+            'raid_level': '1+0', 'is_root_volume': True,
+            'size_mb': 204800,
+            'controller': 'RAID.Integrated.1-1'}
+
+        self.logical_disks = [self.logical_disk.copy()]
+        self.target_raid_configuration = {'logical_disks': self.logical_disks}
+        self.node.target_raid_config = self.target_raid_configuration
+        self.node.save()
+
+        physical_disks = self._generate_physical_disks()
+        mock_list_physical_disks.return_value = physical_disks
+
+        processed_logical_disks = drac_raid._validate_volume_size(
+            self.node, self.node.target_raid_config['logical_disks'])
+
+        self.assertEqual(self.logical_disk, processed_logical_disks[0])
+
 
 class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
 
@@ -860,6 +928,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
             self.assertEqual(1, mock_change_physical_disk_state.call_count)
 
             self.node.refresh()
+            self.assertEqual(True,
+                             task.node.driver_internal_info[
+                                 'volume_validation'])
             self.assertEqual(next_substep,
                              task.node.driver_internal_info[
                                  'raid_config_substep'])
@@ -938,6 +1009,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
             self.assertEqual(1, mock_client.create_virtual_disk.call_count)
 
             self.node.refresh()
+            self.assertEqual(False,
+                             task.node.driver_internal_info[
+                                 'volume_validation'])
             self.assertEqual(next_substep,
                              task.node.driver_internal_info[
                                  'raid_config_substep'])
@@ -963,8 +1037,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
         mock_change_physical_disk_state.return_value = {
             'is_reboot_required': constants.RebootRequired.optional,
             'conversion_results': {
-                'RAID.Integrated.1-1': {'is_reboot_required': 'optional',
+                'RAID.Integrated.1-1': {
-                                        'is_commit_required': True}},
+                    'is_reboot_required': constants.RebootRequired.false,
+                    'is_commit_required': False}},
             'commit_required_ids': ['RAID.Integrated.1-1']}
         mock_commit_config.return_value = '42'
 
@@ -974,6 +1049,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
                 task, create_root_volume=False, create_nonroot_volumes=False,
                 delete_existing=False)
 
+            self.assertEqual(False,
+                             task.node.driver_internal_info[
+                                 'volume_validation'])
             self.assertEqual(0, mock_client.create_virtual_disk.call_count)
             self.assertEqual(0, mock_commit_config.call_count)
 
@@ -1039,6 +1117,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
                 task, create_root_volume=True, create_nonroot_volumes=False,
                 delete_existing=True)
 
+            self.assertEqual(True,
+                             task.node.driver_internal_info[
+                                 'volume_validation'])
             mock_commit_config.assert_called_with(
                 task.node, raid_controller='RAID.Integrated.1-1',
                 realtime=True, reboot=False)
@@ -1094,6 +1175,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
                 task, create_root_volume=True, create_nonroot_volumes=True,
                 delete_existing=False)
 
+            self.assertEqual(True,
+                             task.node.driver_internal_info[
+                                 'volume_validation'])
             # Commits to the controller
             mock_commit_config.assert_called_with(
                 mock.ANY, raid_controller='RAID.Integrated.1-1', reboot=False,
@@ -1150,6 +1234,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
                 task, create_root_volume=True, create_nonroot_volumes=True,
                 delete_existing=False)
 
+            self.assertEqual(True,
+                             task.node.driver_internal_info[
+                                 'volume_validation'])
             # Commits to the controller
             mock_commit_config.assert_called_with(
                 mock.ANY, raid_controller='RAID.Integrated.1-1', reboot=False,
@@ -1199,6 +1286,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
                 task, create_root_volume=True, create_nonroot_volumes=True,
                 delete_existing=False)
 
+            self.assertEqual(True,
+                             task.node.driver_internal_info[
+                                 'volume_validation'])
         self.node.refresh()
         self.assertEqual(['42'],
                          self.node.driver_internal_info['raid_config_job_ids'])
@@ -1246,6 +1336,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
                 task, create_root_volume=True, create_nonroot_volumes=True,
                 delete_existing=False)
 
+            self.assertEqual(True,
+                             task.node.driver_internal_info[
+                                 'volume_validation'])
             # Commits to the controller
             mock_commit_config.assert_called_with(
                 mock.ANY, raid_controller='RAID.Integrated.1-1',
@@ -1296,6 +1389,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
                 task, create_root_volume=True, create_nonroot_volumes=True,
                 delete_existing=False)
 
+            self.assertEqual(True,
+                             task.node.driver_internal_info[
+                                 'volume_validation'])
             # Commits to the controller
             mock_commit_config.assert_called_with(
                 mock.ANY, raid_controller='RAID.Integrated.1-1',
@@ -1353,6 +1449,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
                 task, create_root_volume=True, create_nonroot_volumes=True,
                 delete_existing=False)
 
+            self.assertEqual(True,
+                             task.node.driver_internal_info[
+                                 'volume_validation'])
             # Commits to the controller
             mock_commit_config.assert_called_with(
                 mock.ANY, raid_controller='RAID.Integrated.1-1', reboot=False,
@@ -1433,6 +1532,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
                 task, create_root_volume=True, create_nonroot_volumes=True,
                 delete_existing=False)
 
+            self.assertEqual(True,
+                             task.node.driver_internal_info[
+                                 'volume_validation'])
             # Commits to the controller
             mock_commit_config.assert_called_with(
                 mock.ANY, raid_controller='RAID.Integrated.1-1', reboot=False,
@@ -1535,6 +1637,9 @@ class DracRaidInterfaceTestCase(test_utils.BaseDracTest):
                 task, create_root_volume=True, create_nonroot_volumes=True,
                 delete_existing=False)
 
+            self.assertEqual(True,
+                             task.node.driver_internal_info[
+                                 'volume_validation'])
             # Commits to the controller
             mock_commit_config.assert_called_with(
                 mock.ANY, raid_controller='RAID.Integrated.1-1', reboot=False,

releasenotes/notes/fix-drives-conversion-before-raid-creation-ea1f7eb425f79f2f.yaml

@@ -0,0 +1,22 @@
+fixes:
+  - |
+    Certain RAID controllers (PERC H730P) require physical disks
+    to be switched from non-RAID (JBOD) mode to RAID mode to be
+    included in a virtual disk.  When this conversion happens,
+    the available free space on the physical disk is reduced due
+    to some space being allocated to RAID mode housekeeping.
+    If the user requests a virtual disk (a RAID 1 for example)
+    with a size close to the max size of the physical disks when
+    they are in JBOD mode, then creation of the virtual disk
+    following conversion of the physical disks from JBOD to RAID
+    mode will fail since there is not enough space due to the
+    space used by RAID mode housekeeping.
+    This patch works around this issue by recalculating the RAID
+    volume size after physical disk conversion has completed and
+    the free space on the converted drives is known.  Note that
+    this may result in a virtual disk that is slightly smaller
+    than the requested size, but still the max size that the
+    drives can support.
+    See bug
+    `bug 2007359 <https://storyboard.openstack.org/#!/story/2007359>`_
+    for more details