GPFS: Add update_access()

Add update_access() implementation to replace the deprecated
allow_access() and deny_access().

Change-Id: I206f1284bddc02452087e24061619ca3c04395a4
Implements: blueprint gpfs-update-access

manila/share/drivers/ibm/gpfs.py

@@ -512,15 +512,20 @@ class GPFSShareDriver(driver.ExecuteMixin, driver.GaneshaMixin,
     def ensure_share(self, ctx, share, share_server=None):
         """Ensure that storage are mounted and exported."""
 
-    def allow_access(self, ctx, share, access, share_server=None):
+    def update_access(self, context, share, access_rules, add_rules,
-        """Allow access to the share."""
+                      delete_rules, share_server=None):
+        """Update access rules for given share."""
+        helper = self._get_helper(share)
         location = self._get_share_path(share)
-        self._get_helper(share).allow_access(location, share, access)
 
-    def deny_access(self, ctx, share, access, share_server=None):
+        for access in delete_rules:
-        """Deny access to the share."""
+            helper.deny_access(location, share, access)
-        location = self._get_share_path(share)
+
-        self._get_helper(share).deny_access(location, share, access)
+        for access in add_rules:
+            helper.allow_access(location, share, access)
+
+        if not (add_rules or delete_rules):
+            helper.resync_access(location, share, access_rules)
 
     def check_for_setup_error(self):
         """Returns an error if prerequisites aren't met."""
@@ -787,7 +792,7 @@ class NASHelperBase(object):
         """Construct location of new export."""
         return ':'.join([self.configuration.gpfs_share_export_ip, local_path])
 
-    def get_export_options(self, share, access, helper, options_not_allowed):
+    def get_export_options(self, share, access, helper):
         """Get the export options."""
         extra_specs = share_types.get_extra_specs_from_share(share)
         if helper == 'KNFS':
@@ -797,11 +802,13 @@ class NASHelperBase(object):
         else:
             export_options = None
 
-        if export_options:
+        options = self._get_validated_opt_list(export_options)
-            options = export_options.lower().split(',')
+        options.append(self.get_access_option(access))
-        else:
+        return ','.join(options)
-            options = []
 
+    def _validate_export_options(self, options):
+        """Validate the export options."""
+        options_not_allowed = self._get_options_not_allowed()
         invalid_options = [
             option for option in options if option in options_not_allowed
         ]
@@ -811,32 +818,39 @@ class NASHelperBase(object):
                                                 'it is set by access_type.'
                                                 % invalid_options)
 
-        if access['access_level'] == constants.ACCESS_LEVEL_RO:
+    def _get_validated_opt_list(self, export_options):
-            if helper == 'KNFS':
+        """Validate the export options and return an option list."""
-                options.append(constants.ACCESS_LEVEL_RO)
+        if export_options:
-            elif helper == 'CES':
+            options = export_options.lower().split(',')
-                options.append('access_type=ro')
+            self._validate_export_options(options)
         else:
-            if helper == 'KNFS':
+            options = []
-                options.append(constants.ACCESS_LEVEL_RW)
+        return options
-            elif helper == 'CES':
-                options.append('access_type=rw')
 
-        return ','.join(options)
+    @abc.abstractmethod
+    def get_access_option(self, access):
+        """Get access option string based on access level."""
+
+    @abc.abstractmethod
+    def _get_options_not_allowed(self):
+        """Get access options that are not allowed in extra-specs."""
 
     @abc.abstractmethod
     def remove_export(self, local_path, share):
         """Remove export."""
 
     @abc.abstractmethod
-    def allow_access(self, local_path, share, access_type, access):
+    def allow_access(self, local_path, share, access):
         """Allow access to the host."""
 
     @abc.abstractmethod
-    def deny_access(self, local_path, share, access_type, access,
+    def deny_access(self, local_path, share, access):
-                    force=False):
         """Deny access to the host."""
 
+    @abc.abstractmethod
+    def resync_access(self, local_path, share, access_rules):
+        """Re-sync all access rules for given share."""
+
 
 class KNFSHelper(NASHelperBase):
     """Wrapper for Kernel NFS Commands."""
