Improve 3PAR driver VLUN creation and deletion

This patch improves the creation and deletion logic for VLUNs
for the 3PAR drivers. The changes will help to prevent VLUNs
from being left behind after volume detaches. It also prevents
extra VLUNs being created when initialize_connection is called
multiple times for the same volume and host.

Closes-Bug: #1469816
Change-Id: I497564cb4bd661252570eb2e69962f7d5f3c07da

diff --git a/cinder/tests/unit/test_hp3par.py b/cinder/tests/unit/test_hp3par.py
index 1d331c58432c8c9b1e4666344065232f8cfa8e31..6eb61195ec00b810e084a55cbe746389756d0e86 100644 (file)
--- a/cinder/tests/unit/test_hp3par.py
+++ b/cinder/tests/unit/test_hp3par.py
@@ -2096,6 +2096,7 @@ class HP3PARBaseDriver(object):
                     self.VOLUME_3PAR_NAME,
                     None,
                     self.FAKE_HOST),
+                mock.call.getHostVLUNs(self.FAKE_HOST),
                 mock.call.deleteHost(self.FAKE_HOST),
                 mock.call.removeVolumeMetaData(
                     self.VOLUME_3PAR_NAME, CHAP_USER_KEY),
@@ -2997,6 +2998,8 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
                 'name': self.FAKE_HOST
             }]
         }
+        mock_client.getVLUN.side_effect = [
+            hpexceptions.HTTPNotFound('fake')]
         mock_client.getHostVLUNs.return_value = [
             {'active': True,
              'volumeName': self.VOLUME_3PAR_NAME,
@@ -3023,6 +3026,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
                                           '123456789054321']),
                 mock.call.getHost(self.FAKE_HOST),
                 mock.call.getPorts(),
+                mock.call.getVLUN(self.VOLUME_3PAR_NAME),
                 mock.call.createVLUN(
                     self.VOLUME_3PAR_NAME,
                     auto=True,
@@ -3069,6 +3073,8 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
                 'name': self.FAKE_HOST
             }]
         }
+        mock_client.getVLUN.side_effect = [
+            hpexceptions.HTTPNotFound('fake')]
         mock_client.getHostVLUNs.return_value = [
             {'active': True,
              'volumeName': self.VOLUME_3PAR_NAME,
@@ -3109,6 +3115,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
                 mock.ANY,
                 mock.call.getHost(self.FAKE_HOST),
                 mock.call.getPorts(),
+                mock.call.getVLUN(self.VOLUME_3PAR_NAME),
                 mock.call.getPorts(),
                 mock.call.createVLUN(
                     self.VOLUME_3PAR_NAME,
@@ -3154,6 +3161,8 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
                 'name': self.FAKE_HOST
             }]
         }
+        mock_client.getVLUN.side_effect = [
+            hpexceptions.HTTPNotFound('fake')]
         mock_client.getHostVLUNs.return_value = [
             {'active': True,
              'volumeName': self.VOLUME_3PAR_NAME,
@@ -3180,6 +3189,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
                                           '123456789054321']),
                 mock.call.getHost(self.FAKE_HOST),
                 mock.call.getPorts(),
+                mock.call.getVLUN(self.VOLUME_3PAR_NAME),
                 mock.call.createVLUN(
                     self.VOLUME_3PAR_NAME,
                     auto=True,
@@ -3203,6 +3213,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
         effects = [
             [{'active': True, 'volumeName': self.VOLUME_3PAR_NAME,
               'lun': None, 'type': 0}],
+            hpexceptions.HTTPNotFound,
             hpexceptions.HTTPNotFound]
 
         mock_client.getHostVLUNs.side_effect = effects
@@ -3220,6 +3231,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
                 self.VOLUME_3PAR_NAME,
                 None,
                 self.FAKE_HOST),
+            mock.call.getHostVLUNs(self.FAKE_HOST),
             mock.call.deleteHost(self.FAKE_HOST),
             mock.call.getHostVLUNs(self.FAKE_HOST),
             mock.call.getPorts()]
