From 9575a426fa4adcd79f1fa0c3a1d1008d8010c44c Mon Sep 17 00:00:00 2001 From: "Yunhong, Jiang" Date: Tue, 31 Jul 2012 18:15:59 +0800 Subject: [PATCH] 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 --- bin/nova-manage | 12 ++++-- nova/db/api.py | 12 +++--- nova/db/sqlalchemy/api.py | 40 +++++++++++-------- nova/tests/test_instance_types_extra_specs.py | 15 +++---- nova/tests/test_nova_manage.py | 9 +++-- 5 files changed, 51 insertions(+), 37 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index 1bd0691f496c..020cebbb1695 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 5e390d9f9a18..d270ba9dda95 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 1f5b076438c3..36e39868bc9b 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 e78d6d0b36f3..f53840b868b0 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 e0431360f37b..805ac8e0d6d2 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")