From 0162a42970b833c2d5d0802ff4c55f65fa253ee2 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Wed, 12 Oct 2011 16:28:24 -0400 Subject: Convert instancetype.flavorid to string Fixes bug 861666. This also removes some direct database access in favor of using nova.compute.instance_types throughout the code. Change-Id: I572cc19454fa76f435f5672d3d6e7ed55c8817da --- nova/tests/api/openstack/test_flavors.py | 32 +++--- nova/tests/api/openstack/test_servers.py | 8 +- nova/tests/test_compute.py | 6 +- nova/tests/test_instance_types.py | 186 ++++++++++++++++++++----------- 4 files changed, 147 insertions(+), 85 deletions(-) (limited to 'nova/tests') diff --git a/nova/tests/api/openstack/test_flavors.py b/nova/tests/api/openstack/test_flavors.py index 6cea114b1..703009e3c 100644 --- a/nova/tests/api/openstack/test_flavors.py +++ b/nova/tests/api/openstack/test_flavors.py @@ -16,14 +16,15 @@ # under the License. import json -import webob + from lxml import etree +import webob from nova.api.openstack import flavors -import nova.db.api +from nova.api.openstack import xmlutil +import nova.compute.instance_types from nova import exception from nova import test -from nova.api.openstack import xmlutil from nova.tests.api.openstack import fakes from nova import wsgi @@ -48,30 +49,33 @@ FAKE_FLAVORS = { } -def fake_instance_type_get_by_flavor_id(context, flavorid): +def fake_instance_type_get_by_flavor_id(flavorid): return FAKE_FLAVORS['flavor %s' % flavorid] -def fake_instance_type_get_all(context, inactive=False, filters=None): +def fake_instance_type_get_all(inactive=False, filters=None): def reject_min(db_attr, filter_attr): return filter_attr in filters and\ int(flavor[db_attr]) < int(filters[filter_attr]) filters = filters or {} - for flavor in FAKE_FLAVORS.values(): + output = {} + for (flavor_name, flavor) in FAKE_FLAVORS.items(): if reject_min('memory_mb', 'min_memory_mb'): continue elif reject_min('local_gb', 'min_local_gb'): continue - yield flavor + output[flavor_name] = flavor + + return output -def empty_instance_type_get_all(context, inactive=False, filters=None): +def empty_instance_type_get_all(inactive=False, filters=None): return {} -def return_instance_type_not_found(context, flavor_id): +def return_instance_type_not_found(flavor_id): raise exception.InstanceTypeNotFound(flavor_id=flavor_id) @@ -80,9 +84,10 @@ class FlavorsTest(test.TestCase): super(FlavorsTest, self).setUp() fakes.stub_out_networking(self.stubs) fakes.stub_out_rate_limiting(self.stubs) - self.stubs.Set(nova.db.api, "instance_type_get_all", + self.stubs.Set(nova.compute.instance_types, "get_all_types", fake_instance_type_get_all) - self.stubs.Set(nova.db.api, "instance_type_get_by_flavor_id", + self.stubs.Set(nova.compute.instance_types, + "get_instance_type_by_flavor_id", fake_instance_type_get_by_flavor_id) def tearDown(self): @@ -90,7 +95,8 @@ class FlavorsTest(test.TestCase): super(FlavorsTest, self).tearDown() def test_get_flavor_by_invalid_id(self): - self.stubs.Set(nova.db.api, "instance_type_get_by_flavor_id", + self.stubs.Set(nova.compute.instance_types, + "get_instance_type_by_flavor_id", return_instance_type_not_found) req = webob.Request.blank('/v1.1/fake/flavors/asdf') res = req.get_response(fakes.wsgi_app()) @@ -216,7 +222,7 @@ class FlavorsTest(test.TestCase): self.assertEqual(flavor, expected) def test_get_empty_flavor_list(self): - self.stubs.Set(nova.db.api, "instance_type_get_all", + self.stubs.Set(nova.compute.instance_types, "get_all_types", empty_instance_type_get_all) req = webob.Request.blank('/v1.1/fake/flavors') diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index 8ca45297e..f3c61e48e 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -19,11 +19,11 @@ import base64 import datetime import json -from lxml import etree import unittest import urlparse from xml.dom import minidom +from lxml import etree import webob import nova.api.openstack @@ -40,13 +40,13 @@ import nova.db.api from nova.db.sqlalchemy.models import InstanceMetadata from nova import exception from nova import flags +import nova.image.fake +import nova.rpc import nova.scheduler.api from nova import test from nova.tests.api.openstack import common from nova.tests.api.openstack import fakes from nova import utils -import nova.image.fake -import nova.rpc FLAGS = flags.FLAGS @@ -2615,7 +2615,7 @@ class ServersViewBuilderTest(test.TestCase): "hostname": "", "host": "", "instance_type": { - "flavorid": 1, + "flavorid": '1', }, "user_data": "", "reservation_id": "", diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py index d2f0b2193..dafaea54a 100644 --- a/nova/tests/test_compute.py +++ b/nova/tests/test_compute.py @@ -695,7 +695,7 @@ class ComputeTestCase(test.TestCase): inst_ref = db.instance_get(context, instance_id) instance_type_ref = db.instance_type_get(context, inst_ref['instance_type_id']) - self.assertEqual(instance_type_ref['flavorid'], 1) + self.assertEqual(instance_type_ref['flavorid'], '1') db.instance_update(self.context, instance_id, {'host': 'foo'}) @@ -715,7 +715,7 @@ class ComputeTestCase(test.TestCase): inst_ref = db.instance_get(context, instance_id) instance_type_ref = db.instance_type_get(context, inst_ref['instance_type_id']) - self.assertEqual(instance_type_ref['flavorid'], 3) + self.assertEqual(instance_type_ref['flavorid'], '3') # Finally, revert and confirm the old flavor has been applied self.compute.revert_resize(context, inst_ref['uuid'], @@ -726,7 +726,7 @@ class ComputeTestCase(test.TestCase): inst_ref = db.instance_get(context, instance_id) instance_type_ref = db.instance_type_get(context, inst_ref['instance_type_id']) - self.assertEqual(instance_type_ref['flavorid'], 1) + self.assertEqual(instance_type_ref['flavorid'], '1') self.compute.terminate_instance(context, instance_id) diff --git a/nova/tests/test_instance_types.py b/nova/tests/test_instance_types.py index 1ed34fba3..a3fb8a049 100644 --- a/nova/tests/test_instance_types.py +++ b/nova/tests/test_instance_types.py @@ -37,27 +37,18 @@ class InstanceTypeTestCase(test.TestCase): def setUp(self): super(InstanceTypeTestCase, self).setUp() session = get_session() - max_flavorid = session.query(models.InstanceTypes).\ - order_by("flavorid desc").\ - first() - max_id = session.query(models.InstanceTypes).\ - order_by("id desc").\ - first() - self.flavorid = max_flavorid["flavorid"] + 1 - self.id = max_id["id"] + 1 - self.name = str(int(time.time())) - - def _nonexistent_flavor_name(self): - """return an instance type name not in the DB""" - nonexistent_flavor = "sdfsfsdf" + + def _generate_name(self): + """return a name not in the DB""" + nonexistent_flavor = str(int(time.time())) flavors = instance_types.get_all_types() while nonexistent_flavor in flavors: nonexistent_flavor += "z" else: return nonexistent_flavor - def _nonexistent_flavor_id(self): - """return an instance type ID not in the DB""" + def _generate_flavorid(self): + """return a flavorid not in the DB""" nonexistent_flavor = 2700 flavor_ids = [value["id"] for key, value in\ instance_types.get_all_types().iteritems()] @@ -72,95 +63,160 @@ class InstanceTypeTestCase(test.TestCase): def test_instance_type_create_then_delete(self): """Ensure instance types can be created""" - starting_inst_list = instance_types.get_all_types() - instance_types.create(self.name, 256, 1, 120, self.flavorid) - new = instance_types.get_all_types() - self.assertNotEqual(len(starting_inst_list), - len(new), + name = 'Small Flavor' + flavorid = 'flavor1' + + original_list = instance_types.get_all_types() + + # create new type and make sure values stick + inst_type = instance_types.create(name, 256, 1, 120, flavorid) + inst_type_id = inst_type['id'] + self.assertEqual(inst_type['flavorid'], flavorid) + self.assertEqual(inst_type['name'], name) + self.assertEqual(inst_type['memory_mb'], 256) + self.assertEqual(inst_type['vcpus'], 1) + self.assertEqual(inst_type['local_gb'], 120) + self.assertEqual(inst_type['swap'], 0) + self.assertEqual(inst_type['rxtx_quota'], 0) + self.assertEqual(inst_type['rxtx_cap'], 0) + + # make sure new type shows up in list + new_list = instance_types.get_all_types() + self.assertNotEqual(len(original_list), len(new_list), 'instance type was not created') - instance_types.destroy(self.name) - self.assertEqual(1, - instance_types.get_instance_type(self.id)["deleted"]) - self.assertEqual(starting_inst_list, instance_types.get_all_types()) - instance_types.purge(self.name) - self.assertEqual(len(starting_inst_list), - len(instance_types.get_all_types()), + + # destroy instance and make sure deleted flag is set to True + instance_types.destroy(name) + inst_type = instance_types.get_instance_type(inst_type_id) + self.assertEqual(1, inst_type["deleted"]) + + # deleted instance should not be in list anymoer + new_list = instance_types.get_all_types() + self.assertEqual(original_list, new_list) + + # ensure instances are gone after purge + instance_types.purge(name) + new_list = instance_types.get_all_types() + self.assertEqual(original_list, new_list, 'instance type not purged') def test_get_all_instance_types(self): """Ensures that all instance types can be retrieved""" session = get_session() - total_instance_types = session.query(models.InstanceTypes).\ - count() + total_instance_types = session.query(models.InstanceTypes).count() inst_types = instance_types.get_all_types() self.assertEqual(total_instance_types, len(inst_types)) def test_invalid_create_args_should_fail(self): """Ensures that instance type creation fails with invalid args""" - self.assertRaises( - exception.InvalidInput, - instance_types.create, self.name, 0, 1, 120, self.flavorid) - self.assertRaises( - exception.InvalidInput, - instance_types.create, self.name, 256, -1, 120, self.flavorid) - self.assertRaises( - exception.InvalidInput, - instance_types.create, self.name, 256, 1, "aa", self.flavorid) + invalid_sigs = [ + (('Zero memory', 0, 1, 10, 'flavor1'), {}), + (('Negative memory', -256, 1, 10, 'flavor1'), {}), + (('Non-integer memory', 'asdf', 1, 10, 'flavor1'), {}), + + (('Zero vcpus', 256, 0, 10, 'flavor1'), {}), + (('Negative vcpus', 256, -1, 10, 'flavor1'), {}), + (('Non-integer vcpus', 256, 'a', 10, 'flavor1'), {}), + + (('Negative storage', 256, 1, -1, 'flavor1'), {}), + (('Non-integer storage', 256, 1, 'a', 'flavor1'), {}), + + (('Negative swap', 256, 1, 10, 'flavor1'), {'swap': -1}), + (('Non-integer swap', 256, 1, 10, 'flavor1'), {'swap': -1}), + + (('Negative rxtx_quota', 256, 1, 10, 'f1'), {'rxtx_quota': -1}), + (('Non-integer rxtx_quota', 256, 1, 10, 'f1'), {'rxtx_quota': -1}), + + (('Negative rxtx_cap', 256, 1, 10, 'f1'), {'rxtx_cap': -1}), + (('Non-integer rxtx_cap', 256, 1, 10, 'f1'), {'rxtx_cap': 'a'}), + ] + + for (args, kwargs) in invalid_sigs: + self.assertRaises(exception.InvalidInput, + instance_types.create, *args, **kwargs) def test_non_existent_inst_type_shouldnt_delete(self): """Ensures that instance type creation fails with invalid args""" - self.assertRaises(exception.ApiError, + self.assertRaises(exception.InstanceTypeNotFoundByName, instance_types.destroy, - self._nonexistent_flavor_name()) + 'unknown_flavor') + + def test_duplicate_names_fail(self): + """Ensures that name duplicates raise ApiError""" + name = 'some_name' + instance_types.create(name, 256, 1, 120, 'flavor1') + self.assertRaises(exception.ApiError, + instance_types.create, + name, 256, 1, 120, 'flavor2') - def test_repeated_inst_types_should_raise_api_error(self): - """Ensures that instance duplicates raises ApiError""" - new_name = self.name + "dup" - instance_types.create(new_name, 256, 1, 120, self.flavorid + 1) - instance_types.destroy(new_name) - self.assertRaises( - exception.ApiError, - instance_types.create, new_name, 256, 1, 120, self.flavorid) + def test_duplicate_flavorids_fail(self): + """Ensures that flavorid duplicates raise ApiError""" + flavorid = 'flavor1' + instance_types.create('name one', 256, 1, 120, flavorid) + self.assertRaises(exception.ApiError, + instance_types.create, + 'name two', 256, 1, 120, flavorid) def test_will_not_destroy_with_no_name(self): """Ensure destroy sad path of no name raises error""" - self.assertRaises(exception.ApiError, - instance_types.destroy, - self._nonexistent_flavor_name()) + self.assertRaises(exception.InstanceTypeNotFoundByName, + instance_types.destroy, None) def test_will_not_purge_without_name(self): """Ensure purge without a name raises error""" - self.assertRaises(exception.InvalidInstanceType, + self.assertRaises(exception.InstanceTypeNotFoundByName, instance_types.purge, None) def test_will_not_purge_with_wrong_name(self): """Ensure purge without correct name raises error""" - self.assertRaises(exception.ApiError, + self.assertRaises(exception.InstanceTypeNotFound, instance_types.purge, - self._nonexistent_flavor_name()) + 'unknown_flavor') def test_will_not_get_bad_default_instance_type(self): """ensures error raised on bad default instance type""" - FLAGS.default_instance_type = self._nonexistent_flavor_name() + FLAGS.default_instance_type = 'unknown_flavor' self.assertRaises(exception.InstanceTypeNotFoundByName, instance_types.get_default_instance_type) - def test_will_not_get_instance_type_by_name_with_no_name(self): + def test_will_get_instance_type_by_id(self): + default_instance_type = instance_types.get_default_instance_type() + instance_type_id = default_instance_type['id'] + fetched = instance_types.get_instance_type(instance_type_id) + self.assertEqual(default_instance_type, fetched) + + def test_will_not_get_instance_type_by_unknown_id(self): """Ensure get by name returns default flavor with no name""" - self.assertEqual(instance_types.get_default_instance_type(), - instance_types.get_instance_type_by_name(None)) + self.assertRaises(exception.InstanceTypeNotFound, + instance_types.get_instance_type, 10000) - def test_will_not_get_instance_type_with_bad_name(self): + def test_will_not_get_instance_type_with_bad_id(self): """Ensure get by name returns default flavor with bad name""" self.assertRaises(exception.InstanceTypeNotFound, - instance_types.get_instance_type, - self._nonexistent_flavor_name()) + instance_types.get_instance_type, 'asdf') - def test_will_not_get_flavor_by_bad_flavor_id(self): + def test_instance_type_get_by_None_name_returns_default(self): + """Ensure get by name returns default flavor with no name""" + default = instance_types.get_default_instance_type() + actual = instance_types.get_instance_type_by_name(None) + self.assertEqual(default, actual) + + def test_will_not_get_instance_type_with_bad_name(self): + """Ensure get by name returns default flavor with bad name""" + self.assertRaises(exception.InstanceTypeNotFoundByName, + instance_types.get_instance_type_by_name, 10000) + + def test_will_not_get_instance_by_unknown_flavor_id(self): """Ensure get by flavor raises error with wrong flavorid""" - self.assertRaises(exception.InstanceTypeNotFound, - instance_types.get_instance_type_by_name, - self._nonexistent_flavor_id()) + self.assertRaises(exception.FlavorNotFound, + instance_types.get_instance_type_by_flavor_id, + 'unknown_flavor') + + def test_will_get_instance_by_flavor_id(self): + default_instance_type = instance_types.get_default_instance_type() + flavorid = default_instance_type['flavorid'] + fetched = instance_types.get_instance_type_by_flavor_id(flavorid) + self.assertEqual(default_instance_type, fetched) class InstanceTypeFilteringTest(test.TestCase): -- cgit