openstack/cinder
Code Issues Proposed changes

Convert volume_type id from int to uuid.

Browse Source 
This converts the volume_type id from int to uuid.  In
addition, this also corrects an issue of deleting volume_types
by name.  Even though the client/api is taking the id, it was
converting to the name of the volume_type for the db access.

We also want to continue enforcing that the name is unique on creation
so as to avoid any confusion and for clarity in Horizon.  The api
has also been enhanced to allow you to specify the type by name
or by uuid.

This does NOT modify the things like the volume details display field
(currently shows the type 'name' not 'uuid'), these types of changes
will go in to V2 of the API.

Implements blueprint vol-type-to-uuid

Change-Id: I1c54ff2a1e0c5df5891408fc11b15176db4028c3
This commit is contained in:
john-griffith
2012-12-02 00:01:49 -07:00
committed by John Griffith
parent b1e4232996
commit 51a438c8f3
12 changed files with 242 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
cinder/api/v1/volumes.py
View File

@@ -294,8 +294,17 @@ class VolumeController(wsgi.Controller):
req_volume_type = volume.get('volume_type', None)
if req_volume_type:
if not uuidutils.is_uuid_like(req_volume_type):
try:
kwargs['volume_type'] = volume_types.get_volume_type_by_name(
kwargs['volume_type'] = \
volume_types.get_volume_type_by_name(
context, req_volume_type)
except exception.VolumeTypeNotFound:
explanation = 'Volume type not found.'
raise exc.HTTPNotFound(explanation=explanation)
else:
try:
kwargs['volume_type'] = volume_types.get_volume_type(
context, req_volume_type)
except exception.VolumeTypeNotFound:
explanation = 'Volume type not found.'

4
cinder/db/api.py
View File

@@ -353,9 +353,9 @@ def volume_type_get_by_name(context, name):
return IMPL.volume_type_get_by_name(context, name)
def volume_type_destroy(context, name):
def volume_type_destroy(context, id):
"""Delete a volume type."""
return IMPL.volume_type_destroy(context, name)
return IMPL.volume_type_destroy(context, id)
def volume_get_active_by_window(context, begin, end=None, project_id=None):

21
cinder/db/sqlalchemy/api.py
View File

@@ -1313,13 +1313,21 @@ def volume_type_create(context, values):
{'extra_specs' : {'k1': 'v1', 'k2': 'v2', ...}}
"""
if not values.get('id'):
values['id'] = str(uuid.uuid4())
session = get_session()
with session.begin():
try:
volume_type_get_by_name(context, values['name'], session)
raise exception.VolumeTypeExists(name=values['name'])
raise exception.VolumeTypeExists(id=values['name'])
except exception.VolumeTypeNotFoundByName:
pass
try:
volume_type_get(context, values['id'], session)
raise exception.VolumeTypeExists(id=values['id'])
except exception.VolumeTypeNotFound:
pass
try:
values['extra_specs'] = _metadata_refs(values.get('extra_specs'),
models.VolumeTypeExtraSpecs)
@@ -1383,19 +1391,18 @@ def volume_type_get_by_name(context, name, session=None):
@require_admin_context
def volume_type_destroy(context, name):
def volume_type_destroy(context, id):
volume_type_get(context, id)
session = get_session()
with session.begin():
volume_type_ref = volume_type_get_by_name(context, name,
session=session)
volume_type_id = volume_type_ref['id']
session.query(models.VolumeTypes).\
filter_by(id=volume_type_id).\
filter_by(id=id).\
update({'deleted': True,
'deleted_at': timeutils.utcnow(),
'updated_at': literal_column('updated_at')})
session.query(models.VolumeTypeExtraSpecs).\
filter_by(volume_type_id=volume_type_id).\
filter_by(volume_type_id=id).\
update({'deleted': True,
'deleted_at': timeutils.utcnow(),
'updated_at': literal_column('updated_at')})

155
cinder/db/sqlalchemy/migrate_repo/versions/004_volume_type_to_uuid.py Normal file
View File

@@ -0,0 +1,155 @@
# vim: tabstop=4 shiftwidth=4 softtabstop=4
# 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.
import uuid
from cinder.openstack.common import log as logging
from migrate import ForeignKeyConstraint
from sqlalchemy import Integer, MetaData, String, Table
LOG = logging.getLogger(__name__)
def upgrade(migrate_engine):
"""Convert volume_type_id to UUID."""
meta = MetaData()
meta.bind = migrate_engine
volumes = Table('volumes', meta, autoload=True)
volume_types = Table('volume_types', meta, autoload=True)
extra_specs = Table('volume_type_extra_specs', meta, autoload=True)
fkey_remove_list = [volumes.c.volume_type_id,
volume_types.c.id,
extra_specs.c.volume_type_id]
for column in fkey_remove_list:
fkeys = list(column.foreign_keys)
if fkeys:
fkey_name = fkeys[0].constraint.name
fkey = ForeignKeyConstraint(columns=[column],
refcolumns=[volume_types.c.id],
name=fkey_name)
try:
fkey.drop()
except Exception:
if migrate_engine.url.get_dialect().name.startswith('sqlite'):
pass
else:
raise
volumes.c.volume_type_id.alter(String(36))
volume_types.c.id.alter(String(36))
extra_specs.c.volume_type_id.alter(String(36))
vtype_list = list(volume_types.select().execute())
for t in vtype_list:
new_id = str(uuid.uuid4())
volumes.update().\
where(volumes.c.volume_type_id == t['id']).\
values(volume_type_id=new_id).execute()
extra_specs.update().\
where(extra_specs.c.volume_type_id == t['id']).\
values(volume_type_id=new_id).execute()
volume_types.update().\
where(volume_types.c.id == t['id']).\
values(id=new_id).execute()
for column in fkey_remove_list:
fkeys = list(column.foreign_keys)
if fkeys:
fkey_name = fkeys[0].constraint.name
fkey = ForeignKeyConstraint(columns=[column],
refcolumns=[volume_types.c.id],
name=fkey_name)
try:
fkey.create()
LOG.info('Created foreign key %s' % fkey_name)
except Exception:
if migrate_engine.url.get_dialect().name.startswith('sqlite'):
pass
else:
raise
def downgrade(migrate_engine):
"""Convert volume_type from UUID back to int."""
meta = MetaData()
meta.bind = migrate_engine
volumes = Table('volumes', meta, autoload=True)
volume_types = Table('volume_types', meta, autoload=True)
extra_specs = Table('volume_type_extra_specs', meta, autoload=True)
fkey_remove_list = [volumes.c.volume_type_id,
volume_types.c.id,
extra_specs.c.volume_type_id]
for column in fkey_remove_list:
fkeys = list(column.foreign_keys)
if fkeys:
fkey_name = fkeys[0].constraint.name
fkey = ForeignKeyConstraint(columns=[column],
refcolumns=[volume_types.c.id],
name=fkey_name)
try:
fkey.drop()
except Exception:
if migrate_engine.url.get_dialect().name.startswith('sqlite'):
pass
else:
raise
volumes.c.volume_type_id.alter(Integer)
volume_types.c.id.alter(Integer)
extra_specs.c.volume_type_id.alter(Integer)
vtype_list = list(volume_types.select().execute())
new_id = 1
for t in vtype_list:
volumes.update().\
where(volumes.c.volume_type_id == t['id']).\
values(volume_type_id=new_id).execute()
extra_specs.update().\
where(extra_specs.c.volume_type_id == t['id']).\
values(volume_type_id=new_id).execute()
volume_types.update().\
where(volume_types.c.id == t['id']).\
values(id=new_id).execute()
new_id += 1
for column in fkey_remove_list:
fkeys = list(column.foreign_keys)
if fkeys:
fkey_name = fkeys[0].constraint.name
fkey = ForeignKeyConstraint(columns=[column],
refcolumns=[volume_types.c.id],
name=fkey_name)
try:
fkey.create()
LOG.info('Created foreign key %s' % fkey_name)
except Exception:
if migrate_engine.url.get_dialect().name.startswith('sqlite'):
pass
else:
raise

6
cinder/db/sqlalchemy/models.py
View File

@@ -155,7 +155,7 @@ class Volume(BASE, CinderBase):
provider_location = Column(String(255))
provider_auth = Column(String(255))
volume_type_id = Column(Integer)
volume_type_id = Column(String(36))
class VolumeMetadata(BASE, CinderBase):
@@ -175,7 +175,7 @@ class VolumeMetadata(BASE, CinderBase):
class VolumeTypes(BASE, CinderBase):
"""Represent possible volume_types of volumes offered."""
__tablename__ = "volume_types"
id = Column(Integer, primary_key=True)
id = Column(String(36), primary_key=True)
name = Column(String(255))
volumes = relationship(Volume,
@@ -192,7 +192,7 @@ class VolumeTypeExtraSpecs(BASE, CinderBase):
id = Column(Integer, primary_key=True)
key = Column(String(255))
value = Column(String(255))
volume_type_id = Column(Integer,
volume_type_id = Column(String(36),
ForeignKey('volume_types.id'),
nullable=False)
volume_type = relationship(

2
cinder/exception.py
View File

@@ -372,7 +372,7 @@ class KeyPairExists(Duplicate):
class VolumeTypeExists(Duplicate):
message = _("Volume Type %(name)s already exists.")
message = _("Volume Type %(id)s already exists.")
class MigrationError(CinderException):

2
cinder/tests/api/openstack/fakes.py
View File

@@ -195,7 +195,7 @@ def stub_volume(id, **kwargs):
'display_description': 'displaydesc',
'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
'snapshot_id': None,
'volume_type_id': 'fakevoltype',
'volume_type_id': '3e196c20-3c06-11e2-81c1-0800200c9a66',
'volume_metadata': [],
'volume_type': {'name': 'vol_type_name'}}

1
cinder/tests/api/v1/test_volumes.py
View File

@@ -95,7 +95,6 @@ class VolumeApiTest(test.TestCase):
vol_type = FLAGS.default_volume_type
db.volume_type_create(context.get_admin_context(),
dict(name=vol_type, extra_specs={}))
db_vol_type = db.volume_type_get_by_name(context.get_admin_context(),
vol_type)

30
cinder/tests/test_migrations.py
View File

@@ -303,3 +303,33 @@ class TestMigrations(test.TestCase):
self.assertEqual(version,
migration_api.db_version(engine,
TestMigrations.REPOSITORY))
def test_migration_004(self):
"""Test that volume_type_id migration works correctly."""
for (key, engine) in self.engines.items():
migration_api.version_control(engine,
TestMigrations.REPOSITORY,
migration.INIT_VERSION)
migration_api.upgrade(engine, TestMigrations.REPOSITORY, 3)
metadata = sqlalchemy.schema.MetaData()
metadata.bind = engine
migration_api.upgrade(engine, TestMigrations.REPOSITORY, 4)
volumes = sqlalchemy.Table('volumes',
metadata,
autoload=True)
volume_types = sqlalchemy.Table('volume_types',
metadata,
autoload=True)
extra_specs = sqlalchemy.Table('volume_type_extra_specs',
metadata,
autoload=True)
self.assertTrue(isinstance(volumes.c.volume_type_id.type,
sqlalchemy.types.VARCHAR))
self.assertTrue(isinstance(volume_types.c.id.type,
sqlalchemy.types.VARCHAR))
self.assertTrue(isinstance(extra_specs.c.volume_type_id.type,
sqlalchemy.types.VARCHAR))
self.assertTrue(extra_specs.c.volume_type_id.foreign_keys)

14
cinder/tests/test_volume_types.py
View File

@@ -49,7 +49,7 @@ class VolumeTypeTestCase(test.TestCase):
"""Ensure volume types can be created and deleted."""
prev_all_vtypes = volume_types.get_all_types(self.ctxt)
volume_types.create(self.ctxt,
type_ref = volume_types.create(self.ctxt,
self.vol_type1_name,
self.vol_type1_specs)
new = volume_types.get_volume_type_by_name(self.ctxt,
@@ -67,7 +67,7 @@ class VolumeTypeTestCase(test.TestCase):
len(new_all_vtypes),
'drive type was not created')
volume_types.destroy(self.ctxt, self.vol_type1_name)
volume_types.destroy(self.ctxt, type_ref['id'])
new_all_vtypes = volume_types.get_all_types(self.ctxt)
self.assertEqual(prev_all_vtypes,
new_all_vtypes,
@@ -82,7 +82,7 @@ class VolumeTypeTestCase(test.TestCase):
def test_get_default_volume_type(self):
"""Ensures default volume type can be retrieved."""
volume_types.create(self.ctxt,
type_ref = volume_types.create(self.ctxt,
fake_flags.def_vol_type,
{})
default_vol_type = volume_types.get_default_volume_type()
@@ -98,15 +98,15 @@ class VolumeTypeTestCase(test.TestCase):
def test_non_existent_vol_type_shouldnt_delete(self):
"""Ensures that volume type creation fails with invalid args."""
self.assertRaises(exception.VolumeTypeNotFoundByName,
self.assertRaises(exception.VolumeTypeNotFound,
volume_types.destroy, self.ctxt, "sfsfsdfdfs")
def test_repeated_vol_types_shouldnt_raise(self):
"""Ensures that volume duplicates don't raise."""
new_name = self.vol_type1_name + "dup"
volume_types.create(self.ctxt, new_name)
volume_types.destroy(self.ctxt, new_name)
volume_types.create(self.ctxt, new_name)
type_ref = volume_types.create(self.ctxt, new_name)
volume_types.destroy(self.ctxt, type_ref['id'])
type_ref = volume_types.create(self.ctxt, new_name)
def test_invalid_volume_types_params(self):
"""Ensures that volume type creation fails with invalid args."""

4
cinder/tests/test_volume_types_extra_specs.py
View File

@@ -45,9 +45,9 @@ class VolumeTypeExtraSpecsTestCase(test.TestCase):
def tearDown(self):
# Remove the volume type from the database
db.volume_type_destroy(context.get_admin_context(),
self.vol_type1['name'])
self.vol_type1['id'])
db.volume_type_destroy(context.get_admin_context(),
self.vol_type2_noextra['name'])
self.vol_type2_noextra['id'])
super(VolumeTypeExtraSpecsTestCase, self).tearDown()
def test_volume_type_specs_get(self):

11
cinder/volume/volume_types.py
View File

@@ -34,22 +34,23 @@ LOG = logging.getLogger(__name__)
def create(context, name, extra_specs={}):
"""Creates volume types."""
try:
db.volume_type_create(context,
type_ref = db.volume_type_create(context,
dict(name=name,
extra_specs=extra_specs))
except exception.DBError, e:
LOG.exception(_('DB error: %s') % e)
raise exception.VolumeTypeCreateFailed(name=name,
extra_specs=extra_specs)
return type_ref
def destroy(context, name):
def destroy(context, id):
"""Marks volume types as deleted."""
if name is None:
msg = _("name cannot be None")
if id is None:
msg = _("id cannot be None")
raise exception.InvalidVolumeType(reason=msg)
else:
db.volume_type_destroy(context, name)
db.volume_type_destroy(context, id)
def get_all_types(context, inactive=0, search_opts={}):