From 9db3cd1e4d72462eb9303fd917f2d19a823cf4f0 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Thu, 3 Jul 2025 11:48:44 +0200 Subject: [PATCH] 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 --- doc/source/admin/hardware_managers.rst | 4 + ironic_python_agent/hardware.py | 47 +++++++- .../tests/unit/test_hardware.py | 104 ++++++++++++++++++ .../notes/filter-device-90e90f0814f26b6d.yaml | 5 + 4 files changed, 154 insertions(+), 6 deletions(-) create mode 100644 releasenotes/notes/filter-device-90e90f0814f26b6d.yaml diff --git a/doc/source/admin/hardware_managers.rst b/doc/source/admin/hardware_managers.rst index 065d25853..f9eb360af 100644 --- a/doc/source/admin/hardware_managers.rst +++ b/doc/source/admin/hardware_managers.rst @@ -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 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 4c1aac3fb..d90bad406 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -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 + ] diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 7da6aa222..5d999ed40 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -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) diff --git a/releasenotes/notes/filter-device-90e90f0814f26b6d.yaml b/releasenotes/notes/filter-device-90e90f0814f26b6d.yaml new file mode 100644 index 000000000..7cf839383 --- /dev/null +++ b/releasenotes/notes/filter-device-90e90f0814f26b6d.yaml @@ -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.

AltStyle によって変換されたページ (->オリジナル) /