ganesha: dynamically update access of share

You can dynamically update access lists of exports with Ganesha
version >= 2.4. Make ganesha library use this feature in a new helper
class, GaneshaNASHelper2, to cleanly implement share access rules
changes without undesired interruptions.

When updating a share's access rules, the new helper class differs
from the older GaneshaNASHelper class as follows:
* Looks for an existing export and edits its client access list;
  creates a new export if it can't find one; and removes an export if
  the access list ends up empty. Rather than awkwardly create or
  remove an export per addition or removal of an access rule.
* Issues DBUS UpdateAccess command to dynamically update an export.

Implements: bp ganesha-dynamic-update-export

Co-Authored-By: Csaba Henk <chenk@redhat.com>

Change-Id: I01ec100c0afe28a84e9afa8e0660d299e4b3d160

doc/source/devref/ganesha.rst

@@ -33,7 +33,18 @@ Supported operations
 Requirements
 ------------
 
-`NFS-Ganesha <https://github.com/nfs-ganesha/nfs-ganesha/wiki>`__ 2.1 or newer.
+- Preferred:
+
+  `NFS-Ganesha <https://github.com/nfs-ganesha/nfs-ganesha/wiki>`_ v2.4 or
+  later, which allows dynamic update of access rules. And use manila's
+  ``ganesha.GaneshaNASHelper2`` class as described later in
+  :ref:`ganesha_using_library`.
+
+- For use with limitations documented in :ref:`ganesha_known_issues`:
+
+  `NFS-Ganesha <https://github.com/nfs-ganesha/nfs-ganesha/wiki>`_ v2.1 to
+  v2.3. And use manila's ``ganesha.GaneshaNASHelper`` class as described later
+  in :ref:`ganesha_using_library`.
 
 NFS-Ganesha configuration
 -------------------------
@@ -82,19 +93,30 @@ These are:
 - `ganesha_export_template_dir` = directory from where Ganesha loads
     export customizations (cf. "Customizing Ganesha exports").
 
+.. _ganesha_using_library:
+
 Using Ganesha Library in drivers
 --------------------------------
 
 A driver that wants to use the Ganesha Library has to inherit
 from ``driver.GaneshaMixin``.
 
-The driver has to contain a subclass of ``ganesha.GaneshaNASHelper``,
+The driver has to contain a subclass of ``ganesha.GaneshaNASHelper2``,
 instantiate it along with the driver instance and delegate
-``allow_access`` and ``deny_access`` methods to it (when appropriate,
-ie. when ``access_proto`` is NFS).
+``update_access`` method to it (when appropriate, i.e., when ``access_proto``
+is NFS).
+
+.. note::
+
+    You can also subclass ``ganesha.GaneshaNASHelper``. It works with
+    NFS-Ganesha v2.1 to v2.3 that doesn't support dynamic update of exports.
+    To update access rules without having to restart NFS-Ganesha server, the
+    class manipulates exports created per share access rule (rather than per
+    share) introducing limitations documented in :ref:`ganesha_known_issues`.
+
 
 In the following we explain what has to be implemented by the
