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

@@ -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),
+            has_carrier=netutils.interface_has_carrier(interface_name),
-            lldp=self._get_lldp_data(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:
+        return netutils.get_ipv4_addr(interface_id)
-            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
 
     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]

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

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)
 

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