openstack/nova
Code Issues Proposed changes

Merge "Fix AvailabilityZone check for hosts in multiple aggregates"

Browse Source
This commit is contained in:
Jenkins
2014-05-21 10:55:08 +00:00
committed by Gerrit Code Review
parent ac5e992d14 0ad5a64dc9
commit 839fe777e2
2 changed files with 89 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

85
nova/compute/api.py
View File

@@ -3287,32 +3287,14 @@ class AggregateAPI(base.Base):
aggregates = aggregate_obj.AggregateList.get_all(context)
return [self._reformat_aggregate_info(agg) for agg in aggregates]
def is_safe_to_update_az(self, context, aggregate, metadata,
action_name):
"""Determine if updates alter an aggregate's availability zone."""
if 'availability_zone' in metadata:
aggregate_az = aggregate.metadata.get("availability_zone")
for host in aggregate.hosts:
host_az = availability_zones.get_host_availability_zone(
context, host)
if (host_az and host_az != metadata["availability_zone"]
and host_az != CONF.default_availability_zone and
host_az != aggregate_az):
msg = _("This aggregate contains hosts in"
" an existing availability zone")
raise exception.InvalidAggregateAction(
action=action_name,
aggregate_id=aggregate.id,
reason=msg)
@wrap_exception()
def update_aggregate(self, context, aggregate_id, values):
"""Update the properties of an aggregate."""
aggregate = aggregate_obj.Aggregate.get_by_id(context, aggregate_id)
if 'name' in values:
aggregate.name = values.pop('name')
self.is_safe_to_update_az(context, aggregate,
values, "update aggregate")
self.is_safe_to_update_az(context, values, aggregate=aggregate,
action_name="update_aggregate")
if values:
aggregate.metadata = values
aggregate.save()
@@ -3326,8 +3308,8 @@ class AggregateAPI(base.Base):
def update_aggregate_metadata(self, context, aggregate_id, metadata):
"""Updates the aggregate metadata."""
aggregate = aggregate_obj.Aggregate.get_by_id(context, aggregate_id)
self.is_safe_to_update_az(context, aggregate,
metadata, "update aggregate metadata")
self.is_safe_to_update_az(context, metadata, aggregate=aggregate,
action_name="update_aggregate_metadata")
aggregate.update_metadata(metadata)
# If updated metadata include availability_zones, then the cache
# which stored availability_zones and host need to be reset
@@ -3354,19 +3336,59 @@ class AggregateAPI(base.Base):
"delete.end",
aggregate_payload)
def _check_az_for_host(self, aggregate_meta, host_az, aggregate_id):
def is_safe_to_update_az(self, context, metadata, aggregate,
hosts=None, action_name="add_host_to_aggregate"):
"""Determine if updates alter an aggregate's availability zone.
:param context: local context
:param metadata: Target metadata for updating aggregate
:param aggregate: Aggregate to update
:param hosts: Hosts to check. If None, aggregate.hosts is used
:type hosts: list
:action_name: Calling method for logging purposes
"""
if 'availability_zone' in metadata:
_hosts = hosts or aggregate.hosts
for host in _hosts:
host_az = availability_zones.get_host_availability_zone(
context, host)
if host_az == CONF.default_availability_zone:
# NOTE(sbauza): Aggregate with AZ set to default AZ can
# exist, we need to check
host_aggs = aggregate_obj.AggregateList.get_by_host(
context, host, key='availability_zone')
default_aggs = [agg for agg in host_aggs
if agg['metadata'].get(
'availability_zone'
) == CONF.default_availability_zone]
else:
default_aggs = []
if (host_az != aggregate.metadata.get('availability_zone') and
(host_az != CONF.default_availability_zone or
len(default_aggs) != 0)):
self._check_az_for_host(
metadata, host_az, aggregate.id,
action_name=action_name)
def _check_az_for_host(self, aggregate_meta, host_az, aggregate_id,
action_name="add_host_to_aggregate"):
# NOTE(mtreinish) The availability_zone key returns a set of
# zones so loop over each zone. However there should only
# ever be one zone in the set because an aggregate can only
# have a single availability zone set at one time.
for aggregate_az in aggregate_meta["availability_zone"]:
if isinstance(aggregate_meta["availability_zone"], six.string_types):
azs = set([aggregate_meta["availability_zone"]])
else:
azs = aggregate_meta["availability_zone"]
for aggregate_az in azs:
# NOTE(mtreinish) Ensure that the aggregate_az is not none
# if it is none then that is just a regular aggregate and
# it is valid to have a host in multiple aggregates.
if aggregate_az and aggregate_az != host_az:
msg = _("Host already in availability zone "
"%s") % host_az
action_name = "add_host_to_aggregate"
raise exception.InvalidAggregateAction(
action=action_name, aggregate_id=aggregate_id,
reason=msg)
@@ -3389,14 +3411,13 @@ class AggregateAPI(base.Base):
aggregate_payload)
# validates the host; ComputeHostNotFound is raised if invalid
service_obj.Service.get_by_compute_host(context, host_name)
host_az = availability_zones.get_host_availability_zone(context,
host_name)
if host_az and host_az != CONF.default_availability_zone:
aggregate_meta = self.db.aggregate_metadata_get_by_metadata_key(
context, aggregate_id, 'availability_zone')
if aggregate_meta.get("availability_zone"):
self._check_az_for_host(aggregate_meta, host_az, aggregate_id)
metadata = self.db.aggregate_metadata_get_by_metadata_key(
context, aggregate_id, 'availability_zone')
aggregate = aggregate_obj.Aggregate.get_by_id(context, aggregate_id)
self.is_safe_to_update_az(context, metadata, hosts=[host_name],
aggregate=aggregate)
aggregate.add_host(context, host_name)
self._update_az_cache_for_host(context, host_name, aggregate.metadata)
#NOTE(jogo): Send message to host to support resource pools

