openstack/nova
Code Issues Proposed changes

Don't assume contents of values after aggregate_update

Browse Source 
aggregate_update() was explicitly setting values['metadata'] for the
caller. However, it has also popped availability_zone, which it
doesn't replace. Also, other _update() functions in the db api don't
do this after modifying values in place, so anything which assumed
this behaviour would be inconsistent.

The only code which did assume this behaviour was in a test. This
patch fixes the test.

Change-Id: Iaa21576655da037e6186e900234e2c29c9e3a652
This commit is contained in:
Matthew Booth
2014-11-14 10:51:31 +00:00
parent 1eaa53de5e
commit c4611b9967
2 changed files with 8 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
nova/db/sqlalchemy/api.py
View File

@@ -5413,7 +5413,6 @@ def aggregate_update(context, aggregate_id, values):
aggregate.update(values)
aggregate.save(session=session)
values['metadata'] = metadata
return aggregate_get(context, aggregate.id)
else:
raise exception.AggregateNotFound(aggregate_id=aggregate_id)

13
nova/tests/unit/db/test_db_api.py
View File

@@ -689,11 +689,13 @@ class AggregateDBApiTestCase(test.TestCase):
values = _get_fake_aggr_values()
values['metadata'] = _get_fake_aggr_metadata()
values['availability_zone'] = 'different_avail_zone'
expected_metadata = copy.deepcopy(values['metadata'])
expected_metadata['availability_zone'] = values['availability_zone']
db.aggregate_update(ctxt, result['id'], values)
expected = db.aggregate_metadata_get(ctxt, result['id'])
metadata = db.aggregate_metadata_get(ctxt, result['id'])
updated = db.aggregate_get(ctxt, result['id'])
self.assertThat(values['metadata'],
matchers.DictMatches(expected))
self.assertThat(metadata,
matchers.DictMatches(expected_metadata))
self.assertNotEqual(result['availability_zone'],
updated['availability_zone'])
@@ -703,9 +705,10 @@ class AggregateDBApiTestCase(test.TestCase):
values = _get_fake_aggr_values()
values['metadata'] = _get_fake_aggr_metadata()
values['metadata']['fake_key1'] = 'foo'
expected_metadata = copy.deepcopy(values['metadata'])
db.aggregate_update(ctxt, result['id'], values)
expected = db.aggregate_metadata_get(ctxt, result['id'])
self.assertThat(values['metadata'], matchers.DictMatches(expected))
metadata = db.aggregate_metadata_get(ctxt, result['id'])
self.assertThat(metadata, matchers.DictMatches(expected_metadata))
def test_aggregate_update_zone_with_existing_metadata(self):
ctxt = context.get_admin_context()