Fix 'host delete' delete's host of another segment

Currently masakari delete's the host which doesn't belong to the
segment requested in the 'host delete' API's request URL because
host was not getting filtered on the basis of failover_segment_id.

Filtered the host based on 'failover_segment_id'. Added a new
exception 'HostNotFoundUnderFailoverSegment' which will be raised
from db API 'host_get_by_uuid' if the host is not found under the
requested failover_segment. This exception is not caught at API
controller layer as it is a subclass of 'HostNotFound' which is
already caught in controller.

Closes-Bug: #1697629
Change-Id: I3d3e8cffa2f8ea443ccd13b1db61fda55bc07a0d
This commit is contained in:
Dinesh Bhor 2017-07-05 17:05:55 +05:30
parent 237bee37f8
commit a1bbcd4d98
8 changed files with 66 additions and 13 deletions

View File

@ -192,17 +192,18 @@ def host_get_all_by_filters(
marker=marker)
def host_get_by_uuid(context, host_uuid):
def host_get_by_uuid(context, host_uuid, segment_uuid=None):
"""Get host information by uuid.
:param context: context to query under
:param host_uuid: uuid of host
:param segment_uuid: uuid of failover_segment
:returns: dictionary-like object containing host
:raises: exception.HostNotFound if host with 'host_uuid' doesn't exist
"""
return IMPL.host_get_by_uuid(context, host_uuid)
return IMPL.host_get_by_uuid(context, host_uuid, segment_uuid=segment_uuid)
def host_get_by_id(context, host_id):

View File

@ -402,18 +402,25 @@ def host_get_all_by_filters(
@oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True)
@main_context_manager.reader
def host_get_by_uuid(context, host_uuid):
return _host_get_by_uuid(context, host_uuid)
def host_get_by_uuid(context, host_uuid, segment_uuid=None):
return _host_get_by_uuid(context, host_uuid, segment_uuid=segment_uuid)
def _host_get_by_uuid(context, host_uuid):
def _host_get_by_uuid(context, host_uuid, segment_uuid=None):
query = model_query(context, models.Host
).filter_by(uuid=host_uuid
).options(joinedload('failover_segment'))
if segment_uuid:
query = query.filter_by(failover_segment_id=segment_uuid)
result = query.first()
if not result:
raise exception.HostNotFound(id=host_uuid)
if segment_uuid:
raise exception.HostNotFoundUnderFailoverSegment(
host_uuid=host_uuid, segment_uuid=segment_uuid)
else:
raise exception.HostNotFound(id=host_uuid)
return result

View File

@ -356,3 +356,8 @@ class LockAlreadyAcquired(MasakariException):
class IgnoreInstanceRecoveryException(MasakariException):
msg_fmt = _('Instance recovery is ignored.')
class HostNotFoundUnderFailoverSegment(HostNotFound):
msg_fmt = _("Host '%(host_uuid)s' under failover_segment "
"'%(segment_uuid)s' could not be found.")

View File

@ -198,7 +198,7 @@ class HostAPI(object):
"""Delete the host"""
segment = objects.FailoverSegment.get_by_uuid(context, segment_uuid)
host = objects.Host.get_by_uuid(context, id)
host = objects.Host.get_by_uuid(context, id, segment_uuid=segment_uuid)
if is_failover_segment_under_recovery(segment):
msg = _("Host %s can't be deleted as "
"it is in-use to process notifications.") % host.uuid

View File

