Allow adding host only if it exists in nova
Presently, you can add any hostname to the failover segment. It doesn't check whether this host exists or not in nova. This patch fixes this issue by checking whether the hostname exists in nova or not before adding it to the failover segment. 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 create. Closes-Bug: #1800073 Change-Id: I0ccc9f3a61e3f17f2cb7c1ad1888947c6fe724c8
This commit is contained in:
parent
55480c0184
commit
b21c763de0
|
@ -114,6 +114,8 @@ Response Codes
|
||||||
A conflict(409) is returned if host with same name is already present under
|
A conflict(409) is returned if host with same name is already present under
|
||||||
given segment.
|
given segment.
|
||||||
|
|
||||||
|
BadRequest (400) is returned if host doesn't exists in nova.
|
||||||
|
|
||||||
Request
|
Request
|
||||||
-------
|
-------
|
||||||
|
|
||||||
|
|
|
@ -97,8 +97,8 @@ class HostsController(wsgi.Controller):
|
||||||
return {'hosts': hosts}
|
return {'hosts': hosts}
|
||||||
|
|
||||||
@wsgi.response(http.CREATED)
|
@wsgi.response(http.CREATED)
|
||||||
@extensions.expected_errors((http.FORBIDDEN, http.NOT_FOUND,
|
@extensions.expected_errors((http.BAD_REQUEST, http.FORBIDDEN,
|
||||||
http.CONFLICT))
|
http.NOT_FOUND, http.CONFLICT))
|
||||||
@validation.schema(schema.create)
|
@validation.schema(schema.create)
|
||||||
def create(self, req, segment_id, body):
|
def create(self, req, segment_id, body):
|
||||||
"""Creates a host."""
|
"""Creates a host."""
|
||||||
|
@ -107,6 +107,8 @@ class HostsController(wsgi.Controller):
|
||||||
host_data = body.get('host')
|
host_data = body.get('host')
|
||||||
try:
|
try:
|
||||||
host = self.api.create_host(context, segment_id, host_data)
|
host = self.api.create_host(context, segment_id, host_data)
|
||||||
|
except exception.HostNotFoundByName as e:
|
||||||
|
raise exc.HTTPBadRequest(explanation=e.message)
|
||||||
except exception.FailoverSegmentNotFound as e:
|
except exception.FailoverSegmentNotFound as e:
|
||||||
raise exc.HTTPNotFound(explanation=e.format_message())
|
raise exc.HTTPNotFound(explanation=e.format_message())
|
||||||
except exception.HostExists as e:
|
except exception.HostExists as e:
|
||||||
|
|
|
@ -39,7 +39,7 @@ CONF.import_group('keystone_authtoken', 'keystonemiddleware.auth_token')
|
||||||
|
|
||||||
LOG = logging.getLogger(__name__)
|
LOG = logging.getLogger(__name__)
|
||||||
|
|
||||||
NOVA_API_VERSION = "2.14"
|
NOVA_API_VERSION = "2.53"
|
||||||
|
|
||||||
nova_extensions = [ext for ext in
|
nova_extensions = [ext for ext in
|
||||||
nova_client.discover_extensions(NOVA_API_VERSION)
|
nova_client.discover_extensions(NOVA_API_VERSION)
|
||||||
|
@ -243,3 +243,18 @@ class API(object):
|
||||||
msg = ('Call unlock server command for instance %(uuid)s')
|
msg = ('Call unlock server command for instance %(uuid)s')
|
||||||
LOG.info(msg, {'uuid': uuid})
|
LOG.info(msg, {'uuid': uuid})
|
||||||
return nova.servers.unlock(uuid)
|
return nova.servers.unlock(uuid)
|
||||||
|
|
||||||
|
@translate_nova_exception
|
||||||
|
def hypervisor_search(self, context, host_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})
|
||||||
|
try:
|
||||||
|
hypervisors_list = nova.hypervisors.search(host_name)
|
||||||
|
if host_name not in [host.hypervisor_hostname for host in
|
||||||
|
hypervisors_list]:
|
||||||
|
raise exception.HostNotFoundByName(host_name=host_name)
|
||||||
|
except nova_exception.NotFound:
|
||||||
|
raise exception.HostNotFoundByName(host_name=host_name)
|
||||||
|
|
|
@ -18,6 +18,7 @@ from oslo_log import log as logging
|
||||||
from oslo_utils import strutils
|
from oslo_utils import strutils
|
||||||
from oslo_utils import uuidutils
|
from oslo_utils import uuidutils
|
||||||
|
|
||||||
|
from masakari.compute import nova
|
||||||
import masakari.conf
|
import masakari.conf
|
||||||
from masakari.engine import rpcapi as engine_rpcapi
|
from masakari.engine import rpcapi as engine_rpcapi
|
||||||
from masakari import exception
|
from masakari import exception
|
||||||
|
@ -166,6 +167,8 @@ class HostAPI(object):
|
||||||
host.reserved = strutils.bool_from_string(
|
host.reserved = strutils.bool_from_string(
|
||||||
host_data.get('reserved', False), strict=True)
|
host_data.get('reserved', False), strict=True)
|
||||||
|
|
||||||
|
novaclient = nova.API()
|
||||||
|
novaclient.hypervisor_search(context, host.name)
|
||||||
host.create()
|
host.create()
|
||||||
return host
|
return host
|
||||||
|
|
||||||
|
|
|
@ -293,3 +293,28 @@ class NovaApiTestCase(test.TestCase):
|
||||||
|
|
||||||
mock_novaclient.assert_called_once_with(self.ctx)
|
mock_novaclient.assert_called_once_with(self.ctx)
|
||||||
mock_servers.unlock.assert_called_once_with(uuidsentinel.fake_server)
|
mock_servers.unlock.assert_called_once_with(uuidsentinel.fake_server)
|
||||||
|
|
||||||
|
@mock.patch('masakari.compute.nova.novaclient')
|
||||||
|
def test_hypervisor_search_non_existing_host_name(self, mock_novaclient):
|
||||||
|
mock_novaclient.return_value.hypervisors.search.side_effect = (
|
||||||
|
nova_exception.NotFound(http.NOT_FOUND))
|
||||||
|
|
||||||
|
self.assertRaises(exception.HostNotFoundByName,
|
||||||
|
self.api.hypervisor_search, context, 'abc')
|
||||||
|
|
||||||
|
@mock.patch('masakari.compute.nova.novaclient')
|
||||||
|
def test_hypervisor_search_case_sensitive(self, mock_novaclient):
|
||||||
|
test_hypers = [
|
||||||
|
dict(id=1,
|
||||||
|
uuid=uuidsentinel.hyper1,
|
||||||
|
hypervisor_hostname="xyz.one"),
|
||||||
|
dict(id=2,
|
||||||
|
uuid=uuidsentinel.hyper2,
|
||||||
|
hypervisor_hostname="xyz.two", ),
|
||||||
|
dict(id=3,
|
||||||
|
uuid=uuidsentinel.hyper3,
|
||||||
|
hypervisor_hostname="XYZ", )
|
||||||
|
]
|
||||||
|
mock_novaclient.hypervisors.search.return_value = test_hypers
|
||||||
|
self.assertRaises(exception.HostNotFoundByName,
|
||||||
|
self.api.hypervisor_search, context, 'xyz')
|
||||||
|
|
|
@ -19,6 +19,7 @@ import copy
|
||||||
import mock
|
import mock
|
||||||
from oslo_utils import timeutils
|
from oslo_utils import timeutils
|
||||||
|
|
||||||
|
from masakari.compute import nova as nova_obj
|
||||||
from masakari.engine import rpcapi as engine_rpcapi
|
from masakari.engine import rpcapi as engine_rpcapi
|
||||||
from masakari import exception
|
from masakari import exception
|
||||||
from masakari.ha import api as ha_api
|
from masakari.ha import api as ha_api
|
||||||
|
@ -254,11 +255,14 @@ class HostAPITestCase(test.NoDBTestCase):
|
||||||
|
|
||||||
@mock.patch.object(host_obj, 'Host')
|
@mock.patch.object(host_obj, 'Host')
|
||||||
@mock.patch.object(host_obj.Host, 'create')
|
@mock.patch.object(host_obj.Host, 'create')
|
||||||
|
@mock.patch.object(nova_obj.API, 'hypervisor_search')
|
||||||
@mock.patch.object(segment_obj.FailoverSegment, 'get_by_uuid')
|
@mock.patch.object(segment_obj.FailoverSegment, 'get_by_uuid')
|
||||||
def test_create(self, mock_get, mock_host_create, mock_host_obj):
|
def test_create(self, mock_get, mock_hypervisor_search,
|
||||||
|
mock_host_create, mock_host_obj):
|
||||||
mock_get.return_value = self.failover_segment
|
mock_get.return_value = self.failover_segment
|
||||||
|
|
||||||
host_data = {
|
host_data = {
|
||||||
"name": "host-1", "type": "fake-type",
|
"name": 'host-1', "type": "fake-type",
|
||||||
"reserved": False,
|
"reserved": False,
|
||||||
"on_maintenance": False,
|
"on_maintenance": False,
|
||||||
"control_attributes": "fake-control_attributes"
|
"control_attributes": "fake-control_attributes"
|
||||||
|
@ -270,11 +274,33 @@ class HostAPITestCase(test.NoDBTestCase):
|
||||||
host_data)
|
host_data)
|
||||||
self._assert_host_data(self.host, _make_host_obj(result))
|
self._assert_host_data(self.host, _make_host_obj(result))
|
||||||
|
|
||||||
|
@mock.patch.object(nova_obj.API, 'hypervisor_search')
|
||||||
|
@mock.patch.object(segment_obj.FailoverSegment, 'get_by_uuid')
|
||||||
|
def test_create_non_existing_host(self, mock_segment_get,
|
||||||
|
mock_hypervisor_search):
|
||||||
|
mock_segment_get.return_value = self.failover_segment
|
||||||
|
mock_hypervisor_search.side_effect = exception\
|
||||||
|
.HostNotFoundByName(host_name='host-2')
|
||||||
|
|
||||||
|
host_data = {
|
||||||
|
"name": 'host-2',
|
||||||
|
"type": "fake-type",
|
||||||
|
"reserved": False,
|
||||||
|
"on_maintenance": False,
|
||||||
|
"control_attributes": "fake-control_attributes"
|
||||||
|
}
|
||||||
|
|
||||||
|
self.assertRaises(exception.HostNotFoundByName,
|
||||||
|
self.host_api.create_host,
|
||||||
|
self.context, uuidsentinel.fake_segment1, host_data)
|
||||||
|
|
||||||
@mock.patch('oslo_utils.uuidutils.generate_uuid')
|
@mock.patch('oslo_utils.uuidutils.generate_uuid')
|
||||||
@mock.patch('masakari.db.host_create')
|
@mock.patch('masakari.db.host_create')
|
||||||
@mock.patch.object(host_obj.Host, '_from_db_object')
|
@mock.patch.object(host_obj.Host, '_from_db_object')
|
||||||
|
@mock.patch.object(nova_obj.API, 'hypervisor_search')
|
||||||
@mock.patch.object(segment_obj.FailoverSegment, 'get_by_uuid')
|
@mock.patch.object(segment_obj.FailoverSegment, 'get_by_uuid')
|
||||||
def test_create_convert_boolean_attributes(self, mock_get_segment,
|
def test_create_convert_boolean_attributes(self, mock_get_segment,
|
||||||
|
mock_hypervisor_search,
|
||||||
mock__from_db_object,
|
mock__from_db_object,
|
||||||
mock_host_create,
|
mock_host_create,
|
||||||
mock_generate_uuid):
|
mock_generate_uuid):
|
||||||
|
|
Loading…
Reference in New Issue