openstack/deb-nova
Code Issues Proposed changes

Make flavor-manage api call destroy with Flavor object

Browse Source 
The flavor-manage api delete command currently need 3 separate db calls
to remove a flavor from the db. This is due to the use of flavor name to
identify the db flavor when the api command itself take flavorid as
input.

This change reworks this to only issue a single db call that now use
flavorid all way through to the db. It also gets rid of the in between
flavors module by calling destroy on a Flavor object right from the api.

Additional test changes are made to similarily bypass the flavors module
and instead call destroy() directly on the Flavor objects so the flavors
method becomes unused and thus can be removed.

Change-Id: I62bc11887283201be0bcdcfe9fa58b3f6156c754
This commit is contained in:
Hans Lindgren
2015-11-09 19:29:20 +01:00
parent fe8a119e8d
commit 43e6a06718
11 changed files with 47 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
nova/api/openstack/compute/flavor_manage.py
View File

@@ -20,6 +20,7 @@ from nova.api import validation
from nova.compute import flavors
from nova import exception
from nova.i18n import _
from nova import objects
ALIAS = "os-flavor-manage"
@@ -43,14 +44,12 @@ class FlavorManageController(wsgi.Controller):
context = req.environ['nova.context']
authorize(context)
flavor = objects.Flavor(context=context, flavorid=id)
try:
flavor = flavors.get_flavor_by_flavor_id(
id, ctxt=context, read_deleted="no")
flavor.destroy()
except exception.FlavorNotFound as e:
raise webob.exc.HTTPNotFound(explanation=e.format_message())
flavors.destroy(flavor['name'])
# NOTE(oomichi): Return 200 for backwards compatibility but should be 201
# as this operation complete the creation of flavor resource.
@wsgi.action("create")

8
nova/api/openstack/compute/legacy_v2/contrib/flavormanage.py
View File

@@ -18,6 +18,7 @@ from nova.api.openstack import wsgi
from nova.compute import flavors
from nova import exception
from nova.i18n import _
from nova import objects
authorize = extensions.extension_authorizer('compute', 'flavormanage')
@@ -34,14 +35,13 @@ class FlavorManageController(wsgi.Controller):
def _delete(self, req, id):
context = req.environ['nova.context']
authorize(context)
flavor = objects.Flavor(context=context, flavorid=id)
try:
flavor = flavors.get_flavor_by_flavor_id(
id, ctxt=context, read_deleted="no")
flavor.destroy()
except exception.FlavorNotFound as e:
raise webob.exc.HTTPNotFound(explanation=e.format_message())
flavors.destroy(flavor['name'])
return webob.Response(status_int=202)
@wsgi.action("create")

16
nova/compute/flavors.py
View File

