Merge "[Cephfs] Fix erroneous share mode override on ensure_shares"

manila/share/drivers/cephfs/driver.py

@@ -465,8 +465,9 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin,
             "sub_name": share["id"],
             "size": size,
             "namespace_isolated": True,
-            "mode": self._cephfs_volume_mode,
+            "mode": self._cephfs_volume_mode
         }
+
         if share['share_group_id'] is not None:
             argdict.update({"group_name": share["share_group_id"]})
 
@@ -540,8 +541,14 @@ class CephFSDriver(driver.ExecuteMixin, driver.GaneshaMixin,
             share_server=share_server)
 
     def ensure_share(self, context, share, share_server=None):
-        # Creation is idempotent
-        return self.create_share(context, share, share_server)
+        try:
+            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):
         # resize FS subvolume/share

manila/tests/share/drivers/cephfs/test_driver.py

@@ -236,34 +236,37 @@ class CephFSDriverTestCase(test.TestCase):
             share_server=None)
 
     def test_ensure_share(self):
-        create_share_prefix = "fs subvolume create"
-        get_path_prefix = "fs subvolume getpath"
-
-        create_share_dict = {
-            "vol_name": self._driver.volname,
-            "sub_name": self._share["id"],
-            "size": self._share["size"] * units.Gi,
-            "namespace_isolated": True,
-            "mode": DEFAULT_VOLUME_MODE,
+        expected_exports = {
+            'path': '1.2.3.4,5.6.7.8:/foo/bar',
+            'is_admin_only': False,
+            'metadata': {},
         }
+        self.mock_object(
+            self._driver, '_get_export_locations',
+            mock.Mock(return_value=expected_exports))
 
-        get_path_dict = {
-            "vol_name": self._driver.volname,
-            "sub_name": self._share["id"],
-        }
+        returned_exports = self._driver.ensure_share(self._context,
+                                                     self._share)
 
-        self._driver.ensure_share(self._context,
-                                  self._share)
+        self._driver._get_export_locations.assert_called_once_with(self._share)
 
-        driver.rados_command.assert_has_calls([
-            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(expected_exports, returned_exports)
 
-        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):
         clone_status_prefix = "fs clone status"

releasenotes/notes/bug-2002394-fix-bad-mode-enforcement-on-ensure-shares-a2e4d8f6c07c8cf5.yaml

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