diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 2da80160f..ffcad55cb 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -435,6 +435,8 @@ def md_get_raid_devices(): :return: A python dict containing details about the discovered RAID devices """ + # Note(Boushra): mdadm output is similar to lsblk, but not + # identical; do not use il_utils.parse_device_tags report = il_utils.execute('mdadm', '--examine', '--scan')[0] lines = report.splitlines() result = {} diff --git a/ironic_python_agent/partition_utils.py b/ironic_python_agent/partition_utils.py index ab933e03f..0e1a0c118 100644 --- a/ironic_python_agent/partition_utils.py +++ b/ironic_python_agent/partition_utils.py @@ -22,7 +22,6 @@ import gzip import io import math import os -import shlex import shutil import stat import tempfile @@ -611,80 +610,71 @@ def get_partition(device, uuid): try: ipa_utils.rescan_device(device) - lsblk = utils.execute( + lsblk, _ = utils.execute( 'lsblk', '-PbioKNAME,UUID,PARTUUID,TYPE,LABEL', device) - report = lsblk[0] - for line in report.split('\n'): - part = {} - # Split into KEY=VAL pairs - vals = shlex.split(line) - for key, val in (v.split('=', 1) for v in vals): - part[key] = val.strip() - # Ignore non partition - if part.get('TYPE') not in ['md', 'part']: - # NOTE(TheJulia): This technically creates an edge failure - # case where a filesystem on a whole block device sans - # partitioning would behave differently. - continue + if lsblk: + for dev in utils.parse_device_tags(lsblk): + # Ignore non partition + if dev.get('TYPE') not in ['md', 'part']: + # NOTE(TheJulia): This technically creates an edge failure + # case where a filesystem on a whole block device sans + # partitioning would behave differently. + continue - if part.get('UUID') == uuid: - LOG.debug("Partition %(uuid)s found on device " - "%(dev)s", {'uuid': uuid, 'dev': device}) - return '/dev/' + part.get('KNAME') - if part.get('PARTUUID') == uuid: - LOG.debug("Partition %(uuid)s found on device " - "%(dev)s", {'uuid': uuid, 'dev': device}) - return '/dev/' + part.get('KNAME') - if part.get('LABEL') == uuid: - LOG.debug("Partition %(uuid)s found on device " - "%(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. - # NOTE(rg): alternative: blkid -l -t UUID=/PARTUUID= - 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}) + if dev.get('UUID') == uuid: + LOG.debug("Partition %(uuid)s found on device " + "%(dev)s", {'uuid': uuid, 'dev': device}) + return '/dev/' + dev.get('KNAME') + if dev.get('PARTUUID') == uuid: + LOG.debug("Partition %(uuid)s found on device " + "%(dev)s", {'uuid': uuid, 'dev': device}) + return '/dev/' + dev.get('KNAME') + if dev.get('LABEL') == uuid: + LOG.debug("Partition %(uuid)s found on device " + "%(dev)s", {'uuid': uuid, 'dev': device}) + return '/dev/' + dev.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. + # NOTE(rg): alternative: blkid -l -t UUID=/PARTUUID= try: - findfs, stderr = utils.execute( - 'findfs', 'PARTUUID=%s' % uuid) + findfs, stderr = utils.execute('findfs', 'UUID=%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}) + 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 %(id)s failed.' + 'Error: %(err)s', {'id': uuid, 'err': e}) - # Last fallback: In case we cannot find the partition by UUID - # and the deploy device is an md device, we check if the md - # device has a partition (which we assume to contain the root fs). - if hardware.is_md_device(device): - md_partition = device + 'p1' - if (os.path.exists(md_partition) - and stat.S_ISBLK(os.stat(md_partition).st_mode)): - LOG.debug("Found md device with partition %s", - md_partition) - return md_partition - else: - LOG.debug('Could not find partition %(part)s on md ' - 'device %(dev)s', - {'part': md_partition, - 'dev': device}) + # Last fallback: In case we cannot find the partition by UUID + # and the deploy device is an md device, we check if the md + # device has a partition (which we assume to contain the + # root fs). + if hardware.is_md_device(device): + md_partition = device + 'p1' + if (os.path.exists(md_partition) + and stat.S_ISBLK(os.stat(md_partition).st_mode)): + LOG.debug("Found md device with partition %s", + md_partition) + return md_partition + else: + LOG.debug('Could not find partition %(part)s on md ' + 'device %(dev)s', {'part': md_partition, + 'dev': device}) - # Partition not found, time to escalate. - error_msg = ("No partition with UUID %(uuid)s found on " - "device %(dev)s" % {'uuid': uuid, 'dev': device}) - LOG.error(error_msg) - raise errors.DeviceNotFound(error_msg) + # Partition not found, time to escalate. + error_msg = ("No partition with UUID %(uuid)s found on " + "device %(dev)s" % {'uuid': uuid, 'dev': device}) + LOG.error(error_msg) + raise errors.DeviceNotFound(error_msg) except processutils.ProcessExecutionError as e: error_msg = ('Finding the partition with UUID %(uuid)s on ' 'device %(dev)s failed with %(err)s' % diff --git a/ironic_python_agent/tests/unit/test_partition_utils.py b/ironic_python_agent/tests/unit/test_partition_utils.py index b9893f1bd..93b6fab47 100644 --- a/ironic_python_agent/tests/unit/test_partition_utils.py +++ b/ironic_python_agent/tests/unit/test_partition_utils.py @@ -1320,7 +1320,7 @@ class TestGetPartition(base.IronicAgentTest): 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) - mock_execute.side_effect = (None, None, [lsblk_output]) + mock_execute.side_effect = ((None, ''), (None, ''), (lsblk_output, '')) root_part = partition_utils.get_partition( self.fake_dev, self.fake_root_uuid) @@ -1338,7 +1338,7 @@ class TestGetPartition(base.IronicAgentTest): KNAME="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part" KNAME="test2" UUID="" TYPE="part"''') mock_execute.side_effect = ( - None, None, [lsblk_output], + (None, ''), (None, ''), (lsblk_output, ''), processutils.ProcessExecutionError('boom'), processutils.ProcessExecutionError('kaboom')) @@ -1359,7 +1359,7 @@ class TestGetPartition(base.IronicAgentTest): KNAME="test2" UUID="" TYPE="part"''') findfs_output = ('/dev/loop0\n', None) mock_execute.side_effect = ( - None, None, [lsblk_output], + (None, ''), (None, ''), (lsblk_output, ''), processutils.ProcessExecutionError('boom'), findfs_output) @@ -1394,9 +1394,9 @@ class TestGetPartition(base.IronicAgentTest): mock_is_md_device.side_effect = [False, False] lsblk_output = ('''KNAME="test" UUID="" TYPE="disk" KNAME="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part" - KNAME="test2" UUID="903e7bf9-8a13-4f7f-811b-25dc16faf6f7" TYPE="part" \ + KNAME="test2" UUID="903e7bf9-8a13-4f7f-811b-25dc16faf6f7" TYPE="part"\ LABEL="%s"''' % self.fake_root_uuid) - mock_execute.side_effect = (None, None, [lsblk_output]) + mock_execute.side_effect = ((None, ''), (None, ''), (lsblk_output, '')) root_part = partition_utils.get_partition( self.fake_dev, self.fake_root_uuid) @@ -1413,7 +1413,7 @@ class TestGetPartition(base.IronicAgentTest): 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) - mock_execute.side_effect = (None, None, [lsblk_output]) + mock_execute.side_effect = ((None, ''), (None, ''), (lsblk_output, '')) root_part = partition_utils.get_partition( self.fake_dev, self.fake_root_uuid)