Skip to content

Commit

Permalink
Use an inner join on aggregate_hosts in aggregate_get_by_host
Browse files Browse the repository at this point in the history
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
  • Loading branch information
David Ripton committed Jun 11, 2013
1 parent 685228d commit 7fadc01
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
13 changes: 10 additions & 3 deletions nova/db/sqlalchemy/api.py
Expand Up @@ -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()


Expand Down
5 changes: 5 additions & 0 deletions nova/tests/db/test_db_api.py
Expand Up @@ -1435,17 +1435,22 @@ def test_aggregate_get(self):
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])
Expand Down

0 comments on commit 7fadc01

Please sign in to comment.