[Cephfs] Fix erroneous share mode override on ensure_shares
In order to ensure that a share existed, we used to send the same request to Ceph as we would do with the share creation. In that request, we will set the share mode, which is a value set by the deployer through the `cephfs_volume_mode` config option. If the share was already created, Ceph would not override the value we were forwarding for the permission mode. After a bugfix [1], the value of mode started being reinforced when sent, even if the subvolume was already created. If the value for `cephfs_volume_mode` changed after the share was created, the mode is now being overwritten after manila startup on ensure_shares. Fixed this issue by modifying the ensure_shares to only confirm if the export exists. [1] https://tracker.ceph.com/issues/54375 Closes-Bug: #2002394 Change-Id: Ic5b5b42b882ce1346b4b46d1b29aa31740933e0e
This commit is contained in:
parent
6a799bfda3
commit
c61c1977c2
@ -465,8 +465,9 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin,
|
|||||||
"sub_name": share["id"],
|
"sub_name": share["id"],
|
||||||
"size": size,
|
"size": size,
|
||||||
"namespace_isolated": True,
|
"namespace_isolated": True,
|
||||||
"mode": self._cephfs_volume_mode,
|
"mode": self._cephfs_volume_mode
|
||||||
}
|
}
|
||||||
|
|
||||||
if share['share_group_id'] is not None:
|
if share['share_group_id'] is not None:
|
||||||
argdict.update({"group_name": share["share_group_id"]})
|
argdict.update({"group_name": share["share_group_id"]})
|
||||||
|
|
||||||
@ -540,8 +541,14 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin,
|
|||||||
share_server=share_server)
|
share_server=share_server)
|
||||||
|
|
||||||
def ensure_share(self, context, share, share_server=None):
|
def ensure_share(self, context, share, share_server=None):
|
||||||
# Creation is idempotent
|
try:
|
||||||
return self.create_share(context, share, share_server)
|
export_location = self._get_export_locations(share)
|
||||||
|
except exception.ShareBackendException as e:
|
||||||
|
if 'does not exist' in str(e).lower():
|
||||||
|
raise exception.ShareResourceNotFound(share_id=share['id'])
|
||||||
|
raise
|
||||||
|
|
||||||
|
return export_location
|
||||||
|
|
||||||
def extend_share(self, share, new_size, share_server=None):
|
def extend_share(self, share, new_size, share_server=None):
|
||||||
# resize FS subvolume/share
|
# resize FS subvolume/share
|
||||||
|
@ -236,34 +236,37 @@ class CephFSDriverTestCase(test.TestCase):
|
|||||||
share_server=None)
|
share_server=None)
|
||||||
|
|
||||||
def test_ensure_share(self):
|
def test_ensure_share(self):
|
||||||
create_share_prefix = "fs subvolume create"
|
expected_exports = {
|
||||||
get_path_prefix = "fs subvolume getpath"
|
'path': '1.2.3.4,5.6.7.8:/foo/bar',
|
||||||
|
'is_admin_only': False,
|
||||||
create_share_dict = {
|
'metadata': {},
|
||||||
"vol_name": self._driver.volname,
|
|
||||||
"sub_name": self._share["id"],
|
|
||||||
"size": self._share["size"] * units.Gi,
|
|
||||||
"namespace_isolated": True,
|
|
||||||
"mode": DEFAULT_VOLUME_MODE,
|
|
||||||
}
|
}
|
||||||
|
self.mock_object(
|
||||||
|
self._driver, '_get_export_locations',
|
||||||
|
mock.Mock(return_value=expected_exports))
|
||||||
|
|
||||||
get_path_dict = {
|
returned_exports = self._driver.ensure_share(self._context,
|
||||||
"vol_name": self._driver.volname,
|
self._share)
|
||||||
"sub_name": self._share["id"],
|
|
||||||
}
|
|
||||||
|
|
||||||
self._driver.ensure_share(self._context,
|
self._driver._get_export_locations.assert_called_once_with(self._share)
|
||||||
self._share)
|
|
||||||
|
|
||||||
driver.rados_command.assert_has_calls([
|
self.assertEqual(expected_exports, returned_exports)
|
||||||
mock.call(self._driver.rados_client,
|
|
||||||
create_share_prefix,
|
|
||||||
create_share_dict),
|
|
||||||
mock.call(self._driver.rados_client,
|
|
||||||
get_path_prefix,
|
|
||||||
get_path_dict)])
|
|
||||||
|
|
||||||
self.assertEqual(2, driver.rados_command.call_count)
|
def test_ensure_share_subvolume_not_found(self):
|
||||||
|
message = f"Error ENOENT: subvolume {self._share['id']} does not exist"
|
||||||
|
expected_exception = exception.ShareBackendException(message)
|
||||||
|
|
||||||
|
self.mock_object(
|
||||||
|
self._driver, '_get_export_locations',
|
||||||
|
mock.Mock(side_effect=expected_exception))
|
||||||
|
|
||||||
|
self.assertRaises(
|
||||||
|
exception.ShareResourceNotFound,
|
||||||
|
self._driver.ensure_share,
|
||||||
|
self._context,
|
||||||
|
self._share)
|
||||||
|
|
||||||
|
self._driver._get_export_locations.assert_called_once_with(self._share)
|
||||||
|
|
||||||
def test_delete_share(self):
|
def test_delete_share(self):
|
||||||
clone_status_prefix = "fs clone status"
|
clone_status_prefix = "fs clone status"
|
||||||
|
@ -0,0 +1,13 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
Fixed an issue that made the CephFS driver to override the permissions in
|
||||||
|
a share. After `a bugfix <https://tracker.ceph.com/issues/54375>`_, Ceph's
|
||||||
|
idempotent creation of shares had a change on its behavior. If a share
|
||||||
|
mode was modified outside of Manila, or the configuration value for
|
||||||
|
`cephfs_volume_mode` was changed in Manila when shares had already been
|
||||||
|
created, these shares would have their mode changed while Manila attempted
|
||||||
|
to ensure that such share exists using the idempotent creation, potentially
|
||||||
|
breaking clients. The CephFS driver will no longer send create calls to the
|
||||||
|
backend when ensuring a share exists. For more details, please refer to
|
||||||
|
`Bug #2002394 <https://bugs.launchpad.net/manila/+bug/2002394>`_
|
Loading…
x
Reference in New Issue
Block a user