Add node auto-discovery support for in-band inspection

This is a MVP of auto-discovery with no extra customization and no new
auto_discovered field from the spec.

Change-Id: I1528096aa08da6af4ac3c45b71d00e86947ed556

doc/source/admin/inspection/data.rst

@@ -50,6 +50,8 @@ Use `jq` to filter the parts you need, e.g. only the inventory itself:
      }
    }
 
+.. _plugin-data:
+
 Plugin data
 -----------
 

doc/source/admin/inspection/discovery.rst

@@ -0,0 +1,44 @@
+Node auto-discovery
+===================
+
+The Bare Metal service is capable of automatically enrolling new nodes that
+somehow (through external means, e.g. :ref:`configure-unmanaged-inspection`)
+boot into an IPA ramdisk and call back with inspection data. This feature must
+be enabled explicitly in the configuration:
+
+.. code-block:: ini
+
+   [DEFAULT]
+   default_inspect_interface = agent
+
+   [auto_discovery]
+   enabled = True
+   driver = ipmi
+
+The newly created nodes will appear in the ``enroll`` provision state with the
+``driver`` field set to the value specified in the configuration, as well as a
+boolean ``auto_discovered`` flag in the :ref:`plugin-data`.
+
+After the node is enrolled, it will automatically go through the normal
+inspection process, which includes, among other things, creating ports.
+Any errors during this process will be reflected in the node's ``last_error``
+field (the node will not be deleted).
+
+.. TODO(dtantsur): inspection rules examples once ready
+
+Limitations
+-----------
+
+* Setting BMC credentials is a manual task. The Bare Metal service does not
+  generate new credentials for you even on those machines where it's possible
+  through ``ipmitool``.
+
+* Node uniqueness is checked using the supplied MAC addresses. In rare cases,
+  it is possible to create duplicate nodes.
+
+* Enabling discovery allows anyone with API access to create nodes with given
+  MAC addresses and store inspection data of arbitrary size for them. This can
+  be used for denial-of-service attacks.
+
+* Setting ``default_inspect_interface`` is required for the inspection flow
+  to continue correctly after the node creation.

doc/source/admin/inspection/index.rst

@@ -26,6 +26,7 @@ ironic-inspector_ service.
    managed
    data
    hooks
+   discovery
 
 Configuration
 -------------

ironic/api/controllers/v1/ramdisk.py

@@ -16,10 +16,12 @@ from http import client as http_client
 
 from oslo_config import cfg
 from oslo_log import log
+from oslo_utils import uuidutils
 from pecan import rest
 
 from ironic import api
 from ironic.api.controllers.v1 import node as node_ctl
+from ironic.api.controllers.v1 import notification_utils as notify
 from ironic.api.controllers.v1 import utils as api_utils
 from ironic.api.controllers.v1 import versions
 from ironic.api import method