@@ -921,39 +935,56 @@ class KNFSHelper(NASHelperBase):
     def remove_export(self, local_path, share):
         """Remove export."""
 
-    def allow_access(self, local_path, share, access):
+    def get_access_option(self, access):
+        """Get access option string based on access level."""
+        return access['access_level']
+
+    def _get_options_not_allowed(self):
+        """Get access options that are not allowed in extra-specs."""
+        return list(constants.ACCESS_LEVELS)
+
+    def _get_exports(self):
+        """Get exportfs output."""
+        try:
+            out, __ = self._execute('exportfs', run_as_root=True)
+        except exception.ProcessExecutionError as e:
+            msg = (_('Failed to check exports on the systems. '
+                     ' Error: %s.') % e)
+            LOG.error(msg)
+            raise exception.GPFSException(msg)
+        return out
+
+    def allow_access(self, local_path, share, access, error_on_exists=True):
         """Allow access to one or more vm instances."""
 
         if access['access_type'] != 'ip':
             raise exception.InvalidShareAccess(reason='Only ip access type '
                                                       'supported.')
 
-        out = self._has_client_access(local_path, access['access_to'])
+        if error_on_exists:
+            # check if present in export
+            out = re.search(
+                re.escape(local_path) + '[\s\n]*'
+                + re.escape(access['access_to']), self._get_exports())
 
-        if out:
+            if out is not None:
-            access_type = access['access_type']
+                access_type = access['access_type']
-            access_to = access['access_to']
+                access_to = access['access_to']
-            raise exception.ShareAccessExists(access_type=access_type,
+                raise exception.ShareAccessExists(access_type=access_type,
-                                              access=access_to)
+                                                  access=access_to)
 
-        options_not_allowed = list(constants.ACCESS_LEVELS)
+        export_opts = self.get_export_options(share, access, 'KNFS')
-        export_opts = self.get_export_options(share, access, 'KNFS',
-                                              options_not_allowed)
         cmd = ['exportfs', '-o', export_opts,
                ':'.join([access['access_to'], local_path])]
         try:
             self._publish_access(*cmd)
-        except exception.ProcessExecutionError as e:
+        except exception.ProcessExecutionError:
-            msg = (_('Failed to allow access for share %(sharename)s. '
+            msg = _('Failed to allow access for share %s.') % share['name']
-                     'Error: %(excmsg)s.') %
+            LOG.exception(msg)
-                   {'sharename': share['name'],
-                    'excmsg': e})
-            LOG.error(msg)
             raise exception.GPFSException(msg)
 
-    def deny_access(self, local_path, share, access, force=False):
+    def _deny_ip(self, local_path, share, ip):
         """Remove access for one or more vm instances."""
-        ip = access['access_to']
         cmd = ['exportfs', '-u', ':'.join([ip, local_path])]
         try:
             # Can get exit code 0 for success or 1 for already gone (also
@@ -970,6 +1001,25 @@ class KNFSHelper(NASHelperBase):
         # So, verify that the IP access was completely removed.
         self._verify_denied_access(local_path, share, ip)
 
+    def deny_access(self, local_path, share, access):
+        """Remove access for one or more vm instances."""
+        self._deny_ip(local_path, share, access['access_to'])
+
+    def _remove_other_access(self, local_path, share, access_rules):
+        """Remove any client access that is not in access_rules."""
+        exports = self._get_exports()
+        gpfs_ips = set(NFSHelper.get_host_list(exports, local_path))
+        manila_ips = set([x['access_to'] for x in access_rules])
+        remove_ips = gpfs_ips - manila_ips
+        for ip in remove_ips:
+            self._deny_ip(local_path, share, ip)
+
+    def resync_access(self, local_path, share, access_rules):
+        """Re-sync all access rules for given share."""
+        for access in access_rules:
+            self.allow_access(local_path, share, access, error_on_exists=False)
+        self._remove_other_access(local_path, share, access_rules)
+
 
 class CESHelper(NASHelperBase):
     """Wrapper for NFS by Spectrum Scale CES"""