@ -29,7 +29,9 @@ LOG = logging.getLogger(__name__)
class Host(base.MasakariPersistentObject, base.MasakariObject,
base.MasakariObjectDictCompat):
VERSION = '1.0'
# Version 1.0: Initial version
# Version 1.1: Added 'segment_uuid' parameter to 'get_by_uuid' method
VERSION = '1.1'
fields = {
'id': fields.IntegerField(),
@ -64,8 +66,8 @@ class Host(base.MasakariPersistentObject, base.MasakariObject,
return cls._from_db_object(context, cls(), db_inst)
@base.remotable_classmethod
def get_by_uuid(cls, context, uuid):
db_inst = db.host_get_by_uuid(context, uuid)
def get_by_uuid(cls, context, uuid, segment_uuid=None):
db_inst = db.host_get_by_uuid(context, uuid, segment_uuid=segment_uuid)
return cls._from_db_object(context, cls(), db_inst)
@base.remotable_classmethod

View File

@ -461,6 +461,16 @@ class HostTestCase(test.TestCase):
self.req, uuidsentinel.fake_segment1,
uuidsentinel.fake_host_3)
@mock.patch.object(ha_api.HostAPI, 'delete_host')
def test_delete_host_not_found_for_failover_segment(self, mock_delete):
mock_delete.side_effect = exception.HostNotFoundUnderFailoverSegment(
host_uuid=uuidsentinel.fake_host_3,
segment_uuid=uuidsentinel.fake_segment1)
self.assertRaises(exc.HTTPNotFound, self.controller.delete,
self.req, uuidsentinel.fake_segment1,
uuidsentinel.fake_host_3)
class HostTestCasePolicyNotAuthorized(test.NoDBTestCase):
"""Test Case for host non admin."""

View File

@ -231,11 +231,16 @@ class HostsTestCase(test.TestCase, ModelsObjectComparatorMixin):
def _create_host(self, values):
return db.host_create(self.ctxt, values)
def _test_get_host(self, method, filter):
def _test_get_host(self, method, host_uuid_filter,
failover_segment_id_filter=None):
hosts = [self._create_host(p) for p in self._get_fake_values_list()]
ignored_key = ['failover_segment']
for host in hosts:
real_host = method(self.ctxt, host[filter])
if failover_segment_id_filter:
real_host = method(self.ctxt, host[host_uuid_filter],
host[failover_segment_id_filter])
else:
real_host = method(self.ctxt, host[host_uuid_filter])
self._assertEqualObjects(host, real_host, ignored_key)
self.assertEqual(host['failover_segment'].items(),
real_host['failover_segment'].items())
@ -256,6 +261,29 @@ class HostsTestCase(test.TestCase, ModelsObjectComparatorMixin):
def test_host_get_by_name(self):
self._test_get_host(db.host_get_by_name, 'name')
def test_host_get_by_host_uuid_and_failover_segment_id(self):
self._test_get_host(db.host_get_by_uuid, 'uuid', 'failover_segment_id')
def test_host_get_by_uuid_filter_by_invalid_failover_segment(self):
# create hosts under failover_segment
# 'uuidsentinel.failover_segment_id'
for host in self._get_fake_values_list():
self._create_host(host)
# create one more failover_segment
values = {'uuid': uuidsentinel.failover_segment_id_1,
'name': 'fake_segment_name_1',
'service_type': 'fake_service_type',
'description': 'fake_description',
'recovery_method': 'auto'}
db.failover_segment_create(self.ctxt, values)
# try to get host with failover_segment
# 'uuidsentinel.failover_segment_id_1'
self.assertRaises(
exception.HostNotFoundUnderFailoverSegment, db.host_get_by_uuid,
self.ctxt, uuidsentinel.uuid_1, uuidsentinel.failover_segment_id_1)
def test_host_update(self):
update = {'name': 'updated_name', 'type': 'updated_type'}
updated = {'uuid': uuidsentinel.fake_uuid,

View File

@ -654,7 +654,7 @@ class TestRegistry(test.NoDBTestCase):
object_data = {
'FailoverSegment': '1.0-5e8b8bc8840b35439b5f2b621482d15d',
'FailoverSegmentList': '1.0-dfc5c6f5704d24dcaa37b0bbb03cbe60',
'Host': '1.0-803264cd1563db37d0bf7cff48e34c1d',
'Host': '1.1-3fc4d548fa220c76906426095e5971fc',
'HostList': '1.0-25ebe1b17fbd9f114fae8b6a10d198c0',
'Notification': '1.0-eedfa3c203c100897021bd23f0ddf68c',
'NotificationList': '1.0-25ebe1b17fbd9f114fae8b6a10d198c0',