36
nova/tests/compute/test_compute.py
View File

@@ -9506,6 +9506,23 @@ class ComputeAPIAggrTestCase(BaseTestCase):
self.assertEqual(msg.event_type,
'aggregate.updateprop.end')
def test_update_aggregate_az_fails_with_nova_az(self):
# Ensure aggregate's availability zone can't be updated,
# when aggregate has hosts in other availability zone
fake_notifier.NOTIFICATIONS = []
values = _create_service_entries(self.context)
fake_zone = values.keys()[0]
fake_host = values[fake_zone][0]
aggr1 = self._init_aggregate_with_host(None, 'fake_aggregate1',
CONF.default_availability_zone,
fake_host)
aggr2 = self._init_aggregate_with_host(None, 'fake_aggregate2', None,
fake_host)
metadata = {'availability_zone': 'another_zone'}
self.assertRaises(exception.InvalidAggregateAction,
self.api.update_aggregate,
self.context, aggr2['id'], metadata)
def test_update_aggregate_metadata(self):
# Ensure metadata can be updated.
aggr = self.api.create_aggregate(self.context, 'fake_aggregate',
@@ -9732,6 +9749,25 @@ class ComputeAPIAggrTestCase(BaseTestCase):
self.api.add_host_to_aggregate,
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
# to default
values = _create_service_entries(self.context)
fake_zone = values.keys()[0]
fake_host = values[fake_zone][0]
aggr = self.api.create_aggregate(self.context,
'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)
fake_zone2 = "another_zone"
aggr2 = self.api.create_aggregate(self.context,
'fake_aggregate2', fake_zone2)
self.assertRaises(exception.InvalidAggregateAction,
self.api.add_host_to_aggregate,
self.context, aggr2['id'], fake_host)
def test_add_host_to_aggregate_multiple(self):
# Ensure we can add multiple hosts to an aggregate.
values = _create_service_entries(self.context)