Use a new rest client for every Coraid ESM command

It's workaround for the session corruption bug
in the ESM Appliance.

fix bug #1214433

Change-Id: Ie844f3c95675c4d734390c23ed2d421c159ce8a7

diff --git a/cinder/tests/test_coraid.py b/cinder/tests/test_coraid.py
index ada0080bc617163f30e10590e8cf4cab715e251c..7b5b9de50b7dff91caa3197a6c628d3c5811b946 100644 (file)
--- a/cinder/tests/test_coraid.py
+++ b/cinder/tests/test_coraid.py
@@ -243,8 +243,7 @@ class CoraidDriverTestCase(test.TestCase):
         configuration.use_multipath_for_image_xfer = False
         self.fake_rpc = FakeRpc()
 
-        self.mox.StubOutWithMock(coraid.CoraidRESTClient, 'rpc')
-        coraid.CoraidRESTClient.rpc = self.fake_rpc
+        self.stubs.Set(coraid.CoraidRESTClient, 'rpc', self.fake_rpc)
 
         self.driver = coraid.CoraidDriver(configuration=configuration)
         self.driver.do_setup({})
@@ -310,13 +309,19 @@ class CoraidDriverApplianceTestCase(CoraidDriverLoginSuccessTestCase):
         self.fake_rpc.handle('configure', {}, [resize_volume_request],
                              reply)
 
-        real_reply = self.driver._appliance.resize_volume(fake_volume_name,
-                                                          new_volume_size)
+        real_reply = self.driver.appliance.resize_volume(fake_volume_name,
+                                                         new_volume_size)
 
         self.assertEqual(reply['configState'], real_reply['configState'])
 
 
 class CoraidDriverIntegrationalTestCase(CoraidDriverLoginSuccessTestCase):
+    def setUp(self):
+        super(CoraidDriverIntegrationalTestCase, self).setUp()
+        self.appliance = self.driver.appliance
+        # NOTE(nsobolevsky) prevent re-creation esm appliance
+        self.stubs.Set(coraid.CoraidDriver, 'appliance', self.appliance)
+
     def test_create_volume(self):
         self.mock_volume_types()
 
@@ -370,19 +375,20 @@ class CoraidDriverIntegrationalTestCase(CoraidDriverLoginSuccessTestCase):
 
         self.mox.ReplayAll()
 
-        self.driver._appliance.ping()
+        self.driver.appliance.ping()
 
         self.mox.VerifyAll()
 
     def test_ping_failed(self):
-        def rpc(*args, **kwargs):
-            raise Exception("Some exception")
+        def rpc(handle, url_params, data,
+                allow_empty_response=True):
+            raise test.TestingException("Some exception")
 
-        self.stubs.Set(self.driver._appliance, 'rpc', rpc)
+        self.stubs.Set(self.driver.appliance, 'rpc', rpc)
         self.mox.ReplayAll()
 
         self.assertRaises(exception.CoraidESMNotAvailable,
-                          self.driver._appliance.ping)
+                          self.driver.appliance.ping)
 
         self.mox.VerifyAll()
 
@@ -408,25 +414,21 @@ class CoraidDriverIntegrationalTestCase(CoraidDriverLoginSuccessTestCase):
 
         self.assertRaises(
             exception.VolumeNotFound,
-            self.driver._appliance.delete_lun,
+            self.driver.appliance.delete_lun,
             fake_volume['name'])
 
         self.mox.VerifyAll()
 
-    def test_delete_not_existing_volume_appliance_is_ok(self):
-        self.mox.StubOutWithMock(self.driver._appliance, 'delete_lun')
-
+    def test_delete_not_existing_volumeappliance_is_ok(self):
         def delete_lun(volume_name):
             raise exception.VolumeNotFound(volume_id=fake_volume['name'])
 
-        self.driver._appliance.delete_lun = delete_lun
-
-        self.mox.StubOutWithMock(self.driver._appliance, 'ping')
+        self.stubs.Set(self.driver.appliance, 'delete_lun', delete_lun)
 
         def ping():
             pass
 
-        self.driver._appliance.ping = ping
+        self.stubs.Set(self.driver.appliance, 'ping', ping)
 
         self.mox.ReplayAll()
 
@@ -434,20 +436,18 @@ class CoraidDriverIntegrationalTestCase(CoraidDriverLoginSuccessTestCase):
 
         self.mox.VerifyAll()
 
-    def test_delete_not_existing_volume_sleeping_appliance(self):
-        self.mox.StubOutWithMock(self.driver._appliance, 'delete_lun')
-
+    def test_delete_not_existing_volume_sleepingappliance(self):
         def delete_lun(volume_name):
             raise exception.VolumeNotFound(volume_id=fake_volume['name'])
 