@@ -988,15 +1038,64 @@ class CESHelper(NASHelperBase):
             raise exception.GPFSException(msg)
         return out
 
+    @staticmethod
+    def _fix_export_data(data, headers):
+        """Export data split by ':' may need fixing if client had colons."""
+
+        # If an IPv6 client shows up then ':' delimiters don't work.
+        # So use header positions to get data before/after Clients.
+        # Then what is left in between can be joined back into a client IP.
+        client_index = headers.index('Clients')
+        # reverse_client_index is distance from end.
+        reverse_client_index = len(headers) - (client_index + 1)
+        after_client_index = len(data) - reverse_client_index
+
+        before_client = data[:client_index]
+        client = data[client_index: after_client_index]
+        after_client = data[after_client_index:]
+
+        result_data = before_client
+        result_data.append(':'.join(client))  # Fixes colons in client IP
+        result_data.extend(after_client)
+        return result_data
+
+    def _get_nfs_client_exports(self, local_path):
+        """Get the current NFS client export details from GPFS."""
+
+        out = self._execute_mmnfs_command(
+            ('list', '-n', local_path, '-Y'),
+            'Failed to get exports from the system.')
+
+        # Remove the header line and use the headers to describe the data
+        lines = out.splitlines()
+        for line in lines:
+            data = line.split(':')
+            if "HEADER" in data:
+                headers = data
+                lines.remove(line)
+                break
+        else:
+            msg = _('Failed to parse exports for path %s. '
+                    'No HEADER found.') % local_path
+            LOG.error(msg)
+            raise exception.GPFSException(msg)
+
+        exports = []
+        for line in lines:
+            data = line.split(':')
+            if len(data) < 3:
+                continue  # Skip empty lines (and anything less than minimal).
+
+            result_data = self._fix_export_data(data, headers)
+            exports.append(dict(zip(headers, result_data)))
+
+        return exports
+
     def _has_client_access(self, local_path, access_to=None):
         """Check path for any export or for one with a specific IP address."""
-        err_msg = 'Failed to check exports on the system.'
+        gpfs_clients = self._get_nfs_client_exports(local_path)
-        out = self._execute_mmnfs_command(('list', '-n', local_path, '-Y'),
+        return gpfs_clients and (access_to is None or access_to in [
-                                          err_msg)
+            x['Clients'] for x in gpfs_clients])
-        if access_to:
-            return ':' + access_to + ':' in out
-        else:
-            return ':' + local_path + ':' in out
 
     def remove_export(self, local_path, share):
         """Remove export."""
@@ -1005,6 +1104,17 @@ class CESHelper(NASHelperBase):
                        % share['name'])
             self._execute_mmnfs_command(('remove', local_path), err_msg)
 
+    def _get_options_not_allowed(self):
+        """Get access options that are not allowed in extra-specs."""
+        return ['access_type=ro', 'access_type=rw']
+
+    def get_access_option(self, access):
+        """Get access option string based on access level."""
+        if access['access_level'] == constants.ACCESS_LEVEL_RO:
+            return 'access_type=ro'
+        else:
+            return 'access_type=rw'
+
     def allow_access(self, local_path, share, access):
         """Allow access to the host."""
 
@@ -1013,9 +1123,7 @@ class CESHelper(NASHelperBase):
                                                       'supported.')
         has_exports = self._has_client_access(local_path)
 
