Make Flavor create() and destroy() work against API DB

This makes Flavor.create() always create new flavors in the API DB,
and destroy delete from either (favoring the API DB first, like the
rest of the methods).

Now that we're this far, it also adds functional tests for the basic
operations of create, save, query, and destroy for flavors. It does
these both for flavors in the main database and the API database,
to prove that they work for both (except for create, which is always
against the API DB now).

Note that this also removes a couple of tests for reading deleted
flavors, which are now no longer legit cases across the board.

Related to blueprint flavor-cell-api

Change-Id: Id5fc2f95179beb9fd057a37744815a8e640e0c32
This commit is contained in:
Dan Smith
2016-03-18 11:00:08 -07:00
parent c368a3e802
commit e11b6c0682
6 changed files with 373 additions and 71 deletions

View File

@@ -125,6 +125,65 @@ def _flavor_extra_specs_del(context, flavor_id, key):
extra_specs_key=key, flavor_id=flavor_id)
@db_api.api_context_manager.writer
def _flavor_create(context, values):
specs = values.get('extra_specs')
db_specs = []
if specs:
for k, v in specs.items():
db_spec = api_models.FlavorExtraSpecs()
db_spec['key'] = k
db_spec['value'] = v
db_specs.append(db_spec)
projects = values.get('projects')
db_projects = []
if projects:
for project in set(projects):
db_project = api_models.FlavorProjects()
db_project['project_id'] = project
db_projects.append(db_project)
values['extra_specs'] = db_specs
values['projects'] = db_projects
db_flavor = api_models.Flavors()
db_flavor.update(values)
try:
db_flavor.save(context.session)
except db_exc.DBDuplicateEntry as e:
if 'flavorid' in e.columns:
raise exception.FlavorIdExists(flavor_id=values['flavorid'])
raise exception.FlavorExists(name=values['name'])
except Exception as e:
raise db_exc.DBError(e)
return _dict_with_extra_specs(db_flavor)
@db_api.api_context_manager.writer
def _flavor_destroy(context, flavor_id=None, name=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)
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)
context.session.query(api_models.FlavorProjects).\
filter_by(flavor_id=result.id).delete()
context.session.query(api_models.FlavorExtraSpecs).\
filter_by(flavor_id=result.id).delete()
context.session.delete(result)
# TODO(berrange): Remove NovaObjectDictCompat
@base.NovaObjectRegistry.register
class Flavor(base.NovaPersistentObject, base.NovaObject,
@@ -203,6 +262,10 @@ class Flavor(base.NovaPersistentObject, base.NovaObject,
flavor.extra_specs = db_flavor['extra_specs']
if 'projects' in expected_attrs:
if 'projects' in db_flavor:
flavor['projects'] = [x['project_id']
for x in db_flavor['projects']]
else:
flavor._load_projects()
flavor.obj_reset_changes()
@@ -381,6 +444,10 @@ class Flavor(base.NovaPersistentObject, base.NovaObject,
self._remove_access(project_id)
self._load_projects()
@staticmethod
def _flavor_create(context, updates):
return _flavor_create(context, updates)
@base.remotable
def create(self):
if self.obj_attr_is_set('id'):
@@ -391,8 +458,7 @@ class Flavor(base.NovaPersistentObject, base.NovaObject,
for attr in OPTIONAL_FIELDS:
if attr in updates:
expected_attrs.append(attr)
projects = updates.pop('projects', [])
db_flavor = db.flavor_create(self._context, updates, projects=projects)
db_flavor = self._flavor_create(self._context, updates)
self._from_db_object(self._context, self, db_flavor,
expected_attrs=expected_attrs)
@@ -480,8 +546,24 @@ class Flavor(base.NovaPersistentObject, base.NovaObject,
if added_projects or deleted_projects:
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)
@base.remotable
def destroy(self):
# NOTE(danms): Historically the only way to delete a flavor
# is via name, which is not very precise. We need to be able to
# support the light construction of a flavor object and subsequent
# delete request with only our name filled out. However, if we have
# our id property, we should instead delete with that since it's
# far more specific.
try:
if 'id' in self:
self._flavor_destroy(self._context, flavor_id=self.id)
else:
self._flavor_destroy(self._context, name=self.name)
except exception.FlavorNotFound:
db.flavor_destroy(self._context, self.name)

View File

@@ -0,0 +1,231 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from nova import context
from nova import db
from nova.db.sqlalchemy import api as db_api
from nova.db.sqlalchemy import api_models
from nova import exception
from nova import objects
from nova import test
from nova.tests import fixtures
fake_api_flavor = {
'created_at': None,
'updated_at': None,
'name': 'm1.foo',
'memory_mb': 1024,
'vcpus': 4,
'root_gb': 20,
'ephemeral_gb': 0,
'flavorid': 'm1.foo',
'swap': 0,
'rxtx_factor': 1.0,
'vcpu_weight': 1,
'disabled': False,
'is_public': True,
'extra_specs': {'foo': 'bar'},
'projects': ['project1', 'project2'],
}
class FlavorObjectTestCase(test.NoDBTestCase):
USES_DB_SELF = True
def setUp(self):
super(FlavorObjectTestCase, self).setUp()
self.useFixture(fixtures.Database())
self.useFixture(fixtures.Database(database='api'))
self.context = context.RequestContext('fake-user', 'fake-project')
def test_create(self):
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
self.assertIn('id', flavor)
# Make sure we find this in the API database
flavor2 = objects.Flavor._flavor_get_from_db(self.context, flavor.id)
self.assertEqual(flavor.id, flavor2['id'])
# Make sure we don't find it in the main database
self.assertRaises(exception.FlavorNotFoundByName,
db.flavor_get_by_name, self.context, flavor.name)
self.assertRaises(exception.FlavorNotFound,
db.flavor_get_by_flavor_id, self.context,
flavor.flavorid)
# Default flavors will overlap with our id, so just query and make sure
# they are different flavors
main_flavor = db.flavor_get(self.context, flavor.id)
self.assertNotEqual(flavor.name, main_flavor['name'])
def test_get_with_no_projects(self):
fields = dict(fake_api_flavor, projects=[])
flavor = objects.Flavor(context=self.context, **fields)
flavor.create()
flavor = objects.Flavor.get_by_flavor_id(self.context, flavor.flavorid)
self.assertEqual([], flavor.projects)
def test_get_with_projects_and_specs(self):
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
flavor = objects.Flavor.get_by_id(self.context, flavor.id)
self.assertEqual(fake_api_flavor['projects'], flavor.projects)
self.assertEqual(fake_api_flavor['extra_specs'], flavor.extra_specs)
def _test_query(self, flavor):
flavor2 = objects.Flavor.get_by_id(self.context, flavor.id)
self.assertEqual(flavor.id, flavor2.id)
flavor2 = objects.Flavor.get_by_flavor_id(self.context,
flavor.flavorid)
self.assertEqual(flavor.id, flavor2.id)
flavor2 = objects.Flavor.get_by_name(self.context, flavor.name)
self.assertEqual(flavor.id, flavor2.id)
def test_query_api(self):
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
self._test_query(flavor)
def test_query_main(self):
flavor = objects.Flavor.get_by_flavor_id(self.context, '3')
self._test_query(flavor)
def _test_save(self, flavor):
flavor.extra_specs['marty'] = 'mcfly'
flavor.extra_specs['foo'] = 'bart'
projects = list(flavor.projects)
flavor.projects.append('project3')
flavor.save()
flavor2 = objects.Flavor.get_by_flavor_id(self.context,
flavor.flavorid)
self.assertEqual({'marty': 'mcfly', 'foo': 'bart'},
flavor2.extra_specs)
self.assertEqual(set(projects + ['project3']), set(flavor.projects))
del flavor.extra_specs['foo']
del flavor.projects[-1]
flavor.save()
flavor2 = objects.Flavor.get_by_flavor_id(self.context,
flavor.flavorid)
self.assertEqual({'marty': 'mcfly'}, flavor2.extra_specs)
self.assertEqual(set(projects), set(flavor2.projects))
def test_save_api(self):
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
self._test_save(flavor)
def test_save_main(self):
flavor = objects.Flavor.get_by_flavor_id(self.context, '3')
self._test_save(flavor)
@staticmethod
@db_api.api_context_manager.reader
def _collect_flavor_residue_api(context, flavor):
flavors = context.session.query(api_models.Flavors).\
filter_by(id=flavor.id).all()
specs = context.session.query(api_models.FlavorExtraSpecs).\
filter_by(flavor_id=flavor.id).all()
projects = context.session.query(api_models.FlavorProjects).\
filter_by(flavor_id=flavor.id).all()
return len(flavors) + len(specs) + len(projects)
def _test_destroy(self, flavor):
flavor.destroy()
self.assertRaises(exception.FlavorNotFound,
objects.Flavor.get_by_name, self.context,
flavor.name)
def test_destroy_api(self):
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
self._test_destroy(flavor)
self.assertEqual(
0, self._collect_flavor_residue_api(self.context, flavor))
def test_destroy_main(self):
flavor = objects.Flavor.get_by_flavor_id(self.context, '3')
self._test_destroy(flavor)
def test_destroy_missing_flavor_by_name(self):
flavor = objects.Flavor(context=self.context, name='foo')
self.assertRaises(exception.FlavorNotFoundByName,
flavor.destroy)
def test_destroy_missing_flavor_by_id(self):
flavor = objects.Flavor(context=self.context, name='foo', id=1234)
self.assertRaises(exception.FlavorNotFound,
flavor.destroy)
def _test_get_all(self, expect_len, marker=None, limit=None):
flavors = objects.FlavorList.get_all(self.context, marker=marker,
limit=limit)
self.assertEqual(expect_len, len(flavors))
return flavors
def test_get_all(self):
expect_len = len(db_api.flavor_get_all(self.context))
self._test_get_all(expect_len)
def test_get_all_with_some_api_flavors(self):
expect_len = len(db_api.flavor_get_all(self.context))
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
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'])
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
self._test_get_all(1)
def test_get_all_with_marker_in_api(self):
db_flavors = db_api.flavor_get_all(self.context)
db_flavor_ids = [x['flavorid'] for x in db_flavors]
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
fake_flavor2 = dict(fake_api_flavor, name='m1.zoo', flavorid='m1.zoo')
flavor = objects.Flavor(context=self.context, **fake_flavor2)
flavor.create()
result = self._test_get_all(3, marker='m1.foo', limit=3)
result_flavorids = [x.flavorid for x in result]
self.assertEqual(['m1.zoo'] + db_flavor_ids[:2], result_flavorids)
def test_get_all_with_marker_in_main(self):
db_flavors = db_api.flavor_get_all(self.context)
db_flavor_ids = [x['flavorid'] for x in db_flavors]
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
fake_flavor2 = dict(fake_api_flavor, name='m1.zoo', flavorid='m1.zoo')
flavor = objects.Flavor(context=self.context, **fake_flavor2)
flavor.create()
result = self._test_get_all(2, marker='3', limit=3)
result_flavorids = [x.flavorid for x in result]
self.assertEqual(db_flavor_ids[3:], result_flavorids)
def test_get_all_with_marker_in_neither(self):
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
fake_flavor2 = dict(fake_api_flavor, name='m1.zoo', flavorid='m1.zoo')
flavor = objects.Flavor(context=self.context, **fake_flavor2)
flavor.create()
self.assertRaises(exception.MarkerNotFound,
self._test_get_all, 2, marker='noflavoratall')

View File

@@ -17,8 +17,8 @@
import six
from nova import context
from nova import db
from nova import exception as ex
from nova import objects
from nova import test
from nova.tests import fixtures as nova_fixtures
from nova.tests.functional import integrated_helpers as helper
@@ -147,7 +147,7 @@ class FlavorManageFullstack(test.TestCase):
self.assertEqual(400, resp.status, resp)
# ... and ensure that we didn't leak it into the db
self.assertRaises(ex.FlavorNotFound,
db.flavor_get_by_flavor_id,
objects.Flavor.get_by_flavor_id,
ctx, flav['flavor']['id'])
# bounds conditions - invalid ram
@@ -157,7 +157,7 @@ class FlavorManageFullstack(test.TestCase):
self.assertEqual(400, resp.status)
# ... and ensure that we didn't leak it into the db
self.assertRaises(ex.FlavorNotFound,
db.flavor_get_by_flavor_id,
objects.Flavor.get_by_flavor_id,
ctx, flav['flavor']['id'])
# NOTE(sdague): if there are other bounds conditions that
@@ -175,11 +175,6 @@ class FlavorManageFullstack(test.TestCase):
self.api.api_post('flavors', new_flav)
self.api.api_delete('flavors/%s' % new_flav['flavor']['id'])
# It is valid to directly fetch details of a deleted flavor
resp = self.api.api_get('flavors/%s' % new_flav['flavor']['id'])
self.assertEqual(200, resp.status)
self.assertFlavorAPIEqual(new_flav['flavor'], resp.body['flavor'])
# deleted flavor should not show up in a list
resp = self.api.api_get('flavors')
self.assertFlavorNotInList(new_flav['flavor'], resp.body)
@@ -202,7 +197,7 @@ class FlavorManageFullstack(test.TestCase):
# Create flavor and ensure it made it to the database
self.api.api_post('flavors', flav1)
flav1db = db.flavor_get_by_flavor_id(ctx, flav1['flavor']['id'])
flav1db = objects.Flavor.get_by_flavor_id(ctx, flav1['flavor']['id'])
self.assertFlavorDbEqual(flav1['flavor'], flav1db)
# Ensure new flavor is seen in the listing
@@ -212,7 +207,7 @@ class FlavorManageFullstack(test.TestCase):
# Delete flavor and ensure it was removed from the database
self.api.api_delete('flavors/%s' % flav1['flavor']['id'])
self.assertRaises(ex.FlavorNotFound,
db.flavor_get_by_flavor_id,
objects.Flavor.get_by_flavor_id,
ctx, flav1['flavor']['id'])
resp = self.api.api_delete('flavors/%s' % flav1['flavor']['id'],
@@ -232,7 +227,7 @@ class FlavorManageFullstack(test.TestCase):
self.assertEqual(403, resp.status)
# ... and that it didn't leak through
self.assertRaises(ex.FlavorNotFound,
db.flavor_get_by_flavor_id,
objects.Flavor.get_by_flavor_id,
ctx, flav1['flavor']['id'])
# Create the flavor as the admin user
@@ -244,4 +239,4 @@ class FlavorManageFullstack(test.TestCase):
self.assertEqual(403, resp.status)
# ... and ensure that we didn't actually delete the flavor,
# this will throw an exception if we did.
db.flavor_get_by_flavor_id(ctx, flav1['flavor']['id'])
objects.Flavor.get_by_flavor_id(ctx, flav1['flavor']['id'])

View File

@@ -71,7 +71,7 @@ def fake_destroy(flavorname):
pass
def fake_create(context, kwargs, projects=None):
def fake_create(context, kwargs):
newflavor = fake_db_flavor()
flavorid = kwargs.get('flavorid')
@@ -103,7 +103,7 @@ class FlavorManageTestV21(test.NoDBTestCase):
"get_flavor_by_flavor_id",
fake_get_flavor_by_flavor_id)
self.stubs.Set(flavors, "destroy", fake_destroy)
self.stub_out("nova.db.flavor_create", fake_create)
self.stub_out("nova.objects.flavor._flavor_create", fake_create)
self.request_body = {
"flavor": {

View File

@@ -15,6 +15,7 @@
import datetime
import mock
from oslo_db import exception as db_exc
from nova import db
from nova.db.sqlalchemy import api as db_api
@@ -194,13 +195,13 @@ class _TestFlavor(object):
mock_api_del.assert_called_once_with(elevated, 12345, '456')
self.assertFalse(mock_main_del.called)
def test_create(self):
@mock.patch('nova.objects.Flavor._flavor_create')
def test_create(self, mock_create):
mock_create.return_value = fake_api_flavor
flavor = flavor_obj.Flavor(context=self.context)
flavor.name = 'm1.foo'
flavor.extra_specs = fake_flavor['extra_specs']
with mock.patch.object(db, 'flavor_create') as create:
create.return_value = fake_flavor
flavor.create()
self.assertEqual(self.context, flavor._context)
@@ -208,28 +209,24 @@ class _TestFlavor(object):
flavor._context = None
self._compare(self, fake_flavor, flavor)
def test_create_with_projects(self):
@mock.patch('nova.objects.Flavor._flavor_create')
def test_create_with_projects(self, mock_create):
context = self.context.elevated()
flavor = flavor_obj.Flavor(context=context)
flavor.name = 'm1.foo'
flavor.extra_specs = fake_flavor['extra_specs']
flavor.projects = ['project-1', 'project-2']
db_flavor = dict(fake_flavor, projects=list(flavor.projects))
with mock.patch.multiple(db, flavor_create=mock.DEFAULT,
flavor_access_get_by_flavor_id=mock.DEFAULT
) as methods:
methods['flavor_create'].return_value = db_flavor
methods['flavor_access_get_by_flavor_id'].return_value = [
{'project_id': 'project-1'},
{'project_id': 'project-2'}]
db_flavor = dict(fake_flavor,
projects=[{'project_id': pid}
for pid in flavor.projects])
mock_create.return_value = db_flavor
flavor.create()
methods['flavor_create'].assert_called_once_with(
context,
{'name': 'm1.foo',
'extra_specs': fake_flavor['extra_specs']},
projects=['project-1', 'project-2'])
mock_create.assert_called_once_with(
context, {'name': 'm1.foo',
'extra_specs': fake_flavor['extra_specs'],
'projects': ['project-1', 'project-2']})
self.assertEqual(context, flavor._context)
# NOTE(danms): Orphan this to avoid lazy-loads
@@ -241,6 +238,14 @@ class _TestFlavor(object):
flavor = flavor_obj.Flavor(context=self.context, id=123)
self.assertRaises(exception.ObjectActionError, flavor.create)
@mock.patch('nova.db.sqlalchemy.api_models.Flavors')
def test_create_duplicate(self, mock_flavors):
mock_flavors.return_value.save.side_effect = db_exc.DBDuplicateEntry
fields = dict(fake_api_flavor)
del fields['id']
flavor = objects.Flavor(self.context, **fields)
self.assertRaises(exception.FlavorExists, flavor.create)
@mock.patch('nova.db.flavor_access_add')
@mock.patch('nova.db.flavor_access_remove')
@mock.patch('nova.db.flavor_extra_specs_delete')
@@ -328,7 +333,7 @@ class _TestFlavor(object):
self.assertEqual(['project-1', 'project-3'], flavor.projects)
mock_add.assert_called_once_with(self.context, 123, 'project-3')
@mock.patch('nova.db.flavor_create')
@mock.patch('nova.objects.Flavor._flavor_create')
@mock.patch('nova.db.flavor_extra_specs_delete')
@mock.patch('nova.db.flavor_extra_specs_update_or_create')
def test_save_deleted_extra_specs(self, mock_update, mock_delete,
@@ -350,11 +355,23 @@ class _TestFlavor(object):
self.assertRaises(exception.ObjectActionError, flavor.save)
def test_destroy(self):
flavor = flavor_obj.Flavor(context=self.context, id=123, name='foo')
flavor = flavor_obj.Flavor(context=self.context, name='foo')
with mock.patch.object(db, 'flavor_destroy') as destroy:
flavor.destroy()
destroy.assert_called_once_with(self.context, flavor.name)
@mock.patch('nova.objects.Flavor._flavor_destroy')
def test_destroy_api_by_id(self, mock_destroy):
flavor = flavor_obj.Flavor(context=self.context, id=123)
flavor.destroy()
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')
flavor.destroy()
mock_destroy.assert_called_once_with(self.context, name=flavor.name)
def test_load_projects(self):
flavor = flavor_obj.Flavor(context=self.context, flavorid='foo')
with mock.patch.object(db, 'flavor_access_get_by_flavor_id') as get:
@@ -371,6 +388,13 @@ class _TestFlavor(object):
self.assertEqual(['a', 'b'], flavor.projects)
mock_get_projects.assert_called_once_with(self.context, 'm1.foo')
def test_from_db_loads_projects(self):
fake = dict(fake_api_flavor, projects=[{'project_id': 'foo'}])
obj = objects.Flavor._from_db_object(self.context, objects.Flavor(),
fake, expected_attrs=['projects'])
self.assertIn('projects', obj)
self.assertEqual(['foo'], obj.projects)
def test_load_anything_else(self):
flavor = flavor_obj.Flavor()
self.assertRaises(exception.ObjectActionError,

View File

@@ -104,36 +104,6 @@ class InstanceTypeTestCase(test.TestCase):
self.assertIsInstance(fetched, objects.Flavor)
self.assertEqual(default_instance_type.flavorid, fetched.flavorid)
def test_can_read_deleted_types_using_flavor_id(self):
# Ensure deleted flavors can be read when querying flavor_id.
inst_type_name = "test"
inst_type_flavor_id = "test1"
inst_type = flavors.create(inst_type_name, 256, 1, 120, 100,
inst_type_flavor_id)
self.assertEqual(inst_type_name, inst_type.name)
# NOTE(jk0): The deleted flavor will show up here because the context
# in get_flavor_by_flavor_id() is set to use read_deleted by
# default.
flavors.destroy(inst_type.name)
deleted_inst_type = flavors.get_flavor_by_flavor_id(
inst_type_flavor_id)
self.assertEqual(inst_type_name, deleted_inst_type.name)
def test_read_deleted_false_converting_flavorid(self):
"""Ensure deleted flavors are not returned when not needed (for
example when creating a server and attempting to translate from
flavorid to instance_type_id.
"""
flavors.create("instance_type1", 256, 1, 120, 100, "test1")
flavors.destroy("instance_type1")
flavors.create("instance_type1_redo", 256, 1, 120, 100, "test1")
instance_type = flavors.get_flavor_by_flavor_id(
"test1", read_deleted="no")
self.assertEqual("instance_type1_redo", instance_type.name)
def test_get_all_flavors_sorted_list_sort(self):
# Test default sort
all_flavors = flavors.get_all_flavors_sorted_list()
@@ -464,7 +434,7 @@ class CreateInstanceTypeTest(test.TestCase):
flavors.destroy('flavor')
self.assertRaises(exception.FlavorNotFound,
objects.Flavor.get_by_id, ctxt, flavor.id)
objects.Flavor.get_by_name, ctxt, flavor.name)
# Deleted instance should not be in list anymore
new_list = objects.FlavorList.get_all(ctxt)