openstack
/
nova
Code Issues Proposed changes

Remove NovaObjectDictCompat from Aggregate 

This change removes the NovaObjectDictCompat mixin from the Aggregate
versioned object. All occurrences of key access or get have been changed
to use getattr/setattr. Because none of the objects externally-facing
functionality has changed, the object version does not need to be
updated.

There was a fake aggregate in the xen api tests that contained two
empty dictionaries for a couple of keys in the metadata. These are actually strings in the
code, so they have been changed to empty strings.

Within the tests for the aggregate API, the fake dictionaries that were
returned by the AggregateAPI stubs were structured incorrectly. Within
the object, the availability zone is stored in the metadata dictionary,
not the top-level dictionary. Also, those dictionaries were changed to
Aggregate objects, so the stubs now return true objects (similar to how
the actual code works). The result of the calls are then compared by the
object primitives.

There was usage of .items() in _marshall_aggregate() within the api
controller for aggregates, so a unit test was added to run through that
function to make sure it is transforming the aggregate object correctly.
(props to Tempest for finding that one for me).

Partially-Implements: bp rm-object-dict-compat
Change-Id: Ibcb896b2eef673bce23491161ff394bcc87aa541
This commit is contained in:
Ryan Rossiter 2016-01-06 22:50:54 +00:00
parent 4c78c2346e
commit 5831d4f89a
8 changed files with 205 additions and 125 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
nova/api/openstack/compute/aggregates.py
View File

@ -199,13 +199,20 @@ class AggregateController(wsgi.Controller):
def _marshall_aggregate(self, aggregate):
_aggregate = {}
for key, value in aggregate.items():
for key, value in self._build_aggregate_items(aggregate):
# NOTE(danms): The original API specified non-TZ-aware timestamps
if isinstance(value, datetime.datetime):
value = value.replace(tzinfo=None)
_aggregate[key] = value
return {"aggregate": _aggregate}
def _build_aggregate_items(self, aggregate):
keys = aggregate.obj_fields
for key in keys:
if (aggregate.obj_attr_is_set(key)
or key in aggregate.obj_extra_fields):
yield key, getattr(aggregate, key)
class Aggregates(extensions.V21APIExtensionBase):
"""Admin-only aggregate administration."""

9
nova/api/openstack/compute/legacy_v2/contrib/aggregates.py
View File

@ -268,13 +268,20 @@ class AggregateController(object):
def _marshall_aggregate(self, aggregate):
_aggregate = {}
for key, value in aggregate.items():
for key, value in self._build_aggregate_items(aggregate):
# NOTE(danms): The original API specified non-TZ-aware timestamps
if isinstance(value, datetime.datetime):
value = value.replace(tzinfo=None)
_aggregate[key] = value
return {"aggregate": _aggregate}
def _build_aggregate_items(self, aggregate):
keys = aggregate.obj_fields
for key in keys:
if (aggregate.obj_attr_is_set(key)
or key in aggregate.obj_extra_fields):
yield key, getattr(aggregate, key)
class Aggregates(extensions.ExtensionDescriptor):
"""Admin-only aggregate administration."""

2
nova/compute/api.py
View File

@ -3630,7 +3630,7 @@ class AggregateAPI(base.Base):
# NOTE(jogo): Send message to host to support resource pools
self.compute_rpcapi.add_aggregate_host(context,
aggregate=aggregate, host_param=host_name, host=host_name)
aggregate_payload.update({'name': aggregate['name']})
aggregate_payload.update({'name': aggregate.name})
compute_utils.notify_about_aggregate_update(context,
"addhost.end",
aggregate_payload)

6
nova/objects/aggregate.py
View File

