Allow updating host name only if it exists in nova

Presently, you can update host name with any non-existing host name.
It doesn't check whether this host exists or not in nova. This patch
fixes this issue by checking whether the host name exists in nova or
not before updating it. If it doesn't exists, it will raise 400 error.

APIImpact
BadRequest(400) is returned if host doesn't exists in nova instead of
200 during host update.

Closes-Bug: #1814656

Change-Id: Ibd113f2328deae0f1114544436631bdc434eff92
This commit is contained in:
jayashri bidwe 2019-02-05 15:59:24 +05:30
parent 1a18e940a0
commit de2f4d637d
7 changed files with 55 additions and 17 deletions

View File

@ -256,6 +256,8 @@ Response Codes
notification table i.e. any host from the failover segment has
notification status as new/error/running.
BadRequest (400) is returned if host doesn't exists in nova.
Request
-------

View File

@ -107,7 +107,7 @@ class HostsController(wsgi.Controller):
host_data = body.get('host')
try:
host = self.api.create_host(context, segment_id, host_data)
except exception.HostNotFoundByName as e:
except exception.HypervisorNotFoundByName as e:
raise exc.HTTPBadRequest(explanation=e.message)
except exception.FailoverSegmentNotFound as e:
raise exc.HTTPNotFound(explanation=e.format_message())
@ -129,8 +129,8 @@ class HostsController(wsgi.Controller):
raise exc.HTTPNotFound(explanation=e.format_message())
return {'host': host}
@extensions.expected_errors((http.FORBIDDEN, http.NOT_FOUND,
http.CONFLICT))
@extensions.expected_errors((http.BAD_REQUEST, http.FORBIDDEN,
http.NOT_FOUND, http.CONFLICT))
@validation.schema(schema.update)
def update(self, req, segment_id, id, body):
"""Updates the existing host."""
@ -139,6 +139,8 @@ class HostsController(wsgi.Controller):
host_data = body.get('host')
try:
host = self.api.update_host(context, segment_id, id, host_data)
except exception.HypervisorNotFoundByName as e:
raise exc.HTTPBadRequest(explanation=e.message)
except exception.HostNotFound as e:
raise exc.HTTPNotFound(explanation=e.format_message())
except exception.FailoverSegmentNotFound as e:

View File

