From 966db1c18c0c2e3626fb66e6d22ceb92d2f7d61e Mon Sep 17 00:00:00 2001 From: Moshe Levi Date: Thu, 17 Nov 2016 02:08:13 +0200 Subject: [PATCH] Dispatched out network interface info to all hardware managers This patch dispatches out the network_interface_info to allow vendor hardware managers to plug the spacific implementation. It also move neworking releated methods form hardware to netutils Related-Bug: #1532534 Change-Id: Idcd25c4753c009b5ba70bea97ee4eb83391a77a9 --- ironic_python_agent/hardware.py | 36 ++++++++----------- ironic_python_agent/netutils.py | 21 +++++++++++ ironic_python_agent/tests/unit/test_agent.py | 9 ++--- .../tests/unit/test_hardware.py | 25 ++++++++++--- 4 files changed, 60 insertions(+), 31 deletions(-) diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index b89eb5ac1..7c8787753 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -21,7 +21,6 @@ import time from ironic_lib import disk_utils from ironic_lib import utils as il_utils -import netifaces from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log @@ -291,6 +290,9 @@ class HardwareManager(object): def get_boot_info(self): raise errors.IncompatibleHardwareMethodError() + def get_interface_info(self, interface_name): + raise errors.IncompatibleHardwareMethodError() + def erase_block_device(self, node, block_device): """Attempt to erase a block device. @@ -496,7 +498,7 @@ class GenericHardwareManager(HardwareManager): if self.lldp_data: return self.lldp_data.get(interface_name) - def _get_interface_info(self, interface_name): + def get_interface_info(self, interface_name): addr_path = '{}/class/net/{}/address'.format(self.sys_path, interface_name) with open(addr_path) as addr_file: @@ -505,29 +507,12 @@ class GenericHardwareManager(HardwareManager): return NetworkInterface( interface_name, mac_addr, ipv4_address=self.get_ipv4_addr(interface_name), - has_carrier=self._interface_has_carrier(interface_name), - lldp=self._get_lldp_data(interface_name), + has_carrier=netutils.interface_has_carrier(interface_name), vendor=_get_device_info(interface_name, 'net', 'vendor'), product=_get_device_info(interface_name, 'net', 'device')) def get_ipv4_addr(self, interface_id): - try: - addrs = netifaces.ifaddresses(interface_id) - return addrs[netifaces.AF_INET][0]['addr'] - except (ValueError, IndexError, KeyError): - # No default IPv4 address found - return None - - def _interface_has_carrier(self, interface_name): - path = '{}/class/net/{}/carrier'.format(self.sys_path, - interface_name) - try: - with open(path, 'rt') as fp: - return fp.read().strip() == '1' - except EnvironmentError: - LOG.debug('No carrier information for interface %s', - interface_name) - return False + return netutils.get_ipv4_addr(interface_id) def _is_device(self, interface_name): device_path = '{}/class/net/{}/device'.format(self.sys_path, @@ -535,13 +520,20 @@ class GenericHardwareManager(HardwareManager): return os.path.exists(device_path) def list_network_interfaces(self): + network_interfaces_list = [] iface_names = os.listdir('{}/class/net'.format(self.sys_path)) iface_names = [name for name in iface_names if self._is_device(name)] if CONF.collect_lldp: self._cache_lldp_data(iface_names) - return [self._get_interface_info(name) for name in iface_names] + for iface_name in iface_names: + result = dispatch_to_managers( + 'get_interface_info', interface_name=iface_name) + result.lldp = self._get_lldp_data(iface_name) + network_interfaces_list.append(result) + + return network_interfaces_list def get_cpus(self): lines = utils.execute('lscpu')[0] diff --git a/ironic_python_agent/netutils.py b/ironic_python_agent/netutils.py index c5a8bfb9d..f1078fb8e 100644 --- a/ironic_python_agent/netutils.py +++ b/ironic_python_agent/netutils.py @@ -19,6 +19,7 @@ import socket import struct import sys +import netifaces from oslo_config import cfg from oslo_log import log as logging @@ -191,3 +192,23 @@ def _get_lldp_info(interfaces): lldp_info[name] = [] return lldp_info + + +def get_ipv4_addr(interface_id): + try: + addrs = netifaces.ifaddresses(interface_id) + return addrs[netifaces.AF_INET][0]['addr'] + except (ValueError, IndexError, KeyError): + # No default IPv4 address found + return None + + +def interface_has_carrier(interface_name): + path = '/sys/class/net/{}/carrier'.format(interface_name) + try: + with open(path, 'rt') as fp: + return fp.read().strip() == '1' + except EnvironmentError: + LOG.debug('No carrier information for interface %s', + interface_name) + return False diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index 7c35ec682..349cd9de7 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -30,6 +30,7 @@ from ironic_python_agent import errors from ironic_python_agent.extensions import base from ironic_python_agent import hardware from ironic_python_agent import inspector +from ironic_python_agent import netutils from ironic_python_agent import utils EXPECTED_ERROR = RuntimeError('command execution failed') @@ -448,7 +449,7 @@ class TestAdvertiseAddress(test_base.BaseTestCase): self.assertFalse(mock_exec.called) self.assertFalse(mock_gethostbyname.called) - @mock.patch.object(hardware.GenericHardwareManager, 'get_ipv4_addr', + @mock.patch.object(netutils, 'get_ipv4_addr', autospec=True) def test_with_network_interface(self, mock_get_ipv4, mock_exec, mock_gethostbyname): @@ -459,11 +460,11 @@ class TestAdvertiseAddress(test_base.BaseTestCase): self.assertEqual(agent.Host('1.2.3.4', 9990), self.agent.advertise_address) - mock_get_ipv4.assert_called_once_with(mock.ANY, 'em1') + mock_get_ipv4.assert_called_once_with('em1') self.assertFalse(mock_exec.called) self.assertFalse(mock_gethostbyname.called) - @mock.patch.object(hardware.GenericHardwareManager, 'get_ipv4_addr', + @mock.patch.object(netutils, 'get_ipv4_addr', autospec=True) def test_with_network_interface_failed(self, mock_get_ipv4, mock_exec, @@ -474,7 +475,7 @@ class TestAdvertiseAddress(test_base.BaseTestCase): self.assertRaises(errors.LookupAgentIPError, self.agent.set_agent_advertise_addr) - mock_get_ipv4.assert_called_once_with(mock.ANY, 'em1') + mock_get_ipv4.assert_called_once_with('em1') self.assertFalse(mock_exec.called) self.assertFalse(mock_gethostbyname.called) diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index c4b33f01b..ab1661d4b 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -305,6 +305,7 @@ class TestGenericHardwareManager(test_base.BaseTestCase): clean_steps = self.hardware.get_clean_steps(self.node, []) self.assertEqual(expected_clean_steps, clean_steps) + @mock.patch('ironic_python_agent.hardware._get_managers') @mock.patch('netifaces.ifaddresses') @mock.patch('os.listdir') @mock.patch('os.path.exists') @@ -313,7 +314,9 @@ class TestGenericHardwareManager(test_base.BaseTestCase): mocked_open, mocked_exists, mocked_listdir, - mocked_ifaddresses): + mocked_ifaddresses, + mocked_get_managers): + mocked_get_managers.return_value = [hardware.GenericHardwareManager()] mocked_listdir.return_value = ['lo', 'eth0'] mocked_exists.side_effect = [False, True] mocked_open.return_value.__enter__ = lambda s: s @@ -331,6 +334,7 @@ class TestGenericHardwareManager(test_base.BaseTestCase): self.assertIsNone(interfaces[0].lldp) self.assertTrue(interfaces[0].has_carrier) + @mock.patch('ironic_python_agent.hardware._get_managers') @mock.patch('ironic_python_agent.netutils.get_lldp_info') @mock.patch('netifaces.ifaddresses') @mock.patch('os.listdir') @@ -341,7 +345,9 @@ class TestGenericHardwareManager(test_base.BaseTestCase): mocked_exists, mocked_listdir, mocked_ifaddresses, - mocked_lldp_info): + mocked_lldp_info, + mocked_get_managers): + mocked_get_managers.return_value = [hardware.GenericHardwareManager()] CONF.set_override('collect_lldp', True) mocked_listdir.return_value = ['lo', 'eth0'] mocked_exists.side_effect = [False, True] @@ -372,6 +378,7 @@ class TestGenericHardwareManager(test_base.BaseTestCase): self.assertEqual(expected_lldp_info, interfaces[0].lldp) self.assertTrue(interfaces[0].has_carrier) + @mock.patch('ironic_python_agent.hardware._get_managers') @mock.patch('ironic_python_agent.netutils.get_lldp_info') @mock.patch('netifaces.ifaddresses') @mock.patch('os.listdir') @@ -379,7 +386,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase): @mock.patch('six.moves.builtins.open') def test_list_network_interfaces_with_lldp_error( self, mocked_open, mocked_exists, mocked_listdir, - mocked_ifaddresses, mocked_lldp_info): + mocked_ifaddresses, mocked_lldp_info, mocked_get_managers): + mocked_get_managers.return_value = [hardware.GenericHardwareManager()] CONF.set_override('collect_lldp', True) mocked_listdir.return_value = ['lo', 'eth0'] mocked_exists.side_effect = [False, True] @@ -399,6 +407,7 @@ class TestGenericHardwareManager(test_base.BaseTestCase): self.assertIsNone(interfaces[0].lldp) self.assertTrue(interfaces[0].has_carrier) + @mock.patch('ironic_python_agent.hardware._get_managers') @mock.patch('netifaces.ifaddresses') @mock.patch('os.listdir') @mock.patch('os.path.exists') @@ -407,7 +416,10 @@ class TestGenericHardwareManager(test_base.BaseTestCase): mocked_open, mocked_exists, mocked_listdir, - mocked_ifaddresses): + mocked_ifaddresses, + mocked_get_managers): + + mocked_get_managers.return_value = [hardware.GenericHardwareManager()] mocked_listdir.return_value = ['lo', 'eth0'] mocked_exists.side_effect = [False, True] mocked_open.return_value.__enter__ = lambda s: s @@ -425,6 +437,7 @@ class TestGenericHardwareManager(test_base.BaseTestCase): self.assertFalse(interfaces[0].has_carrier) self.assertIsNone(interfaces[0].vendor) + @mock.patch('ironic_python_agent.hardware._get_managers') @mock.patch('netifaces.ifaddresses') @mock.patch('os.listdir') @mock.patch('os.path.exists') @@ -433,7 +446,9 @@ class TestGenericHardwareManager(test_base.BaseTestCase): mocked_open, mocked_exists, mocked_listdir, - mocked_ifaddresses): + mocked_ifaddresses, + mocked_get_managers): + mocked_get_managers.return_value = [hardware.GenericHardwareManager()] mocked_listdir.return_value = ['lo', 'eth0'] mocked_exists.side_effect = [False, True] mocked_open.return_value.__enter__ = lambda s: s