diff --git a/doc/source/devref/hpe_3par_driver.rst b/doc/source/devref/hpe_3par_driver.rst index 1a3e530272..f92404b2fe 100644 --- a/doc/source/devref/hpe_3par_driver.rst +++ b/doc/source/devref/hpe_3par_driver.rst @@ -36,21 +36,15 @@ The following operations are supported with HPE 3PAR File Persona: - Allow/deny NFS share access * IP access rules are required for NFS share access - * Shares created from snapshots are always read-only - * Shares not created from snapshots are read-write (and subject to ACLs) - Allow/deny CIFS share access * CIFS shares require user access rules. * User access requires a 3PAR local user (LDAP and AD is not yet supported) - * Shares created from snapshots are always read-only - * Shares not created from snapshots are read-write (and subject to ACLs) - Create/delete snapshots - Create shares from snapshots - * Shares created from snapshots are always read-only - Share networks are not supported. Shares are created directly on the 3PAR without the use of a share server or service VM. Network connectivity is setup outside of manila. diff --git a/manila/share/drivers/hpe/hpe_3par_driver.py b/manila/share/drivers/hpe/hpe_3par_driver.py index a5c5806cd4..6392f8bc08 100644 --- a/manila/share/drivers/hpe/hpe_3par_driver.py +++ b/manila/share/drivers/hpe/hpe_3par_driver.py @@ -269,28 +269,6 @@ class HPE3ParShareDriver(driver.ShareDriver): return share_server['backend_details'].get('ip') if share_server else ( self.share_ip_address) - @staticmethod - def _build_export_location(protocol, ip, path): - - if not ip: - message = _('Failed to build export location due to missing IP.') - raise exception.InvalidInput(message) - - if not path: - message = _('Failed to build export location due to missing path.') - raise exception.InvalidInput(message) - - if protocol == 'NFS': - location = ':'.join((ip, path)) - elif protocol == 'CIFS': - location = '\\\\%s\%s' % (ip, path) - else: - message = _('Invalid protocol. Expected NFS or CIFS. ' - 'Got %s.') % protocol - raise exception.InvalidInput(message) - - return location - @staticmethod def build_share_comment(share): """Create an informational only comment to help admins and testers.""" @@ -325,7 +303,7 @@ class HPE3ParShareDriver(driver.ShareDriver): comment=self.build_share_comment(share) ) - return self._build_export_location(protocol, ip, path) + return self._hpe3par.build_export_location(protocol, ip, path) def create_share_from_snapshot(self, context, share, snapshot, share_server=None): @@ -345,10 +323,11 @@ class HPE3ParShareDriver(driver.ShareDriver): snapshot['id'], self.fpg, self.vfs, + size=share['size'], comment=self.build_share_comment(share) ) - return self._build_export_location(protocol, ip, path) + return self._hpe3par.build_export_location(protocol, ip, path) def delete_share(self, context, share, share_server=None): """Deletes share and its fstore.""" diff --git a/manila/share/drivers/hpe/hpe_3par_mediator.py b/manila/share/drivers/hpe/hpe_3par_mediator.py index 3c86cf0e7c..41fb1f3975 100644 --- a/manila/share/drivers/hpe/hpe_3par_mediator.py +++ b/manila/share/drivers/hpe/hpe_3par_mediator.py @@ -23,9 +23,10 @@ from oslo_utils import importutils from oslo_utils import units import six +from manila.data import utils as data_utils from manila import exception from manila import utils -from manila.i18n import _, _LI, _LW +from manila.i18n import _, _LI, _LW, _LE hpe3parclient = importutils.try_import("hpe3parclient") if hpe3parclient: @@ -56,6 +57,7 @@ DOES_NOT_EXIST = 'does not exist, cannot' LOCAL_IP = '127.0.0.1' LOCAL_IP_RO = '127.0.0.2' SUPER_SHARE = 'OPENSTACK_SUPER_SHARE' +TMP_RO_SNAP_EXPORT = "Temp RO snapshot export as source for creating RW share." class HPE3ParMediator(object): @@ -73,10 +75,11 @@ class HPE3ParMediator(object): 2.0.4 - Remove file tree on delete when using nested shares #1538800 2.0.5 - Reduce the fsquota by share size when a share is deleted #1582931 + 2.0.6 - Read-write share from snapshot (using driver mount and copy) """ - VERSION = "2.0.5" + VERSION = "2.0.6" def __init__(self, **kwargs): @@ -199,6 +202,25 @@ class HPE3ParMediator(object): 'err': six.text_type(e)}) # don't raise exception on logout() + @staticmethod + def build_export_location(protocol, ip, path): + + if not ip: + message = _('Failed to build export location due to missing IP.') + raise exception.InvalidInput(reason=message) + + if not path: + message = _('Failed to build export location due to missing path.') + raise exception.InvalidInput(reason=message) + + share_proto = HPE3ParMediator.ensure_supported_protocol(protocol) + if share_proto == 'nfs': + location = ':'.join((ip, path)) + else: + location = r'\\%s\%s' % (ip, path) + + return location + def get_provisioned_gb(self, fpg): total_mb = 0 try: @@ -359,7 +381,8 @@ class HPE3ParMediator(object): return ','.join(options) def _build_createfshare_kwargs(self, protocol, fpg, fstore, readonly, - sharedir, extra_specs, comment): + sharedir, extra_specs, comment, + client_ip=None): createfshare_kwargs = dict(fpg=fpg, fstore=fstore, sharedir=sharedir, @@ -371,21 +394,27 @@ class HPE3ParMediator(object): LOG.warning(msg) if protocol == 'nfs': - # New NFS shares needs seed IP to prevent "all" access. - # Readonly and readwrite NFS shares client IPs cannot overlap. - if readonly: - createfshare_kwargs['clientip'] = LOCAL_IP_RO + if client_ip: + createfshare_kwargs['clientip'] = client_ip else: - createfshare_kwargs['clientip'] = LOCAL_IP + # New NFS shares needs seed IP to prevent "all" access. + # Readonly and readwrite NFS shares client IPs cannot overlap. + if readonly: + createfshare_kwargs['clientip'] = LOCAL_IP_RO + else: + createfshare_kwargs['clientip'] = LOCAL_IP options = self._get_nfs_options(extra_specs, readonly) createfshare_kwargs['options'] = options else: # To keep the original (Kilo, Liberty) behavior where CIFS IP # access rules were required in addition to user rules enable - # this to use a local seed IP instead of the default (all allowed). + # this to use a seed IP instead of the default (all allowed). if self.hpe3par_require_cifs_ip: - createfshare_kwargs['allowip'] = LOCAL_IP + if client_ip: + createfshare_kwargs['allowip'] = client_ip + else: + createfshare_kwargs['allowip'] = LOCAL_IP smb_opts = (ACCESS_BASED_ENUM, CONTINUOUS_AVAIL, CACHE) @@ -455,7 +484,8 @@ class HPE3ParMediator(object): raise exception.ShareBackendException(msg=msg) def _create_share(self, project_id, share_id, protocol, extra_specs, - fpg, vfs, fstore, sharedir, readonly, size, comment): + fpg, vfs, fstore, sharedir, readonly, size, comment, + client_ip=None): share_name = self.ensure_prefix(share_id, readonly=readonly) if not (sharedir or self.hpe3par_fstore_per_share): @@ -471,13 +501,15 @@ class HPE3ParMediator(object): else: fstore = self.ensure_prefix(project_id, protocol) - createfshare_kwargs = self._build_createfshare_kwargs(protocol, - fpg, - fstore, - readonly, - sharedir, - extra_specs, - comment) + createfshare_kwargs = self._build_createfshare_kwargs( + protocol, + fpg, + fstore, + readonly, + sharedir, + extra_specs, + comment, + client_ip=client_ip) if not use_existing_fstore: @@ -538,7 +570,8 @@ class HPE3ParMediator(object): def create_share(self, project_id, share_id, share_proto, extra_specs, fpg, vfs, fstore=None, sharedir=None, readonly=False, size=None, - comment=OPEN_STACK_MANILA): + comment=OPEN_STACK_MANILA, + client_ip=None): """Create the share and return its path. This method can create a share when called by the driver or when @@ -556,6 +589,8 @@ class HPE3ParMediator(object): :param sharedir: (optional) Share directory. :param readonly: (optional) Create share as read-only. :param size: (optional) Size limit for file store if creating one. + :param comment: (optional) Comment to set on the share. + :param client_ip: (optional) IP address to give access to. :return: share path string """ @@ -570,7 +605,8 @@ class HPE3ParMediator(object): sharedir, readonly, size, - comment) + comment, + client_ip=client_ip) if protocol == 'nfs': return share['sharePath'] @@ -580,6 +616,7 @@ class HPE3ParMediator(object): def create_share_from_snapshot(self, share_id, share_proto, extra_specs, orig_project_id, orig_share_id, snapshot_id, fpg, vfs, + size=None, comment=OPEN_STACK_MANILA): protocol = self.ensure_supported_protocol(share_proto) @@ -612,7 +649,28 @@ class HPE3ParMediator(object): sharedir = '.snapshot/%s/%s' % (snapshot['snapName'], orig_share_name) - return self.create_share( + if protocol == "smb" and (not self.hpe3par_cifs_admin_access_username + or not self.hpe3par_cifs_admin_access_password): + LOG.warning(_LW("hpe3par_cifs_admin_access_username and " + "hpe3par_cifs_admin_access_password must be " + "provided in order for CIFS shares created from " + "snapshots to be writable.")) + return self.create_share( + orig_project_id, + share_id, + protocol, + extra_specs, + fpg, + vfs, + fstore=fstore, + sharedir=sharedir, + readonly=True, + comment=comment, + ) + + # Export the snapshot as read-only to copy from. + temp = ' '.join((comment, TMP_RO_SNAP_EXPORT)) + source_path = self.create_share( orig_project_id, share_id, protocol, @@ -622,9 +680,120 @@ class HPE3ParMediator(object): fstore=fstore, sharedir=sharedir, readonly=True, - comment=comment, + comment=temp, + client_ip=self.my_ip ) + try: + share_name = self.ensure_prefix(share_id) + dest_path = self.create_share( + orig_project_id, + share_id, + protocol, + extra_specs, + fpg, + vfs, + fstore=fstore, + readonly=False, + size=size, + comment=comment, + client_ip=','.join((self.my_ip, LOCAL_IP)) + ) + + try: + if protocol == 'smb': + self._grant_admin_smb_access( + protocol, fpg, vfs, fstore, comment, share=share_name) + + ro_share_name = self.ensure_prefix(share_id, readonly=True) + self._grant_admin_smb_access( + protocol, fpg, vfs, fstore, temp, share=ro_share_name) + + ip = self.hpe3par_share_ip_address + source_location = self.build_export_location( + protocol, ip, source_path) + dest_location = self.build_export_location( + protocol, ip, dest_path) + + self._copy_share_data( + share_id, source_location, dest_location, protocol) + + # Revoke the admin access that was needed to copy to the dest. + if protocol == 'nfs': + self._change_access(DENY, + orig_project_id, + share_id, + protocol, + 'ip', + self.my_ip, + 'rw', + fpg, + vfs) + else: + self._revoke_admin_smb_access( + protocol, fpg, vfs, fstore, comment) + + except Exception as e: + msg = _LE('Exception during mount and copy from RO snapshot ' + 'to RW share: %s') + LOG.error(msg, e) + self._delete_share(share_name, protocol, fpg, vfs, fstore) + raise + + finally: + self._delete_ro_share( + orig_project_id, share_id, protocol, fpg, vfs, fstore) + + return dest_path + + def _copy_share_data(self, dest_id, source_location, dest_location, + protocol): + + mount_location = "%s%s" % (self.hpe3par_share_mount_path, dest_id) + source_share_dir = '/'.join((mount_location, "source_snap")) + dest_share_dir = '/'.join((mount_location, "dest_share")) + + dirs_to_remove = [] + dirs_to_unmount = [] + try: + utils.execute('mkdir', '-p', source_share_dir, run_as_root=True) + dirs_to_remove.append(source_share_dir) + self._mount_share(protocol, source_location, source_share_dir) + dirs_to_unmount.append(source_share_dir) + + utils.execute('mkdir', dest_share_dir, run_as_root=True) + dirs_to_remove.append(dest_share_dir) + self._mount_share(protocol, dest_location, dest_share_dir) + dirs_to_unmount.append(dest_share_dir) + + self._copy_data(source_share_dir, dest_share_dir) + finally: + for d in dirs_to_unmount: + self._unmount_share(d) + + if dirs_to_remove: + dirs_to_remove.append(mount_location) + utils.execute('rmdir', *dirs_to_remove, run_as_root=True) + + def _copy_data(self, source_share_dir, dest_share_dir): + + err_msg = None + err_data = None + try: + copy = data_utils.Copy(source_share_dir, dest_share_dir, '') + copy.run() + progress = copy.get_progress()['total_progress'] + if progress != 100: + err_msg = _("Failed to copy data, reason: " + "Total progress %d != 100.") + err_data = progress + except Exception as err: + err_msg = _("Failed to copy data, reason: %s.") + err_data = six.text_type(err) + + if err_msg: + raise exception.ShareBackendException(msg=err_msg % err_data) + def _delete_share(self, share_name, protocol, fpg, vfs, fstore): try: self._client.removefshare( @@ -636,6 +805,20 @@ class HPE3ParMediator(object): LOG.exception(msg) raise exception.ShareBackendException(msg=msg) + def _delete_ro_share(self, project_id, share_id, protocol, + fpg, vfs, fstore): + share_name_ro = self.ensure_prefix(share_id, readonly=True) + if not fstore: + fstore = self._find_fstore(project_id, + share_name_ro, + protocol, + fpg, + vfs, + allow_cross_protocol=True) + if fstore: + self._delete_share(share_name_ro, protocol, fpg, vfs, fstore) + return fstore + def delete_share(self, project_id, share_id, share_size, share_proto, fpg, vfs): @@ -653,46 +836,39 @@ class HPE3ParMediator(object): self._delete_share(share_name, protocol, fpg, vfs, fstore) removed_writable = True - share_name_ro = self.ensure_prefix(share_id, readonly=True) - if not fstore: - fstore = self._find_fstore(project_id, - share_name_ro, - protocol, - fpg, - vfs, - allow_cross_protocol=True) - if fstore: - self._delete_share(share_name_ro, protocol, fpg, vfs, fstore) + # Try to delete the read-only twin share, too. + fstore = self._delete_ro_share( + project_id, share_id, protocol, fpg, vfs, fstore) - if fstore == share_name: - try: - self._client.removefstore(vfs, fstore, fpg=fpg) - except Exception as e: - msg = (_('Failed to remove fstore %(fstore)s: %(e)s') % - {'fstore': fstore, 'e': six.text_type(e)}) - LOG.exception(msg) - raise exception.ShareBackendException(msg=msg) + if fstore == share_name: + try: + self._client.removefstore(vfs, fstore, fpg=fpg) + except Exception as e: + msg = (_('Failed to remove fstore %(fstore)s: %(e)s') % + {'fstore': fstore, 'e': six.text_type(e)}) + LOG.exception(msg) + raise exception.ShareBackendException(msg=msg) - elif removed_writable: - try: - # Attempt to remove file tree on delete when using nested - # shares. If the file tree cannot be removed for whatever - # reason, we will not treat this as an error_deleting - # issue. We will allow the delete to continue as requested. - self._delete_file_tree( - share_name, protocol, fpg, vfs, fstore) - # reduce the fsquota by share size when a tree is deleted. - self._update_capacity_quotas( - fstore, 0, share_size, fpg, vfs) - except Exception as e: - msg = _LW('Exception during cleanup of deleted ' - 'share %(share)s in filestore %(fstore)s: %(e)s') - data = { - 'fstore': fstore, - 'share': share_name, - 'e': six.text_type(e), - } - LOG.warning(msg, data) + elif removed_writable: + try: + # Attempt to remove file tree on delete when using nested + # shares. If the file tree cannot be removed for whatever + # reason, we will not treat this as an error_deleting + # issue. We will allow the delete to continue as requested. + self._delete_file_tree( + share_name, protocol, fpg, vfs, fstore) + # reduce the fsquota by share size when a tree is deleted. + self._update_capacity_quotas( + fstore, 0, share_size, fpg, vfs) + except Exception as e: + msg = _LW('Exception during cleanup of deleted ' + 'share %(share)s in filestore %(fstore)s: %(e)s') + data = { + 'fstore': fstore, + 'share': share_name, + 'e': six.text_type(e), + } + LOG.warning(msg, data) def _delete_file_tree(self, share_name, protocol, fpg, vfs, fstore): # If the share protocol is CIFS, we need to make sure the admin @@ -722,11 +898,43 @@ class HPE3ParMediator(object): self._delete_share_directory(share_dir) # Unmount the super share. - self._unmount_super_share(mount_location) + self._unmount_share(mount_location) # Delete the mount directory. self._delete_share_directory(mount_location) + def _grant_admin_smb_access(self, protocol, fpg, vfs, fstore, comment, + share=SUPER_SHARE): + user = '+%s:fullcontrol' % self.hpe3par_cifs_admin_access_username + setfshare_kwargs = { + 'fpg': fpg, + 'fstore': fstore, + 'comment': comment, + 'allowperm': user, + } + try: + self._client.setfshare( + protocol, vfs, share, **setfshare_kwargs) + except Exception as err: + raise exception.ShareBackendException( + msg=_("There was an error adding permissions: %s") % err) + + def _revoke_admin_smb_access(self, protocol, fpg, vfs, fstore, comment, + share=SUPER_SHARE): + user = '-%s:fullcontrol' % self.hpe3par_cifs_admin_access_username + setfshare_kwargs = { + 'fpg': fpg, + 'fstore': fstore, + 'comment': comment, + 'allowperm': user, + } + try: + self._client.setfshare( + protocol, vfs, share, **setfshare_kwargs) + except Exception as err: + raise exception.ShareBackendException( + msg=_("There was an error revoking permissions: %s") % err) + def _create_super_share(self, protocol, fpg, vfs, fstore, readonly=False): sharedir = '' extra_specs = {} @@ -761,20 +969,7 @@ class HPE3ParMediator(object): # If the share is CIFS, we need to grant access to the specified admin. if protocol == 'smb': - user = '+%s:fullcontrol' % self.hpe3par_cifs_admin_access_username - setfshare_kwargs = { - 'fpg': fpg, - 'fstore': fstore, - 'comment': comment, - 'allowperm': user, - } - try: - result = self._client.setfshare( - protocol, vfs, SUPER_SHARE, **setfshare_kwargs) - except Exception as err: - message = (_("There was an error adding permissions: " - "%s.") % six.text_type(err)) - raise exception.ShareMountException(reason=message) + self._grant_admin_smb_access(protocol, fpg, vfs, fstore, comment) def _create_mount_directory(self, mount_location): try: @@ -785,34 +980,42 @@ class HPE3ParMediator(object): six.text_type(err)) LOG.warning(message) - def _mount_super_share(self, protocol, mount_location, fpg, vfs, fstore): + def _mount_share(self, protocol, export_location, mount_dir): + if protocol == 'nfs': + cmd = ('mount', '-t', 'nfs', export_location, mount_dir) + utils.execute(*cmd, run_as_root=True) + else: + export_location = export_location.replace('\\', '/') + cred = ('username=' + self.hpe3par_cifs_admin_access_username + + ',password=' + + self.hpe3par_cifs_admin_access_password + + ',domain=' + self.hpe3par_cifs_admin_access_domain) + cmd = ('mount', '-t', 'cifs', export_location, mount_dir, + '-o', cred) + utils.execute(*cmd, run_as_root=True) + + def _mount_super_share(self, protocol, mount_dir, fpg, vfs, fstore): try: - mount_path = self._generate_mount_path(protocol, fpg, vfs, fstore) - if protocol == 'nfs': - utils.execute('mount', '-t', 'nfs', mount_path, mount_location, - run_as_root=True) - LOG.debug("Execute mount. mount_location: %s", mount_location) - else: - user = ('username=' + self.hpe3par_cifs_admin_access_username + - ',password=' + - self.hpe3par_cifs_admin_access_password + - ',domain=' + self.hpe3par_cifs_admin_access_domain) - utils.execute('mount', '-t', 'cifs', mount_path, - mount_location, '-o', user, run_as_root=True) + mount_location = self._generate_mount_path( + protocol, fpg, vfs, fstore) + self._mount_share(protocol, mount_location, mount_dir) except Exception as err: message = (_LW("There was an error mounting the super share: " "%s. The nested file tree will not be deleted."), six.text_type(err)) LOG.warning(message) - def _unmount_super_share(self, mount_location): + def _unmount_share(self, mount_location): try: utils.execute('umount', mount_location, run_as_root=True) except Exception as err: - message = (_LW("There was an error unmounting the super share: " - "%s. The nested file tree will not be deleted."), - six.text_type(err)) - LOG.warning(message) + message = _LW("There was an error unmounting the share at " + "%(mount_location)s: %(error)s") + msg_data = { + 'mount_location': mount_location, + 'error': six.text_type(err), + } + LOG.warning(message, msg_data) def _delete_share_directory(self, directory): try: diff --git a/manila/tests/share/drivers/hpe/test_hpe_3par_constants.py b/manila/tests/share/drivers/hpe/test_hpe_3par_constants.py index 500dfbdd32..7de051c6d1 100644 --- a/manila/tests/share/drivers/hpe/test_hpe_3par_constants.py +++ b/manila/tests/share/drivers/hpe/test_hpe_3par_constants.py @@ -18,6 +18,8 @@ NFS_LOWER = 'nfs' IP = 'ip' USER = 'user' USERNAME = 'USERNAME_0' +ADD_USERNAME = '+USERNAME_0:fullcontrol' +DROP_USERNAME = '-USERNAME_0:fullcontrol' PASSWORD = 'PASSWORD_0' READ_WRITE = 'rw' READ_ONLY = 'ro' @@ -32,6 +34,7 @@ CIDR_PREFIX = '24' # Constants to use with Mock and expect in results EXPECTED_IP_10203040 = '10.20.30.40' EXPECTED_IP_1234 = '1.2.3.4' +EXPECTED_MY_IP = '9.8.7.6' EXPECTED_IP_127 = '127.0.0.1' EXPECTED_IP_127_2 = '127.0.0.2' EXPECTED_ACCESS_LEVEL = 'foo_access' diff --git a/manila/tests/share/drivers/hpe/test_hpe_3par_driver.py b/manila/tests/share/drivers/hpe/test_hpe_3par_driver.py index 969000a3ba..5d78c24e3b 100644 --- a/manila/tests/share/drivers/hpe/test_hpe_3par_driver.py +++ b/manila/tests/share/drivers/hpe/test_hpe_3par_driver.py @@ -72,6 +72,11 @@ class HPE3ParDriverTestCase(test.TestCase): self.mock_object(hpe3parmediator, 'HPE3ParMediator') self.mock_mediator_constructor = hpe3parmediator.HPE3ParMediator self.mock_mediator = self.mock_mediator_constructor() + # restore needed static methods + self.mock_mediator.ensure_supported_protocol = ( + self.real_hpe_3par_mediator.ensure_supported_protocol) + self.mock_mediator.build_export_location = ( + self.real_hpe_3par_mediator.build_export_location) self.driver = hpe3pardriver.HPE3ParShareDriver( configuration=self.conf) @@ -293,6 +298,8 @@ class HPE3ParDriverTestCase(test.TestCase): self.mock_mediator.create_share.return_value = ( constants.EXPECTED_SHARE_NAME) + hpe3parmediator.HPE3ParMediator = self.real_hpe_3par_mediator + location = self.do_create_share(constants.CIFS, constants.SHARE_TYPE_ID, constants.EXPECTED_PROJECT_ID, @@ -319,6 +326,7 @@ class HPE3ParDriverTestCase(test.TestCase): self.mock_mediator.create_share.return_value = ( constants.EXPECTED_SHARE_PATH) + hpe3parmediator.HPE3ParMediator = self.real_hpe_3par_mediator location = self.do_create_share(constants.NFS, constants.SHARE_TYPE_ID, @@ -347,6 +355,7 @@ class HPE3ParDriverTestCase(test.TestCase): self.mock_mediator.create_share_from_snapshot.return_value = ( constants.EXPECTED_SHARE_NAME) + hpe3parmediator.HPE3ParMediator = self.real_hpe_3par_mediator snapshot_instance = constants.SNAPSHOT_INSTANCE.copy() snapshot_instance['protocol'] = constants.CIFS @@ -369,7 +378,8 @@ class HPE3ParDriverTestCase(test.TestCase): constants.EXPECTED_SNAP_ID, constants.EXPECTED_FPG, constants.EXPECTED_VFS, - comment=mock.ANY), + comment=mock.ANY, + size=constants.EXPECTED_SIZE_2), ] self.mock_mediator.assert_has_calls(expected_calls) @@ -381,6 +391,7 @@ class HPE3ParDriverTestCase(test.TestCase): self.mock_mediator.create_share_from_snapshot.return_value = ( constants.EXPECTED_SHARE_PATH) + hpe3parmediator.HPE3ParMediator = self.real_hpe_3par_mediator location = self.do_create_share_from_snapshot( constants.NFS, @@ -400,7 +411,8 @@ class HPE3ParDriverTestCase(test.TestCase): constants.EXPECTED_SNAP_ID, constants.EXPECTED_FPG, constants.EXPECTED_VFS, - comment=mock.ANY), + comment=mock.ANY, + size=constants.EXPECTED_SIZE_1), ] self.mock_mediator.assert_has_calls(expected_calls) @@ -700,27 +712,6 @@ class HPE3ParDriverTestCase(test.TestCase): def test_get_network_allocations_number(self): self.assertEqual(1, self.driver.get_network_allocations_number()) - def test_build_export_location_bad_protocol(self): - self.assertRaises(exception.InvalidInput, - self.driver._build_export_location, - "BOGUS", - constants.EXPECTED_IP_1234, - constants.EXPECTED_SHARE_PATH) - - def test_build_export_location_bad_ip(self): - self.assertRaises(exception.InvalidInput, - self.driver._build_export_location, - constants.NFS, - None, - None) - - def test_build_export_location_bad_path(self): - self.assertRaises(exception.InvalidInput, - self.driver._build_export_location, - constants.NFS, - constants.EXPECTED_IP_1234, - None) - def test_setup_server(self): """Setup server by creating a new FSIP.""" diff --git a/manila/tests/share/drivers/hpe/test_hpe_3par_mediator.py b/manila/tests/share/drivers/hpe/test_hpe_3par_mediator.py index facedca59e..1e1645565c 100644 --- a/manila/tests/share/drivers/hpe/test_hpe_3par_mediator.py +++ b/manila/tests/share/drivers/hpe/test_hpe_3par_mediator.py @@ -19,6 +19,7 @@ import mock if 'hpe3parclient' not in sys.modules: sys.modules['hpe3parclient'] = mock.Mock() +from manila.data import utils as data_utils from manila import exception from manila.share.drivers.hpe import hpe_3par_mediator as hpe3parmediator from manila import test @@ -38,6 +39,21 @@ class HPE3ParMediatorTestCase(test.TestCase): def setUp(self): super(HPE3ParMediatorTestCase, self).setUp() + # Fake utils.execute + self.mock_object(utils, 'execute', mock.Mock(return_value={})) + + # Fake data_utils.Copy + class FakeCopy(object): + + def run(self): + pass + + def get_progress(self): + return {'total_progress': 100} + + self.mock_copy = self.mock_object( + data_utils, 'Copy', mock.Mock(return_value=FakeCopy())) + # This is the fake client to use. self.mock_client = mock.Mock() @@ -68,7 +84,8 @@ class HPE3ParMediatorTestCase(test.TestCase): hpe3par_cifs_admin_access_password=constants.PASSWORD, hpe3par_cifs_admin_access_domain=constants.EXPECTED_CIFS_DOMAIN, hpe3par_share_mount_path=constants.EXPECTED_MOUNT_PATH, - ssh_conn_timeout=constants.TIMEOUT) + ssh_conn_timeout=constants.TIMEOUT, + my_ip=constants.EXPECTED_MY_IP) def test_mediator_no_client(self): """Test missing hpe3parclient error.""" @@ -493,15 +510,22 @@ class HPE3ParMediatorTestCase(test.TestCase): self.assertEqual(constants.EXPECTED_SHARE_ID, location) - expected_kwargs = { + expected_kwargs_ro = { 'comment': mock.ANY, 'fpg': constants.EXPECTED_FPG, 'fstore': constants.EXPECTED_FSTORE, - 'sharedir': '.snapshot/%s/%s' % (constants.EXPECTED_SNAP_ID, - constants.EXPECTED_SHARE_ID), } + expected_kwargs_rw = expected_kwargs_ro.copy() + + expected_kwargs_ro['sharedir'] = '.snapshot/%s/%s' % ( + constants.EXPECTED_SNAP_ID, constants.EXPECTED_SHARE_ID) + expected_kwargs_rw['sharedir'] = constants.EXPECTED_SHARE_ID + if require_cifs_ip: - expected_kwargs['allowip'] = constants.EXPECTED_IP_127 + expected_kwargs_ro['allowip'] = constants.EXPECTED_MY_IP + expected_kwargs_rw['allowip'] = ( + ','.join((constants.EXPECTED_MY_IP, + constants.EXPECTED_IP_127))) expected_calls = [ mock.call.getfsnap('*_%s' % constants.EXPECTED_SNAP_ID, @@ -512,15 +536,94 @@ class HPE3ParMediatorTestCase(test.TestCase): mock.call.createfshare(constants.SMB_LOWER, constants.EXPECTED_VFS, constants.EXPECTED_SHARE_ID, - **expected_kwargs), + **expected_kwargs_ro), mock.call.getfshare(constants.SMB_LOWER, constants.EXPECTED_SHARE_ID, fpg=constants.EXPECTED_FPG, vfs=constants.EXPECTED_VFS, - fstore=constants.EXPECTED_FSTORE)] + fstore=constants.EXPECTED_FSTORE), + mock.call.createfshare(constants.SMB_LOWER, + constants.EXPECTED_VFS, + constants.EXPECTED_SHARE_ID, + **expected_kwargs_rw), + mock.call.getfshare(constants.SMB_LOWER, + constants.EXPECTED_SHARE_ID, + fpg=constants.EXPECTED_FPG, + vfs=constants.EXPECTED_VFS, + fstore=constants.EXPECTED_FSTORE), + mock.call.setfshare(constants.SMB_LOWER, + constants.EXPECTED_VFS, + constants.EXPECTED_SHARE_ID, + allowperm=constants.ADD_USERNAME, + comment=mock.ANY, + fpg=constants.EXPECTED_FPG, + fstore=constants.EXPECTED_FSTORE), + mock.call.setfshare(constants.SMB_LOWER, + constants.EXPECTED_VFS, + constants.EXPECTED_SHARE_ID, + allowperm=constants.ADD_USERNAME, + comment=mock.ANY, + fpg=constants.EXPECTED_FPG, + fstore=constants.EXPECTED_FSTORE), + mock.call.setfshare(constants.SMB_LOWER, + constants.EXPECTED_VFS, + constants.EXPECTED_SUPER_SHARE, + allowperm=constants.DROP_USERNAME, + comment=mock.ANY, + fpg=constants.EXPECTED_FPG, + fstore=constants.EXPECTED_FSTORE), + mock.call.removefshare(constants.SMB_LOWER, + constants.EXPECTED_VFS, + constants.EXPECTED_SHARE_ID, + fpg=constants.EXPECTED_FPG, + fstore=constants.EXPECTED_FSTORE), + ] self.mock_client.assert_has_calls(expected_calls) + def test_mediator_create_cifs_share_from_snapshot_ro(self): + self.init_mediator() + + # RO because CIFS admin access username is not configured + self.mediator.hpe3par_cifs_admin_access_username = None + + self.mock_client.getfsnap.return_value = { + 'message': None, + 'total': 1, + 'members': [{'snapName': constants.EXPECTED_SNAP_ID, + 'fstoreName': constants.EXPECTED_FSTORE}] + } + + location = self.mediator.create_share_from_snapshot( + constants.EXPECTED_SHARE_ID, + constants.CIFS, + constants.EXPECTED_EXTRA_SPECS, + constants.EXPECTED_PROJECT_ID, + constants.EXPECTED_SHARE_ID, + constants.EXPECTED_SNAP_ID, + constants.EXPECTED_FPG, + constants.EXPECTED_VFS, + comment=constants.EXPECTED_COMMENT) + + self.assertEqual(constants.EXPECTED_SHARE_ID, location) + + share_dir = '.snapshot/%s/%s' % ( + constants.EXPECTED_SNAP_ID, constants.EXPECTED_SHARE_ID) + + expected_kwargs_ro = { + 'comment': constants.EXPECTED_COMMENT, + 'fpg': constants.EXPECTED_FPG, + 'fstore': constants.EXPECTED_FSTORE, + 'sharedir': share_dir, + } + + self.mock_client.createfshare.assert_called_once_with( + constants.SMB_LOWER, + constants.EXPECTED_VFS, + constants.EXPECTED_SHARE_ID, + **expected_kwargs_ro + ) + def test_mediator_create_nfs_share_from_snapshot(self): self.init_mediator() @@ -558,16 +661,106 @@ class HPE3ParMediatorTestCase(test.TestCase): (constants.EXPECTED_SNAP_ID, constants.EXPECTED_SHARE_ID), fstore=constants.EXPECTED_FSTORE, - clientip=constants.EXPECTED_IP_127_2, + clientip=constants.EXPECTED_MY_IP, options='ro,no_root_squash,insecure'), mock.call.getfshare(constants.NFS_LOWER, constants.EXPECTED_SHARE_ID, fpg=constants.EXPECTED_FPG, vfs=constants.EXPECTED_VFS, - fstore=constants.EXPECTED_FSTORE)] + fstore=constants.EXPECTED_FSTORE), + mock.call.createfshare(constants.NFS_LOWER, + constants.EXPECTED_VFS, + constants.EXPECTED_SHARE_ID, + comment=mock.ANY, + fpg=constants.EXPECTED_FPG, + sharedir=constants.EXPECTED_SHARE_ID, + fstore=constants.EXPECTED_FSTORE, + clientip=','.join(( + constants.EXPECTED_MY_IP, + constants.EXPECTED_IP_127)), + options='rw,no_root_squash,insecure'), + mock.call.getfshare(constants.NFS_LOWER, + constants.EXPECTED_SHARE_ID, + fpg=constants.EXPECTED_FPG, + vfs=constants.EXPECTED_VFS, + fstore=constants.EXPECTED_FSTORE), + mock.call.getfshare(constants.NFS_LOWER, + constants.EXPECTED_SHARE_ID, + fpg=constants.EXPECTED_FPG, + vfs=constants.EXPECTED_VFS, + fstore=constants.EXPECTED_FSTORE), + mock.call.setfshare(constants.NFS_LOWER, + constants.EXPECTED_VFS, + constants.EXPECTED_SHARE_ID, + clientip=''.join(('-', + constants.EXPECTED_MY_IP)), + comment=mock.ANY, + fpg=constants.EXPECTED_FPG, + fstore=constants.EXPECTED_FSTORE), + mock.call.removefshare(constants.NFS_LOWER, + constants.EXPECTED_VFS, + constants.EXPECTED_SHARE_ID, + fpg=constants.EXPECTED_FPG, + fstore=constants.EXPECTED_FSTORE), + ] self.mock_client.assert_has_calls(expected_calls) + def test_mediator_create_share_from_snap_copy_incomplete(self): + self.init_mediator() + + self.mock_client.getfsnap.return_value = { + 'message': None, + 'total': 1, + 'members': [{'snapName': constants.EXPECTED_SNAP_ID, + 'fstoreName': constants.EXPECTED_FSTORE}] + } + + mock_bad_copy = mock.Mock() + mock_bad_copy.get_progress.return_value = {'total_progress': 99} + self.mock_object( + data_utils, 'Copy', mock.Mock(return_value=mock_bad_copy)) + + self.assertRaises(exception.ShareBackendException, + self.mediator.create_share_from_snapshot, + constants.EXPECTED_SHARE_ID, + constants.NFS, + constants.EXPECTED_EXTRA_SPECS, + constants.EXPECTED_PROJECT_ID, + constants.EXPECTED_SHARE_ID, + constants.EXPECTED_SNAP_ID, + constants.EXPECTED_FPG, + constants.EXPECTED_VFS) + self.assertTrue(mock_bad_copy.run.called) + self.assertTrue(mock_bad_copy.get_progress.called) + + def test_mediator_create_share_from_snap_copy_exception(self): + self.init_mediator() + + self.mock_client.getfsnap.return_value = { + 'message': None, + 'total': 1, + 'members': [{'snapName': constants.EXPECTED_SNAP_ID, + 'fstoreName': constants.EXPECTED_FSTORE}] + } + + mock_bad_copy = mock.Mock() + mock_bad_copy.run.side_effect = Exception('run exception') + self.mock_object( + data_utils, 'Copy', mock.Mock(return_value=mock_bad_copy)) + + self.assertRaises(exception.ShareBackendException, + self.mediator.create_share_from_snapshot, + constants.EXPECTED_SHARE_ID, + constants.NFS, + constants.EXPECTED_EXTRA_SPECS, + constants.EXPECTED_PROJECT_ID, + constants.EXPECTED_SHARE_ID, + constants.EXPECTED_SNAP_ID, + constants.EXPECTED_FPG, + constants.EXPECTED_VFS) + self.assertTrue(mock_bad_copy.run.called) + def test_mediator_create_share_from_snap_not_found(self): self.init_mediator() @@ -793,7 +986,7 @@ class HPE3ParMediatorTestCase(test.TestCase): '_delete_share_directory', mock.Mock(return_value={})) self.mock_object(self.mediator, - '_unmount_super_share', + '_unmount_share', mock.Mock(return_value={})) self.mock_object(self.mediator, '_update_capacity_quotas', @@ -815,7 +1008,7 @@ class HPE3ParMediatorTestCase(test.TestCase): mock.call.createfshare(constants.SMB_LOWER, constants.EXPECTED_VFS, constants.EXPECTED_SUPER_SHARE, - allowip=None, + allowip=constants.EXPECTED_MY_IP, comment=( constants.EXPECTED_SUPER_SHARE_COMMENT), fpg=constants.EXPECTED_FPG, @@ -849,7 +1042,7 @@ class HPE3ParMediatorTestCase(test.TestCase): mock.call(expected_share_path), mock.call(expected_mount_path), ]) - self.mediator._unmount_super_share.assert_called_once_with( + self.mediator._unmount_share.assert_called_once_with( expected_mount_path) self.mediator._update_capacity_quotas.assert_called_once_with( constants.EXPECTED_FSTORE, @@ -910,7 +1103,7 @@ class HPE3ParMediatorTestCase(test.TestCase): '_delete_share_directory', mock.Mock(return_value={})) self.mock_object(self.mediator, - '_unmount_super_share', + '_unmount_share', mock.Mock(return_value={})) self.mediator.delete_share(constants.EXPECTED_PROJECT_ID, @@ -929,7 +1122,7 @@ class HPE3ParMediatorTestCase(test.TestCase): mock.call.createfshare(constants.SMB_LOWER, constants.EXPECTED_VFS, constants.EXPECTED_SUPER_SHARE, - allowip=None, + allowip=constants.EXPECTED_MY_IP, comment=( constants.EXPECTED_SUPER_SHARE_COMMENT), fpg=constants.EXPECTED_FPG, @@ -963,7 +1156,7 @@ class HPE3ParMediatorTestCase(test.TestCase): constants.EXPECTED_VFS, constants.EXPECTED_FSTORE) self.mediator._delete_share_directory.assert_called_with( expected_mount_path) - self.mediator._unmount_super_share.assert_called_with( + self.mediator._unmount_share.assert_called_with( expected_mount_path) def test_mediator_create_snapshot(self): @@ -2505,8 +2698,6 @@ class HPE3ParMediatorTestCase(test.TestCase): def test__create_mount_directory(self): self.init_mediator() - self.mock_object(utils, 'execute', mock.Mock(return_value={})) - mount_location = '/mnt/foo' self.mediator._create_mount_directory(mount_location) @@ -2531,8 +2722,6 @@ class HPE3ParMediatorTestCase(test.TestCase): def test__mount_super_share(self): self.init_mediator() - self.mock_object(utils, 'execute', mock.Mock(return_value={})) - # Test mounting NFS share. protocol = 'nfs' mount_location = '/mnt/foo' @@ -2582,8 +2771,6 @@ class HPE3ParMediatorTestCase(test.TestCase): def test__delete_share_directory(self): self.init_mediator() - self.mock_object(utils, 'execute', mock.Mock(return_value={})) - mount_location = '/mnt/foo' self.mediator._delete_share_directory(mount_location) @@ -2603,26 +2790,23 @@ class HPE3ParMediatorTestCase(test.TestCase): # Warning is logged (no exception thrown). self.assertTrue(mock_log.warning.called) - def test__unmount_super_share(self): + def test__unmount_share(self): self.init_mediator() - self.mock_object(utils, 'execute', mock.Mock(return_value={})) + mount_dir = '/mnt/foo' + self.mediator._unmount_share(mount_dir) - mount_location = '/mnt/foo' - self.mediator._unmount_super_share(mount_location) + utils.execute.assert_called_with('umount', mount_dir, run_as_root=True) - utils.execute.assert_called_with('umount', mount_location, - run_as_root=True) - - def test__unmount_super_share_error(self): + def test__unmount_share_error(self): self.init_mediator() self.mock_object(utils, 'execute', mock.Mock(side_effect=Exception('umount error.'))) mock_log = self.mock_object(hpe3parmediator, 'LOG') - mount_location = '/mnt/foo' - self.mediator._unmount_super_share(mount_location) + mount_dir = '/mnt/foo' + self.mediator._unmount_share(mount_dir) # Warning is logged (no exception thrown). self.assertTrue(mock_log.warning.called) @@ -2664,13 +2848,49 @@ class HPE3ParMediatorTestCase(test.TestCase): Exception("setfshare error.")) self.assertRaises( - exception.ShareMountException, + exception.ShareBackendException, self.mediator._create_super_share, constants.SMB_LOWER, constants.EXPECTED_FPG, constants.EXPECTED_VFS, constants.EXPECTED_FSTORE) + def test__revoke_admin_smb_access_error(self): + self.init_mediator() + + self.mock_client.setfshare.side_effect = ( + Exception("setfshare error")) + + self.assertRaises( + exception.ShareBackendException, + self.mediator._revoke_admin_smb_access, + constants.SMB_LOWER, + constants.EXPECTED_FPG, + constants.EXPECTED_VFS, + constants.EXPECTED_FSTORE, + constants.EXPECTED_COMMENT) + + def test_build_export_location_bad_protocol(self): + self.assertRaises(exception.InvalidInput, + self.mediator.build_export_location, + "BOGUS", + constants.EXPECTED_IP_1234, + constants.EXPECTED_SHARE_PATH) + + def test_build_export_location_bad_ip(self): + self.assertRaises(exception.InvalidInput, + self.mediator.build_export_location, + constants.NFS, + None, + None) + + def test_build_export_location_bad_path(self): + self.assertRaises(exception.InvalidInput, + self.mediator.build_export_location, + constants.NFS, + constants.EXPECTED_IP_1234, + None) + class OptionMatcher(object): """Options string order can vary. Compare as lists.""" diff --git a/releasenotes/notes/hpe3par-rw-snapshot-shares-f7c33b4bf528bf00.yaml b/releasenotes/notes/hpe3par-rw-snapshot-shares-f7c33b4bf528bf00.yaml new file mode 100644 index 0000000000..4f4827f6d8 --- /dev/null +++ b/releasenotes/notes/hpe3par-rw-snapshot-shares-f7c33b4bf528bf00.yaml @@ -0,0 +1,3 @@ +--- +features: + - Add read-write functionality for HPE 3PAR shares from snapshots.