x
/
mogan
Code Issues Proposed changes

Add checks for aggregate availability_zone 

* Do not allow empty availablity zone
* A node can not be in different availablity zone

Change-Id: I82096c60f9d871e5f3ddcbc905dfe62f70b19b79
This commit is contained in:
Zhenguo Niu 2017-08-02 11:08:31 +08:00
parent e3434da4dc
commit 3c15f2554a
6 changed files with 121 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

78
mogan/api/controllers/v1/aggregates.py
View File

@ -27,6 +27,7 @@ from mogan.api.controllers.v1 import utils as api_utils
from mogan.api import expose
from mogan.api import validation
from mogan.common import exception
from mogan.common.i18n import _
from mogan.common import policy
from mogan import objects
@ -116,10 +117,28 @@ class AggregateNodeController(rest.RestController):
"""Add node to the given aggregate."""
validation.check_schema(node, agg_schema.add_aggregate_node)
# check whether the aggregate exists
objects.Aggregate.get(pecan.request.context, aggregate_uuid)
node = node['node']
# check whether the node is already in another az
db_aggregate = objects.Aggregate.get(pecan.request.context,
aggregate_uuid)
if 'availability_zone' in db_aggregate['metadata']:
node_aggs = pecan.request.engine_api.list_node_aggregates(
pecan.request.context, node)
aggregates = objects.AggregateList.get_by_metadata_key(
pecan.request.context, 'availability_zone')
agg_az = db_aggregate.metadata['availability_zone']
conflict_azs = [
agg.metadata['availability_zone'] for agg in aggregates
if agg.uuid in node_aggs['aggregates'] and
agg.metadata['availability_zone'] != agg_az]
if conflict_azs:
msg = _("Node %(node)s is already in availability zone(s) "
"%(az)s") % {"node": node, "az": conflict_azs}
raise wsme.exc.ClientSideError(
msg, status_code=http_client.BAD_REQUEST)
pecan.request.engine_api.add_aggregate_node(
pecan.request.context, aggregate_uuid, node['node'])
pecan.request.context, aggregate_uuid, node)
@policy.authorize_wsgi("mogan:aggregate_node", "delete")
@expose.expose(None, types.uuid, wtypes.text,
@ -161,11 +180,19 @@ class AggregateController(rest.RestController):
@expose.expose(Aggregate, body=types.jsontype,
status_code=http_client.CREATED)
def post(self, aggregate):
"""Create an new aggregate.
"""Create a new aggregate.
:param aggregate: an aggregate within the request body.
"""
validation.check_schema(aggregate, agg_schema.create_aggregate)
metadata = aggregate.get('metadata')
if metadata and 'availability_zone' in metadata:
if not metadata['availability_zone']:
msg = _("Aggregate %s does not support empty named "
"availability zone") % aggregate['name']
raise wsme.exc.ClientSideError(
msg, status_code=http_client.BAD_REQUEST)
new_aggregate = objects.Aggregate(pecan.request.context, **aggregate)
new_aggregate.create()
# Set the HTTP Location Header
@ -193,6 +220,14 @@ class AggregateController(rest.RestController):
except api_utils.JSONPATCH_EXCEPTIONS as e:
raise exception.PatchError(patch=patch, reason=e)
# Check if this tries to change availability zone
az_changed = False
for patch_dict in patch:
if patch_dict['path'] == '/metadata/availability_zone' \
and patch_dict['op'] != 'remove':
az_changed = True
break
# Update only the fields that have changed
for field in objects.Aggregate.fields:
try:
@ -205,6 +240,41 @@ class AggregateController(rest.RestController):
if db_aggregate[field] != patch_val:
db_aggregate[field] = patch_val
if field == 'metadata' and az_changed:
if not patch_val['availability_zone']:
msg = _("Aggregate %s does not support empty named "
"availability zone") % aggregate.name
raise wsme.exc.ClientSideError(
msg, status_code=http_client.BAD_REQUEST)
# ensure it's safe to update availability_zone
aggregates = objects.AggregateList.get_by_metadata_key(
pecan.request.context, 'availability_zone')
nodes = pecan.request.engine_api.list_aggregate_nodes(
pecan.request.context, db_aggregate['uuid'])
filtered_aggs = []
for agg in aggregates:
agg_nodes = \
pecan.request.engine_api.list_aggregate_nodes(
pecan.request.context, agg.uuid)
for node in agg_nodes['nodes']:
if node in nodes['nodes']:
filtered_aggs.append(agg)
break
new_az = patch_val['availability_zone']
conflict_azs = [
agg.metadata['availability_zone']
for agg in filtered_aggs
if agg.metadata['availability_zone'] != new_az and
agg.uuid != db_aggregate['uuid']
]
if conflict_azs:
msg = _("One or more nodes already in different "
"availability zone(s) %s") % conflict_azs
raise wsme.exc.ClientSideError(
msg, status_code=http_client.BAD_REQUEST)
db_aggregate.save()
return Aggregate.convert_with_links(db_aggregate)

4
mogan/engine/api.py
View File

@ -569,3 +569,7 @@ class API(object):
def remove_aggregate(self, context, aggregate_uuid):
"""Remove the aggregate."""
return self.engine_rpcapi.remove_aggregate(context, aggregate_uuid)
def list_node_aggregates(self, context, node):
"""Get the node aggregates list."""
return self.engine_rpcapi.list_node_aggregates(context, node)

5
mogan/engine/manager.py
View File

@ -595,3 +595,8 @@ class EngineManager(base_manager.BaseEngineManager):
def remove_aggregate(self, context, aggregate_uuid):
self.scheduler_client.reportclient \
.remove_aggregate(aggregate_uuid)
def list_node_aggregates(self, context, node):
aggregates = self.scheduler_client.reportclient \
.get_aggregates_from_node(node)
return aggregates

4
mogan/engine/rpcapi.py
View File

@ -116,3 +116,7 @@ class EngineAPI(object):
cctxt = self.client.prepare(topic=self.topic, server=CONF.host)
return cctxt.call(context, 'remove_aggregate',
aggregate_uuid=aggregate_uuid)
def list_node_aggregates(self, context, node):
cctxt = self.client.prepare(topic=self.topic, server=CONF.host)
return cctxt.call(context, 'list_node_aggregates', node=node)

13
mogan/scheduler/client/report.py
View File

@ -795,3 +795,16 @@ class SchedulerReportClient(object):
if aggregate_uuid in aggs:
new_aggs = aggs - set([aggregate_uuid])
self._put_provider_aggregates(rp, list(new_aggs))
def get_aggregates_from_node(self, node):
# Use the aggregates we cached
rps = self._resource_providers
for id, rp in rps.items():
if node == rp['name']:
rp_uuid = id
break
else:
raise exception.NodeNotFound(node=node)
aggs = self._provider_aggregate_map[rp_uuid]
return {'aggregates': list(aggs)}

21
mogan/tests/functional/api/v1/test_aggregates.py
View File

@ -14,6 +14,7 @@
import mock
import six
from six.moves import http_client
from mogan.tests.functional.api import v1 as v1_test
@ -49,6 +50,15 @@ class TestAggregate(v1_test.APITestV1):
self.assertIn('uuid', resp)
self.assertIn('links', resp)
def test_aggregate_post_with_empty_az(self):
body = {"name": "test_empty",
"metadata": {"availability_zone": ""}}
response = self.post_json(
'/aggregates', body, headers=self.headers, expect_errors=True)
self.assertEqual(http_client.BAD_REQUEST, response.status_code)
self.assertEqual('application/json', response.content_type)
self.assertTrue(response.json['error_message'])
def test_aggregate_get_all(self):
self._prepare_aggregates()
resp = self.get_json('/aggregates', headers=self.headers)
@ -87,3 +97,14 @@ class TestAggregate(v1_test.APITestV1):
headers=self.headers)
self.assertEqual('updated_name', resp['name'])
self.assertItemsEqual({'k1': 'v1', 'k2': 'v2'}, resp['metadata'])
def test_aggregate_update_with_empty_az(self):
self._prepare_aggregates()
response = self.patch_json('/aggregates/' + self.AGGREGATE_UUIDS[0],
[{'path': '/metadata/availability_zone',
'value': '', 'op': 'add'}],
headers=self.headers,
expect_errors=True)
self.assertEqual(http_client.BAD_REQUEST, response.status_code)
self.assertEqual('application/json', response.content_type)
self.assertTrue(response.json['error_message'])