@ -20,10 +20,8 @@ from nova.objects import base
from nova.objects import fields
# TODO(berrange): Remove NovaObjectDictCompat
@base.NovaObjectRegistry.register
class Aggregate(base.NovaPersistentObject, base.NovaObject,
base.NovaObjectDictCompat):
class Aggregate(base.NovaPersistentObject, base.NovaObject):
# Version 1.0: Initial version
# Version 1.1: String attributes updated to support unicode
VERSION = '1.1'
@ -44,7 +42,7 @@ class Aggregate(base.NovaPersistentObject, base.NovaObject,
db_key = 'metadetails'
else:
db_key = key
aggregate[key] = db_aggregate[db_key]
setattr(aggregate, key, db_aggregate[db_key])
aggregate._context = context
aggregate.obj_reset_changes()
return aggregate

111
nova/tests/unit/api/openstack/compute/test_aggregates.py
View File

@ -24,24 +24,60 @@ from nova.api.openstack.compute.legacy_v2.contrib import aggregates \
from nova.compute import api as compute_api
from nova import context
from nova import exception
from nova import objects
from nova import test
from nova.tests.unit.api.openstack import fakes
from nova.tests.unit import matchers
def _make_agg_obj(agg_dict):
return objects.Aggregate(**agg_dict)
def _make_agg_list(agg_list):
return objects.AggregateList(objects=[_make_agg_obj(a) for a in agg_list])
def _transform_aggregate_az(agg_dict):
# the Aggregate object looks for availability_zone within metadata,
# so if availability_zone is in the top-level dict, move it down into
# metadata. We also have to delete the key from the top-level dict because
# availability_zone is a read-only property on the Aggregate object
md = agg_dict.get('metadata', {})
if 'availability_zone' in agg_dict:
md['availability_zone'] = agg_dict['availability_zone']
del agg_dict['availability_zone']
agg_dict['metadata'] = md
return agg_dict
def _transform_aggregate_list_azs(agg_list):
for agg_dict in agg_list:
yield _transform_aggregate_az(agg_dict)
AGGREGATE_LIST = [
{"name": "aggregate1", "id": "1", "availability_zone": "nova1"},
{"name": "aggregate2", "id": "2", "availability_zone": "nova1"},
{"name": "aggregate3", "id": "3", "availability_zone": "nova2"},
{"name": "aggregate1", "id": "4", "availability_zone": "nova1"}]
{"name": "aggregate1", "id": "1",
"metadata": {"availability_zone": "nova1"}},
{"name": "aggregate2", "id": "2",
"metadata": {"availability_zone": "nova1"}},
{"name": "aggregate3", "id": "3",
"metadata": {"availability_zone": "nova2"}},
{"name": "aggregate1", "id": "4",
"metadata": {"availability_zone": "nova1"}}]
AGGREGATE_LIST = _make_agg_list(AGGREGATE_LIST)
AGGREGATE = {"name": "aggregate1",
"id": "1",
"availability_zone": "nova1",
"metadata": {"foo": "bar"},
"hosts": ["host1, host2"]}
"metadata": {"foo": "bar",
"availability_zone": "nova1"},
"hosts": ["host1", "host2"]}
AGGREGATE = _make_agg_obj(AGGREGATE)
FORMATTED_AGGREGATE = {"name": "aggregate1",
"id": "1",
"availability_zone": "nova1"}
"metadata": {"availability_zone": "nova1"}}
FORMATTED_AGGREGATE = _make_agg_obj(FORMATTED_AGGREGATE)
class FakeRequest(object):
@ -76,8 +112,8 @@ class AggregateTestCaseV21(test.NoDBTestCase):
stub_list_aggregates)
result = self.controller.index(self.req)
self.assertEqual(AGGREGATE_LIST, result["aggregates"])
result = _transform_aggregate_list_azs(result['aggregates'])
self._assert_agg_data(AGGREGATE_LIST, _make_agg_list(result))
def test_index_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
@ -96,7 +132,8 @@ class AggregateTestCaseV21(test.NoDBTestCase):
result = self.controller.create(self.req, body={"aggregate":
{"name": "test",
"availability_zone": "nova1"}})
self.assertEqual(FORMATTED_AGGREGATE, result["aggregate"])
result = _transform_aggregate_az(result['aggregate'])
self._assert_agg_data(FORMATTED_AGGREGATE, _make_agg_obj(result))
def test_create_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
@ -175,7 +212,9 @@ class AggregateTestCaseV21(test.NoDBTestCase):
result = self.controller.create(self.req,
body={"aggregate": {"name": "test"}})
self.assertEqual(FORMATTED_AGGREGATE, result["aggregate"])
result = _transform_aggregate_az(result['aggregate'])
self._assert_agg_data(FORMATTED_AGGREGATE, _make_agg_obj(result))
def test_create_with_null_name(self):
self.assertRaises(self.bad_request, self.controller.create,
@ -238,8 +277,8 @@ class AggregateTestCaseV21(test.NoDBTestCase):
stub_get_aggregate)
aggregate = self.controller.show(self.req, "1")
self.assertEqual(AGGREGATE, aggregate["aggregate"])
aggregate = _transform_aggregate_az(aggregate['aggregate'])
self._assert_agg_data(AGGREGATE, _make_agg_obj(aggregate))
def test_show_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
@ -270,7 +309,8 @@ class AggregateTestCaseV21(test.NoDBTestCase):
result = self.controller.update(self.req, "1", body=body)
self.assertEqual(AGGREGATE, result["aggregate"])
result = _transform_aggregate_az(result['aggregate'])
self._assert_agg_data(AGGREGATE, _make_agg_obj(result))
def test_update_no_admin(self):
body = {"aggregate": {"availability_zone": "nova"}}
@ -288,7 +328,8 @@ class AggregateTestCaseV21(test.NoDBTestCase):
result = self.controller.update(self.req, "1", body=body)
self.assertEqual(AGGREGATE, result["aggregate"])
result = _transform_aggregate_az(result['aggregate'])
self._assert_agg_data(AGGREGATE, _make_agg_obj(result))
def test_update_with_only_availability_zone(self):
body = {"aggregate": {"availability_zone": "nova1"}}
@ -298,7 +339,9 @@ class AggregateTestCaseV21(test.NoDBTestCase):
self.stubs.Set(self.controller.api, "update_aggregate",
stub_update_aggregate)
result = self.controller.update(self.req, "1", body=body)
self.assertEqual(AGGREGATE, result["aggregate"])
result = _transform_aggregate_az(result['aggregate'])
self._assert_agg_data(AGGREGATE, _make_agg_obj(result))
def test_update_with_no_updates(self):
test_metadata = {"aggregate": {}}
@ -384,7 +427,8 @@ class AggregateTestCaseV21(test.NoDBTestCase):
body={"add_host": {"host":
"host1"}})
self.assertEqual(aggregate["aggregate"], AGGREGATE)
aggregate = _transform_aggregate_az(aggregate['aggregate'])
self._assert_agg_data(AGGREGATE, aggregate)
def test_add_host_no_admin(self):
self.assertRaises(exception.PolicyNotAuthorized,
@ -458,7 +502,8 @@ class AggregateTestCaseV21(test.NoDBTestCase):
self.assertEqual("1", aggregate, "aggregate")
self.assertEqual("host1", host, "host")
stub_remove_host_from_aggregate.called = True
return {}
# at minimum, metadata is always set to {} in the api
return _make_agg_obj({'metadata': {}})
self.stubs.Set(self.controller.api,
"remove_host_from_aggregate",
stub_remove_host_from_aggregate)
@ -545,7 +590,8 @@ class AggregateTestCaseV21(test.NoDBTestCase):
result = eval(self.set_metadata)(self.req, "1", body=body)
self.assertEqual(AGGREGATE, result["aggregate"])
result = _transform_aggregate_az(result['aggregate'])
self._assert_agg_data(AGGREGATE, _make_agg_obj(result))
def test_set_metadata_delete(self):
body = {"set_metadata": {"metadata": {"foo": None}}}
@ -555,7 +601,8 @@ class AggregateTestCaseV21(test.NoDBTestCase):
mocked.return_value = AGGREGATE
result = eval(self.set_metadata)(self.req, "1", body=body)
self.assertEqual(AGGREGATE, result["aggregate"])
result = _transform_aggregate_az(result['aggregate'])
self._assert_agg_data(AGGREGATE, _make_agg_obj(result))
mocked.assert_called_once_with(self.context, "1",
body["set_metadata"]["metadata"])
@ -646,6 +693,28 @@ class AggregateTestCaseV21(test.NoDBTestCase):
self.controller.delete,
self.req, "agg1")
def test_marshall_aggregate(self):
# _marshall_aggregate() just basically turns the aggregate returned
# from the AggregateAPI into a dict, so this tests that transform.
# We would expect the dictionary that comes out is the same one
# that we pump into the aggregate object in the first place
agg = {'name': 'aggregate1',
'id': 1,
'metadata': {'foo': 'bar', 'availability_zone': 'nova'},
'hosts': ['host1', 'host2']}
agg_obj = _make_agg_obj(agg)
marshalled_agg = self.controller._marshall_aggregate(agg_obj)
# _marshall_aggregate() puts all fields and obj_extra_fields in the
# top-level dict, so we need to put availability_zone at the top also
agg['availability_zone'] = 'nova'
self.assertEqual(agg, marshalled_agg['aggregate'])
def _assert_agg_data(self, expected, actual):
expected_data = expected.obj_to_primitive()['nova_object.data']
actual_data = expected.obj_to_primitive()['nova_object.data']
self.assertEqual(expected_data, actual_data)
class AggregateTestCaseV2(AggregateTestCaseV21):
add_host = 'self.controller.action'

