openstack/ironic-python-agent
Code Issues Proposed changes

Improve Disk Detection

Browse Source 
The previous implementation of list_block_devices used blockdev,
which would list partitions, software RAID and other devices as block devices.
By switching to lsblk, the agent can filter down to only physical block
devices, which is all the agent cares about for any of its operations.

This change adds two new fields to the BlockDevice class: model, a string of
the block devices reported model, and rotational, a boolean representing a
spinning disk (True) or a solid state disk (False). This data can be useful
for vendor hardware managers.

Change-Id: I385c3bb378c2c49385bca14a1d7efa074933becf
Closes-Bug: 1344351
This commit is contained in:
Josh Gachnang 2014-07-18 08:13:12 -07:00
parent 5e1aa0a8f8
commit 83782018f7
4 changed files with 114 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
ironic_python_agent/errors.py
View File

@ -214,5 +214,14 @@ class BlockDeviceEraseError(RESTError):
self.details = details self.details = details
class BlockDeviceError(RESTError):
"""Error raised when a block devices causes an unknown error"""
message = 'Block device caused unknown error'
def __init__(self, details):
super(BlockDeviceError, self).__init__(details)
self.details = details
class ExtensionError(Exception): class ExtensionError(Exception):
pass pass

49
ironic_python_agent/hardware.py
View File

@ -15,6 +15,7 @@
import abc import abc
import functools import functools
import os import os
import shlex
import netifaces import netifaces
import psutil import psutil
@ -47,11 +48,13 @@ class HardwareType(object):
class BlockDevice(encoding.Serializable): class BlockDevice(encoding.Serializable):
serializable_fields = ('name', 'size') serializable_fields = ('name', 'model', 'size', 'rotational')
def __init__(self, name, size): def __init__(self, name, model, size, rotational):
self.name = name self.name = name
self.model = model
self.size = size self.size = size
self.rotational = rotational
class NetworkInterface(encoding.Serializable): class NetworkInterface(encoding.Serializable):
@ -210,18 +213,40 @@ class GenericHardwareManager(HardwareManager):
return Memory(int(psutil.phymem_usage().total)) return Memory(int(psutil.phymem_usage().total))
def list_block_devices(self): def list_block_devices(self):
report = utils.execute('blockdev', '--report', """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 dependant devices
(like md or dm devices), i to ensure ascii characters only,
and o to specify the fields we need
:return: A list of BlockDevices
"""
report = utils.execute('lsblk', '-PbdioKNAME,MODEL,SIZE,ROTA,TYPE',
check_exit_code=[0])[0] check_exit_code=[0])[0]
lines = report.split('\n') lines = report.split('\n')
lines = [line.split() for line in lines if line != '']
startsec_idx = lines[0].index('StartSec') devices = []
device_idx = lines[0].index('Device') for line in lines:
size_idx = lines[0].index('Size') device = {}
# If a device doesn't start at sector 0, assume it is a partition # Split into KEY=VAL pairs
return [BlockDevice(line[device_idx], vals = shlex.split(line)
int(line[size_idx])) for key, val in (v.split('=', 1) for v in vals):
for line device[key] = val.strip()
in lines[1:] if int(line[startsec_idx]) == 0] # Ignore non disk
if device.get('TYPE') != 'disk':
continue
# Ensure all required keys are at least present, even if blank
diff = set(['KNAME', 'MODEL', 'SIZE', 'ROTA']) - set(device.keys())
if diff:
raise errors.BlockDeviceError(
'%s must be returned by lsblk.' % diff)
devices.append(BlockDevice(name='/dev/' + device['KNAME'],
model=device['MODEL'],
size=int(device['SIZE']),
rotational=bool(int(device['ROTA']))))
return devices
def get_os_install_device(self): def get_os_install_device(self):
# Find the first device larger than 4GB, assume it is the OS disk # Find the first device larger than 4GB, assume it is the OS disk

78
ironic_python_agent/tests/hardware.py
View File