@@ -298,6 +300,45 @@ DATA_VALIDATOR = args.schema({
 class ContinueInspectionController(rest.RestController):
     """Controller handling inspection data from deploy ramdisk."""
 
+    def _auto_enroll(self, macs, bmc_addresses):
+        context = api.request.context
+        new_node = objects.Node(
+            context,
+            conductor_group='',  # TODO(dtantsur): default_conductor_group
+            driver=CONF.auto_discovery.driver,
+            provision_state=states.ENROLL,
+            resource_class=CONF.default_resource_class,
+            uuid=uuidutils.generate_uuid())
+
+        try:
+            topic = api.request.rpcapi.get_topic_for(new_node)
+        except exception.NoValidHost as e:
+            LOG.error("Failed to find a conductor to handle the newly "
+                      "enrolled node with driver %s: %s", new_node.driver, e)
+            # NOTE(dtantsur): do not disclose any information to the caller
+            raise exception.IronicException()
+
+        LOG.info("Enrolling the newly discovered node %(uuid)s with driver "
+                 "%(driver)s, MAC addresses [%(macs)s] and BMC address(es) "
+                 "[%(bmc)s]",
+                 {'driver': new_node.driver,
+                  'uuid': new_node.uuid,
+                  'macs': ', '.join(macs or ()),
+                  'bmc': ', '.join(bmc_addresses or ())})
+
+        notify.emit_start_notification(context, new_node, 'create')
+        with notify.handle_error_notification(context, new_node, 'create'):
+            try:
+                node = api.request.rpcapi.create_node(
+                    context, new_node, topic=topic)
+            except exception.IronicException:
+                LOG.exception("Failed to enroll node with driver %s",
+                              new_node.driver)
+                # NOTE(dtantsur): do not disclose any information to the caller
+                raise exception.IronicException()
+
+        return node, topic
+
     @method.expose(status_code=http_client.ACCEPTED)
     @method.body('data')
     @args.validate(data=DATA_VALIDATOR, node_uuid=args.uuid)
@@ -333,9 +374,18 @@ class ContinueInspectionController(rest.RestController):
         if not macs and not bmc_addresses and not node_uuid:
             raise exception.BadRequest(_('No lookup information provided'))
 
+        try:
             rpc_node = inspect_utils.lookup_node(
                 api.request.context, macs, bmc_addresses, node_uuid=node_uuid)
-
+        except inspect_utils.AutoEnrollPossible:
+            if not CONF.auto_discovery.enabled:
+                raise exception.NotFound()
+            rpc_node, topic = self._auto_enroll(macs, bmc_addresses)
+            # TODO(dtantsur): consider adding a Node-level property to make
+            # newly discovered nodes searchable via API. The flag in
+            # plugin_data is for compatibility with ironic-inspector.
+            data[inspect_utils.AUTO_DISCOVERED_FLAG] = True
+        else:
             try:
                 topic = api.request.rpcapi.get_topic_for(rpc_node)
             except exception.NoValidHost as e:

ironic/common/exception.py

@@ -273,6 +273,10 @@ class NodeNotFound(NotFound):
     _msg_fmt = _("Node %(node)s could not be found.")
 
 
+class DuplicateNodeOnLookup(NodeNotFound):
+    pass  # Same error message, the difference only matters internally
+
+
 class PortgroupNotFound(NotFound):
     _msg_fmt = _("Portgroup %(portgroup)s could not be found.")
 

ironic/conductor/inspection.py

@@ -149,7 +149,11 @@ def continue_inspection(task, inventory, plugin_data):
             utils.node_history_record(task.node, event=error,
                                       event_type=states.INTROSPECTION,
                                       error=True)
+            if node.provision_state != states.ENROLL:
                 task.process_event('fail')
 
+    if node.provision_state != states.ENROLL:
         task.process_event('done')
         LOG.info('Successfully finished inspection of node %s', node.uuid)
+    else:
+        LOG.info('Successfully finished auto-discovery of node %s', node.uuid)

ironic/conductor/manager.py

@@ -3706,13 +3706,26 @@ class ConductorManager(base_manager.BaseConductorManager):
                                   purpose='continue inspection',
                                   shared=False) as task:
             # TODO(dtantsur): support active state (re-)inspection
-            if task.node.provision_state != states.INSPECTWAIT:
+            accepted_states = {states.INSPECTWAIT}
+            if CONF.auto_discovery.enabled:
+                accepted_states.add(states.ENROLL)
+
+            if task.node.provision_state not in accepted_states:
                 LOG.error('Refusing to process inspection data for node '
                           '%(node)s in invalid state %(state)s',
                           {'node': task.node.uuid,
                            'state': task.node.provision_state})
                 raise exception.NotFound()
 
+            if task.node.provision_state == states.ENROLL:
+                task.set_spawn_error_hook(
+                    utils.provisioning_error_handler,
+                    task.node, states.ENROLL, None)
+                task.spawn_after(
+                    self._spawn_worker,
+                    inspection.continue_inspection,
+                    task, inventory, plugin_data)
+            else:
                 task.process_event(
                     'resume',
                     callback=self._spawn_worker,

ironic/conf/inspector.py

@@ -142,9 +142,25 @@ opts = [
                        'option is used by the "root-device" inspection hook.'))
 ]
 
