openstack/nova
Code Issues Proposed changes

Block flavor creation until main database is empty

Browse Source 
This makes Flavor.create() fail until the main database has had all
of its flavors migrated. Since we want to avoid any overlap or clashes
in integer ids we need to enforce this. The flavor API was poorly
designed in that it exposes this internal id. Luckily, flavor creation
is infrequent and flavor migration is fast.

The flavors in our database are hard-coded in the first migration,
which means we have to basically do the migration during every
unit test in order to avoid having a legacy environment. Since that
leaves us with deleted flavors in the DB (which screws up the DB
archiving tests), this adds a hard_delete flag to the migration to
do an actual purge of those from our database. What a mess.

Related to blueprint flavor-cell-api

Depends-On: I8ab03af9d2f4974f26a7f8487ec978caea957e45
Change-Id: Iea063448f43da1043d16dd521d255dd29a0e1bc5
This commit is contained in:
Dan Smith
2016-03-21 07:21:28 -07:00
parent 74767a7a03
commit 17a8e8a68c
11 changed files with 168 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -19,6 +19,7 @@ from nova.api.openstack import wsgi
from nova.api import validation
from nova.compute import flavors
from nova import exception
from nova.i18n import _
ALIAS = "os-flavor-manage"
@@ -84,6 +85,9 @@ class FlavorManageController(wsgi.Controller):
except (exception.FlavorExists,
exception.FlavorIdExists) as err:
raise webob.exc.HTTPConflict(explanation=err.format_message())
except exception.ObjectActionError:
raise webob.exc.HTTPConflict(explanation=_(
'Not all flavors have been migrated to the API database'))
except exception.FlavorCreateFailed as err:
raise webob.exc.HTTPInternalServerError(explanation=
err.format_message())

39
nova/objects/flavor.py
View File

@@ -26,6 +26,7 @@ from nova import db
from nova.db.sqlalchemy import api as db_api
from nova.db.sqlalchemy.api import require_context
from nova.db.sqlalchemy import api_models
from nova.db.sqlalchemy import models as main_models
from nova import exception
from nova.i18n import _LW
from nova import objects
@@ -189,6 +190,16 @@ def _flavor_destroy(context, flavor_id=None, name=None):
context.session.delete(result)
@db_api.main_context_manager.reader
def _ensure_migrated(context):
result = context.session.query(main_models.InstanceTypes).\
filter_by(deleted=0).count()
if result:
LOG.warning(_LW('Main database contains %(count)i unmigrated flavors'),
{'count': result})
return result == 0
# TODO(berrange): Remove NovaObjectDictCompat
@base.NovaObjectRegistry.register
class Flavor(base.NovaPersistentObject, base.NovaObject,
@@ -453,11 +464,24 @@ class Flavor(base.NovaPersistentObject, base.NovaObject,
def _flavor_create(context, updates):
return _flavor_create(context, updates)
@staticmethod
def _ensure_migrated(context):
return _ensure_migrated(context)
@base.remotable
def create(self):
if self.obj_attr_is_set('id'):
raise exception.ObjectActionError(action='create',
reason='already created')
# NOTE(danms): Once we have made it past a point where we know
# all flavors have been migrated, we can remove this. Ideally
# in Ocata with a blocker migration to be sure.
if not self._ensure_migrated(self._context):
raise exception.ObjectActionError(
action='create',
reason='main database still contains flavors')
updates = self.obj_get_changes()
expected_attrs = []
for attr in OPTIONAL_FIELDS:
@@ -671,7 +695,15 @@ def _get_main_db_flavor_ids(context, limit):
limit(limit)]
def migrate_flavors(ctxt, count):
@db_api.main_context_manager.writer
def _destroy_flavor_hard(context, name):
# NOTE(danms): We don't need this imported at runtime, so
# keep it separate here
from nova.db.sqlalchemy import models
context.session.query(models.InstanceTypes).filter_by(name=name).delete()
def migrate_flavors(ctxt, count, hard_delete=False):
main_db_ids = _get_main_db_flavor_ids(ctxt, count)
if not main_db_ids:
return 0, 0
@@ -686,7 +718,10 @@ def migrate_flavors(ctxt, count):
for field in flavor.fields}
flavor._flavor_create(ctxt, flavor_values)
count_hit += 1
db.flavor_destroy(ctxt, flavor.name)
if hard_delete:
_destroy_flavor_hard(ctxt, flavor.name)
else:
db.flavor_destroy(ctxt, flavor.name)
except exception.FlavorNotFound:
LOG.warning(_LW('Flavor id %(id)i disappeared during migration'),
{'id': flavor_id})

