cephfs_native: add read-only share support

Add read-only share support for cephfs_native driver
using CephFSVolumeClient's enhanced authorize() interface.
Ensure backwards compatibility with older version of
CephFSVolumeClient by checking it's version attribute, or
lack thereof.

The support for read-only authorize() was added in
CephFSVolumeClient with the following commits in Ceph:

Master branch:
https://github.com/ceph/ceph/commit/011ea5e7fb35ee0

Jewel branch:
https://github.com/ceph/ceph/commit/2cd3ed8a59786be

Change-Id: I29eb45104c04da1706c3978441babe9c4a52ca02
DocImpact
Partially-Implements: bp cephfs-native-driver-enhancements

doc/source/devref/cephfs_native_driver.rst

@@ -42,7 +42,10 @@ The following operations are supported with CephFS backend:
 - Allow/deny CephFS share access
 
   * Only ``cephx`` access type is supported for CephFS protocol.
-  * Only Read/write access level is supported.
+  * ``read-only`` access level is supported in Newton or later versions
+    of manila.
+  * ``read-write`` access level is supported in Mitaka or later versions
+    of manila.
 
 - Extend/shrink share
 - Create/delete snapshot

doc/source/devref/share_back_ends_feature_support_mapping.rst

@@ -114,7 +114,7 @@ Mapping of share drivers and share access rules support
 +----------------------------------------+--------------+----------------+------------+--------------+--------------+----------------+------------+------------+
 |              Oracle ZFSSA              |  NFS,CIFS(K) |       \-       |     \-     |      \-      |      \-      |       \-       |     \-     |     \-     |
 +----------------------------------------+--------------+----------------+------------+--------------+--------------+----------------+------------+------------+
-|              CephFS Native             |      \-      |       \-       |     \-     |   CEPH(M)    |      \-      |       \-       |     \-     |     \-     |
+|              CephFS Native             |      \-      |       \-       |     \-     |  CEPHFS (M)  |      \-      |       \-       |     \-     | CEPHFS (N) |
 +----------------------------------------+--------------+----------------+------------+--------------+--------------+----------------+------------+------------+
 |                 Tegile                 |    NFS (M)   |NFS (M),CIFS (M)|     \-     |      \-      |    NFS (M)   |NFS (M),CIFS (M)|     \-     |     \-     |
 +----------------------------------------+--------------+----------------+------------+--------------+--------------+----------------+------------+------------+

manila/share/drivers/cephfs/cephfs_native.py

@@ -207,10 +207,6 @@ class CephFSNativeDriver(driver.ShareDriver,):
             raise exception.InvalidShareAccess(
                 reason=_("Only 'cephx' access type allowed."))
 
-        if access['access_level'] == constants.ACCESS_LEVEL_RO:
-            raise exception.InvalidShareAccessLevel(
-                level=constants.ACCESS_LEVEL_RO)
-
         ceph_auth_id = access['access_to']
 
         # We need to check here rather than the API or Manila Client to see
@@ -224,8 +220,18 @@ class CephFSNativeDriver(driver.ShareDriver,):
                              ceph_auth_id)
             raise exception.InvalidInput(message=error_message)
 
-        auth_result = self.volume_client.authorize(self._share_path(share),
-                                                   ceph_auth_id)
+        # TODO(rraja): Log the Ceph point release version, once available, in
+        # which the volume client can enable read-only access.
+        if not getattr(self.volume_client, 'version', None):
+            if access['access_level'] == constants.ACCESS_LEVEL_RO:
+                raise exception.InvalidShareAccessLevel(
+                    level=constants.ACCESS_LEVEL_RO)
+            auth_result = self.volume_client.authorize(
+                self._share_path(share), ceph_auth_id)
+        else:
+            readonly = access['access_level'] == constants.ACCESS_LEVEL_RO
+            auth_result = self.volume_client.authorize(
+                self._share_path(share), ceph_auth_id, readonly=readonly)
 
         return auth_result['auth_key']
 

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

@@ -14,6 +14,7 @@
 #    under the License.
 
 
+import ddt
 import mock
 from oslo_utils import units
 
@@ -47,6 +48,7 @@ class MockVolumeClientModule(object):
 
     class CephFSVolumeClient(mock.Mock):
         mock_used_bytes = 0
+        version = 1
 
         def __init__(self, *args, **kwargs):
             mock.Mock.__init__(self, spec=[
@@ -71,6 +73,7 @@ class MockVolumeClientModule(object):
             })
 
 