-        options_not_allowed = ['access_type=ro', 'access_type=rw']
+        export_opts = self.get_export_options(share, access, 'CES')
-        export_opts = self.get_export_options(share, access, 'CES',
-                                              options_not_allowed)
 
         if not has_exports:
             cmd = ['add', local_path, '-c',
@@ -1040,3 +1148,80 @@ class CESHelper(NASHelperBase):
             self._execute_mmnfs_command(('change', local_path,
                                          '--nfsremove', access['access_to']),
                                         err_msg)
+
+    def _get_client_opts(self, access, opts_list):
+        """Get client options string for access rule and NFS options."""
+        nfs_opts = ','.join([self.get_access_option(access)] + opts_list)
+
+        return '%(ip)s(%(nfs_opts)s)' % {'ip': access['access_to'],
+                                         'nfs_opts': nfs_opts}
+
+    def _get_share_opts(self, share):
+        """Get a list of NFS options from the share's share type."""
+        extra_specs = share_types.get_extra_specs_from_share(share)
+        opts_list = self._get_validated_opt_list(
+            extra_specs.get('ces:export_options'))
+        return opts_list
+
+    def _nfs_change(self, local_path, share, access_rules, gpfs_clients):
+        """Bulk add/update/remove of access rules for share."""
+        opts_list = self._get_share_opts(share)
+
+        # Create a map of existing client access rules from GPFS.
+        # Key from 'Clients' is an IP address or
+        # Value from 'Access_Type' is RW|RO (case varies)
+        gpfs_map = {
+            x['Clients']: x['Access_Type'].lower() for x in gpfs_clients}
+        gpfs_ips = set(gpfs_map.keys())
+
+        manila_ips = set([x['access_to'] for x in access_rules])
+        add_ips = manila_ips - gpfs_ips
+        update_ips = gpfs_ips.intersection(manila_ips)
+        remove_ips = gpfs_ips - manila_ips
+
+        adds = []
+        updates = []
+        if add_ips or update_ips:
+            for access in access_rules:
+                ip = access['access_to']
+                if ip in add_ips:
+                    adds.append(self._get_client_opts(access, opts_list))
+                elif (ip in update_ips
+                      and access['access_level'] != gpfs_map[ip]):
+                    updates.append(self._get_client_opts(access, opts_list))
+
+        if remove_ips or adds or updates:
+            cmd = ['change', local_path]
+            if remove_ips:
+                cmd.append('--nfsremove')
+                cmd.append(','.join(remove_ips))
+            if adds:
+                cmd.append('--nfsadd')
+                cmd.append(';'.join(adds))
+            if updates:
+                cmd.append('--nfschange')
+                cmd.append(';'.join(updates))
+            err_msg = ('Failed to resync access for share %s.' % share['name'])
+            self._execute_mmnfs_command(cmd, err_msg)
+
+    def _nfs_add(self, access_rules, local_path, share):
+        """Bulk add of access rules to share."""
+        if not access_rules:
+            return
+
+        opts_list = self._get_share_opts(share)
+        client_options = []
+        for access in access_rules:
+            client_options.append(self._get_client_opts(access, opts_list))
+
+        cmd = ['add', local_path, '-c', ';'.join(client_options)]
+        err_msg = ('Failed to resync access for share %s.' % share['name'])
+        self._execute_mmnfs_command(cmd, err_msg)
+
+    def resync_access(self, local_path, share, access_rules):
+        """Re-sync all access rules for given share."""
+        gpfs_clients = self._get_nfs_client_exports(local_path)
+        if not gpfs_clients:
+            self._nfs_add(access_rules, local_path, share)
+        else:
+            self._nfs_change(local_path, share, access_rules, gpfs_clients)

manila/tests/share/drivers/ibm/test_gpfs.py

@@ -447,25 +447,88 @@ mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Cl
                                                            '0:10G')
         self._driver._get_gpfs_device.assert_called_once_with()
 
-    def test_allow_access(self):
+    def test_update_access_allow(self):
+        """Test allow_access functionality via update_access."""
         self._driver._get_share_path = mock.Mock(
             return_value=self.fakesharepath
         )
         self._helper_fake.allow_access = mock.Mock()
-        self._driver.allow_access(self._context, self.share,
+
-                                  self.access, share_server=None)
+        self._driver.update_access(self._context,
+                                   self.share,
+                                   ["ignored"],
+                                   [self.access],
+                                   [],
+                                   share_server=None)
+
         self._helper_fake.allow_access.assert_called_once_with(
             self.fakesharepath, self.share, self.access)
+        self.assertFalse(self._helper_fake.resync_access.called)
         self._driver._get_share_path.assert_called_once_with(self.share)
 