-        self.driver._appliance.delete_lun = delete_lun
-
-        self.mox.StubOutWithMock(self.driver._appliance, 'ping')
+        self.stubs.Set(self.driver.appliance, 'delete_lun', delete_lun)
 
         def ping():
             raise exception.CoraidESMNotAvailable(reason="Any reason")
 
-        self.driver._appliance.ping = ping
+        self.stubs.Set(self.driver.appliance, 'ping', ping)
+
+        self.driver.appliance.ping = ping
 
         self.mox.ReplayAll()
 
@@ -507,9 +507,9 @@ class CoraidDriverIntegrationalTestCase(CoraidDriverLoginSuccessTestCase):
     def test_create_volume_from_snapshot(self):
         self.mock_volume_types()
 
-        self.mox.StubOutWithMock(self.driver._appliance, 'resize_volume')
-        self.driver._appliance.resize_volume(fake_volume_name,
-                                             fake_volume['size'])\
+        self.mox.StubOutWithMock(self.driver.appliance, 'resize_volume')
+        self.driver.appliance.resize_volume(fake_volume_name,
+                                            fake_volume['size'])\
             .AndReturn(None)
 
         fetch_request = {'shelf': 'cms',
@@ -607,9 +607,9 @@ class CoraidDriverIntegrationalTestCase(CoraidDriverLoginSuccessTestCase):
     def test_create_cloned_volume_with_resize(self):
         self.mock_volume_types([fake_repository_name])
 
-        self.mox.StubOutWithMock(self.driver._appliance, 'resize_volume')
-        self.driver._appliance.resize_volume(fake_big_clone_volume['name'],
-                                             fake_big_clone_volume['size'])\
+        self.mox.StubOutWithMock(self.driver.appliance, 'resize_volume')
+        self.driver.appliance.resize_volume(fake_big_clone_volume['name'],
+                                            fake_big_clone_volume['size'])\
             .AndReturn(None)
 
         fetch_request = {'shelf': 'cms',
@@ -658,8 +658,8 @@ class CoraidDriverIntegrationalTestCase(CoraidDriverLoginSuccessTestCase):
         self.mox.VerifyAll()
 
     def test_extend_volume(self):
-        self.mox.StubOutWithMock(self.driver._appliance, 'resize_volume')
-        self.driver._appliance.resize_volume(fake_volume_name, 10)\
+        self.mox.StubOutWithMock(self.driver.appliance, 'resize_volume')
+        self.driver.appliance.resize_volume(fake_volume_name, 10)\
             .AndReturn(None)
 
         self.mox.ReplayAll()
@@ -831,3 +831,27 @@ class CoraidDriverImageTestCases(CoraidDriverTestCase):
                                          fake_image_id)
 
         self.mox.VerifyAll()
+
+
+class CoraidResetConnectionTestCase(CoraidDriverTestCase):
+    def test_create_new_appliance_for_every_request(self):
+        self.mox.StubOutWithMock(coraid, 'CoraidRESTClient')
+        self.mox.StubOutWithMock(coraid, 'CoraidAppliance')
+
+        coraid.CoraidRESTClient(mox.IgnoreArg())
+        coraid.CoraidRESTClient(mox.IgnoreArg())
+
+        coraid.CoraidAppliance(mox.IgnoreArg(),
+                               mox.IgnoreArg(),
+                               mox.IgnoreArg(),
+                               mox.IgnoreArg()).AndReturn('fake_app1')
+        coraid.CoraidAppliance(mox.IgnoreArg(),
+                               mox.IgnoreArg(),
+                               mox.IgnoreArg(),
+                               mox.IgnoreArg()).AndReturn('fake_app2')
+        self.mox.ReplayAll()
+
+        self.assertEqual(self.driver.appliance, 'fake_app1')
+        self.assertEqual(self.driver.appliance, 'fake_app2')
+
+        self.mox.VerifyAll()

diff --git a/cinder/volume/drivers/coraid.py b/cinder/volume/drivers/coraid.py
index 8a5d1bc68a2e59106dcedcb69de26005d2fed90c..c42f6ba331cd93863cd02f656d62aa4a1db1b0d9 100644 (file)
--- a/cinder/volume/drivers/coraid.py
+++ b/cinder/volume/drivers/coraid.py
@@ -407,7 +407,6 @@ class CoraidDriver(driver.VolumeDriver):
     def __init__(self, *args, **kwargs):
         super(CoraidDriver, self).__init__(*args, **kwargs)
         self.configuration.append_config_values(coraid_opts)
-        self._appliance = None
 
         self._stats = {'driver_version': self.VERSION,
                        'free_capacity_gb': 'unknown',
@@ -418,20 +417,23 @@ class CoraidDriver(driver.VolumeDriver):
         backend_name = self.configuration.safe_get('volume_backend_name')
         self._stats['volume_backend_name'] = backend_name or 'EtherCloud ESM'
 
-    def do_setup(self, context):
-        """Initialize the volume driver."""
+    @property
+    def appliance(self):
+        # NOTE(nsobolevsky): This is workaround for bug in the ESM appliance.
+        # If there is a lot of request with the same session/cookie/connection,
+        # the appliance could corrupt all following request in session.
+        # For that purpose we just create a new appliance.
         esm_url = "https://{0}:8443".format(
             self.configuration.coraid_esm_address)
 
-        rest_client = CoraidRESTClient(esm_url)
-        self._appliance = CoraidAppliance(rest_client,
-                                          self.configuration.coraid_user,
-                                          self.configuration.coraid_password,
-                                          self.configuration.coraid_group)
+        return CoraidAppliance(CoraidRESTClient(esm_url),
+                               self.configuration.coraid_user,
+                               self.configuration.coraid_password,
+                               self.configuration.coraid_group)
 
     def check_for_setup_error(self):
         """Return an error if prerequisites aren't met."""
-        self._appliance.ping()
+        self.appliance.ping()
 
     def _get_repository(self, volume_type):
         """Get the ESM Repository from the Volume Type.
@@ -451,60 +453,53 @@ class CoraidDriver(driver.VolumeDriver):
     def create_volume(self, volume):
         """Create a Volume."""
         repository = self._get_repository(volume['volume_type'])
-        self._appliance.create_lun(repository, volume['name'], volume['size'])
-        # NOTE(jbr_): The manager currently interprets any return as
-        # being the model_update for provider location.
-        # return None to not break it (thank to jgriffith and DuncanT)
-        return
+        self.appliance.create_lun(repository, volume['name'], volume['size'])
 
     def create_cloned_volume(self, volume, src_vref):
         dst_volume_repository = self._get_repository(volume['volume_type'])
 
-        self._appliance.clone_volume(src_vref['name'],
-                                     volume['name'],
-                                     dst_volume_repository)
+        self.appliance.clone_volume(src_vref['name'],
+                                    volume['name'],
+                                    dst_volume_repository)
 
         if volume['size'] != src_vref['size']:
-            self._appliance.resize_volume(volume['name'], volume['size'])
-
-        return
+            self.appliance.resize_volume(volume['name'], volume['size'])
 
     def delete_volume(self, volume):
         """Delete a Volume."""
         try:
-            self._appliance.delete_lun(volume['name'])
+            self.appliance.delete_lun(volume['name'])
         except exception.VolumeNotFound:
-            self._appliance.ping()
+            self.appliance.ping()
 
     def create_snapshot(self, snapshot):
         """Create a Snapshot."""
         volume_name = snapshot['volume_name']
         snapshot_name = snapshot['name']
-        self._appliance.create_snapshot(volume_name, snapshot_name)
+        self.appliance.create_snapshot(volume_name, snapshot_name)
 
     def delete_snapshot(self, snapshot):
         """Delete a Snapshot."""
         snapshot_name = snapshot['name']
-        self._appliance.delete_snapshot(snapshot_name)
+        self.appliance.delete_snapshot(snapshot_name)
 
     def create_volume_from_snapshot(self, volume, snapshot):
         """Create a Volume from a Snapshot."""
         snapshot_name = snapshot['name']
         repository = self._get_repository(volume['volume_type'])
-        self._appliance.create_volume_from_snapshot(snapshot_name,
-                                                    volume['name'],
-                                                    repository)
+        self.appliance.create_volume_from_snapshot(snapshot_name,
+                                                   volume['name'],
+                                                   repository)
         if volume['size'] > snapshot['volume_size']:
-            self._appliance.resize_volume(volume['name'], volume['size'])
+            self.appliance.resize_volume(volume['name'], volume['size'])
 
     def extend_volume(self, volume, new_size):
         """Extend an existing volume."""
-        self._appliance.resize_volume(volume['name'], new_size)
-        return
+        self.appliance.resize_volume(volume['name'], new_size)
 
     def initialize_connection(self, volume, connector):
         """Return connection information."""
-        volume_info = self._appliance.get_volume_info(volume['name'])
+        volume_info = self.appliance.get_volume_info(volume['name'])
 
         shelf = volume_info['shelf']
         lun = volume_info['lun']
@@ -522,7 +517,7 @@ class CoraidDriver(driver.VolumeDriver):
 
     def _get_repository_capabilities(self):
         repos_list = map(lambda i: i['profile']['fullName'] + ':' + i['name'],
-                         self._appliance.get_all_repos())
+                         self.appliance.get_all_repos())
         return ' '.join(repos_list)
 
     def update_volume_stats(self):