+@ddt.ddt
 class CephFSNativeDriverTestCase(test.TestCase):
     """Test the CephFS native driver.
 
@@ -160,18 +163,51 @@ class CephFSNativeDriverTestCase(test.TestCase):
             self._driver._share_path(self._share),
             data_isolated=True)
 
-    def test_allow_access(self):
-        access_rule = {
+    @ddt.data(None, 1)
+    def test_allow_access_rw(self, volume_client_version):
+        rule = {
             'access_level': constants.ACCESS_LEVEL_RW,
+            'access_to': 'alice',
             'access_type': 'cephx',
-            'access_to': 'alice'
         }
+        self._driver.volume_client.version = volume_client_version
 
-        self._driver._allow_access(self._context, self._share, access_rule)
+        auth_key = self._driver._allow_access(
+            self._context, self._share, rule)
 
+        self.assertEqual("abc123", auth_key)
+        if not volume_client_version:
             self._driver._volume_client.authorize.assert_called_once_with(
                 self._driver._share_path(self._share),
                 "alice")
+        else:
+            self._driver._volume_client.authorize.assert_called_once_with(
+                self._driver._share_path(self._share),
+                "alice",
+                readonly=False)
+
+    @ddt.data(None, 1)
+    def test_allow_access_ro(self, volume_client_version):
+        rule = {
+            'access_level': constants.ACCESS_LEVEL_RO,
+            'access_to': 'alice',
+            'access_type': 'cephx',
+        }
+        self._driver.volume_client.version = volume_client_version
+
+        if not volume_client_version:
+            self.assertRaises(exception.InvalidShareAccessLevel,
+                              self._driver._allow_access,
+                              self._context, self._share, rule)
+        else:
+            auth_key = self._driver._allow_access(self._context, self._share,
+                                                  rule)
+
+            self.assertEqual("abc123", auth_key)
+            self._driver._volume_client.authorize.assert_called_once_with(
+                self._driver._share_path(self._share),
+                "alice",
+                readonly=True)
 
     def test_allow_access_wrong_type(self):
         self.assertRaises(exception.InvalidShareAccess,
@@ -182,15 +218,6 @@ class CephFSNativeDriverTestCase(test.TestCase):
                               'access_to': 'alice'
                           })
 
-    def test_allow_access_ro(self):
-        self.assertRaises(exception.InvalidShareAccessLevel,
-                          self._driver._allow_access,
-                          self._context, self._share, {
-                              'access_level': constants.ACCESS_LEVEL_RO,
-                              'access_type': 'cephx',
-                              'access_to': 'alice'
-                          })
-
     def test_allow_access_same_cephx_id_as_manila_service(self):
         self.assertRaises(exception.InvalidInput,
                           self._driver._allow_access,
@@ -232,7 +259,8 @@ class CephFSNativeDriverTestCase(test.TestCase):
 
         self._driver._volume_client.authorize.assert_called_once_with(
             self._driver._share_path(self._share),
-            "alice")
+            "alice",
+            readonly=False)
         self._driver._volume_client.deauthorize.assert_called_once_with(
             self._driver._share_path(self._share),
             "bob")
@@ -250,7 +278,8 @@ class CephFSNativeDriverTestCase(test.TestCase):
 
         self._driver._volume_client.authorize.assert_called_once_with(
             self._driver._share_path(self._share),
-            "alice")
+            "alice",
+            readonly=False)
 
     def test_extend_share(self):
         new_size_gb = self._share['size'] * 2

manila_tempest_tests/tests/api/test_rules_negative.py

@@ -19,7 +19,6 @@ from tempest.lib import exceptions as lib_exc
 from tempest import test
 import testtools
 
-from manila_tempest_tests import share_exceptions
 from manila_tempest_tests.tests.api import base
 from manila_tempest_tests import utils
 
@@ -348,16 +347,6 @@ class ShareCephxRulesForCephFSNegativeTest(base.BaseSharesTest):
                           self.share["id"], self.access_type, self.access_to,
                           access_level="su")
 
-    @test.attr(type=[base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND])
-    def test_create_access_rule_cephx_with_unsupported_access_level_ro(self):
-        rule = self.shares_v2_client.create_access_rule(
-            self.share["id"], self.access_type, self.access_to,
-            access_level="ro")
-        self.assertRaises(
-            share_exceptions.AccessRuleBuildErrorException,
-            self.shares_client.wait_for_access_rule_status,
-            self.share['id'], rule['id'], "active")
-
 
 def skip_if_cephx_access_type_not_supported_by_client(self, client):
     if client == 'shares_client':

releasenotes/notes/cephfs-native-add-readonly-shares-support-067ccab0217ab5f5.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - For cephfs_native driver, added read-only shares
+    support.