+discovery_opts = [
+    cfg.BoolOpt('enabled',
+                default=False, mutable=True,
+                help=_("Setting this to True enables automatic enrollment "
+                       "of inspected nodes that are not recognized. "
+                       "When enabling this feature, keep in mind that any "
+                       "machine hitting the inspection callback endpoint "
+                       "will be automatically enrolled. The driver must be "
+                       "set when setting this to True.")),
+    cfg.StrOpt('driver',
+               mutable=True,
+               help=_("The default driver to use for newly enrolled nodes. "
+                      "Must be set when enabling auto-discovery.")),
+]
+
 
 def register_opts(conf):
     conf.register_opts(opts, group='inspector')
+    conf.register_opts(discovery_opts, group='auto_discovery')
     auth.register_auth_opts(conf, 'inspector',
                             service_type='baremetal-introspection')
 

ironic/db/sqlalchemy/api.py

@@ -1621,7 +1621,7 @@ class Connection(api.Connection):
                 _('Node with port addresses %s was not found')
                 % addresses)
         except MultipleResultsFound:
-            raise exception.NodeNotFound(
+            raise exception.DuplicateNodeOnLookup(
                 _('Multiple nodes with port addresses %s were found')
                 % addresses)
 

ironic/drivers/modules/inspect_utils.py

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from http import client as http_client
 import socket
 import urllib
 
@@ -30,6 +31,7 @@ from ironic.objects import node_inventory
 
 LOG = logging.getLogger(__name__)
 _OBJECT_NAME_PREFIX = 'inspector_data'
+AUTO_DISCOVERED_FLAG = 'auto_discovered'
 
 
 def create_ports_if_not_exist(task, macs=None):
@@ -222,6 +224,108 @@ def _get_inspection_data_from_swift(node_uuid):
 LOOKUP_CACHE_FIELD = 'lookup_bmc_addresses'
 
 
+class AutoEnrollPossible(exception.IronicException):
+    """Exception to indicate that the node can be enrolled.
+
+    The error message and code is the same as for NotFound to make sure
+    we don't disclose any information when discovery is disabled.
+    """
+    code = http_client.NOT_FOUND
+
+
+def _lookup_by_macs(context, mac_addresses, known_node=None):
+    """Lookup the node by its MAC addresses.
+
+    :param context: Request context.
+    :param mac_addresses: List of MAC addresses reported by the ramdisk.
+    :param known_node: Node object if the UUID was provided by the ramdisk.
+    :returns: Newly found node or known_node if nothing is found.
+    """
+    try:
+        node = objects.Node.get_by_port_addresses(context, mac_addresses)
+    except exception.DuplicateNodeOnLookup:
+        LOG.error('Conflict on inspection lookup: multiple nodes match '
+                  'MAC addresses %s', ', '.join(mac_addresses))
+        raise exception.NotFound()
+    except exception.NotFound as exc:
+        # The exception has enough context already, just log it and move on
+        LOG.debug("Lookup for inspection: %s", exc)
+        return known_node
+
+    if known_node and node.uuid != known_node.uuid:
+        LOG.error('Conflict on inspection lookup: node %(node1)s '
+                  'does not match MAC addresses (%(macs)s), which '
+                  'belong to node %(node2)s. This may be a sign of '
+                  'incorrectly created ports.',
+                  {'node1': known_node.uuid,
+                   'node2': node.uuid,
+                   'macs': ', '.join(mac_addresses)})
+        raise exception.NotFound()
+
+    return node
+
+
+def _lookup_by_bmc(context, bmc_addresses, mac_addresses, known_node=None):
+    """Lookup the node by its BMC (IPMI) addresses.
+
+    :param context: Request context.
+    :param bmc_addresses: List of BMC addresses reported by the ramdisk.
+    :param mac_addresses: List of MAC addresses reported by the ramdisk
+        (for logging purposes).
+    :param known_node: Node object if the UUID was provided by the ramdisk.
+    :returns: Newly found node or known_node if nothing is found.
+    """
+    # NOTE(dtantsur): the same BMC hostname can be used by several nodes,
+    # e.g. in case of Redfish. Find all suitable nodes first.
+    nodes_by_bmc = set()
+    for candidate in objects.Node.list(
+            context,
+            filters={'provision_state': states.INSPECTWAIT},
+            fields=['uuid', 'driver_internal_info']):
+        # This field has to be populated on inspection start
+        for addr in candidate.driver_internal_info.get(
+                LOOKUP_CACHE_FIELD) or ():
+            if addr in bmc_addresses:
+                nodes_by_bmc.add(candidate.uuid)
+
+    # NOTE(dtantsur): if none of the nodes found by the BMC match the one
+    # found by the MACs, something is definitely wrong.
+    if known_node and nodes_by_bmc and known_node.uuid not in nodes_by_bmc:
+        LOG.error('Conflict on inspection lookup: nodes %(node1)s '
+                  'and %(node2)s both satisfy MAC addresses '
+                  '(%(macs)s) and BMC address(s) (%(bmc)s). The cause '
+                  'may be ports attached to a wrong node.',
+                  {'node1': ', '.join(nodes_by_bmc),
+                   'node2': known_node.uuid,
+                   'macs': ', '.join(mac_addresses),
+                   'bmc': ', '.join(bmc_addresses)})
+        raise exception.NotFound()
+
+    # NOTE(dtantsur): at this point, if the node was found by the MAC
+    # addresses, it also matches the BMC address. We only need to handle
+    # the case when the node was not found by the MAC addresses.
+    if not known_node and nodes_by_bmc:
+        if len(nodes_by_bmc) > 1:
+            LOG.error('Several nodes %(nodes)s satisfy BMC address(s) '
+                      '(%(bmc)s), but none of them satisfy MAC addresses '
+                      '(%(macs)s). Ports must be created for a successful '
+                      'inspection in this case.',
+                      {'nodes': ', '.join(nodes_by_bmc),
+                       'macs': ', '.join(mac_addresses),
+                       'bmc': ', '.join(bmc_addresses)})
+            raise exception.NotFound()
+
+        node_uuid = nodes_by_bmc.pop()
+        try:
+            # Fetch the complete object now.
+            return objects.Node.get_by_uuid(context, node_uuid)
+        except exception.NotFound:
+            raise  # Deleted in-between?
+
+    # Fall back to what is known already
+    return known_node
+
+
 def lookup_node(context, mac_addresses, bmc_addresses, node_uuid=None):
     """Do a node lookup by the information from the inventory.
 
@@ -232,6 +336,9 @@ def lookup_node(context, mac_addresses, bmc_addresses, node_uuid=None):
     :raises: NotFound with a generic message for all failures to avoid
         disclosing any information.
     """
+    if not node_uuid and not mac_addresses and not bmc_addresses:
+        raise exception.BadRequest()
+
     node = None
     if node_uuid:
         try:
@@ -243,21 +350,7 @@ def lookup_node(context, mac_addresses, bmc_addresses, node_uuid=None):
             raise exception.NotFound()
 
     if mac_addresses:
-        try:
+        node = _lookup_by_macs(context, mac_addresses, node)
-            node = objects.Node.get_by_port_addresses(context, mac_addresses)
-        except exception.NotFound as exc:
-            # The exception has enough context already, just log it and move on
-            LOG.debug("Lookup for inspection: %s", exc)
-        else:
-            if node_uuid and node.uuid != node_uuid:
-                LOG.error('Conflict on inspection lookup: node %(node1)s '
-                          'does not match MAC addresses (%(macs)s), which '
-                          'belong to node %(node2)s. This may be a sign of '
-                          'incorrectly created ports.',
-                          {'node1': node_uuid,
-                           'node2': node.uuid,
-                           'macs': ', '.join(mac_addresses)})
-                raise exception.NotFound()
 
     # TODO(dtantsur): support active state inspection
     if node and node.provision_state != states.INSPECTWAIT:
@@ -277,59 +370,14 @@ def lookup_node(context, mac_addresses, bmc_addresses, node_uuid=None):
     # to updating wrong nodes.
 
     if bmc_addresses:
-        # NOTE(dtantsur): the same BMC hostname can be used by several nodes,
+        node = _lookup_by_bmc(context, bmc_addresses, mac_addresses, node)
-        # e.g. in case of Redfish. Find all suitable nodes first.
-        nodes_by_bmc = set()
-        for candidate in objects.Node.list(
-                context,
-                filters={'provision_state': states.INSPECTWAIT},
-                fields=['uuid', 'driver_internal_info']):
-            # This field has to be populated on inspection start
-            for addr in candidate.driver_internal_info.get(
-                    LOOKUP_CACHE_FIELD) or ():
-                if addr in bmc_addresses:
-                    nodes_by_bmc.add(candidate.uuid)
-
-        # NOTE(dtantsur): if none of the nodes found by the BMC match the one
-        # found by the MACs, something is definitely wrong.
-        if node and nodes_by_bmc and node.uuid not in nodes_by_bmc:
-            LOG.error('Conflict on inspection lookup: nodes %(node1)s '
-                      'and %(node2)s both satisfy MAC addresses '
-                      '(%(macs)s) and BMC address(s) (%(bmc)s). The cause '
-                      'may be ports attached to a wrong node.',
-                      {'node1': ', '.join(nodes_by_bmc),
-                       'node2': node.uuid,
-                       'macs': ', '.join(mac_addresses),
-                       'bmc': ', '.join(bmc_addresses)})
-            raise exception.NotFound()
-
-        # NOTE(dtantsur): at this point, if the node was found by the MAC
-        # addresses, it also matches the BMC address. We only need to handle
-        # the case when the node was not found by the MAC addresses.
-        if not node and nodes_by_bmc:
-            if len(nodes_by_bmc) > 1:
-                LOG.error('Several nodes %(nodes)s satisfy BMC address(s) '
-                          '(%(bmc)s), but none of them satisfy MAC addresses '
-                          '(%(macs)s). Ports must be created for a successful '
-                          'inspection in this case.',
-                          {'nodes': ', '.join(nodes_by_bmc),
-                           'macs': ', '.join(mac_addresses),
-                           'bmc': ', '.join(bmc_addresses)})
-                raise exception.NotFound()
-
-            node_uuid = nodes_by_bmc.pop()
-            try:
-                # Fetch the complete object now.
-                node = objects.Node.get_by_uuid(context, node_uuid)
-            except exception.NotFound:
-                raise  # Deleted in-between?
 
     if not node:
         LOG.error('No nodes satisfy MAC addresses (%(macs)s) and BMC '
                   'address(s) (%(bmc)s) during inspection lookup',
                   {'macs': ', '.join(mac_addresses),
                    'bmc': ', '.join(bmc_addresses)})
-        raise exception.NotFound()
+        raise AutoEnrollPossible()
 
     LOG.debug('Inspection lookup succeeded for node %(node)s using MAC '
               'addresses %(mac)s and BMC addresses %(bmc)s',

ironic/objects/node.py

@@ -1102,7 +1102,7 @@ class NodeCRUDPayload(NodePayload):
         'driver_info': object_fields.FlexibleDictField(nullable=True)
     }
 
-    def __init__(self, node, chassis_uuid):
+    def __init__(self, node, chassis_uuid=None):
         super(NodeCRUDPayload, self).__init__(node, chassis_uuid=chassis_uuid)
 
 

ironic/tests/unit/api/controllers/v1/test_ramdisk.py

@@ -26,6 +26,7 @@ from oslo_utils import uuidutils
 from ironic.api.controllers import base as api_base
 from ironic.api.controllers import v1 as api_v1
 from ironic.api.controllers.v1 import ramdisk
+from ironic.common import exception
 from ironic.common import states
 from ironic.conductor import rpcapi
 from ironic.drivers.modules import inspect_utils
@@ -525,3 +526,82 @@ class TestContinueInspectionScopedRBAC(TestContinueInspection):
         cfg.CONF.set_override('enforce_new_defaults', True,
                               group='oslo_policy')
         cfg.CONF.set_override('auth_strategy', 'keystone')
+
+
+@mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for', autospec=True,
+                   return_value='test-topic')
+@mock.patch.object(rpcapi.ConductorAPI, 'create_node', autospec=True)
+@mock.patch.object(rpcapi.ConductorAPI, 'continue_inspection', autospec=True)
+@mock.patch.object(inspect_utils, 'lookup_node', autospec=True,
+                   side_effect=inspect_utils.AutoEnrollPossible)
+class TestContinueInspectionAutoDiscovery(test_api_base.BaseApiTest):
+
+    def setUp(self):
+        super().setUp()
+        CONF.set_override('enabled', True, group='auto_discovery')
+        CONF.set_override('driver', 'fake-hardware', group='auto_discovery')
+        self.addresses = ['11:22:33:44:55:66', '66:55:44:33:22:11']
+        self.bmcs = ['192.0.2.42']
+        self.inventory = {
+            'bmc_address': self.bmcs[0],
+            'interfaces': [
+                {'mac_address': mac, 'name': f'em{i}'}
+                for i, mac in enumerate(self.addresses)
+            ],
+        }
+        self.data = {
+            'inventory': self.inventory,
+            'test': 42,
+        }
+        self.node = obj_utils.get_test_node(self.context,
+                                            uuid=uuidutils.generate_uuid(),
+                                            provision_state='enroll')
+
+    def test_enroll(self, mock_lookup, mock_continue, mock_create,
+                    mock_get_topic):
+        mock_create.return_value = self.node
+        response = self.post_json('/continue_inspection', self.data)
+        self.assertEqual(http_client.ACCEPTED, response.status_int)
+        self.assertEqual({'uuid': self.node.uuid}, response.json)
+        mock_lookup.assert_called_once_with(
+            mock.ANY, self.addresses, self.bmcs, node_uuid=None)
+        mock_continue.assert_called_once_with(
+            mock.ANY, mock.ANY, self.node.uuid, inventory=self.inventory,
+            plugin_data={'test': 42, 'auto_discovered': True},
+            topic='test-topic')
+        new_node = mock_create.call_args.args[2]  # create(self, context, node)
+        self.assertEqual('fake-hardware', new_node.driver)
+        self.assertIsNone(new_node.resource_class)
+        self.assertEqual('', new_node.conductor_group)
+        self.assertEqual('enroll', new_node.provision_state)
+
+    def test_wrong_driver(self, mock_lookup, mock_continue, mock_create,
+                          mock_get_topic):
+        mock_get_topic.side_effect = exception.NoValidHost()
+        response = self.post_json(
+            '/continue_inspection', self.data,
+            expect_errors=True)
+        self.assertEqual(http_client.INTERNAL_SERVER_ERROR,
+                         response.status_int)
+        mock_lookup.assert_called_once_with(
+            mock.ANY, self.addresses, self.bmcs, node_uuid=None)
+        mock_create.assert_not_called()
+        mock_continue.assert_not_called()
+
+    def test_override_defaults(self, mock_lookup, mock_continue, mock_create,
+                               mock_get_topic):
+        CONF.set_override('default_resource_class', 'xlarge-1')
+        # TODO(dtantsur): default_conductor_group
+        mock_create.return_value = self.node
+        response = self.post_json('/continue_inspection', self.data)
+        self.assertEqual(http_client.ACCEPTED, response.status_int)
+        mock_lookup.assert_called_once_with(
+            mock.ANY, self.addresses, self.bmcs, node_uuid=None)
+        mock_continue.assert_called_once_with(
+            mock.ANY, mock.ANY, self.node.uuid, inventory=self.inventory,
+            plugin_data={'test': 42, 'auto_discovered': True},
+            topic='test-topic')
+        new_node = mock_create.call_args.args[2]  # create(self, context, node)
+        self.assertEqual('fake-hardware', new_node.driver)
+        self.assertEqual('xlarge-1', new_node.resource_class)
+        self.assertEqual('', new_node.conductor_group)