-    def test_deny_access(self):
+    def test_update_access_deny(self):
+        """Test deny_access functionality via update_access."""
         self._driver._get_share_path = mock.Mock(return_value=self.
                                                  fakesharepath)
         self._helper_fake.deny_access = mock.Mock()
-        self._driver.deny_access(self._context, self.share,
+
-                                 self.access, share_server=None)
+        self._driver.update_access(self._context,
+                                   self.share,
+                                   ["ignored"],
+                                   [],
+                                   [self.access],
+                                   share_server=None)
+
         self._helper_fake.deny_access.assert_called_once_with(
             self.fakesharepath, self.share, self.access)
+        self.assertFalse(self._helper_fake.resync_access.called)
+        self._driver._get_share_path.assert_called_once_with(self.share)
+
+    def test_update_access_both(self):
+        """Test update_access with allow and deny lists."""
+        self._driver._get_share_path = mock.Mock(return_value=self.
+                                                 fakesharepath)
+        self._helper_fake.deny_access = mock.Mock()
+        self._helper_fake.allow_access = mock.Mock()
+        self._helper_fake.resync_access = mock.Mock()
+
+        access_1 = fake_share.fake_access(access_to="1.1.1.1")
+        access_2 = fake_share.fake_access(access_to="2.2.2.2")
+        self._driver.update_access(self._context,
+                                   self.share,
+                                   ["ignore"],
+                                   [access_1],
+                                   [access_2],
+                                   share_server=None)
+
+        self.assertFalse(self._helper_fake.resync_access.called)
+        self._helper_fake.allow_access.assert_called_once_with(
+            self.fakesharepath, self.share, access_1)
+        self._helper_fake.deny_access.assert_called_once_with(
+            self.fakesharepath, self.share, access_2)
+        self._driver._get_share_path.assert_called_once_with(self.share)
+
+    def test_update_access_resync(self):
+        """Test recovery mode update_access."""
+        self._driver._get_share_path = mock.Mock(return_value=self.
+                                                 fakesharepath)
+        self._helper_fake.deny_access = mock.Mock()
+        self._helper_fake.allow_access = mock.Mock()
+        self._helper_fake.resync_access = mock.Mock()
+
+        access_1 = fake_share.fake_access(access_to="1.1.1.1")
+        access_2 = fake_share.fake_access(access_to="2.2.2.2")
+        self._driver.update_access(self._context,
+                                   self.share,
+                                   [access_1, access_2],
+                                   [],
+                                   [],
+                                   share_server=None)
+
+        self._helper_fake.resync_access.assert_called_once_with(
+            self.fakesharepath, self.share, [access_1, access_2])
+        self.assertFalse(self._helper_fake.allow_access.called)
+        self.assertFalse(self._helper_fake.allow_access.called)
         self._driver._get_share_path.assert_called_once_with(self.share)
 
     def test__check_gpfs_state_active(self):
@@ -1020,23 +1083,49 @@ mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Cl
         )
         self._driver.configuration.gpfs_share_export_ip = orig_value
 
-    def test_knfs_get_export_options(self):
+    def test_knfs_resync_access(self):
+        self._knfs_helper.allow_access = mock.Mock()
+        path = self.fakesharepath
+        to_remove = '3.3.3.3'
+        fake_exportfs_before = ('%(path)s\n\t\t%(ip)s\n'
+                                '/other/path\n\t\t4.4.4.4\n' %
+                                {'path': path, 'ip': to_remove})
+        fake_exportfs_after = '/other/path\n\t\t4.4.4.4\n'
+        self._knfs_helper._execute = mock.Mock(
+            return_value=(fake_exportfs_before, ''))
+        self._knfs_helper._publish_access = mock.Mock(
+            side_effect=[[(fake_exportfs_before, '')],
+                         [(fake_exportfs_after, '')]])
+
+        access_1 = fake_share.fake_access(access_to="1.1.1.1")
+        access_2 = fake_share.fake_access(access_to="2.2.2.2")
+        self._knfs_helper.resync_access(path, self.share, [access_1, access_2])
+
+        self._knfs_helper.allow_access.assert_has_calls([
+            mock.call(path, self.share, access_1, error_on_exists=False),
+            mock.call(path, self.share, access_2, error_on_exists=False)])
+        self._knfs_helper._execute.assert_called_once_with(
+            'exportfs', run_as_root=True)
+        self._knfs_helper._publish_access.assert_has_calls([
+            mock.call('exportfs', '-u',
+                      '%(ip)s:%(path)s' % {'ip': to_remove, 'path': path},
+                      check_exit_code=[0, 1]),
+            mock.call('exportfs')])
+
+    @ddt.data('rw', 'ro')
+    def test_knfs_get_export_options(self, access_level):
         mock_out = {"knfs:export_options": "no_root_squash"}
         self.mock_object(share_types, 'get_extra_specs_from_share',
                          mock.Mock(return_value=mock_out))
