openstack/ironic-python-agent
Code Issues Proposed changes

Replace shlex module with helper function

Browse Source 
Used helper function, `parse_device_tags`
from ironic_lib instead of the
shlex module for their identical
functionality. Updated
mock_execute.side_effect for lsblk
compatibility in utils.execute.

Closes-Bug: #2037572
Change-Id: I6600e054f9644c67ab003f0e0f6c380b5c217223
This commit is contained in:
Boushra Bettir
2023-10-11 21:38:25 -07:00
parent cb61a8d6c0
commit dbf3e5408d
3 changed files with 66 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
ironic_python_agent/hardware.py
View File

@@ -435,6 +435,8 @@ def md_get_raid_devices():
:return: A python dict containing details about the discovered RAID :return: A python dict containing details about the discovered RAID
devices 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] report = il_utils.execute('mdadm', '--examine', '--scan')[0]
lines = report.splitlines() lines = report.splitlines()
result = {} result = {}

126
ironic_python_agent/partition_utils.py
View File

@@ -22,7 +22,6 @@ import gzip
import io import io
import math import math
import os import os
import shlex
import shutil import shutil
import stat import stat
import tempfile import tempfile
@@ -611,80 +610,71 @@ def get_partition(device, uuid):
try: try:
ipa_utils.rescan_device(device) ipa_utils.rescan_device(device)
lsblk = utils.execute( lsblk, _ = utils.execute(
'lsblk', '-PbioKNAME,UUID,PARTUUID,TYPE,LABEL', device) 'lsblk', '-PbioKNAME,UUID,PARTUUID,TYPE,LABEL', device)
report = lsblk[0] if lsblk:
for line in report.split('\n'): for dev in utils.parse_device_tags(lsblk):
part = {} # Ignore non partition
# Split into KEY=VAL pairs if dev.get('TYPE') not in ['md', 'part']:
vals = shlex.split(line) # NOTE(TheJulia): This technically creates an edge failure
for key, val in (v.split('=', 1) for v in vals): # case where a filesystem on a whole block device sans
part[key] = val.strip() # partitioning would behave differently.
# Ignore non partition continue
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 part.get('UUID') == uuid: if dev.get('UUID') == uuid:
LOG.debug("Partition %(uuid)s found on device " LOG.debug("Partition %(uuid)s found on device "
"%(dev)s", {'uuid': uuid, 'dev': device}) "%(dev)s", {'uuid': uuid, 'dev': device})
return '/dev/' + part.get('KNAME') return '/dev/' + dev.get('KNAME')
if part.get('PARTUUID') == uuid: if dev.get('PARTUUID') == uuid:
LOG.debug("Partition %(uuid)s found on device " LOG.debug("Partition %(uuid)s found on device "
"%(dev)s", {'uuid': uuid, 'dev': device}) "%(dev)s", {'uuid': uuid, 'dev': device})
return '/dev/' + part.get('KNAME') return '/dev/' + dev.get('KNAME')
if part.get('LABEL') == uuid: if dev.get('LABEL') == uuid:
LOG.debug("Partition %(uuid)s found on device " LOG.debug("Partition %(uuid)s found on device "
"%(dev)s", {'uuid': uuid, 'dev': device}) "%(dev)s", {'uuid': uuid, 'dev': device})
return '/dev/' + part.get('KNAME') return '/dev/' + dev.get('KNAME')
else: else:
# NOTE(TheJulia): We may want to consider moving towards using # NOTE(TheJulia): We may want to consider moving towards using
# findfs in the future, if we're comfortable with the execution # findfs in the future, if we're comfortable with the execution
# and interaction. There is value in either way though. # and interaction. There is value in either way though.
# NOTE(rg): alternative: blkid -l -t UUID=/PARTUUID= # 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})
try: try:
findfs, stderr = utils.execute( findfs, stderr = utils.execute('findfs', 'UUID=%s' % uuid)
'findfs', 'PARTUUID=%s' % uuid)
return findfs.strip() return findfs.strip()
except processutils.ProcessExecutionError as e: except processutils.ProcessExecutionError as e:
LOG.debug('Secondary fallback detection attempt for ' LOG.debug('First fallback detection attempt for locating '
'locating partition via UUID %(uuid)s failed. ' 'partition via UUID %(uuid)s failed. '
'Error: %(err)s', 'Error: %(err)s', {'uuid': uuid, 'err': e})
{'uuid': uuid, try:
'err': e}) 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 # Last fallback: In case we cannot find the partition by UUID
# and the deploy device is an md device, we check if the md # 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). # device has a partition (which we assume to contain the
if hardware.is_md_device(device): # root fs).
md_partition = device + 'p1' if hardware.is_md_device(device):
if (os.path.exists(md_partition) md_partition = device + 'p1'
and stat.S_ISBLK(os.stat(md_partition).st_mode)): if (os.path.exists(md_partition)
LOG.debug("Found md device with partition %s", and stat.S_ISBLK(os.stat(md_partition).st_mode)):
md_partition) LOG.debug("Found md device with partition %s",
return md_partition md_partition)
else: return md_partition
LOG.debug('Could not find partition %(part)s on md ' else:
'device %(dev)s', LOG.debug('Could not find partition %(part)s on md '
{'part': md_partition, 'device %(dev)s', {'part': md_partition,
'dev': device}) 'dev': device})
# Partition not found, time to escalate. # Partition not found, time to escalate.
error_msg = ("No partition with UUID %(uuid)s found on " error_msg = ("No partition with UUID %(uuid)s found on "
"device %(dev)s" % {'uuid': uuid, 'dev': device}) "device %(dev)s" % {'uuid': uuid, 'dev': device})
LOG.error(error_msg) LOG.error(error_msg)
raise errors.DeviceNotFound(error_msg) raise errors.DeviceNotFound(error_msg)
except processutils.ProcessExecutionError as e: except processutils.ProcessExecutionError as e:
error_msg = ('Finding the partition with UUID %(uuid)s on ' error_msg = ('Finding the partition with UUID %(uuid)s on '
'device %(dev)s failed with %(err)s' % 'device %(dev)s failed with %(err)s' %

12
ironic_python_agent/tests/unit/test_partition_utils.py
View File

@@ -1320,7 +1320,7 @@ class TestGetPartition(base.IronicAgentTest):
lsblk_output = ('''KNAME="test" UUID="" TYPE="disk" lsblk_output = ('''KNAME="test" UUID="" TYPE="disk"
KNAME="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part" KNAME="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part"
KNAME="test2" UUID="%s" TYPE="part"''' % self.fake_root_uuid) 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( root_part = partition_utils.get_partition(
self.fake_dev, self.fake_root_uuid) 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="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part"
KNAME="test2" UUID="" TYPE="part"''') KNAME="test2" UUID="" TYPE="part"''')
mock_execute.side_effect = ( mock_execute.side_effect = (
None, None, [lsblk_output], (None, ''), (None, ''), (lsblk_output, ''),
processutils.ProcessExecutionError('boom'), processutils.ProcessExecutionError('boom'),
processutils.ProcessExecutionError('kaboom')) processutils.ProcessExecutionError('kaboom'))
@@ -1359,7 +1359,7 @@ class TestGetPartition(base.IronicAgentTest):
KNAME="test2" UUID="" TYPE="part"''') KNAME="test2" UUID="" TYPE="part"''')
findfs_output = ('/dev/loop0\n', None) findfs_output = ('/dev/loop0\n', None)
mock_execute.side_effect = ( mock_execute.side_effect = (
None, None, [lsblk_output], (None, ''), (None, ''), (lsblk_output, ''),
processutils.ProcessExecutionError('boom'), processutils.ProcessExecutionError('boom'),
findfs_output) findfs_output)
@@ -1394,9 +1394,9 @@ class TestGetPartition(base.IronicAgentTest):
mock_is_md_device.side_effect = [False, False] mock_is_md_device.side_effect = [False, False]
lsblk_output = ('''KNAME="test" UUID="" TYPE="disk" lsblk_output = ('''KNAME="test" UUID="" TYPE="disk"
KNAME="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part" 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) 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( root_part = partition_utils.get_partition(
self.fake_dev, self.fake_root_uuid) self.fake_dev, self.fake_root_uuid)
@@ -1413,7 +1413,7 @@ class TestGetPartition(base.IronicAgentTest):
lsblk_output = ('''KNAME="test" UUID="" TYPE="disk" lsblk_output = ('''KNAME="test" UUID="" TYPE="disk"
KNAME="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part" KNAME="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part"
KNAME="test2" PARTUUID="%s" TYPE="part"''' % self.fake_root_uuid) 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( root_part = partition_utils.get_partition(
self.fake_dev, self.fake_root_uuid) self.fake_dev, self.fake_root_uuid)