From 3ee17e86249b8ed6a6060ae4d6f225e85b456a13 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Mon, 14 Oct 2019 14:30:11 -0700 Subject: [PATCH] Last resort fallback to find a partition Falls back to attempt to use findfs to locate a UUID or PARTUUID match as opposed to trying to list and enumerate through lsblk output. Can confirm that tinycore 8.x's findfs binary works expected. Story: 2006724 Task: 37141 Change-Id: I4d488e4a1ab680eb1353b158c3339cb30b056ada --- ironic_python_agent/extensions/image.py | 25 ++++++++++++++++ .../tests/unit/extensions/test_image.py | 30 ++++++++++++++++++- .../fallback-to-findfs-59abde55221e1e84.yaml | 10 +++++++ 3 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/fallback-to-findfs-59abde55221e1e84.yaml diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index d9fced263..ce5d59cd0 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -75,6 +75,9 @@ def _get_partition(device, uuid): part[key] = val.strip() # Ignore non partition if part.get('TYPE') != 'part': + # NOTE(TheJulia): This techincally creates an edge failure + # case where a filesystem on a whole block device sans + # partitioning would behave differently. continue if part.get('UUID') == uuid: @@ -86,6 +89,28 @@ def _get_partition(device, uuid): "%(dev)s", {'uuid': uuid, 'dev': device}) return '/dev/' + part.get('KNAME') else: + # NOTE(TheJulia): We may want to consider moving towards using + # findfs in the future, if we're comfortable with the execution + # and interaction. There is value in either way though. + try: + findfs, stderr = utils.execute('findfs', 'UUID=%s' % uuid) + return findfs.strip() + except processutils.ProcessExecutionError as e: + LOG.debug('First fallback detection attempt for locating ' + 'partition via UUID %(uuid)s failed. ' + 'Error: %(err)s', + {'uuid': uuid, + 'err': e}) + try: + findfs, stderr = utils.execute( + 'findfs', 'PARTUUID=%s' % uuid) + return findfs.strip() + except processutils.ProcessExecutionError as e: + LOG.debug('Secondary fallback detection attempt for ' + 'locating partition via UUID %(uuid)s failed. ' + 'Error: %(err)s', + {'uuid': uuid, + 'err': e}) error_msg = ("No partition with UUID %(uuid)s found on " "device %(dev)s" % {'uuid': uuid, 'dev': device}) LOG.error(error_msg) diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index ce66b3fe7..9cf764923 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -368,7 +368,10 @@ class TestImageExtension(base.IronicAgentTest): lsblk_output = ('''KNAME="test" UUID="" TYPE="disk" KNAME="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part" KNAME="test2" UUID="" TYPE="part"''') - mock_execute.side_effect = (None, None, [lsblk_output]) + mock_execute.side_effect = ( + None, None, [lsblk_output], + processutils.ProcessExecutionError('boom'), + processutils.ProcessExecutionError('kaboom')) self.assertRaises(errors.DeviceNotFound, image._get_partition, self.fake_dev, @@ -381,6 +384,31 @@ class TestImageExtension(base.IronicAgentTest): mock_execute.assert_has_calls(expected) self.assertFalse(mock_dispatch.called) + @mock.patch.object(hardware, 'is_md_device', autospec=True) + def test__get_partition_fallback_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" UUID="" TYPE="part"''') + findfs_output = ('/dev/loop0\n', None) + mock_execute.side_effect = ( + None, None, [lsblk_output], + processutils.ProcessExecutionError('boom'), + findfs_output) + + result = image._get_partition(self.fake_dev, self.fake_root_uuid) + self.assertEqual('/dev/loop0', result) + expected = [mock.call('partx', '-u', self.fake_dev, attempts=3, + delay_on_retry=True), + mock.call('udevadm', 'settle'), + mock.call('lsblk', '-PbioKNAME,UUID,PARTUUID,TYPE', + self.fake_dev), + mock.call('findfs', 'UUID=%s' % self.fake_root_uuid), + mock.call('findfs', 'PARTUUID=%s' % self.fake_root_uuid)] + mock_execute.assert_has_calls(expected) + self.assertFalse(mock_dispatch.called) + @mock.patch.object(hardware, 'is_md_device', autospec=True) def test__get_partition_command_fail(self, mock_is_md_device, mock_execute, mock_dispatch): diff --git a/releasenotes/notes/fallback-to-findfs-59abde55221e1e84.yaml b/releasenotes/notes/fallback-to-findfs-59abde55221e1e84.yaml new file mode 100644 index 000000000..bc70a6af5 --- /dev/null +++ b/releasenotes/notes/fallback-to-findfs-59abde55221e1e84.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixes an issue with the tinyIPA CI testing image by providing a fallback + root volume uuid detection method via the ``findfs`` utility, which is + also already packaged in most distributions with ``lsblk``. + + This fallback was necesary as the ``lsblk`` command in ``TinyCore`` Linux, + upon which TinyIPA is built, does not return data as expected for + volume UUID values.