@ -113,6 +113,18 @@ HDPARM_INFO_TEMPLATE = (
'Checksum: correct\n' 'Checksum: correct\n'
) )
BLK_DEVICE_TEMPLATE = (
'KNAME="sda" MODEL="TinyUSB Drive" SIZE="3116853504" '
'ROTA="0" TYPE="disk"\n'
'KNAME="sdb" MODEL="Fastable SD131 7" SIZE="31016853504" '
'ROTA="0" TYPE="disk"\n'
'KNAME="sdc" MODEL="NWD-BLP4-1600 " SIZE="1765517033472" '
' ROTA="0" TYPE="disk"\n'
'KNAME="sdd" MODEL="NWD-BLP4-1600 " SIZE="1765517033472" '
' ROTA="0" TYPE="disk"\n'
'KNAME="loop0" MODEL="" SIZE="109109248" ROTA="1" TYPE="loop"'
)
class FakeHardwareManager(hardware.GenericHardwareManager): class FakeHardwareManager(hardware.GenericHardwareManager):
def __init__(self, hardware_support): def __init__(self, hardware_support):
@ -183,18 +195,10 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
@mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'execute')
def test_get_os_install_device(self, mocked_execute): def test_get_os_install_device(self, mocked_execute):
mocked_execute.return_value = ( mocked_execute.return_value = (BLK_DEVICE_TEMPLATE, '')
'RO RA SSZ BSZ StartSec Size Device\n'
'rw 256 512 4096 0 249578283616 /dev/sda\n'
'rw 256 512 4096 2048 8587837440 /dev/sda1\n'
'rw 256 512 4096 124967424 15728640 /dev/sda2\n'
'rw 256 512 4096 0 31016853504 /dev/sdb\n'
'rw 256 512 4096 0 249578283616 /dev/sdc\n', '')
self.assertEqual(self.hardware.get_os_install_device(), '/dev/sdb') self.assertEqual(self.hardware.get_os_install_device(), '/dev/sdb')
mocked_execute.assert_called_once_with('blockdev', mocked_execute.assert_called_once_with(
'--report', 'lsblk', '-PbdioKNAME,MODEL,SIZE,ROTA,TYPE', check_exit_code=[0])
check_exit_code=[0])
@mock.patch('psutil.cpu_count') @mock.patch('psutil.cpu_count')
@mock.patch(OPEN_FUNCTION_NAME) @mock.patch(OPEN_FUNCTION_NAME)
@ -282,8 +286,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
self.hardware.list_block_devices = mock.Mock() self.hardware.list_block_devices = mock.Mock()
self.hardware.list_block_devices.return_value = [ self.hardware.list_block_devices.return_value = [
hardware.BlockDevice('/dev/sdj', 1073741824), hardware.BlockDevice('/dev/sdj', 'big', 1073741824, True),
hardware.BlockDevice('/dev/hdaa', 65535), hardware.BlockDevice('/dev/hdaa', 'small', 65535, False),
] ]
hardware_info = self.hardware.list_hardware_info() hardware_info = self.hardware.list_hardware_info()
@ -294,6 +298,36 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
self.assertEqual(hardware_info['interfaces'], self.assertEqual(hardware_info['interfaces'],
self.hardware.list_network_interfaces()) self.hardware.list_network_interfaces())
@mock.patch.object(utils, 'execute')
def test_list_block_device(self, mocked_execute):
mocked_execute.return_value = (BLK_DEVICE_TEMPLATE, '')
devices = self.hardware.list_block_devices()
expected_devices = [
hardware.BlockDevice(name='/dev/sda',
model='TinyUSB Drive',
size=3116853504,
rotational=False),
hardware.BlockDevice(name='/dev/sdb',
model='Fastable SD131 7',
size=31016853504,
rotational=False),
hardware.BlockDevice(name='/dev/sdc',
model='NWD-BLP4-1600',
size=1765517033472,
rotational=False),
hardware.BlockDevice(name='/dev/sdd',
model='NWD-BLP4-1600',
size=1765517033472,
rotational=False)
]
self.assertEqual(4, len(expected_devices))
for expected, device in zip(expected_devices, devices):
# Compare all attrs of the objects
for attr in ['name', 'model', 'size', 'rotational']:
self.assertEqual(getattr(expected, attr),
getattr(device, attr))
@mock.patch.object(utils, 'execute') @mock.patch.object(utils, 'execute')
def test_erase_block_device_ata_success(self, mocked_execute): def test_erase_block_device_ata_success(self, mocked_execute):
hdparm_info_fields = { hdparm_info_fields = {
@ -308,7 +342,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
(HDPARM_INFO_TEMPLATE % hdparm_info_fields, ''), (HDPARM_INFO_TEMPLATE % hdparm_info_fields, ''),
] ]
block_device = hardware.BlockDevice('/dev/sda', 1073741824) block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824,
True)
self.hardware.erase_block_device(block_device) self.hardware.erase_block_device(block_device)
mocked_execute.assert_has_calls([ mocked_execute.assert_has_calls([
mock.call('hdparm', '-I', '/dev/sda'), mock.call('hdparm', '-I', '/dev/sda'),
@ -327,7 +362,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
(hdparm_output, '') (hdparm_output, '')
] ]
block_device = hardware.BlockDevice('/dev/sda', 1073741824) block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824,
True)
self.assertRaises(errors.BlockDeviceEraseError, self.assertRaises(errors.BlockDeviceEraseError,
self.hardware.erase_block_device, self.hardware.erase_block_device,
block_device) block_device)
@ -344,7 +380,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
(hdparm_output, '') (hdparm_output, '')
] ]
block_device = hardware.BlockDevice('/dev/sda', 1073741824) block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824,
True)
self.assertRaises(errors.BlockDeviceEraseError, self.assertRaises(errors.BlockDeviceEraseError,
self.hardware.erase_block_device, self.hardware.erase_block_device,
block_device) block_device)
@ -361,7 +398,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
(hdparm_output, '') (hdparm_output, '')
] ]
block_device = hardware.BlockDevice('/dev/sda', 1073741824) block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824,
True)
self.assertRaises(errors.BlockDeviceEraseError, self.assertRaises(errors.BlockDeviceEraseError,
self.hardware.erase_block_device, self.hardware.erase_block_device,
block_device) block_device)
@ -378,7 +416,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
(hdparm_output, '') (hdparm_output, '')
] ]
block_device = hardware.BlockDevice('/dev/sda', 1073741824) block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824,
True)
self.assertRaises(errors.BlockDeviceEraseError, self.assertRaises(errors.BlockDeviceEraseError,
self.hardware.erase_block_device, self.hardware.erase_block_device,
block_device) block_device)
@ -406,7 +445,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
(hdparm_output_after, ''), (hdparm_output_after, ''),
] ]
block_device = hardware.BlockDevice('/dev/sda', 1073741824) block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824,
True)
self.assertRaises(errors.BlockDeviceEraseError, self.assertRaises(errors.BlockDeviceEraseError,
self.hardware.erase_block_device, self.hardware.erase_block_device,
block_device) block_device)

14
ironic_python_agent/tests/ironic_api_client.py
View File

@ -46,8 +46,8 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
], ],
'cpu': hardware.CPU('Awesome Jay CPU x10 9001', '9001', '10'), 'cpu': hardware.CPU('Awesome Jay CPU x10 9001', '9001', '10'),
'disks': [ 'disks': [
hardware.BlockDevice('/dev/sdj', '9001'), hardware.BlockDevice('/dev/sdj', 'small', '9001', False),
hardware.BlockDevice('/dev/hdj', '9002'), hardware.BlockDevice('/dev/hdj', 'big', '9002', False),
], ],
'memory': hardware.Memory('8675309'), 'memory': hardware.Memory('8675309'),
} }
@ -162,13 +162,17 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
}, },
u'disks': [ u'disks': [
{ {
u'model': u'small',
u'name': u'/dev/sdj', u'name': u'/dev/sdj',
u'size': u'9001', u'rotational': False,
u'size': u'9001'
}, },
{ {
u'model': u'big',
u'name': u'/dev/hdj', u'name': u'/dev/hdj',
u'size': u'9002', u'rotational': False,
}, u'size': u'9002'
}
], ],
u'memory': { u'memory': {
u'total': u'8675309', u'total': u'8675309',