From 366a44a1bb4e235f6382af02962ba2a192cffa4d Mon Sep 17 00:00:00 2001 From: Ryan Bridges Date: Thu, 15 Feb 2018 09:01:32 -0800 Subject: [PATCH] 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 | 27 ++++++++++--------- ironic/tests/unit/db/test_nodes.py | 20 ++++++++++++++ ...d-and-uuid-filtering-to-sqalchemy-api.yaml | 5 ++++ 3 files changed, 40 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/add-id-and-uuid-filtering-to-sqalchemy-api.yaml diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index 6327c1aa2f..cf686c8f35 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/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 diff --git a/ironic/tests/unit/db/test_nodes.py b/ironic/tests/unit/db/test_nodes.py index 4cab10440c..6bf6a87e7f 100644 --- a/ironic/tests/unit/db/test_nodes.py +++ b/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) diff --git a/releasenotes/notes/add-id-and-uuid-filtering-to-sqalchemy-api.yaml b/releasenotes/notes/add-id-and-uuid-filtering-to-sqalchemy-api.yaml new file mode 100644 index 0000000000..d1118e7f31 --- /dev/null +++ b/releasenotes/notes/add-id-and-uuid-filtering-to-sqalchemy-api.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - Fixes `bug 1749755 `_ + causing timeouts to not work properly because an unsupported + sqalchemy filter was being used.