Allow sqalchemy filtering by id and uuid

These additions will allow us to filter nodes
by node uuid and id. This filter API is used
in many places throughout the code base. It is
natural to expect that this API would allow us to
filter by node id and uuid in addtion to the other
supported parameters. This also fixes a 3 year old bug.

This change from lucasagomes has a bug:
https://review.openstack.org/#/c/197141/

In conductor/manager.py, he calls _fail_if_in_state()
and uses the node_id as a filter. However this filter
effectively does nothing because sqalchemy does not
know how to filter by node id on the backend. My changes
fix this problem and make the API more intuitive

Closes-Bug: #1749755

Change-Id: I4efc0d5cd5d5d6108a334f954e1718203b47da0a

ironic/db/sqlalchemy/api.py

@@ -219,8 +219,21 @@ class Connection(api.Connection):
 
     def _add_nodes_filters(self, query, filters):
         if filters is None:
-            filters = []
-
+            filters = dict()
+        supported_filters = {'console_enabled', 'maintenance', 'driver',
+                             'resource_class', 'provision_state', 'uuid', 'id',
+                             'chassis_uuid', 'associated', 'reserved',
+                             'reserved_by_any_of', 'provisioned_before',
+                             'inspection_started_before'}
+        unsupported_filters = set(filters).difference(supported_filters)
+        if unsupported_filters:
+            msg = _("SqlAlchemy API does not support "
+                    "filtering by %s") % ', '.join(unsupported_filters)
+            raise ValueError(msg)
+        for field in ['console_enabled', 'maintenance', 'driver',
+                      'resource_class', 'provision_state', 'uuid', 'id']:
+            if field in filters:
+                query = query.filter_by(**{field: filters[field]})
         if 'chassis_uuid' in filters:
             # get_chassis_by_uuid() to raise an exception if the chassis
             # is not found
@@ -239,14 +252,6 @@ class Connection(api.Connection):
         if 'reserved_by_any_of' in filters:
             query = query.filter(models.Node.reservation.in_(
                 filters['reserved_by_any_of']))
-        if 'maintenance' in filters:
-            query = query.filter_by(maintenance=filters['maintenance'])
-        if 'driver' in filters:
-            query = query.filter_by(driver=filters['driver'])
-        if 'resource_class' in filters:
-            query = query.filter_by(resource_class=filters['resource_class'])
-        if 'provision_state' in filters:
-            query = query.filter_by(provision_state=filters['provision_state'])
         if 'provisioned_before' in filters:
             limit = (timeutils.utcnow() -
                      datetime.timedelta(seconds=filters['provisioned_before']))
@@ -256,8 +261,6 @@ class Connection(api.Connection):
                      (datetime.timedelta(
                          seconds=filters['inspection_started_before'])))
             query = query.filter(models.Node.inspection_started_at < limit)
-        if 'console_enabled' in filters:
-            query = query.filter_by(console_enabled=filters['console_enabled'])
 
         return query
 

ironic/tests/unit/db/test_nodes.py

@@ -186,6 +186,26 @@ class DbNodeTestCase(base.DbTestCase):
         self.assertEqual(sorted([node1.id, node3.id]),
                          sorted([r.id for r in res]))
 
+        res = self.dbapi.get_node_list(filters={'id': node1.id})
+        self.assertEqual([node1.id], [r.id for r in res])
+
+        res = self.dbapi.get_node_list(filters={'uuid': node1.uuid})
+        self.assertEqual([node1.id], [r.id for r in res])
+
+        # ensure unknown filters explode
+        filters = {'bad_filter': 'foo'}
+        self.assertRaisesRegex(ValueError,
+                               'bad_filter',
+                               self.dbapi.get_node_list,
+                               filters=filters)
+
+        # even with good filters present
+        filters = {'bad_filter': 'foo', 'id': node1.id}
+        self.assertRaisesRegex(ValueError,
+                               'bad_filter',
+                               self.dbapi.get_node_list,
+                               filters=filters)
+
     @mock.patch.object(timeutils, 'utcnow', autospec=True)
     def test_get_nodeinfo_list_provision(self, mock_utcnow):
         past = datetime.datetime(2000, 1, 1, 0, 0)

releasenotes/notes/add-id-and-uuid-filtering-to-sqalchemy-api.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - Fixes `bug 1749755 <https://bugs.launchpad.net/ironic/+bug/1749755>`_
+    causing timeouts to not work properly because an unsupported
+    sqalchemy filter was being used.