From defc4a8761e116bdc61af528a79b11d47b4e6768 Mon Sep 17 00:00:00 2001 From: Arne Wiebalck Date: Fri, 4 Oct 2019 11:29:04 +0200 Subject: [PATCH] Software RAID: Identify the root fs via its UUID from image metadata In order to install the bootloader on instances with software RAID, the IPA needs to chroot into the deployed image's root fs. Currently, the IPA assumes the first partition to contain the root fs, so this breaks with images where this is not the case. The proposed change extracts the root fs UUID from the image's metadata instead and passes it to the IPA (in the same way this is done for partition images). It keeps the current behaviour to send the UUID extracted from the driver internal info in case the metadata is not set on the image. For the time being, the IPA completely ignores the UUID passed anyway, so to make this fully work the IPA will also need to be patched. Change-Id: I69bd53aa24b7b10384dc1399a0acaa21d38d6bd9 Story: #2006649 Task: #37081 --- ironic/drivers/modules/agent_base_vendor.py | 24 +++++++++- .../drivers/modules/test_agent_base_vendor.py | 44 +++++++++++++++++-- ...raid_use_rootfs_uuid-f61eb671d696d251.yaml | 13 ++++++ 3 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/sofware_raid_use_rootfs_uuid-f61eb671d696d251.yaml diff --git a/ironic/drivers/modules/agent_base_vendor.py b/ironic/drivers/modules/agent_base_vendor.py index 1f0710023d..4d055abaa3 100644 --- a/ironic/drivers/modules/agent_base_vendor.py +++ b/ironic/drivers/modules/agent_base_vendor.py @@ -27,6 +27,7 @@ import retrying from ironic.common import boot_devices from ironic.common import exception from ironic.common.i18n import _ +from ironic.common import image_service from ironic.common import states from ironic.conductor import steps as conductor_steps from ironic.conductor import utils as manager_utils @@ -827,9 +828,30 @@ class AgentDeployMixin(HeartbeatMixin): LOG.debug('Node %s has a Software RAID configuration', node.uuid) software_raid = True - root_uuid = internal_info.get('root_uuid_or_disk_id') break + # For software RAID try to get the UUID of the root fs from the + # image's metadata (via Glance). Fall back to the driver internal + # info in case it is not available (e.g. not set or there's no Glance). + if software_raid: + image_source = node.instance_info.get('image_source') + try: + context = task.context + context.is_admin = True + glance = image_service.GlanceImageService( + context=context) + image_info = glance.show(image_source) + image_properties = image_info.get('properties') + root_uuid = image_properties['rootfs_uuid'] + LOG.debug('Got rootfs_uuid from Glance: %s', root_uuid) + except Exception as e: + LOG.warning('Could not get \'rootfs_uuid\' property for ' + 'image %(image)s from Glance: %(error)s.', + {'image': image_source, 'error': e}) + root_uuid = internal_info.get('root_uuid_or_disk_id') + LOG.debug('Got rootfs_uuid from driver internal info: ' + ' %s', root_uuid) + whole_disk_image = internal_info.get('is_whole_disk_image') if software_raid or (root_uuid and not whole_disk_image): LOG.debug('Installing the bootloader for node %(node)s on ' diff --git a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py index afb14c02f6..96e02cae34 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py @@ -21,6 +21,7 @@ from oslo_config import cfg from ironic.common import boot_devices from ironic.common import exception +from ironic.common import image_service from ironic.common import states from ironic.conductor import steps as conductor_steps from ironic.conductor import task_manager @@ -1189,11 +1190,13 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) + @mock.patch.object(image_service, 'GlanceImageService', autospec=True) @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) def test_configure_local_boot_on_software_raid( - self, install_bootloader_mock, try_set_boot_device_mock): + self, install_bootloader_mock, try_set_boot_device_mock, + GlanceImageService_mock): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = True @@ -1211,12 +1214,48 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): } ] } - self.deploy.configure_local_boot(task) + self.assertTrue(GlanceImageService_mock.called) self.assertTrue(install_bootloader_mock.called) try_set_boot_device_mock.assert_called_once_with( task, boot_devices.DISK, persistent=True) + @mock.patch.object(image_service, 'GlanceImageService', autospec=True) + @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) + @mock.patch.object(agent_client.AgentClient, 'install_bootloader', + autospec=True) + def test_configure_local_boot_on_software_raid_exception( + self, install_bootloader_mock, try_set_boot_device_mock, + GlanceImageService_mock): + GlanceImageService_mock.side_effect = Exception('Glance not found') + with task_manager.acquire(self.context, self.node['uuid'], + shared=False) as task: + task.node.driver_internal_info['is_whole_disk_image'] = True + root_uuid = "1efecf88-2b58-4d4e-8fbd-7bef1a40a1b0" + task.node.driver_internal_info['root_uuid_or_disk_id'] = root_uuid + task.node.target_raid_config = { + "logical_disks": [ + { + "size_gb": 100, + "raid_level": "1", + "controller": "software", + }, + { + "size_gb": 'MAX', + "raid_level": "0", + "controller": "software", + } + ] + } + self.deploy.configure_local_boot(task) + self.assertTrue(GlanceImageService_mock.called) + # check if the root_uuid comes from the driver_internal_info + install_bootloader_mock.assert_called_once_with( + mock.ANY, task.node, root_uuid=root_uuid, + efi_system_part_uuid=None, prep_boot_part_uuid=None) + try_set_boot_device_mock.assert_called_once_with( + task, boot_devices.DISK, persistent=True) + @mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True) @mock.patch.object(agent_client.AgentClient, 'install_bootloader', autospec=True) @@ -1237,7 +1276,6 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): } ] } - self.deploy.configure_local_boot(task) self.assertFalse(install_bootloader_mock.called) try_set_boot_device_mock.assert_called_once_with( diff --git a/releasenotes/notes/sofware_raid_use_rootfs_uuid-f61eb671d696d251.yaml b/releasenotes/notes/sofware_raid_use_rootfs_uuid-f61eb671d696d251.yaml new file mode 100644 index 0000000000..25495dee80 --- /dev/null +++ b/releasenotes/notes/sofware_raid_use_rootfs_uuid-f61eb671d696d251.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + Software RAID is no longer limited to images which have the root file + system in the first partition. +upgrade: + - | + For Software RAID, the IPA no longer assumes that the root file system + of the deployed image is in the first partition. Instead, it will use + the UUID passed from the conductor. Operators need hence to make sure + that the conductor has the correct UUID (which either comes from the + 'rootfs_uuid' field in the image metadata or from the + 'root_uuid_or_disk_id' in the nodes 'internal_driver_info'.)