-        access = self.access
+        access = fake_share.fake_access(access_level=access_level)
-        options_not_allowed = ['rw', 'ro']
+        out = self._knfs_helper.get_export_options(self.share, access, 'KNFS')
-        out = self._knfs_helper.get_export_options(self.share, access,
+        self.assertEqual("no_root_squash,%s" % access_level, out)
-                                                   'KNFS', options_not_allowed)
-        self.assertEqual("no_root_squash,rw", out)
 
     def test_knfs_get_export_options_default(self):
         self.mock_object(share_types, 'get_extra_specs_from_share',
                          mock.Mock(return_value={}))
         access = self.access
-        options_not_allowed = ['rw', 'ro']
+        out = self._knfs_helper.get_export_options(self.share, access, 'KNFS')
-        out = self._knfs_helper.get_export_options(self.share, access,
-                                                   'KNFS', options_not_allowed)
         self.assertEqual("rw", out)
 
     def test_knfs_get_export_options_invalid_option_ro(self):
@@ -1044,22 +1133,20 @@ mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Cl
         self.mock_object(share_types, 'get_extra_specs_from_share',
                          mock.Mock(return_value=mock_out))
         access = self.access
-        options_not_allowed = ['rw', 'ro']
         share = fake_share.fake_share(share_type="fake_share_type")
         self.assertRaises(exception.InvalidInput,
                           self._knfs_helper.get_export_options,
-                          share, access, 'KNFS', options_not_allowed)
+                          share, access, 'KNFS')
 
     def test_knfs_get_export_options_invalid_option_rw(self):
         mock_out = {"knfs:export_options": "rw"}
         self.mock_object(share_types, 'get_extra_specs_from_share',
                          mock.Mock(return_value=mock_out))
         access = self.access
-        options_not_allowed = ['rw', 'ro']
         share = fake_share.fake_share(share_type="fake_share_type")
         self.assertRaises(exception.InvalidInput,
                           self._knfs_helper.get_export_options,
-                          share, access, 'KNFS', options_not_allowed)
+                          share, access, 'KNFS')
 
     @ddt.data(("/gpfs0/share-fakeid\t10.0.0.1", None),
               ("", None),
@@ -1094,15 +1181,13 @@ mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Cl
         )
         self._knfs_helper._publish_access = mock.Mock()
         access = self.access
-        options_not_allowed = ['rw', 'ro']
         local_path = self.fakesharepath
         self._knfs_helper.allow_access(local_path, self.share, access)
         self._knfs_helper._execute.assert_called_once_with('exportfs',
                                                            run_as_root=True)
         self.assertTrue(re.search.called)
         self._knfs_helper.get_export_options.assert_any_call(
-            self.share, access, 'KNFS',
+            self.share, access, 'KNFS')
-            options_not_allowed)
         cmd = ['exportfs', '-o', export_opts, ':'.join([access['access_to'],
                                                        local_path])]
         self._knfs_helper._publish_access.assert_called_once_with(*cmd)
@@ -1122,6 +1207,21 @@ mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Cl
         self.assertTrue(re.search.called)
         self.assertFalse(self._knfs_helper.get_export_options.called)
 
