From b21c763de0fc816df17f5fc1f3a4518672a5ee36 Mon Sep 17 00:00:00 2001 From: jayashri bidwe Date: Fri, 2 Nov 2018 18:52:41 +0530 Subject: [PATCH] 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 --- api-ref/source/hosts.inc | 2 ++ masakari/api/openstack/ha/hosts.py | 6 +++-- masakari/compute/nova.py | 17 +++++++++++++- masakari/ha/api.py | 3 +++ masakari/tests/unit/compute/test_nova.py | 25 ++++++++++++++++++++ masakari/tests/unit/ha/test_api.py | 30 ++++++++++++++++++++++-- 6 files changed, 78 insertions(+), 5 deletions(-) diff --git a/api-ref/source/hosts.inc b/api-ref/source/hosts.inc index 5ba9c4e8..0f0a7b1a 100644 --- a/api-ref/source/hosts.inc +++ b/api-ref/source/hosts.inc @@ -114,6 +114,8 @@ Response Codes A conflict(409) is returned if host with same name is already present under given segment. + BadRequest (400) is returned if host doesn't exists in nova. + Request ------- diff --git a/masakari/api/openstack/ha/hosts.py b/masakari/api/openstack/ha/hosts.py index 746cf990..ad5f5b4a 100644 --- a/masakari/api/openstack/ha/hosts.py +++ b/masakari/api/openstack/ha/hosts.py @@ -97,8 +97,8 @@ class HostsController(wsgi.Controller): return {'hosts': hosts} @wsgi.response(http.CREATED) - @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.create) def create(self, req, segment_id, body): """Creates a host.""" @@ -107,6 +107,8 @@ 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: + raise exc.HTTPBadRequest(explanation=e.message) except exception.FailoverSegmentNotFound as e: raise exc.HTTPNotFound(explanation=e.format_message()) except exception.HostExists as e: diff --git a/masakari/compute/nova.py b/masakari/compute/nova.py index ee18ef0d..a3310ed9 100644 --- a/masakari/compute/nova.py +++ b/masakari/compute/nova.py @@ -39,7 +39,7 @@ CONF.import_group('keystone_authtoken', 'keystonemiddleware.auth_token') LOG = logging.getLogger(__name__) -NOVA_API_VERSION = "2.14" +NOVA_API_VERSION = "2.53" nova_extensions = [ext for ext in nova_client.discover_extensions(NOVA_API_VERSION) @@ -243,3 +243,18 @@ class API(object): msg = ('Call unlock server command for instance %(uuid)s') LOG.info(msg, {'uuid': 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) diff --git a/masakari/ha/api.py b/masakari/ha/api.py index eff951bb..3459e8ea 100644 --- a/masakari/ha/api.py +++ b/masakari/ha/api.py @@ -18,6 +18,7 @@ from oslo_log import log as logging from oslo_utils import strutils from oslo_utils import uuidutils +from masakari.compute import nova import masakari.conf from masakari.engine import rpcapi as engine_rpcapi from masakari import exception @@ -166,6 +167,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) host.create() return host diff --git a/masakari/tests/unit/compute/test_nova.py b/masakari/tests/unit/compute/test_nova.py index 00b4472f..86eb48ee 100644 --- a/masakari/tests/unit/compute/test_nova.py +++ b/masakari/tests/unit/compute/test_nova.py @@ -293,3 +293,28 @@ class NovaApiTestCase(test.TestCase): mock_novaclient.assert_called_once_with(self.ctx) 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') diff --git a/masakari/tests/unit/ha/test_api.py b/masakari/tests/unit/ha/test_api.py index 36629ea3..98e84629 100644 --- a/masakari/tests/unit/ha/test_api.py +++ b/masakari/tests/unit/ha/test_api.py @@ -19,6 +19,7 @@ import copy import mock from oslo_utils import timeutils +from masakari.compute import nova as nova_obj from masakari.engine import rpcapi as engine_rpcapi from masakari import exception 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, 'create') + @mock.patch.object(nova_obj.API, 'hypervisor_search') @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 + host_data = { - "name": "host-1", "type": "fake-type", + "name": 'host-1', "type": "fake-type", "reserved": False, "on_maintenance": False, "control_attributes": "fake-control_attributes" @@ -270,11 +274,33 @@ class HostAPITestCase(test.NoDBTestCase): host_data) 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('masakari.db.host_create') @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') def test_create_convert_boolean_attributes(self, mock_get_segment, + mock_hypervisor_search, mock__from_db_object, mock_host_create, mock_generate_uuid):