openstack/cinder
Code Issues Proposed changes

Use resource_backend for volumes and groups

Browse Source 
resource_backend should be used during groups update validation
procedure  instead of 'host' attribute because it works well
both in clustered and single node deployments.

Change-Id: I4f80bb3e39e04ac5c524c7c1ad8859eef76a910a
Closes-Bug: #1876133
This commit is contained in:
Ivan Kolodyazhny 2020-04-30 18:46:07 +03:00
parent fff95fa6a6
commit 7d211d6221
3 changed files with 47 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
cinder/group/api.py
View File

@ -723,7 +723,7 @@ class API(base.Base):
for add_vol in add_volumes_list: for add_vol in add_volumes_list:
try: try:
add_vol_ref = self.db.volume_get(context, add_vol) add_vol_ref = objects.Volume.get_by_id(context, add_vol)
except exception.VolumeNotFound: except exception.VolumeNotFound:
msg = (_("Cannot add volume %(volume_id)s to " msg = (_("Cannot add volume %(volume_id)s to "
"group %(group_id)s because volume cannot be " "group %(group_id)s because volume cannot be "
@ -731,7 +731,7 @@ class API(base.Base):
{'volume_id': add_vol, {'volume_id': add_vol,
'group_id': group.id}) 'group_id': group.id})
raise exception.InvalidVolume(reason=msg) raise exception.InvalidVolume(reason=msg)
orig_group = add_vol_ref.get('group_id', None) orig_group = add_vol_ref.group_id
if orig_group: if orig_group:
# If volume to be added is already in the group to be updated, # If volume to be added is already in the group to be updated,
# it should have been removed from the add_volumes_list in the # it should have been removed from the add_volumes_list in the
@ -740,7 +740,7 @@ class API(base.Base):
msg = (_("Cannot add volume %(volume_id)s to group " msg = (_("Cannot add volume %(volume_id)s to group "
"%(group_id)s because it is already in " "%(group_id)s because it is already in "
"group %(orig_group)s.") % "group %(orig_group)s.") %
{'volume_id': add_vol_ref['id'], {'volume_id': add_vol_ref.id,
'group_id': group.id, 'group_id': group.id,
'orig_group': orig_group}) 'orig_group': orig_group})
raise exception.InvalidVolume(reason=msg) raise exception.InvalidVolume(reason=msg)
@ -749,15 +749,15 @@ class API(base.Base):
msg = (_("Cannot add volume %(volume_id)s to group " msg = (_("Cannot add volume %(volume_id)s to group "
"%(group_id)s as they belong to different " "%(group_id)s as they belong to different "
"projects.") % "projects.") %
{'volume_id': add_vol_ref['id'], {'volume_id': add_vol_ref.id,
'group_id': group.id}) 'group_id': group.id})
raise exception.InvalidVolume(reason=msg) raise exception.InvalidVolume(reason=msg)
add_vol_type_id = add_vol_ref.get('volume_type_id', None) add_vol_type_id = add_vol_ref.volume_type_id
if not add_vol_type_id: if not add_vol_type_id:
msg = (_("Cannot add volume %(volume_id)s to group " msg = (_("Cannot add volume %(volume_id)s to group "
"%(group_id)s because it has no volume " "%(group_id)s because it has no volume "
"type.") % "type.") %
{'volume_id': add_vol_ref['id'], {'volume_id': add_vol_ref.id,
'group_id': group.id}) 'group_id': group.id})
raise exception.InvalidVolume(reason=msg) raise exception.InvalidVolume(reason=msg)
vol_type_ids = [v_type.id for v_type in group.volume_types] vol_type_ids = [v_type.id for v_type in group.volume_types]
@ -766,27 +766,29 @@ class API(base.Base):
"%(group_id)s because volume type " "%(group_id)s because volume type "
"%(volume_type)s is not supported by the " "%(volume_type)s is not supported by the "
"group.") % "group.") %
{'volume_id': add_vol_ref['id'], {'volume_id': add_vol_ref.id,
'group_id': group.id, 'group_id': group.id,
'volume_type': add_vol_type_id}) 'volume_type': add_vol_type_id})
raise exception.InvalidVolume(reason=msg) raise exception.InvalidVolume(reason=msg)
if (add_vol_ref['status'] not in if (add_vol_ref.status not in
VALID_ADD_VOL_TO_GROUP_STATUS): VALID_ADD_VOL_TO_GROUP_STATUS):
msg = (_("Cannot add volume %(volume_id)s to group " msg = (_("Cannot add volume %(volume_id)s to group "
"%(group_id)s because volume is in an " "%(group_id)s because volume is in an "
"invalid state: %(status)s. Valid states are: " "invalid state: %(status)s. Valid states are: "
"%(valid)s.") % "%(valid)s.") %
{'volume_id': add_vol_ref['id'], {'volume_id': add_vol_ref.id,
'group_id': group.id, 'group_id': group.id,
'status': add_vol_ref['status'], 'status': add_vol_ref.status,
'valid': VALID_ADD_VOL_TO_GROUP_STATUS}) 'valid': VALID_ADD_VOL_TO_GROUP_STATUS})
raise exception.InvalidVolume(reason=msg) raise exception.InvalidVolume(reason=msg)
# group.host and add_vol_ref['host'] are in this format: # group.resource_backend and add_vol_ref.resource_backend are
# 'host@backend#pool'. Extract host (host@backend) before # in this format like 'host@backend#pool' in a non-HA
# doing comparison. # deployment and will contain cluster_name in
vol_host = volume_utils.extract_host(add_vol_ref['host']) # A/A HA deployment.
group_host = volume_utils.extract_host(group.host) vol_host = volume_utils.extract_host(
add_vol_ref.resource_backend)
group_host = volume_utils.extract_host(group.resource_backend)
if group_host != vol_host: if group_host != vol_host:
raise exception.InvalidVolume( raise exception.InvalidVolume(
reason=_("Volume is not local to this node.")) reason=_("Volume is not local to this node."))
@ -794,12 +796,12 @@ class API(base.Base):
# Volume exists. It will be added to CG. # Volume exists. It will be added to CG.
if add_volumes_new: if add_volumes_new:
add_volumes_new += "," add_volumes_new += ","
add_volumes_new += add_vol_ref['id'] add_volumes_new += add_vol_ref.id
else: else:
msg = (_("Cannot add volume %(volume_id)s to group " msg = (_("Cannot add volume %(volume_id)s to group "
"%(group_id)s because volume does not exist.") % "%(group_id)s because volume does not exist.") %
{'volume_id': add_vol_ref['id'], {'volume_id': add_vol_ref.id,
'group_id': group.id}) 'group_id': group.id})
raise exception.InvalidVolume(reason=msg) raise exception.InvalidVolume(reason=msg)

