Merge "Use association_proxy for ports node_uuid"

9 files changed, 33 insertions, 44 deletions

diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py
index adc21ebc2..d70c13f27 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 555ff164d..76c40fc2f 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 a0cefea35..7a6e6862e 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 34de1b364..6fc8e21ab 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 b4d0e78eb..d45bee4b5 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 6823c3b51..7092a4012 100644
--- a/ironic/tests/unit/api/controllers/v1/test_port.py
+++ b/ironic/tests/unit/api/controllers/v1/test_port.py
@@ -238,35 +238,6 @@ class TestListPorts(test_api_base.BaseApiTest):
 
     # 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)
     def test_list_with_deleted_port_group(self, mock_get_pg):
         # check that we don't end up with HTTP 400 when port group deletion
diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py
index c59ee1cc4..87ba4b514 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 2e45ee043..017a0d210 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 cf7633808..4c7280216 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']