Merge "Use all valid MAC's for lookup"

ironic_inspector/plugins/discovery.py

@@ -52,7 +52,7 @@ def _extract_node_driver_info(introspection_data):
 
 
 def _check_existing_nodes(introspection_data, node_driver_info, ironic):
-    macs = introspection_data.get('macs')
+    macs = utils.get_valid_macs(introspection_data)
     if macs:
         # verify existing ports
         for mac in macs:

ironic_inspector/plugins/standard.py

@@ -191,6 +191,20 @@ class ValidateInterfacesHook(base.ProcessingHook):
                               iface, data=data)
                     continue
 
+                if not mac:
+                    LOG.debug('Skipping interface %s without link information',
+                              name, data=data)
+                    continue
+
+                if not utils.is_valid_mac(mac):
+                    LOG.warning(_LW('MAC %(mac)s for interface %(name)s is '
+                                    'not valid, skipping'),
+                                {'mac': mac, 'name': name},
+                                data=data)
+                    continue
+
+                mac = mac.lower()
+
                 LOG.debug('Found interface %(name)s with MAC "%(mac)s" and '
                           'IP address "%(ip)s"',
                           {'name': name, 'mac': mac, 'ip': ip}, data=data)
@@ -223,20 +237,6 @@ class ValidateInterfacesHook(base.ProcessingHook):
             mac = iface.get('mac')
             ip = iface.get('ip')
 
-            if not mac:
-                LOG.debug('Skipping interface %s without link information',
-                          name, data=data)
-                continue
-
-            if not utils.is_valid_mac(mac):
-                LOG.warning(_LW('MAC %(mac)s for interface %(name)s is not '
-                                'valid, skipping'),
-                            {'mac': mac, 'name': name},
-                            data=data)
-                continue
-
-            mac = mac.lower()
-
             if name == 'lo' or (ip and netaddr.IPAddress(ip).is_loopback()):
                 LOG.debug('Skipping local interface %s', name, data=data)
                 continue

ironic_inspector/process.py

@@ -39,7 +39,7 @@ def _find_node_info(introspection_data, failures):
     try:
         return node_cache.find_node(
             bmc_address=introspection_data.get('ipmi_address'),
-            mac=introspection_data.get('macs'))
+            mac=utils.get_valid_macs(introspection_data))
     except utils.NotFoundInCacheError as exc:
         not_found_hook = plugins_base.node_not_found_hook_manager()
         if not_found_hook is None:

ironic_inspector/test/test_plugins_discovery.py

@@ -102,7 +102,10 @@ class TestEnrollNodeNotFoundHook(test_base.NodeTest):
     def test__check_existing_nodes_existing_mac(self):
         self.ironic.port.list.return_value = [mock.MagicMock(
             address=self.macs[0], uuid='fake_port')]
-        introspection_data = {'macs': self.macs}
+        introspection_data = {
+            'all_interfaces': {'eth%d' % i: {'mac': m}
+                               for i, m in enumerate(self.macs)}
+        }
         node_driver_info = {}
 
         self.assertRaises(utils.Error,

ironic_inspector/test/test_process.py

@@ -54,6 +54,7 @@ class BaseTest(test_base.NodeTest):
         self.all_ports = [mock.Mock(uuid=uuidutils.generate_uuid(),
                                     address=mac) for mac in self.macs]
         self.ports = [self.all_ports[1]]
+        self.all_macs = self.macs + ['DE:AD:BE:EF:DE:AD']
 
 
 @mock.patch.object(process, '_process_node', autospec=True)
@@ -90,7 +91,9 @@ class TestProcess(BaseTest):
         self.assertEqual([self.pxe_mac], self.data['macs'])
 
         pop_mock.assert_called_once_with(bmc_address=self.bmc_address,
-                                         mac=self.data['macs'])
+                                         mac=mock.ANY)
+        actual_macs = pop_mock.call_args[1]['mac']
+        self.assertEqual(sorted(self.all_macs), sorted(actual_macs))
         cli.node.get.assert_called_once_with(self.uuid)
         process_mock.assert_called_once_with(cli.node.get.return_value,
                                              self.data, pop_mock.return_value)
@@ -100,8 +103,9 @@ class TestProcess(BaseTest):
         del self.data['ipmi_address']
         process.process(self.data)
 
-        pop_mock.assert_called_once_with(bmc_address=None,
-                                         mac=self.data['macs'])
+        pop_mock.assert_called_once_with(bmc_address=None, mac=mock.ANY)
+        actual_macs = pop_mock.call_args[1]['mac']
+        self.assertEqual(sorted(self.all_macs), sorted(actual_macs))
         cli.node.get.assert_called_once_with(self.uuid)
         process_mock.assert_called_once_with(cli.node.get.return_value,
                                              self.data, pop_mock.return_value)

ironic_inspector/utils.py

@@ -198,3 +198,10 @@ def get_auth_strategy():
     if CONF.authenticate is not None:
         return 'keystone' if CONF.authenticate else 'noauth'
     return CONF.auth_strategy
+
+
+def get_valid_macs(data):
+    """Get a list of valid MAC's from the introspection data."""
+    return [m['mac']
+            for m in data.get('all_interfaces', {}).values()
+            if m.get('mac')]

releasenotes/notes/lookup-all-macs-eead528c0b764ad7.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - The lookup procedure now uses all valid MAC's, not only the MAC(s) that
+    will be used for creating port(s).
+  - The "enroll" node_not_found_hook now uses all valid MAC's to check node
+    existence, not only the MAC(s) that will be used for creating port(s).