28
cinder/tests/unit/group/test_groups_api.py
View File

@ -274,6 +274,8 @@ class GroupAPITestCase(test.TestCase):
self.group_api._validate_add_volumes, self.ctxt, self.group_api._validate_add_volumes, self.ctxt,
[], ['123456789'], grp) [], ['123456789'], grp)
@ddt.data(['test_host@fakedrv#fakepool', 'test_host@fakedrv#fakepool'],
['test_host@fakedrv#fakepool', 'test_host2@fakedrv#fakepool'])
@mock.patch('cinder.volume.rpcapi.VolumeAPI.update_group') @mock.patch('cinder.volume.rpcapi.VolumeAPI.update_group')
@mock.patch('cinder.db.volume_get_all_by_generic_group') @mock.patch('cinder.db.volume_get_all_by_generic_group')
@mock.patch('cinder.group.api.API._cast_create_group') @mock.patch('cinder.group.api.API._cast_create_group')
@ -281,7 +283,7 @@ class GroupAPITestCase(test.TestCase):
@mock.patch('cinder.objects.Group') @mock.patch('cinder.objects.Group')
@mock.patch('cinder.db.group_type_get') @mock.patch('cinder.db.group_type_get')
@mock.patch('cinder.db.volume_types_get_by_name_or_id') @mock.patch('cinder.db.volume_types_get_by_name_or_id')
def test_update(self, mock_volume_types_get, def test_update(self, hosts, mock_volume_types_get,
mock_group_type_get, mock_group, mock_group_type_get, mock_group,
mock_update_quota, mock_cast_create_group, mock_update_quota, mock_cast_create_group,
mock_volume_get_all, mock_rpc_update_group): mock_volume_get_all, mock_rpc_update_group):
@ -307,26 +309,32 @@ class GroupAPITestCase(test.TestCase):
self.assertEqual(grp.obj_to_primitive(), ret_group.obj_to_primitive()) self.assertEqual(grp.obj_to_primitive(), ret_group.obj_to_primitive())
ret_group.volume_types = [vol_type] ret_group.volume_types = [vol_type]
ret_group.host = "test_host@fakedrv#fakepool" ret_group.host = hosts[0]
# set resource_backend directly because ret_group
# is instance of MagicMock
ret_group.resource_backend = 'fake-cluster'
ret_group.status = fields.GroupStatus.AVAILABLE ret_group.status = fields.GroupStatus.AVAILABLE
ret_group.id = fake.GROUP_ID ret_group.id = fake.GROUP_ID
vol1 = utils.create_volume( vol1 = utils.create_volume(
self.ctxt, host=ret_group.host, self.ctxt, host=hosts[1],
availability_zone=ret_group.availability_zone,
volume_type_id=fake.VOLUME_TYPE_ID)
vol2 = utils.create_volume(
self.ctxt, host=ret_group.host,
availability_zone=ret_group.availability_zone, availability_zone=ret_group.availability_zone,
volume_type_id=fake.VOLUME_TYPE_ID, volume_type_id=fake.VOLUME_TYPE_ID,
group_id=fake.GROUP_ID) cluster_name='fake-cluster')
vol2 = utils.create_volume(
self.ctxt, host=hosts[1],
availability_zone=ret_group.availability_zone,
volume_type_id=fake.VOLUME_TYPE_ID,
group_id=fake.GROUP_ID,
cluster_name='fake-cluster')
vol2_dict = { vol2_dict = {
'id': vol2.id, 'id': vol2.id,
'group_id': fake.GROUP_ID, 'group_id': fake.GROUP_ID,
'volume_type_id': fake.VOLUME_TYPE_ID, 'volume_type_id': fake.VOLUME_TYPE_ID,
'availability_zone': ret_group.availability_zone, 'availability_zone': ret_group.availability_zone,
'host': ret_group.host, 'host': hosts[1],
'status': 'available', 'status': 'available',
} }
mock_volume_get_all.return_value = [vol2_dict] mock_volume_get_all.return_value = [vol2_dict]

10
releasenotes/notes/fix-groups-actions-in-a-a-mode-5d554b30a26da22c.yaml Normal file
View File

@ -0,0 +1,10 @@
---
fixes:
- |
Fixed volume group action in Active/Active HA deployment:
* Update group
(`#1876133 <https://bugs.launchpad.net/cinder/+bug/1876133>`_)
* Create group from group snapshot
(`#1867906 <https://bugs.launchpad.net/cinder/+bug/1867906>`_)