Flavor extra specs extension use instance_type id

bug 1031263

The nova database API use instance_type id as parameter to access flavor
extra spec. However, the flavor extra_specs API extension use flavor_id as
parameter wrongly.

As the instance_type ID is a purely nova internal ID, the database should
not expose it and instead use flavor_id as the parameter

Change-Id: I5f509cb7c4457d8c399df32f559a874d498be762
Signed-off-by: Yunhong, Jiang <yunhong.jiang@intel.com>

5 files changed, 51 insertions, 37 deletions

diff --git a/bin/nova-manage b/bin/nova-manage
index 1bd0691f4..020cebbb1 100755
--- a/bin/nova-manage
+++ b/bin/nova-manage
@@ -1003,9 +1003,10 @@ class InstanceTypeCommands(object):
 
             ctxt = context.get_admin_context()
             ext_spec = {key: value}
-            db.instance_type_extra_specs_update_or_create(ctxt,
-                                                          inst_type["id"],
-                                                          ext_spec)
+            db.instance_type_extra_specs_update_or_create(
+                            ctxt,
+                            inst_type["flavorid"],
+                            ext_spec)
             print _("Key %(key)s set to %(value)s on instance"
                             " type %(name)s") % locals()
         except exception.DBError, e:
@@ -1025,7 +1026,10 @@ class InstanceTypeCommands(object):
                 sys.exit(2)
 
             ctxt = context.get_admin_context()
-            db.instance_type_extra_specs_delete(ctxt, inst_type["id"], key)
+            db.instance_type_extra_specs_delete(
+                        ctxt,
+                        inst_type["flavorid"],
+                        key)
 
             print _("Key %(key)s on instance type %(name)s unset") % locals()
         except exception.DBError, e:
diff --git a/nova/db/api.py b/nova/db/api.py
index 5e390d9f9..d270ba9dd 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -1542,21 +1542,21 @@ def bw_usage_update(context,
 ####################
 
 
-def instance_type_extra_specs_get(context, instance_type_id):
+def instance_type_extra_specs_get(context, flavor_id):
     """Get all extra specs for an instance type."""
-    return IMPL.instance_type_extra_specs_get(context, instance_type_id)
+    return IMPL.instance_type_extra_specs_get(context, flavor_id)
 
 
-def instance_type_extra_specs_delete(context, instance_type_id, key):
+def instance_type_extra_specs_delete(context, flavor_id, key):
     """Delete the given extra specs item."""
-    IMPL.instance_type_extra_specs_delete(context, instance_type_id, key)
+    IMPL.instance_type_extra_specs_delete(context, flavor_id, key)
 
 
-def instance_type_extra_specs_update_or_create(context, instance_type_id,
+def instance_type_extra_specs_update_or_create(context, flavor_id,
                                                extra_specs):
     """Create or update instance type extra specs. This adds or modifies the
     key/value pairs specified in the extra specs dict argument"""
-    IMPL.instance_type_extra_specs_update_or_create(context, instance_type_id,
+    IMPL.instance_type_extra_specs_update_or_create(context, flavor_id,
                                                     extra_specs)
 
 
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 1f5b07643..36e39868b 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -4193,17 +4193,23 @@ def bw_usage_update(context,
 ####################
 
 
-def _instance_type_extra_specs_get_query(context, instance_type_id,
+def _instance_type_extra_specs_get_query(context, flavor_id,
                                          session=None):
+    # Two queries necessary because join with update doesn't work.
+    t = model_query(context, models.InstanceTypes.id,
+                    session=session, read_deleted="no").\
+              filter(models.InstanceTypes.flavorid == flavor_id).\
+              subquery()
     return model_query(context, models.InstanceTypeExtraSpecs,
                        session=session, read_deleted="no").\
-                    filter_by(instance_type_id=instance_type_id)
+                       filter(models.InstanceTypeExtraSpecs.\
+                              instance_type_id.in_(t))
 
 
 @require_context
-def instance_type_extra_specs_get(context, instance_type_id):
+def instance_type_extra_specs_get(context, flavor_id):
     rows = _instance_type_extra_specs_get_query(
-                            context, instance_type_id).\
+                            context, flavor_id).\
                     all()
 
     result = {}
@@ -4214,43 +4220,45 @@ def instance_type_extra_specs_get(context, instance_type_id):
 
 
 @require_context
-def instance_type_extra_specs_delete(context, instance_type_id, key):
+def instance_type_extra_specs_delete(context, flavor_id, key):
+    # Don't need synchronize the session since we will not use the query result
     _instance_type_extra_specs_get_query(
-                            context, instance_type_id).\
-        filter_by(key=key).\
+                            context, flavor_id).\
+        filter(models.InstanceTypeExtraSpecs.key == key).\
         update({'deleted': True,
                 'deleted_at': timeutils.utcnow(),
-                'updated_at': literal_column('updated_at')})
+                'updated_at': literal_column('updated_at')},
+                synchronize_session=False)
 
 
 @require_context
-def instance_type_extra_specs_get_item(context, instance_type_id, key,
+def instance_type_extra_specs_get_item(context, flavor_id, key,
                                        session=None):
     result = _instance_type_extra_specs_get_query(
-                            context, instance_type_id, session=session).\
-                    filter_by(key=key).\
+                            context, flavor_id, session=session).\
+                    filter(models.InstanceTypeExtraSpecs.key == key).\
                     first()
-
     if not result:
         raise exception.InstanceTypeExtraSpecsNotFound(
-                extra_specs_key=key, instance_type_id=instance_type_id)
+                extra_specs_key=key, instance_type_id=flavor_id)
 
     return result
 
 
 @require_context
