diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index 4999878ba1..1c1be3b68d 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -539,15 +539,25 @@ class PortsController(rest.RestController): if not pdict.get('uuid'): pdict['uuid'] = uuidutils.generate_uuid() - new_port = objects.Port(context, **pdict) + rpc_port = objects.Port(context, **pdict) + rpc_node = objects.Node.get_by_id(context, rpc_port.node_id) notify_extra = {'node_uuid': port.node_uuid, 'portgroup_uuid': port.portgroup_uuid} - notify.emit_start_notification(context, new_port, 'create', + notify.emit_start_notification(context, rpc_port, 'create', **notify_extra) - with notify.handle_error_notification(context, new_port, 'create', + with notify.handle_error_notification(context, rpc_port, 'create', **notify_extra): - new_port.create() + # TODO(mgoddard): In RPC API v1.41, port creation was moved to the + # conductor service to facilitate validation of the physical + # network field of ports in portgroups. Further consideration is + # required determine how best to support rolling upgrades from a + # release in which ports are created by the API service to one in + # which they are created by the conductor service, while ensuring + # that all required validation is performed. + topic = pecan.request.rpcapi.get_topic_for(rpc_node) + new_port = pecan.request.rpcapi.create_port(context, rpc_port, + topic) notify.emit_end_notification(context, new_port, 'create', **notify_extra) # Set the HTTP Location Header diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 3b143b445c..c81e20f8d4 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -70,7 +70,7 @@ RELEASE_MAPPING = { } }, 'master': { - 'rpc': '1.40', + 'rpc': '1.41', 'objects': { 'Node': '1.21', 'Conductor': '1.2', diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index d9a4699971..3b4c6014d2 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -89,7 +89,7 @@ class ConductorManager(base_manager.BaseConductorManager): # NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's. # NOTE(pas-ha): This also must be in sync with # ironic.common.release_mappings.RELEASE_MAPPING['master'] - RPC_API_VERSION = '1.40' + RPC_API_VERSION = '1.41' target = messaging.Target(version=RPC_API_VERSION) @@ -1836,6 +1836,26 @@ class ConductorManager(base_manager.BaseConductorManager): notify_utils.emit_console_notification( task, 'console_set', fields.NotificationStatus.END) + @METRICS.timer('ConductorManager.create_port') + @messaging.expected_exceptions(exception.NodeLocked, + exception.MACAlreadyExists) + def create_port(self, context, port_obj): + """Create a port. + + :param context: request context. + :param port_obj: a changed (but not saved) port object. + :raises: NodeLocked if node is locked by another conductor + :raises: MACAlreadyExists if the port has a MAC which is registered on + another port already. + """ + port_uuid = port_obj.uuid + LOG.debug("RPC create_port called for port %s.", port_uuid) + + with task_manager.acquire(context, port_obj.node_id, + purpose='port create'): + port_obj.create() + return port_obj + @METRICS.timer('ConductorManager.update_port') @messaging.expected_exceptions(exception.NodeLocked, exception.FailedToUpdateMacOnPort, diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index 01e7687a9f..eed896e047 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -89,13 +89,14 @@ class ConductorAPI(object): | 1.38 - Added vif_attach, vif_detach, vif_list | 1.39 - Added timeout optional parameter to change_node_power_state | 1.40 - Added inject_nmi + | 1.41 - Added create_port """ # NOTE(rloo): This must be in sync with manager.ConductorManager's. # NOTE(pas-ha): This also must be in sync with # ironic.common.release_mappings.RELEASE_MAPPING['master'] - RPC_API_VERSION = '1.40' + RPC_API_VERSION = '1.41' def __init__(self, topic=None): super(ConductorAPI, self).__init__() @@ -469,6 +470,21 @@ class ConductorAPI(object): return cctxt.call(context, 'set_console_mode', node_id=node_id, enabled=enabled) + def create_port(self, context, port_obj, topic=None): + """Synchronously, have a conductor validate and create a port. + + Create the port's information in the database and return a port object. + The conductor will lock related node and trigger specific driver + actions if they are needed. + + :param context: request context. + :param port_obj: a created (but not saved) port object. + :param topic: RPC topic. Defaults to self.topic. + :returns: created port object. + """ + cctxt = self.client.prepare(topic=topic or self.topic, version='1.41') + return cctxt.call(context, 'create_port', port_obj=port_obj) + def update_port(self, context, port_obj, topic=None): """Synchronously, have a conductor update the port's information. diff --git a/ironic/tests/unit/api/v1/test_ports.py b/ironic/tests/unit/api/v1/test_ports.py index 31d1970bed..5b4f3ce617 100644 --- a/ironic/tests/unit/api/v1/test_ports.py +++ b/ironic/tests/unit/api/v1/test_ports.py @@ -34,7 +34,6 @@ from ironic.api.controllers.v1 import versions from ironic.common import exception from ironic.common import utils as common_utils from ironic.conductor import rpcapi -from ironic import objects from ironic.objects import fields as obj_fields from ironic.tests import base from ironic.tests.unit.api import base as test_api_base @@ -54,6 +53,16 @@ def post_get_test_port(**kw): return port +def _rpcapi_create_port(self, context, port, topic): + """Fake used to mock out the conductor RPCAPI's create_port method. + + Performs creation of the port object and returns the created port as-per + the real method. + """ + port.create() + return port + + class TestPortObject(base.TestCase): @mock.patch("pecan.request") @@ -1055,6 +1064,8 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) +@mock.patch.object(rpcapi.ConductorAPI, 'create_port', autospec=True, + side_effect=_rpcapi_create_port) class TestPost(test_api_base.BaseApiTest): def setUp(self): @@ -1065,11 +1076,17 @@ class TestPost(test_api_base.BaseApiTest): self.headers = {api_base.Version.string: str( versions.MAX_VERSION_STRING)} + p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for') + self.mock_gtf = p.start() + self.mock_gtf.return_value = 'test-topic' + self.addCleanup(p.stop) + @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', autospec=True) @mock.patch.object(notification_utils, '_emit_api_notification') @mock.patch.object(timeutils, 'utcnow') - def test_create_port(self, mock_utcnow, mock_notify, mock_warn): + def test_create_port(self, mock_utcnow, mock_notify, mock_warn, + mock_create): pdict = post_get_test_port() test_time = datetime.datetime(2000, 1, 1, 0, 0) mock_utcnow.return_value = test_time @@ -1087,6 +1104,8 @@ class TestPost(test_api_base.BaseApiTest): expected_location = '/v1/ports/%s' % pdict['uuid'] self.assertEqual(urlparse.urlparse(response.location).path, expected_location) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'create', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.START, @@ -1099,7 +1118,7 @@ class TestPost(test_api_base.BaseApiTest): portgroup_uuid=self.portgroup.uuid)]) self.assertEqual(0, mock_warn.call_count) - def test_create_port_min_api_version(self): + def test_create_port_min_api_version(self, mock_create): pdict = post_get_test_port( node_uuid=self.node.uuid) pdict.pop('local_link_connection') @@ -1110,8 +1129,10 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.CREATED, response.status_int) self.assertEqual(self.node.uuid, response.json['node_uuid']) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') - def test_create_port_doesnt_contain_id(self): + def test_create_port_doesnt_contain_id(self, mock_create): with mock.patch.object(self.dbapi, 'create_port', wraps=self.dbapi.create_port) as cp_mock: pdict = post_get_test_port(extra={'foo': 123}) @@ -1122,10 +1143,13 @@ class TestPost(test_api_base.BaseApiTest): cp_mock.assert_called_once_with(mock.ANY) # Check that 'id' is not in first arg of positional args self.assertNotIn('id', cp_mock.call_args[0][0]) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') @mock.patch.object(notification_utils.LOG, 'exception', autospec=True) @mock.patch.object(notification_utils.LOG, 'warning', autospec=True) - def test_create_port_generate_uuid(self, mock_warning, mock_exception): + def test_create_port_generate_uuid(self, mock_warning, mock_exception, + mock_create): pdict = post_get_test_port() del pdict['uuid'] response = self.post_json('/ports', pdict, headers=self.headers) @@ -1135,10 +1159,11 @@ class TestPost(test_api_base.BaseApiTest): self.assertTrue(uuidutils.is_uuid_like(result['uuid'])) self.assertFalse(mock_warning.called) self.assertFalse(mock_exception.called) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') @mock.patch.object(notification_utils, '_emit_api_notification') - @mock.patch.object(objects.Port, 'create') - def test_create_port_error(self, mock_create, mock_notify): + def test_create_port_error(self, mock_notify, mock_create): mock_create.side_effect = Exception() pdict = post_get_test_port() self.post_json('/ports', pdict, headers=self.headers, @@ -1153,8 +1178,10 @@ class TestPost(test_api_base.BaseApiTest): obj_fields.NotificationStatus.ERROR, node_uuid=self.node.uuid, portgroup_uuid=self.portgroup.uuid)]) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') - def test_create_port_valid_extra(self): + def test_create_port_valid_extra(self, mock_create): pdict = post_get_test_port(extra={'str': 'foo', 'int': 123, 'float': 0.1, 'bool': True, 'list': [1, 2], 'none': None, @@ -1163,8 +1190,10 @@ class TestPost(test_api_base.BaseApiTest): result = self.get_json('/ports/%s' % pdict['uuid'], headers=self.headers) self.assertEqual(pdict['extra'], result['extra']) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') - def test_create_port_no_mandatory_field_address(self): + def test_create_port_no_mandatory_field_address(self, mock_create): pdict = post_get_test_port() del pdict['address'] response = self.post_json('/ports', pdict, expect_errors=True, @@ -1172,31 +1201,36 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertEqual('application/json', response.content_type) self.assertTrue(response.json['error_message']) + self.assertFalse(mock_create.called) - def test_create_port_no_mandatory_field_node_uuid(self): + def test_create_port_no_mandatory_field_node_uuid(self, mock_create): pdict = post_get_test_port() del pdict['node_uuid'] response = self.post_json('/ports', pdict, expect_errors=True) self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertEqual('application/json', response.content_type) self.assertTrue(response.json['error_message']) + self.assertFalse(mock_create.called) - def test_create_port_invalid_addr_format(self): + def test_create_port_invalid_addr_format(self, mock_create): pdict = post_get_test_port(address='invalid-format') response = self.post_json('/ports', pdict, expect_errors=True) self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertEqual('application/json', response.content_type) self.assertTrue(response.json['error_message']) + self.assertFalse(mock_create.called) - def test_create_port_address_normalized(self): + def test_create_port_address_normalized(self, mock_create): address = 'AA:BB:CC:DD:EE:FF' pdict = post_get_test_port(address=address) self.post_json('/ports', pdict, headers=self.headers) result = self.get_json('/ports/%s' % pdict['uuid'], headers=self.headers) self.assertEqual(address.lower(), result['address']) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') - def test_create_port_with_hyphens_delimiter(self): + def test_create_port_with_hyphens_delimiter(self, mock_create): pdict = post_get_test_port() colonsMAC = pdict['address'] hyphensMAC = colonsMAC.replace(':', '-') @@ -1205,30 +1239,35 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertEqual('application/json', response.content_type) self.assertTrue(response.json['error_message']) + self.assertFalse(mock_create.called) - def test_create_port_invalid_node_uuid_format(self): + def test_create_port_invalid_node_uuid_format(self, mock_create): pdict = post_get_test_port(node_uuid='invalid-format') response = self.post_json('/ports', pdict, expect_errors=True) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertTrue(response.json['error_message']) + self.assertFalse(mock_create.called) - def test_node_uuid_to_node_id_mapping(self): + def test_node_uuid_to_node_id_mapping(self, mock_create): pdict = post_get_test_port(node_uuid=self.node['uuid']) self.post_json('/ports', pdict, headers=self.headers) # GET doesn't return the node_id it's an internal value port = self.dbapi.get_port_by_uuid(pdict['uuid']) self.assertEqual(self.node['id'], port.node_id) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') - def test_create_port_node_uuid_not_found(self): + def test_create_port_node_uuid_not_found(self, mock_create): pdict = post_get_test_port( node_uuid='1a1a1a1a-2b2b-3c3c-4d4d-5e5e5e5e5e5e') response = self.post_json('/ports', pdict, expect_errors=True) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertTrue(response.json['error_message']) + self.assertFalse(mock_create.called) - def test_create_port_portgroup_uuid_not_found(self): + def test_create_port_portgroup_uuid_not_found(self, mock_create): pdict = post_get_test_port( portgroup_uuid='1a1a1a1a-2b2b-3c3c-4d4d-5e5e5e5e5e5e') response = self.post_json('/ports', pdict, expect_errors=True, @@ -1236,16 +1275,19 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertTrue(response.json['error_message']) + self.assertFalse(mock_create.called) - def test_create_port_portgroup_uuid_not_found_old_api_version(self): + def test_create_port_portgroup_uuid_not_found_old_api_version(self, + mock_create): pdict = post_get_test_port( portgroup_uuid='1a1a1a1a-2b2b-3c3c-4d4d-5e5e5e5e5e5e') response = self.post_json('/ports', pdict, expect_errors=True) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) self.assertTrue(response.json['error_message']) + self.assertFalse(mock_create.called) - def test_create_port_portgroup(self): + def test_create_port_portgroup(self, mock_create): pdict = post_get_test_port( portgroup_uuid=self.portgroup.uuid, node_uuid=self.node.uuid) @@ -1253,8 +1295,10 @@ class TestPost(test_api_base.BaseApiTest): response = self.post_json('/ports', pdict, headers=self.headers) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.CREATED, response.status_int) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') - def test_create_port_portgroup_different_nodes(self): + def test_create_port_portgroup_different_nodes(self, mock_create): pdict = post_get_test_port( portgroup_uuid=self.portgroup.uuid, node_uuid=uuidutils.generate_uuid()) @@ -1263,8 +1307,9 @@ class TestPost(test_api_base.BaseApiTest): expect_errors=True) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.BAD_REQUEST, response.status_int) + self.assertFalse(mock_create.called) - def test_create_port_portgroup_old_api_version(self): + def test_create_port_portgroup_old_api_version(self, mock_create): pdict = post_get_test_port( portgroup_uuid=self.portgroup.uuid, node_uuid=self.node.uuid @@ -1274,12 +1319,13 @@ class TestPost(test_api_base.BaseApiTest): headers=headers) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + self.assertFalse(mock_create.called) - def test_create_port_address_already_exist(self): + @mock.patch.object(notification_utils, '_emit_api_notification') + def test_create_port_address_already_exist(self, mock_notify, mock_create): address = 'AA:AA:AA:11:22:33' - pdict = post_get_test_port(address=address) - self.post_json('/ports', pdict, headers=self.headers) - pdict['uuid'] = uuidutils.generate_uuid() + mock_create.side_effect = exception.MACAlreadyExists(mac=address) + pdict = post_get_test_port(address=address, node_id=self.node.id) response = self.post_json('/ports', pdict, expect_errors=True, headers=self.headers) self.assertEqual(http_client.CONFLICT, response.status_int) @@ -1287,16 +1333,30 @@ class TestPost(test_api_base.BaseApiTest): error_msg = response.json['error_message'] self.assertTrue(error_msg) self.assertIn(address, error_msg.upper()) + self.assertTrue(mock_create.called) - def test_create_port_with_internal_field(self): + mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'create', + obj_fields.NotificationLevel.INFO, + obj_fields.NotificationStatus.START, + node_uuid=self.node.uuid, + portgroup_uuid=pdict['portgroup_uuid']), + mock.call(mock.ANY, mock.ANY, 'create', + obj_fields.NotificationLevel.ERROR, + obj_fields.NotificationStatus.ERROR, + node_uuid=self.node.uuid, + portgroup_uuid=pdict['portgroup_uuid'])]) + + def test_create_port_with_internal_field(self, mock_create): pdict = post_get_test_port() pdict['internal_info'] = {'a': 'b'} response = self.post_json('/ports', pdict, expect_errors=True) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertTrue(response.json['error_message']) + self.assertFalse(mock_create.called) - def test_create_port_some_invalid_local_link_connection_key(self): + def test_create_port_some_invalid_local_link_connection_key(self, + mock_create): pdict = post_get_test_port( local_link_connection={'switch_id': 'value1', 'port_id': 'Ethernet1/15', @@ -1306,8 +1366,9 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertTrue(response.json['error_message']) + self.assertFalse(mock_create.called) - def test_create_port_local_link_connection_keys(self): + def test_create_port_local_link_connection_keys(self, mock_create): pdict = post_get_test_port( local_link_connection={'switch_id': '0a:1b:2c:3d:4e:5f', 'port_id': 'Ethernet1/15', @@ -1315,8 +1376,11 @@ class TestPost(test_api_base.BaseApiTest): response = self.post_json('/ports', pdict, headers=self.headers) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.CREATED, response.status_int) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') - def test_create_port_local_link_connection_switch_id_bad_mac(self): + def test_create_port_local_link_connection_switch_id_bad_mac(self, + mock_create): pdict = post_get_test_port( local_link_connection={'switch_id': 'zz:zz:zz:zz:zz:zz', 'port_id': 'Ethernet1/15', @@ -1326,8 +1390,10 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertTrue(response.json['error_message']) + self.assertFalse(mock_create.called) - def test_create_port_local_link_connection_missing_mandatory(self): + def test_create_port_local_link_connection_missing_mandatory(self, + mock_create): pdict = post_get_test_port( local_link_connection={'switch_id': '0a:1b:2c:3d:4e:5f', 'switch_info': 'fooswitch'}) @@ -1335,16 +1401,20 @@ class TestPost(test_api_base.BaseApiTest): headers=self.headers) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.BAD_REQUEST, response.status_int) + self.assertFalse(mock_create.called) - def test_create_port_local_link_connection_missing_optional(self): + def test_create_port_local_link_connection_missing_optional(self, + mock_create): pdict = post_get_test_port( local_link_connection={'switch_id': '0a:1b:2c:3d:4e:5f', 'port_id': 'Ethernet1/15'}) response = self.post_json('/ports', pdict, headers=self.headers) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.CREATED, response.status_int) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') - def test_create_port_with_llc_old_api_version(self): + def test_create_port_with_llc_old_api_version(self, mock_create): headers = {api_base.Version.string: '1.14'} pdict = post_get_test_port( local_link_connection={'switch_id': '0a:1b:2c:3d:4e:5f', @@ -1353,8 +1423,9 @@ class TestPost(test_api_base.BaseApiTest): expect_errors=True) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + self.assertFalse(mock_create.called) - def test_create_port_with_pxe_enabled_old_api_version(self): + def test_create_port_with_pxe_enabled_old_api_version(self, mock_create): headers = {api_base.Version.string: '1.14'} pdict = post_get_test_port(pxe_enabled=False) del pdict['local_link_connection'] @@ -1363,26 +1434,31 @@ class TestPost(test_api_base.BaseApiTest): expect_errors=True) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + self.assertFalse(mock_create.called) - def test_portgroups_subresource_post(self): + def test_portgroups_subresource_post(self, mock_create): headers = {api_base.Version.string: '1.24'} pdict = post_get_test_port() response = self.post_json('/portgroups/%s/ports' % self.portgroup.uuid, pdict, headers=headers, expect_errors=True) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.FORBIDDEN, response.status_int) + self.assertFalse(mock_create.called) @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', autospec=True) - def test_create_port_with_extra_vif_port_id_deprecated(self, mock_warn): + def test_create_port_with_extra_vif_port_id_deprecated(self, mock_warn, + mock_create): pdict = post_get_test_port(pxe_enabled=False, extra={'vif_port_id': 'foo'}) response = self.post_json('/ports', pdict, headers=self.headers) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.CREATED, response.status_int) self.assertEqual(1, mock_warn.call_count) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') - def _test_create_port(self, has_vif=False, in_portgroup=False, + def _test_create_port(self, mock_create, has_vif=False, in_portgroup=False, pxe_enabled=True, standalone_ports=True, http_status=http_client.CREATED): extra = {} @@ -1410,71 +1486,82 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual(expected_portgroup_uuid, response.json['portgroup_uuid']) self.assertEqual(extra, response.json['extra']) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') + else: + self.assertFalse(mock_create.called) - def test_create_port_novif_pxe_noportgroup(self): - self._test_create_port(has_vif=False, in_portgroup=False, + def test_create_port_novif_pxe_noportgroup(self, mock_create): + self._test_create_port(mock_create, has_vif=False, in_portgroup=False, pxe_enabled=True, http_status=http_client.CREATED) - def test_create_port_novif_nopxe_noportgroup(self): - self._test_create_port(has_vif=False, in_portgroup=False, + def test_create_port_novif_nopxe_noportgroup(self, mock_create): + self._test_create_port(mock_create, has_vif=False, in_portgroup=False, pxe_enabled=False, http_status=http_client.CREATED) - def test_create_port_vif_pxe_noportgroup(self): - self._test_create_port(has_vif=True, in_portgroup=False, + def test_create_port_vif_pxe_noportgroup(self, mock_create): + self._test_create_port(mock_create, has_vif=True, in_portgroup=False, pxe_enabled=True, http_status=http_client.CREATED) - def test_create_port_vif_nopxe_noportgroup(self): - self._test_create_port(has_vif=True, in_portgroup=False, + def test_create_port_vif_nopxe_noportgroup(self, mock_create): + self._test_create_port(mock_create, has_vif=True, in_portgroup=False, pxe_enabled=False, http_status=http_client.CREATED) - def test_create_port_novif_pxe_portgroup_standalone_ports(self): - self._test_create_port(has_vif=False, in_portgroup=True, + def test_create_port_novif_pxe_portgroup_standalone_ports(self, + mock_create): + self._test_create_port(mock_create, has_vif=False, in_portgroup=True, pxe_enabled=True, standalone_ports=True, http_status=http_client.CREATED) - def test_create_port_novif_pxe_portgroup_nostandalone_ports(self): - self._test_create_port(has_vif=False, in_portgroup=True, + def test_create_port_novif_pxe_portgroup_nostandalone_ports(self, + mock_create): + self._test_create_port(mock_create, has_vif=False, in_portgroup=True, pxe_enabled=True, standalone_ports=False, http_status=http_client.CONFLICT) - def test_create_port_novif_nopxe_portgroup_standalone_ports(self): - self._test_create_port(has_vif=False, in_portgroup=True, + def test_create_port_novif_nopxe_portgroup_standalone_ports(self, + mock_create): + self._test_create_port(mock_create, has_vif=False, in_portgroup=True, pxe_enabled=False, standalone_ports=True, http_status=http_client.CREATED) - def test_create_port_novif_nopxe_portgroup_nostandalone_ports(self): - self._test_create_port(has_vif=False, in_portgroup=True, + def test_create_port_novif_nopxe_portgroup_nostandalone_ports(self, + mock_create): + self._test_create_port(mock_create, has_vif=False, in_portgroup=True, pxe_enabled=False, standalone_ports=False, http_status=http_client.CREATED) - def test_create_port_vif_pxe_portgroup_standalone_ports(self): - self._test_create_port(has_vif=True, in_portgroup=True, + def test_create_port_vif_pxe_portgroup_standalone_ports(self, mock_create): + self._test_create_port(mock_create, has_vif=True, in_portgroup=True, pxe_enabled=True, standalone_ports=True, http_status=http_client.CREATED) - def test_create_port_vif_pxe_portgroup_nostandalone_ports(self): - self._test_create_port(has_vif=True, in_portgroup=True, + def test_create_port_vif_pxe_portgroup_nostandalone_ports(self, + mock_create): + self._test_create_port(mock_create, has_vif=True, in_portgroup=True, pxe_enabled=True, standalone_ports=False, http_status=http_client.CONFLICT) - def test_create_port_vif_nopxe_portgroup_standalone_ports(self): - self._test_create_port(has_vif=True, in_portgroup=True, + def test_create_port_vif_nopxe_portgroup_standalone_ports(self, + mock_create): + self._test_create_port(mock_create, has_vif=True, in_portgroup=True, pxe_enabled=False, standalone_ports=True, http_status=http_client.CREATED) - def test_create_port_vif_nopxe_portgroup_nostandalone_ports(self): - self._test_create_port(has_vif=True, in_portgroup=True, + def test_create_port_vif_nopxe_portgroup_nostandalone_ports(self, + mock_create): + self._test_create_port(mock_create, has_vif=True, in_portgroup=True, pxe_enabled=False, standalone_ports=False, http_status=http_client.CONFLICT) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 78d11f07da..ef6a18be3c 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -3156,6 +3156,45 @@ class DestroyNodeTestCase(mgr_utils.ServiceSetUpMixin, self.assertFalse(mock_power.called) +@mgr_utils.mock_record_keepalive +class CreatePortTestCase(mgr_utils.ServiceSetUpMixin, + tests_db_base.DbTestCase): + + def test_create_port(self): + node = obj_utils.create_test_node(self.context, driver='fake') + port = obj_utils.get_test_port(self.context, node_id=node.id, + extra={'foo': 'bar'}) + res = self.service.create_port(self.context, port) + self.assertEqual({'foo': 'bar'}, res.extra) + res = objects.Port.get_by_uuid(self.context, port['uuid']) + self.assertEqual({'foo': 'bar'}, res.extra) + + def test_create_port_node_locked(self): + node = obj_utils.create_test_node(self.context, driver='fake', + reservation='fake-reserv') + port = obj_utils.get_test_port(self.context, node_id=node.id) + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.create_port, + self.context, port) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.NodeLocked, exc.exc_info[0]) + self.assertRaises(exception.PortNotFound, port.get_by_uuid, + self.context, port.uuid) + + def test_create_port_mac_exists(self): + node = obj_utils.create_test_node(self.context, driver='fake') + port = obj_utils.create_test_port(self.context, node_id=node.id) + port = obj_utils.get_test_port(self.context, node_id=node.id, + uuid=uuidutils.generate_uuid()) + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.create_port, + self.context, port) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.MACAlreadyExists, exc.exc_info[0]) + self.assertRaises(exception.PortNotFound, port.get_by_uuid, + self.context, port.uuid) + + @mgr_utils.mock_record_keepalive class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin, tests_db_base.DbTestCase): diff --git a/ironic/tests/unit/conductor/test_rpcapi.py b/ironic/tests/unit/conductor/test_rpcapi.py index c108c084a6..95c0f61f6b 100644 --- a/ironic/tests/unit/conductor/test_rpcapi.py +++ b/ironic/tests/unit/conductor/test_rpcapi.py @@ -265,6 +265,13 @@ class RPCAPITestCase(base.DbTestCase): node_id=self.fake_node['uuid'], enabled=True) + def test_create_port(self): + fake_port = dbutils.get_test_port() + self._test_rpcapi('create_port', + 'call', + version='1.41', + port_obj=fake_port) + def test_update_port(self): fake_port = dbutils.get_test_port() self._test_rpcapi('update_port', diff --git a/releasenotes/notes/create-port-on-conductor-b921738b4b2a5def.yaml b/releasenotes/notes/create-port-on-conductor-b921738b4b2a5def.yaml new file mode 100644 index 0000000000..18f11dbb3d --- /dev/null +++ b/releasenotes/notes/create-port-on-conductor-b921738b4b2a5def.yaml @@ -0,0 +1,7 @@ +--- +upgrade: + - Moves port creation logic from the API service to the conductor service. + This is more consistent with port update operations and opens opportunities + for conductor-side validations on ports. However, with this change, port + creation may take longer, and this may limit the number of ports that can + be created in parallel.