@ -245,16 +245,18 @@ class API(object):
return nova.servers.unlock(uuid)
@translate_nova_exception
def hypervisor_search(self, context, host_name):
def hypervisor_search(self, context, hypervisor_name):
"""Search hypervisor with case sensitive hostname."""
nova = novaclient(context)
msg = ("Call hypervisor search command to get list of matching "
"host name '%(host_name)s'")
LOG.info(msg, {'host_name': host_name})
"hypervisor name '%(hypervisor_name)s'")
LOG.info(msg, {'hypervisor_name': hypervisor_name})
try:
hypervisors_list = nova.hypervisors.search(host_name)
if host_name not in [host.hypervisor_hostname for host in
hypervisors_list = nova.hypervisors.search(hypervisor_name)
if hypervisor_name not in [host.hypervisor_hostname for host in
hypervisors_list]:
raise exception.HostNotFoundByName(host_name=host_name)
raise exception.HypervisorNotFoundByName(
hypervisor_name=hypervisor_name)
except nova_exception.NotFound:
raise exception.HostNotFoundByName(host_name=host_name)
raise exception.HypervisorNotFoundByName(
hypervisor_name=hypervisor_name)

View File

@ -276,6 +276,10 @@ class HostNotFoundByName(HostNotFound):
msg_fmt = _("Host with name %(host_name)s could not be found.")
class HypervisorNotFoundByName(NotFound):
msg_fmt = _("Hypervisor with name %(hypervisor_name)s could not be found.")
class FailoverSegmentExists(MasakariException):
msg_fmt = _("Failover segment with name %(name)s already exists.")

View File

@ -123,6 +123,10 @@ class FailoverSegmentAPI(object):
class HostAPI(object):
"""The Host API to manage hosts"""
def _is_valid_host_name(self, context, name):
novaclient = nova.API()
novaclient.hypervisor_search(context, name)
def get_host(self, context, segment_uuid, host_uuid):
"""Get a host by id"""
objects.FailoverSegment.get_by_uuid(context, segment_uuid)
@ -167,8 +171,8 @@ class HostAPI(object):
host.reserved = strutils.bool_from_string(
host_data.get('reserved', False), strict=True)
novaclient = nova.API()
novaclient.hypervisor_search(context, host.name)
self._is_valid_host_name(context, host.name)
host.create()
return host
@ -184,6 +188,9 @@ class HostAPI(object):
LOG.error(msg)
raise exception.HostInUse(msg)
if 'name' in host_data:
self._is_valid_host_name(context, host_data.get('name'))
if 'on_maintenance' in host_data:
host_data['on_maintenance'] = strutils.bool_from_string(
host_data['on_maintenance'], strict=True)

View File

@ -308,7 +308,7 @@ class NovaApiTestCase(test.TestCase):
mock_novaclient.return_value.hypervisors.search.side_effect = (
nova_exception.NotFound(http.NOT_FOUND))
self.assertRaises(exception.HostNotFoundByName,
self.assertRaises(exception.HypervisorNotFoundByName,
self.api.hypervisor_search, context, 'abc')
@mock.patch('masakari.compute.nova.novaclient')
@ -325,5 +325,5 @@ class NovaApiTestCase(test.TestCase):
hypervisor_hostname="XYZ", )
]
mock_novaclient.hypervisors.search.return_value = test_hypers
self.assertRaises(exception.HostNotFoundByName,
self.assertRaises(exception.HypervisorNotFoundByName,
self.api.hypervisor_search, context, 'xyz')

View File

@ -280,7 +280,7 @@ class HostAPITestCase(test.NoDBTestCase):
mock_hypervisor_search):
mock_segment_get.return_value = self.failover_segment
mock_hypervisor_search.side_effect = exception\
.HostNotFoundByName(host_name='host-2')
.HypervisorNotFoundByName(host_name='host-2')
host_data = {
"name": 'host-2',
@ -290,7 +290,7 @@ class HostAPITestCase(test.NoDBTestCase):
"control_attributes": "fake-control_attributes"
}
self.assertRaises(exception.HostNotFoundByName,
self.assertRaises(exception.HypervisorNotFoundByName,
self.host_api.create_host,
self.context, uuidsentinel.fake_segment1, host_data)
@ -348,11 +348,12 @@ class HostAPITestCase(test.NoDBTestCase):
@mock.patch.object(segment_obj.FailoverSegment,
'is_under_recovery')
@mock.patch.object(host_obj, 'Host')
@mock.patch.object(nova_obj.API, 'hypervisor_search')
@mock.patch.object(host_obj.Host, 'save')
@mock.patch.object(host_obj.Host, 'get_by_uuid')
@mock.patch.object(segment_obj.FailoverSegment, 'get_by_uuid')
def test_update(self, mock_segment_get, mock_get,
mock_update, mock_host_obj,
mock_update, mock_hypervisor_search, mock_host_obj,
mock_is_under_recovery):
mock_segment_get.return_value = self.failover_segment
host_data = {"name": "host_1"}
@ -366,6 +367,26 @@ class HostAPITestCase(test.NoDBTestCase):
host_data)
self._assert_host_data(self.host, result)
@mock.patch.object(segment_obj.FailoverSegment,
'is_under_recovery')
@mock.patch.object(nova_obj.API, 'hypervisor_search')
@mock.patch.object(host_obj.Host, 'get_by_uuid')
@mock.patch.object(segment_obj.FailoverSegment, 'get_by_uuid')
def test_update_with_non_existing_host(self, mock_segment_get, mock_get,
mock_hypervisor_search,
mock_is_under_recovery):
mock_segment_get.return_value = self.failover_segment
host_data = {"name": "host-2"}
mock_get.return_value = self.host
mock_hypervisor_search.side_effect = (
exception.HypervisorNotFoundByName(host_name='host-2'))
mock_is_under_recovery.return_value = False
self.assertRaises(exception.HypervisorNotFoundByName,
self.host_api.update_host, self.context,
uuidsentinel.fake_segment,
uuidsentinel.fake_host_1,
host_data)
@mock.patch.object(segment_obj.FailoverSegment,
'is_under_recovery')
@mock.patch.object(host_obj, 'Host')