NetApp: E-Series fix JSONDecodeError on first add

The NetApp E-Series driver would throw a JSONDecodeError on the
addition of a new storage-system. This would occur because a
JSON response was expected, but text/html was being returned as
the Content-Type when a storage-system has been added but is still
coming online.

This patch adds a retry to the driver startup to wait for an array
to come online. It also adds code to avoid the JSONDecodeError
and fail based on the HTTP Response's return code.

Change-Id: I405723039c7a0df10d39bd2eb961ee64d71c2d01

diff --git a/cinder/tests/unit/volume/drivers/netapp/eseries/test_client.py b/cinder/tests/unit/volume/drivers/netapp/eseries/test_client.py
index 87a854ea1f8fbff637a6e1150c71164a90f82020..5a7857d2679379d0cd91ac8f315fbc4eb332a206 100644 (file)
--- a/cinder/tests/unit/volume/drivers/netapp/eseries/test_client.py
+++ b/cinder/tests/unit/volume/drivers/netapp/eseries/test_client.py
@@ -19,6 +19,7 @@ import copy
 
 import ddt
 import mock
+from simplejson import scanner
 
 from cinder import exception
 from cinder import test
@@ -87,6 +88,18 @@ class NetAppEseriesClientDriverTestCase(test.TestCase):
             self.my_client._eval_response(fake_resp)
             self.assertEqual(status_code, exc.status_code)
 
+    def test_eval_response_424(self):
+        status_code = 424
+        fake_resp = mock.Mock()
+        fake_resp.status_code = status_code
+        fake_resp.text = "Fake Error Message"
+
+        with self.assertRaisesRegex(es_exception.WebServiceException,
+                                    "The storage-system is offline") as exc:
+            self.my_client._eval_response(fake_resp)
+
+            self.assertEqual(status_code, exc.status_code)
+
     def test_register_storage_system_does_not_log_password(self):
         self.my_client._eval_response = mock.Mock()
         self.my_client.register_storage_system([], password=self.fake_password)
@@ -752,6 +765,20 @@ class NetAppEseriesClientDriverTestCase(test.TestCase):
 
         self.assertTrue(self.my_client.features.SSC_API_V2.supported)
 
+    def test_invoke_bad_content_type(self):
+        """Tests the invoke behavior with a non-JSON response"""
+        fake_response = mock.Mock()
+        fake_response.json = mock.Mock(side_effect=scanner.JSONDecodeError(
+            '', '{}', 1))
+        fake_response.status_code = 424
+        fake_response.text = "Fake Response"
+        self.mock_object(self.my_client, 'invoke_service',
+                         mock.Mock(return_value=fake_response))
+
+        self.assertRaises(es_exception.WebServiceException,
+                          self.my_client._invoke, 'GET',
+                          eseries_fake.FAKE_ENDPOINT_HTTP)
+
 
 @ddt.ddt
 class TestWebserviceClientTestCase(test.TestCase):

diff --git a/cinder/volume/drivers/netapp/eseries/client.py b/cinder/volume/drivers/netapp/eseries/client.py
index 60897216b18d936f137d0d46e1e20a6bd31d4da5..2e9d25c8f1f063eab9cb200461f631c51f40d677 100644 (file)
--- a/cinder/volume/drivers/netapp/eseries/client.py
+++ b/cinder/volume/drivers/netapp/eseries/client.py
@@ -27,6 +27,7 @@ import uuid
 
 from oslo_log import log as logging
 import requests
+from simplejson import scanner
 import six
 from six.moves import urllib
 
@@ -219,7 +220,13 @@ class RestClient(WebserviceClient):
             res = self.invoke_service(method, url, data=data,
                                       headers=headers,
                                       timeout=timeout, verify=verify)
-            res_dict = res.json() if res.text else None
+
+            try:
+                res_dict = res.json() if res.text else None
+            # This should only occur if we expected JSON, but were sent
+            # something else
+            except scanner.JSONDecodeError:
+                res_dict = None
 
             if cinder_utils.TRACE_API:
                 self._log_http_response(res.status_code, dict(res.headers),
@@ -281,6 +288,8 @@ class RestClient(WebserviceClient):
                         msg = _("The storage array password for %s is "
                                 "incorrect, please update the configured "
                                 "password.") % self._system_id
+            elif status_code == 424:
+                msg = _("Response error - The storage-system is offline.")
             else:
                 msg = _("Response error code - %s.") % status_code
             raise es_exception.WebServiceException(msg,

diff --git a/cinder/volume/drivers/netapp/eseries/library.py b/cinder/volume/drivers/netapp/eseries/library.py
index dc5d0e2064c89fa272daa0e57dcbd4ce9347240a..d01b942fc969fd0318c7ab1f9461865718f5e317 100644 (file)
--- a/cinder/volume/drivers/netapp/eseries/library.py
+++ b/cinder/volume/drivers/netapp/eseries/library.py
@@ -173,8 +173,11 @@ class NetAppESeriesLibrary(object):
     def check_for_setup_error(self):
         self._check_host_type()
         self._check_multipath()
-        self._check_pools()
+        # It is important that this be called before any other methods that
+        # interact with the storage-system. It blocks until the
+        # storage-system comes online.
         self._check_storage_system()
+        self._check_pools()
         self._start_periodic_tasks()
 
     def _check_host_type(self):