Merge "Take into consideration races in XtremIOClient3"

cinder/tests/unit/test_emc_xtremio.py

@@ -563,22 +563,20 @@ class EMCXIODriverFibreChannelTestCase(test.TestCase):
         super(EMCXIODriverFibreChannelTestCase, self).setUp()
         clean_xms_data()
 
-        config = mock.Mock()
-        config.san_login = ''
-        config.san_password = ''
-        config.san_ip = ''
-        config.xtremio_cluster_name = ''
-        config.xtremio_provisioning_factor = 20.0
+        self.config = mock.Mock(san_login='',
+                                san_password='',
+                                san_ip='',
+                                xtremio_cluster_name='',
+                                xtremio_provisioning_factor=20.0)
         self.driver = xtremio.XtremIOFibreChannelDriver(
-            configuration=config)
-        self.driver.client = xtremio.XtremIOClient4(config,
-                                                    config.
-                                                    xtremio_cluster_name)
-
+            configuration=self.config)
         self.data = CommonData()
 
     def test_initialize_terminate_connection(self, req):
         req.side_effect = xms_request
+        self.driver.client = xtremio.XtremIOClient4(
+            self.config, self.config.xtremio_cluster_name)
+
         self.driver.create_volume(self.data.test_volume)
         map_data = self.driver.initialize_connection(self.data.test_volume,
                                                      self.data.connector)
@@ -586,3 +584,43 @@ class EMCXIODriverFibreChannelTestCase(test.TestCase):
         self.driver.terminate_connection(self.data.test_volume,
                                          self.data.connector)
         self.driver.delete_volume(self.data.test_volume)
+
+    def test_race_on_terminate_connection(self, req):
+        """Test for race conditions on num_of_mapped_volumes.
+
+        This test confirms that num_of_mapped_volumes won't break even if we
+        receive a NotFound exception when retrieving info on a specific
+        mapping, as that specific mapping could have been deleted between
+        the request to get the list of exiting mappings and the request to get
+        the info on one of them.
+        """
+        req.side_effect = xms_request
+        self.driver.client = xtremio.XtremIOClient3(
+            self.config, self.config.xtremio_cluster_name)
+        # We'll wrap num_of_mapped_volumes, we'll store here original method
+        original_method = self.driver.client.num_of_mapped_volumes
+
+        def fake_num_of_mapped_volumes(*args, **kwargs):
+            # Add a nonexistent mapping
+            mappings = [{'href': 'volumes/1'}, {'href': 'volumes/12'}]
+
+            # Side effects will be: 1st call returns the list, then we return
+            # data for existing mappings, and on the nonexistent one we added
+            # we return NotFound
+            side_effect = [{'lun-maps': mappings},
+                           {'content': xms_data['lun-maps'][1]},
+                           exception.NotFound]
+
+            with mock.patch.object(self.driver.client, 'req',
+                                   side_effect=side_effect):
+                return original_method(*args, **kwargs)
+
+        self.driver.create_volume(self.data.test_volume)
+        map_data = self.driver.initialize_connection(self.data.test_volume,
+                                                     self.data.connector)
+        self.assertEqual(1, map_data['data']['target_lun'])
+        with mock.patch.object(self.driver.client, 'num_of_mapped_volumes',
+                               side_effect=fake_num_of_mapped_volumes):
+            self.driver.terminate_connection(self.data.test_volume,
+                                             self.data.connector)
+        self.driver.delete_volume(self.data.test_volume)

cinder/volume/drivers/emc/xtremio.py

@@ -198,21 +198,35 @@ class XtremIOClient3(XtremIOClient):
 
     def find_lunmap(self, ig_name, vol_name):
         try:
-            for lm_link in self.req('lun-maps')['lun-maps']:
-                idx = lm_link['href'].split('/')[-1]
-                lm = self.req('lun-maps', idx=int(idx))['content']
-                if lm['ig-name'] == ig_name and lm['vol-name'] == vol_name:
-                    return lm
+            lun_mappings = self.req('lun-maps')['lun-maps']
         except exception.NotFound:
             raise (exception.VolumeDriverException
                    (_("can't find lun-map, ig:%(ig)s vol:%(vol)s") %
                     {'ig': ig_name, 'vol': vol_name}))
 
+        for lm_link in lun_mappings:
+            idx = lm_link['href'].split('/')[-1]
+            # NOTE(geguileo): There can be races so mapped elements retrieved
+            # in the listing may no longer exist.
+            try:
+                lm = self.req('lun-maps', idx=int(idx))['content']
+            except exception.NotFound:
+                continue
+            if lm['ig-name'] == ig_name and lm['vol-name'] == vol_name:
+                return lm
+
+        return None
+
     def num_of_mapped_volumes(self, initiator):
         cnt = 0
         for lm_link in self.req('lun-maps')['lun-maps']:
             idx = lm_link['href'].split('/')[-1]
-            lm = self.req('lun-maps', idx=int(idx))['content']
+            # NOTE(geguileo): There can be races so mapped elements retrieved
+            # in the listing may no longer exist.
+            try:
+                lm = self.req('lun-maps', idx=int(idx))['content']
+            except exception.NotFound:
+                continue
             if lm['ig-name'] == initiator:
                 cnt += 1
         return cnt