Use association_proxy for ports node_uuid

This change adds 'node_uuid' to ironic.objects.port.Port
and adds a relationship using association_proxy in
models.Port. Using the association_proxy removes the need
to do the node lookup to populate node uuid for ports in
the api controller.

NOTE:
 On port create a read is added to read the port from the
 database, this ensures node_uuid is loaded and solves the
 DetachedInstanceError which is otherwise raised.

Bumps Port object version to 1.11

With patch:
  1. Returned 20000 ports in python 2.7768702507019043
     seconds from the DB.
  2. Took 0.433107852935791 seconds to iterate through
     20000 port objects.
     Ports table is roughly 12800000 bytes of JSON.
  3. Took 5.662816762924194 seconds to return all 20000
     ports via ports API call pattern.

Without patch:
  1. Returned 20000 ports in python 1.0273635387420654
     seconds from the DB.
  2. Took 0.4772777557373047 seconds to iterate through
     20000 port objects.
     Ports table is roughly 12800000 bytes of JSON.
  3. Took 147.8800814151764 seconds to return all 20000
     ports via ports API call pattern.

Conclusion:
  Test #1 plain dbapi.get_port_list() test is ~3 times
  slower, but Test #3 doing the API call pattern test
  is ~2500% better.

Story: 2007789
Task: 40035
Change-Id: Iff204b3056f3058f795f05dc1d240f494d60672a

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']