summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMark Sturdevant <mark.sturdevant@ibm.com>2016-12-14 14:49:55 -0800
committerDigvijay Ukirde <digvijay.ukirde@in.ibm.com>2017-04-25 10:10:19 +0000
commitc5545e861c8ee069a6e8c7b710b8813d8cb77012 (patch)
treedeb5695b5cadb96ad85016ed8e9b6032c25d2251
parent13b40b1463251ee13f618651ee3e24428b489d24 (diff)
GPFS CES: Fix bugs related to access rules not foundstable/newton
Several bugs are caused by an error code and exception raised when a path has no NFS exports. Use the machine-readable mmnfs export list command (the -Y option) which does not cause an exception when checking for exports. Co-Authored-By: digvijay2016 <digvijay.ukirde@in.ibm.com> Change-Id: I770756e0a36c5b61386878164b651fadf9730b7f Closes-Bug: #1650043 Closes-Bug: #1650044 Closes-Bug: #1650045 (cherry picked from commit 5e7323cd1f53f7f7096bcd32e84c36cc4858097a)
Notes
Notes (review): Code-Review+1: Goutham Pacha Ravi <gouthampravi@gmail.com> Code-Review+1: Rodrigo Barbieri <rodrigo.barbieri2010@gmail.com> Code-Review+1: Valeriy Ponomaryov <vponomaryov@mirantis.com> Code-Review+1: zhongjun <jun.zhongjun2@gmail.com> Code-Review+1: Jan Provaznik <jan.provaznik@gmail.com> Code-Review+2: Ben Swartzlander <ben@swartzlander.org> Workflow+1: Ben Swartzlander <ben@swartzlander.org> Verified+2: Jenkins Submitted-by: Jenkins Submitted-at: Tue, 06 Jun 2017 15:40:15 +0000 Reviewed-on: https://review.openstack.org/459620 Project: openstack/manila Branch: refs/heads/stable/newton
-rw-r--r--manila/share/drivers/ibm/gpfs.py31
-rw-r--r--manila/tests/share/drivers/ibm/test_gpfs.py117
-rw-r--r--releasenotes/notes/bug-1650043-gpfs-access-bugs-8c10f26ff1f795f4.yaml6
3 files changed, 91 insertions, 63 deletions
diff --git a/manila/share/drivers/ibm/gpfs.py b/manila/share/drivers/ibm/gpfs.py
index 90c9108..61335fe 100644
--- a/manila/share/drivers/ibm/gpfs.py
+++ b/manila/share/drivers/ibm/gpfs.py
@@ -769,14 +769,19 @@ class CESHelper(NASHelperBase):
769 raise exception.GPFSException(msg) 769 raise exception.GPFSException(msg)
770 return out 770 return out
771 771
772 def remove_export(self, local_path, share): 772 def _has_client_access(self, local_path, access_to=None):
773 """Remove export.""" 773 """Check path for any export or for one with a specific IP address."""
774 err_msg = 'Failed to check exports on the system.' 774 err_msg = 'Failed to check exports on the system.'
775 out = self._execute_mmnfs_command(('list', '-n', local_path), err_msg) 775 out = self._execute_mmnfs_command(('list', '-n', local_path, '-Y'),
776 776 err_msg)
777 out = re.search(re.escape(local_path), out) 777 if access_to:
778 return ':' + access_to + ':' in out
779 else:
780 return ':' + local_path + ':' in out
778 781
779 if out is not None: 782 def remove_export(self, local_path, share):
783 """Remove export."""
784 if self._has_client_access(local_path):
780 err_msg = ('Failed to remove export for share %s.' 785 err_msg = ('Failed to remove export for share %s.'
781 % share['name']) 786 % share['name'])
782 self._execute_mmnfs_command(('remove', local_path), err_msg) 787 self._execute_mmnfs_command(('remove', local_path), err_msg)
@@ -787,16 +792,13 @@ class CESHelper(NASHelperBase):
787 if access['access_type'] != 'ip': 792 if access['access_type'] != 'ip':
788 raise exception.InvalidShareAccess(reason='Only ip access type ' 793 raise exception.InvalidShareAccess(reason='Only ip access type '
789 'supported.') 794 'supported.')
790 err_msg = 'Failed to check exports on the system.' 795 has_exports = self._has_client_access(local_path)
791 out = self._execute_mmnfs_command(('list', '-n', local_path), err_msg)
792 796
793 options_not_allowed = ['access_type=ro', 'access_type=rw'] 797 options_not_allowed = ['access_type=ro', 'access_type=rw']
794 export_opts = self.get_export_options(share, access, 'CES', 798 export_opts = self.get_export_options(share, access, 'CES',
795 options_not_allowed) 799 options_not_allowed)
796 800
797 out = re.search(re.escape(local_path), out) 801 if not has_exports:
798
799 if out is None:
800 cmd = ['add', local_path, '-c', 802 cmd = ['add', local_path, '-c',
801 access['access_to'] + 803 access['access_to'] +
802 '(' + export_opts + ')'] 804 '(' + export_opts + ')']
@@ -811,12 +813,9 @@ class CESHelper(NASHelperBase):
811 813
812 def deny_access(self, local_path, share, access, force=False): 814 def deny_access(self, local_path, share, access, force=False):
813 """Deny access to the host.""" 815 """Deny access to the host."""
814 err_msg = 'Failed to check exports on the system.' 816 has_export = self._has_client_access(local_path, access['access_to'])
815 out = self._execute_mmnfs_command(('list', '-n', local_path), err_msg)
816
817 out = re.search(re.escape(access['access_to']), out)
818 817
819 if out is not None: 818 if has_export:
820 err_msg = ('Failed to remove access for share %s.' 819 err_msg = ('Failed to remove access for share %s.'
821 % share['name']) 820 % share['name'])
822 self._execute_mmnfs_command(('change', local_path, 821 self._execute_mmnfs_command(('change', local_path,
diff --git a/manila/tests/share/drivers/ibm/test_gpfs.py b/manila/tests/share/drivers/ibm/test_gpfs.py
index 373c33c..52feb92 100644
--- a/manila/tests/share/drivers/ibm/test_gpfs.py
+++ b/manila/tests/share/drivers/ibm/test_gpfs.py
@@ -56,6 +56,20 @@ class GPFSShareDriverTestCase(test.TestCase):
56 self.fakefspath = "/gpfs0" 56 self.fakefspath = "/gpfs0"
57 self.fakesharepath = "/gpfs0/share-fakeid" 57 self.fakesharepath = "/gpfs0/share-fakeid"
58 self.fakesnapshotpath = "/gpfs0/.snapshots/snapshot-fakesnapshotid" 58 self.fakesnapshotpath = "/gpfs0/.snapshots/snapshot-fakesnapshotid"
59
60 self.fake_ces_exports = """
61mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Clients:Access_Type:Protocols:Transports:Squash:Anonymous_uid:Anonymous_gid:SecType:PrivilegedPort:DefaultDelegations:Manage_Gids:NFS_Commit:
62mmcesnfslsexport:nfsexports:0:1:::/gpfs0/share-fakeid:none:44.3.2.11:RW:3,4:TCP:ROOT_SQUASH:-2:-2:SYS:FALSE:none:FALSE:FALSE:
63mmcesnfslsexport:nfsexports:0:1:::/gpfs0/share-fakeid:none:1:2:3:4:5:6:7:8:RW:3,4:TCP:ROOT_SQUASH:-2:-2:SYS:FALSE:none:FALSE:FALSE:
64mmcesnfslsexport:nfsexports:0:1:::/gpfs0/share-fakeid:none:10.0.0.1:RW:3,4:TCP:ROOT_SQUASH:-2:-2:SYS:FALSE:none:FALSE:FALSE:
65
66 """
67 self.fake_ces_exports_not_found = """
68
69mmcesnfslsexport:nfsexports:HEADER:version:reserved:reserved:Path:Delegations:Clients:Access_Type:Protocols:Transports:Squash:Anonymous_uid:Anonymous_gid:SecType:PrivilegedPort:DefaultDelegations:Manage_Gids:NFS_Commit:
70
71 """
72
59 self.mock_object(gpfs.os.path, 'exists', mock.Mock(return_value=True)) 73 self.mock_object(gpfs.os.path, 'exists', mock.Mock(return_value=True))
60 self._driver._helpers = { 74 self._driver._helpers = {
61 'KNFS': self._helper_fake 75 'KNFS': self._helper_fake
@@ -860,24 +874,49 @@ class GPFSShareDriverTestCase(test.TestCase):
860 self._ces_helper.get_export_options, 874 self._ces_helper.get_export_options,
861 share, access, 'CES', options_not_allowed) 875 share, access, 'CES', options_not_allowed)
862 876
863 def test_ces_remove_export(self): 877 @ddt.data((None, True),
864 mock_out = "Path Delegations Clients\n\ 878 ('44.3.2.11', True),
865 ------------------------\n\ 879 ('44.3.2.1', False),
866 /gpfs0/share-fakeid none *" 880 ('4.3.2.1', False),
881 ('4.3.2.11', False),
882 ('1.2.3.4', False),
883 ('1:2:3:4:5:6:7:8', True))
884 @ddt.unpack
885 def test_ces__has_client_access(self, ip, has_access):
886 mock_out = self.fake_ces_exports
867 self._ces_helper._execute = mock.Mock( 887 self._ces_helper._execute = mock.Mock(
868 return_value=[mock_out, 0]) 888 return_value=(mock_out, ''))
869 889
870 mock_search_out = "/gpfs0/share-fakeid" 890 local_path = self.fakesharepath
871 self.mock_object(re, 'search', mock.Mock(return_value=mock_search_out)) 891 self.assertEqual(has_access,
892 self._ces_helper._has_client_access(local_path, ip))
893
894 self._ces_helper._execute.assert_called_once_with(
895 'mmnfs', 'export', 'list', '-n', local_path, '-Y')
896
897 def test_ces_remove_export_no_exports(self):
898 mock_out = self.fake_ces_exports_not_found
899 self._ces_helper._execute = mock.Mock(
900 return_value=(mock_out, ''))
872 901
873 local_path = self.fakesharepath 902 local_path = self.fakesharepath
903 self._ces_helper.remove_export(local_path, self.share)
904
905 self._ces_helper._execute.assert_called_once_with(
906 'mmnfs', 'export', 'list', '-n', local_path, '-Y')
907
908 def test_ces_remove_export_existing_exports(self):
909 mock_out = self.fake_ces_exports
910 self._ces_helper._execute = mock.Mock(
911 return_value=(mock_out, ''))
874 912
913 local_path = self.fakesharepath
875 self._ces_helper.remove_export(local_path, self.share) 914 self._ces_helper.remove_export(local_path, self.share)
876 915
877 self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'list', 916 self._ces_helper._execute.assert_has_calls([
878 '-n', local_path) 917 mock.call('mmnfs', 'export', 'list', '-n', local_path, '-Y'),
879 self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'remove', 918 mock.call('mmnfs', 'export', 'remove', local_path),
880 local_path) 919 ])
881 920
882 def test_ces_remove_export_exception(self): 921 def test_ces_remove_export_exception(self):
883 local_path = self.fakesharepath 922 local_path = self.fakesharepath
@@ -888,52 +927,42 @@ class GPFSShareDriverTestCase(test.TestCase):
888 local_path, self.share) 927 local_path, self.share)
889 928
890 def test_ces_allow_access(self): 929 def test_ces_allow_access(self):
891 mock_out = "Path Delegations Clients\n\ 930 mock_out = self.fake_ces_exports_not_found
892 ------------------------"
893 self._ces_helper._execute = mock.Mock( 931 self._ces_helper._execute = mock.Mock(
894 return_value=[mock_out, 0]) 932 return_value=(mock_out, ''))
895 933
896 export_opts = "access_type=rw" 934 export_opts = "access_type=rw"
897 self._ces_helper.get_export_options = mock.Mock( 935 self._ces_helper.get_export_options = mock.Mock(
898 return_value=export_opts) 936 return_value=export_opts)
899 self.mock_object(re, 'search', mock.Mock(return_value=None))
900 937
901 access = self.access 938 access = self.access
902 local_path = self.fakesharepath 939 local_path = self.fakesharepath
903 940
904 self._ces_helper.allow_access(local_path, self.share, access) 941 self._ces_helper.allow_access(local_path, self.share, access)
905 942
906 self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'list', 943 self._ces_helper._execute.assert_has_calls([
907 '-n', local_path) 944 mock.call('mmnfs', 'export', 'list', '-n', local_path, '-Y'),
908 self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'add', 945 mock.call('mmnfs', 'export', 'add', local_path, '-c',
909 local_path, '-c', 946 access['access_to'] + '(' + export_opts + ')')])
910 access['access_to'] 947
911 + '(' + export_opts + ')') 948 def test_ces_allow_access_existing_exports(self):
912 949 mock_out = self.fake_ces_exports
913 def test_ces_allow_access_existing_export(self):
914 mock_out = "Path Delegations Clients\n\
915 ------------------------\n\
916 /gpfs0/share-fakeid none *"
917 self._ces_helper._execute = mock.Mock( 950 self._ces_helper._execute = mock.Mock(
918 return_value=[mock_out, 0]) 951 return_value=(mock_out, ''))
919 952
920 export_opts = "access_type=rw" 953 export_opts = "access_type=rw"
921 self._ces_helper.get_export_options = mock.Mock( 954 self._ces_helper.get_export_options = mock.Mock(
922 return_value=export_opts) 955 return_value=export_opts)
923 mock_search_out = "/gpfs0/share-fakeid"
924 self.mock_object(re, 'search', mock.Mock(return_value=mock_search_out))
925 956
926 access = self.access 957 access = self.access
927 local_path = self.fakesharepath 958 local_path = self.fakesharepath
928 959
929 self._ces_helper.allow_access(local_path, self.share, access) 960 self._ces_helper.allow_access(local_path, self.share, access)
930 961
931 self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'list', 962 self._ces_helper._execute.assert_has_calls([
932 '-n', local_path) 963 mock.call('mmnfs', 'export', 'list', '-n', local_path, '-Y'),
933 self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'change', 964 mock.call('mmnfs', 'export', 'change', local_path, '--nfsadd',
934 local_path, '--nfsadd', 965 access['access_to'] + '(' + export_opts + ')')])
935 access['access_to']
936 + '(' + export_opts + ')')
937 966
938 def test_ces_allow_access_invalid_access_type(self): 967 def test_ces_allow_access_invalid_access_type(self):
939 access = fake_share.fake_access(access_type='test') 968 access = fake_share.fake_access(access_type='test')
@@ -952,25 +981,19 @@ class GPFSShareDriverTestCase(test.TestCase):
952 self.share, access) 981 self.share, access)
953 982
954 def test_ces_deny_access(self): 983 def test_ces_deny_access(self):
955 mock_out = "Path Delegations Clients\n\ 984 mock_out = self.fake_ces_exports
956 ------------------------\n\
957 /gpfs0/share-fakeid none *"
958 self._ces_helper._execute = mock.Mock( 985 self._ces_helper._execute = mock.Mock(
959 return_value=[mock_out, 0]) 986 return_value=(mock_out, ''))
960
961 mock_search_out = "/gpfs0/share-fakeid"
962 self.mock_object(re, 'search', mock.Mock(return_value=mock_search_out))
963 987
964 access = self.access 988 access = self.access
965 local_path = self.fakesharepath 989 local_path = self.fakesharepath
966 990
967 self._ces_helper.deny_access(local_path, self.share, access) 991 self._ces_helper.deny_access(local_path, self.share, access)
968 992
969 self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'list', 993 self._ces_helper._execute.assert_has_calls([
970 '-n', local_path) 994 mock.call('mmnfs', 'export', 'list', '-n', local_path, '-Y'),
971 self._ces_helper._execute.assert_any_call('mmnfs', 'export', 'change', 995 mock.call('mmnfs', 'export', 'change', local_path, '--nfsremove',
972 local_path, '--nfsremove', 996 access['access_to'])])
973 access['access_to'])
974 997
975 def test_ces_deny_access_exception(self): 998 def test_ces_deny_access_exception(self):
976 access = self.access 999 access = self.access
diff --git a/releasenotes/notes/bug-1650043-gpfs-access-bugs-8c10f26ff1f795f4.yaml b/releasenotes/notes/bug-1650043-gpfs-access-bugs-8c10f26ff1f795f4.yaml
new file mode 100644
index 0000000..ac0e81f
--- /dev/null
+++ b/releasenotes/notes/bug-1650043-gpfs-access-bugs-8c10f26ff1f795f4.yaml
@@ -0,0 +1,6 @@
1---
2fixes:
3 - Fixed GPFS CES to allow adding a first access rule to a share.
4 - Fixed GPFS CES to allow deleting a share with no access rules.
5 - Fixed GPFS CES to allow deletion of a failed access rule
6 when there are no successful access rules.