Graceful way for hardware managers to ignore certain devices
My use case for this feature is to exclude network devices that use the cdc_ether driver. These USB network interfaces often cause all sorts of issues. For example, some models have the same hardcoded MAC address, which breaks inspection. Currently, to exclude a certain device, a hardware manager must override the entire listing function (in my case, list_interfaces). Not only is it tedious, but it also requires constantly updating the hardware managers to match the implementation in GenericHardware. Realistically, it will cause hardware manager authors to inherit GenericHardware, which is the opposite of how hardware managers should be written. Note that the node-level skip list only affects root device selection and cleaning for block devices. This feature affects everything that uses list_block_devices and is applied before the node-level skip list. This change adds a new hardware manager call filter_device. For each network, block or USB device, it allows a hardware manager to do either of four things: 1. Delegate the decision to a lower level hardware manager by raising IncompatibleHardwareMethodError 2. Remove the device by returning None 3. Change the device by returning a modified instance 4. Return the device unchanged to keep it in the listing. Note that I'm removing debug logging when IncompatibleHardwareMethodError is raised. Not only the log message is incorrect (the error does not necessarily mean that the method is not implemented at all), it already noticeable space in the logs, and with this change will become very noisy. Change-Id: I5437343af6c6157882bcf0600dd89bd20478c948 Signed-off-by: Dmitry Tantsur <dtantsur@protonmail.com>
This commit is contained in:
4 changed files with 154 additions and 6 deletions
@@ -181,6 +181,10 @@ For example::
'skip_block_devices': [{'volume_name': 'large'}, {'volume_name': 'temp'}]
Another option is to completely remove the device from the listing by
implementing the ``filter_device`` call in a site-specific hardware manager.
This affects not just deployment and cleaning, but also inspection and anything
that is relying on built-in device listings.
Shared Disk Cluster Filesystems
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -1350,6 +1350,31 @@ class HardwareManager(object, metaclass=abc.ABCMeta):
"""
raise errors.IncompatibleHardwareMethodError()
def filter_device(self, device):
"""Filter a device in various listings.
This call allows hardware managers to change or remove devices in
listings, such as list_interfaces or list_block_devices without
overriding these calls. Skipped devices will be invisible to the agent,
including security-sensitive processes like cleaning, so use with care.
The device type should be determined from the class of the ``device``
parameter.
If the hardware manager has no opinion about the provided device, it
must raise IncompatibleHardwareMethodError. Otherwise, it must return
the (potentially modified) device to keep it in the listing or None
to exclude it.
The hardware manager must not modify the device if it returns None
or raises IncompatibleHardwareMethodError!
:param device: An object with the device information.
:raises: IncompatibleHardwareMethodError to delegate filtering to
other hardware managers.
:return: The modified device or None to exclude it.
"""
class GenericHardwareManager(HardwareManager):
HARDWARE_MANAGER_NAME = 'generic_hardware_manager'
@@ -1574,7 +1599,7 @@ class GenericHardwareManager(HardwareManager):
'get_interface_info', interface_name=vlan_iface_name)
network_interfaces_list.append(result)
return network_interfaces_list
return filter_devices(network_interfaces_list)
def any_ipmi_device_exists(self):
'''Check for an IPMI device to confirm IPMI capability.'''
@@ -1721,7 +1746,7 @@ class GenericHardwareManager(HardwareManager):
list_all_block_devices(block_type='part',
ignore_raid=True)
)
return block_devices
return filter_devices(block_devices)
def get_skip_list_from_node(self, node,
block_devices=None, just_raids=False):
@@ -1841,7 +1866,7 @@ class GenericHardwareManager(HardwareManager):
vendor=dev.get('vendor', ''),
handle=dev.get('handle', ''))
devices.append(usb_info)
return devices
return filter_devices(devices)
def get_system_vendor_info(self):
try:
@@ -3510,6 +3535,10 @@ class GenericHardwareManager(HardwareManager):
LOG.warning('Cannot flush buffers of device %s: %s',
blkdev.name, e)
def filter_device(self, device):
"""Filter a device in various listings."""
return device # always include, do not modify
def _collect_udev(io_dict):
"""Collect device properties from udev."""
@@ -3693,9 +3722,7 @@ def dispatch_to_managers(method, *args, **kwargs):
{'manager': manager, 'e': e})
raise
except errors.IncompatibleHardwareMethodError:
LOG.debug('HardwareManager %(manager)s does not '
'support %(method)s',
{'manager': manager, 'method': method})
pass
except Exception as e:
LOG.exception('Unexpected error dispatching %(method)s to '
'manager %(manager)s: %(e)s',
@@ -3945,3 +3972,11 @@ def _check_for_special_partitions_filesystems(device, ids, fs_types):
raise errors.ProtectedDeviceError(
device=device,
what=value)
def filter_devices(device_list):
"""Filter devices by using the Hardware Manager's filter_device calls."""
return [
new for orig in device_list
if (new := dispatch_to_managers('filter_device', orig)) is not None
]
@@ -1230,6 +1230,35 @@ class TestGenericHardwareManager(base.IronicAgentTest):
mock.call(block_type='part', ignore_raid=True)],
list_mock.call_args_list)
@mock.patch.object(hardware.GenericHardwareManager, 'filter_device',
autospec=True)
@mock.patch.object(hardware, 'list_all_block_devices', autospec=True)
def test_list_block_devices_with_filter_device(self, list_mock,
filter_mock):
device = hardware.BlockDevice('/dev/hdaa', 'small', 65535, False)
list_mock.return_value = [
device,
hardware.BlockDevice('/dev/rogue', 'fake', 42, True),
]
seen_devices = set()
def _filter(hwmgr, device_to_filter):
self.assertIsInstance(device_to_filter, hardware.BlockDevice)
seen_devices.add(device_to_filter.name)
if 'rogue' in device_to_filter.name:
return None
self.assertEqual(device, device_to_filter)
return device_to_filter
filter_mock.side_effect = _filter
devices = self.hardware.list_block_devices()
self.assertEqual([device], devices)
self.assertEqual({'/dev/hdaa', '/dev/rogue'}, seen_devices)
list_mock.assert_called_once_with(all_serial_and_wwn=False)
def test_get_skip_list_from_node_block_devices_with_skip_list(self):
block_devices = [
hardware.BlockDevice('/dev/sdj', 'big', 1073741824, True),
@@ -5511,6 +5540,27 @@ class TestGenericHardwareManager(base.IronicAgentTest):
self.assertEqual([device], detected_usb_devices)
@mock.patch.object(hardware.GenericHardwareManager, 'filter_device',
autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
def test_get_usb_devices_with_filter_device(self, mocked_execute,
mocked_filter):
seen_devices = set()
device = hardware.USBInfo('MyProduct', 'MyVendor', 'USB:1:2')
def _filter(hwmgr, device_to_filter):
self.assertIsInstance(device_to_filter, hardware.USBInfo)
self.assertEqual(device, device_to_filter)
seen_devices.add(device_to_filter.product)
return None
mocked_filter.side_effect = _filter
mocked_execute.return_value = hws.LSHW_JSON_OUTPUT_V1
detected_usb_devices = self.hardware.get_usb_devices()
self.assertEqual([], detected_usb_devices)
self.assertEqual({'MyProduct'}, seen_devices)
@mock.patch.object(utils, 'get_agent_params',
lambda: {'BOOTIF': 'boot:if'})
@mock.patch.object(os.path, 'isdir', autospec=True)
@@ -6924,6 +6974,60 @@ class TestListNetworkInterfaces(base.IronicAgentTest):
self.assertEqual('eth1.102', interfaces[4].name)
self.assertEqual('eth1.103', interfaces[5].name)
@mock.patch.object(hardware.GenericHardwareManager, 'filter_device',
autospec=True)
def test_list_network_interfaces_with_filter_device(
self, mock_filter_device, mock_has_carrier, mocked_execute,
mocked_open, mocked_exists, mocked_listdir, mocked_net_if_addrs,
mockedget_managers, mocked_lshw, mocked_get_mac_addr):
mocked_lshw.return_value = json.loads(hws.LSHW_JSON_OUTPUT_V2[0])
mocked_listdir.return_value = ['lo', 'eth0', 'eth1']
mocked_exists.side_effect = [False, False, True, True]
mocked_open.return_value.__enter__ = lambda s: s
mocked_open.return_value.__exit__ = mock.Mock()
read_mock = mocked_open.return_value.read
read_mock.side_effect = ['1']
mocked_net_if_addrs.return_value = {
'lo': [
FakeAddr(socket.AF_INET, '127.0.0.1'),
FakeAddr(socket.AF_INET6, '::1'),
FakeAddr(socket.AF_PACKET, '00:00:00:00:00:00')
],
'eth0': [
FakeAddr(socket.AF_INET, '192.168.1.2'),
FakeAddr(socket.AF_INET6, 'fd00::101'),
FakeAddr(socket.AF_PACKET, '00:0c:29:8c:11:b1')
],
'eth1': [
FakeAddr(socket.AF_INET, '192.168.2.2'),
FakeAddr(socket.AF_INET6, 'fd00:1000::101'),
FakeAddr(socket.AF_PACKET, '00:0c:29:8c:11:b2')
]
}
mocked_get_mac_addr.side_effect = lambda iface: {
'lo': '00:00:00:00:00:00',
'eth0': '00:0c:29:8c:11:b1',
'eth1': '00:0c:29:8c:11:b2',
}.get(iface)
mocked_execute.return_value = ('em0\n', '')
mock_has_carrier.return_value = True
seen_devices = set()
def _filter(hwmgr, device):
self.assertIsInstance(device, hardware.NetworkInterface)
seen_devices.add(device.name)
if device.name == 'eth1':
return None
return device
mock_filter_device.side_effect = _filter
interfaces = self.hardware.list_network_interfaces()
self.assertEqual(1, len(interfaces))
self.assertEqual('eth0', interfaces[0].name)
self.assertEqual({'eth0', 'eth1'}, seen_devices)
@mock.patch.object(hardware, 'dispatch_to_managers', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
5
releasenotes/notes/filter-device-90e90f0814f26b6d.yaml
Normal file
5
releasenotes/notes/filter-device-90e90f0814f26b6d.yaml
Normal file
@@ -0,0 +1,5 @@
---
features:
- |
Adds a new way for hardware manager to filter devices in the network
interface, block device, and USB device listings.
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.