-``ganesha.GaneshaNASHelper`` subclass (to which we refer as "helper
+``ganesha.GaneshaNASHelper2`` subclass (to which we refer as "helper
 class").
 
 Ganesha exports are described by so-called *Ganesha export blocks*
@@ -107,7 +129,7 @@ subblock*. The helper class has to implement the ``_fsal_hook``
 method which returns the FSAL subblock (in Python represented as
 a dict with string keys and values). It has one mandatory key,
 ``Name``, to which the value should be the name of the FSAL
-(eg.: ``{"Name": "GLUSTER"}``). Further content of it is
+(eg.: ``{"Name": "CEPH"}``). Further content of it is
 optional and FSAL specific.
 
 Customizing Ganesha exports
@@ -190,6 +212,9 @@ Known Restrictions
 Known Issues
 ------------
 
+Following issues concern only users of `ganesha.GaneshaNASHelper` class that
+works with NFS-Ganesha v2.1 to v2.3.
+
 - The export location for shares of a driver that uses the Ganesha Library
   will be of the format ``<ganesha-server>:/share-<share-id>``. However,
   this is incomplete information, because it pertains only to NFSv3

etc/manila/rootwrap.d/share.filters

@@ -141,6 +141,9 @@ dbus-addexport: RegExpFilter, dbus-send, root, dbus-send, --print-reply, --syste
 # manila/share/drivers/ganesha/manager.py:
 dbus-removeexport: RegExpFilter, dbus-send, root, dbus-send, --print-reply, --system, --dest=org\.ganesha\.nfsd, /org/ganesha/nfsd/ExportMgr, org\.ganesha\.nfsd\.exportmgr\.(Add|Remove)Export, .*
 
+# manila/share/drivers/ganesha/manager.py:
+dbus-updateexport: RegExpFilter, dbus-send, root, dbus-send, --print-reply, --system, --dest=org\.ganesha\.nfsd, /org/ganesha/nfsd/ExportMgr, org\.ganesha\.nfsd\.exportmgr\.UpdateExport, .*, .*
+
 # manila/share/drivers/ganesha/manager.py:
 rmconf: RegExpFilter, sh, root, sh, -c, rm -f /.*/\*\.conf$
 

manila/share/drivers/ganesha/__init__.py

@@ -50,13 +50,13 @@ class NASHelperBase(object):
         """Initializes protocol-specific NAS drivers."""
 
     @abc.abstractmethod
-    def update_access(self, base_path, share, add_rules, delete_rules,
-                      recovery=False):
+    def update_access(self, context, share, access_rules, add_rules,
+                      delete_rules, share_server=None):
         """Update access rules of share."""
 
 
 class GaneshaNASHelper(NASHelperBase):
-    """Execute commands relating to Shares."""
+    """Perform share access changes using Ganesha version < 2.4."""
 
     supported_access_types = ('ip', )
     supported_access_levels = (constants.ACCESS_LEVEL_RW, )
@@ -146,15 +146,96 @@ class GaneshaNASHelper(NASHelperBase):
         """Deny access to the share."""
         self.ganesha.remove_export("%s--%s" % (share['name'], access['id']))
 
-    def update_access(self, base_path, share, add_rules, delete_rules,
-                      recovery=False):
+    def update_access(self, context, share, access_rules, add_rules,
+                      delete_rules, share_server=None):
         """Update access rules of share."""
-
-        if recovery:
+        if not (add_rules or delete_rules):
+            add_rules = access_rules
             self.ganesha.reset_exports()
             self.ganesha.restart_service()
 
         for rule in add_rules:
-            self._allow_access(base_path, share, rule)
+            self._allow_access('/', share, rule)
         for rule in delete_rules:
-            self._deny_access(base_path, share, rule)
+            self._deny_access('/', share, rule)
+
+
+class GaneshaNASHelper2(GaneshaNASHelper):
+    """Perform share access changes using Ganesha version >= 2.4."""
+
+    def _get_export_path(self, share):
+        """Subclass this to return export path."""
+        raise NotImplementedError()
+
+    def _get_export_pseudo_path(self, share):
+        """Subclass this to return export pseudo path."""
+        raise NotImplementedError()
+
+    def update_access(self, context, share, access_rules, add_rules,
+                      delete_rules, share_server=None):
+        """Update access rules of share.
+
+        Creates an export per share. Modifies access rules of shares by
+        dynamically updating exports via DBUS.
+        """
+
+        confdict = {}
+        existing_access_rules = []
+
+        if self.ganesha._check_export_file_exists(share['name']):
+            confdict = self.ganesha._read_export_file(share['name'])
+            existing_access_rules = confdict["EXPORT"]["CLIENT"]
+            if not isinstance(existing_access_rules, list):
+                existing_access_rules = [existing_access_rules]
+        else:
+            if not access_rules:
+                LOG.warning("Trying to remove export file '%s' but it's "
+                            "already gone",
+                            self.ganesha._getpath(share['name']))
+                return
+
+        wanted_rw_clients, wanted_ro_clients = [], []
+        for rule in access_rules:
+            if rule['access_level'] == 'rw':
+                wanted_rw_clients.append(rule['access_to'])
+            elif rule['access_level'] == 'ro':
+                wanted_ro_clients.append(rule['access_to'])
+
+        if access_rules:
+            # Add or Update export.
+            clients = []
+            if wanted_ro_clients:
+                clients.append({
+                    'Access_Type': 'ro',
+                    'Clients': ','.join(wanted_ro_clients)
+                })
+            if wanted_rw_clients:
+                clients.append({
+                    'Access_Type': 'rw',
+                    'Clients': ','.join(wanted_rw_clients)
+                })
+
+            if existing_access_rules:
+                # Update existing export.
+                ganesha_utils.patch(confdict, {
+                    'EXPORT': {
+                        'CLIENT': clients
+                    }
+                })
+                self.ganesha.update_export(share['name'], confdict)
+            else:
+                # Add new export.
+                ganesha_utils.patch(confdict, self.export_template, {
+                    'EXPORT': {
+                        'Export_Id': self.ganesha.get_export_id(),
+                        'Path': self._get_export_path(share),
+                        'Pseudo': self._get_export_pseudo_path(share),
+                        'Tag': share['name'],
+                        'CLIENT': clients,
+                        'FSAL': self._fsal_hook(None, share, None)
+                    }
+                })
+                self.ganesha.add_export(share['name'], confdict)
+        else:
+            # No clients have access to the share. Remove export.
+            self.ganesha.remove_export(share['name'])

manila/share/drivers/ganesha/manager.py

@@ -130,12 +130,19 @@ def _dump_to_conf(confdict, out=sys.stdout, indent=0):
         for k, v in confdict.items():
             if v is None:
                 continue
-            out.write(' ' * (indent * IWIDTH) + k + ' ')
             if isinstance(v, dict):
+                out.write(' ' * (indent * IWIDTH) + k + ' ')
                 out.write("{\n")
                 _dump_to_conf(v, out, indent + 1)
                 out.write(' ' * (indent * IWIDTH) + '}')
+            elif isinstance(v, list):
+                for item in v:
+                    out.write(' ' * (indent * IWIDTH) + k + ' ')
+                    out.write("{\n")
+                    _dump_to_conf(item, out, indent + 1)
+                    out.write(' ' * (indent * IWIDTH) + '}\n')
             else:
+                out.write(' ' * (indent * IWIDTH) + k + ' ')
                 out.write('= ')
                 _dump_to_conf(v, out, indent)
                 out.write(';')
@@ -152,14 +159,39 @@ def parseconf(conf):
     """Parse Ganesha config.
 
     Both native format and JSON are supported.
+
+    Convert config to a (nested) dictionary.
     """
+    def list_to_dict(l):
+        # Convert a list of key-value pairs stored as tuples to a dict.
+        # For tuples with identical keys, preserve all the values in a
+        # list. e.g., argument [('k', 'v1'), ('k', 'v2')] to function
+        # returns {'k': ['v1', 'v2']}.
+        d = {}
+        for i in l:
+            if isinstance(i, tuple):
+                k, v = i
+                if isinstance(v, list):
+                    v = list_to_dict(v)
+                if k in d:
+                    d[k] = [d[k]]
+                    d[k].append(v)
+                else:
+                    d[k] = v
+        return d
 
     try:
         # allow config to be specified in JSON --
         # for sake of people who might feel Ganesha config foreign.
         d = jsonutils.loads(conf)
     except ValueError:
-        d = jsonutils.loads(_conf2json(conf))
+        # Customize JSON decoder to convert Ganesha config to a list
+        # of key-value pairs stored as tuples. This allows multiple
+        # occurrences of a config block to be later converted to a
+        # dict key-value pair, with block name being the key and a
+        # list of block contents being the value.
+        l = jsonutils.loads(_conf2json(conf), object_pairs_hook=lambda x: x)
+        d = list_to_dict(l)
     return d
 
 
@@ -251,6 +283,18 @@ class GaneshaManager(object):
         return parseconf(self.execute("cat", self._getpath(name),
                                       message='reading export ' + name)[0])
 
+    def _check_export_file_exists(self, name):
+        """Check whether export exists."""
+        try:
+            self.execute('test', '-f', self._getpath(name), makelog=False,
+                         run_as_root=False)
+            return True
+        except exception.GaneshaCommandFailure as e:
+            if e.exit_code == 1:
+                return False
+            else:
+                raise
+
     def _write_export_file(self, name, confdict):
         """Write confdict to the export file of name."""
         for k, v in ganesha_utils.walk(confdict):
@@ -300,6 +344,20 @@ class GaneshaManager(object):
                 self._mkindex()
             raise
 
+    def update_export(self, name, confdict):
+        """Update an export to Ganesha specified by confdict."""
+        xid = confdict["EXPORT"]["Export_Id"]
+        old_confdict = self._read_export_file(name)
+
+        path = self._write_export_file(name, confdict)
+        try:
+            self._dbus_send_ganesha("UpdateExport", "string:" + path,
+                                    "string:EXPORT(Export_Id=%d)" % xid)
+        except Exception:
+            # Revert the export file update.
+            self._write_export_file(name, old_confdict)
+            raise
+
     def remove_export(self, name):
         """Remove an export from Ganesha."""
         try:

manila/tests/share/drivers/ganesha/test_manager.py

@@ -15,6 +15,7 @@
 
 import re
 
+import ddt
 import mock
 from oslo_serialization import jsonutils
 import six
@@ -32,18 +33,27 @@ test_ganesha_cnf = """EXPORT {
     Export_Id = 101;
     CLIENT {
         Clients = ip1;
+        Access_Level = ro;
+    }
+    CLIENT {
+        Clients = ip2;
+        Access_Level = rw;
     }
 }"""
 test_dict_unicode = {
     u'EXPORT': {
         u'Export_Id': 101,
-        u'CLIENT': {u'Clients': u"ip1"}
+        u'CLIENT': [
+            {u'Clients': u"ip1", u'Access_Level': u'ro'},
+            {u'Clients': u"ip2", u'Access_Level': u'rw'}]
     }
 }
 test_dict_str = {
     'EXPORT': {
         'Export_Id': 101,
-        'CLIENT': {'Clients': "ip1"}
+        'CLIENT': [
+            {'Clients': 'ip1', 'Access_Level': 'ro'},
+            {'Clients': 'ip2', 'Access_Level': 'rw'}]
     }
 }
 
@@ -61,6 +71,11 @@ class GaneshaConfigTests(test.TestCase):
     ref_ganesha_cnf = """EXPORT {
     CLIENT {
         Clients = ip1;
+        Access_Level = ro;
+    }
+    CLIENT {
+        Clients = ip2;
+        Access_Level = rw;
     }
     Export_Id = 101;
 }"""
@@ -103,8 +118,14 @@ class GaneshaConfigTests(test.TestCase):
         Clients = ip1;
     }
 }"""
+        result_dict_unicode = {
+            u'EXPORT': {
+                u'CLIENT': {u'Clients': u'ip1'},
+                u'Export_Id': 101
+            }
+        }
         ret = manager._conf2json(test_ganesha_cnf_with_comment)
-        self.assertEqual(test_dict_unicode, jsonutils.loads(ret))
+        self.assertEqual(result_dict_unicode, jsonutils.loads(ret))
 
     def test_parseconf_ganesha_cnf_input(self):
         ret = manager.parseconf(test_ganesha_cnf)
@@ -126,6 +147,7 @@ class GaneshaConfigTests(test.TestCase):
                                            ganesha_cnf))
 
 
+@ddt.ddt
 class GaneshaManagerTestCase(test.TestCase):
     """Tests GaneshaManager."""
 
@@ -283,6 +305,41 @@ class GaneshaManagerTestCase(test.TestCase):
         manager.parseconf.assert_called_once_with(test_ganesha_cnf)
         self.assertEqual(test_dict_unicode, ret)
 
+    def test_check_export_file_exists(self):
+        self.mock_object(self._manager, '_getpath',
+                         mock.Mock(return_value=test_path))
+        self.mock_object(self._manager, 'execute',
+                         mock.Mock(return_value=(test_ganesha_cnf,)))
+
+        ret = self._manager._check_export_file_exists(test_name)
+
+        self._manager._getpath.assert_called_once_with(test_name)
+        self._manager.execute.assert_called_once_with(
+            'test', '-f', test_path, makelog=False, run_as_root=False)
+        self.assertTrue(ret)
+
+    @ddt.data(1, 4)
+    def test_check_export_file_exists_error(self, exit_code):
+        self.mock_object(self._manager, '_getpath',
+                         mock.Mock(return_value=test_path))
+        self.mock_object(
+            self._manager, 'execute',
+            mock.Mock(side_effect=exception.GaneshaCommandFailure(
+                exit_code=exit_code))
+        )
+
+        if exit_code == 1:
+            ret = self._manager._check_export_file_exists(test_name)
+            self.assertFalse(ret)
+        else:
+            self.assertRaises(exception.GaneshaCommandFailure,
+                              self._manager._check_export_file_exists,
+                              test_name)
+
+        self._manager._getpath.assert_called_once_with(test_name)
+        self._manager.execute.assert_called_once_with(
+            'test', '-f', test_path, makelog=False, run_as_root=False)
+
     def test_write_export_file(self):
         self.mock_object(manager, 'mkconf',
                          mock.Mock(return_value=test_ganesha_cnf))
@@ -416,6 +473,54 @@ class GaneshaManagerTestCase(test.TestCase):
         self._manager._mkindex.assert_called_once_with()
         self.assertFalse(self._manager._remove_export_dbus.called)
 
+    def test_update_export(self):
+        confdict = {
+            'EXPORT': {
+                'Export_Id': 101,
+                'CLIENT': {'Clients': 'ip1', 'Access_Level': 'ro'},
+            }
+        }
+        self.mock_object(self._manager, '_read_export_file',
+                         mock.Mock(return_value=test_dict_unicode))
+        self.mock_object(self._manager, '_write_export_file',
+                         mock.Mock(return_value=test_path))
+        self.mock_object(self._manager, '_dbus_send_ganesha')
+
+        self._manager.update_export(test_name, confdict)
+
+        self._manager._read_export_file.assert_called_once_with(test_name)
+        self._manager._write_export_file.assert_called_once_with(test_name,
+                                                                 confdict)
+        self._manager._dbus_send_ganesha.assert_called_once_with(
+            'UpdateExport', 'string:' + test_path,
+            'string:EXPORT(Export_Id=101)')
+
+    def test_update_export_error(self):
+        confdict = {
+            'EXPORT': {
+                'Export_Id': 101,
+                'CLIENT': {'Clients': 'ip1', 'Access_Level': 'ro'},
+            }
+        }
+        self.mock_object(self._manager, '_read_export_file',
+                         mock.Mock(return_value=test_dict_unicode))
+        self.mock_object(self._manager, '_write_export_file',
+                         mock.Mock(return_value=test_path))
+        self.mock_object(
+            self._manager, '_dbus_send_ganesha',
+            mock.Mock(side_effect=exception.GaneshaCommandFailure))
+
+        self.assertRaises(exception.GaneshaCommandFailure,
+                          self._manager.update_export, test_name, confdict)
+
+        self._manager._read_export_file.assert_called_once_with(test_name)
+        self._manager._write_export_file.assert_has_calls([
+            mock.call(test_name, confdict),
+            mock.call(test_name, test_dict_unicode)])
+        self._manager._dbus_send_ganesha.assert_called_once_with(
+            'UpdateExport', 'string:' + test_path,
+            'string:EXPORT(Export_Id=101)')
+
     def test_remove_export(self):
         self.mock_object(self._manager, '_read_export_file',
                          mock.Mock(return_value=test_dict_unicode))

manila/tests/share/drivers/test_ganesha.py

@@ -21,6 +21,7 @@ import ddt
 import mock
 from oslo_config import cfg
 
+from manila import context
 from manila import exception
 from manila.share import configuration as config
 from manila.share.drivers import ganesha
@@ -61,6 +62,7 @@ class GaneshaNASHelperTestCase(test.TestCase):
         CONF.set_default('ganesha_export_template_dir',
                          '/fakedir2/faketempl.d')
         CONF.set_default('ganesha_service_name', 'ganesha.fakeservice')
+        self._context = context.get_admin_context()
         self._execute = mock.Mock(return_value=('', ''))
         self.fake_conf = config.Configuration(None)
         self.fake_conf_dir_path = '/fakedir0/exports.d'
@@ -256,17 +258,16 @@ class GaneshaNASHelperTestCase(test.TestCase):
             'fakename--fakeaccid')
         self.assertIsNone(ret)
 
-    @ddt.data({}, {'recovery': False})
-    def test_update_access_for_allow(self, kwargs):
+    def test_update_access_for_allow(self):
         self.mock_object(self._helper, '_allow_access')
         self.mock_object(self._helper, '_deny_access')
 
         self._helper.update_access(
-            '/some/path', 'aShare', add_rules=["example.com"], delete_rules=[],
-            **kwargs)
+            self._context, self.share, access_rules=[self.access],
+            add_rules=[self.access], delete_rules=[])
 
         self._helper._allow_access.assert_called_once_with(
-            '/some/path', 'aShare', 'example.com')
+            '/', self.share, self.access)
 
         self.assertFalse(self._helper._deny_access.called)
         self.assertFalse(self._helper.ganesha.reset_exports.called)
@@ -277,10 +278,11 @@ class GaneshaNASHelperTestCase(test.TestCase):
         self.mock_object(self._helper, '_deny_access')
 
         self._helper.update_access(
-            '/some/path', 'aShare', [], delete_rules=["example.com"])
+            self._context, self.share, access_rules=[],
+            add_rules=[], delete_rules=[self.access])
 
         self._helper._deny_access.assert_called_once_with(
-            '/some/path', 'aShare', 'example.com')
+            '/', self.share, self.access)
 
         self.assertFalse(self._helper._allow_access.called)
         self.assertFalse(self._helper.ganesha.reset_exports.called)
@@ -291,12 +293,143 @@ class GaneshaNASHelperTestCase(test.TestCase):
         self.mock_object(self._helper, '_deny_access')
 
         self._helper.update_access(
-            '/some/path', 'aShare', add_rules=["example.com"], delete_rules=[],
-            recovery=True)
+            self._context, self.share, access_rules=[self.access],
+            add_rules=[], delete_rules=[])
 
         self._helper._allow_access.assert_called_once_with(
-            '/some/path', 'aShare', 'example.com')
+            '/', self.share, self.access)
 
         self.assertFalse(self._helper._deny_access.called)
         self.assertTrue(self._helper.ganesha.reset_exports.called)
         self.assertTrue(self._helper.ganesha.restart_service.called)
+
+
+@ddt.ddt
+class GaneshaNASHelper2TestCase(test.TestCase):
+    """Tests GaneshaNASHelper2."""
+
+    def setUp(self):
+        super(GaneshaNASHelper2TestCase, self).setUp()
+
+        CONF.set_default('ganesha_config_path', '/fakedir0/fakeconfig')
+        CONF.set_default('ganesha_db_path', '/fakedir1/fake.db')
+        CONF.set_default('ganesha_export_dir', '/fakedir0/export.d')
+        CONF.set_default('ganesha_export_template_dir',
+                         '/fakedir2/faketempl.d')
+        CONF.set_default('ganesha_service_name', 'ganesha.fakeservice')
+        self._context = context.get_admin_context()
+        self._execute = mock.Mock(return_value=('', ''))
+        self.fake_conf = config.Configuration(None)
+        self.fake_conf_dir_path = '/fakedir0/exports.d'
+        self._helper = ganesha.GaneshaNASHelper2(
+            self._execute, self.fake_conf, tag='faketag')
+        self._helper.ganesha = mock.Mock()
+        self._helper.export_template = {}
+        self.share = fake_share.fake_share()
+        self.rule1 = fake_share.fake_access(access_level='ro')
+        self.rule2 = fake_share.fake_access(access_level='rw',
+                                            access_to='10.0.0.2')
+
+    def test_update_access_add_export(self):
+        mock_gh = self._helper.ganesha
+        self.mock_object(mock_gh, '_check_export_file_exists',
+                         mock.Mock(return_value=False))
+        self.mock_object(mock_gh, 'get_export_id',
+                         mock.Mock(return_value=100))
+        self.mock_object(self._helper, '_get_export_path',
+                         mock.Mock(return_value='/fakepath'))
+        self.mock_object(self._helper, '_get_export_pseudo_path',
+                         mock.Mock(return_value='/fakepath'))
+        self.mock_object(self._helper, '_fsal_hook',
+                         mock.Mock(return_value={'Name': 'fake'}))
+        result_confdict = {
+            'EXPORT': {
+                'Export_Id': 100,
+                'Path': '/fakepath',
+                'Pseudo': '/fakepath',
+                'Tag': 'fakename',
+                'CLIENT': [{
+                    'Access_Type': 'ro',
+                    'Clients': '10.0.0.1'}],
+                'FSAL': {'Name': 'fake'}
+            }
+        }
+
+        self._helper.update_access(
+            self._context, self.share, access_rules=[self.rule1],
+            add_rules=[], delete_rules=[])
+
+        mock_gh._check_export_file_exists.assert_called_once_with('fakename')
+        mock_gh.get_export_id.assert_called_once_with()
+        self._helper._get_export_path.assert_called_once_with(self.share)
+        (self._helper._get_export_pseudo_path.assert_called_once_with(
+            self.share))
+        self._helper._fsal_hook.assert_called_once_with(
+            None, self.share, None)
+        mock_gh.add_export.assert_called_once_with(
+            'fakename', result_confdict)
+        self.assertFalse(mock_gh.update_export.called)
+        self.assertFalse(mock_gh.remove_export.called)
+
+    @ddt.data({'Access_Type': 'ro', 'Clients': '10.0.0.1'},
+              [{'Access_Type': 'ro', 'Clients': '10.0.0.1'}])
+    def test_update_access_update_export(self, client):
+        mock_gh = self._helper.ganesha
+        self.mock_object(mock_gh, '_check_export_file_exists',
+                         mock.Mock(return_value=True))
+        self.mock_object(
+            mock_gh, '_read_export_file',
+            mock.Mock(return_value={'EXPORT': {'CLIENT': client}})
+        )
+        result_confdict = {
+            'EXPORT': {
+                'CLIENT': [
+                    {'Access_Type': 'ro', 'Clients': '10.0.0.1'},
+                    {'Access_Type': 'rw', 'Clients': '10.0.0.2'}]
+            }
+        }
+
+        self._helper.update_access(
+            self._context, self.share, access_rules=[self.rule1, self.rule2],
+            add_rules=[self.rule2], delete_rules=[])
+
+        mock_gh._check_export_file_exists.assert_called_once_with('fakename')
+        mock_gh.update_export.assert_called_once_with('fakename',
+                                                      result_confdict)
+        self.assertFalse(mock_gh.add_export.called)
+        self.assertFalse(mock_gh.remove_export.called)
+
+    def test_update_access_remove_export(self):
+        mock_gh = self._helper.ganesha
+        self.mock_object(mock_gh, '_check_export_file_exists',
+                         mock.Mock(return_value=True))
+        client = {'Access_Type': 'ro', 'Clients': '10.0.0.1'}
+        self.mock_object(
+            mock_gh, '_read_export_file',
+            mock.Mock(return_value={'EXPORT': {'CLIENT': client}})
+        )
+
+        self._helper.update_access(
+            self._context, self.share, access_rules=[],
+            add_rules=[], delete_rules=[self.rule1])
+
+        mock_gh._check_export_file_exists.assert_called_once_with('fakename')
+        mock_gh.remove_export.assert_called_once_with('fakename')
+        self.assertFalse(mock_gh.add_export.called)
+        self.assertFalse(mock_gh.update_export.called)
+
+    def test_update_access_export_file_already_removed(self):
+        mock_gh = self._helper.ganesha
+        self.mock_object(mock_gh, '_check_export_file_exists',
+                         mock.Mock(return_value=False))
+        self.mock_object(ganesha.LOG, 'warning')
+
+        self._helper.update_access(
+            self._context, self.share, access_rules=[],
+            add_rules=[], delete_rules=[self.rule1])
+
+        mock_gh._check_export_file_exists.assert_called_once_with('fakename')
+        ganesha.LOG.warning.assert_called_once_with(mock.ANY, mock.ANY)
+        self.assertFalse(mock_gh.add_export.called)
+        self.assertFalse(mock_gh.update_export.called)
+        self.assertFalse(mock_gh.remove_export.called)

releasenotes/notes/ganesha-dynamic-update-access-be80bd1cb785e733.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - The new class `ganesha.GaneshaNASHelper2` in the ganesha library uses
+    dynamic update of export feature of NFS-Ganesha versions v2.4 or newer
+    to modify access rules of a share in a clean way. It modifies exports
+    created per share rather than per share access rule (as with
+    `ganesha.GaneshaNASHelper`) that introduced limitations and unintuitive
+    end user experience.