From 5d4550122d265f8ada934b8570d555ac94234ce9 Mon Sep 17 00:00:00 2001 From: Abitha Palaniappan Date: Tue, 22 Dec 2015 12:58:14 -0800 Subject: [PATCH] Validate cluster flavor with image metadata cue cluster creation fails if the flavor disk size is too small for the broker image.this fix validates the flavor disk size with image metadata ,raising the exception from api instead of nova. Change-Id: I18330c66cddf527d5fddfdbc29656e0313b04e16 --- cue/api/controllers/v1/cluster.py | 73 +++++++++++++++++---- cue/common/exception.py | 5 ++ cue/tests/functional/api/__init__.py | 4 ++ cue/tests/functional/api/v1/test_cluster.py | 58 ++++++++++++++++ cue/tests/functional/fixtures/nova.py | 51 +++++++++++--- cue/tests/functional/utils.py | 4 +- devstack/plugin.sh | 2 +- 7 files changed, 173 insertions(+), 24 deletions(-) diff --git a/cue/api/controllers/v1/cluster.py b/cue/api/controllers/v1/cluster.py index e3523f40..de914cca 100644 --- a/cue/api/controllers/v1/cluster.py +++ b/cue/api/controllers/v1/cluster.py @@ -20,6 +20,7 @@ """ import sys +from novaclient import exceptions as nova_exc from oslo_config import cfg from oslo_log import log as logging from oslo_utils import uuidutils @@ -31,6 +32,7 @@ from wsme import types as wtypes import wsmeext.pecan as wsme_pecan from cue.api.controllers import base +import cue.client as client from cue.common import exception from cue.common.i18n import _ # noqa from cue.common.i18n import _LI # noqa @@ -198,6 +200,49 @@ def delete_complete_cluster(context, cluster_id): class ClusterController(rest.RestController): """Manages operations on specific Cluster of nodes.""" + def _validate_flavor(self, image_id, cluster_flavor): + """Checks if flavor satisfies minimum requirement of image metadata. + + :param image_id: image id of the broker. + :param cluster_flavor: flavor id of the cluster. + :raises: exception.ConfigurationError + :raises: exception.InternalServerError + :raises: exception.Invalid + """ + nova_client = client.nova_client() + + # get image metadata + try: + image_metadata = nova_client.images.get(image_id) + image_minRam = image_metadata.minRam + image_minDisk = image_metadata.minDisk + except nova_exc.ClientException as ex: + if ex.http_status == 404: + raise exception.ConfigurationError(_('Invalid image %s ' + 'configured') % image_id) + else: + raise exception.InternalServerError + + # get flavor metadata + try: + flavor_metadata = nova_client.flavors.get(cluster_flavor) + flavor_ram = flavor_metadata.ram + flavor_disk = flavor_metadata.disk + except nova_exc.ClientException as ex: + if ex.http_status == 404: + raise exception.Invalid(_('Invalid flavor %s provided') % + cluster_flavor) + else: + raise exception.InternalServerError + + # validate flavor with broker image metadata + if (flavor_disk < image_minDisk): + raise exception.Invalid(_("Flavor disk is smaller than the " + "minimum %s required for broker") % image_minDisk) + elif (flavor_ram < image_minRam): + raise exception.Invalid(_("Flavor ram is smaller than the " + "minimum %s required for broker") % image_minRam) + @wsme_pecan.wsexpose(Cluster, wtypes.text, status_code=200) def get_one(self, cluster_id): """Return this cluster.""" @@ -248,6 +293,8 @@ class ClusterController(rest.RestController): :param data: cluster parameters within the request body. """ context = pecan.request.context + request_data = data.as_dict() + cluster_flavor = request_data['flavor'] if data.size <= 0: raise exception.Invalid(_("Invalid cluster size provided")) @@ -275,7 +322,14 @@ class ClusterController(rest.RestController): default_rabbit_user = data.authentication.token['username'] default_rabbit_pass = data.authentication.token['password'] - request_data = data.as_dict() + broker_name = CONF.default_broker_name + + # get the image id of default broker + image_id = objects.BrokerMetadata.get_image_id_by_broker_name( + context, broker_name) + + # validate cluster flavor + self._validate_flavor(image_id, cluster_flavor) # convert 'network_id' from list to string type for objects/cluster # compatibility @@ -307,11 +361,6 @@ class ClusterController(rest.RestController): # generate unique erlang cookie to be used by all nodes in the new # cluster, erlang cookies are strings of up to 255 characters erlang_cookie = uuidutils.generate_uuid() - broker_name = CONF.default_broker_name - - # get the image id of default broker - image_id = objects.BrokerMetadata.get_image_id_by_broker_name( - context, broker_name) job_args = { 'tenant_id': new_cluster.project_id, @@ -320,9 +369,9 @@ class ClusterController(rest.RestController): 'volume_size': cluster.volume_size, 'port': '5672', 'context': context.to_dict(), - # TODO(sputnik13: this needs to come from the create request and - # default to a configuration value rather than always using config - # value + # TODO(sputnik13: this needs to come from the create request + # and default to a configuration value rather than always using + # config value 'security_groups': [CONF.os_security_group], 'port': CONF.rabbit_port, 'key_name': CONF.openstack.os_key_name, @@ -337,9 +386,9 @@ class ClusterController(rest.RestController): flow_kwargs=flow_kwargs, tx_uuid=job_uuid) - LOG.info(_LI('Create Cluster Request Cluster ID %(cluster_id)s Cluster' - ' size %(size)s network ID %(network_id)s Job ID ' - '%(job_id)s Broker name %(broker_name)s') % ( + LOG.info(_LI('Create Cluster Request Cluster ID %(cluster_id)s ' + 'Cluster size %(size)s network ID %(network_id)s ' + 'Job ID %(job_id)s Broker name %(broker_name)s') % ( {"cluster_id": cluster.id, "size": cluster.size, "network_id": diff --git a/cue/common/exception.py b/cue/common/exception.py index 569402be..bfe71adc 100644 --- a/cue/common/exception.py +++ b/cue/common/exception.py @@ -149,3 +149,8 @@ class VmBuildingException(CueException): class VmErrorException(CueException): message = _("VM is not in a building state") + + +class InternalServerError(CueException): + message = _("Internal Server Error") + code = 500 diff --git a/cue/tests/functional/api/__init__.py b/cue/tests/functional/api/__init__.py index f3cdfe50..e356dc48 100644 --- a/cue/tests/functional/api/__init__.py +++ b/cue/tests/functional/api/__init__.py @@ -23,6 +23,7 @@ import pecan.testing from cue.api import hooks from cue.tests.functional import base +from cue.tests.functional.fixtures import nova OPT_GROUP_NAME = 'keystone_authtoken' cfg.CONF.import_group(OPT_GROUP_NAME, "keystonemiddleware.auth_token") @@ -36,6 +37,9 @@ class APITest(base.FunctionalTestCase): """ PATH_PREFIX = '' + additional_fixtures = [ + nova.NovaClient + ] def setUp(self): super(APITest, self).setUp() diff --git a/cue/tests/functional/api/v1/test_cluster.py b/cue/tests/functional/api/v1/test_cluster.py index 0aeec6b0..569229f0 100644 --- a/cue/tests/functional/api/v1/test_cluster.py +++ b/cue/tests/functional/api/v1/test_cluster.py @@ -503,6 +503,64 @@ class TestCreateCluster(api.APITest, self.assertEqual(400, data.status_code, 'Invalid status code value received.') + def test_create_flavor_too_small_disk(self): + """test create a cluster with flavor having too small disk for + + image. + """ + api_cluster = test_utils.create_api_test_cluster(flavor='x-tiny-disk') + data = self.post_json('/clusters', params=api_cluster, + headers=self.auth_headers, expect_errors=True) + + self.assertEqual(400, data.status_code, + 'Invalid status code value received.') + + def test_create_flavor_too_small_ram(self): + """test create a cluster with flavor having too small ram for image.""" + api_cluster = test_utils.create_api_test_cluster(flavor='x-tiny-ram') + data = self.post_json('/clusters', params=api_cluster, + headers=self.auth_headers, expect_errors=True) + + self.assertEqual(400, data.status_code, + 'Invalid status code value received.') + + def test_create_flavor_invalid(self): + """test create a cluster with invalid flavor.""" + api_cluster = test_utils.create_api_test_cluster( + flavor='invalid_flavor') + + data = self.post_json('/clusters', params=api_cluster, + headers=self.auth_headers, expect_errors=True) + + self.assertEqual(400, data.status_code, + 'Invalid status code value received.') + + def test_create_invalid_image(self): + """test create a cluster with invalid image configured.""" + # add a new broker image that is not in nova image-list + self.CONF.config(default_broker_name='rabbitmq1') + broker_values = { + 'name': 'rabbitmq1', + 'active': '1', + } + broker = objects.Broker(**broker_values) + broker.create_broker(None) + broker_list = broker.get_brokers(None) + metadata_value = { + 'key': 'IMAGE', + 'value': 'ea329926-b8bf-11e5-9912-ba0be0483c18', + 'broker_id': broker_list[1]['id'] + } + metadata = objects.BrokerMetadata(**metadata_value) + metadata.create_broker_metadata(None) + + api_cluster = test_utils.create_api_test_cluster() + data = self.post_json('/clusters', params=api_cluster, + headers=self.auth_headers, expect_errors=True) + + self.assertEqual(500, data.status_code, + 'Invalid status code value received.') + def test_create_invalid_volume_size(self): """test with invalid volume_size parameter.""" diff --git a/cue/tests/functional/fixtures/nova.py b/cue/tests/functional/fixtures/nova.py index edb0a914..781d7467 100644 --- a/cue/tests/functional/fixtures/nova.py +++ b/cue/tests/functional/fixtures/nova.py @@ -50,7 +50,7 @@ class ImageDetails(object): updated=None): self.created = created self.human_id = human_id - self.id = id + self.id = id or str(uuid.uuid4()) self.minDisk = minDisk self.minRam = minRam self.name = name @@ -140,7 +140,14 @@ class NovaClient(base.BaseFixture): image_list = ['cirros-0.3.2-x86_64-uec-kernel'] if not flavor_list: - flavor_list = ['m1.tiny'] + flavor_tiny = FlavorDetails(name='m1.tiny') + flavor_tiny_disk = FlavorDetails(name='x-tiny-disk', disk=1) + flavor_tiny_ram = FlavorDetails(name='x-tiny-ram', ram=256) + flavor_list = [flavor_tiny, flavor_tiny_disk, flavor_tiny_ram] + for flavor in flavor_list: + self._flavor_list.update({ + flavor.id: flavor + }) if not security_group_list: security_group_list = [] @@ -148,17 +155,13 @@ class NovaClient(base.BaseFixture): self._vm_limit = vm_limit if vm_limit else 3 for image in image_list: - image_detail = ImageDetails(name=image) + image_detail = ImageDetails(name=image, + id='f7e8c49b-7d1e-472f-a78b-7c46a39c85be', + minDisk=4, minRam=512) self._image_list.update({ image_detail.id: image_detail }) - for flavor in flavor_list: - flavor_detail = FlavorDetails(name=flavor) - self._flavor_list.update({ - flavor_detail.id: flavor_detail - }) - self._security_group_list = security_group_list def setUp(self): @@ -173,7 +176,9 @@ class NovaClient(base.BaseFixture): v2_client.servers.interface_list = self.list_interfaces v2_client.images.find = self.find_images v2_client.images.list = self.list_images + v2_client.images.get = self.get_image v2_client.flavors.find = self.find_flavors + v2_client.flavors.get = self.get_flavor v2_client.server_groups.create = self.create_vm_group v2_client.server_groups.delete = self.delete_vm_group v2_client.server_groups.get = self.get_vm_group @@ -318,6 +323,20 @@ class NovaClient(base.BaseFixture): if image_detail.name == name: return image_detail + def get_image(self, id, **kwargs): + """Mock'd version of novaclient...image_get(). + + Gets an image detail based on provided image id + + :param id: Image name. + :return: Image detail matching provided image id. + """ + for image_detail in self._image_list.values(): + if image_detail.id == id: + return image_detail + + raise nova_exc.NotFound(404) + def list_images(self, retrieve_all=True, **kwargs): """Mock'd version of novaclient...list_images(). @@ -340,6 +359,20 @@ class NovaClient(base.BaseFixture): if flavor_detail.name == name: return flavor_detail + def get_flavor(self, name, **kwargs): + """Mock'd version of novaclient...flavors_get(). + + Finds a flavor detail based on provided name. + + :param name: Flavor name. + :return: Flavor detail matching provided flavor name. + """ + for flavor_detail in self._flavor_list.values(): + if flavor_detail.name == name: + return flavor_detail + + raise nova_exc.NotFound(404) + def list_interfaces(self, server, **kwargs): """Mock'd version of novaclient...interface_list(). diff --git a/cue/tests/functional/utils.py b/cue/tests/functional/utils.py index 05cca3f2..a8788aed 100644 --- a/cue/tests/functional/utils.py +++ b/cue/tests/functional/utils.py @@ -29,7 +29,7 @@ def get_test_cluster(**kw): 'network_id': kw.get('network_id', '3dc26c0b-03f2-4d2e-ae87-c02d7f33c788'), 'status': kw.get('status', 'BUILDING'), - 'flavor': kw.get('flavor', 'flavor1'), + 'flavor': kw.get('flavor', 'm1.tiny'), 'size': kw.get('size', 1), 'volume_size': kw.get('volume_size', 10), 'deleted': kw.get('deleted', False), @@ -204,7 +204,7 @@ def get_test_node_dict(**kw): ), 'instance_id': kw.get('instance_id', 'b7cf7433-60f7-4d09-a759-cee12d8a3cb3'), - 'flavor': kw.get('flavor', 'flavor1'), + 'flavor': kw.get('flavor', 'm1.tiny'), 'status': kw.get('status', 'BUILDING'), 'management_ip': kw.get('management_ip', '172.1.1.1'), 'created_at': kw.get('created_at', timeutils.utcnow()), diff --git a/devstack/plugin.sh b/devstack/plugin.sh index 59a81b79..14347e54 100644 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -114,7 +114,7 @@ function disk_image_create_upload { openstack --os-token $token --os-url $GLANCE_SERVICE_PROTOCOL://$GLANCE_HOSTPORT \ image create --container-format bare --disk-format qcow2 --public \ - --file $image_path $image_name + --min-disk 2 --file $image_path $image_name } # Restore xtrace