[NetApp] Follow up fixes for update security service

This patch is a follow up of [1] to address some comments
added in the review process. It also adds more unit tests to
validate the new code added.

[1] https://review.opendev.org/c/openstack/manila/+/775032

Change-Id: If7b8628fa05200363a129eb19d9dc80fb7e3bc36
Signed-off-by: Douglas Viroel <viroel@gmail.com>

manila/share/drivers/netapp/dataontap/client/client_cmode.py

@@ -1614,6 +1614,9 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
             self.send_request('ldap-config-delete')
         except netapp_api.NaApiError as e:
             if e.code != netapp_api.EOBJECTNOTFOUND:
+                # Delete previously created ldap client
+                self._delete_ldap_client(new_security_service)
+
                 msg = _("An error occurred while deleting original LDAP "
                         "configuration. %s")
                 raise exception.NetAppException(msg % e.message)
@@ -1633,9 +1636,17 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
             self._delete_ldap_client(current_security_service)
         except netapp_api.NaApiError as e:
             if e.code != netapp_api.EOBJECTNOTFOUND:
+                current_config_name = (
+                    hashlib.md5(six.b(
+                        current_security_service['id'])).hexdigest())
                 msg = _("An error occurred while deleting original LDAP "
-                        "client configuration. %s")
-                raise exception.NetAppException(msg % e.message)
+                        "client configuration %(current_config)s. "
+                        "Error details: %(e_msg)s")
+                msg_args = {
+                    'current_config': current_config_name,
+                    'e_msg': e.message,
+                }
+                LOG.warning(msg, msg_args)
             else:
                 msg = _("Original LDAP client configuration was not found.")
                 LOG.debug(msg)

manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py

@@ -1425,7 +1425,7 @@ class NetAppCmodeMultiSVMFileStorageLibrary(
                 current_security_service=current_security_service):
             msg = _("The requested security service update is not supported "
                     "by the NetApp driver.")
-            LOG.exception(msg)
+            LOG.error(msg)
             raise exception.NetAppException(msg)
 
         if current_security_service is None:

manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py

@@ -7822,6 +7822,47 @@ class NetAppClientCmodeTestCase(test.TestCase):
             mock.call('ldap-config-create', api_args)])
         mock_delete_client.assert_called_once_with(current_ldap_service)
 
+    def test_modify_ldap_config_delete_failure(self):
+        current_ldap_service = fake.LDAP_AD_SECURITY_SERVICE
+        new_ldap_service = fake.LDAP_LINUX_SECURITY_SERVICE
+        mock_create_client = self.mock_object(
+            self.client, '_create_ldap_client')
+        mock_send_request = self.mock_object(
+            self.client, 'send_request', mock.Mock(
+                side_effect=netapp_api.NaApiError(code=netapp_api.EAPIERROR)))
+        mock_delete_client = self.mock_object(
+            self.client, '_delete_ldap_client')
+
+        self.assertRaises(exception.NetAppException,
+                          self.client.modify_ldap,
+                          new_ldap_service,
+                          current_ldap_service)
+
+        mock_create_client.assert_called_once_with(new_ldap_service)
+        mock_send_request.assert_called_once_with('ldap-config-delete')
+        mock_delete_client.assert_called_once_with(new_ldap_service)
+
+    def test_modify_ldap_current_config_delete_error(self):
+        current_ldap_service = fake.LDAP_AD_SECURITY_SERVICE
+        new_ldap_service = fake.LDAP_LINUX_SECURITY_SERVICE
+        config_name = hashlib.md5(six.b(new_ldap_service['id'])).hexdigest()
+        mock_create_client = self.mock_object(
+            self.client, '_create_ldap_client')
+        mock_send_request = self.mock_object(
+            self.client, 'send_request')
+        mock_delete_client = self.mock_object(
+            self.client, '_delete_ldap_client', mock.Mock(
+                side_effect=netapp_api.NaApiError(code=netapp_api.EAPIERROR)))
+
+        self.client.modify_ldap(new_ldap_service, current_ldap_service)
+
+        api_args = {'client-config': config_name, 'client-enabled': 'true'}
+        mock_create_client.assert_called_once_with(new_ldap_service)
+        mock_send_request.assert_has_calls([
+            mock.call('ldap-config-delete'),
+            mock.call('ldap-config-create', api_args)])
+        mock_delete_client.assert_called_once_with(current_ldap_service)
+
     def test_create_fpolicy_event(self):
         self.mock_object(self.client, 'send_request')
 

releasenotes/notes/netapp-add-security-service-update-718a68ebe60fd2b5.yaml

@@ -5,6 +5,6 @@ features:
     they are associated with in use share networks. Both add and update
     operations are supported by all three security service types:
     ``active_directory``, ``kerberos`` and ``ldap``. In order to update their
-    parameters in a non-disruptively way, ``active_directory`` and ``kerberos``
+    parameters in a non-disruptive way, ``active_directory`` and ``kerberos``
     don't support ``domain`` updates.