Merge "Phase 1 - SQLAlchemy 2.0 Compatability"

ironic/conductor/base_manager.py

@@ -88,10 +88,14 @@ class BaseConductorManager(object):
         # clear all locks held by this conductor before registering
         self.dbapi.clear_node_reservations_for_conductor(self.host)
 
-    def init_host(self, admin_context=None):
+    def init_host(self, admin_context=None, start_consoles=True,
+                  start_allocations=True):
         """Initialize the conductor host.
 
         :param admin_context: the admin context to pass to periodic tasks.
+        :param start_consoles: If consoles should be started in intialization.
+        :param start_allocations: If allocations should be started in
+                                  initialization.
         :raises: RuntimeError when conductor is already running.
         :raises: NoDriversLoaded when no drivers are enabled on the conductor.
         :raises: DriverNotFound if a driver is enabled that does not exist.
@@ -189,8 +193,9 @@ class BaseConductorManager(object):
 
         # Start consoles if it set enabled in a greenthread.
         try:
-            self._spawn_worker(self._start_consoles,
+            if start_consoles:
-                               ironic_context.get_admin_context())
+                self._spawn_worker(self._start_consoles,
+                                   ironic_context.get_admin_context())
         except exception.NoFreeConductorWorker:
             LOG.warning('Failed to start worker for restarting consoles.')
 
@@ -207,8 +212,9 @@ class BaseConductorManager(object):
 
         # Resume allocations that started before the restart.
         try:
-            self._spawn_worker(self._resume_allocations,
+            if start_allocations:
-                               ironic_context.get_admin_context())
+                self._spawn_worker(self._resume_allocations,
+                                   ironic_context.get_admin_context())
         except exception.NoFreeConductorWorker:
             LOG.warning('Failed to start worker for resuming allocations.')
 
@@ -539,6 +545,7 @@ class BaseConductorManager(object):
             try:
                 with task_manager.acquire(context, node_uuid, shared=False,
                                           purpose='start console') as task:
+
                     notify_utils.emit_console_notification(
                         task, 'console_restore',
                         obj_fields.NotificationStatus.START)

ironic/conductor/manager.py

@@ -2203,18 +2203,16 @@ class ConductorManager(base_manager.BaseConductorManager):
         """
         LOG.debug('RPC set_console_mode called for node %(node)s with '
                   'enabled %(enabled)s', {'node': node_id, 'enabled': enabled})
-
+        with task_manager.acquire(context, node_id, shared=True,
-        with task_manager.acquire(context, node_id, shared=False,
                                   purpose='setting console mode') as task:
             node = task.node
-
             task.driver.console.validate(task)
-
             if enabled == node.console_enabled:
                 op = 'enabled' if enabled else 'disabled'
                 LOG.info("No console action was triggered because the "
                          "console is already %s", op)
             else:
+                task.upgrade_lock()
                 node.last_error = None
                 node.save()
                 task.spawn_after(self._spawn_worker,
@@ -3469,7 +3467,6 @@ class ConductorManager(base_manager.BaseConductorManager):
                                                            self.conductor.id):
                         # Another conductor has taken over, skipping
                         continue
-
                     LOG.debug('Taking over allocation %s', allocation.uuid)
                     allocations.do_allocate(context, allocation)
                 except Exception:

ironic/db/sqlalchemy/__init__.py

@@ -13,6 +13,4 @@
 from oslo_db.sqlalchemy import enginefacade
 
 # NOTE(dtantsur): we want sqlite as close to a real database as possible.
-# FIXME(stephenfin): we need to remove reliance on autocommit semantics ASAP
+enginefacade.configure(sqlite_fk=True)
-# since it's not compatible with SQLAlchemy 2.0
-enginefacade.configure(sqlite_fk=True, __autocommit=True)

ironic/db/sqlalchemy/api.py

@@ -19,9 +19,11 @@ import datetime
 import json
 import threading
 
+from oslo_concurrency import lockutils
 from oslo_db import api as oslo_db_api
 from oslo_db import exception as db_exc
 from oslo_db.sqlalchemy import enginefacade
+from oslo_db.sqlalchemy import orm as sa_orm
 from oslo_db.sqlalchemy import utils as db_utils
 from oslo_log import log
 from oslo_utils import netutils
@@ -53,6 +55,10 @@ LOG = log.getLogger(__name__)
 
 _CONTEXT = threading.local()
 
+
+RESERVATION_SEMAPHORE = "reserve_node_db_lock"
+synchronized = lockutils.synchronized_with_prefix('ironic-')
+
 # NOTE(mgoddard): We limit the number of traits per node to 50 as this is the
 # maximum number of traits per resource provider allowed in placement.
 MAX_TRAITS_PER_NODE = 50
@@ -83,6 +89,11 @@ def _wrap_session(session):
 def _get_node_query_with_all_for_single_node():
     """Return a query object for the Node joined with all relevant fields.
 
+    Deprecated: This method, while useful, returns a "Legacy Query" object
+    which, while useful is considered a legacy object from SQLAlchemy
+    which at some point may be removed. SQLAlchemy encourages all users
+    to move to the unified ORM/Core Select interface.
+
     This method utilizes a joined load query which creates a result set
     where corresponding traits, and tags, are joined together in the result
     set.
@@ -109,9 +120,10 @@ def _get_node_query_with_all_for_single_node():
 
     Where joins are super in-efficent for Ironic, is where nodes are being
     enumerated, as the above result set pattern is not just for one node, but
-    potentially thousands of nodes. In that case, we should use the
+    potentially thousands of nodes. Please consider using _get_node_select
-    _get_node_query_with_all_for_list helper to return a more appropriate
+    which results in a primary query for the nodes, and then performs
-    query object which will be more efficient for the end user.
+    additional targeted queries for the joined tables, as opposed to
+    performing client side de-duplication.
 
     :returns: a query object.
     """
@@ -131,45 +143,33 @@ def _get_node_query_with_all_for_single_node():
             .options(joinedload('traits')))
 
 
