diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index b6b120a4d8..cb371f2526 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1410,21 +1410,6 @@ class NodesController(rest.RestController): n_interface is not wtypes.Unset): raise exception.NotAcceptable() - # NOTE(vsaienko) The validation is performed on API side, - # all conductors and api should have the same list of - # enabled_network_interfaces. - # TODO(vsaienko) remove it once driver-composition-reform - # is implemented. - if (n_interface is not wtypes.Unset and - not api_utils.is_valid_network_interface(n_interface)): - error_msg = _("Cannot create node with the invalid network " - "interface '%(n_interface)s'. Enabled network " - "interfaces are: %(enabled_int)s") - raise wsme.exc.ClientSideError( - error_msg % {'n_interface': n_interface, - 'enabled_int': CONF.enabled_network_interfaces}, - status_code=http_client.BAD_REQUEST) - # NOTE(deva): get_topic_for checks if node.driver is in the hash ring # and raises NoValidHost if it is not. # We need to ensure that node has a UUID before it can @@ -1433,7 +1418,7 @@ class NodesController(rest.RestController): node.uuid = uuidutils.generate_uuid() try: - pecan.request.rpcapi.get_topic_for(node) + topic = pecan.request.rpcapi.get_topic_for(node) except exception.NoValidHost as e: # NOTE(deva): convert from 404 to 400 because client can see # list of available drivers and shouldn't request @@ -1448,7 +1433,8 @@ class NodesController(rest.RestController): new_node = objects.Node(pecan.request.context, **node.as_dict()) - new_node.create() + new_node = pecan.request.rpcapi.create_node( + pecan.request.context, new_node, topic) # Set the HTTP Location Header pecan.response.location = link.build_url('nodes', new_node.uuid) return Node.convert_with_links(new_node) @@ -1476,17 +1462,6 @@ class NodesController(rest.RestController): if n_interfaces and not api_utils.allow_network_interface(): raise exception.NotAcceptable() - for n_interface in n_interfaces: - if (n_interface is not None and - not api_utils.is_valid_network_interface(n_interface)): - error_msg = _("Node %(node)s: Cannot change " - "network_interface to invalid value: " - "%(n_interface)s") - raise wsme.exc.ClientSideError( - error_msg % {'node': node_ident, - 'n_interface': n_interface}, - status_code=http_client.BAD_REQUEST) - rpc_node = api_utils.get_rpc_node(node_ident) remove_inst_uuid_patch = [{'op': 'remove', 'path': '/instance_uuid'}] diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 832f7f8a95..5f11f6bb6e 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -276,21 +276,6 @@ def check_allowed_fields(fields): raise exception.NotAcceptable() -# NOTE(vsaienko) The validation is performed on API side, all conductors -# and api should have the same list of enabled_network_interfaces. -# TODO(vsaienko) remove it once driver-composition-reform is implemented. -def is_valid_network_interface(network_interface): - """Determine if the provided network_interface is valid. - - Check to see that the provided network_interface is in the enabled - network interfaces list. - - :param: network_interface: the node network interface to check. - :returns: True if the network_interface is valid, False otherwise. - """ - return network_interface in CONF.enabled_network_interfaces - - def check_allow_management_verbs(verb): min_version = MIN_VERB_VERSIONS.get(verb) if min_version is not None and pecan.request.version.minor < min_version: diff --git a/ironic/common/driver_factory.py b/ironic/common/driver_factory.py index 90040b7d85..94d51ed3e1 100644 --- a/ironic/common/driver_factory.py +++ b/ironic/common/driver_factory.py @@ -44,8 +44,11 @@ def build_driver_for_task(task, driver_name=None): :returns: A driver object for the task. :raises: DriverNotFound if node.driver could not be found in the "ironic.drivers" namespace. + :raises: InterfaceNotFoundInEntrypoint if some node interfaces are set + to invalid or unsupported values. """ node = task.node + check_and_update_node_interfaces(node) driver = driver_base.BareDriver() _attach_interfaces_to_driver(driver, node, driver_name=driver_name) return driver @@ -62,12 +65,40 @@ def _attach_interfaces_to_driver(driver, node, driver_name=None): try: net_driver = network_factory.get_driver(network_iface) except KeyError: - raise exception.DriverNotFoundInEntrypoint( - driver_name=network_iface, - entrypoint=network_factory._entrypoint_name) + raise exception.InterfaceNotFoundInEntrypoint( + iface=network_iface, + entrypoint=network_factory._entrypoint_name, + valid=network_factory.names) driver.network = net_driver +def check_and_update_node_interfaces(node): + """Ensure that node interfaces (e.g. for creation or updating) are valid. + + Updates interfaces with calculated defaults, if they are not provided. + + :param node: node object to check and potentially update + :raises: InterfaceNotFoundInEntrypoint on validation failure + :returns: True if any changes were made to the node, otherwise False + """ + # NOTE(dtantsur): objects raise NotImplementedError on accessing fields + # that are known, but missing from an object. Thus, we cannot just use + # getattr(node, 'network_interface', None) here. + if 'network_interface' in node and node.network_interface is not None: + if node.network_interface not in CONF.enabled_network_interfaces: + raise exception.InterfaceNotFoundInEntrypoint( + iface=node.network_interface, + entrypoint=NetworkInterfaceFactory._entrypoint_name, + valid=NetworkInterfaceFactory().names) + else: + node.network_interface = ( + CONF.default_network_interface or + ('flat' if CONF.dhcp.dhcp_provider == 'neutron' else 'noop')) + return True + + return False + + def get_driver(driver_name): """Simple method to get a ref to an instance of a driver. diff --git a/ironic/common/exception.py b/ironic/common/exception.py index ee73f5f25f..3d69ea428c 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -250,6 +250,12 @@ class DriverNotFoundInEntrypoint(DriverNotFound): "'%(entrypoint)s' entrypoint: %(driver_name)s.") +class InterfaceNotFoundInEntrypoint(InvalidParameterValue): + _msg_fmt = _("Could not find the following interface in the " + "'%(entrypoint)s' entrypoint: %(iface)s. Valid interfaces " + "are %(valid)s.") + + class ImageNotFound(NotFound): _msg_fmt = _("Image %(image_id)s could not be found.") diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 03daab5b3b..31c2d42b50 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -82,7 +82,7 @@ class ConductorManager(base_manager.BaseConductorManager): """Ironic Conductor manager main class.""" # NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's. - RPC_API_VERSION = '1.35' + RPC_API_VERSION = '1.36' target = messaging.Target(version=RPC_API_VERSION) @@ -90,10 +90,29 @@ class ConductorManager(base_manager.BaseConductorManager): super(ConductorManager, self).__init__(host, topic) self.power_state_sync_count = collections.defaultdict(int) + @METRICS.timer('ConductorManager.create_node') + @messaging.expected_exceptions(exception.InvalidParameterValue, + exception.InterfaceNotFoundInEntrypoint) + def create_node(self, context, node_obj): + """Create a node in database. + + :param context: an admin context + :param node_obj: a created (but not saved to the database) node object. + :returns: created node object. + :raises: InterfaceNotFoundInEntrypoint if validation fails for any + dynamic interfaces (e.g. network_interface). + :raises: InvalidParameterValue if some fields fail validation. + """ + LOG.debug("RPC create_node called for node %s.", node_obj.uuid) + driver_factory.check_and_update_node_interfaces(node_obj) + node_obj.create() + return node_obj + @METRICS.timer('ConductorManager.update_node') @messaging.expected_exceptions(exception.InvalidParameterValue, exception.NodeLocked, - exception.InvalidState) + exception.InvalidState, + exception.InterfaceNotFoundInEntrypoint) def update_node(self, context, node_obj): """Update a node with the supplied data. @@ -126,15 +145,8 @@ class ConductorManager(base_manager.BaseConductorManager): raise exception.InvalidState( action % {'node': node_obj.uuid, 'allowed': ', '.join(allowed_update_states)}) - net_iface = node_obj.network_interface - if net_iface not in CONF.enabled_network_interfaces: - raise exception.InvalidParameterValue( - _("Cannot change network_interface to invalid value " - "%(n_interface)s for node %(node)s, valid interfaces " - "are: %(valid_choices)s.") % { - 'n_interface': net_iface, 'node': node_obj.uuid, - 'valid_choices': CONF.enabled_network_interfaces, - }) + + driver_factory.check_and_update_node_interfaces(node_obj) driver_name = node_obj.driver if 'driver' in delta else None with task_manager.acquire(context, node_id, shared=False, diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index a61e41fa2d..2c411e934f 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -82,11 +82,12 @@ class ConductorAPI(object): | 1.33 - Added update and destroy portgroup. | 1.34 - Added heartbeat | 1.35 - Added destroy_volume_connector and update_volume_connector + | 1.36 - Added create_node """ # NOTE(rloo): This must be in sync with manager.ConductorManager's. - RPC_API_VERSION = '1.35' + RPC_API_VERSION = '1.36' def __init__(self, topic=None): super(ConductorAPI, self).__init__() @@ -140,6 +141,21 @@ class ConductorAPI(object): host = random.choice(list(hash_ring.hosts)) return self.topic + "." + host + def create_node(self, context, node_obj, topic=None): + """Synchronously, have a conductor validate and create a node. + + Create the node's information in the database and return a node object. + + :param context: request context. + :param node_obj: a created (but not saved) node object. + :param topic: RPC topic. Defaults to self.topic. + :returns: created node object. + :raises: InterfaceNotFoundInEntrypoint if validation fails for any + dynamic interfaces (e.g. network_interface). + """ + cctxt = self.client.prepare(topic=topic or self.topic, version='1.36') + return cctxt.call(context, 'create_node', node_obj=node_obj) + def update_node(self, context, node_obj, topic=None): """Synchronously, have a conductor update the node's information. diff --git a/ironic/conductor/task_manager.py b/ironic/conductor/task_manager.py index e3f9c6d943..cd5b25fd8b 100644 --- a/ironic/conductor/task_manager.py +++ b/ironic/conductor/task_manager.py @@ -187,6 +187,7 @@ class TaskManager(object): from the Node's current driver. :param purpose: human-readable purpose to put to debug logs. :raises: DriverNotFound + :raises: InterfaceNotFoundInEntrypoint :raises: NodeNotFound :raises: NodeLocked diff --git a/ironic/objects/node.py b/ironic/objects/node.py index 46a72c4e33..1fcef73e8f 100644 --- a/ironic/objects/node.py +++ b/ironic/objects/node.py @@ -19,7 +19,6 @@ from oslo_versionedobjects import base as object_base from ironic.common import exception from ironic.common.i18n import _ -from ironic.conf import CONF from ironic.db import api as db_api from ironic.objects import base from ironic.objects import fields as object_fields @@ -28,13 +27,6 @@ from ironic.objects import notification REQUIRED_INT_PROPERTIES = ['local_gb', 'cpus', 'memory_mb'] -def _default_network_interface(): - network_iface = (CONF.default_network_interface or - ('flat' if CONF.dhcp.dhcp_provider == 'neutron' - else 'noop')) - return network_iface - - @base.IronicObjectRegistry.register class Node(base.IronicObject, object_base.VersionedObjectDictCompat): # Version 1.0: Initial version @@ -61,7 +53,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): # Version 1.19: Add fields: boot_interface, console_interface, # deploy_interface, inspect_interface, management_interface, # power_interface, raid_interface, vendor_interface - VERSION = '1.19' + # Version 1.20: Type of network_interface changed to just nullable string + VERSION = '1.20' dbapi = db_api.get_instance() @@ -126,8 +119,7 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): 'deploy_interface': object_fields.StringField(nullable=True), 'inspect_interface': object_fields.StringField(nullable=True), 'management_interface': object_fields.StringField(nullable=True), - 'network_interface': object_fields.StringFieldThatAcceptsCallable( - nullable=False, default=_default_network_interface), + 'network_interface': object_fields.StringField(nullable=True), 'power_interface': object_fields.StringField(nullable=True), 'raid_interface': object_fields.StringField(nullable=True), 'vendor_interface': object_fields.StringField(nullable=True), @@ -445,7 +437,9 @@ class NodePayload(notification.NotificationPayloadBase): 'uuid': ('node', 'uuid') } # Version 1.0: Initial version, based off of Node version 1.18. - VERSION = '1.0' + # Version 1.1: Type of network_interface changed to just nullable string + # similar to version 1.20 of Node. + VERSION = '1.1' fields = { 'clean_step': object_fields.FlexibleDictField(nullable=True), 'console_enabled': object_fields.BooleanField(), @@ -458,7 +452,7 @@ class NodePayload(notification.NotificationPayloadBase): 'last_error': object_fields.StringField(nullable=True), 'maintenance': object_fields.BooleanField(), 'maintenance_reason': object_fields.StringField(nullable=True), - 'network_interface': object_fields.StringFieldThatAcceptsCallable(), + 'network_interface': object_fields.StringField(nullable=True), 'name': object_fields.StringField(nullable=True), 'power_state': object_fields.StringField(nullable=True), 'properties': object_fields.FlexibleDictField(nullable=True), @@ -491,7 +485,8 @@ class NodeSetPowerStateNotification(notification.NotificationBase): class NodeSetPowerStatePayload(NodePayload): """Payload schema for when ironic changes a node's power state.""" # Version 1.0: Initial version - VERSION = '1.0' + # Version 1.1: Parent NodePayload version 1.1 + VERSION = '1.1' fields = { # "to_power" indicates the future target_power_state of the node. A @@ -532,7 +527,8 @@ class NodeCorrectedPowerStatePayload(NodePayload): before the node was updated. """ # Version 1.0: Initial version - VERSION = '1.0' + # Version 1.1: Parent NodePayload version 1.1 + VERSION = '1.1' fields = { 'from_power': object_fields.StringField(nullable=True) @@ -558,7 +554,8 @@ class NodeSetProvisionStateNotification(notification.NotificationBase): class NodeSetProvisionStatePayload(NodePayload): """Payload schema for when ironic changes a node provision state.""" # Version 1.0: Initial version - VERSION = '1.0' + # Version 1.1: Parent NodePayload version 1.1 + VERSION = '1.1' SCHEMA = dict(NodePayload.SCHEMA, **{'instance_info': ('node', 'instance_info')}) diff --git a/ironic/tests/base.py b/ironic/tests/base.py index 565923ac01..50a4a6ee3b 100644 --- a/ironic/tests/base.py +++ b/ironic/tests/base.py @@ -37,6 +37,7 @@ import testtools from ironic.common import config as ironic_config from ironic.common import context as ironic_context +from ironic.common import driver_factory from ironic.common import hash_ring from ironic.conf import CONF from ironic.objects import base as objects_base @@ -110,6 +111,9 @@ class TestCase(testtools.TestCase): self.useFixture(fixtures.EnvironmentVariable('http_proxy')) self.policy = self.useFixture(policy_fixture.PolicyFixture()) + driver_factory.DriverFactory._extension_manager = None + driver_factory.NetworkInterfaceFactory._extension_manager = None + def _set_config(self): self.cfg_fixture = self.useFixture(config_fixture.Config(CONF)) self.config(use_stderr=False, @@ -119,7 +123,9 @@ class TestCase(testtools.TestCase): group='neutron') self.config(provisioning_network_uuid=uuidutils.generate_uuid(), group='neutron') - self.config(enabled_network_interfaces=['flat', 'noop', 'neutron']) + self.config(enabled_drivers=['fake']) + self.config(enabled_network_interfaces=['flat', 'noop', 'neutron'], + default_network_interface=None) self.set_defaults(host='fake-mini', debug=True) self.set_defaults(connection="sqlite://", diff --git a/ironic/tests/unit/api/v1/test_nodes.py b/ironic/tests/unit/api/v1/test_nodes.py index a3af99e7cb..c7a1fe71eb 100644 --- a/ironic/tests/unit/api/v1/test_nodes.py +++ b/ironic/tests/unit/api/v1/test_nodes.py @@ -34,10 +34,10 @@ from ironic.api.controllers.v1 import node as api_node from ironic.api.controllers.v1 import utils as api_utils from ironic.api.controllers.v1 import versions from ironic.common import boot_devices +from ironic.common import driver_factory from ironic.common import exception from ironic.common import states from ironic.conductor import rpcapi -from ironic.conf import CONF from ironic import objects from ironic.tests import base from ironic.tests.unit.api import base as test_api_base @@ -1646,43 +1646,6 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.OK, response.status_code) - def test_update_network_interface_null_sets_default(self): - CONF.set_override('default_network_interface', 'neutron') - node = obj_utils.create_test_node(self.context, - uuid=uuidutils.generate_uuid(), - network_interface='flat') - self.mock_update_node.return_value = node - network_interface = 'neutron' - headers = {api_base.Version.string: str(api_v1.MAX_VER)} - response = self.patch_json('/nodes/%s' % node.uuid, - [{'path': '/network_interface', - 'value': None, - 'op': 'add'}], - headers=headers) - self.assertEqual('application/json', response.content_type) - self.assertEqual(http_client.OK, response.status_code) - # check the node we pass to updated_node - node_arg = self.mock_update_node.call_args[0][1] - self.assertEqual(network_interface, node_arg['network_interface']) - - def test_update_network_interface_remove_sets_default(self): - CONF.set_override('default_network_interface', 'neutron') - node = obj_utils.create_test_node(self.context, - uuid=uuidutils.generate_uuid(), - network_interface='flat') - self.mock_update_node.return_value = node - network_interface = 'neutron' - headers = {api_base.Version.string: str(api_v1.MAX_VER)} - response = self.patch_json('/nodes/%s' % node.uuid, - [{'path': '/network_interface', - 'op': 'remove'}], - headers=headers) - self.assertEqual('application/json', response.content_type) - self.assertEqual(http_client.OK, response.status_code) - # check the node we pass to updated_node - node_arg = self.mock_update_node.call_args[0][1] - self.assertEqual(network_interface, node_arg['network_interface']) - def test_update_network_interface_old_api(self): node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid()) @@ -1757,6 +1720,14 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(http_client.BAD_REQUEST, response.status_code) +def _create_node_locally(node): + driver_factory.check_and_update_node_interfaces(node) + node.create() + return node + + +@mock.patch.object(rpcapi.ConductorAPI, 'create_node', + lambda _api, _ctx, node, _topic: _create_node_locally(node)) class TestPost(test_api_base.BaseApiTest): def setUp(self): @@ -1816,13 +1787,6 @@ class TestPost(test_api_base.BaseApiTest): network_interface='neutron') self.assertEqual('neutron', result['network_interface']) - def test_create_node_default_network_interface(self): - CONF.set_override('default_network_interface', 'neutron') - CONF.set_override('enabled_network_interfaces', 'flat,noop,neutron') - headers = {api_base.Version.string: '1.20'} - result = self._test_create_node(headers=headers) - self.assertEqual('neutron', result['network_interface']) - def test_create_node_name_empty_invalid(self): ndict = test_api_utils.post_get_test_node(name='') response = self.post_json('/nodes', ndict, diff --git a/ironic/tests/unit/common/test_driver_factory.py b/ironic/tests/unit/common/test_driver_factory.py index 64a66a65b8..f2b2e890b3 100644 --- a/ironic/tests/unit/common/test_driver_factory.py +++ b/ironic/tests/unit/common/test_driver_factory.py @@ -30,10 +30,6 @@ class FakeEp(object): class DriverLoadTestCase(base.TestCase): - def setUp(self): - super(DriverLoadTestCase, self).setUp() - driver_factory.DriverFactory._extension_manager = None - def _fake_init_name_err(self, *args, **kwargs): kwargs['on_load_failure_callback'](None, FakeEp, NameError('aaa')) @@ -104,11 +100,6 @@ class WarnUnsupportedDriversTestCase(base.TestCase): class GetDriverTestCase(base.TestCase): - def setUp(self): - super(GetDriverTestCase, self).setUp() - driver_factory.DriverFactory._extension_manager = None - self.config(enabled_drivers=['fake']) - def test_get_driver_known(self): driver = driver_factory.get_driver('fake') self.assertIsInstance(driver, drivers_base.BaseDriver) @@ -119,12 +110,6 @@ class GetDriverTestCase(base.TestCase): class NetworkInterfaceFactoryTestCase(db_base.DbTestCase): - def setUp(self): - super(NetworkInterfaceFactoryTestCase, self).setUp() - driver_factory.DriverFactory._extension_manager = None - driver_factory.NetworkInterfaceFactory._extension_manager = None - self.config(enabled_drivers=['fake']) - @mock.patch.object(driver_factory, '_warn_if_unsupported') def test_build_driver_for_task(self, mock_warn): # flat, neutron, and noop network interfaces are enabled in base test @@ -188,7 +173,7 @@ class NetworkInterfaceFactoryTestCase(db_base.DbTestCase): def test_build_driver_for_task_unknown_network_interface(self): node = obj_utils.create_test_node(self.context, driver='fake', network_interface='meow') - self.assertRaises(exception.DriverNotFoundInEntrypoint, + self.assertRaises(exception.InterfaceNotFoundInEntrypoint, task_manager.acquire, self.context, node.id) @@ -201,3 +186,41 @@ class NewFactoryTestCase(db_base.DbTestCase): factory = NewDriverFactory() self.assertEqual('woof', factory._entrypoint_name) self.assertEqual([], factory._enabled_driver_list) + + +class CheckAndUpdateNodeInterfacesTestCase(db_base.DbTestCase): + def test_no_network_interface(self): + node = obj_utils.get_test_node(self.context, driver='fake') + self.assertTrue(driver_factory.check_and_update_node_interfaces(node)) + self.assertEqual('flat', node.network_interface) + + def test_none_network_interface(self): + node = obj_utils.get_test_node(self.context, driver='fake', + network_interface=None) + self.assertTrue(driver_factory.check_and_update_node_interfaces(node)) + self.assertEqual('flat', node.network_interface) + + def test_no_network_interface_default_from_conf(self): + self.config(default_network_interface='noop') + node = obj_utils.get_test_node(self.context, driver='fake') + self.assertTrue(driver_factory.check_and_update_node_interfaces(node)) + self.assertEqual('noop', node.network_interface) + + def test_no_network_interface_default_from_dhcp(self): + self.config(dhcp_provider='none', group='dhcp') + node = obj_utils.get_test_node(self.context, driver='fake') + self.assertTrue(driver_factory.check_and_update_node_interfaces(node)) + # "none" dhcp provider corresponds to "noop" network_interface + self.assertEqual('noop', node.network_interface) + + def test_valid_network_interface(self): + node = obj_utils.get_test_node(self.context, driver='fake', + network_interface='noop') + self.assertFalse(driver_factory.check_and_update_node_interfaces(node)) + + def test_invalid_network_interface(self): + node = obj_utils.get_test_node(self.context, driver='fake', + network_interface='banana') + self.assertRaises(exception.InterfaceNotFoundInEntrypoint, + driver_factory.check_and_update_node_interfaces, + node) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 47f08975c8..8efd9d521f 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -380,6 +380,38 @@ class ChangeNodePowerStateTestCase(mgr_utils.ServiceSetUpMixin, obj_fields.NotificationLevel.INFO) +@mgr_utils.mock_record_keepalive +class CreateNodeTestCase(mgr_utils.ServiceSetUpMixin, + tests_db_base.DbTestCase): + def test_create_node(self): + node = obj_utils.get_test_node(self.context, driver='fake', + extra={'test': 'one'}) + + res = self.service.create_node(self.context, node) + + self.assertEqual({'test': 'one'}, res['extra']) + res = objects.Node.get_by_uuid(self.context, node['uuid']) + self.assertEqual({'test': 'one'}, res['extra']) + + @mock.patch.object(driver_factory, 'check_and_update_node_interfaces', + autospec=True) + def test_create_node_validation_fails(self, mock_validate): + node = obj_utils.get_test_node(self.context, driver='fake', + extra={'test': 'one'}) + mock_validate.side_effect = exception.InterfaceNotFoundInEntrypoint( + 'boom') + + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.create_node, + self.context, node) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.InterfaceNotFoundInEntrypoint, + exc.exc_info[0]) + + self.assertRaises(exception.NotFound, + objects.Node.get_by_uuid, self.context, node['uuid']) + + @mgr_utils.mock_record_keepalive class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, tests_db_base.DbTestCase): @@ -503,7 +535,8 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, exc = self.assertRaises(messaging.rpc.ExpectedException, self.service.update_node, self.context, node) - self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) + self.assertEqual(exception.InterfaceNotFoundInEntrypoint, + exc.exc_info[0]) node.refresh() self.assertEqual(old_iface, node.network_interface) diff --git a/ironic/tests/unit/drivers/modules/network/test_flat.py b/ironic/tests/unit/drivers/modules/network/test_flat.py index bf4b36b353..19d3cb053a 100644 --- a/ironic/tests/unit/drivers/modules/network/test_flat.py +++ b/ironic/tests/unit/drivers/modules/network/test_flat.py @@ -62,8 +62,10 @@ class TestFlatInterface(db_base.DbTestCase): @mock.patch.object(neutron, 'rollback_ports') def test_add_cleaning_network_no_cleaning_net_uuid(self, rollback_mock, add_mock): - self.config(cleaning_network_uuid='abc', group='neutron') with task_manager.acquire(self.context, self.node.id) as task: + # This has to go after acquire, or acquire will raise + # DriverLoadError. + self.config(cleaning_network_uuid='abc', group='neutron') self.assertRaises(exception.InvalidParameterValue, self.interface.add_cleaning_network, task) self.assertFalse(rollback_mock.called) diff --git a/ironic/tests/unit/objects/test_node.py b/ironic/tests/unit/objects/test_node.py index 5fcaeb9194..467dcea8a4 100644 --- a/ironic/tests/unit/objects/test_node.py +++ b/ironic/tests/unit/objects/test_node.py @@ -18,7 +18,6 @@ from testtools import matchers from ironic.common import context from ironic.common import exception -from ironic.conf import CONF from ironic import objects from ironic.tests.unit.db import base from ironic.tests.unit.db import utils @@ -196,22 +195,3 @@ class TestNodeObject(base.DbTestCase): } node._validate_property_values(values['properties']) self.assertEqual(expect, values['properties']) - - def test_get_network_interface_use_field(self): - CONF.set_override('default_network_interface', None) - for nif in ('neutron', 'flat', 'noop'): - self.node.network_interface = nif - self.assertEqual(nif, self.node.network_interface) - - def test_get_network_interface_use_conf(self): - for nif in ('neutron', 'flat', 'noop'): - CONF.set_override('default_network_interface', nif) - self.node = obj_utils.get_test_node(self.ctxt, **self.fake_node) - self.assertEqual(nif, self.node.network_interface) - - def test_get_network_interface_use_dhcp_provider(self): - CONF.set_override('default_network_interface', None) - for dhcp, nif in (('neutron', 'flat'), ('none', 'noop')): - CONF.set_override('dhcp_provider', dhcp, 'dhcp') - self.node = obj_utils.get_test_node(self.ctxt, **self.fake_node) - self.assertEqual(nif, self.node.network_interface) diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index 43e2b2f447..2d24fb6319 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -404,7 +404,7 @@ class TestObject(_LocalTest, _TestObject): # version bump. It is md5 hash of object fields and remotable methods. # The fingerprint values should only be changed if there is a version bump. expected_object_fingerprints = { - 'Node': '1.19-e8b294016d8d5b322df813f790d092b4', + 'Node': '1.20-b6a13eb50f9d64fa6c9d614c61dbec31', 'MyObj': '1.5-4f5efe8f0fcaf182bbe1c7fe3ba858db', 'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905', 'Port': '1.6-609504503d68982a10f495659990084b', @@ -412,15 +412,15 @@ expected_object_fingerprints = { 'Conductor': '1.1-5091f249719d4a465062a1b3dc7f860d', 'EventType': '1.1-aa2ba1afd38553e3880c267404e8d370', 'NotificationPublisher': '1.0-51a09397d6c0687771fb5be9a999605d', - 'NodePayload': '1.0-ccb491ab5cd247e2ba3f21af4c12eb7c', + 'NodePayload': '1.1-d895cf6411ac666f9e982f85ea0a9499', 'NodeSetPowerStateNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeSetPowerStatePayload': '1.0-80986cc6a099cccd481fe3e288157a07', + 'NodeSetPowerStatePayload': '1.1-b8fab1bea5a2da5900445ab515e41715', 'NodeCorrectedPowerStateNotification': '1.0-59acc533c11d306f149846f922739' 'c15', - 'NodeCorrectedPowerStatePayload': '1.0-2a484d7c342caa9fe488de16dc5f1f1e', + 'NodeCorrectedPowerStatePayload': '1.1-5d1544defc858ae8a722f4cadd511bac', 'NodeSetProvisionStateNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeSetProvisionStatePayload': '1.0-91be7439b9b6b04931c9b99b8e1ea87a', + 'NodeSetProvisionStatePayload': '1.1-743be1f5748f346e3da33390983172b1', 'VolumeConnector': '1.0-3e0252c0ab6e6b9d158d09238a577d97' } diff --git a/releasenotes/notes/create-on-conductor-c1c52a1f022c4048.yaml b/releasenotes/notes/create-on-conductor-c1c52a1f022c4048.yaml new file mode 100644 index 0000000000..82d02642a1 --- /dev/null +++ b/releasenotes/notes/create-on-conductor-c1c52a1f022c4048.yaml @@ -0,0 +1,11 @@ +--- +upgrade: + - In this release node creation logic was moved from the API service to + the conductor service. This is more consistent with other node operations + and opens opportunities for conductor-side validations on nodes. + However, with this change, node creation may take longer, and this may + limit the number of nodes that can be enrolled in parallel. + - The "[DEFAULT]default_network_interface" and "[dhcp]dhcp_provider" + configuration options were previously required for the ironic-api service + to calculate the correct "network_interface" default. Now these options + are only read by the ironic-conductor service.