6
nova/test.py
View File

@@ -49,6 +49,7 @@ from nova import db
from nova.network import manager as network_manager
from nova.network.security_group import openstack_driver
from nova.objects import base as objects_base
from nova.objects import flavor as flavor_obj
from nova.tests import fixtures as nova_fixtures
from nova.tests.unit import conf_fixture
from nova.tests.unit import policy_fixture
@@ -213,6 +214,11 @@ class TestCase(testtools.TestCase):
if self.USES_DB:
self.useFixture(nova_fixtures.Database())
self.useFixture(nova_fixtures.Database(database='api'))
# NOTE(danms): Flavors are encoded in our original migration
# which means we have no real option other than to migrate them
# onlineish every time we build a new database (for now).
ctxt = context.get_admin_context()
flavor_obj.migrate_flavors(ctxt, 100, hard_delete=True)
elif not self.USES_DB_SELF:
self.useFixture(nova_fixtures.DatabasePoisonFixture())

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

@@ -39,6 +39,16 @@ fake_api_flavor = {
}
class ForcedFlavor(objects.Flavor):
"""A Flavor object that lets us create with things still in the main DB.
This is required for us to be able to test mixed scenarios.
"""
@staticmethod
def _ensure_migrated(*args):
return True
class FlavorObjectTestCase(test.NoDBTestCase):
USES_DB_SELF = True
@@ -48,7 +58,13 @@ class FlavorObjectTestCase(test.NoDBTestCase):
self.useFixture(fixtures.Database(database='api'))
self.context = context.RequestContext('fake-user', 'fake-project')
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'])
def test_create(self):
self._delete_main_flavors()
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
self.assertIn('id', flavor)
@@ -64,12 +80,8 @@ class FlavorObjectTestCase(test.NoDBTestCase):
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):
self._delete_main_flavors()
fields = dict(fake_api_flavor, projects=[])
flavor = objects.Flavor(context=self.context, **fields)
flavor.create()
@@ -77,6 +89,7 @@ class FlavorObjectTestCase(test.NoDBTestCase):
self.assertEqual([], flavor.projects)
def test_get_with_projects_and_specs(self):
self._delete_main_flavors()
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
flavor = objects.Flavor.get_by_id(self.context, flavor.id)
@@ -95,6 +108,7 @@ class FlavorObjectTestCase(test.NoDBTestCase):
self.assertEqual(flavor.id, flavor2.id)
def test_query_api(self):
self._delete_main_flavors()
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
self._test_query(flavor)
@@ -126,6 +140,7 @@ class FlavorObjectTestCase(test.NoDBTestCase):
self.assertEqual(set(projects), set(flavor2.projects))
def test_save_api(self):
self._delete_main_flavors()
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
self._test_save(flavor)
@@ -154,6 +169,7 @@ class FlavorObjectTestCase(test.NoDBTestCase):
flavor.name)
def test_destroy_api(self):
self._delete_main_flavors()
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
flavor.create()
self._test_destroy(flavor)
@@ -186,7 +202,7 @@ class FlavorObjectTestCase(test.NoDBTestCase):
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 = ForcedFlavor(context=self.context, **fake_api_flavor)
flavor.create()
self._test_get_all(expect_len + 1)
@@ -194,17 +210,17 @@ class FlavorObjectTestCase(test.NoDBTestCase):
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 = ForcedFlavor(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 = ForcedFlavor(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 = ForcedFlavor(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]
@@ -213,24 +229,30 @@ class FlavorObjectTestCase(test.NoDBTestCase):
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 = ForcedFlavor(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 = ForcedFlavor(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 = ForcedFlavor(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 = ForcedFlavor(context=self.context, **fake_flavor2)
flavor.create()
self.assertRaises(exception.MarkerNotFound,
self._test_get_all, 2, marker='noflavoratall')
def test_create_checks_main_flavors(self):
flavor = objects.Flavor(context=self.context, **fake_api_flavor)
self.assertRaises(exception.ObjectActionError, flavor.create)
self._delete_main_flavors()
flavor.create()
class FlavorMigrationTestCase(test.NoDBTestCase):
def setUp(self):

14
nova/tests/functional/wsgi/test_flavor_manage.py
View File

@@ -17,6 +17,7 @@
import six
from nova import context
from nova import db
from nova import exception as ex
from nova import objects
from nova import test
@@ -179,6 +180,19 @@ class FlavorManageFullstack(test.TestCase):
resp = self.api.api_get('flavors')
self.assertFlavorNotInList(new_flav['flavor'], resp.body)
def test_flavor_create_frozen(self):
ctx = context.get_admin_context()
db.flavor_create(ctx, {
'name': 'foo', 'memory_mb': 512, 'vcpus': 1,
'root_gb': 1, 'ephemeral_gb': 0, 'flavorid': 'foo',
'swap': 0, 'rxtx_factor': 1.0, 'vcpu_weight': 1,
'disabled': False, 'is_public': True,
})
new_flav = {'flavor': rand_flavor()}
resp = self.api.api_post('flavors', new_flav,
check_response_status=False)
self.assertEqual(409, resp.status)
def test_flavor_manage_func(self):
"""Basic flavor creation lifecycle testing.

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

@@ -71,25 +71,17 @@ def fake_destroy(flavorname):
pass
def fake_create(context, kwargs):
newflavor = fake_db_flavor()
flavorid = kwargs.get('flavorid')
if flavorid is None:
flavorid = 1234
newflavor['flavorid'] = flavorid
newflavor["name"] = kwargs.get('name')
newflavor["memory_mb"] = int(kwargs.get('memory_mb'))
newflavor["vcpus"] = int(kwargs.get('vcpus'))
newflavor["root_gb"] = int(kwargs.get('root_gb'))
newflavor["ephemeral_gb"] = int(kwargs.get('ephemeral_gb'))
newflavor["swap"] = kwargs.get('swap')
newflavor["rxtx_factor"] = float(kwargs.get('rxtx_factor'))
newflavor["is_public"] = bool(kwargs.get('is_public'))
newflavor["disabled"] = bool(kwargs.get('disabled'))
return newflavor
def fake_create(newflavor):
newflavor['flavorid'] = 1234
newflavor["name"] = 'test'
newflavor["memory_mb"] = 512
newflavor["vcpus"] = 2
newflavor["root_gb"] = 1
newflavor["ephemeral_gb"] = 1
newflavor["swap"] = 512
newflavor["rxtx_factor"] = 1.0
newflavor["is_public"] = True
newflavor["disabled"] = False
class FlavorManageTestV21(test.NoDBTestCase):
@@ -103,7 +95,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.objects.flavor._flavor_create", fake_create)
self.stub_out("nova.objects.Flavor.create", fake_create)
self.request_body = {
"flavor": {

3
nova/tests/unit/api/openstack/compute/test_flavors_extra_specs.py
View File

@@ -27,7 +27,8 @@ from nova.tests.unit.api.openstack import fakes
from nova.tests.unit.objects import test_flavor
def return_create_flavor_extra_specs(context, flavor_id, extra_specs):
def return_create_flavor_extra_specs(context, flavor_id, extra_specs,
*args, **kwargs):
return stub_flavor_extra_specs()

2
nova/tests/unit/conductor/test_conductor.py
View File

@@ -405,6 +405,8 @@ class _BaseTaskTestCase(object):
@mock.patch('nova.objects.Instance.refresh')
def test_build_instances(self, mock_refresh):
instance_type = flavors.get_default_flavor()
# NOTE(danms): Avoid datetime timezone issues with converted flavors
instance_type.created_at = None
instances = [objects.Instance(context=self.context,
id=i,
uuid=uuid.uuid4(),

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

@@ -4135,10 +4135,30 @@ class InstanceTypeTestCase(BaseInstanceTypeTestCase):
assert_sorted_by_key_both_dir(attr)
def test_flavor_get_all_limit(self):
flavors = [
{'root_gb': 1, 'memory_mb': 100, 'disabled': True,
'is_public': False, 'name': 'flavor1', 'flavorid': 'flavor1'},
{'root_gb': 100, 'memory_mb': 200, 'disabled': True,
'is_public': False, 'name': 'flavor2', 'flavorid': 'flavor2'},
{'root_gb': 100, 'memory_mb': 300, 'disabled': True,
'is_public': False, 'name': 'flavor3', 'flavorid': 'flavor3'},
]
flavors = [self._create_flavor(it) for it in flavors]
limited_flavors = db.flavor_get_all(self.ctxt, limit=2)
self.assertEqual(2, len(limited_flavors))
def test_flavor_get_all_list_marker(self):
flavors = [
{'root_gb': 1, 'memory_mb': 100, 'disabled': True,
'is_public': False, 'name': 'flavor1', 'flavorid': 'flavor1'},
{'root_gb': 100, 'memory_mb': 200, 'disabled': True,
'is_public': False, 'name': 'flavor2', 'flavorid': 'flavor2'},
{'root_gb': 100, 'memory_mb': 300, 'disabled': True,
'is_public': False, 'name': 'flavor3', 'flavorid': 'flavor3'},
]
flavors = [self._create_flavor(it) for it in flavors]
all_flavors = db.flavor_get_all(self.ctxt)
# Set the 3rd result as the marker

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

@@ -82,20 +82,20 @@ class _TestFlavor(object):
def test_get_by_id(self):
with mock.patch.object(db, 'flavor_get') as get:
get.return_value = fake_flavor
flavor = flavor_obj.Flavor.get_by_id(self.context, 1)
flavor = flavor_obj.Flavor.get_by_id(self.context, 100)
self._compare(self, fake_flavor, flavor)
def test_get_by_name(self):
with mock.patch.object(db, 'flavor_get_by_name') as get_by_name:
get_by_name.return_value = fake_flavor
flavor = flavor_obj.Flavor.get_by_name(self.context, 'm1.foo')
flavor = flavor_obj.Flavor.get_by_name(self.context, 'm1.legacy')
self._compare(self, fake_flavor, flavor)
def test_get_by_flavor_id(self):
with mock.patch.object(db, 'flavor_get_by_flavor_id') as get_by_id:
get_by_id.return_value = fake_flavor
flavor = flavor_obj.Flavor.get_by_flavor_id(self.context,
'm1.foo')
'm1.legacy')
self._compare(self, fake_flavor, flavor)
@mock.patch('nova.objects.Flavor._flavor_get_from_db')
@@ -334,9 +334,9 @@ class _TestFlavor(object):
mock_add.assert_called_once_with(self.context, 123, 'project-3')
@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,
@mock.patch('nova.objects.Flavor._flavor_extra_specs_del')
@mock.patch('nova.objects.Flavor._flavor_extra_specs_add')
def test_save_deleted_extra_specs(self, mock_add, mock_delete,
mock_create):
mock_create.return_value = dict(fake_flavor,
extra_specs={'key1': 'value1'})
@@ -346,9 +346,8 @@ class _TestFlavor(object):
flavor.create()
flavor.extra_specs = {}
flavor.save()
mock_delete.assert_called_once_with(self.context, flavor.flavorid,
'key1')
self.assertFalse(mock_update.called)
mock_delete.assert_called_once_with(self.context, flavor.id, 'key1')
self.assertFalse(mock_add.called)
def test_save_invalid_fields(self):
flavor = flavor_obj.Flavor(id=123)
@@ -471,18 +470,23 @@ class TestFlavorRemote(test_objects._RemoteTest, _TestFlavor):
class _TestFlavorList(object):
def test_get_all_from_db(self):
db_flavors = [
_TestFlavor._create_api_flavor(self.context),
_TestFlavor._create_api_flavor(self.context, 'm1.bar'),
]
db_flavors.extend(db_api.flavor_get_all(self.context))
@mock.patch('nova.db.flavor_get_all')
def test_get_all_from_db(self, mock_get):
# Get a list of the actual flavors in the API DB
api_flavors = flavor_obj._flavor_get_all_from_db(self.context,
False, None,
'flavorid', 'asc',
None, None)
# Return a fake flavor from the main DB query
db_flavors = [fake_flavor]
mock_get.return_value = db_flavors
flavors = objects.FlavorList.get_all(self.context)
self.assertEqual(len(db_flavors), len(flavors))
# Make sure we're getting all flavors from the api and main
# db queries
self.assertEqual(len(db_flavors) + len(api_flavors), len(flavors))
def test_get_all_from_db_with_limit(self):
_TestFlavor._create_api_flavor(self.context)
_TestFlavor._create_api_flavor(self.context, 'm1.bar')
flavors = objects.FlavorList.get_all(self.context,
limit=1)
self.assertEqual(1, len(flavors))

14
releasenotes/notes/flavors-moved-to-api-database-b33489ed3b1b246b.yaml Normal file
View File

@@ -0,0 +1,14 @@
---
upgrade:
- Flavors are being moved to the API database for CellsV2. In this
release, the online data migrations will move any flavors you have
in your main database to the API database, retaining all
attributes. Until this is complete, new attempts to create flavors
will return an HTTP 409 to avoid creating flavors in one place that
may conflict with flavors you already have and are yet to be
migrated.
- Note that flavors can no longer be soft-deleted as the API
database does not replicate the legacy soft-delete functionality
from the main database. As such, deleted flavors are not migrated
and the behavior users will experience will be the same as if a
purge of deleted records was performed.