@@ -3282,6 +3294,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
         effects = [
             [{'active': True, 'volumeName': self.VOLUME_3PAR_NAME,
               'lun': None, 'type': 0}],
+            hpexceptions.HTTPNotFound,
             hpexceptions.HTTPNotFound]
 
         mock_client.queryHost.return_value = {
@@ -3299,6 +3312,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
                 self.VOLUME_3PAR_NAME,
                 None,
                 self.FAKE_HOST),
+            mock.call.getHostVLUNs(self.FAKE_HOST),
             mock.call.deleteHost(self.FAKE_HOST),
             mock.call.getHostVLUNs(self.FAKE_HOST),
             mock.call.getPorts()]
@@ -3368,6 +3382,7 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
                 self.VOLUME_3PAR_NAME,
                 None,
                 self.FAKE_HOST),
+            mock.call.getHostVLUNs(self.FAKE_HOST),
             mock.call.getHostVLUNs(self.FAKE_HOST)]
 
         with mock.patch.object(hpcommon.HP3PARCommon,
@@ -3739,6 +3754,7 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
              'volumeName': self.VOLUME_3PAR_NAME,
              'lun': self.TARGET_LUN, 'type': 0}]
         mock_client.getVLUN.return_value = {
+            'hostname': self.FAKE_HOST,
             'lun': self.TARGET_LUN,
             'portPos': {'node': 8, 'slot': 1, 'cardPort': 1}}
         location = ("%(volume_name)s,%(lun_id)s,%(host)s,%(nsp)s" %
@@ -3761,13 +3777,7 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
                 mock.call.getHost(self.FAKE_HOST),
                 mock.call.queryHost(iqns=['iqn.1993-08.org.debian:01:222']),
                 mock.call.getHost(self.FAKE_HOST),
-                mock.call.getVLUN(self.VOLUME_3PAR_NAME),
-                mock.call.createVLUN(
-                    self.VOLUME_3PAR_NAME,
-                    auto=True,
-                    hostname='fakehost',
-                    portPos={'node': 8, 'slot': 1, 'cardPort': 1}),
-                mock.call.getHostVLUNs(self.FAKE_HOST)]
+                mock.call.getVLUN(self.VOLUME_3PAR_NAME)]
 
             mock_client.assert_has_calls(
                 self.standard_login +
@@ -3795,6 +3805,7 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
              'volumeName': self.VOLUME_3PAR_NAME,
              'lun': self.TARGET_LUN, 'type': 0}]
         mock_client.getVLUN.return_value = {
+            'hostname': self.FAKE_HOST,
             'lun': self.TARGET_LUN,
             'portPos': {'node': 8, 'slot': 1, 'cardPort': 1}}
         location = ("%(volume_name)s,%(lun_id)s,%(host)s,%(nsp)s" %
@@ -3817,13 +3828,7 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
                 mock.call.getHost(self.FAKE_HOST),
                 mock.call.queryHost(iqns=['iqn.1993-08.org.debian:01:222']),
                 mock.call.getHost(self.FAKE_HOST),
-                mock.call.getVLUN(self.VOLUME_3PAR_NAME),
-                mock.call.createVLUN(
-                    self.VOLUME_3PAR_NAME,
-                    auto=True,
-                    hostname='fakehost',
-                    portPos={'node': 8, 'slot': 1, 'cardPort': 1}),
-                mock.call.getHostVLUNs(self.FAKE_HOST)]
+                mock.call.getVLUN(self.VOLUME_3PAR_NAME)]
 
             mock_client.assert_has_calls(
                 self.standard_login +

diff --git a/cinder/volume/drivers/san/hp/hp_3par_common.py b/cinder/volume/drivers/san/hp/hp_3par_common.py
index 120e49665c2cdf464b4b93f1fbedbf1ff3ce8583..ac75a77db9beba8d2e2f450e4ec1e711a932a891 100644 (file)
--- a/cinder/volume/drivers/san/hp/hp_3par_common.py
+++ b/cinder/volume/drivers/san/hp/hp_3par_common.py
@@ -178,10 +178,11 @@ class HP3PARCommon(object):
         2.0.43 - Report the capability of supporting multiattach
         2.0.44 - Update help strings to reduce the 3PAR user role requirements
         2.0.45 - Python 3 fixes
+        2.0.46 - Improved VLUN creation and deletion logic. #1469816
 
     """
 
-    VERSION = "2.0.45"
+    VERSION = "2.0.46"
 
     stats = {}
 
@@ -794,24 +795,57 @@ class HP3PARCommon(object):
         volume_name = self._get_3par_vol_name(volume['id'])
         vluns = self.client.getHostVLUNs(hostname)
 
+        # Find all the VLUNs associated with the volume. The VLUNs will then
+        # be split into groups based on the active status of the VLUN. If there
+        # are active VLUNs detected a delete will be attempted on them. If
+        # there are no active VLUNs but there are inactive VLUNs, then the
+        # inactive VLUNs will be deleted. The inactive VLUNs are the templates
+        # on the 3PAR backend.
+        active_volume_vluns = []
+        inactive_volume_vluns = []
+        volume_vluns = []
+
         for vlun in vluns:
             if volume_name in vlun['volumeName']:
-                break
-        else:
-            LOG.info(_LI("3PAR vlun for volume %(name)s not found on host "
-                         "%(host)s"), {'name': volume_name, 'host': hostname})
+                if vlun['active']:
+                    active_volume_vluns.append(vlun)
+                else:
+                    inactive_volume_vluns.append(vlun)
+        if active_volume_vluns:
+            volume_vluns = active_volume_vluns
+        elif inactive_volume_vluns:
+            volume_vluns = inactive_volume_vluns
+
+        if not volume_vluns:
+            msg = (
+                _LW("3PAR vlun for volume %(name)s not found on "
+                    "host %(host)s"), {'name': volume_name, 'host': hostname})
+            LOG.warning(msg)
             return
 
         # VLUN Type of MATCHED_SET 4 requires the port to be provided
-        if self.VLUN_TYPE_MATCHED_SET == vlun['type']:
-            self.client.deleteVLUN(volume_name, vlun['lun'], hostname,
-                                   vlun['portPos'])
-        else:
-            self.client.deleteVLUN(volume_name, vlun['lun'], hostname)
+        removed_luns = []
+        for vlun in volume_vluns:
+            if self.VLUN_TYPE_MATCHED_SET == vlun['type']:
+                self.client.deleteVLUN(volume_name, vlun['lun'], hostname,
+                                       vlun['portPos'])
+            else:
+                # This is HOST_SEES or a type that is not MATCHED_SET.
+                # By deleting one VLUN, all the others should be deleted, too.
+                if vlun['lun'] not in removed_luns:
+                    self.client.deleteVLUN(volume_name, vlun['lun'], hostname)
+                    removed_luns.append(vlun['lun'])
 
         # Determine if there are other volumes attached to the host.
         # This will determine whether we should try removing host from host set
         # and deleting the host.
+        vluns = []
+        try:
+            vluns = self.client.getHostVLUNs(hostname)
+        except hpexceptions.HTTPNotFound:
+            LOG.debug("All VLUNs removed from host %s", hostname)
+            pass
+
         for vlun in vluns:
             if volume_name not in vlun['volumeName']:
                 # Found another volume

diff --git a/cinder/volume/drivers/san/hp/hp_3par_fc.py b/cinder/volume/drivers/san/hp/hp_3par_fc.py
index 8740dda1c4f15198e46b18645b6a343b16892a5f..2dc770919aeac91f8802574e356eccb96afc8fc3 100644 (file)
--- a/cinder/volume/drivers/san/hp/hp_3par_fc.py
+++ b/cinder/volume/drivers/san/hp/hp_3par_fc.py
@@ -78,10 +78,11 @@ class HP3PARFCDriver(cinder.volume.driver.FibreChannelDriver):
         2.0.14 - Removed usage of host name cache #1398914
         2.0.15 - Added support for updated detach_volume attachment.
         2.0.16 - Added encrypted property to initialize_connection #1439917
+        2.0.17 - Improved VLUN creation and deletion logic. #1469816
 
     """
 
-    VERSION = "2.0.16"
+    VERSION = "2.0.17"
 
     def __init__(self, *args, **kwargs):
         super(HP3PARFCDriver, self).__init__(*args, **kwargs)
@@ -228,17 +229,30 @@ class HP3PARFCDriver(cinder.volume.driver.FibreChannelDriver):
             target_wwns, init_targ_map, numPaths = \
                 self._build_initiator_target_map(common, connector)
 
-            # now that we have a host, create the VLUN
-            if self.lookup_service is not None and numPaths == 1:
-                nsp = None
-                active_fc_port_list = common.get_active_fc_target_ports()
-                for port in active_fc_port_list:
-                    if port['portWWN'].lower() == target_wwns[0].lower():
-                        nsp = port['nsp']
-                        break
-                vlun = common.create_vlun(volume, host, nsp)
+            # check if a VLUN already exists for this host
+            existing_vlun = None
+            try:
+                vol_name = common._get_3par_vol_name(volume['id'])
+                existing_vlun = common.client.getVLUN(vol_name)
+            except hpexceptions.HTTPNotFound:
+                # ignore, vlun will be created later
+                pass
+
+            vlun = None
+            if not existing_vlun or host['name'] != existing_vlun['hostname']:
+                # now that we have a host, create the VLUN
+                if self.lookup_service is not None and numPaths == 1:
+                    nsp = None
+                    active_fc_port_list = common.get_active_fc_target_ports()
+                    for port in active_fc_port_list:
+                        if port['portWWN'].lower() == target_wwns[0].lower():
+                            nsp = port['nsp']
+                            break
+                    vlun = common.create_vlun(volume, host, nsp)
+                else:
+                    vlun = common.create_vlun(volume, host)
             else:
-                vlun = common.create_vlun(volume, host)
+                vlun = existing_vlun
 
             info = {'driver_volume_type': 'fibre_channel',
                     'data': {'target_lun': vlun['lun'],

diff --git a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py
index cbf35f470c286cbd36391504bda6e8f87b56faeb..c898ab051c58257904ac58a7ebf86ab1cd8e5187 100644 (file)
--- a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py
+++ b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py
@@ -87,10 +87,11 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver):
         2.0.15 - Added support for updated detach_volume attachment.
         2.0.16 - Added encrypted property to initialize_connection #1439917
         2.0.17 - Python 3 fixes
+        2.0.18 - Improved VLUN creation and deletion logic. #1469816
 
     """
 
-    VERSION = "2.0.17"
+    VERSION = "2.0.18"
 
     def __init__(self, *args, **kwargs):
         super(HP3PARISCSIDriver, self).__init__(*args, **kwargs)
@@ -290,6 +291,7 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver):
                 connector)
 
             least_used_nsp = None
+            existing_vlun = None
             try:
                 vol_name = common._get_3par_vol_name(volume['id'])
                 existing_vlun = common.client.getVLUN(vol_name)
@@ -309,8 +311,12 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver):
                     common,
                     host['name'])
 
-            # now that we have a host, create the VLUN
-            vlun = common.create_vlun(volume, host, least_used_nsp)
+            vlun = None
+            if not existing_vlun or host['name'] != existing_vlun['hostname']:
+                # now that we have a host, create the VLUN
+                vlun = common.create_vlun(volume, host, least_used_nsp)
+            else:
+                vlun = existing_vlun
 
             if least_used_nsp is None:
                 LOG.warning(_LW("Least busy iSCSI port not found, "