summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--ironic_python_agent/extensions/image.py51
-rw-r--r--ironic_python_agent/hardware.py4
-rw-r--r--ironic_python_agent/tests/unit/extensions/test_image.py47
-rw-r--r--ironic_python_agent/tests/unit/samples/hardware_samples.py7
-rw-r--r--ironic_python_agent/tests/unit/test_hardware.py26
-rw-r--r--releasenotes/notes/handle-partuuid-for-fstab-e0aadea20a056982.yaml10
6 files changed, 135 insertions, 10 deletions
diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py
index 892aa288..de880f84 100644
--- a/ironic_python_agent/extensions/image.py
+++ b/ironic_python_agent/extensions/image.py
@@ -485,17 +485,58 @@ def _try_preserve_efi_assets(device, path,
def _append_uefi_to_fstab(fs_path, efi_system_part_uuid):
"""Append the efi partition id to the filesystem table.
- :param fs_path:
- :param efi_system_part_uuid:
+ :param fs_path: The path to the filesystem.
+ :param efi_system_part_uuid: uuid to use to try and find the
+ partition. Warning: this may be
+ a partition uuid or a actual uuid.
"""
fstab_file = os.path.join(fs_path, 'etc/fstab')
if not os.path.exists(fstab_file):
return
try:
- fstab_string = ("UUID=%s\t/boot/efi\tvfat\tumask=0077\t"
- "0\t1\n") % efi_system_part_uuid
+ # Collect all of the block devices so we appropriately match UUID
+ # or PARTUUID into an fstab entry.
+ block_devs = hardware.list_all_block_devices(block_type='part')
+
+ # Default to uuid, but if we find a partuuid instead, that is okay,
+ # we just need to know later on.
+ fstab_label = None
+ for bdev in block_devs:
+ # Check UUID first
+ if bdev.uuid and efi_system_part_uuid in bdev.uuid:
+ LOG.debug('Identified block device %(dev)s UUID %(uuid)s '
+ 'for UEFI boot. Proceeding with fstab update using '
+ 'a UUID.',
+ {'dev': bdev.name,
+ 'uuid': efi_system_part_uuid})
+ # What we have works, and is correct, we can break the loop
+ fstab_label = 'UUID'
+ break
+ # Fallback to PARTUUID, since we don't know if the provided
+ # UUID matches a PARTUUID, or UUID field, and the fstab entry
+ # needs to match it.
+ if bdev.partuuid and efi_system_part_uuid in bdev.partuuid:
+ LOG.debug('Identified block device %(dev)s partition UUID '
+ '%(uuid)s for UEFI boot. Proceeding with fstab '
+ 'update using a PARTUUID.',
+ {'dev': bdev.name,
+ 'uuid': efi_system_part_uuid})
+ fstab_label = 'PARTUUID'
+ break
+
+ if not fstab_label:
+ # Fallback to prior behavior, which should generally be correct.
+ LOG.warning('Falling back to fstab entry addition label of UUID. '
+ 'We could not identify which UUID or PARTUUID '
+ 'identifier label should be used, thus UUID will be '
+ 'used.')
+ fstab_label = 'UUID'
+
+ fstab_string = ("%s=%s\t/boot/efi\tvfat\tumask=0077\t"
+ "0\t1\n") % (fstab_label, efi_system_part_uuid)
with open(fstab_file, "r+") as fstab:
- if efi_system_part_uuid not in fstab.read():
+ already_present_string = fstab_label + '=' + efi_system_part_uuid
+ if already_present_string not in fstab.read():
fstab.writelines(fstab_string)
except (OSError, EnvironmentError, IOError) as exc:
LOG.debug('Failed to add entry to /etc/fstab. Error %s', exc)
diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index ceafe31e..62e39f1d 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -545,6 +545,7 @@ def list_all_block_devices(block_type='disk',
'block', 'vendor'),
by_path=by_path_name,
uuid=device['UUID'],
+ partuuid=device['PARTUUID'],
**extra))
return devices
@@ -612,7 +613,7 @@ class BlockDevice(encoding.SerializableComparable):
def __init__(self, name, model, size, rotational, wwn=None, serial=None,
vendor=None, wwn_with_extension=None,
wwn_vendor_extension=None, hctl=None, by_path=None,
- uuid=None):
+ uuid=None, partuuid=None):
self.name = name
self.model = model
self.size = size
@@ -625,6 +626,7 @@ class BlockDevice(encoding.SerializableComparable):
self.wwn_vendor_extension = wwn_vendor_extension
self.hctl = hctl
self.by_path = by_path
+ self.partuuid = partuuid
class NetworkInterface(encoding.SerializableComparable):
diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py
index 1834559b..00aaf706 100644
--- a/ironic_python_agent/tests/unit/extensions/test_image.py
+++ b/ironic_python_agent/tests/unit/extensions/test_image.py
@@ -751,6 +751,18 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
mock_is_md_device.return_value = False
mock_md_get_raid_devices.return_value = {}
mock_exists.side_effect = iter([False, True, False, True, True])
+ partuuid_device = ('KNAME="sda" MODEL="DRIVE 0" SIZE="10240000" '
+ 'ROTA="1" TYPE="disk" UUID="987654-3210" '
+ 'PARTUUID=""\n'
+ 'KNAME="sda0" MODEL="DRIVE 0" SIZE="102400" '
+ 'ROTA="1" TYPE="part" '
+ 'UUID="' + self.fake_efi_system_part_uuid + '" '
+ 'PARTUUID="1234-2918"\n')
+ exec_side_effect = [('', '')] * 16
+ exec_side_effect.append((partuuid_device, ''))
+ exec_side_effect.extend([('', '')] * 8)
+ mock_execute.side_effect = exec_side_effect
+
with mock.patch('builtins.open', mock.mock_open()) as mock_open:
image._install_grub2(
self.fake_dev, root_uuid=self.fake_root_uuid,
@@ -805,6 +817,9 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
'GRUB_DISABLE_OS_PROBER': 'true',
'GRUB_SAVEDEFAULT': 'true'},
use_standard_locale=True),
+ mock.call('udevadm', 'settle'),
+ mock.call('lsblk', '-Pbia',
+ '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'),
mock.call('umount', self.fake_dir + '/boot/efi',
attempts=3, delay_on_retry=True),
mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' %
@@ -849,8 +864,21 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
environ_mock.get.return_value = '/sbin'
mock_is_md_device.return_value = False
mock_md_get_raid_devices.return_value = {}
+ partuuid_device = ('KNAME="sda" MODEL="DRIVE 0" SIZE="10240000" '
+ 'ROTA="1" TYPE="disk" UUID="987654-3210" '
+ 'PARTUUID=""\n'
+ 'KNAME="sda0" MODEL="DRIVE 0" SIZE="102400" '
+ 'ROTA="1" TYPE="part" UUID="987654-3210" '
+ 'PARTUUID="' + self.fake_efi_system_part_uuid
+ + '"\n')
+ exec_side_effect = [('', '')] * 16
+ exec_side_effect.append((partuuid_device, ''))
+ exec_side_effect.extend([('', '')] * 8)
+ mock_execute.side_effect = exec_side_effect
+ # Validates the complete opposite path *and* no-write behavior
+ # occurs if the entry already exists.
fstab_data = (
- 'UUID=%s\tpath vfat option' % self.fake_efi_system_part_uuid)
+ 'PARTUUID=%s\tpath vfat option' % self.fake_efi_system_part_uuid)
mock_exists.side_effect = [True, False, True, True, True, False,
True, True]
with mock.patch('builtins.open',
@@ -916,6 +944,9 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
'GRUB_DISABLE_OS_PROBER': 'true',
'GRUB_SAVEDEFAULT': 'true'},
use_standard_locale=True),
+ mock.call('udevadm', 'settle'),
+ mock.call('lsblk', '-Pbia',
+ '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'),
mock.call('umount', self.fake_dir + '/boot/efi',
attempts=3, delay_on_retry=True),
mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' %
@@ -1026,6 +1057,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
@mock.patch.object(os, 'listdir', lambda *_: ['file1', 'file2'])
@mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False)
+ @mock.patch.object(image, '_append_uefi_to_fstab', autospec=True)
@mock.patch.object(shutil, 'copy2', autospec=True)
@mock.patch.object(os.path, 'isfile', autospec=True)
@mock.patch.object(image, '_efi_boot_setup', autospec=True)
@@ -1042,7 +1074,8 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
mock_is_md_device, mock_exists,
mock_copytree, mock_efi_setup,
mock_isfile, mock_copy2,
- mock_execute, mock_dispatch):
+ mock_fstab_append, mock_execute,
+ mock_dispatch):
mock_exists.return_value = True
mock_efi_setup.return_value = True
mock_get_part_uuid.side_effect = [self.fake_root_part,
@@ -1106,6 +1139,9 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
uuid=self.fake_root_uuid)
mock_get_part_uuid.assert_any_call(self.fake_dev,
uuid=self.fake_efi_system_part_uuid)
+ mock_fstab_append.assert_called_once_with(
+ self.fake_dir,
+ self.fake_efi_system_part_uuid)
self.assertFalse(mock_dispatch.called)
@mock.patch.object(os.path, 'ismount', lambda *_: False)
@@ -1344,6 +1380,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
self.fake_efi_system_part_uuid)
@mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False)
+ @mock.patch.object(image, '_append_uefi_to_fstab', autospec=True)
@mock.patch.object(os, 'listdir', autospec=True)
@mock.patch.object(shutil, 'copy2', autospec=True)
@mock.patch.object(os.path, 'isfile', autospec=True)
@@ -1361,8 +1398,8 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
mock_is_md_device, mock_exists,
mock_copytree, mock_efi_setup,
mock_isfile, mock_copy2,
- mock_oslistdir, mock_execute,
- mock_dispatch):
+ mock_oslistdir, mock_append_to_fstab,
+ mock_execute, mock_dispatch):
mock_exists.return_value = True
mock_efi_setup.return_value = True
mock_get_part_uuid.side_effect = [self.fake_root_part,
@@ -1431,6 +1468,8 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
uuid=self.fake_efi_system_part_uuid)
self.assertFalse(mock_dispatch.called)
self.assertEqual(2, mock_oslistdir.call_count)
+ mock_append_to_fstab.assert_called_with(self.fake_dir,
+ self.fake_efi_system_part_uuid)
@mock.patch.object(os.path, 'ismount', lambda *_: False)
@mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False)
diff --git a/ironic_python_agent/tests/unit/samples/hardware_samples.py b/ironic_python_agent/tests/unit/samples/hardware_samples.py
index 3ab54153..8cc9b1ea 100644
--- a/ironic_python_agent/tests/unit/samples/hardware_samples.py
+++ b/ironic_python_agent/tests/unit/samples/hardware_samples.py
@@ -160,6 +160,13 @@ RAID_BLK_DEVICE_TEMPLATE = (
'PARTUUID=""'
)
+PARTUUID_DEVICE_TEMPLATE = (
+ 'KNAME="sda" MODEL="DRIVE 0" SIZE="1765517033472" '
+ 'ROTA="1" TYPE="disk" UUID="" PARTUUID=""\n'
+ 'KNAME="sda1" MODEL="DRIVE 0" SIZE="107373133824" '
+ 'ROTA="1" TYPE="part" UUID="987654-3210" PARTUUID="1234-5678"\n'
+)
+
SHRED_OUTPUT_0_ITERATIONS_ZERO_FALSE = ()
SHRED_OUTPUT_1_ITERATION_ZERO_TRUE = (
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index a9deb79b..60820727 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -66,6 +66,13 @@ RAID_BLK_DEVICE_TEMPLATE_DEVICES = [
vendor="FooTastic", uuid=""),
]
+BLK_DEVICE_TEMPLATE_PARTUUID_DEVICE = [
+ hardware.BlockDevice(name='/dev/sda1', model='DRIVE 0',
+ size=107373133824, rotational=True,
+ vendor="FooTastic", uuid="987654-3210",
+ partuuid="1234-5678"),
+]
+
class FakeHardwareManager(hardware.GenericHardwareManager):
def __init__(self, hardware_support):
@@ -4291,6 +4298,25 @@ class TestModuleFunctions(base.IronicAgentTest):
self.assertEqual(RAID_BLK_DEVICE_TEMPLATE_DEVICES, result)
mocked_udev.assert_called_once_with()
+ @mock.patch.object(os, 'readlink', autospec=True)
+ @mock.patch.object(hardware, '_get_device_info',
+ lambda x, y, z: 'FooTastic')
+ @mock.patch.object(hardware, '_udev_settle', autospec=True)
+ @mock.patch.object(hardware.pyudev.Devices, "from_device_file",
+ autospec=False)
+ def test_list_all_block_devices_partuuid_success(
+ self, mocked_fromdevfile,
+ mocked_udev, mocked_readlink,
+ mocked_execute):
+ mocked_readlink.return_value = '../../sda'
+ mocked_fromdevfile.return_value = {}
+ mocked_execute.return_value = (hws.PARTUUID_DEVICE_TEMPLATE, '')
+ result = hardware.list_all_block_devices(block_type='part')
+ mocked_execute.assert_called_once_with(
+ 'lsblk', '-Pbia', '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID')
+ self.assertEqual(BLK_DEVICE_TEMPLATE_PARTUUID_DEVICE, result)
+ mocked_udev.assert_called_once_with()
+
@mock.patch.object(hardware, '_get_device_info',
lambda x, y: "FooTastic")
@mock.patch.object(hardware, '_udev_settle', autospec=True)
diff --git a/releasenotes/notes/handle-partuuid-for-fstab-e0aadea20a056982.yaml b/releasenotes/notes/handle-partuuid-for-fstab-e0aadea20a056982.yaml
new file mode 100644
index 00000000..f9b6bbc5
--- /dev/null
+++ b/releasenotes/notes/handle-partuuid-for-fstab-e0aadea20a056982.yaml
@@ -0,0 +1,10 @@
+---
+fixes:
+ - |
+ Fixes handling of a Partition UUID being returned instead of a
+ Partition's UUID when the OS may not return the Partition's UUID in time.
+ These two fields are typically referred to as PARTUUID and UUID,
+ respectively. Often these sorts of issues arise under heavy IO load.
+ We now scan, and identify which "UUID" we identified, and update
+ a Linux fstab entry appropriately. For more information, please see
+ `story #2009881 <https://storyboard.openstack.org/#!/story/2009881>`_.