diff --git a/doc/source/admin/inspection/data.rst b/doc/source/admin/inspection/data.rst index 437691113b..5b2e4a508a 100644 --- a/doc/source/admin/inspection/data.rst +++ b/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 ----------- diff --git a/doc/source/admin/inspection/discovery.rst b/doc/source/admin/inspection/discovery.rst new file mode 100644 index 0000000000..952f43e859 --- /dev/null +++ b/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. diff --git a/doc/source/admin/inspection/index.rst b/doc/source/admin/inspection/index.rst index c73487d5bf..21542e3266 100644 --- a/doc/source/admin/inspection/index.rst +++ b/doc/source/admin/inspection/index.rst @@ -26,6 +26,7 @@ ironic-inspector_ service. managed data hooks + discovery Configuration ------------- diff --git a/ironic/api/controllers/v1/ramdisk.py b/ironic/api/controllers/v1/ramdisk.py index f405af451d..47b320b24c 100644 --- a/ironic/api/controllers/v1/ramdisk.py +++ b/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,14 +374,23 @@ class ContinueInspectionController(rest.RestController): if not macs and not bmc_addresses and not node_uuid: raise exception.BadRequest(_('No lookup information provided')) - rpc_node = inspect_utils.lookup_node( - api.request.context, macs, bmc_addresses, node_uuid=node_uuid) - try: - topic = api.request.rpcapi.get_topic_for(rpc_node) - except exception.NoValidHost as e: - e.code = http_client.BAD_REQUEST - raise + 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: + e.code = http_client.BAD_REQUEST + raise if api_utils.new_continue_inspection_endpoint(): # This has to happen before continue_inspection since processing diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 67a48d9204..59d60f3ba0 100644 --- a/ironic/common/exception.py +++ b/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.") diff --git a/ironic/conductor/inspection.py b/ironic/conductor/inspection.py index 4a2888a13d..6236397437 100644 --- a/ironic/conductor/inspection.py +++ b/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) - task.process_event('fail') + if node.provision_state != states.ENROLL: + task.process_event('fail') - task.process_event('done') - LOG.info('Successfully finished inspection of node %s', node.uuid) + 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) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index f93e31c74e..5121adaa3e 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -3706,19 +3706,32 @@ 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() - task.process_event( - 'resume', - callback=self._spawn_worker, - call_args=(inspection.continue_inspection, - task, inventory, plugin_data), - err_handler=utils.provisioning_error_handler) + 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, + call_args=(inspection.continue_inspection, + task, inventory, plugin_data), + err_handler=utils.provisioning_error_handler) @METRICS.timer('ConductorManager.do_node_service') @messaging.expected_exceptions(exception.InvalidParameterValue, diff --git a/ironic/conf/inspector.py b/ironic/conf/inspector.py index c3ec562e24..ce5d51cde8 100644 --- a/ironic/conf/inspector.py +++ b/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') diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index b3bda9802c..b7227ae448 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/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) diff --git a/ironic/drivers/modules/inspect_utils.py b/ironic/drivers/modules/inspect_utils.py index 7cb7b4095c..e0d41b9d8a 100644 --- a/ironic/drivers/modules/inspect_utils.py +++ b/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 = 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() + node = _lookup_by_macs(context, mac_addresses, node) # 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, - # 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? + node = _lookup_by_bmc(context, bmc_addresses, mac_addresses, node) 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', diff --git a/ironic/objects/node.py b/ironic/objects/node.py index 64bfee5c8f..eb854d73de 100644 --- a/ironic/objects/node.py +++ b/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) diff --git a/ironic/tests/unit/api/controllers/v1/test_ramdisk.py b/ironic/tests/unit/api/controllers/v1/test_ramdisk.py index f93c8a54a3..03bfe088ae 100644 --- a/ironic/tests/unit/api/controllers/v1/test_ramdisk.py +++ b/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) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index c08137a4ab..423279ee3f 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -8605,17 +8605,34 @@ 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) - exc = self.assertRaises(messaging.rpc.ExpectedException, - self.service.continue_inspection, - self.context, node.id, - {"test": "inventory"}, - ["plugin data"]) - self.assertEqual(exception.NotFound, exc.exc_info[0]) + provision_state=states.ENROLL) + self.service.continue_inspection(self.context, node.id, + {"test": "inventory"}, + ["plugin data"]) node.refresh() - self.assertEqual(states.AVAILABLE, node.provision_state) + 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, + {"test": "inventory"}, + ["plugin data"]) + self.assertEqual(exception.NotFound, exc.exc_info[0]) + node.refresh() + self.assertEqual(state, node.provision_state) @mgr_utils.mock_record_keepalive diff --git a/ironic/tests/unit/drivers/modules/test_inspect_utils.py b/ironic/tests/unit/drivers/modules/test_inspect_utils.py index bc9738eae7..2580fb3722 100644 --- a/ironic/tests/unit/drivers/modules/test_inspect_utils.py +++ b/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): diff --git a/releasenotes/notes/auto-discovery-e90267eae7fb6f96.yaml b/releasenotes/notes/auto-discovery-e90267eae7fb6f96.yaml new file mode 100644 index 0000000000..3785409beb --- /dev/null +++ b/releasenotes/notes/auto-discovery-e90267eae7fb6f96.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Adds node auto-discovery support to the ``agent`` inspection + implementation.