-def _get_node_query_with_all_for_list():
+def _get_node_select():
-    """Return a query object for the Node with queried extra fields.
+    """Returns a SQLAlchemy Select Object for Nodes.
 
-    This method returns a query object joining tags and traits in a pattern
+    This method returns a pre-formatted select object which models
-    where the result set is first built, and then the resulting associations
+    the entire Node object, allowing callers to operate on a node like
-    are queried separately and the objects are reconciled by SQLAlchemy to
+    they would have with an SQLAlchemy ORM Query Object.
-    build the composite objects based upon the associations.
 
-    This results in the following query pattern when the query is executed:
+    This object *also* performs two additional select queries, in the form
+    of a selectin operation, to achieve the same results of a Join query,
+    but without the join query itself, and the client side load.
 
-    select $fields from nodes where x;
+    This method is best utilized when retrieving lists of nodes.
-    # SQLAlchemy creates a list of associated node IDs.
-    select $fields from tags where node_id in ('1', '3', '37268');
-    select $fields from traits where node_id in ('1', '3', '37268');
 
-    SQLAlchemy then returns a result set where the tags and traits are
+    Select objects in this fashion were  added as a result of SQLAlchemy 1.4
-    composited together efficently as opposed to having to deduplicate
+    in preparation for SQLAlchemy 2.0's release to provide a unified
-    the result set. This shifts additional load to the database which
+    select interface.
-    was previously a high overhead operation with-in the conductor...
-    which results in a slower conductor.
 
