Merge "Convert instancetype.flavorid to string"

4 files changed, 147 insertions, 85 deletions

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):