From 06da39f3733df0ae9b6d630d60aae2f0389af3b6 Mon Sep 17 00:00:00 2001 From: Goutham Pacha Ravi Date: Wed, 11 Sep 2024 22:23:19 -0700 Subject: [PATCH] Fix "preferred" export locations in the CephFS driver The CephFS driver used to set the "preferred" export path by sending back an export location model update; however, the "preferred" keyword is expected in the export location's metadata, not in the export location dict. Change-Id: Id2214412fe75456cdb52edb5ff261d32b063655f Closes-Bug: #2053100 (cherry picked from commit 17088aa57af3ae2dd5a16e2e8558ec471d2c1cc5) (cherry picked from commit 620933cdb98f3560a480923ba096a6215d7d0edd) --- manila/share/drivers/cephfs/driver.py | 13 ++- .../tests/share/drivers/cephfs/test_driver.py | 105 +++++++++++------- ...referred-path-update-70147668e0f19c4d.yaml | 7 ++ 3 files changed, 81 insertions(+), 44 deletions(-) create mode 100644 releasenotes/notes/bug-2053100-fix-ceph-driver-preferred-path-update-70147668e0f19c4d.yaml diff --git a/manila/share/drivers/cephfs/driver.py b/manila/share/drivers/cephfs/driver.py index 5964f600ba..f71d2d3d54 100644 --- a/manila/share/drivers/cephfs/driver.py +++ b/manila/share/drivers/cephfs/driver.py @@ -1068,8 +1068,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 @@ -1219,7 +1220,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 @@ -1278,7 +1279,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.""" @@ -1313,13 +1314,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 badc7e4ffe..fbcdfdf2a1 100644 --- a/manila/tests/share/drivers/cephfs/test_driver.py +++ b/manila/tests/share/drivers/cephfs/test_driver.py @@ -1105,8 +1105,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): @@ -1129,25 +1130,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) @@ -1432,13 +1436,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 = ( @@ -1449,17 +1455,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, @@ -1480,46 +1488,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.