130
nova/tests/unit/compute/test_compute.py
View File

@ -344,7 +344,7 @@ class BaseTestCase(test.TestCase):
def _init_aggregate_with_host(self, aggr, aggr_name, zone, host):
if not aggr:
aggr = self.api.create_aggregate(self.context, aggr_name, zone)
aggr = self.api.add_host_to_aggregate(self.context, aggr['id'], host)
aggr = self.api.add_host_to_aggregate(self.context, aggr.id, host)
return aggr
@ -10247,11 +10247,11 @@ class ComputeAPIAggrTestCase(BaseTestCase):
# Ensure we can create an aggregate without an availability zone
aggr = self.api.create_aggregate(self.context, 'fake_aggregate',
None)
self.api.delete_aggregate(self.context, aggr['id'])
self.api.delete_aggregate(self.context, aggr.id)
db.aggregate_get(self.context.elevated(read_deleted='yes'),
aggr['id'])
aggr.id)
self.assertRaises(exception.AggregateNotFound,
self.api.delete_aggregate, self.context, aggr['id'])
self.api.delete_aggregate, self.context, aggr.id)
def test_check_az_for_aggregate(self):
# Ensure all conflict hosts can be returned
@ -10268,14 +10268,14 @@ class ComputeAPIAggrTestCase(BaseTestCase):
metadata = {'availability_zone': 'another_zone'}
self.assertRaises(exception.InvalidAggregateActionUpdate,
self.api.update_aggregate,
self.context, aggr2['id'], metadata)
self.context, aggr2.id, metadata)
def test_update_aggregate(self):
# Ensure metadata can be updated.
aggr = self.api.create_aggregate(self.context, 'fake_aggregate',
'fake_zone')
fake_notifier.NOTIFICATIONS = []
self.api.update_aggregate(self.context, aggr['id'],
self.api.update_aggregate(self.context, aggr.id,
{'name': 'new_fake_aggregate'})
self.assertIsNone(availability_zones._get_cache().get('cache'))
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2)
@ -10299,7 +10299,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
fake_host)
metadata = {'name': 'new_fake_aggregate'}
fake_notifier.NOTIFICATIONS = []
self.api.update_aggregate(self.context, aggr2['id'], metadata)
self.api.update_aggregate(self.context, aggr2.id, metadata)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2)
msg = fake_notifier.NOTIFICATIONS[0]
self.assertEqual(msg.event_type,
@ -10321,7 +10321,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
fake_host)
metadata = {'availability_zone': 'new_fake_zone'}
fake_notifier.NOTIFICATIONS = []
self.api.update_aggregate(self.context, aggr1['id'], metadata)
self.api.update_aggregate(self.context, aggr1.id, metadata)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2)
msg = fake_notifier.NOTIFICATIONS[0]
self.assertEqual(msg.event_type,
@ -10344,12 +10344,12 @@ class ComputeAPIAggrTestCase(BaseTestCase):
metadata = {'availability_zone': 'another_zone'}
self.assertRaises(exception.InvalidAggregateActionUpdate,
self.api.update_aggregate,
self.context, aggr2['id'], metadata)
self.context, aggr2.id, metadata)
fake_host2 = values[0][1][1]
aggr3 = self._init_aggregate_with_host(None, 'fake_aggregate3',
None, fake_host2)
metadata = {'availability_zone': fake_zone}
self.api.update_aggregate(self.context, aggr3['id'], metadata)
self.api.update_aggregate(self.context, aggr3.id, metadata)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 15)
msg = fake_notifier.NOTIFICATIONS[13]
self.assertEqual(msg.event_type,
@ -10372,7 +10372,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
metadata = {'availability_zone': 'another_zone'}
self.assertRaises(exception.InvalidAggregateActionUpdate,
self.api.update_aggregate,
self.context, aggr2['id'], metadata)
self.context, aggr2.id, metadata)
def test_update_aggregate_metadata(self):
# Ensure metadata can be updated.
@ -10383,7 +10383,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
'availability_zone': 'fake_zone'}
fake_notifier.NOTIFICATIONS = []
availability_zones._get_cache().add('fake_key', 'fake_value')
aggr = self.api.update_aggregate_metadata(self.context, aggr['id'],
aggr = self.api.update_aggregate_metadata(self.context, aggr.id,
metadata)
self.assertIsNone(availability_zones._get_cache().get('fake_key'))
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2)
@ -10399,7 +10399,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
'foo_key2': 'foo_value2',
'availability_zone': 'fake_zone'}
expected = self.api.update_aggregate_metadata(self.context,
aggr['id'], metadata)
aggr.id, metadata)
self.assertEqual(2, len(fake_notifier.NOTIFICATIONS))
msg = fake_notifier.NOTIFICATIONS[0]
self.assertEqual('aggregate.updatemetadata.start', msg.event_type)
@ -10407,7 +10407,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
msg = fake_notifier.NOTIFICATIONS[1]
self.assertEqual('aggregate.updatemetadata.end', msg.event_type)
self.assertEqual(expected_payload_meta_data, msg.payload['meta_data'])
self.assertThat(expected['metadata'],
self.assertThat(expected.metadata,
matchers.DictMatches({'availability_zone': 'fake_zone',
'foo_key2': 'foo_value2'}))
@ -10424,7 +10424,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
fake_host)
metadata = {'foo_key2': 'foo_value3'}
fake_notifier.NOTIFICATIONS = []
aggr2 = self.api.update_aggregate_metadata(self.context, aggr2['id'],
aggr2 = self.api.update_aggregate_metadata(self.context, aggr2.id,
metadata)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2)
msg = fake_notifier.NOTIFICATIONS[0]
@ -10433,7 +10433,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
msg = fake_notifier.NOTIFICATIONS[1]
self.assertEqual(msg.event_type,
'aggregate.updatemetadata.end')
self.assertThat(aggr2['metadata'],
self.assertThat(aggr2.metadata,
matchers.DictMatches({'foo_key2': 'foo_value3'}))
def test_update_aggregate_metadata_az_change(self):
@ -10449,7 +10449,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
fake_host)
metadata = {'availability_zone': 'new_fake_zone'}
fake_notifier.NOTIFICATIONS = []
self.api.update_aggregate_metadata(self.context, aggr1['id'], metadata)
self.api.update_aggregate_metadata(self.context, aggr1.id, metadata)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2)
msg = fake_notifier.NOTIFICATIONS[0]
self.assertEqual(msg.event_type,
@ -10465,13 +10465,13 @@ class ComputeAPIAggrTestCase(BaseTestCase):
'fake_zone')
metadata = {'foo_key1': 'foo_value1'}
aggr = self.api.update_aggregate_metadata(self.context,
aggr['id'],
aggr.id,
metadata)
metadata = {'availability_zone': 'new_fake_zone'}
aggr = self.api.update_aggregate(self.context,
aggr['id'],
aggr.id,
metadata)
self.assertThat(aggr['metadata'], matchers.DictMatches(
self.assertThat(aggr.metadata, matchers.DictMatches(
{'availability_zone': 'new_fake_zone', 'foo_key1': 'foo_value1'}))
def test_update_aggregate_metadata_az_fails(self):
@ -10488,11 +10488,11 @@ class ComputeAPIAggrTestCase(BaseTestCase):
metadata = {'availability_zone': 'another_zone'}
self.assertRaises(exception.InvalidAggregateActionUpdateMeta,
self.api.update_aggregate_metadata,
self.context, aggr2['id'], metadata)
self.context, aggr2.id, metadata)
aggr3 = self._init_aggregate_with_host(None, 'fake_aggregate3',
None, fake_host)
metadata = {'availability_zone': fake_zone}
self.api.update_aggregate_metadata(self.context, aggr3['id'], metadata)
self.api.update_aggregate_metadata(self.context, aggr3.id, metadata)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 15)
msg = fake_notifier.NOTIFICATIONS[13]
self.assertEqual(msg.event_type,
@ -10514,7 +10514,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
self.assertEqual(msg.event_type,
'aggregate.create.end')
fake_notifier.NOTIFICATIONS = []
self.api.delete_aggregate(self.context, aggr['id'])
self.api.delete_aggregate(self.context, aggr.id)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2)
msg = fake_notifier.NOTIFICATIONS[0]
self.assertEqual(msg.event_type,
@ -10523,9 +10523,9 @@ class ComputeAPIAggrTestCase(BaseTestCase):
self.assertEqual(msg.event_type,
'aggregate.delete.end')
db.aggregate_get(self.context.elevated(read_deleted='yes'),
aggr['id'])
aggr.id)
self.assertRaises(exception.AggregateNotFound,
self.api.delete_aggregate, self.context, aggr['id'])
self.api.delete_aggregate, self.context, aggr.id)
def test_delete_non_empty_aggregate(self):
# Ensure InvalidAggregateAction is raised when non empty aggregate.
@ -10533,9 +10533,9 @@ class ComputeAPIAggrTestCase(BaseTestCase):
[['fake_availability_zone', ['fake_host']]])
aggr = self.api.create_aggregate(self.context, 'fake_aggregate',
'fake_availability_zone')
self.api.add_host_to_aggregate(self.context, aggr['id'], 'fake_host')
self.api.add_host_to_aggregate(self.context, aggr.id, 'fake_host')
self.assertRaises(exception.InvalidAggregateActionDelete,
self.api.delete_aggregate, self.context, aggr['id'])
self.api.delete_aggregate, self.context, aggr.id)
def test_add_host_to_aggregate(self):
# Ensure we can add a host to an aggregate.
@ -10546,7 +10546,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
'fake_aggregate', fake_zone)
def fake_add_aggregate_host(*args, **kwargs):
hosts = kwargs["aggregate"]["hosts"]
hosts = kwargs["aggregate"].hosts
self.assertIn(fake_host, hosts)
self.stubs.Set(self.api.compute_rpcapi, 'add_aggregate_host',
@ -10561,7 +10561,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
fake_notifier.NOTIFICATIONS = []
aggr = self.api.add_host_to_aggregate(self.context,
aggr['id'], fake_host)
aggr.id, fake_host)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2)
msg = fake_notifier.NOTIFICATIONS[0]
self.assertEqual(msg.event_type,
@ -10569,7 +10569,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
msg = fake_notifier.NOTIFICATIONS[1]
self.assertEqual(msg.event_type,
'aggregate.addhost.end')
self.assertEqual(len(aggr['hosts']), 1)
self.assertEqual(len(aggr.hosts), 1)
def test_add_host_to_aggr_with_no_az(self):
values = _create_service_entries(self.context)
@ -10577,15 +10577,15 @@ class ComputeAPIAggrTestCase(BaseTestCase):
fake_host = values[0][1][0]
aggr = self.api.create_aggregate(self.context,
'fake_aggregate', fake_zone)
aggr = self.api.add_host_to_aggregate(self.context, aggr['id'],
aggr = self.api.add_host_to_aggregate(self.context, aggr.id,
fake_host)
aggr_no_az = self.api.create_aggregate(self.context, 'fake_aggregate2',
None)
aggr_no_az = self.api.add_host_to_aggregate(self.context,
aggr_no_az['id'],
aggr_no_az.id,
fake_host)
self.assertIn(fake_host, aggr['hosts'])
self.assertIn(fake_host, aggr_no_az['hosts'])
self.assertIn(fake_host, aggr.hosts)
self.assertIn(fake_host, aggr_no_az.hosts)
def test_add_host_to_multi_az(self):
# Ensure we can't add a host to different availability zone
@ -10595,14 +10595,14 @@ class ComputeAPIAggrTestCase(BaseTestCase):
aggr = self.api.create_aggregate(self.context,
'fake_aggregate', fake_zone)
aggr = self.api.add_host_to_aggregate(self.context,
aggr['id'], fake_host)
self.assertEqual(len(aggr['hosts']), 1)
aggr.id, fake_host)
self.assertEqual(len(aggr.hosts), 1)
fake_zone2 = "another_zone"
aggr2 = self.api.create_aggregate(self.context,
'fake_aggregate2', fake_zone2)
self.assertRaises(exception.InvalidAggregateActionAdd,
self.api.add_host_to_aggregate,
self.context, aggr2['id'], fake_host)
self.context, aggr2.id, fake_host)
def test_add_host_to_multi_az_with_nova_agg(self):
# Ensure we can't add a host if already existing in an agg with AZ set
@ -10613,14 +10613,14 @@ class ComputeAPIAggrTestCase(BaseTestCase):
'fake_aggregate',
CONF.default_availability_zone)
aggr = self.api.add_host_to_aggregate(self.context,
aggr['id'], fake_host)
self.assertEqual(len(aggr['hosts']), 1)
aggr.id, fake_host)
self.assertEqual(len(aggr.hosts), 1)
fake_zone2 = "another_zone"
aggr2 = self.api.create_aggregate(self.context,
'fake_aggregate2', fake_zone2)
self.assertRaises(exception.InvalidAggregateActionAdd,
self.api.add_host_to_aggregate,
self.context, aggr2['id'], fake_host)
self.context, aggr2.id, fake_host)
def test_add_host_to_aggregate_multiple(self):
# Ensure we can add multiple hosts to an aggregate.
@ -10630,8 +10630,8 @@ class ComputeAPIAggrTestCase(BaseTestCase):
'fake_aggregate', fake_zone)
for host in values[0][1]:
aggr = self.api.add_host_to_aggregate(self.context,
aggr['id'], host)
self.assertEqual(len(aggr['hosts']), len(values[0][1]))
aggr.id, host)
self.assertEqual(len(aggr.hosts), len(values[0][1]))
def test_add_host_to_aggregate_raise_not_found(self):
# Ensure ComputeHostNotFound is raised when adding invalid host.
@ -10640,7 +10640,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
fake_notifier.NOTIFICATIONS = []
self.assertRaises(exception.ComputeHostNotFound,
self.api.add_host_to_aggregate,
self.context, aggr['id'], 'invalid_host')
self.context, aggr.id, 'invalid_host')
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2)
self.assertEqual(fake_notifier.NOTIFICATIONS[1].publisher_id,
'compute.fake-mini')
@ -10653,11 +10653,11 @@ class ComputeAPIAggrTestCase(BaseTestCase):
'fake_aggregate', fake_zone)
for host in values[0][1]:
aggr = self.api.add_host_to_aggregate(self.context,
aggr['id'], host)
aggr.id, host)
host_to_remove = values[0][1][0]
def fake_remove_aggregate_host(*args, **kwargs):
hosts = kwargs["aggregate"]["hosts"]
hosts = kwargs["aggregate"].hosts
self.assertNotIn(host_to_remove, hosts)
self.stubs.Set(self.api.compute_rpcapi, 'remove_aggregate_host',
@ -10671,7 +10671,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
fake_notifier.NOTIFICATIONS = []
expected = self.api.remove_host_from_aggregate(self.context,
aggr['id'],
aggr.id,
host_to_remove)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 2)
msg = fake_notifier.NOTIFICATIONS[0]
@ -10680,7 +10680,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
msg = fake_notifier.NOTIFICATIONS[1]
self.assertEqual(msg.event_type,
'aggregate.removehost.end')
self.assertEqual(len(aggr['hosts']) - 1, len(expected['hosts']))
self.assertEqual(len(aggr.hosts) - 1, len(expected.hosts))
def test_remove_host_from_aggregate_raise_not_found(self):
# Ensure ComputeHostNotFound is raised when removing invalid host.
@ -10689,7 +10689,7 @@ class ComputeAPIAggrTestCase(BaseTestCase):
'fake_zone')
self.assertRaises(exception.ComputeHostNotFound,
self.api.remove_host_from_aggregate,
self.context, aggr['id'], 'invalid_host')
self.context, aggr.id, 'invalid_host')
def test_aggregate_list(self):
aggregate = self.api.create_aggregate(self.context,
@ -10700,28 +10700,26 @@ class ComputeAPIAggrTestCase(BaseTestCase):
meta_aggregate = self.api.create_aggregate(self.context,
'fake_aggregate2',
'fake_zone2')
self.api.update_aggregate_metadata(self.context, meta_aggregate['id'],
self.api.update_aggregate_metadata(self.context, meta_aggregate.id,
metadata)
aggregate_list = self.api.get_aggregate_list(self.context)
self.assertIn(aggregate['id'],
map(lambda x: x['id'], aggregate_list))
self.assertIn(meta_aggregate['id'],
map(lambda x: x['id'], aggregate_list))
self.assertIn(aggregate.id,
map(lambda x: x.id, aggregate_list))
self.assertIn(meta_aggregate.id,
map(lambda x: x.id, aggregate_list))
self.assertIn('fake_aggregate',
map(lambda x: x['name'], aggregate_list))
map(lambda x: x.name, aggregate_list))
self.assertIn('fake_aggregate2',
map(lambda x: x['name'], aggregate_list))
map(lambda x: x.name, aggregate_list))
self.assertIn('fake_zone',
map(lambda x: x['availability_zone'], aggregate_list))
map(lambda x: x.availability_zone, aggregate_list))
self.assertIn('fake_zone2',
map(lambda x: x['availability_zone'], aggregate_list))
test_meta_aggregate = aggregate_list[1]
self.assertIn('foo_key1', test_meta_aggregate.get('metadata'))
self.assertIn('foo_key2', test_meta_aggregate.get('metadata'))
self.assertEqual('foo_value1',
test_meta_aggregate.get('metadata')['foo_key1'])
self.assertEqual('foo_value2',
test_meta_aggregate.get('metadata')['foo_key2'])
map(lambda x: x.availability_zone, aggregate_list))
test_agg_meta = getattr(aggregate_list[1], 'metadata', None)
self.assertIn('foo_key1', test_agg_meta)
self.assertIn('foo_key2', test_agg_meta)
self.assertEqual('foo_value1', test_agg_meta['foo_key1'])
self.assertEqual('foo_value2', test_agg_meta['foo_key2'])
def test_aggregate_list_with_hosts(self):
values = _create_service_entries(self.context)
@ -10729,11 +10727,11 @@ class ComputeAPIAggrTestCase(BaseTestCase):
host_aggregate = self.api.create_aggregate(self.context,
'fake_aggregate',
fake_zone)
self.api.add_host_to_aggregate(self.context, host_aggregate['id'],
self.api.add_host_to_aggregate(self.context, host_aggregate.id,
values[0][1][0])
aggregate_list = self.api.get_aggregate_list(self.context)
aggregate = aggregate_list[0]
self.assertIn(values[0][1][0], aggregate.get('hosts'))
self.assertIn(values[0][1][0], getattr(aggregate, 'hosts', None))
class ComputeAPIAggrCallsSchedulerTestCase(test.NoDBTestCase):

19
nova/tests/unit/virt/xenapi/test_xenapi.py
View File

@ -3059,7 +3059,7 @@ class XenAPIAggregateTestCase(stubs.XenAPITestBase):
aggregate = self._aggregate_setup()
self.conn._pool.add_to_aggregate(self.context, aggregate, "host")
result = db.aggregate_get(self.context, aggregate['id'])
result = db.aggregate_get(self.context, aggregate.id)
self.assertTrue(fake_init_pool.called)
self.assertThat(self.fake_metadata,
matchers.DictMatches(result['metadetails']))
@ -3138,7 +3138,7 @@ class XenAPIAggregateTestCase(stubs.XenAPITestBase):
aggregate = self._aggregate_setup(metadata=self.fake_metadata)
self.conn._pool.remove_from_aggregate(self.context, aggregate, "host")
result = db.aggregate_get(self.context, aggregate['id'])
result = db.aggregate_get(self.context, aggregate.id)
self.assertTrue(fake_clear_pool.called)
self.assertThat({'availability_zone': 'fake_zone',
pool_states.POOL_FLAG: 'XenAPI',
@ -3210,16 +3210,16 @@ class XenAPIAggregateTestCase(stubs.XenAPITestBase):
# let's mock the fact that the aggregate is ready!
metadata = {pool_states.POOL_FLAG: "XenAPI",
pool_states.KEY: pool_states.ACTIVE}
db.aggregate_metadata_add(self.context, aggr['id'], metadata)
db.aggregate_metadata_add(self.context, aggr.id, metadata)
for aggregate_host in values[fake_zone]:
aggr = self.api.add_host_to_aggregate(self.context,
aggr['id'], aggregate_host)
aggr.id, aggregate_host)
# let's mock the fact that the aggregate is in error!
expected = self.api.remove_host_from_aggregate(self.context,
aggr['id'],
aggr.id,
values[fake_zone][0])
self.assertEqual(len(aggr['hosts']) - 1, len(expected['hosts']))
self.assertEqual(expected['metadata'][pool_states.KEY],
self.assertEqual(len(aggr.hosts) - 1, len(expected.hosts))
self.assertEqual(expected.metadata[pool_states.KEY],
pool_states.ACTIVE)
def test_remove_host_from_aggregate_invalid_dismissed_status(self):
@ -3309,10 +3309,11 @@ class HypervisorPoolTestCase(test.NoDBTestCase):
'hosts': [],
'metadata': {
'master_compute': 'master',
pool_states.POOL_FLAG: {},
pool_states.KEY: {}
pool_states.POOL_FLAG: '',
pool_states.KEY: ''
}
}
fake_aggregate = objects.Aggregate(**fake_aggregate)
def test_slave_asks_master_to_add_slave_to_pool(self):
slave = ResourcePoolWithStubs()

44
nova/virt/xenapi/pool.py
View File

@ -64,27 +64,27 @@ class ResourcePool(object):
except Exception:
LOG.exception(_LE('Aggregate %(aggregate_id)s: unrecoverable '
'state during operation on %(host)s'),
{'aggregate_id': aggregate['id'], 'host': host})
{'aggregate_id': aggregate.id, 'host': host})
def add_to_aggregate(self, context, aggregate, host, slave_info=None):
"""Add a compute host to an aggregate."""
if not pool_states.is_hv_pool(aggregate['metadata']):
if not pool_states.is_hv_pool(aggregate.metadata):
return
invalid = {pool_states.CHANGING: _('setup in progress'),
pool_states.DISMISSED: _('aggregate deleted'),
pool_states.ERROR: _('aggregate in error')}
if (aggregate['metadata'][pool_states.KEY] in invalid.keys()):
if (aggregate.metadata[pool_states.KEY] in invalid.keys()):
raise exception.InvalidAggregateActionAdd(
aggregate_id=aggregate['id'],
reason=invalid[aggregate['metadata'][pool_states.KEY]])
aggregate_id=aggregate.id,
reason=invalid[aggregate.metadata[pool_states.KEY]])
if (aggregate['metadata'][pool_states.KEY] == pool_states.CREATED):
if (aggregate.metadata[pool_states.KEY] == pool_states.CREATED):
aggregate.update_metadata({pool_states.KEY: pool_states.CHANGING})
if len(aggregate['hosts']) == 1:
if len(aggregate.hosts) == 1:
# this is the first host of the pool -> make it master
self._init_pool(aggregate['id'], aggregate['name'])
self._init_pool(aggregate.id, aggregate.name)
# save metadata so that we can find the master again
metadata = {'master_compute': host,
host: self._host_uuid,
@ -93,12 +93,12 @@ class ResourcePool(object):
else:
# the pool is already up and running, we need to figure out
# whether we can serve the request from this host or not.
master_compute = aggregate['metadata']['master_compute']
master_compute = aggregate.metadata['master_compute']
if master_compute == CONF.host and master_compute != host:
# this is the master -> do a pool-join
# To this aim, nova compute on the slave has to go down.
# NOTE: it is assumed that ONLY nova compute is running now
self._join_slave(aggregate['id'], host,
self._join_slave(aggregate.id, host,
slave_info.get('compute_uuid'),
slave_info.get('url'), slave_info.get('user'),
slave_info.get('passwd'))
@ -115,47 +115,47 @@ class ResourcePool(object):
def remove_from_aggregate(self, context, aggregate, host, slave_info=None):
"""Remove a compute host from an aggregate."""
slave_info = slave_info or dict()
if not pool_states.is_hv_pool(aggregate['metadata']):
if not pool_states.is_hv_pool(aggregate.metadata):
return
invalid = {pool_states.CREATED: _('no hosts to remove'),
pool_states.CHANGING: _('setup in progress'),
pool_states.DISMISSED: _('aggregate deleted')}
if aggregate['metadata'][pool_states.KEY] in invalid.keys():
if aggregate.metadata[pool_states.KEY] in invalid.keys():
raise exception.InvalidAggregateActionDelete(
aggregate_id=aggregate['id'],
reason=invalid[aggregate['metadata'][pool_states.KEY]])
aggregate_id=aggregate.id,
reason=invalid[aggregate.metadata[pool_states.KEY]])
master_compute = aggregate['metadata']['master_compute']
master_compute = aggregate.metadata['master_compute']
if master_compute == CONF.host and master_compute != host:
# this is the master -> instruct it to eject a host from the pool
host_uuid = aggregate['metadata'][host]
self._eject_slave(aggregate['id'],
host_uuid = aggregate.metadata[host]
self._eject_slave(aggregate.id,
slave_info.get('compute_uuid'), host_uuid)
aggregate.update_metadata({host: None})
elif master_compute == host:
# Remove master from its own pool -> destroy pool only if the
# master is on its own, otherwise raise fault. Destroying a
# pool made only by master is fictional
if len(aggregate['hosts']) > 1:
if len(aggregate.hosts) > 1:
# NOTE: this could be avoided by doing a master
# re-election, but this is simpler for now.
raise exception.InvalidAggregateActionDelete(
aggregate_id=aggregate['id'],
aggregate_id=aggregate.id,
reason=_('Unable to eject %s '
'from the pool; pool not empty')
% host)
self._clear_pool(aggregate['id'])
self._clear_pool(aggregate.id)
aggregate.update_metadata({'master_compute': None, host: None})
elif master_compute and master_compute != host:
# A master exists -> forward pool-eject request to master
slave_info = self._create_slave_info()
self.compute_rpcapi.remove_aggregate_host(
context, aggregate['id'], host, master_compute, slave_info)
context, aggregate.id, host, master_compute, slave_info)
else:
# this shouldn't have happened
raise exception.AggregateError(aggregate_id=aggregate['id'],
raise exception.AggregateError(aggregate_id=aggregate.id,
action='remove_from_aggregate',
reason=_('Unable to eject %s '
'from the pool; No master found')