-def instance_type_extra_specs_update_or_create(context, instance_type_id,
+def instance_type_extra_specs_update_or_create(context, flavor_id,
                                                specs):
     session = get_session()
     spec_ref = None
+    instance_type = instance_type_get_by_flavor_id(context, flavor_id)
     for key, value in specs.iteritems():
         try:
             spec_ref = instance_type_extra_specs_get_item(
-                context, instance_type_id, key, session)
+                context, flavor_id, key, session)
         except exception.InstanceTypeExtraSpecsNotFound, e:
             spec_ref = models.InstanceTypeExtraSpecs()
         spec_ref.update({"key": key, "value": value,
-                         "instance_type_id": instance_type_id,
+                         "instance_type_id": instance_type["id"],
                          "deleted": 0})
         spec_ref.save(session=session)
     return specs
diff --git a/nova/tests/test_instance_types_extra_specs.py b/nova/tests/test_instance_types_extra_specs.py
index e78d6d0b3..f53840b86 100644
--- a/nova/tests/test_instance_types_extra_specs.py
+++ b/nova/tests/test_instance_types_extra_specs.py
@@ -41,6 +41,7 @@ class InstanceTypeExtraSpecsTestCase(test.TestCase):
         ref = db.instance_type_create(self.context,
                                           values)
         self.instance_type_id = ref["id"]
+        self.flavorid = ref["flavorid"]
 
     def tearDown(self):
         # Remove the instance type from the database
@@ -55,7 +56,7 @@ class InstanceTypeExtraSpecsTestCase(test.TestCase):
                                  xpu_model="Tesla 2050")
         actual_specs = db.instance_type_extra_specs_get(
                               self.context,
-                              self.instance_type_id)
+                              self.flavorid)
         self.assertEquals(expected_specs, actual_specs)
 
     def test_instance_type_extra_specs_delete(self):
@@ -64,11 +65,11 @@ class InstanceTypeExtraSpecsTestCase(test.TestCase):
                                  xpu_arch="fermi",
                                  xpus="2")
         db.instance_type_extra_specs_delete(self.context,
-                                      self.instance_type_id,
+                                      self.flavorid,
                                       "xpu_model")
         actual_specs = db.instance_type_extra_specs_get(
                               self.context,
-                              self.instance_type_id)
+                              self.flavorid)
         self.assertEquals(expected_specs, actual_specs)
 
     def test_instance_type_extra_specs_update(self):
@@ -79,11 +80,11 @@ class InstanceTypeExtraSpecsTestCase(test.TestCase):
                                  xpu_model="Tesla 2050")
         db.instance_type_extra_specs_update_or_create(
                               self.context,
-                              self.instance_type_id,
+                              self.flavorid,
                               dict(cpu_model="Sandy Bridge"))
         actual_specs = db.instance_type_extra_specs_get(
                               self.context,
-                              self.instance_type_id)
+                              self.flavorid)
         self.assertEquals(expected_specs, actual_specs)
 
     def test_instance_type_extra_specs_create(self):
@@ -96,12 +97,12 @@ class InstanceTypeExtraSpecsTestCase(test.TestCase):
                                  net_mbps="10000")
         db.instance_type_extra_specs_update_or_create(
                               self.context,
-                              self.instance_type_id,
+                              self.flavorid,
                               dict(net_arch="ethernet",
                                    net_mbps=10000))
         actual_specs = db.instance_type_extra_specs_get(
                               self.context,
-                              self.instance_type_id)
+                              self.flavorid)
         self.assertEquals(expected_specs, actual_specs)
 
     def test_instance_type_get_with_extra_specs(self):
diff --git a/nova/tests/test_nova_manage.py b/nova/tests/test_nova_manage.py
index e0431360f..805ac8e0d 100644
--- a/nova/tests/test_nova_manage.py
+++ b/nova/tests/test_nova_manage.py
@@ -284,6 +284,7 @@ class InstanceTypeCommandsTestCase(test.TestCase):
                                       values)
         self.instance_type_name = ref["name"]
         self.instance_type_id = ref["id"]
+        self.instance_type_flavorid = ref["flavorid"]
         self.set_key = nova_manage.InstanceTypeCommands().set_key
         self.unset_key = nova_manage.InstanceTypeCommands().unset_key
 
@@ -307,7 +308,7 @@ class InstanceTypeCommandsTestCase(test.TestCase):
         self.set_key(self.instance_type_name, "k1", "v1")
         actual_specs = db.instance_type_extra_specs_get(
                               context.get_admin_context(),
-                              self.instance_type_id)
+                              self.instance_type_flavorid)
         self.assertEquals(expected_specs, actual_specs)
 
         self.unset_key(self.instance_type_name, "k1")
@@ -322,13 +323,13 @@ class InstanceTypeCommandsTestCase(test.TestCase):
         self.set_key(self.instance_type_name, "k1", "v1")
         actual_specs = db.instance_type_extra_specs_get(
                               context.get_admin_context(),
-                              self.instance_type_id)
+                              self.instance_type_flavorid)
         self.assertEquals(expected_specs, actual_specs)
 
         self.set_key(self.instance_type_name, "k1", "v2")
         actual_specs = db.instance_type_extra_specs_get(
                               context.get_admin_context(),
-                              self.instance_type_id)
+                              self.instance_type_flavorid)
         self.assertEquals(updated_specs, actual_specs)
 
         self.unset_key(self.instance_type_name, "k1")
@@ -343,7 +344,7 @@ class InstanceTypeCommandsTestCase(test.TestCase):
         self.set_key(self.instance_type_name, "k3", "v3")
         actual_specs = db.instance_type_extra_specs_get(
                               context.get_admin_context(),
-                              self.instance_type_id)
+                              self.instance_type_flavorid)
         self.assertEquals(two_items_extra_specs, actual_specs)
 
         self.unset_key(self.instance_type_name, "k1")