+    def test_knfs_allow_access_publish_exception(self):
+        self._knfs_helper.get_export_options = mock.Mock()
+        self._knfs_helper._publish_access = mock.Mock(
+            side_effect=exception.ProcessExecutionError('boom'))
+
+        self.assertRaises(exception.GPFSException,
+                          self._knfs_helper.allow_access,
+                          self.fakesharepath,
+                          self.share,
+                          self.access,
+                          error_on_exists=False)
+
+        self.assertTrue(self._knfs_helper.get_export_options.called)
+        self.assertTrue(self._knfs_helper._publish_access.called)
+
     def test_knfs_allow_access_invalid_access(self):
         access = fake_share.fake_access(access_type='test')
         self.assertRaises(exception.InvalidShareAccess,
@@ -1270,23 +1370,22 @@ mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Cl
                       check_exit_code=True, run_as_root=False),
             mock.call(fake_command, check_exit_code=True, run_as_root=True)])
 
-    def test_ces_get_export_options(self):
+    @ddt.data('rw', 'ro')
+    def test_ces_get_export_options(self, access_level):
         mock_out = {"ces:export_options": "squash=no_root_squash"}
         self.mock_object(share_types, 'get_extra_specs_from_share',
                          mock.Mock(return_value=mock_out))
-        access = self.access
+        access = fake_share.fake_access(access_level=access_level)
-        options_not_allowed = ['access_type=ro', 'access_type=rw']
+        out = self._ces_helper.get_export_options(self.share, access, 'CES')
-        out = self._ces_helper.get_export_options(self.share, access,
+        self.assertEqual("squash=no_root_squash,access_type=%s" % access_level,
-                                                  'CES', options_not_allowed)
+                         out)
-        self.assertEqual("squash=no_root_squash,access_type=rw", out)
 
     def test_ces_get_export_options_default(self):
         self.mock_object(share_types, 'get_extra_specs_from_share',
                          mock.Mock(return_value={}))
         access = self.access
-        options_not_allowed = ['access_type=ro', 'access_type=rw']
         out = self._ces_helper.get_export_options(self.share, access,
-                                                  'CES', options_not_allowed)
+                                                  'CES')
         self.assertEqual("access_type=rw", out)
 
     def test_ces_get_export_options_invalid_option_ro(self):
@@ -1294,22 +1393,47 @@ mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Cl
         self.mock_object(share_types, 'get_extra_specs_from_share',
                          mock.Mock(return_value=mock_out))
         access = self.access
-        options_not_allowed = ['access_type=ro', 'access_type=rw']
         share = fake_share.fake_share(share_type="fake_share_type")
         self.assertRaises(exception.InvalidInput,
                           self._ces_helper.get_export_options,
-                          share, access, 'CES', options_not_allowed)
+                          share, access, 'CES')
 
     def test_ces_get_export_options_invalid_option_rw(self):
         mock_out = {"ces:export_options": "access_type=rw"}
         self.mock_object(share_types, 'get_extra_specs_from_share',
                          mock.Mock(return_value=mock_out))
         access = self.access
-        options_not_allowed = ['access_type=ro', 'access_type=rw']
         share = fake_share.fake_share(share_type="fake_share_type")
         self.assertRaises(exception.InvalidInput,
                           self._ces_helper.get_export_options,
-                          share, access, 'CES', options_not_allowed)
+                          share, access, 'CES')
+
+    def test__get_nfs_client_exports_exception(self):
+        self._ces_helper._execute = mock.Mock(return_value=('junk', ''))
+
+        local_path = self.fakesharepath
+        self.assertRaises(exception.GPFSException,
+                          self._ces_helper._get_nfs_client_exports,
+                          local_path)
+
+        self._ces_helper._execute.assert_called_once_with(
+            'mmnfs', 'export', 'list', '-n', local_path, '-Y')
+
+    @ddt.data('44.3.2.11', '1:2:3:4:5:6:7:8')
+    def test__fix_export_data(self, ip):
+        data = None
+        for line in self.fake_ces_exports.splitlines():
+            if "HEADER" in line:
+                headers = line.split(':')
+            if ip in line:
+                data = line.split(':')
+                break
+        self.assertIsNotNone(
+            data, "Test data did not contain a line with the test IP.")
+
+        result_data = self._ces_helper._fix_export_data(data, headers)
+
+        self.assertEqual(ip, result_data[headers.index('Clients')])
 
     @ddt.data((None, True),
               ('44.3.2.11', True),
@@ -1317,6 +1441,9 @@ mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Cl
               ('4.3.2.1', False),
               ('4.3.2.11', False),
               ('1.2.3.4', False),
+              ('', False),
+              ('*', False),
+              ('.', False),
               ('1:2:3:4:5:6:7:8', True))
     @ddt.unpack
     def test_ces__has_client_access(self, ip, has_access):
