From ffe5a72f44f28d6932f326caf4680fc125b2dce0 Mon Sep 17 00:00:00 2001 From: xing-yang Date: Wed, 12 Jul 2017 15:47:46 -0700 Subject: [PATCH] Pop "consistencygroup" from volume object Got the following error during the groups tempest test: DBAPIError exception wrapped from (psycopg2.ProgrammingError) multiple assignments to same column "consistencygroup_id". The solution is to pop "consistencygroup" from volume object before saving to db. This is because consistencygroup is the name of a relationship in the ORM Volume model, so SQLA tries to do some kind of update of the foreign key based on the provided updates if 'consistencygroup' is in updates. Closes-bug: #1703706 Change-Id: I9489ebd20c0744d69431bc91634e1decffcb9ed9 --- cinder/objects/volume.py | 16 ++++++++++------ cinder/tests/unit/objects/test_volume.py | 15 ++++++++++++++- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py index 42afe4bd412..50be64e2aba 100644 --- a/cinder/objects/volume.py +++ b/cinder/objects/volume.py @@ -342,12 +342,16 @@ class Volume(cleanable.CinderCleanableObject, base.CinderObject, def save(self): updates = self.cinder_obj_get_changes() if updates: - if 'consistencygroup' in updates: - # NOTE(xyang): Allow this to pass if 'consistencygroup' is - # set to None. This is to support backward compatibility. - if updates.get('consistencygroup'): - raise exception.ObjectActionError( - action='save', reason=_('consistencygroup changed')) + # NOTE(xyang): Allow this to pass if 'consistencygroup' is + # set to None. This is to support backward compatibility. + # Also remove 'consistencygroup' from updates because + # consistencygroup is the name of a relationship in the ORM + # Volume model, so SQLA tries to do some kind of update of + # the foreign key based on the provided updates if + # 'consistencygroup' is in updates. + if updates.pop('consistencygroup', None): + raise exception.ObjectActionError( + action='save', reason=_('consistencygroup changed')) if 'group' in updates: raise exception.ObjectActionError( action='save', reason=_('group changed')) diff --git a/cinder/tests/unit/objects/test_volume.py b/cinder/tests/unit/objects/test_volume.py index 0f02b5e8d28..6aceea4dde1 100644 --- a/cinder/tests/unit/objects/test_volume.py +++ b/cinder/tests/unit/objects/test_volume.py @@ -63,15 +63,28 @@ class TestVolume(test_objects.BaseObjectsTestCase): self.assertEqual(db_volume['id'], volume.id) @mock.patch('cinder.db.volume_update') - def test_save(self, volume_update): + @ddt.data(False, True) + def test_save(self, test_cg, volume_update): db_volume = fake_volume.fake_db_volume() volume = objects.Volume._from_db_object(self.context, objects.Volume(), db_volume) volume.display_name = 'foobar' + if test_cg: + volume.consistencygroup = None volume.save() volume_update.assert_called_once_with(self.context, volume.id, {'display_name': 'foobar'}) + def test_save_error(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 = ( + fake_consistencygroup.fake_consistencyobject_obj(self.context)) + self.assertRaises(exception.ObjectActionError, + volume.save) + @mock.patch('cinder.db.volume_metadata_update', return_value={'key1': 'value1'}) @mock.patch('cinder.db.volume_update')