-    :returns: a query object.
+    :returns: a select object
     """
-    # NOTE(TheJulia): When comparing CI rubs *with* this being the default
-    # for all general list operations, at 10k nodes, this pattern appears
-    # to be on-par with a 5% variability between the two example benchmark
-    # tests. That being said, the test *does* not include tags or traits
-    # in it's test data set so client side deduplication is not measured.
 
-    # NOTE(TheJulia): Basic benchmark difference
+    # NOTE(TheJulia): This returns a query in the SQLAlchemy 1.4->2.0
-    # tests data creation: 67.117 seconds
+    # migration style as query model loading is deprecated.
-    # 2.32 seconds to obtain all nodes from SQLAlchemy (10k nodes)
+
-    # 4.99 seconds to obtain all nodes *and* have node objects (10k nodes)
+    # This must use selectinload to avoid later need to invokededuplication.
-    # If this holds true, the required record deduplication with joinedload
+    return (sa.select(models.Node)
-    # may be basically the same amount of overhead as requesting the tags
+            .options(selectinload(models.Node.tags),
-    # and traits separately.
+                     selectinload(models.Node.traits)))
-    return (model_query(models.Node)
-            .options(selectinload('tags'))
-            .options(selectinload('traits')))
 
 
 def _get_deploy_template_query_with_steps():
@@ -332,8 +332,10 @@ def add_allocation_filter_by_conductor(query, value):
 
 
 def _paginate_query(model, limit=None, marker=None, sort_key=None,
-                    sort_dir=None, query=None):
+                    sort_dir=None, query=None, return_base_tuple=False):
-    if not query:
+    # NOTE(TheJulia): We can't just ask for the bool of query if it is
+    # populated, so we need to ask if it is None.
+    if query is None:
         query = model_query(model)
     sort_keys = ['id']
     if sort_key and sort_key not in sort_keys:
@@ -345,7 +347,28 @@ def _paginate_query(model, limit=None, marker=None, sort_key=None,
         raise exception.InvalidParameterValue(
             _('The sort_key value "%(key)s" is an invalid field for sorting')
             % {'key': sort_key})
-    return query.all()
+    with _session_for_read() as session:
+        # NOTE(TheJulia): SQLAlchemy 2.0 no longer returns pre-uniqued result
+        # sets in ORM mode, so we need to explicitly ask for it to be unique
+        # before returning it to the caller.
+        if isinstance(query, sa_orm.Query):
+            # The classic ORM query object result set which is deprecated
+            # in advance of SQLAlchemy 2.0.
+            return query.all()
+        else:
+            # In this case, we have a sqlalchemy.sql.selectable.Select
+            # (most likely) which utilizes the unified select interface.
+            res = session.execute(query).fetchall()
+            if len(res) == 0:
+                # Return an empty list instead of a class with no objects.
+                return []
+            if return_base_tuple:
+                # The caller expects a tuple, lets just give it to them.
+                return res
+            # Everything is a tuple in a resultset from the unified interface
+            # but for objects, our model expects just object access,
+            # so we extract and return them.
+            return [r[0] for r in res]
 
 
 def _filter_active_conductors(query, interval=None):
@@ -514,15 +537,16 @@ class Connection(api.Connection):
         else:
             columns = [getattr(models.Node, c) for c in columns]
 
-        query = model_query(*columns, base_model=models.Node)
+        query = sa.select(*columns)
         query = self._add_nodes_filters(query, filters)
         return _paginate_query(models.Node, limit, marker,
-                               sort_key, sort_dir, query)
+                               sort_key, sort_dir, query,
+                               return_base_tuple=True)
 
     def get_node_list(self, filters=None, limit=None, marker=None,
                       sort_key=None, sort_dir=None, fields=None):
         if not fields:
-            query = _get_node_query_with_all_for_list()
+            query = _get_node_select()
             query = self._add_nodes_filters(query, filters)
             return _paginate_query(models.Node, limit, marker,
                                    sort_key, sort_dir, query)
@@ -559,24 +583,25 @@ class Connection(api.Connection):
             # with SQLAlchemy.
             traits_found = True
             use_columns.remove('traits')
-
         # Generate the column object list so SQLAlchemy only fulfills
         # the requested columns.
         use_columns = [getattr(models.Node, c) for c in use_columns]
-
         # In essence, traits (and anything else needed to generate the
         # composite objects) need to be reconciled without using a join
         # as multiple rows can be generated in the result set being returned
         # from the database server. In this case, with traits, we use
         # a selectinload pattern.
         if traits_found:
-            query = model_query(models.Node).options(
+            query = sa.select(models.Node).options(
-                Load(models.Node).load_only(*use_columns),
+                selectinload(models.Node.traits),
-                selectinload(models.Node.traits))
+                Load(models.Node).load_only(*use_columns)
+            )
         else:
-            query = model_query(models.Node).options(
+            # Note for others, if you ask for a whole model, it is
-                Load(models.Node).load_only(*use_columns))
+            # modeled, i.e. you can access it as an object.
-
+            query = sa.select(models.NodeBase).options(
+                Load(models.Node).load_only(*use_columns)
+            )
         query = self._add_nodes_filters(query, filters)
         return _paginate_query(models.Node, limit, marker,
                                sort_key, sort_dir, query)
@@ -618,40 +643,85 @@ class Connection(api.Connection):
 
         return mapping
 
+    @synchronized(RESERVATION_SEMAPHORE, fair=True)
+    def _reserve_node_place_lock(self, tag, node_id, node):
+        try:
+            # NOTE(TheJulia): We explicitly do *not* synch the session
+            # so the other actions in the conductor do not become aware
+            # that the lock is in place and believe they hold the lock.
+            # This necessitates an overall lock in the code side, so
+            # we avoid conditions where two separate threads can believe
+            # they hold locks at the same time.
+            with _session_for_write() as session:
+                res = session.execute(
+                    sa.update(models.Node).
+                    where(models.Node.id == node.id).
+                    where(models.Node.reservation == None).  # noqa
+                    values(reservation=tag).
+                    execution_options(synchronize_session=False))
+                session.flush()
+            node = self._get_node_by_id_no_joins(node.id)
+            # NOTE(TheJulia): In SQLAlchemy 2.0 style, we don't
+            # magically get a changed node as they moved from the
+            # many ways to do things to singular ways to do things.
+            if res.rowcount != 1:
+                # Nothing updated and node exists. Must already be
+                # locked.
+                raise exception.NodeLocked(node=node.uuid,
+                                           host=node.reservation)
+        except NoResultFound:
+            # In the event that someone has deleted the node on
+            # another thread.
+            raise exception.NodeNotFound(node=node_id)
+
     @oslo_db_api.retry_on_deadlock
     def reserve_node(self, tag, node_id):
-        with _session_for_write():
+        with _session_for_read():
-            query = _get_node_query_with_all_for_single_node()
-            query = add_identity_filter(query, node_id)
-            count = query.filter_by(reservation=None).update(
-                {'reservation': tag}, synchronize_session=False)
             try:
+                # TODO(TheJulia): Figure out a good way to query
+                # this so that we do it as light as possible without
+                # the full object invocation, which will speed lock
+                # activities. Granted, this is all at the DB level
+                # so maybe that is okay in the grand scheme of things.
+                query = model_query(models.Node)
+                query = add_identity_filter(query, node_id)
                 node = query.one()
-                if count != 1:
-                    # Nothing updated and node exists. Must already be
-                    # locked.
-                    raise exception.NodeLocked(node=node.uuid,
-                                               host=node['reservation'])
-                return node
             except NoResultFound:
                 raise exception.NodeNotFound(node=node_id)
+            if node.reservation:
+                # Fail fast, instead of attempt the update.
+                raise exception.NodeLocked(node=node.uuid,
+                                           host=node.reservation)
+        self._reserve_node_place_lock(tag, node_id, node)
+        # Return a node object as that is the contract for this method.
+        return self.get_node_by_id(node.id)
 
     @oslo_db_api.retry_on_deadlock
     def release_node(self, tag, node_id):
-        with _session_for_write():
+        with _session_for_read():
-            query = model_query(models.Node)
-            query = add_identity_filter(query, node_id)
-            # be optimistic and assume we usually release a reservation
-            count = query.filter_by(reservation=tag).update(
-                {'reservation': None}, synchronize_session=False)
             try:
-                if count != 1:
+                query = model_query(models.Node)
-                    node = query.one()
+                query = add_identity_filter(query, node_id)
-                    if node['reservation'] is None:
+                node = query.one()
+            except NoResultFound:
+                raise exception.NodeNotFound(node=node_id)
+        with _session_for_write() as session:
+            try:
+                res = session.execute(
+                    sa.update(models.Node).
+                    where(models.Node.id == node.id).
+                    where(models.Node.reservation == tag).
+                    values(reservation=None).
+                    execution_options(synchronize_session=False)
+                )
+                node = self.get_node_by_id(node.id)
+                if res.rowcount != 1:
+                    if node.reservation is None:
                         raise exception.NodeNotLocked(node=node.uuid)
                     else:
                         raise exception.NodeLocked(node=node.uuid,
                                                    host=node['reservation'])
+                session.flush()
             except NoResultFound:
                 raise exception.NodeNotFound(node=node_id)
 
@@ -677,47 +747,68 @@ class Connection(api.Connection):
 
         node = models.Node()
         node.update(values)
-        with _session_for_write() as session:
+        try:
-            try:
+            with _session_for_write() as session:
                 session.add(node)
+                # Set tags & traits to [] for new created node
+                # NOTE(mgoddard): We need to set the tags and traits fields in
+                # the session context, otherwise SQLAlchemy will try and fail
+                # to lazy load the attributes, resulting in an exception being
+                # raised.
+                node['tags'] = []
+                node['traits'] = []
                 session.flush()
-            except db_exc.DBDuplicateEntry as exc:
+        except db_exc.DBDuplicateEntry as exc:
-                if 'name' in exc.columns:
+            if 'name' in exc.columns:
-                    raise exception.DuplicateName(name=values['name'])
+                raise exception.DuplicateName(name=values['name'])
-                elif 'instance_uuid' in exc.columns:
+            elif 'instance_uuid' in exc.columns:
-                    raise exception.InstanceAssociated(
+                raise exception.InstanceAssociated(
-                        instance_uuid=values['instance_uuid'],
+                    instance_uuid=values['instance_uuid'],
-                        node=values['uuid'])
+                    node=values['uuid'])
-                raise exception.NodeAlreadyExists(uuid=values['uuid'])
+            raise exception.NodeAlreadyExists(uuid=values['uuid'])
-            # Set tags & traits to [] for new created node
-            # NOTE(mgoddard): We need to set the tags and traits fields in the
-            # session context, otherwise SQLAlchemy will try and fail to lazy
-            # load the attributes, resulting in an exception being raised.
-            node['tags'] = []
-            node['traits'] = []
         return node
 
-    def get_node_by_id(self, node_id):
+    def _get_node_by_id_no_joins(self, node_id):
-        query = _get_node_query_with_all_for_single_node()
+        # TODO(TheJulia): Maybe replace with this with a minimal
-        query = query.filter_by(id=node_id)
+        # "get these three fields" thing.
         try:
-            return query.one()
+            with _session_for_read() as session:
+                # Explicitly load NodeBase as the invocation of the
+                # priamary model object reesults in the join query
+                # triggering.
+                return session.execute(
+                    sa.select(models.NodeBase).filter_by(id=node_id).limit(1)
+                ).scalars().first()
+        except NoResultFound:
+            raise exception.NodeNotFound(node=node_id)
+
+    def get_node_by_id(self, node_id):
+        try:
+            query = _get_node_select()
+            with _session_for_read() as session:
+                return session.scalars(
+                    query.filter_by(id=node_id).limit(1)
+                ).unique().one()
         except NoResultFound:
             raise exception.NodeNotFound(node=node_id)
 
     def get_node_by_uuid(self, node_uuid):
-        query = _get_node_query_with_all_for_single_node()
-        query = query.filter_by(uuid=node_uuid)
         try:
-            return query.one()
+            query = _get_node_select()
+            with _session_for_read() as session:
+                return session.scalars(
+                    query.filter_by(uuid=node_uuid).limit(1)
+                ).unique().one()
         except NoResultFound:
             raise exception.NodeNotFound(node=node_uuid)
 
     def get_node_by_name(self, node_name):
-        query = _get_node_query_with_all_for_single_node()
-        query = query.filter_by(name=node_name)
         try:
-            return query.one()
+            query = _get_node_select()
+            with _session_for_read() as session:
+                return session.scalars(
+                    query.filter_by(name=node_name).limit(1)
+                ).unique().one()
         except NoResultFound:
             raise exception.NodeNotFound(node=node_name)
 
@@ -725,15 +816,14 @@ class Connection(api.Connection):
         if not uuidutils.is_uuid_like(instance):
             raise exception.InvalidUUID(uuid=instance)
 
-        query = _get_node_query_with_all_for_single_node()
-        query = query.filter_by(instance_uuid=instance)
-
         try:
-            result = query.one()
+            query = _get_node_select()
+            with _session_for_read() as session:
+                return session.scalars(
+                    query.filter_by(instance_uuid=instance).limit(1)
+                ).unique().one()
         except NoResultFound:
-            raise exception.InstanceNotFound(instance=instance)
+            raise exception.InstanceNotFound(instance_uuid=instance)
-
-        return result
 
     @oslo_db_api.retry_on_deadlock
     def destroy_node(self, node_id):
@@ -849,6 +939,9 @@ class Connection(api.Connection):
             # Return the updated node model joined with all relevant fields.
             query = _get_node_query_with_all_for_single_node()
             query = add_identity_filter(query, node_id)
+            # FIXME(TheJulia): This entire method needs to be re-written to
+            # use the proper execution format for SQLAlchemy 2.0. Likely
+            # A query, independent update, and a re-query on the transaction.
             return query.one()
 
     def get_port_by_id(self, port_id):
@@ -925,15 +1018,15 @@ class Connection(api.Connection):
 
         port = models.Port()
         port.update(values)
-        with _session_for_write() as session:
+        try:
-            try:
+            with _session_for_write() as session:
                 session.add(port)
                 session.flush()
-            except db_exc.DBDuplicateEntry as exc:
+        except db_exc.DBDuplicateEntry as exc:
-                if 'address' in exc.columns:
+            if 'address' in exc.columns:
-                    raise exception.MACAlreadyExists(mac=values['address'])
+                raise exception.MACAlreadyExists(mac=values['address'])
-                raise exception.PortAlreadyExists(uuid=values['uuid'])
+            raise exception.PortAlreadyExists(uuid=values['uuid'])
-            return port
+        return port
 
     @oslo_db_api.retry_on_deadlock
     def update_port(self, port_id, values):
@@ -1110,13 +1203,13 @@ class Connection(api.Connection):
 
         chassis = models.Chassis()
         chassis.update(values)
-        with _session_for_write() as session:
+        try:
-            try:
+            with _session_for_write() as session:
                 session.add(chassis)
                 session.flush()
-            except db_exc.DBDuplicateEntry:
+        except db_exc.DBDuplicateEntry:
-                raise exception.ChassisAlreadyExists(uuid=values['uuid'])
+            raise exception.ChassisAlreadyExists(uuid=values['uuid'])
-            return chassis
+        return chassis
 
     @oslo_db_api.retry_on_deadlock
     def update_chassis(self, chassis_id, values):
@@ -1293,6 +1386,13 @@ class Connection(api.Connection):
     def register_conductor_hardware_interfaces(self, conductor_id, interfaces):
         with _session_for_write() as session:
             try:
+                try:
+                    session.begin()
+                except sa.exc.InvalidRequestError:
+                    # When running unit tests, the transaction reports as
+                    # already started, where as in service startup this is
+                    # the first write op.
+                    pass
                 for iface in interfaces:
                     conductor_hw_iface = models.ConductorHardwareInterfaces()
                     conductor_hw_iface['conductor_id'] = conductor_id
@@ -1388,6 +1488,8 @@ class Connection(api.Connection):
         q = q.filter(models.Port.address.in_(addresses))
 
         try:
+            # FIXME(TheJulia): This needs to be updated to be
+            # an explicit query to identify the node for SQLAlchemy.
             return q.one()
         except NoResultFound:
             raise exception.NodeNotFound(
@@ -1586,6 +1688,8 @@ class Connection(api.Connection):
         if not versions:
             return []
 
+        if model_name == 'Node':
+            model_name = 'NodeBase'
         model = models.get_class(model_name)
 
         # NOTE(rloo): .notin_ does not handle null:
@@ -1614,7 +1718,11 @@ class Connection(api.Connection):
         """
         object_versions = release_mappings.get_object_versions()
         table_missing_ok = False
