openstack/manila
Code Issues Proposed changes

Merge "Fix "preferred" export locations in the CephFS driver" into stable/2024.2

Browse Source
This commit is contained in:
Zuul 2024-09-26 19:51:26 +00:00 committed by Gerrit Code Review
commit 8c57c7a46f
3 changed files with 81 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
manila/share/drivers/cephfs/driver.py
View File

@ -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})

105
manila/tests/share/drivers/cephfs/test_driver.py
View File

@ -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

7
releasenotes/notes/bug-2053100-fix-ceph-driver-preferred-path-update-70147668e0f19c4d.yaml Normal file
View File

@ -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 <https://launchpad.net/bugs/2053100>`_
for more details.