diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py index 7658aa5cbeb..950e92283ee 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py @@ -213,6 +213,12 @@ IGROUP1 = { 'initiator-group-name': IGROUP1_NAME, } +CUSTOM_IGROUP = { + 'initiator-group-os-type': 'linux', + 'initiator-group-type': 'fcp', + 'initiator-group-name': 'node1', +} + ISCSI_VOLUME = { 'name': 'fake_volume', 'id': 'fake_id', diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py index 06448e1a8af..a81778bc54f 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py @@ -400,10 +400,13 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): self.assertEqual( 0, self.library._create_igroup_add_initiators.call_count) + @ddt.data([], [fake.CUSTOM_IGROUP]) @mock.patch.object(uuid, 'uuid4', mock.Mock(return_value=fake.UUID1)) - def test_get_or_create_igroup_none_preexisting(self): - """This method also tests _create_igroup_add_initiators.""" - self.zapi_client.get_igroup_by_initiators.return_value = [] + def test_get_or_create_igroup_none_preexisting(self, igroups): + """Test _create_igroup_add_initiators with not OS igroups.""" + # We only care about the OpenStack igroups, so we must have the same + # result if there are no igroups and if the igroup is a custom one. + self.zapi_client.get_igroup_by_initiators.return_value = igroups igroup_name, os, ig_type = self.library._get_or_create_igroup( fake.FC_FORMATTED_INITIATORS, 'fcp', 'linux') diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py index 75d03b4bff0..398d3d97e5c 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py @@ -207,12 +207,17 @@ class NetAppBlockStorageCmodeLibraryTestCase(test.TestCase): self.library.vserver, volume_list, []) def test_find_mapped_lun_igroup(self): - igroups = [fake.IGROUP1] + igroups = [fake.IGROUP1, fake.CUSTOM_IGROUP] self.zapi_client.get_igroup_by_initiators.return_value = igroups - lun_maps = [{'initiator-group': fake.IGROUP1_NAME, - 'lun-id': '1', - 'vserver': fake.VSERVER_NAME}] + lun_maps = [ + {'initiator-group': fake.IGROUP1_NAME, + 'lun-id': '1', + 'vserver': fake.VSERVER_NAME}, + {'initiator-group': fake.CUSTOM_IGROUP['initiator-group-name'], + 'lun-id': '2', + 'vserver': fake.VSERVER_NAME} + ] self.zapi_client.get_lun_map.return_value = lun_maps (igroup, lun_id) = self.library._find_mapped_lun_igroup( @@ -253,12 +258,11 @@ class NetAppBlockStorageCmodeLibraryTestCase(test.TestCase): self.assertIsNone(lun_id) def test_find_mapped_lun_igroup_no_igroup_prefix(self): - igroups = [{'initiator-group-os-type': 'linux', - 'initiator-group-type': 'fcp', - 'initiator-group-name': 'igroup2'}] + igroups = [fake.CUSTOM_IGROUP] + expected_igroup = fake.CUSTOM_IGROUP['initiator-group-name'] self.zapi_client.get_igroup_by_initiators.return_value = igroups - lun_maps = [{'initiator-group': 'igroup2', + lun_maps = [{'initiator-group': expected_igroup, 'lun-id': '1', 'vserver': fake.VSERVER_NAME}] self.zapi_client.get_lun_map.return_value = lun_maps @@ -266,8 +270,8 @@ class NetAppBlockStorageCmodeLibraryTestCase(test.TestCase): (igroup, lun_id) = self.library._find_mapped_lun_igroup( fake.LUN_PATH, fake.FC_FORMATTED_INITIATORS) - self.assertIsNone(igroup) - self.assertIsNone(lun_id) + self.assertEqual(expected_igroup, igroup) + self.assertEqual('1', lun_id) def test_clone_lun_zero_block_count(self): """Test for when clone lun is not passed a block count.""" diff --git a/cinder/volume/drivers/netapp/dataontap/block_base.py b/cinder/volume/drivers/netapp/dataontap/block_base.py index 30e3b8d25cd..3ff5fdd61f3 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_base.py +++ b/cinder/volume/drivers/netapp/dataontap/block_base.py @@ -479,16 +479,17 @@ class NetAppBlockStorageLibrary(object): Creates igroup if not already present with given host os type, igroup type and adds initiators. """ + # Backend supports different igroups with the same initiators, so + # instead of reusing non OpenStack igroups, we make sure we only use + # our own, thus being compatible with custom igroups. igroups = self.zapi_client.get_igroup_by_initiators(initiator_list) - igroup_name = None - - if igroups: - igroup = igroups[0] + for igroup in igroups: igroup_name = igroup['initiator-group-name'] - host_os_type = igroup['initiator-group-os-type'] - initiator_group_type = igroup['initiator-group-type'] - - if not igroup_name: + if igroup_name.startswith(na_utils.OPENSTACK_PREFIX): + host_os_type = igroup['initiator-group-os-type'] + initiator_group_type = igroup['initiator-group-type'] + break + else: igroup_name = self._create_igroup_add_initiators( initiator_group_type, host_os_type, initiator_list) return igroup_name, host_os_type, initiator_group_type diff --git a/cinder/volume/drivers/netapp/dataontap/block_cmode.py b/cinder/volume/drivers/netapp/dataontap/block_cmode.py index 4836245579f..59d5b04fea2 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_cmode.py +++ b/cinder/volume/drivers/netapp/dataontap/block_cmode.py @@ -195,17 +195,20 @@ class NetAppBlockStorageCmodeLibrary(block_base.NetAppBlockStorageLibrary, def _find_mapped_lun_igroup(self, path, initiator_list): """Find an igroup for a LUN mapped to the given initiator(s).""" - initiator_igroups = self.zapi_client.get_igroup_by_initiators( - initiator_list) - lun_maps = self.zapi_client.get_lun_map(path) - if initiator_igroups and lun_maps: - for igroup in initiator_igroups: - igroup_name = igroup['initiator-group-name'] - if igroup_name.startswith(na_utils.OPENSTACK_PREFIX): - for lun_map in lun_maps: - if lun_map['initiator-group'] == igroup_name: - return igroup_name, lun_map['lun-id'] - return None, None + igroups = [igroup['initiator-group-name'] for igroup in + self.zapi_client.get_igroup_by_initiators(initiator_list)] + # Map igroup-name to lun-id, but only for the requested initiators. + lun_map = {v['initiator-group']: v['lun-id'] + for v in self.zapi_client.get_lun_map(path) + if v['initiator-group'] in igroups} + + igroup_name = lun_id = None + # Give preference to OpenStack igroups, just use the last one if not + # present to allow unmapping old mappings that used a custom igroup. + for igroup_name, lun_id in lun_map.items(): + if igroup_name.startswith(na_utils.OPENSTACK_PREFIX): + break + return igroup_name, lun_id def _clone_lun(self, name, new_name, space_reserved=None, qos_policy_group_name=None, src_block=0, dest_block=0, diff --git a/releasenotes/notes/fix-netapp-custom-igroup-e049b4f3b341dd54.yaml b/releasenotes/notes/fix-netapp-custom-igroup-e049b4f3b341dd54.yaml new file mode 100644 index 00000000000..f8f328d7616 --- /dev/null +++ b/releasenotes/notes/fix-netapp-custom-igroup-e049b4f3b341dd54.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fix NetApp iSCSI and FC driver issues with custom initiator groups. + (bug 1697490).