From 7fadc012939faf063bbe9ad3a2f35067ce16a4b2 Mon Sep 17 00:00:00 2001 From: David Ripton Date: Wed, 5 Jun 2013 15:14:31 -0400 Subject: Use an inner join on aggregate_hosts in aggregate_get_by_host Fixes bug 1187708 By default, sqlalchemy.orm.joinedload does a left outer join. But in aggregate_get_by_host, we only want to get aggregates rows with aggregate_hosts rows that match the host, and a left outer join also gave us aggregates rows with no matching aggregate_hosts row. Also improve test_aggregate_get_by_host to catch this bug. Change-Id: Ida9647af02ab58229abe3c50dc3d2c7ec9792a5e --- nova/db/sqlalchemy/api.py | 13 ++++++++++--- nova/tests/db/test_db_api.py | 5 +++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index adacc6ead..094b73fa0 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -4614,12 +4614,19 @@ def aggregate_get(context, aggregate_id): @require_admin_context def aggregate_get_by_host(context, host, key=None): - query = _aggregate_get_query(context, models.Aggregate, - models.AggregateHost.host, host) + """Return rows that match host (mandatory) and metadata key (optional). + + :param host matches host, and is required. + :param key Matches metadata key, if not None. + """ + query = model_query(context, models.Aggregate) + query = query.options(joinedload('_hosts', innerjoin=True)) + query = query.options(joinedload('_metadata')) + query = query.filter(models.AggregateHost.host == host) if key: query = query.join("_metadata").filter( - models.AggregateMetadata.key == key) + models.AggregateMetadata.key == key) return query.all() diff --git a/nova/tests/db/test_db_api.py b/nova/tests/db/test_db_api.py index 04b5bdc33..b7b6cd3ab 100644 --- a/nova/tests/db/test_db_api.py +++ b/nova/tests/db/test_db_api.py @@ -1435,17 +1435,22 @@ class AggregateDBApiTestCase(test.TestCase): def test_aggregate_get_by_host(self): ctxt = context.get_admin_context() values = {'name': 'fake_aggregate2'} + values2 = {'name': 'fake_aggregate4'} a1 = _create_aggregate_with_hosts(context=ctxt) a2 = _create_aggregate_with_hosts(context=ctxt, values=values) + # a3 has no hosts and should not be in the results. + a3 = _create_aggregate(context=ctxt, values=values2) r1 = db.aggregate_get_by_host(ctxt, 'foo.openstack.org') self.assertEqual([a1['id'], a2['id']], [x['id'] for x in r1]) def test_aggregate_get_by_host_with_key(self): ctxt = context.get_admin_context() values = {'name': 'fake_aggregate2'} + values2 = {'name': 'fake_aggregate4'} a1 = _create_aggregate_with_hosts(context=ctxt, metadata={'goodkey': 'good'}) a2 = _create_aggregate_with_hosts(context=ctxt, values=values) + a3 = _create_aggregate(context=ctxt, values=values2) # filter result by key r1 = db.aggregate_get_by_host(ctxt, 'foo.openstack.org', key='goodkey') self.assertEqual([a1['id']], [x['id'] for x in r1]) -- cgit