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:
parent
237bee37f8
commit
a1bbcd4d98
|
@ -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):
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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.")
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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."""
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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',
|
||||
|
|
Loading…
Reference in New Issue