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:
parent
1a18e940a0
commit
de2f4d637d
|
@ -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
|
||||
-------
|
||||
|
||||
|
|
|
@ -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:
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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.")
|
||||
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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')
|
||||
|
|
|
@ -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')
|
||||
|
|
Loading…
Reference in New Issue