ironic/tests/unit/conductor/test_manager.py

@@ -8605,9 +8605,26 @@ class ContinueInspectionTestCase(mgr_utils.ServiceSetUpMixin,
             self.service, inspection.continue_inspection, mock.ANY,
             {"test": "inventory"}, ["plugin data"])
 
-    def test_wrong_state(self):
+    @mock.patch.object(manager.ConductorManager, '_spawn_worker',
+                       autospec=True)
+    def test_continue_with_discovery(self, mock_spawn):
+        CONF.set_override('enabled', True, group='auto_discovery')
         node = obj_utils.create_test_node(self.context,
-                                          provision_state=states.AVAILABLE)
+                                          provision_state=states.ENROLL)
+        self.service.continue_inspection(self.context, node.id,
+                                         {"test": "inventory"},
+                                         ["plugin data"])
+        node.refresh()
+        self.assertEqual(states.ENROLL, node.provision_state)
+        mock_spawn.assert_called_once_with(
+            self.service, inspection.continue_inspection, mock.ANY,
+            {"test": "inventory"}, ["plugin data"])
+
+    def test_wrong_state(self):
+        for state in (states.ENROLL, states.AVAILABLE, states.ACTIVE):
+            node = obj_utils.create_test_node(self.context,
+                                              uuid=uuidutils.generate_uuid(),
+                                              provision_state=state)
             exc = self.assertRaises(messaging.rpc.ExpectedException,
                                     self.service.continue_inspection,
                                     self.context, node.id,
@@ -8615,7 +8632,7 @@ class ContinueInspectionTestCase(mgr_utils.ServiceSetUpMixin,
                                     ["plugin data"])
             self.assertEqual(exception.NotFound, exc.exc_info[0])
             node.refresh()
-        self.assertEqual(states.AVAILABLE, node.provision_state)
+            self.assertEqual(state, node.provision_state)
 
 
 @mgr_utils.mock_record_keepalive

ironic/tests/unit/drivers/modules/test_inspect_utils.py

@@ -322,7 +322,7 @@ class LookupNodeTestCase(db_base.DbTestCase):
                                    address=self.mac2)
 
     def test_no_input(self):
