From ef47d62f4359c5b5a1b3a14a7949fd3992304f13 Mon Sep 17 00:00:00 2001 From: Szymon Borkowski Date: Wed, 23 Nov 2016 17:14:15 +0100 Subject: [PATCH] Add a new Hardware Manager for CNA network card This patch adds a new hardware manager, which will disable the embedded LLDP agent on Intel CNA network cards in order to allow the gathering of LLDP data during the inspection process. Change-Id: I572756ac6a7bf67a7f446738ba9d145e1c1bdc48 Closes-Bug: #1623659 --- ironic_python_agent/hardware.py | 20 ++- ironic_python_agent/hardware_managers/cna.py | 91 +++++++++++ .../tests/unit/hardware_managers/test_cna.py | 152 ++++++++++++++++++ ironic_python_agent/tests/unit/test_agent.py | 17 +- .../tests/unit/test_hardware.py | 59 +++++++ setup.cfg | 1 + 6 files changed, 332 insertions(+), 8 deletions(-) create mode 100644 ironic_python_agent/hardware_managers/cna.py create mode 100644 ironic_python_agent/tests/unit/hardware_managers/test_cna.py diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index cb3eb6a07..eadb35088 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -488,15 +488,25 @@ class GenericHardwareManager(HardwareManager): LOG.warning('No disks detected in %d seconds', CONF.disk_wait_delay * CONF.disk_wait_attempts) - def _cache_lldp_data(self, interface_names): + def collect_lldp_data(self, interface_names): + """Collect and convert LLDP info from the node. + + In order to process the LLDP information later, the raw data needs to + be converted for serialization purposes. + + :param interface_names: list of names of node's interfaces. + :return: a dict, containing the lldp data from every interface. + """ + interface_names = [name for name in interface_names if name != 'lo'] + lldp_data = {} try: raw_lldp_data = netutils.get_lldp_info(interface_names) except Exception: # NOTE(sambetts) The get_lldp_info function will log this exception # and we don't invalidate any existing data in the cache if we fail # to get data to replace it so just return. - return + return lldp_data for ifname, tlvs in raw_lldp_data.items(): # NOTE(sambetts) Convert each type-length-value (TLV) value to hex # so that it can be serialised safely @@ -508,7 +518,8 @@ class GenericHardwareManager(HardwareManager): except (binascii.Error, binascii.Incomplete) as e: LOG.warning('An error occurred while processing TLV type ' '%s for interface %s: %s', (typ, ifname, e)) - self.lldp_data[ifname] = processed_tlvs + lldp_data[ifname] = processed_tlvs + return lldp_data def _get_lldp_data(self, interface_name): if self.lldp_data: @@ -541,7 +552,8 @@ class GenericHardwareManager(HardwareManager): iface_names = [name for name in iface_names if self._is_device(name)] if CONF.collect_lldp: - self._cache_lldp_data(iface_names) + self.lldp_data = dispatch_to_managers('collect_lldp_data', + interface_names=iface_names) for iface_name in iface_names: result = dispatch_to_managers( diff --git a/ironic_python_agent/hardware_managers/cna.py b/ironic_python_agent/hardware_managers/cna.py new file mode 100644 index 000000000..e29623279 --- /dev/null +++ b/ironic_python_agent/hardware_managers/cna.py @@ -0,0 +1,91 @@ +# Copyright (C) 2016 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +from oslo_config import cfg +from oslo_log import log + +from ironic_python_agent import hardware +from ironic_python_agent import utils + +LOG = log.getLogger() +CONF = cfg.CONF + + +def _detect_cna_card(): + addr_path = '/sys/class/net' + for net_dev in os.listdir(addr_path): + try: + link_command = 'readlink {}/{}/device/driver/module'.format( + addr_path, net_dev) + out = utils.execute(link_command.split()) + if not out or out[1]: + continue + except OSError: + continue + driver_name = os.path.basename(out[0].strip()) + if driver_name == 'i40e': + return True + return False + + +def _disable_embedded_lldp_agent_in_cna_card(): + addr_path = '/sys/kernel/debug/i40e' + failed_dirs = [] + if os.path.exists(addr_path): + addr_dirs = os.listdir(addr_path) + else: + LOG.warning('Driver i40e was not loaded properly') + return + for inner_dir in addr_dirs: + try: + command_path = '{}/{}/command'.format(addr_path, inner_dir) + with open(command_path, 'w') as command_file: + command_file.write('lldp stop') + except (OSError, IOError): + failed_dirs.append(inner_dir) + continue + if failed_dirs: + LOG.warning('Failed to disable the embedded LLDP on Intel CNA network ' + 'card. Addresses of failed pci devices: {}' + .format(str(failed_dirs).strip('[]'))) + + +class IntelCnaHardwareManager(hardware.GenericHardwareManager): + HARDWARE_MANAGER_NAME = 'IntelCnaHardwareManager' + HARDWARE_MANAGER_VERSION = '1.0' + + def evaluate_hardware_support(self): + if _detect_cna_card(): + LOG.debug('Found Intel CNA network card') + return hardware.HardwareSupport.MAINLINE + else: + LOG.debug('No Intel CNA network card found') + return hardware.HardwareSupport.NONE + + def collect_lldp_data(self, interface_names): + """Collect and convert LLDP info from the node. + + On Intel CNA cards, in order to make LLDP info collecting possible, + the embedded LLDP agent, which runs inside that card, needs to be + turned off. Then we can give the control back to the super class. + + :param interface_names: list of names of node's interfaces. + :return: a dict, containing the lldp data from every interface. + """ + + _disable_embedded_lldp_agent_in_cna_card() + return super(IntelCnaHardwareManager, self).collect_lldp_data( + interface_names) diff --git a/ironic_python_agent/tests/unit/hardware_managers/test_cna.py b/ironic_python_agent/tests/unit/hardware_managers/test_cna.py new file mode 100644 index 000000000..e686f349a --- /dev/null +++ b/ironic_python_agent/tests/unit/hardware_managers/test_cna.py @@ -0,0 +1,152 @@ +# Copyright (C) 2016 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +import mock +from oslo_config import cfg +from oslotest import base as test_base + +from ironic_python_agent import hardware +from ironic_python_agent.hardware_managers import cna +from ironic_python_agent import utils + +CONF = cfg.CONF + + +class TestIntelCnaHardwareManager(test_base.BaseTestCase): + def setUp(self): + super(TestIntelCnaHardwareManager, self).setUp() + self.hardware = cna.IntelCnaHardwareManager() + self.node = {'uuid': 'dda135fb-732d-4742-8e72-df8f3199d244', + 'driver_internal_info': {}} + CONF.clear_override('disk_wait_attempts') + CONF.clear_override('disk_wait_delay') + + @mock.patch.object(os, 'listdir', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test_detect_cna_card(self, mock_execute, mock_listdir): + def mock_return_execute(*args, **kwargs): + if 'eth0' in args[0][1]: + return '/foo/bar/fake', '' + if 'eth1' in args[0][1]: + return '/foo/bar/i40e', '' + + mock_listdir.return_value = ['eth0', 'eth1'] + mock_execute.side_effect = mock_return_execute + self.assertEqual(True, cna._detect_cna_card()) + + @mock.patch.object(os, 'listdir', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test_detect_cna_card_execute_error(self, mock_execute, mock_listdir): + def mock_return_execute(*args, **kwargs): + if 'eth0' in args[0][1]: + return '/foo/bar/fake', '' + if 'eth1' in args[0][1]: + return '', 'fake error' + if 'eth2' in args[0][1]: + raise OSError('fake') + + mock_listdir.return_value = ['eth0', 'eth1', 'eth2'] + mock_execute.side_effect = mock_return_execute + self.assertEqual(False, cna._detect_cna_card()) + + @mock.patch.object(os, 'listdir', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test_detect_cna_card_no_i40e_driver(self, mock_execute, mock_listdir): + def mock_return_execute(*args, **kwargs): + if 'eth0' in args[0][1]: + return '/foo/bar/fake1', '' + if 'eth1' in args[0][1]: + return '/foo/bar/fake2', '' + + mock_listdir.return_value = ['eth0', 'eth1'] + mock_execute.side_effect = mock_return_execute + self.assertEqual(False, cna._detect_cna_card()) + + @mock.patch.object(cna, 'LOG', autospec=True) + @mock.patch.object(os, 'listdir', autospec=True) + @mock.patch.object(os.path, 'exists', autospec=True) + def test_disable_embedded_lldp_agent_in_cna_card(self, mock_exists, + mock_listdir, mock_log): + mock_exists.return_value = True + mock_listdir.return_value = ['foo', 'bar'] + write_mock = mock.mock_open() + with mock.patch('six.moves.builtins.open', write_mock, create=True): + cna._disable_embedded_lldp_agent_in_cna_card() + write_mock().write.assert_called_with('lldp stop') + self.assertEqual(False, mock_log.warning.called) + + @mock.patch.object(cna, 'LOG', autospec=True) + @mock.patch.object(os.path, 'exists', autospec=True) + def test_disable_embedded_lldp_agent_wrong_dir_path(self, mock_exists, + mock_log): + mock_exists.return_value = False + cna._disable_embedded_lldp_agent_in_cna_card() + expected_log_message = 'Driver i40e was not loaded properly' + mock_log.warning.assert_called_once_with(expected_log_message) + + @mock.patch.object(cna, 'LOG', autospec=True) + @mock.patch.object(os, 'listdir', autospec=True) + @mock.patch.object(os.path, 'exists', autospec=True) + def test_disable_embedded_lldp_agent_write_error(self, mock_exists, + mock_listdir, mock_log): + mock_exists.return_value = True + listdir_dict = ['foo', 'bar'] + mock_listdir.return_value = listdir_dict + write_mock = mock.mock_open() + with mock.patch('six.moves.builtins.open', write_mock, create=True): + write_mock.side_effect = IOError('fake error') + cna._disable_embedded_lldp_agent_in_cna_card() + expected_log_message = ('Failed to disable the embedded LLDP on ' + 'Intel CNA network card. Addresses of ' + 'failed pci devices: {}' + .format(str(listdir_dict).strip('[]'))) + mock_log.warning.assert_called_once_with(expected_log_message) + + @mock.patch.object(cna, 'LOG', autospec=True) + @mock.patch.object(cna, '_detect_cna_card', autospec=True) + def test_evaluate_hardware_support(self, mock_detect_card, mock_log): + mock_detect_card.return_value = True + expected_support = hardware.HardwareSupport.MAINLINE + actual_support = self.hardware.evaluate_hardware_support() + self.assertEqual(expected_support, actual_support) + mock_log.debug.assert_called_once() + + @mock.patch.object(cna, 'LOG', autospec=True) + @mock.patch.object(cna, '_detect_cna_card', autospec=True) + def test_evaluate_hardware_support_no_cna_card_detected(self, + mock_detect_card, + mock_log): + mock_detect_card.return_value = False + expected_support = hardware.HardwareSupport.NONE + actual_support = self.hardware.evaluate_hardware_support() + self.assertEqual(expected_support, actual_support) + mock_log.debug.assert_called_once() + + @mock.patch.object(hardware.GenericHardwareManager, 'collect_lldp_data', + autospec=True) + def test_collect_lldp_data(self, mock_super_collect): + iface_names = ['eth0', 'eth1'] + returned_lldp_data = [ + (0, 'foo'), + (1, 'bar'), + ] + mock_super_collect.return_value = returned_lldp_data + with mock.patch.object(cna, + '_disable_embedded_lldp_agent_in_cna_card'): + result = self.hardware.collect_lldp_data(iface_names) + mock_super_collect.assert_called_once_with(self.hardware, + iface_names) + self.assertEqual(returned_lldp_data, result) diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index b78e1aaea..62bd509e5 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -367,12 +367,14 @@ class TestBaseAgent(test_base.BaseTestCase): mock_dispatch.assert_has_calls(expected_dispatch_calls) mock_sleep.assert_has_calls(expected_sleep_calls) + @mock.patch('ironic_python_agent.hardware_managers.cna._detect_cna_card', + autospec=True) @mock.patch.object(time, 'sleep', autospec=True) @mock.patch('wsgiref.simple_server.make_server', autospec=True) @mock.patch.object(hardware, '_check_for_iscsi', autospec=True) @mock.patch.object(hardware.HardwareManager, 'list_hardware_info') def test_run_with_sleep(self, mock_check_for_iscsi, mock_list_hardware, - mock_make_server, mock_sleep): + mock_make_server, mock_sleep, mock_cna): CONF.set_override('inspection_callback_url', '', enforce_type=True) wsgi_server = mock_make_server.return_value wsgi_server.start.side_effect = KeyboardInterrupt() @@ -388,6 +390,7 @@ class TestBaseAgent(test_base.BaseTestCase): 'heartbeat_timeout': 300 } } + mock_cna.return_value = False self.agent.run() listen_addr = agent.Host('192.0.2.1', 9999) @@ -545,10 +548,13 @@ class TestAdvertiseAddress(test_base.BaseTestCase): @mock.patch.object(netutils, 'get_ipv4_addr', autospec=True) - def test_with_network_interface(self, mock_get_ipv4, mock_exec, + @mock.patch('ironic_python_agent.hardware_managers.cna._detect_cna_card', + autospec=True) + def test_with_network_interface(self, mock_cna, mock_get_ipv4, mock_exec, mock_gethostbyname): self.agent.network_interface = 'em1' mock_get_ipv4.return_value = '1.2.3.4' + mock_cna.return_value = False self.agent.set_agent_advertise_addr() @@ -560,10 +566,13 @@ class TestAdvertiseAddress(test_base.BaseTestCase): @mock.patch.object(netutils, 'get_ipv4_addr', autospec=True) - def test_with_network_interface_failed(self, mock_get_ipv4, mock_exec, - mock_gethostbyname): + @mock.patch('ironic_python_agent.hardware_managers.cna._detect_cna_card', + autospec=True) + def test_with_network_interface_failed(self, mock_cna, mock_get_ipv4, + mock_exec, mock_gethostbyname): self.agent.network_interface = 'em1' mock_get_ipv4.return_value = None + mock_cna.return_value = False self.assertRaises(errors.LookupAgentIPError, self.agent.set_agent_advertise_addr) diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 9b9fe4573..edfb76c1c 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import binascii import os import time @@ -305,6 +306,64 @@ class TestGenericHardwareManager(test_base.BaseTestCase): clean_steps = self.hardware.get_clean_steps(self.node, []) self.assertEqual(expected_clean_steps, clean_steps) + @mock.patch('binascii.hexlify') + @mock.patch('ironic_python_agent.netutils.get_lldp_info') + def test_collect_lldp_data(self, mock_lldp_info, mock_hexlify): + if_names = ['eth0', 'lo'] + mock_lldp_info.return_value = {if_names[0]: [ + (0, b''), + (1, b'foo\x01'), + (2, b'\x02bar')], + } + mock_hexlify.side_effect = [ + b'', + b'666f6f01', + b'02626172' + ] + expected_lldp_data = { + 'eth0': [ + (0, ''), + (1, '666f6f01'), + (2, '02626172')], + } + result = self.hardware.collect_lldp_data(if_names) + self.assertEqual(True, if_names[0] in result) + self.assertEqual(expected_lldp_data, result) + + @mock.patch('ironic_python_agent.netutils.get_lldp_info') + def test_collect_lldp_data_netutils_exception(self, mock_lldp_info): + if_names = ['eth0', 'lo'] + mock_lldp_info.side_effect = Exception('fake error') + result = self.hardware.collect_lldp_data(if_names) + expected_lldp_data = {} + self.assertEqual(expected_lldp_data, result) + + @mock.patch.object(hardware, 'LOG', autospec=True) + @mock.patch('binascii.hexlify') + @mock.patch('ironic_python_agent.netutils.get_lldp_info') + def test_collect_lldp_data_decode_exception(self, mock_lldp_info, + mock_hexlify, mock_log): + if_names = ['eth0', 'lo'] + mock_lldp_info.return_value = {if_names[0]: [ + (0, b''), + (1, b'foo\x01'), + (2, b'\x02bar')], + } + mock_hexlify.side_effect = [ + b'', + b'666f6f01', + binascii.Error('fake_error') + ] + expected_lldp_data = { + 'eth0': [ + (0, ''), + (1, '666f6f01')], + } + result = self.hardware.collect_lldp_data(if_names) + mock_log.warning.assert_called_once() + self.assertEqual(True, if_names[0] in result) + self.assertEqual(expected_lldp_data, result) + @mock.patch('ironic_python_agent.hardware._get_managers') @mock.patch('netifaces.ifaddresses') @mock.patch('os.listdir') diff --git a/setup.cfg b/setup.cfg index c4326c550..819eabfb5 100644 --- a/setup.cfg +++ b/setup.cfg @@ -30,6 +30,7 @@ ironic_python_agent.extensions = ironic_python_agent.hardware_managers = generic = ironic_python_agent.hardware:GenericHardwareManager mlnx = ironic_python_agent.hardware_managers.mlnx:MellanoxDeviceHardwareManager + cna = ironic_python_agent.hardware_managers.cna:IntelCnaHardwareManager ironic_python_agent.inspector.collectors = default = ironic_python_agent.inspector:collect_default