openstack/ironic-python-agent
Code Issues Proposed changes

Merge "Fix multi-device behavior"

Browse Source
This commit is contained in:
Zuul 2018-11-12 13:56:57 +00:00 committed by Gerrit Code Review
commit 3176ea483c
3 changed files with 127 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
ironic_python_agent/hardware.py
View File

@ -112,19 +112,31 @@ def _check_for_iscsi():
"Error: %s", e)
def list_all_block_devices(block_type='disk'):
def list_all_block_devices(block_type='disk',
ignore_raid=False):
"""List all physical block devices
The switches we use for lsblk: P for KEY="value" output, b for size output
in bytes, d to exclude dependent devices (like md or dm devices), i to
ensure ascii characters only, and o to specify the fields/columns we need.
in bytes, i to ensure ascii characters only, and o to specify the
fields/columns we need.
Broken out as its own function to facilitate custom hardware managers that
don't need to subclass GenericHardwareManager.
:param block_type: Type of block device to find
:param ignore_raid: Ignore auto-identified raid devices, example: md0
Defaults to false as these are generally disk
devices and should be treated as such if encountered.
:return: A list of BlockDevices
"""
def _is_known_device(existing, new_device_name):
"""Return true if device name is already known."""
for known_dev in existing:
if os.path.join('/dev', new_device_name) == known_dev.name:
return True
return False
_udev_settle()
# map device names to /dev/disk/by-path symbolic links that points to it
@ -144,14 +156,16 @@ def list_all_block_devices(block_type='disk'):
by_path_mapping[devname] = path
except OSError as e:
# NOTE(TheJulia): This is for multipath detection, and will raise
# some warning logs with unrelated tests.
LOG.warning("Path %(path)s is inaccessible, /dev/disk/by-path/* "
"version of block device name is unavailable "
"Cause: %(error)s", {'path': disk_by_path_dir, 'error': e})
columns = ['KNAME', 'MODEL', 'SIZE', 'ROTA', 'TYPE']
report = utils.execute('lsblk', '-Pbdi', '-o{}'.format(','.join(columns)),
report = utils.execute('lsblk', '-Pbi', '-o{}'.format(','.join(columns)),
check_exit_code=[0])[0]
lines = report.split('\n')
lines = report.splitlines()
context = pyudev.Context()
devices = []
@ -162,12 +176,27 @@ def list_all_block_devices(block_type='disk'):
for key, val in (v.split('=', 1) for v in vals):
device[key] = val.strip()
# Ignore block types not specified
if device.get('TYPE') != block_type:
LOG.debug(
"TYPE did not match. Wanted: {!r} but found: {!r}".format(
block_type, line))
devtype = device.get('TYPE')
# We already have devices, we should ensure we don't store duplicates.
if _is_known_device(devices, device.get('KNAME')):
continue
# Search for raid in the reply type, as RAID is a
# disk device, and we should honor it if is present.
# Other possible type values, which we skip recording:
# lvm, part, rom, loop
if devtype != block_type:
if devtype is not None and 'raid' in devtype and not ignore_raid:
LOG.debug(
"TYPE detected to contain 'raid', signifying a RAID "
"volume. Found: {!r}".format(line))
else:
LOG.debug(
"TYPE did not match. Wanted: {!r} but found: {!r}".format(
block_type, line))
continue
# Ensure all required columns are at least present, even if blank
missing = set(columns) - set(device)
if missing:

75
ironic_python_agent/tests/unit/test_hardware.py
View File

@ -156,6 +156,32 @@ BLK_DEVICE_TEMPLATE_SMALL_DEVICES = [
vendor="FooTastic"),
]
# NOTE(TheJulia): This list intentionally contains duplicates
# as the code filters them out by kernel device name.
RAID_BLK_DEVICE_TEMPLATE = (
'KNAME="sda" MODEL="DRIVE 0" SIZE="1765517033472" '
'ROTA="1" TYPE="disk"\n'
'KNAME="sdb" MODEL="DRIVE 1" SIZE="1765517033472" '
'ROTA="1" TYPE="disk"\n'
'KNAME="sdb" MODEL="DRIVE 1" SIZE="1765517033472" '
'ROTA="1" TYPE="disk"\n'
'KNAME="md0" MODEL="RAID" SIZE="1765517033470" '
'ROTA="0" TYPE="raid1"\n'
'KNAME="md0" MODEL="RAID" SIZE="1765517033470" '
'ROTA="0" TYPE="raid1"'
)
RAID_BLK_DEVICE_TEMPLATE_DEVICES = [
hardware.BlockDevice(name='/dev/sda', model='DRIVE 0',
size=1765517033472, rotational=True,
vendor="FooTastic"),
hardware.BlockDevice(name='/dev/sdb', model='DRIVE 1',
size=1765517033472, rotational=True,
vendor="FooTastic"),
hardware.BlockDevice(name='/dev/md0', model='RAID',
size=1765517033470, rotational=False,
vendor="FooTastic"),
]
SHRED_OUTPUT_0_ITERATIONS_ZERO_FALSE = ()
SHRED_OUTPUT_1_ITERATION_ZERO_TRUE = (
@ -815,7 +841,29 @@ class TestGenericHardwareManager(base.IronicAgentTest):
mocked_execute.return_value = (BLK_DEVICE_TEMPLATE, '')
self.assertEqual('/dev/sdb', self.hardware.get_os_install_device())
mocked_execute.assert_called_once_with(
'lsblk', '-Pbdi', '-oKNAME,MODEL,SIZE,ROTA,TYPE',
'lsblk', '-Pbi', '-oKNAME,MODEL,SIZE,ROTA,TYPE',
check_exit_code=[0])
mock_cached_node.assert_called_once_with()
@mock.patch.object(os, 'readlink', autospec=True)
@mock.patch.object(os, 'listdir', autospec=True)
@mock.patch.object(hardware, 'get_cached_node', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
def test_get_os_install_device_raid(self, mocked_execute,
mock_cached_node, mocked_listdir,
mocked_readlink):
# NOTE(TheJulia): The readlink and listdir mocks are just to satisfy
# what is functionally an available path check and that information
# is stored in the returned result for use by root device hints.
mocked_readlink.side_effect = '../../sda'
mocked_listdir.return_value = ['1:0:0:0']
mock_cached_node.return_value = None
mocked_execute.return_value = (RAID_BLK_DEVICE_TEMPLATE, '')
# This should ideally select the smallest device and in theory raid
# should always be smaller
self.assertEqual('/dev/md0', self.hardware.get_os_install_device())
mocked_execute.assert_called_once_with(
'lsblk', '-Pbi', '-oKNAME,MODEL,SIZE,ROTA,TYPE',
check_exit_code=[0])
mock_cached_node.assert_called_once_with()
@ -834,7 +882,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
ex = self.assertRaises(errors.DeviceNotFound,
self.hardware.get_os_install_device)
mocked_execute.assert_called_once_with(
'lsblk', '-Pbdi', '-oKNAME,MODEL,SIZE,ROTA,TYPE',
'lsblk', '-Pbi', '-oKNAME,MODEL,SIZE,ROTA,TYPE',
check_exit_code=[0])
self.assertIn(str(4 * units.Gi), ex.details)
mock_cached_node.assert_called_once_with()
@ -2201,11 +2249,30 @@ class TestModuleFunctions(base.IronicAgentTest):
mocked_execute.return_value = (BLK_DEVICE_TEMPLATE_SMALL, '')
result = hardware.list_all_block_devices()
mocked_execute.assert_called_once_with(
'lsblk', '-Pbdi', '-oKNAME,MODEL,SIZE,ROTA,TYPE',
'lsblk', '-Pbi', '-oKNAME,MODEL,SIZE,ROTA,TYPE',
check_exit_code=[0])
self.assertEqual(BLK_DEVICE_TEMPLATE_SMALL_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.Device, "from_device_file",
autospec=False)
def test_list_all_block_devices_success_raid(self, mocked_fromdevfile,
mocked_udev, mocked_readlink,
mocked_execute):
mocked_readlink.return_value = '../../sda'
mocked_fromdevfile.return_value = {}
mocked_execute.return_value = (RAID_BLK_DEVICE_TEMPLATE, '')
result = hardware.list_all_block_devices()
mocked_execute.assert_called_once_with(
'lsblk', '-Pbi', '-oKNAME,MODEL,SIZE,ROTA,TYPE',
check_exit_code=[0])
self.assertEqual(RAID_BLK_DEVICE_TEMPLATE_DEVICES, 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)
@ -2214,7 +2281,7 @@ class TestModuleFunctions(base.IronicAgentTest):
mocked_execute.return_value = ('TYPE="foo" MODEL="model"', '')
result = hardware.list_all_block_devices()
mocked_execute.assert_called_once_with(
'lsblk', '-Pbdi', '-oKNAME,MODEL,SIZE,ROTA,TYPE',
'lsblk', '-Pbi', '-oKNAME,MODEL,SIZE,ROTA,TYPE',
check_exit_code=[0])
self.assertEqual([], result)
mocked_udev.assert_called_once_with()

18
releasenotes/notes/ataraid_does_not_appear_as_disk-8a260e66b3496bf6.yaml Normal file
View File

@ -0,0 +1,18 @@
---
fixes:
- |
Fixes an issue where devices offered via ATARAID
(or even software RAID setup by a hardware manager)
were excluded from the list as they are not returned
as type disk, even though they should be considered
both disks and block devices.
See `story <https://storyboard.openstack.org/#!/story/2003445>`_
for more details.
upgrade:
- |
Operators with custom hardware managers to support software RAID
may wish to ensure that they have removed the software RAID prior
to any stock cleaning step for erasing block devices executes.
As a result of a bug fix, such devices may now be picked up in
the list of possible devices to delete, which may extend cleaning
depending on the deployment configuration.