-        for model in models.Base.__subclasses__():
+        models_to_check = models.Base.__subclasses__()
+        # We need to append Node to the list as it is a subclass of
+        # NodeBase, which is intentional to delineate excess queries.
+        models_to_check.append(models.Node)
+        for model in models_to_check:
             if model.__name__ not in object_versions:
                 continue
 
@@ -1688,8 +1796,9 @@ class Connection(api.Connection):
         mapping = release_mappings.RELEASE_MAPPING['master']['objects']
         total_to_migrate = 0
         total_migrated = 0
-
+        all_models = models.Base.__subclasses__()
-        sql_models = [model for model in models.Base.__subclasses__()
+        all_models.append(models.Node)
+        sql_models = [model for model in all_models
                       if model.__name__ in mapping]
         for model in sql_models:
             version = mapping[model.__name__][0]
@@ -2238,6 +2347,7 @@ class Connection(api.Connection):
                 # Return the updated template joined with all relevant fields.
                 query = _get_deploy_template_query_with_steps()
                 query = add_identity_filter(query, template_id)
+                # FIXME(TheJulia): This needs to be fixed for SQLAlchemy 2.0.
                 return query.one()
         except db_exc.DBDuplicateEntry as e:
             if 'name' in e.columns:
@@ -2260,6 +2370,7 @@ class Connection(api.Connection):
         query = (_get_deploy_template_query_with_steps()
                  .filter_by(**{field: value}))
         try:
+            # FIXME(TheJulia): This needs to be fixed for SQLAlchemy 2.0
             return query.one()
         except NoResultFound:
             raise exception.DeployTemplateNotFound(template=value)

ironic/db/sqlalchemy/models.py

@@ -19,6 +19,7 @@ SQLAlchemy models for baremetal data.
 """
 
 from os import path
+from typing import List
 from urllib import parse as urlparse
 
 from oslo_db import options as db_options
@@ -27,8 +28,8 @@ from oslo_db.sqlalchemy import types as db_types
 from sqlalchemy import Boolean, Column, DateTime, false, Index
 from sqlalchemy import ForeignKey, Integer
 from sqlalchemy import schema, String, Text
-from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy import orm
+from sqlalchemy.orm import declarative_base
 
 from ironic.common import exception
 from ironic.common.i18n import _
@@ -116,8 +117,8 @@ class ConductorHardwareInterfaces(Base):
     default = Column(Boolean, default=False, nullable=False)
 
 
-class Node(Base):
+class NodeBase(Base):
-    """Represents a bare metal node."""
+    """Represents a base bare metal node."""
 
     __tablename__ = 'nodes'
     __table_args__ = (
@@ -213,6 +214,32 @@ class Node(Base):
     secure_boot = Column(Boolean, nullable=True)
 
 
+class Node(NodeBase):
+    """Represents a bare metal node."""
+
+    # NOTE(TheJulia): The purpose of the delineation between NodeBase and Node
+    # is to facilitate a hard delineation for queries where we do not need to
+    # populate additional information needlessly which would normally populate
+    # from the access of the property. In this case, Traits and Tags.
+    # The other reason we do this, is because these are generally "joined"
+    # data structures, we cannot de-duplicate node objects with unhashable dict
+    # data structures.
+
+    # NOTE(TheJulia): The choice of selectin lazy population is intentional
+    # as it causes a subselect to occur, skipping the need for deduplication
+    # in general. This puts a slightly higher query load on the DB server, but
+    # means *far* less gets shipped over the wire in the end.
+    traits: orm.Mapped[List['NodeTrait']] = orm.relationship(  # noqa
+        "NodeTrait",
+        back_populates="node",
+        lazy="selectin")
+
+    tags: orm.Mapped[List['NodeTag']] = orm.relationship(  # noqa
+        "NodeTag",
+        back_populates="node",
+        lazy="selectin")
+
+
 class Port(Base):
     """Represents a network port of a bare metal node."""
 
@@ -270,7 +297,6 @@ class NodeTag(Base):
 
     node = orm.relationship(
         "Node",
-        backref='tags',
         primaryjoin='and_(NodeTag.node_id == Node.id)',
         foreign_keys=node_id
     )
@@ -327,7 +353,6 @@ class NodeTrait(Base):
     trait = Column(String(255), primary_key=True, nullable=False)
     node = orm.relationship(
         "Node",
-        backref='traits',
         primaryjoin='and_(NodeTrait.node_id == Node.id)',
         foreign_keys=node_id
     )

ironic/tests/unit/common/test_release_mappings.py

@@ -91,13 +91,17 @@ class ReleaseMappingsTestCase(base.TestCase):
 
     def test_contains_all_db_objects(self):
         self.assertIn('master', release_mappings.RELEASE_MAPPING)
-        model_names = set((s.__name__ for s in models.Base.__subclasses__()))
+        use_models = models.Base.__subclasses__()
+        use_models.append(models.Node)
+        model_names = set((s.__name__ for s in use_models))
         # NOTE(xek): As a rule, all models which can be changed between
         # releases or are sent through RPC should have their counterpart
         # versioned objects. Do not add an exception for such objects,
         # initialize them with the version 1.0 instead.
+        # NodeBase is also excluded as it is covered by Node.
         exceptions = set(['NodeTag', 'ConductorHardwareInterfaces',
-                          'NodeTrait', 'DeployTemplateStep'])
+                          'NodeTrait', 'DeployTemplateStep',
+                          'NodeBase'])
         model_names -= exceptions
         # NodeTrait maps to two objects
         model_names |= set(['Trait', 'TraitList'])

ironic/tests/unit/conductor/mgr_utils.py

@@ -127,7 +127,12 @@ class ServiceSetUpMixin(object):
     def setUp(self):
         super(ServiceSetUpMixin, self).setUp()
         self.hostname = 'test-host'
-        self.config(node_locked_retry_attempts=1, group='conductor')
+        # Relies upon the default number of "NodeLocked" retries as
+        # in unit testing, sqllite is not operated in a transactional
+        # way and utilizes asynchonous IO. Locking, in particular, can
+        # detect this, and it can cause some false or delayed inpressions
+        # of lock status, causing lock failures.
+        self.config(node_locked_retry_attempts=3, group='conductor')
         self.config(node_locked_retry_interval=0, group='conductor')
 
         self.service = manager.ConductorManager(self.hostname, 'test-topic')
@@ -139,15 +144,18 @@ class ServiceSetUpMixin(object):
             return
         self.service.del_host()
 
-    def _start_service(self, start_periodic_tasks=False):
+    def _start_service(self, start_periodic_tasks=False, start_consoles=True,
+                       start_allocations=True):
         if start_periodic_tasks:
-            self.service.init_host()
+            self.service.init_host(start_consoles=start_consoles,
+                                   start_allocations=start_allocations)
         else:
             with mock.patch.object(periodics, 'PeriodicWorker', autospec=True):
                 with mock.patch.object(pxe_utils, 'place_common_config',
                                        autospec=True):
                     self.service.prepare_host()
-                    self.service.init_host()
+                    self.service.init_host(start_consoles=start_consoles,
+                                           start_allocations=start_allocations)
         self.addCleanup(self._stop_service)
 
 

ironic/tests/unit/conductor/test_allocations.py

@@ -209,7 +209,7 @@ class AllocationTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
             state='allocating',
             conductor_affinity=dead_conductor.id)
 
-        self._start_service()
+        self._start_service(start_allocations=False)
         with mock.patch.object(self.dbapi, 'get_offline_conductors',
                                autospec=True) as mock_conds:
             mock_conds.return_value = [dead_conductor.id]

ironic/tests/unit/conductor/test_base_manager.py

@@ -494,9 +494,11 @@ class StartConsolesTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         obj_utils.create_test_node(
             self.context,
             uuid=uuidutils.generate_uuid(),
-            driver='fake-hardware'
+            driver='fake-hardware',
         )
-        self._start_service()
+        # Enable consoles *after* service has started, otherwise it races
+        # as the service startup also launches consoles.
+        self._start_service(start_consoles=False)
         self.service._start_consoles(self.context)
         self.assertEqual(2, mock_start_console.call_count)
         mock_notify.assert_has_calls(

ironic/tests/unit/conductor/test_manager.py

@@ -7256,7 +7256,7 @@ class DoNodeTakeOverTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
                                                     mock_take_over,
                                                     mock_start_console,
                                                     mock_notify):
-        self._start_service()
+        self._start_service(start_consoles=False)
         node = obj_utils.create_test_node(self.context, driver='fake-hardware',
                                           console_enabled=True)
         di_info = node.driver_internal_info

ironic/tests/unit/db/test_conductor.py

@@ -166,7 +166,9 @@ class DbConductorTestCase(base.DbTestCase):
         c = self._create_test_cdr()
         self.dbapi.touch_conductor(c.hostname)
         self.assertEqual(2, mock_update.call_count)
-        self.assertEqual(2, mock_sleep.call_count)
+        # Count that it was called, but not the number of times
+        # as this is *actually* time.sleep via import from oslo_db.api
+        self.assertTrue(mock_sleep.called)
 
     def test_touch_conductor_not_found(self):
         # A conductor's heartbeat will not create a new record,

ironic/tests/unit/db/test_nodes.py

@@ -367,10 +367,10 @@ class DbNodeTestCase(base.DbTestCase):
         res = self.dbapi.get_node_list(filters={'maintenance': False})
         self.assertEqual([node1.id], [r.id for r in res])
 
-        res = self.dbapi.get_nodeinfo_list(filters={'fault': 'boom'})
+        res = self.dbapi.get_node_list(filters={'fault': 'boom'})
         self.assertEqual([node2.id], [r.id for r in res])
 
-        res = self.dbapi.get_nodeinfo_list(filters={'fault': 'moob'})
+        res = self.dbapi.get_node_list(filters={'fault': 'moob'})
         self.assertEqual([], [r.id for r in res])
 
         res = self.dbapi.get_node_list(filters={'resource_class': 'foo'})
@@ -558,6 +558,9 @@ class DbNodeTestCase(base.DbTestCase):
                                               'cat': 'meow'},
                                           internal_info={'corgi': 'rocks'},
                                           deploy_interface='purring_machine')
+            utils.create_test_node_traits(node_id=node.id,
+                                          traits=['atrait'])
+
             uuids.append(str(node['uuid']))
         req_fields = ['uuid',
                       'provision_state',

releasenotes/notes/prepare-for-sqlalchemy-20-e817f340f261b1a2.yaml

@@ -0,0 +1,7 @@
+---
+upgrade:
+  - |
+    Ironic has started the process of upgrading the code base to support
+    SQLAlchemy 2.0 in anticipation of it's release. This results in the
+    minimum version of SQLAlchemy becoming 1.4.0 as it contains migration
+    features for the move to SQLAlchemy 2.0.

requirements.txt

@@ -6,7 +6,7 @@
 # of appearance. Changing the order has an impact on the overall integration
 # process, which may cause wedges in the gate later.
 pbr>=3.1.1 # Apache-2.0
-SQLAlchemy>=1.2.19 # MIT
+SQLAlchemy>=1.4.0 # MIT
 alembic>=1.4.2 # MIT
 automaton>=1.9.0 # Apache-2.0
 eventlet!=0.18.3,!=0.20.1,>=0.18.2 # MIT

tox.ini

@@ -11,6 +11,7 @@ setenv = VIRTUAL_ENV={envdir}
          PYTHONDONTWRITEBYTECODE = 1
          LANGUAGE=en_US
          LC_ALL=en_US.UTF-8
+         PYTHONUNBUFFERED=1
 deps =
   -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master}
   -r{toxinidir}/requirements.txt