@@ -21,7 +21,6 @@
import re
import uuid
from oslo_log import log as logging
from oslo_utils import strutils
import six
@@ -31,14 +30,11 @@ from nova import context
from nova import db
from nova import exception
from nova.i18n import _
from nova.i18n import _LE
from nova import objects
from nova import utils
CONF = nova.conf.CONF
LOG = logging.getLogger(__name__)
# NOTE(luisg): Flavor names can include non-ascii characters so that users can
# create flavor names in locales that use them, however flavor IDs are limited
# to ascii characters.
@@ -168,18 +164,6 @@ def create(name, memory, vcpus, root_gb, ephemeral_gb=0, flavorid=None,
return flavor
def destroy(name):
"""Marks flavor as deleted."""
try:
if not name:
raise ValueError()
flavor = objects.Flavor(context=context.get_admin_context(), name=name)
flavor.destroy()
except (ValueError, exception.NotFound):
LOG.exception(_LE('Instance type %s not found for deletion'), name)
raise exception.FlavorNotFoundByName(flavor_name=name)
def get_all_flavors_sorted_list(ctxt=None, filters=None, sort_key='flavorid',
sort_dir='asc', limit=None, marker=None):
"""Get all non-deleted flavors as a sorted list.

4
nova/db/api.py
View File

@@ -1527,9 +1527,9 @@ def flavor_get_by_flavor_id(context, id, read_deleted=None):
return IMPL.flavor_get_by_flavor_id(context, id, read_deleted)
def flavor_destroy(context, name):
def flavor_destroy(context, flavor_id):
"""Delete an instance type."""
return IMPL.flavor_destroy(context, name)
return IMPL.flavor_destroy(context, flavor_id)
def flavor_access_get_by_flavor_id(context, flavor_id):

6
nova/db/sqlalchemy/api.py
View File

@@ -5204,13 +5204,13 @@ def flavor_get_by_flavor_id(context, flavor_id, read_deleted):
@main_context_manager.writer
def flavor_destroy(context, name):
def flavor_destroy(context, flavor_id):
"""Marks specific flavor as deleted."""
ref = model_query(context, models.InstanceTypes, read_deleted="no").\
filter_by(name=name).\
filter_by(flavorid=flavor_id).\
first()
if not ref:
raise exception.FlavorNotFoundByName(flavor_name=name)
raise exception.FlavorNotFound(flavor_id=flavor_id)
ref.soft_delete(context.session)
model_query(context, models.InstanceTypeExtraSpecs, read_deleted="no").\

19
nova/objects/flavor.py
View File

@@ -168,20 +168,17 @@ def _flavor_create(context, values):
@db_api.api_context_manager.writer
def _flavor_destroy(context, flavor_id=None, name=None):
def _flavor_destroy(context, flavor_id=None, flavorid=None):
query = context.session.query(api_models.Flavors)
if flavor_id is not None:
query = query.filter(api_models.Flavors.id == flavor_id)
else:
query = query.filter(api_models.Flavors.name == name)
query = query.filter(api_models.Flavors.flavorid == flavorid)
result = query.first()
if not result:
if flavor_id is not None:
raise exception.FlavorNotFound(flavor_id=flavor_id)
else:
raise exception.FlavorNotFoundByName(flavor_name=name)
raise exception.FlavorNotFound(flavor_id=(flavor_id or flavorid))
context.session.query(api_models.FlavorProjects).\
filter_by(flavor_id=result.id).delete()
@@ -576,8 +573,8 @@ class Flavor(base.NovaPersistentObject, base.NovaObject,
self.save_projects(added_projects, deleted_projects)
@staticmethod
def _flavor_destroy(context, flavor_id=None, name=None):
return _flavor_destroy(context, flavor_id=flavor_id, name=name)
def _flavor_destroy(context, flavor_id=None, flavorid=None):
return _flavor_destroy(context, flavor_id=flavor_id, flavorid=flavorid)
@base.remotable
def destroy(self):
@@ -591,9 +588,9 @@ class Flavor(base.NovaPersistentObject, base.NovaObject,
if 'id' in self:
self._flavor_destroy(self._context, flavor_id=self.id)
else:
self._flavor_destroy(self._context, name=self.name)
self._flavor_destroy(self._context, flavorid=self.flavorid)
except exception.FlavorNotFound:
db.flavor_destroy(self._context, self.name)
db.flavor_destroy(self._context, self.flavorid)
@db_api.api_context_manager.reader
@@ -721,7 +718,7 @@ def migrate_flavors(ctxt, count, hard_delete=False):
if hard_delete:
_destroy_flavor_hard(ctxt, flavor.name)
else:
db.flavor_destroy(ctxt, flavor.name)
db.flavor_destroy(ctxt, flavor.flavorid)
except exception.FlavorNotFound:
LOG.warning(_LW('Flavor id %(id)i disappeared during migration'),
{'id': flavor_id})

14
nova/tests/functional/db/test_flavor.py
View File

@@ -61,7 +61,7 @@ class FlavorObjectTestCase(test.NoDBTestCase):
def _delete_main_flavors(self):
flavors = db_api.flavor_get_all(self.context)
for flavor in flavors:
db_api.flavor_destroy(self.context, flavor['name'])
db_api.flavor_destroy(self.context, flavor['flavorid'])
def test_create(self):
self._delete_main_flavors()
@@ -181,13 +181,13 @@ class FlavorObjectTestCase(test.NoDBTestCase):
flavor = objects.Flavor.get_by_flavor_id(self.context, 'mainflavor')
self._test_destroy(flavor)
def test_destroy_missing_flavor_by_name(self):
flavor = objects.Flavor(context=self.context, name='foo')
self.assertRaises(exception.FlavorNotFoundByName,
def test_destroy_missing_flavor_by_flavorid(self):
flavor = objects.Flavor(context=self.context, flavorid='foo')
self.assertRaises(exception.FlavorNotFound,
flavor.destroy)
def test_destroy_missing_flavor_by_id(self):
flavor = objects.Flavor(context=self.context, name='foo', id=1234)
flavor = objects.Flavor(context=self.context, flavorid='foo', id=1234)
self.assertRaises(exception.FlavorNotFound,
flavor.destroy)
@@ -208,9 +208,7 @@ class FlavorObjectTestCase(test.NoDBTestCase):
self._test_get_all(expect_len + 1)
def test_get_all_with_all_api_flavors(self):
db_flavors = db_api.flavor_get_all(self.context)
for flavor in db_flavors:
db_api.flavor_destroy(self.context, flavor['name'])
self._delete_main_flavors()
flavor = ForcedFlavor(context=self.context, **fake_api_flavor)
flavor.create()
self._test_get_all(1)

25
nova/tests/unit/api/openstack/compute/test_flavor_manage.py
View File

@@ -15,6 +15,7 @@
import datetime
import mock
from oslo_serialization import jsonutils
import six
import webob
@@ -52,20 +53,6 @@ def fake_db_flavor(**updates):
return db_flavor
def fake_get_flavor_by_flavor_id(flavorid, ctxt=None, read_deleted='yes'):
if flavorid == 'failtest':
raise exception.FlavorNotFound(flavor_id=flavorid)
elif not str(flavorid) == '1234':
raise Exception("This test expects flavorid 1234, not %s" % flavorid)
if read_deleted != 'no':
raise test.TestingException("Should not be reading deleted")
return fake_db_flavor(flavorid=flavorid)
def fake_destroy(flavorname):
pass
def fake_create(newflavor):
newflavor['flavorid'] = 1234
newflavor["name"] = 'test'
@@ -86,10 +73,6 @@ class FlavorManageTestV21(test.NoDBTestCase):
def setUp(self):
super(FlavorManageTestV21, self).setUp()
self.stubs.Set(flavors,
"get_flavor_by_flavor_id",
fake_get_flavor_by_flavor_id)
self.stubs.Set(flavors, "destroy", fake_destroy)
self.stub_out("nova.objects.Flavor.create", fake_create)
self.request_body = {
@@ -117,7 +100,8 @@ class FlavorManageTestV21(test.NoDBTestCase):
'os-flavor-access', 'flavors',
'os-flavor-extra-data'))
def test_delete(self):
@mock.patch('nova.objects.Flavor.destroy')
def test_delete(self, mock_destroy):
res = self.controller._delete(self._get_http_request(), 1234)
# NOTE: on v2.1, http status code is set as wsgi_code of API
@@ -130,9 +114,10 @@ class FlavorManageTestV21(test.NoDBTestCase):
self.assertEqual(202, status_int)
# subsequent delete should fail
mock_destroy.side_effect = exception.FlavorNotFound(flavor_id=1234)
self.assertRaises(webob.exc.HTTPNotFound,
self.controller._delete, self._get_http_request(),
"failtest")
1234)
def _test_create_missing_parameter(self, parameter):
body = {

12
nova/tests/unit/db/test_db_api.py
View File

@@ -4018,7 +4018,7 @@ class InstanceTypeTestCase(BaseInstanceTypeTestCase):
flavor2 = self._create_flavor({'name': 'name2', 'flavorid': 'a2',
'extra_specs': specs2})
db.flavor_destroy(self.ctxt, 'name1')
db.flavor_destroy(self.ctxt, 'a1')
self.assertRaises(exception.FlavorNotFound,
db.flavor_get, self.ctxt, flavor1['id'])
@@ -4067,7 +4067,7 @@ class InstanceTypeTestCase(BaseInstanceTypeTestCase):
def test_flavor_get_all(self):
# NOTE(boris-42): Remove base instance types
for it in db.flavor_get_all(self.ctxt):
db.flavor_destroy(self.ctxt, it['name'])
db.flavor_destroy(self.ctxt, it['flavorid'])
flavors = [
{'root_gb': 600, 'memory_mb': 100, 'disabled': True,
@@ -4294,7 +4294,7 @@ class InstanceTypeTestCase(BaseInstanceTypeTestCase):
def test_flavor_get_by_flavor_id_deleted(self):
flavor = self._create_flavor({'name': 'abc', 'flavorid': '123'})
db.flavor_destroy(self.ctxt, 'abc')
db.flavor_destroy(self.ctxt, '123')
flavor_by_fid = db.flavor_get_by_flavor_id(self.ctxt,
flavor['flavorid'], read_deleted='yes')
@@ -4306,7 +4306,7 @@ class InstanceTypeTestCase(BaseInstanceTypeTestCase):
param_dict = {'name': 'abc', 'flavorid': '123'}
self._create_flavor(param_dict)
db.flavor_destroy(self.ctxt, 'abc')
db.flavor_destroy(self.ctxt, '123')
# Recreate the flavor with the same params
flavor = self._create_flavor(param_dict)
@@ -4472,13 +4472,13 @@ class InstanceTypeAccessTestCase(BaseInstanceTypeTestCase):
for v in values:
self._create_flavor_access(*v)
db.flavor_destroy(self.ctxt, flavor1['name'])
db.flavor_destroy(self.ctxt, flavor1['flavorid'])
p = (self.ctxt, flavor1['flavorid'])
self.assertEqual(0, len(db.flavor_access_get_by_flavor_id(*p)))
p = (self.ctxt, flavor2['flavorid'])
self.assertEqual(1, len(db.flavor_access_get_by_flavor_id(*p)))
db.flavor_destroy(self.ctxt, flavor2['name'])
db.flavor_destroy(self.ctxt, flavor2['flavorid'])
self.assertEqual(0, len(db.flavor_access_get_by_flavor_id(*p)))

15
nova/tests/unit/objects/test_flavor.py
View File

@@ -353,11 +353,13 @@ class _TestFlavor(object):
flavor = flavor_obj.Flavor(id=123)
self.assertRaises(exception.ObjectActionError, flavor.save)
def test_destroy(self):
flavor = flavor_obj.Flavor(context=self.context, name='foo')
@mock.patch('nova.objects.Flavor._flavor_destroy')
def test_destroy(self, mock_destroy):
mock_destroy.side_effect = exception.FlavorNotFound(flavor_id='foo')
flavor = flavor_obj.Flavor(context=self.context, flavorid='foo')
with mock.patch.object(db, 'flavor_destroy') as destroy:
flavor.destroy()
destroy.assert_called_once_with(self.context, flavor.name)
destroy.assert_called_once_with(self.context, flavor.flavorid)
@mock.patch('nova.objects.Flavor._flavor_destroy')
def test_destroy_api_by_id(self, mock_destroy):
@@ -366,10 +368,11 @@ class _TestFlavor(object):
mock_destroy.assert_called_once_with(self.context, flavor_id=flavor.id)
@mock.patch('nova.objects.Flavor._flavor_destroy')
def test_destroy_api_by_name(self, mock_destroy):
flavor = flavor_obj.Flavor(context=self.context, name='foo')
def test_destroy_api_by_flavorid(self, mock_destroy):
flavor = flavor_obj.Flavor(context=self.context, flavorid='foo')
flavor.destroy()
mock_destroy.assert_called_once_with(self.context, name=flavor.name)
mock_destroy.assert_called_once_with(self.context,
flavorid=flavor.flavorid)
def test_load_projects(self):
flavor = flavor_obj.Flavor(context=self.context, flavorid='foo')

13
nova/tests/unit/test_flavors.py
View File

@@ -61,17 +61,6 @@ DEFAULT_FLAVOR_OBJS = [
class InstanceTypeTestCase(test.TestCase):
"""Test cases for flavor code."""
def test_non_existent_inst_type_should_not_delete(self):
# Ensures that flavor creation fails with invalid args.
self.assertRaises(exception.FlavorNotFoundByName,
flavors.destroy,
'unknown_flavor')
def test_will_not_destroy_with_no_name(self):
# Ensure destroy said path of no name raises error.
self.assertRaises(exception.FlavorNotFoundByName,
flavors.destroy, None)
def test_will_not_get_bad_default_instance_type(self):
# ensures error raised on bad default flavor.
self.flags(default_flavor='unknown_flavor')
@@ -432,7 +421,7 @@ class CreateInstanceTypeTest(test.TestCase):
self.assertNotEqual(len(original_list), len(new_list),
'instance type was not created')
flavors.destroy('flavor')
flavor.destroy()
self.assertRaises(exception.FlavorNotFound,
objects.Flavor.get_by_name, ctxt, flavor.name)