-        self.assertRaises(exception.NotFound, utils.lookup_node,
+        self.assertRaises(exception.BadRequest, utils.lookup_node,
                           self.context, [], [], None)
 
     def test_by_macs(self):
@@ -335,7 +335,7 @@ class LookupNodeTestCase(db_base.DbTestCase):
         self.assertEqual(self.node.uuid, result.uuid)
 
     def test_by_mac_not_found(self):
-        self.assertRaises(exception.NotFound, utils.lookup_node,
+        self.assertRaises(utils.AutoEnrollPossible, utils.lookup_node,
                           self.context, [self.unknown_mac], [], None)
 
     def test_by_mac_wrong_state(self):
@@ -368,13 +368,21 @@ class LookupNodeTestCase(db_base.DbTestCase):
         self.assertEqual(self.node.uuid, result.uuid)
 
     def test_by_bmc_not_found(self):
-        self.assertRaises(exception.NotFound, utils.lookup_node,
+        self.assertRaises(utils.AutoEnrollPossible, utils.lookup_node,
                           self.context, [], ['192.168.1.1'], None)
 
+    def test_by_bmc_and_mac_not_found(self):
+        self.assertRaises(utils.AutoEnrollPossible, utils.lookup_node,
+                          self.context, [self.unknown_mac],
+                          ['192.168.1.1'], None)
+
     def test_by_bmc_wrong_state(self):
         self.node.provision_state = states.AVAILABLE
         self.node.save()
-        self.assertRaises(exception.NotFound, utils.lookup_node,
+        # Limitation of auto-discovery: cannot de-duplicate nodes by BMC
+        # addresses only. Should not happen too often in reality.
+        # If it does happen, auto-discovery will create a duplicate node.
+        self.assertRaises(utils.AutoEnrollPossible, utils.lookup_node,
                           self.context, [], [self.bmc], None)
 
     def test_conflicting_macs_and_bmc(self):

releasenotes/notes/auto-discovery-e90267eae7fb6f96.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    Adds node auto-discovery support to the ``agent`` inspection
+    implementation.