From: Szymon Wroblewski Date: Thu, 29 Oct 2015 11:45:06 +0000 (+0100) Subject: Add some missing fields to Volume object X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=287c64ef4dc6e3ca5ff871dd9cb9d38906c21a6e;p=openstack-build%2Fcinder-build.git Add some missing fields to Volume object Volume object was missing fields that were defined on the SQLAlchemy model as relationships or backrefs such as consistencygroup, glance_metadata and snapshots. This commit adds fields representing these relationships. Partial-Bug: 1509012 Co-Authored-By: Szymon Borkowski Change-Id: I2f4cae34833b6b97278adf2abb52cef44405c093 --- diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py index 85579fe58..a423f9191 100644 --- a/cinder/objects/volume.py +++ b/cinder/objects/volume.py @@ -33,10 +33,12 @@ class Volume(base.CinderPersistentObject, base.CinderObject, # Version 1.0: Initial version # Version 1.1: Added metadata, admin_metadata, volume_attachment, and # volume_type - VERSION = '1.1' + # Version 1.2: Added glance_metadata, consistencygroup and snapshots + VERSION = '1.2' - OPTIONAL_FIELDS = ('metadata', 'admin_metadata', - 'volume_type', 'volume_attachment') + OPTIONAL_FIELDS = ('metadata', 'admin_metadata', 'glance_metadata', + 'volume_type', 'volume_attachment', 'consistencygroup', + 'snapshots') DEFAULT_EXPECTED_ATTR = ('admin_metadata', 'metadata') @@ -86,9 +88,13 @@ class Volume(base.CinderPersistentObject, base.CinderObject, 'metadata': fields.DictOfStringsField(nullable=True), 'admin_metadata': fields.DictOfStringsField(nullable=True), + 'glance_metadata': fields.DictOfStringsField(nullable=True), 'volume_type': fields.ObjectField('VolumeType', nullable=True), 'volume_attachment': fields.ObjectField('VolumeAttachmentList', nullable=True), + 'consistencygroup': fields.ObjectField('ConsistencyGroup', + nullable=True), + 'snapshots': fields.ObjectField('SnapshotList', nullable=True), } # NOTE(thangp): obj_extra_fields is used to hold properties that are not @@ -111,6 +117,7 @@ class Volume(base.CinderPersistentObject, base.CinderObject, super(Volume, self).__init__(*args, **kwargs) self._orig_metadata = {} self._orig_admin_metadata = {} + self._orig_glance_metadata = {} self._reset_metadata_tracking() @@ -126,6 +133,10 @@ class Volume(base.CinderPersistentObject, base.CinderObject, self._orig_admin_metadata = (dict(self.admin_metadata) if 'admin_metadata' in self else {}) + if fields is None or 'glance_metadata' in fields: + self._orig_glance_metadata = (dict(self.glance_metadata) + if 'glance_metadata' in self + else {}) def obj_what_changed(self): changes = super(Volume, self).obj_what_changed() @@ -134,6 +145,9 @@ class Volume(base.CinderPersistentObject, base.CinderObject, if ('admin_metadata' in self and self.admin_metadata != self._orig_admin_metadata): changes.add('admin_metadata') + if ('glance_metadata' in self and + self.glance_metadata != self._orig_glance_metadata): + changes.add('glance_metadata') return changes @@ -168,6 +182,12 @@ class Volume(base.CinderPersistentObject, base.CinderObject, if metadata: volume.admin_metadata = {item['key']: item['value'] for item in metadata} + if 'glance_metadata' in expected_attrs: + volume.glance_metadata = {} + metadata = db_volume.get('volume_glance_metadata', []) + if metadata: + volume.glance_metadata = {item['key']: item['value'] + for item in metadata} if 'volume_type' in expected_attrs: db_volume_type = db_volume.get('volume_type') if db_volume_type: @@ -180,6 +200,18 @@ class Volume(base.CinderPersistentObject, base.CinderObject, objects.VolumeAttachment, db_volume.get('volume_attachment')) volume.volume_attachment = attachments + if 'consistencygroup' in expected_attrs: + consistencygroup = objects.ConsistencyGroup(context) + consistencygroup._from_db_object(context, + consistencygroup, + db_volume['consistencygroup']) + volume.consistencygroup = consistencygroup + if 'snapshots' in expected_attrs: + snapshots = base.obj_make_list( + context, objects.SnapshotList(context), + objects.Snapshot, + db_volume['snapshots']) + volume.snapshots = snapshots volume._context = context volume.obj_reset_changes() @@ -191,6 +223,17 @@ class Volume(base.CinderPersistentObject, base.CinderObject, raise exception.ObjectActionError(action='create', reason=_('already created')) updates = self.cinder_obj_get_changes() + + if 'consistencygroup' in updates: + raise exception.ObjectActionError( + action='create', reason=_('consistencygroup assigned')) + if 'glance_metadata' in updates: + raise exception.ObjectActionError( + action='create', reason=_('glance_metadata assigned')) + if 'snapshots' in updates: + raise exception.ObjectActionError( + action='create', reason=_('snapshots assigned')) + db_volume = db.volume_create(self._context, updates) self._from_db_object(self._context, self, db_volume) @@ -198,6 +241,15 @@ class Volume(base.CinderPersistentObject, base.CinderObject, def save(self): updates = self.cinder_obj_get_changes() if updates: + if 'consistencygroup' in updates: + raise exception.ObjectActionError( + action='save', reason=_('consistencygroup changed')) + if 'glance_metadata' in updates: + raise exception.ObjectActionError( + action='save', reason=_('glance_metadata changed')) + if 'snapshots' in updates: + raise exception.ObjectActionError( + action='save', reason=_('snapshots changed')) if 'metadata' in updates: # Metadata items that are not specified in the # self.metadata will be deleted @@ -234,6 +286,9 @@ class Volume(base.CinderPersistentObject, base.CinderObject, if self._context.is_admin: self.admin_metadata = db.volume_admin_metadata_get( self._context, self.id) + elif attrname == 'glance_metadata': + self.glance_metadata = db.volume_glance_metadata_get( + self._context, self.id) elif attrname == 'volume_type': self.volume_type = objects.VolumeType.get_by_id( self._context, self.volume_type_id) @@ -241,6 +296,13 @@ class Volume(base.CinderPersistentObject, base.CinderObject, attachments = objects.VolumeAttachmentList.get_all_by_volume_id( self._context, self.id) self.volume_attachment = attachments + elif attrname == 'consistencygroup': + consistencygroup = objects.ConsistencyGroup.get_by_id( + self._context, self.consistencygroup_id) + self.consistencygroup = consistencygroup + elif attrname == 'snapshots': + self.snapshots = objects.SnapshotList.get_all_for_volume( + self._context, self.id) self.obj_reset_changes(fields=[attrname]) diff --git a/cinder/tests/unit/fake_consistencygroup.py b/cinder/tests/unit/fake_consistencygroup.py index ed52e63fc..042321e0a 100644 --- a/cinder/tests/unit/fake_consistencygroup.py +++ b/cinder/tests/unit/fake_consistencygroup.py @@ -19,9 +19,9 @@ from cinder import objects def fake_db_consistencygroup(**updates): db_values = { - 'id': 1, - 'user_id': 2, - 'project_id': 3, + 'id': '1', + 'user_id': '2', + 'project_id': '3', 'host': 'FakeHost', } for name, field in objects.ConsistencyGroup.fields.items(): diff --git a/cinder/tests/unit/fake_snapshot.py b/cinder/tests/unit/fake_snapshot.py index 0bc79edc1..1478beca7 100644 --- a/cinder/tests/unit/fake_snapshot.py +++ b/cinder/tests/unit/fake_snapshot.py @@ -19,11 +19,11 @@ from cinder.objects import snapshot def fake_db_snapshot(**updates): db_snapshot = { - 'id': 1, + 'id': '1', 'volume_id': 'fake_id', 'status': "creating", 'progress': '0%', - 'volume_size': 1, + 'volume_size': '1', 'display_name': 'fake_name', 'display_description': 'fake_description', 'metadata': {}, diff --git a/cinder/tests/unit/fake_volume.py b/cinder/tests/unit/fake_volume.py index 3eda5a2a4..070371602 100644 --- a/cinder/tests/unit/fake_volume.py +++ b/cinder/tests/unit/fake_volume.py @@ -26,11 +26,11 @@ def fake_db_volume(**updates): 'status': 'available', 'attach_status': 'detached', 'previous_status': None, - 'metadata': {}, - 'admin_metadata': {}, 'volume_attachment': [], 'volume_metadata': [], 'volume_admin_metadata': [], + 'volume_glance_metadata': [], + 'snapshots': [], } for name, field in objects.Volume.fields.items(): diff --git a/cinder/tests/unit/objects/test_objects.py b/cinder/tests/unit/objects/test_objects.py index 80a6a2834..b70783479 100644 --- a/cinder/tests/unit/objects/test_objects.py +++ b/cinder/tests/unit/objects/test_objects.py @@ -32,7 +32,7 @@ object_data = { 'ServiceList': '1.0-d242d3384b68e5a5a534e090ff1d5161', 'Snapshot': '1.0-a6c33eefeadefb324d79f72f66c54e9a', 'SnapshotList': '1.0-71661e7180ef6cc51501704a9bea4bf1', - 'Volume': '1.1-6037de222b09c330b33b419a0c1b10c6', + 'Volume': '1.2-97c3977846dae6588381e7bd3e6e6558', 'VolumeAttachment': '1.0-f14a7c03ffc5b93701d496251a5263aa', 'VolumeAttachmentList': '1.0-307d2b6c8dd55ef854f6386898e9e98e', 'VolumeList': '1.1-03ba6cb8c546683e64e15c50042cb1a3', diff --git a/cinder/tests/unit/objects/test_volume.py b/cinder/tests/unit/objects/test_volume.py index feafe2d65..2875af8b9 100644 --- a/cinder/tests/unit/objects/test_volume.py +++ b/cinder/tests/unit/objects/test_volume.py @@ -17,15 +17,21 @@ import mock from cinder import context from cinder import exception from cinder import objects +from cinder.tests.unit import fake_consistencygroup +from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume from cinder.tests.unit import objects as test_objects class TestVolume(test_objects.BaseObjectsTestCase): + @staticmethod + def _compare(test, db, obj): + db = {k: v for k, v in db.items() + if not k.endswith('metadata') or k.startswith('volume')} + test_objects.BaseObjectsTestCase._compare(test, db, obj) - @mock.patch('cinder.db.volume_glance_metadata_get', return_value={}) @mock.patch('cinder.db.sqlalchemy.api.volume_get') - def test_get_by_id(self, volume_get, volume_glance_metadata_get): + def test_get_by_id(self, volume_get): db_volume = fake_volume.fake_db_volume() volume_get.return_value = db_volume volume = objects.Volume.get_by_id(self.context, 1) @@ -98,6 +104,30 @@ class TestVolume(test_objects.BaseObjectsTestCase): admin_metadata_update.assert_called_once_with( admin_context, volume.id, {'key1': 'value1'}, True) + def test_save_with_glance_metadata(self): + db_volume = fake_volume.fake_db_volume() + volume = objects.Volume._from_db_object(self.context, + objects.Volume(), db_volume) + volume.display_name = 'foobar' + volume.glance_metadata = {'key1': 'value1'} + self.assertRaises(exception.ObjectActionError, volume.save) + + def test_save_with_consistencygroup(self): + db_volume = fake_volume.fake_db_volume() + volume = objects.Volume._from_db_object(self.context, + objects.Volume(), db_volume) + volume.display_name = 'foobar' + volume.consistencygroup = objects.ConsistencyGroup() + self.assertRaises(exception.ObjectActionError, volume.save) + + def test_save_with_snapshots(self): + db_volume = fake_volume.fake_db_volume() + volume = objects.Volume._from_db_object(self.context, + objects.Volume(), db_volume) + volume.display_name = 'foobar' + volume.snapshots = objects.SnapshotList() + self.assertRaises(exception.ObjectActionError, volume.save) + @mock.patch('cinder.db.volume_destroy') def test_destroy(self, volume_destroy): db_volume = fake_volume.fake_db_volume() @@ -129,12 +159,17 @@ class TestVolume(test_objects.BaseObjectsTestCase): metadata_delete.assert_called_once_with(self.context, '1', 'key2') @mock.patch('cinder.db.volume_metadata_get') + @mock.patch('cinder.db.volume_glance_metadata_get') @mock.patch('cinder.db.volume_admin_metadata_get') @mock.patch('cinder.objects.volume_type.VolumeType.get_by_id') @mock.patch('cinder.objects.volume_attachment.VolumeAttachmentList.' 'get_all_by_volume_id') - def test_obj_load_attr(self, mock_va_get_all_by_vol, mock_vt_get_by_id, - mock_admin_metadata_get, mock_metadata_get): + @mock.patch('cinder.objects.consistencygroup.ConsistencyGroup.get_by_id') + @mock.patch('cinder.objects.snapshot.SnapshotList.get_all_for_volume') + def test_obj_load_attr(self, mock_sl_get_all_for_volume, mock_cg_get_by_id, + mock_va_get_all_by_vol, mock_vt_get_by_id, + mock_admin_metadata_get, mock_glance_metadata_get, + mock_metadata_get): volume = objects.Volume._from_db_object( self.context, objects.Volume(), fake_volume.fake_db_volume()) @@ -144,6 +179,13 @@ class TestVolume(test_objects.BaseObjectsTestCase): self.assertEqual(metadata, volume.metadata) mock_metadata_get.assert_called_once_with(self.context, volume.id) + # Test glance_metadata lazy-loaded field + glance_metadata = {'foo': 'bar'} + mock_glance_metadata_get.return_value = glance_metadata + self.assertEqual(glance_metadata, volume.glance_metadata) + mock_glance_metadata_get.assert_called_once_with( + self.context, volume.id) + # Test volume_type lazy-loaded field volume_type = objects.VolumeType(context=self.context, id=5) mock_vt_get_by_id.return_value = volume_type @@ -151,6 +193,20 @@ class TestVolume(test_objects.BaseObjectsTestCase): mock_vt_get_by_id.assert_called_once_with(self.context, volume.volume_type_id) + # Test consistencygroup lazy-loaded field + consistencygroup = objects.ConsistencyGroup(context=self.context, id=2) + mock_cg_get_by_id.return_value = consistencygroup + self.assertEqual(consistencygroup, volume.consistencygroup) + mock_cg_get_by_id.assert_called_once_with(self.context, + volume.consistencygroup_id) + + # Test snapshots lazy-loaded field + snapshots = objects.SnapshotList(context=self.context, id=2) + mock_sl_get_all_for_volume.return_value = snapshots + self.assertEqual(snapshots, volume.snapshots) + mock_sl_get_all_for_volume.assert_called_once_with(self.context, + volume.id) + # Test volume_attachment lazy-loaded field va_objs = [objects.VolumeAttachment(context=self.context, id=i) for i in [3, 4, 5]] @@ -175,11 +231,43 @@ class TestVolume(test_objects.BaseObjectsTestCase): self.assertEqual(adm_metadata, volume.admin_metadata) mock_admin_metadata_get.assert_called_once_with(adm_context, volume.id) + def test_from_db_object_with_all_expected_attributes(self): + expected_attrs = ['metadata', 'admin_metadata', 'glance_metadata', + 'volume_type', 'volume_attachment', + 'consistencygroup'] + + db_metadata = [{'key': 'foo', 'value': 'bar'}] + db_admin_metadata = [{'key': 'admin_foo', 'value': 'admin_bar'}] + db_glance_metadata = [{'key': 'glance_foo', 'value': 'glance_bar'}] + db_volume_type = fake_volume.fake_db_volume_type() + db_volume_attachments = fake_volume.fake_db_volume_attachment() + db_consistencygroup = fake_consistencygroup.fake_db_consistencygroup() + db_snapshots = fake_snapshot.fake_db_snapshot() + + db_volume = fake_volume.fake_db_volume( + volume_metadata=db_metadata, + volume_admin_metadata=db_admin_metadata, + volume_glance_metadata=db_glance_metadata, + volume_type=db_volume_type, + volume_attachment=[db_volume_attachments], + consistencygroup=db_consistencygroup, + snapshots=[db_snapshots], + ) + volume = objects.Volume._from_db_object(self.context, objects.Volume(), + db_volume, expected_attrs) + + self.assertEqual({'foo': 'bar'}, volume.metadata) + self.assertEqual({'admin_foo': 'admin_bar'}, volume.admin_metadata) + self.assertEqual({'glance_foo': 'glance_bar'}, volume.glance_metadata) + self._compare(self, db_volume_type, volume.volume_type) + self._compare(self, db_volume_attachments, volume.volume_attachment) + self._compare(self, db_consistencygroup, volume.consistencygroup) + self._compare(self, db_snapshots, volume.snapshots) + class TestVolumeList(test_objects.BaseObjectsTestCase): - @mock.patch('cinder.db.volume_glance_metadata_get', return_value={}) @mock.patch('cinder.db.volume_get_all') - def test_get_all(self, volume_get_all, volume_glance_metadata_get): + def test_get_all(self, volume_get_all): db_volume = fake_volume.fake_db_volume() volume_get_all.return_value = [db_volume] diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index efe42f362..86908de0f 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -1600,26 +1600,25 @@ class VolumeManager(manager.SchedulerDependentManager): rpcapi = volume_rpcapi.VolumeAPI() # Create new volume on remote host - new_vol_values = dict(volume) - del new_vol_values['id'] - del new_vol_values['_name_id'] - new_vol_values.pop('name', None) - # We don't copy volume_type because the db sets that according to - # volume_type_id, which we do copy - new_vol_values.pop('volume_type', None) + + skip = {'id', '_name_id', 'name', 'host', 'status', + 'attach_status', 'migration_status', 'volume_type', + 'consistencygroup', 'volume_attachment'} + # We don't copy volume_type, consistencygroup and volume_attachment, + # because the db sets that according to [field]_id, which we do copy. + # We also skip some other values that are either set manually later or + # during creation of Volume object. + new_vol_values = {k: volume[k] for k in set(volume.keys()) - skip} if new_type_id: new_vol_values['volume_type_id'] = new_type_id - new_vol_values['host'] = host['host'] - new_vol_values['status'] = 'creating' - - # FIXME(jdg): using a : delimeter is confusing to - # me below here. We're adding a string member to a dict - # using a :, which is kind of a poor choice in this case - # I think - new_vol_values['migration_status'] = 'target:%s' % volume['id'] - new_vol_values['attach_status'] = 'detached' - new_vol_values.pop('volume_attachment', None) - new_volume = objects.Volume(context=ctxt, **new_vol_values) + new_volume = objects.Volume( + context=ctxt, + host=host['host'], + status='creating', + attach_status='detached', + migration_status='target:%s' % volume['id'], + **new_vol_values + ) new_volume.create() rpcapi.create_volume(ctxt, new_volume, host['host'], None, None, allow_reschedule=False)