diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py index 6062cfedfb..bf8b713ba0 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -959,11 +959,26 @@ class NetAppCmodeFileStorageLibrary(object): def update_access(self, context, share, access_rules, add_rules=None, delete_rules=None, share_server=None): """Updates access rules for a share.""" - vserver, vserver_client = self._get_vserver(share_server=share_server) + try: + vserver, vserver_client = self._get_vserver( + share_server=share_server) + except (exception.InvalidInput, + exception.VserverNotSpecified, + exception.VserverNotFound) as error: + LOG.warning(_LW("Could not determine share server for share " + "%(share)s during access rules update. " + "Error: %(error)s"), + {'share': share['id'], 'error': error}) + return + share_name = self._get_valid_share_name(share['id']) - helper = self._get_helper(share) - helper.set_client(vserver_client) - helper.update_access(share, share_name, access_rules) + if self._share_exists(share_name, vserver_client): + helper = self._get_helper(share) + helper.set_client(vserver_client) + helper.update_access(share, share_name, access_rules) + else: + LOG.warning(_LW("Could not update access rules, share %s does " + "not exist."), share['id']) def setup_server(self, network_info, metadata=None): raise NotImplementedError() diff --git a/manila/share/drivers/netapp/dataontap/protocols/base.py b/manila/share/drivers/netapp/dataontap/protocols/base.py index fc202fa031..8089b05551 100644 --- a/manila/share/drivers/netapp/dataontap/protocols/base.py +++ b/manila/share/drivers/netapp/dataontap/protocols/base.py @@ -27,7 +27,7 @@ def access_rules_synchronized(f): def wrapped_func(self, *args, **kwargs): # The first argument is always a share, which has an ID - key = "cifs-access-%s" % args[0]['id'] + key = "share-access-%s" % args[0]['id'] @utils.synchronized(key) def source_func(self, *args, **kwargs): diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py index 85c8165ce0..945b01e1f7 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py @@ -1745,27 +1745,84 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): def test_update_access(self): + vserver_client = mock.Mock() + mock_get_vserver = self.mock_object( + self.library, '_get_vserver', + mock.Mock(return_value=(fake.VSERVER1, vserver_client))) protocol_helper = mock.Mock() protocol_helper.update_access.return_value = None self.mock_object(self.library, '_get_helper', mock.Mock(return_value=protocol_helper)) - vserver_client = mock.Mock() - self.mock_object(self.library, - '_get_vserver', - mock.Mock(return_value=(fake.VSERVER1, - vserver_client))) + mock_share_exists = self.mock_object(self.library, + '_share_exists', + mock.Mock(return_value=True)) self.library.update_access(self.context, fake.SHARE, [fake.SHARE_ACCESS], share_server=fake.SHARE_SERVER) + mock_get_vserver.assert_called_once_with( + share_server=fake.SHARE_SERVER) + share_name = self.library._get_valid_share_name(fake.SHARE['id']) + mock_share_exists.assert_called_once_with(share_name, vserver_client) protocol_helper.set_client.assert_called_once_with(vserver_client) protocol_helper.update_access.assert_called_once_with( - fake.SHARE, - fake.SHARE_NAME, - [fake.SHARE_ACCESS]) + fake.SHARE, fake.SHARE_NAME, [fake.SHARE_ACCESS]) + + @ddt.data(exception.InvalidInput(reason='fake_reason'), + exception.VserverNotSpecified(), + exception.VserverNotFound(vserver='fake_vserver')) + def test_update_access_no_share_server(self, get_vserver_exception): + + mock_get_vserver = self.mock_object( + self.library, '_get_vserver', + mock.Mock(side_effect=get_vserver_exception)) + protocol_helper = mock.Mock() + protocol_helper.update_access.return_value = None + self.mock_object(self.library, + '_get_helper', + mock.Mock(return_value=protocol_helper)) + mock_share_exists = self.mock_object(self.library, '_share_exists') + + self.library.update_access(self.context, + fake.SHARE, + [fake.SHARE_ACCESS], + share_server=fake.SHARE_SERVER) + + mock_get_vserver.assert_called_once_with( + share_server=fake.SHARE_SERVER) + self.assertFalse(mock_share_exists.called) + self.assertFalse(protocol_helper.set_client.called) + self.assertFalse(protocol_helper.update_access.called) + + def test_update_access_share_not_found(self): + + vserver_client = mock.Mock() + mock_get_vserver = self.mock_object( + self.library, '_get_vserver', + mock.Mock(return_value=(fake.VSERVER1, vserver_client))) + protocol_helper = mock.Mock() + protocol_helper.update_access.return_value = None + self.mock_object(self.library, + '_get_helper', + mock.Mock(return_value=protocol_helper)) + mock_share_exists = self.mock_object(self.library, + '_share_exists', + mock.Mock(return_value=False)) + + self.library.update_access(self.context, + fake.SHARE, + [fake.SHARE_ACCESS], + share_server=fake.SHARE_SERVER) + + mock_get_vserver.assert_called_once_with( + share_server=fake.SHARE_SERVER) + share_name = self.library._get_valid_share_name(fake.SHARE['id']) + mock_share_exists.assert_called_once_with(share_name, vserver_client) + self.assertFalse(protocol_helper.set_client.called) + self.assertFalse(protocol_helper.update_access.called) def test_setup_server(self): self.assertRaises(NotImplementedError,