@@ -1441,3 +1568,70 @@ mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Cl
         self.assertRaises(exception.GPFSException,
                           self._ces_helper.deny_access, local_path,
                           self.share, access)
+
+    def test_ces_resync_access_add(self):
+        mock_out = self.fake_ces_exports_not_found
+        self._ces_helper._execute = mock.Mock(return_value=(mock_out, ''))
+        self.mock_object(share_types, 'get_extra_specs_from_share',
+                         mock.Mock(return_value={}))
+
+        access_rules = [self.access]
+        local_path = self.fakesharepath
+        self._ces_helper.resync_access(local_path, self.share, access_rules)
+
+        self._ces_helper._execute.assert_has_calls([
+            mock.call('mmnfs', 'export', 'list', '-n', local_path, '-Y'),
+            mock.call('mmnfs', 'export', 'add', local_path, '-c',
+                      self.access['access_to'] + '(' + "access_type=rw" + ')')
+        ])
+        share_types.get_extra_specs_from_share.assert_called_once_with(
+            self.share)
+
+    def test_ces_resync_access_change(self):
+
+        class SortedMatch(object):
+            def __init__(self, f, expected):
+                self.assertEqual = f
+                self.expected = expected
+
+            def __eq__(self, actual):
+                expected_list = self.expected.split(',')
+                actual_list = actual.split(',')
+                self.assertEqual(sorted(expected_list), sorted(actual_list))
+                return True
+
+        mock_out = self.fake_ces_exports
+        self._ces_helper._execute = mock.Mock(
+            return_value=(mock_out, ''))
+        self.mock_object(share_types, 'get_extra_specs_from_share',
+                         mock.Mock(return_value={}))
+
+        access_rules = [fake_share.fake_access(access_to='1.1.1.1'),
+                        fake_share.fake_access(
+                            access_to='10.0.0.1', access_level='ro')]
+        local_path = self.fakesharepath
+        self._ces_helper.resync_access(local_path, self.share, access_rules)
+
+        share_types.get_extra_specs_from_share.assert_called_once_with(
+            self.share)
+        to_remove = '1:2:3:4:5:6:7:8,44.3.2.11'
+        to_add = access_rules[0]['access_to'] + '(' + "access_type=rw" + ')'
+        to_change = access_rules[1]['access_to'] + '(' + "access_type=ro" + ')'
+        self._ces_helper._execute.assert_has_calls([
+            mock.call('mmnfs', 'export', 'list', '-n', local_path, '-Y'),
+            mock.call('mmnfs', 'export', 'change', local_path,
+                      '--nfsremove', SortedMatch(self.assertEqual, to_remove),
+                      '--nfsadd', to_add,
+                      '--nfschange', to_change)
+        ])
+
+    def test_ces_resync_nothing(self):
+        """Test that hits the add-no-rules case."""
+        mock_out = self.fake_ces_exports_not_found
+        self._ces_helper._execute = mock.Mock(return_value=(mock_out, ''))
+
+        local_path = self.fakesharepath
+        self._ces_helper.resync_access(local_path, None, [])
+
+        self._ces_helper._execute.assert_called_once_with(
+            'mmnfs', 'export', 'list', '-n', local_path, '-Y')