diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index e5b1b1aaa..ee63285a9 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -48,6 +48,15 @@ def _get_partition(device, uuid): LOG.warning("Couldn't re-read the partition table " "on device %s", device) + # If the deploy device is an md device, we want to install on + # the first partition. We clearly take a shortcut here for now. + # TODO(arne_wiebalck): Would it possible to use the partition + # UUID and use the "normal" discovery instead? + if hardware.is_md_device(device): + md_partition = device + 'p1' + LOG.debug("Found md device with partition %s", md_partition) + return md_partition + lsblk = utils.execute('lsblk', '-PbioKNAME,UUID,PARTUUID,TYPE', device) report = lsblk[0] for line in report.split('\n'): @@ -102,6 +111,16 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, if prep_boot_part_uuid: device = _get_partition(device, uuid=prep_boot_part_uuid) + # If the root device is an md device (or partition), restart the device + # (to help grub finding it) and identify the underlying holder disks + # to install grub. + disks = [] + if hardware.is_md_device(device): + hardware.md_restart(device) + disks = hardware.get_holder_disks(device) + else: + disks.append(device) + utils.execute('mount', root_partition, path) for fs in BIND_MOUNTS: utils.execute('mount', '-o', 'bind', fs, path + fs) @@ -125,11 +144,18 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, path_variable = os.environ.get('PATH', '') path_variable = '%s:/bin:/usr/sbin' % path_variable - # Install grub - utils.execute('chroot %(path)s /bin/sh -c ' - '"%(bin)s-install %(dev)s"' % - {'path': path, 'bin': binary_name, 'dev': device}, - shell=True, env_variables={'PATH': path_variable}) + # Install grub. Normally, grub goes to one disk only. In case of + # md devices, grub goes to all underlying holder (RAID-1) disks. + LOG.info("GRUB2 will be installed on disks %s", disks) + for grub_disk in disks: + LOG.debug("Installing GRUB2 on disk %s", grub_disk) + utils.execute('chroot %(path)s /bin/sh -c ' + '"%(bin)s-install %(dev)s"' % + {'path': path, 'bin': binary_name, + 'dev': grub_disk}, + shell=True, env_variables={'PATH': path_variable}) + LOG.debug("GRUB2 successfully installed on device %s", grub_disk) + # Also run grub-install with --removable, this installs grub to the # EFI fallback path. Useful if the NVRAM wasn't written correctly, # was reset or if testing with virt as libvirt resets the NVRAM diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index ed4d57baa..38b9d4063 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -143,7 +143,7 @@ def _get_component_devices(raid_device): return component_devices -def _get_holder_disks(raid_device): +def get_holder_disks(raid_device): """Get the holder disks of a Software RAID device. Examine an md device and return its underlying disks. @@ -155,6 +155,7 @@ def _get_holder_disks(raid_device): return [] holder_disks = [] + try: out, _ = utils.execute('mdadm', '--detail', raid_device, use_standard_locale=True) @@ -173,7 +174,7 @@ def _get_holder_disks(raid_device): return holder_disks -def _is_md_device(raid_device): +def is_md_device(raid_device): """Check if a device is an md device Check if a device is a Software RAID (md) device. @@ -190,7 +191,7 @@ def _is_md_device(raid_device): return False -def _md_restart(raid_device): +def md_restart(raid_device): """Restart an md device Stop and re-assemble a Software RAID (md) device. @@ -1282,8 +1283,8 @@ class GenericHardwareManager(HardwareManager): valid or if there was an error when creating the RAID devices. """ - LOG.info("Creating Software RAID") + # No RAID config: do nothing raid_config = node.get('target_raid_config', {}) # No 'software' controller: do nothing. If 'controller' is @@ -1332,7 +1333,6 @@ class GenericHardwareManager(HardwareManager): raise errors.SoftwareRAIDError(msg) # Create the partitions which will become the component devices. - logical_disks = raid_config.get('logical_disks') sector = '2048s' for logical_disk in logical_disks: psize = logical_disk['size_gb'] @@ -1404,7 +1404,7 @@ class GenericHardwareManager(HardwareManager): component_devices = _get_component_devices(raid_device.name) LOG.debug("Found component devices {}".format( component_devices)) - holder_disks = _get_holder_disks(raid_device.name) + holder_disks = get_holder_disks(raid_device.name) LOG.debug("Found holder disks {}".format( holder_disks)) diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 74942fa4c..6e6fef25b 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -91,12 +91,15 @@ class TestImageExtension(base.IronicAgentTest): prep_boot_part_uuid=self.fake_prep_boot_part_uuid) mock_iscsi_clean.assert_called_once_with(self.fake_dev) + @mock.patch.object(hardware, 'is_md_device', autospec=True) @mock.patch.object(os, 'environ', autospec=True) @mock.patch.object(image, '_get_partition', autospec=True) def test__install_grub2(self, mock_get_part_uuid, environ_mock, - mock_execute, mock_dispatch): + mock_is_md_device, mock_execute, + mock_dispatch): mock_get_part_uuid.return_value = self.fake_root_part environ_mock.get.return_value = '/sbin' + mock_is_md_device.side_effect = [False] image._install_grub2(self.fake_dev, self.fake_root_uuid) expected = [mock.call('mount', '/dev/fake2', self.fake_dir), @@ -128,13 +131,16 @@ class TestImageExtension(base.IronicAgentTest): uuid=self.fake_root_uuid) self.assertFalse(mock_dispatch.called) + @mock.patch.object(hardware, 'is_md_device', autospec=True) @mock.patch.object(os, 'environ', autospec=True) @mock.patch.object(image, '_get_partition', autospec=True) def test__install_grub2_prep(self, mock_get_part_uuid, environ_mock, - mock_execute, mock_dispatch): + mock_is_md_device, mock_execute, + mock_dispatch): mock_get_part_uuid.side_effect = [self.fake_root_part, self.fake_prep_boot_part] environ_mock.get.return_value = '/sbin' + mock_is_md_device.side_effect = [False] image._install_grub2(self.fake_dev, self.fake_root_uuid, prep_boot_part_uuid=self.fake_prep_boot_part_uuid) @@ -170,15 +176,17 @@ class TestImageExtension(base.IronicAgentTest): uuid=self.fake_prep_boot_part_uuid) self.assertFalse(mock_dispatch.called) + @mock.patch.object(hardware, 'is_md_device', autospec=True) @mock.patch.object(os, 'environ', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) @mock.patch.object(image, '_get_partition', autospec=True) def test__install_grub2_uefi(self, mock_get_part_uuid, mkdir_mock, - environ_mock, mock_execute, - mock_dispatch): + environ_mock, mock_is_md_device, + mock_execute, mock_dispatch): mock_get_part_uuid.side_effect = [self.fake_root_part, self.fake_efi_system_part] environ_mock.get.return_value = '/sbin' + mock_is_md_device.return_value = False image._install_grub2( self.fake_dev, root_uuid=self.fake_root_uuid, @@ -224,14 +232,16 @@ class TestImageExtension(base.IronicAgentTest): uuid=self.fake_efi_system_part_uuid) self.assertFalse(mock_dispatch.called) + @mock.patch.object(hardware, 'is_md_device', autospec=True) @mock.patch.object(os, 'environ', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) @mock.patch.object(image, '_get_partition', autospec=True) def test__install_grub2_uefi_umount_fails( - self, mock_get_part_uuid, mkdir_mock, environ_mock, mock_execute, - mock_dispatch): + self, mock_get_part_uuid, mkdir_mock, environ_mock, + mock_is_md_device, mock_execute, mock_dispatch): mock_get_part_uuid.side_effect = [self.fake_root_part, self.fake_efi_system_part] + mock_is_md_device.return_value = False def umount_raise_func(*args, **kwargs): if args[0] == 'umount': @@ -270,14 +280,16 @@ class TestImageExtension(base.IronicAgentTest): attempts=3, delay_on_retry=True)] mock_execute.assert_has_calls(expected) + @mock.patch.object(hardware, 'is_md_device', autospec=True) @mock.patch.object(os, 'environ', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) @mock.patch.object(image, '_get_partition', autospec=True) def test__install_grub2_uefi_mount_fails( - self, mock_get_part_uuid, mkdir_mock, environ_mock, mock_execute, - mock_dispatch): + self, mock_get_part_uuid, mkdir_mock, environ_mock, + mock_is_md_device, mock_execute, mock_dispatch): mock_get_part_uuid.side_effect = [self.fake_root_part, self.fake_efi_system_part] + mock_is_md_device.side_effect = [False] def mount_raise_func(*args, **kwargs): if args[0] == 'mount': @@ -314,7 +326,10 @@ class TestImageExtension(base.IronicAgentTest): uuid=self.fake_root_uuid) self.assertFalse(mock_dispatch.called) - def test__get_partition(self, mock_execute, mock_dispatch): + @mock.patch.object(hardware, 'is_md_device', autospec=True) + def test__get_partition(self, mock_is_md_device, mock_execute, + mock_dispatch): + mock_is_md_device.side_effect = [False] lsblk_output = ('''KNAME="test" UUID="" TYPE="disk" KNAME="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part" KNAME="test2" UUID="%s" TYPE="part"''' % self.fake_root_uuid) @@ -330,8 +345,10 @@ class TestImageExtension(base.IronicAgentTest): mock_execute.assert_has_calls(expected) self.assertFalse(mock_dispatch.called) - def test__get_partition_no_device_found(self, mock_execute, - mock_dispatch): + @mock.patch.object(hardware, 'is_md_device', autospec=True) + def test__get_partition_no_device_found(self, mock_is_md_device, + mock_execute, mock_dispatch): + mock_is_md_device.side_effect = [False] lsblk_output = ('''KNAME="test" UUID="" TYPE="disk" KNAME="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part" KNAME="test2" UUID="" TYPE="part"''') @@ -348,8 +365,10 @@ class TestImageExtension(base.IronicAgentTest): mock_execute.assert_has_calls(expected) self.assertFalse(mock_dispatch.called) - def test__get_partition_command_fail(self, mock_execute, - mock_dispatch): + @mock.patch.object(hardware, 'is_md_device', autospec=True) + def test__get_partition_command_fail(self, mock_is_md_device, + mock_execute, mock_dispatch): + mock_is_md_device.side_effect = [False] mock_execute.side_effect = (None, None, processutils.ProcessExecutionError('boom')) self.assertRaises(errors.CommandExecutionError, @@ -364,7 +383,10 @@ class TestImageExtension(base.IronicAgentTest): mock_execute.assert_has_calls(expected) self.assertFalse(mock_dispatch.called) - def test__get_partition_partuuid(self, mock_execute, mock_dispatch): + @mock.patch.object(hardware, 'is_md_device', autospec=True) + def test__get_partition_partuuid(self, mock_is_md_device, mock_execute, + mock_dispatch): + mock_is_md_device.side_effect = [False] lsblk_output = ('''KNAME="test" UUID="" TYPE="disk" KNAME="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part" KNAME="test2" PARTUUID="%s" TYPE="part"''' % self.fake_root_uuid) diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index cd2df1b57..c68c0e37a 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -2291,11 +2291,11 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertEqual(['/dev/vde1', '/dev/vdf1'], component_devices) @mock.patch.object(utils, 'execute', autospec=True) - def test__get_holder_disks(self, mocked_execute): + def test_get_holder_disks(self, mocked_execute): mocked_execute.side_effect = [(MDADM_DETAIL_OUTPUT, '')] raid_device = hardware.BlockDevice('/dev/md0', 'RAID-1', 1073741824, True) - holder_disks = hardware._get_holder_disks(raid_device.name) + holder_disks = hardware.get_holder_disks(raid_device.name) self.assertEqual(['/dev/vde', '/dev/vdf'], holder_disks) @mock.patch.object(hardware, 'list_all_block_devices', autospec=True) @@ -2311,8 +2311,8 @@ class TestGenericHardwareManager(base.IronicAgentTest): hardware._get_component_devices.side_effect = [ ["/dev/sda1", "/dev/sda2"], ["/dev/sdb1", "/dev/sdb2"]] - hardware._get_holder_disks = mock.Mock() - hardware._get_holder_disks.side_effect = [ + hardware.get_holder_disks = mock.Mock() + hardware.get_holder_disks.side_effect = [ ["/dev/sda", "/dev/sdb"], ["/dev/sda", "/dev/sdb"]] mocked_execute.side_effect = [