diff options
author | Dan Bungert <[email protected]> | 2024-02-28 00:51:47 +0000 |
---|---|---|
committer | Server Team CI Bot <[email protected]> | 2024-02-28 00:51:47 +0000 |
commit | fc39d7444cc4f3e103b71e02bb558498e5911536 (patch) | |
tree | 462c40a4fefa0146e37a14b5df1e5d4a5e85a3f5 | |
parent | a85f7d6ed856994d8ddbaac2a67e4eb7ff4ec421 (diff) |
block_meta: improve get_volume_uuid
blkid can return 3 different types of UUIDs, and they aren't
interchangable. Clarify which one we're using.
Also, leverage the existing blkid() wrapper to do so.
-rw-r--r-- | curtin/block/__init__.py | 16 | ||||
-rw-r--r-- | curtin/commands/block_meta.py | 9 | ||||
-rw-r--r-- | tests/integration/test_block_meta.py | 1 | ||||
-rw-r--r-- | tests/unittests/test_block.py | 17 | ||||
-rw-r--r-- | tests/unittests/test_commands_block_meta.py | 16 | ||||
-rw-r--r-- | tests/unittests/test_partitioning.py | 14 |
6 files changed, 39 insertions, 34 deletions
diff --git a/curtin/block/__init__.py b/curtin/block/__init__.py index 87990d63..2e19467a 100644 --- a/curtin/block/__init__.py +++ b/curtin/block/__init__.py @@ -811,16 +811,16 @@ def read_sys_block_size_bytes(device): return size -def get_volume_uuid(path): +def get_volume_id(path): """ - Get uuid of disk with given path. This address uniquely identifies - the device and remains consistant across reboots + Get identifier of device with given path. This address uniquely identifies + the device and remains consistant across reboots. """ - (out, _err) = util.subp(["blkid", "-o", "export", path], capture=True) - for line in out.splitlines(): - if "UUID" in line: - return line.split('=')[-1] - return '' + ids = blkid([path])[path] + for key in ("UUID", "PARTUUID", "PTUUID"): + if key in ids: + return (key, ids[key]) + return (None, '') def get_mountpoints(): diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py index 9fde9c65..c7bd0dd9 100644 --- a/curtin/commands/block_meta.py +++ b/curtin/commands/block_meta.py @@ -1733,10 +1733,15 @@ def dm_crypt_handler(info, storage_config, context): if state['fstab']: state_dir = os.path.dirname(state['fstab']) crypt_tab_location = os.path.join(state_dir, "crypttab") - uuid = block.get_volume_uuid(volume_path) + key, value = block.get_volume_id(volume_path) + if key is None: + raise RuntimeError(f"Failed to find suitable ID for {volume_path}") + util.write_file( crypt_tab_location, - f"{dm_name} UUID={uuid} {crypttab_keyfile} {options}\n", omode="a") + f"{dm_name} {key}={value} {crypttab_keyfile} {options}\n", + omode="a", + ) else: LOG.info("fstab configuration is not present in environment, so \ cannot locate an appropriate directory to write crypttab in \ diff --git a/tests/integration/test_block_meta.py b/tests/integration/test_block_meta.py index ec9b33ca..568f466f 100644 --- a/tests/integration/test_block_meta.py +++ b/tests/integration/test_block_meta.py @@ -1324,7 +1324,6 @@ table-length: 256'''.encode() crypttab = fp.read() tokens = re.split(r'\s+', crypttab) self.assertEqual(cryptoswap, tokens[0]) - self.assertTrue(tokens[1].startswith("UUID=")) self.assertEqual("/dev/urandom", tokens[2]) self.assertEqual("swap,initramfs", tokens[3]) diff --git a/tests/unittests/test_block.py b/tests/unittests/test_block.py index 1ef8f80e..2818fe4b 100644 --- a/tests/unittests/test_block.py +++ b/tests/unittests/test_block.py @@ -16,17 +16,18 @@ from curtin import block class TestBlock(CiTestCase): - @mock.patch("curtin.block.util") - def test_get_volume_uuid(self, mock_util): + @mock.patch("curtin.block.util.subp") + def test_get_volume_id(self, mock_subp): path = "/dev/sda1" - expected_call = ["blkid", "-o", "export", path] - mock_util.subp.return_value = (""" - UUID=182e8e23-5322-46c9-a1b8-cf2c6a88f9f7 - """, "") + expected_call = ["blkid", "-o", "full", path] + mock_subp.return_value = ( + "/dev/sda1: UUID=182e8e23-5322-46c9-a1b8-cf2c6a88f9f7", "", + ) - uuid = block.get_volume_uuid(path) + key, uuid = block.get_volume_id(path) - mock_util.subp.assert_called_with(expected_call, capture=True) + mock_subp.assert_called_with(expected_call, capture=True) + self.assertEqual(key, "UUID") self.assertEqual(uuid, "182e8e23-5322-46c9-a1b8-cf2c6a88f9f7") @mock.patch("curtin.block.get_proc_mounts") diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py index 5adb46ca..c2d592dc 100644 --- a/tests/unittests/test_commands_block_meta.py +++ b/tests/unittests/test_commands_block_meta.py @@ -627,8 +627,8 @@ class TestBlockMeta(CiTestCase): 'mock_assert_clear') self.add_patch('curtin.block.iscsi.volpath_is_iscsi', 'mock_volpath_is_iscsi') - self.add_patch('curtin.block.get_volume_uuid', - 'mock_block_get_volume_uuid') + self.add_patch('curtin.block.get_volume_id', + 'mock_block_get_volume_id') self.add_patch('curtin.commands.block_meta._get_volume_type', 'mock_get_volume_type') self.add_patch('curtin.commands.block_meta.udevadm_info', @@ -1313,7 +1313,7 @@ class TestFstabData(CiTestCase): self.assertEqual(1, m_uinfo.call_count) self.assertEqual(1, m_vol_type.call_count) - @patch('curtin.block.get_volume_uuid') + @patch('curtin.block.get_volume_id') def test_fstab_line_for_data__spec_and_dev_prefers_spec(self, m_get_uuid): """fstab_line_for_data should prefer spec over device.""" spec = "/dev/xvda1" @@ -2048,8 +2048,8 @@ class DmCryptCommon(CiTestCase): self.cipher = self.random_string() self.keysize = self.random_string() self.m_block.zkey_supported.return_value = False - self.block_uuids = [random_uuid() for unused in range(2)] - self.m_block.get_volume_uuid.side_effect = self.block_uuids + self.block_uuids = [("UUID", random_uuid()) for unused in range(2)] + self.m_block.get_volume_id.side_effect = self.block_uuids self.m_which.return_value = False self.fstab = self.tmp_path('fstab') @@ -2469,8 +2469,8 @@ class TestCrypttab(DmCryptCommon): data = util.load_file(self.crypttab) self.assertEqual( - f"cryptroot UUID={self.block_uuids[0]} none luks\n" - f"cryptfoo UUID={self.block_uuids[1]} none luks\n", + f"cryptroot UUID={self.block_uuids[0][1]} none luks\n" + f"cryptfoo UUID={self.block_uuids[1][1]} none luks\n", data ) @@ -2509,7 +2509,7 @@ class TestCrypttab(DmCryptCommon): self.storage_config['dmcrypt0'], self.storage_config, empty_context ) - uuid = self.block_uuids[0] + uuid = self.block_uuids[0][1] self.assertEqual( f"cryptswap UUID={uuid} /dev/urandom swap,initramfs\n", util.load_file(self.crypttab), diff --git a/tests/unittests/test_partitioning.py b/tests/unittests/test_partitioning.py index 50ff07cc..ab092601 100644 --- a/tests/unittests/test_partitioning.py +++ b/tests/unittests/test_partitioning.py @@ -195,7 +195,7 @@ class TestBlock(CiTestCase): "target": "/tmp/mntdir"} mock_get_path_to_storage_volume.return_value = "/dev/fake0" - mock_block.get_volume_uuid.return_value = "UUID123" + mock_block.get_volume_id.return_value = ("UUID", "UUID123") curtin.commands.block_meta.mount_handler( self.storage_config.get("sda2_mount"), self.storage_config) @@ -208,7 +208,7 @@ class TestBlock(CiTestCase): args = mock_get_path_to_storage_volume.call_args_list self.assertTrue(len(args) == 1) self.assertTrue(args[0] == mock.call("sda2", self.storage_config)) - mock_block.get_volume_uuid.assert_called_with("/dev/fake0") + mock_block.get_volume_id.assert_called_with("/dev/fake0") curtin.commands.block_meta.mount_handler( self.storage_config.get("raid_mount"), self.storage_config) @@ -260,7 +260,7 @@ class TestBlock(CiTestCase): "/tmp/dir/fstab"} mock_get_path_to_storage_volume.return_value = "/dev/fake0" mock_tempfile.mkstemp.return_value = ["fp", tmp_path] - mock_block.get_volume_uuid.return_value = "UUID123" + mock_block.get_volume_id.return_value = ("UUID", "UUID123") curtin.commands.block_meta.dm_crypt_handler( self.storage_config.get("crypt0_key"), self.storage_config) @@ -280,7 +280,7 @@ class TestBlock(CiTestCase): calls[1]) mock_remove.assert_called_with(tmp_path) mock_open.assert_called_with("/tmp/dir/crypttab", "a") - mock_block.get_volume_uuid.assert_called_with( + mock_block.get_volume_id.assert_called_with( mock_get_path_to_storage_volume.return_value) @mock.patch("curtin.commands.block_meta.block") @@ -296,7 +296,7 @@ class TestBlock(CiTestCase): mock_util.load_command_environment.return_value = {"fstab": "/tmp/dir/fstab"} mock_get_path_to_storage_volume.return_value = "/dev/fake0" - mock_block.get_volume_uuid.return_value = "UUID123" + mock_block.get_volume_id.return_value = ("UUID", "UUID123") config = self.storage_config["crypt0_keyfile"] curtin.commands.block_meta.dm_crypt_handler( @@ -318,7 +318,7 @@ class TestBlock(CiTestCase): calls[1]) self.assertFalse(mock_remove.called) mock_remove.assert_not_called() - mock_block.get_volume_uuid.assert_called_with( + mock_block.get_volume_id.assert_called_with( mock_get_path_to_storage_volume.return_value) @mock.patch("curtin.commands.block_meta.block") @@ -330,7 +330,7 @@ class TestBlock(CiTestCase): mock_util.load_command_environment.return_value = {"fstab": "/tmp/dir/fstab"} mock_get_path_to_storage_volume.return_value = "/dev/fake0" - mock_block.get_volume_uuid.return_value = "UUID123" + mock_block.get_volume_id.return_value = ("UUID", "UUID123") self.assertRaises( ValueError, |