diff --git a/manila/share/drivers/cephfs/driver.py b/manila/share/drivers/cephfs/driver.py index 8219b22ee3..4f824a21d7 100644 --- a/manila/share/drivers/cephfs/driver.py +++ b/manila/share/drivers/cephfs/driver.py @@ -1256,8 +1256,9 @@ class NFSProtocolHelperMixin(): export_location = { 'path': export_path, 'is_admin_only': False, - 'metadata': {}, - 'preferred': export_ip['preferred'] + 'metadata': { + 'preferred': export_ip['preferred'], + }, } export_locations.append(export_location) return export_locations @@ -1407,7 +1408,7 @@ class NFSProtocolHelper(NFSProtocolHelperMixin, ganesha.GaneshaNASHelper2): ganesha_export_ips = [self.ganesha_host] export_ips = [] - for ip in ganesha_export_ips: + for ip in set(ganesha_export_ips): export_ips.append({'ip': ip, 'preferred': False}) return export_ips @@ -1466,7 +1467,7 @@ class NFSClusterProtocolHelper(NFSProtocolHelperMixin, ganesha.NASHelperBase): ganesha_server_ips = ( [ganesha_server_ips] if ganesha_server_ips else []) - return ganesha_server_ips + return set(ganesha_server_ips) def _get_export_ips(self): """Get NFS cluster export ips.""" @@ -1501,13 +1502,15 @@ class NFSClusterProtocolHelper(NFSProtocolHelperMixin, ganesha.NASHelperBase): raise exception.ShareBackendException(msg=msg) export_ips = [] - for ip in ceph_nfs_export_ips: + for ip in set(ceph_nfs_export_ips): export_ips.append({'ip': ip, 'preferred': True}) # It's possible for deployers to state additional # NFS interfaces directly via manila.conf. If they do, # these are represented as non-preferred export paths. # This is mostly to allow NFS-Ganesha server migrations. + ganesha_export_ips = (eip for eip in ganesha_export_ips + if eip not in ceph_nfs_export_ips) for ip in ganesha_export_ips: export_ips.append({'ip': ip, 'preferred': False}) diff --git a/manila/tests/share/drivers/cephfs/test_driver.py b/manila/tests/share/drivers/cephfs/test_driver.py index 7c677c9540..4a9d87024f 100644 --- a/manila/tests/share/drivers/cephfs/test_driver.py +++ b/manila/tests/share/drivers/cephfs/test_driver.py @@ -1467,8 +1467,9 @@ class NFSProtocolHelperTestCase(test.TestCase): [{ 'path': '1.2.3.4:/foo/bar', 'is_admin_only': False, - 'metadata': {}, - 'preferred': False + 'metadata': { + 'preferred': False, + }, }], ret) def test_get_export_locations_with_export_ips_configured(self): @@ -1491,25 +1492,28 @@ class NFSProtocolHelperTestCase(test.TestCase): ret = helper.get_export_locations(self._share, cephfs_subvolume_path) - self.assertEqual( + self._assertEqualListsOfObjects( [ { 'path': '127.0.0.1:/foo/bar', 'is_admin_only': False, - 'metadata': {}, - 'preferred': False + 'metadata': { + 'preferred': False, + }, }, { 'path': '[fd3f:c057:1192:1::1]:/foo/bar', 'is_admin_only': False, - 'metadata': {}, - 'preferred': False + 'metadata': { + 'preferred': False, + }, }, { 'path': '[::1]:/foo/bar', 'is_admin_only': False, - 'metadata': {}, - 'preferred': False + 'metadata': { + 'preferred': False, + }, }, ], ret) @@ -1802,13 +1806,15 @@ class NFSClusterProtocolHelperTestCase(test.TestCase): expected_export_locations = [{ 'path': '10.0.0.10:/foo/bar', 'is_admin_only': False, - 'metadata': {}, - 'preferred': True + 'metadata': { + 'preferred': True, + }, }, { 'path': '10.0.0.11:/foo/bar', 'is_admin_only': False, - 'metadata': {}, - 'preferred': True + 'metadata': { + 'preferred': True, + }, }] export_locations = ( @@ -1819,17 +1825,19 @@ class NFSClusterProtocolHelperTestCase(test.TestCase): self._rados_client, cluster_info_prefix, cluster_info_dict) - self.assertEqual(expected_export_locations, export_locations) + self._assertEqualListsOfObjects(expected_export_locations, + export_locations) + + @ddt.data('cephfs_ganesha_server_ip', 'cephfs_ganesha_export_ips') + def test_get_export_locations_ganesha_still_configured(self, confopt): + if confopt == 'cephfs_ganesha_server_ip': + val = '10.0.0.1' + else: + val = ['10.0.0.2', '10.0.0.3'] - @ddt.data( - ('cephfs_ganesha_server_ip', '10.0.0.1'), - ('cephfs_ganesha_export_ips', ['10.0.0.2, 10.0.0.3']) - ) - @ddt.unpack - def test_get_export_locations_ganesha_still_configured(self, opt, val): cluster_info_prefix = "nfs cluster info" nfs_clusterid = self._nfscluster_protocol_helper.nfs_clusterid - self.fake_conf.set_default(opt, val) + self.fake_conf.set_default(confopt, val) cluster_info_dict = { "cluster_id": nfs_clusterid, @@ -1850,46 +1858,65 @@ class NFSClusterProtocolHelperTestCase(test.TestCase): driver.rados_command.return_value = json.dumps(cluster_info) fake_cephfs_subvolume_path = "/foo/bar" - expected_export_locations = [{ - 'path': '10.0.0.10:/foo/bar', - 'is_admin_only': False, - 'metadata': {}, - 'preferred': True - }, { - 'path': '10.0.0.11:/foo/bar', - 'is_admin_only': False, - 'metadata': {}, - 'preferred': True - }] + expected_export_locations = [ + { + 'path': '10.0.0.10:/foo/bar', + 'is_admin_only': False, + 'metadata': { + 'preferred': True, + }, + }, + { + 'path': '10.0.0.11:/foo/bar', + 'is_admin_only': False, + 'metadata': { + 'preferred': True, + }, + }, + ] + if isinstance(val, list): for ip in val: expected_export_locations.append( { 'path': f'{ip}:/foo/bar', 'is_admin_only': False, - 'metadata': {}, - 'preferred': False - } + 'metadata': { + 'preferred': False, + }, + }, ) else: expected_export_locations.append( { 'path': f'{val}:/foo/bar', 'is_admin_only': False, - 'metadata': {}, - 'preferred': False + 'metadata': { + 'preferred': False, + }, } ) + expected_export_locations = sorted( + expected_export_locations, + key=lambda d: d['path'] + ) export_locations = ( self._nfscluster_protocol_helper.get_export_locations( - self._share, fake_cephfs_subvolume_path)) + self._share, fake_cephfs_subvolume_path) + ) + + actual_export_locations = sorted( + export_locations, + key=lambda d: d['path'] + ) driver.rados_command.assert_called_once_with( self._rados_client, cluster_info_prefix, cluster_info_dict) - self.assertEqual(expected_export_locations, export_locations) + self.assertEqual(expected_export_locations, + actual_export_locations) @ddt.ddt diff --git a/releasenotes/notes/bug-2053100-fix-ceph-driver-preferred-path-update-70147668e0f19c4d.yaml b/releasenotes/notes/bug-2053100-fix-ceph-driver-preferred-path-update-70147668e0f19c4d.yaml new file mode 100644 index 0000000000..76dc09245b --- /dev/null +++ b/releasenotes/notes/bug-2053100-fix-ceph-driver-preferred-path-update-70147668e0f19c4d.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + The CephFS driver used to set the "preferred" export path incorrectly. + It is now fixed to set it as part of export location metadata. + See `Launchpad bug 2053100 `_ + for more details.