From: Michael Price <michael.price@netapp.com>
Date: Thu, 12 Nov 2015 22:44:07 +0000 (-0600)
Subject: NetApp: E-Series fix JSONDecodeError on first add
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=489c8d2a5f829e5398d861d29d1a2beb504c6027;p=openstack-build%2Fcinder-build.git

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 87a854ea1..5a7857d26 100644
--- 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 60897216b..2e9d25c8f 100644
--- 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 dc5d0e206..d01b942fc 100644
--- 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):