diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index adc21ebc2e..d70c13f27e 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -123,9 +123,9 @@ def convert_with_links(rpc_port, fields=None, sanitize=True): 'local_link_connection', 'physical_network', 'pxe_enabled', + 'node_uuid', ) ) - api_utils.populate_node_uuid(rpc_port, port) if rpc_port.portgroup_id: pg = objects.Portgroup.get(api.request.context, rpc_port.portgroup_id) port['portgroup_uuid'] = pg.uuid @@ -166,12 +166,10 @@ def list_convert_with_links(rpc_ports, limit, url, fields=None, **kwargs): try: port = convert_with_links(rpc_port, fields=fields, sanitize=False) - except exception.NodeNotFound: # NOTE(dtantsur): node was deleted after we fetched the port # list, meaning that the port was also deleted. Skip it. - LOG.debug('Skipping port %s as its node was deleted', - rpc_port.uuid) - continue + if port['node_uuid'] is None: + continue except exception.PortgroupNotFound: # NOTE(dtantsur): port group was deleted after we fetched the # port list, it may mean that the port was deleted too, but diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 555ff164d1..76c40fc2fd 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -523,7 +523,7 @@ RELEASE_MAPPING = { 'Chassis': ['1.3'], 'Deployment': ['1.0'], 'DeployTemplate': ['1.1'], - 'Port': ['1.10'], + 'Port': ['1.11'], 'Portgroup': ['1.4'], 'Trait': ['1.0'], 'TraitList': ['1.0'], diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index a0cefea35b..7a6e6862e3 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -980,9 +980,7 @@ class Connection(api.Connection): sort_key=None, sort_dir=None, owner=None, project=None): query = sa.select(models.Port).where( - models.Port.portgroup_id == portgroup_id - ) - + models.Port.portgroup_id == portgroup_id) if owner: query = add_port_filter_by_node_owner(query, owner) elif project: diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index 34de1b364a..6fc8e21ab1 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/ironic/db/sqlalchemy/models.py @@ -25,6 +25,7 @@ from urllib import parse as urlparse from oslo_db import options as db_options from oslo_db.sqlalchemy import models from oslo_db.sqlalchemy import types as db_types +from sqlalchemy.ext.associationproxy import association_proxy from sqlalchemy import Boolean, Column, DateTime, false, Index from sqlalchemy import ForeignKey, Integer from sqlalchemy import schema, String, Text @@ -262,6 +263,15 @@ class Port(Base): is_smartnic = Column(Boolean, nullable=True, default=False) name = Column(String(255), nullable=True) + _node_uuid = orm.relationship( + "Node", + viewonly=True, + primaryjoin="(Node.id == Port.node_id)", + lazy="selectin", + ) + node_uuid = association_proxy( + "_node_uuid", "uuid", creator=lambda _i: Node(uuid=_i)) + class Portgroup(Base): """Represents a group of network ports of a bare metal node.""" diff --git a/ironic/objects/port.py b/ironic/objects/port.py index b4d0e78eb1..d45bee4b5a 100644 --- a/ironic/objects/port.py +++ b/ironic/objects/port.py @@ -44,7 +44,8 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): # change) # Version 1.9: Add support for Smart NIC port # Version 1.10: Add name field - VERSION = '1.10' + # Version 1.11: Add node_uuid field + VERSION = '1.11' dbapi = dbapi.get_instance() @@ -52,6 +53,7 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): 'id': object_fields.IntegerField(), 'uuid': object_fields.UUIDField(nullable=True), 'node_id': object_fields.IntegerField(nullable=True), + 'node_uuid': object_fields.UUIDField(nullable=True), 'address': object_fields.MACAddressField(nullable=True), 'extra': object_fields.FlexibleDictField(nullable=True), 'local_link_connection': object_fields.FlexibleDictField( @@ -377,6 +379,10 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): """ values = self.do_version_changes_for_db() db_port = self.dbapi.create_port(values) + # NOTE(hjensas): To avoid lazy load issue (DetachedInstanceError) in + # sqlalchemy, get new port the port from the DB to ensure the node_uuid + # via association_proxy relationship is loaded. + db_port = self.dbapi.get_port_by_id(db_port['id']) self._from_db_object(self._context, self, db_port) # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable diff --git a/ironic/tests/unit/api/controllers/v1/test_port.py b/ironic/tests/unit/api/controllers/v1/test_port.py index 6823c3b518..7092a4012d 100644 --- a/ironic/tests/unit/api/controllers/v1/test_port.py +++ b/ironic/tests/unit/api/controllers/v1/test_port.py @@ -236,35 +236,6 @@ class TestListPorts(test_api_base.BaseApiTest): # never expose the node_id self.assertNotIn('node_id', data['ports'][0]) - # NOTE(jlvillal): autospec=True doesn't work on staticmethods: - # https://bugs.python.org/issue23078 - @mock.patch.object(objects.Node, 'get_by_id', spec_set=types.FunctionType) - def test_list_with_deleted_node(self, mock_get_node): - # check that we don't end up with HTTP 400 when node deletion races - # with listing ports - see https://launchpad.net/bugs/1748893 - obj_utils.create_test_port(self.context, node_id=self.node.id) - mock_get_node.side_effect = exception.NodeNotFound('boom') - data = self.get_json('/ports') - self.assertEqual([], data['ports']) - - # NOTE(jlvillal): autospec=True doesn't work on staticmethods: - # https://bugs.python.org/issue23078 - @mock.patch.object(objects.Node, 'get_by_id', - spec_set=types.FunctionType) - def test_list_detailed_with_deleted_node(self, mock_get_node): - # check that we don't end up with HTTP 400 when node deletion races - # with listing ports - see https://launchpad.net/bugs/1748893 - port = obj_utils.create_test_port(self.context, node_id=self.node.id) - port2 = obj_utils.create_test_port(self.context, node_id=self.node.id, - uuid=uuidutils.generate_uuid(), - address='66:44:55:33:11:22') - mock_get_node.side_effect = [exception.NodeNotFound('boom'), self.node] - data = self.get_json('/ports/detail') - # The "correct" port is still returned - self.assertEqual(1, len(data['ports'])) - self.assertIn(data['ports'][0]['uuid'], {port.uuid, port2.uuid}) - self.assertEqual(self.node.uuid, data['ports'][0]['node_uuid']) - # NOTE(jlvillal): autospec=True doesn't work on staticmethods: # https://bugs.python.org/issue23078 @mock.patch.object(objects.Portgroup, 'get', spec_set=types.FunctionType) diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index c59ee1cc42..87ba4b5147 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -272,6 +272,8 @@ def get_test_port(**kw): 'version': kw.get('version', port.Port.VERSION), 'uuid': kw.get('uuid', '1be26c0b-03f2-4d2e-ae87-c02d7f33c781'), 'node_id': kw.get('node_id', 123), + 'node_uuid': kw.get('node_uuid', + '59d102f7-5840-4299-8ec8-80c0ebae9de1'), 'address': kw.get('address', '52:54:00:cf:2d:31'), 'extra': kw.get('extra', {}), 'created_at': kw.get('created_at'), diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index 2e45ee0436..017a0d2103 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -679,7 +679,7 @@ expected_object_fingerprints = { 'Node': '1.36-8a080e31ba89ca5f09e859bd259b54dc', 'MyObj': '1.5-9459d30d6954bffc7a9afd347a807ca6', 'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905', - 'Port': '1.10-67381b065c597c8d3a13c5dbc6243c33', + 'Port': '1.11-97bf15b61224f26c65e90f007d78bfd2', 'Portgroup': '1.4-71923a81a86743b313b190f5c675e258', 'Conductor': '1.3-d3f53e853b4d58cae5bfbd9a8341af4a', 'EventType': '1.1-aa2ba1afd38553e3880c267404e8d370', diff --git a/ironic/tests/unit/objects/test_port.py b/ironic/tests/unit/objects/test_port.py index cf76338086..4c7280216f 100644 --- a/ironic/tests/unit/objects/test_port.py +++ b/ironic/tests/unit/objects/test_port.py @@ -88,12 +88,16 @@ class TestPortObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn): port = objects.Port(self.context, **self.fake_port) with mock.patch.object(self.dbapi, 'create_port', autospec=True) as mock_create_port: - mock_create_port.return_value = db_utils.get_test_port() + with mock.patch.object(self.dbapi, 'get_port_by_id', + autospec=True) as mock_get_port: + test_port = db_utils.get_test_port() + mock_create_port.return_value = test_port + mock_get_port.return_value = test_port - port.create() + port.create() - args, _kwargs = mock_create_port.call_args - self.assertEqual(objects.Port.VERSION, args[0]['version']) + args, _kwargs = mock_create_port.call_args + self.assertEqual(objects.Port.VERSION, args[0]['version']) def test_save(self): uuid = self.fake_port['uuid']