From c69e564c6f6e523ca8b5e6065a0cf015c102bc4d Mon Sep 17 00:00:00 2001 From: Rajat Dhasmana Date: Fri, 23 Jul 2021 04:08:54 -0400 Subject: [PATCH] Fix: nfs format info limitation Format info support was added to the nfs driver with change[1] and due to a issue with groups, it was restricted for volumes with groups with change[2]. This was a problem with OVO handling of consistency groups/volume groups and is fixed in this patch thereby also removing the limitation on nfs driver. [1] https://review.opendev.org/c/openstack/cinder/+/761152 [2] https://review.opendev.org/c/openstack/cinder/+/780700 Co-Authored-By: Gorka Eguileor Change-Id: I078a54a47e43b1cc83b4fb1a8063b41ab35358a1 --- cinder/objects/volume.py | 17 +++++++++++ cinder/tests/unit/objects/test_volume.py | 30 ++++++++++++++++++++ cinder/volume/drivers/remotefs.py | 13 ++++----- cinder/volume/flows/manager/create_volume.py | 11 ++----- 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py index a4551f5b43a..79588bfd938 100644 --- a/cinder/objects/volume.py +++ b/cinder/objects/volume.py @@ -343,6 +343,23 @@ class Volume(cleanable.CinderCleanableObject, base.CinderObject, 'False!! Fix code here') updates['use_quota'] = use_quota + def populate_consistencygroup(self): + """Populate CG fields based on group fields. + + Method assumes that consistencygroup_id and consistencygroup fields + have not already been set. + + This is a hack to support backward compatibility of consistencygroup, + where we set the fields but don't want to write them to the DB, so we + mark them as not changed, so they won't be stored on the next save(). + """ + self.consistencygroup_id = self.group_id + if self.group_id and self.obj_attr_is_set('group'): + cg = objects.ConsistencyGroup() + cg.from_group(self.group) + self.consistencygroup = cg + self.obj_reset_changes(['consistencygroup', 'consistencygroup_id']) + def create(self): if self.obj_attr_is_set('id'): raise exception.ObjectActionError(action='create', diff --git a/cinder/tests/unit/objects/test_volume.py b/cinder/tests/unit/objects/test_volume.py index d440eba9e7d..332ec16d77e 100644 --- a/cinder/tests/unit/objects/test_volume.py +++ b/cinder/tests/unit/objects/test_volume.py @@ -29,6 +29,18 @@ from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume from cinder.tests.unit import objects as test_objects +fake_group = { + 'id': fake.GROUP_ID, + 'user_id': fake.USER_ID, + 'project_id': fake.PROJECT_ID, + 'host': 'fake_host', + 'availability_zone': 'fake_az', + 'name': 'fake_name', + 'description': 'fake_description', + 'group_type_id': fake.GROUP_TYPE_ID, + 'status': fields.GroupStatus.CREATING, +} + @ddt.ddt class TestVolume(test_objects.BaseObjectsTestCase): @@ -707,3 +719,21 @@ class TestVolumeList(test_objects.BaseObjectsTestCase): include_mock.assert_called_once_with(self.context, cluster, mock.sentinel.partial_rename, **filters) + + @mock.patch('cinder.db.group_create', + return_value=fake_group) + def test_populate_consistencygroup(self, mock_db_grp_create): + db_volume = fake_volume.fake_db_volume() + volume = objects.Volume._from_db_object(self.context, + objects.Volume(), db_volume) + + fake_grp = fake_group.copy() + del fake_grp['id'] + group = objects.Group(context=self.context, + **fake_grp) + group.create() + volume.group_id = group.id + volume.group = group + volume.populate_consistencygroup() + self.assertEqual(volume.group_id, volume.consistencygroup_id) + self.assertEqual(volume.group.id, volume.consistencygroup.id) diff --git a/cinder/volume/drivers/remotefs.py b/cinder/volume/drivers/remotefs.py index 6c3cb44a067..b5b2d49cacd 100644 --- a/cinder/volume/drivers/remotefs.py +++ b/cinder/volume/drivers/remotefs.py @@ -325,13 +325,12 @@ class RemoteFSDriver(driver.BaseVD): self._create_regular_file(volume_path, volume_size) self._set_rw_permissions(volume_path) - if not volume.consistencygroup_id and not volume.group_id: - volume.admin_metadata['format'] = self.format - # This is done here because when creating a volume from image, - # while encountering other volume.save() method fails for - # non-admins - with volume.obj_as_admin(): - volume.save() + volume.admin_metadata['format'] = self.format + # This is done here because when creating a volume from image, + # while encountering other volume.save() method fails for + # non-admins + with volume.obj_as_admin(): + volume.save() def _ensure_shares_mounted(self): """Look for remote shares in the flags and mount them locally.""" diff --git a/cinder/volume/flows/manager/create_volume.py b/cinder/volume/flows/manager/create_volume.py index c3b36723824..1734b3ae478 100644 --- a/cinder/volume/flows/manager/create_volume.py +++ b/cinder/volume/flows/manager/create_volume.py @@ -41,7 +41,6 @@ from cinder.image import image_utils from cinder.message import api as message_api from cinder.message import message_field from cinder import objects -from cinder.objects import consistencygroup from cinder.objects import fields from cinder import utils from cinder.volume.flows import common @@ -1190,14 +1189,8 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask): "Volume driver %s not initialized", driver_name) raise exception.DriverNotInitialized() - # NOTE(xyang): Populate consistencygroup_id and consistencygroup - # fields before passing to the driver. This is to support backward - # compatibility of consistencygroup. - if volume.group_id: - volume.consistencygroup_id = volume.group_id - cg = consistencygroup.ConsistencyGroup() - cg.from_group(volume.group) - volume.consistencygroup = cg + # For backward compatibilty + volume.populate_consistencygroup() create_type = volume_spec.pop('type', None) LOG.info("Volume %(volume_id)s: being created as %(create_type)s "