NetApp cmode iscsi: Fix QOS extra spec

This patch fixes the incorrect behavior where the NetApp cmode
iscsi driver will choose to create a volume on a flexVol that has
a QOS policy group that matches the specified QOS policy group
intended for the cinder volume. The correct behavior is to ignore
the QOS policy group of the flexVol and instead assign the QOS policy
group to the newly created cinder volume.

Change-Id: Ibeb8e3b965af8e79d0294c5cf97410da2b44d40c
Partial-Bug: #1288283

diff --git a/cinder/tests/volume/drivers/netapp/test_iscsi.py b/cinder/tests/volume/drivers/netapp/test_iscsi.py
index 007201cfa8065134682029d94d28a531b716b2f9..dd9e83f2a844845e5ed9de1511261267d81eab0f 100644 (file)
--- a/cinder/tests/volume/drivers/netapp/test_iscsi.py
+++ b/cinder/tests/volume/drivers/netapp/test_iscsi.py
@@ -17,12 +17,78 @@ Mock unit tests for the NetApp iSCSI driver
 """
 
 import mock
+import uuid
 
 from cinder import test
 import cinder.volume.drivers.netapp.api as ntapi
 import cinder.volume.drivers.netapp.iscsi as ntap_iscsi
 
 
+class NetAppDirectISCSIDriverTestCase(test.TestCase):
+
+    def setUp(self):
+        super(NetAppDirectISCSIDriverTestCase, self).setUp()
+        self.driver = ntap_iscsi.NetAppDirectISCSIDriver(
+            configuration=mock.Mock())
+        self.driver.client = mock.Mock()
+        self.fake_volume = str(uuid.uuid4())
+        self.fake_lun = str(uuid.uuid4())
+        self.fake_size = '1024'
+        self.fake_metadata = {
+            'OsType': 'linux',
+            'SpaceReserved': 'true',
+        }
+        self.mock_request = mock.Mock()
+
+    def tearDown(self):
+        super(NetAppDirectISCSIDriverTestCase, self).tearDown()
+
+    def test_create_lun(self):
+        expected_path = '/vol/%s/%s' % (self.fake_volume, self.fake_lun)
+
+        with mock.patch.object(ntapi.NaElement, 'create_node_with_children',
+                               return_value=self.mock_request
+                               ) as mock_create_node:
+            self.driver.create_lun(self.fake_volume,
+                                   self.fake_lun,
+                                   self.fake_size,
+                                   self.fake_metadata)
+
+            mock_create_node.assert_called_once_with(
+                'lun-create-by-size',
+                **{'path': expected_path,
+                   'size': self.fake_size,
+                   'ostype': self.fake_metadata['OsType'],
+                   'space-reservation-enabled':
+                   self.fake_metadata['SpaceReserved']})
+            self.driver.client.invoke_successfully.assert_called_once_with(
+                mock.ANY, True)
+
+    def test_create_lun_with_qos_policy_group(self):
+        expected_path = '/vol/%s/%s' % (self.fake_volume, self.fake_lun)
+        expected_qos_group = 'qos_1'
+
+        with mock.patch.object(ntapi.NaElement, 'create_node_with_children',
+                               return_value=self.mock_request
+                               ) as mock_create_node:
+            self.driver.create_lun(self.fake_volume,
+                                   self.fake_lun,
+                                   self.fake_size,
+                                   self.fake_metadata,
+                                   qos_policy_group=expected_qos_group)
+
+            mock_create_node.assert_called_once_with(
+                'lun-create-by-size',
+                **{'path': expected_path, 'size': self.fake_size,
+                    'ostype': self.fake_metadata['OsType'],
+                    'space-reservation-enabled':
+                    self.fake_metadata['SpaceReserved']})
+            self.mock_request.add_new_child.assert_called_once_with(
+                'qos-policy-group', expected_qos_group)
+            self.driver.client.invoke_successfully.assert_called_once_with(
+                mock.ANY, True)
+
+
 class NetAppiSCSICModeTestCase(test.TestCase):
     """Test case for NetApp's C-Mode iSCSI driver."""
 

diff --git a/cinder/volume/drivers/netapp/iscsi.py b/cinder/volume/drivers/netapp/iscsi.py
index 1dca09de94a4bd901500943393818d44524efda8..24540c1ce2b3935574f999235a8ce0372f18c497 100644 (file)
--- a/cinder/volume/drivers/netapp/iscsi.py
+++ b/cinder/volume/drivers/netapp/iscsi.py
@@ -340,7 +340,7 @@ class NetAppDirectISCSIDriver(driver.ISCSIDriver):
         """Creates an actual lun on filer."""
         raise NotImplementedError()
 
-    def _create_lun(self, volume, lun, size, metadata):
+    def create_lun(self, volume, lun, size, metadata, qos_policy_group=None):
         """Issues api request for creating lun on volume."""
         path = '/vol/%s/%s' % (volume, lun)
         lun_create = NaElement.create_node_with_children(
@@ -348,6 +348,8 @@ class NetAppDirectISCSIDriver(driver.ISCSIDriver):
             **{'path': path, 'size': size,
                 'ostype': metadata['OsType'],
                 'space-reservation-enabled': metadata['SpaceReserved']})
+        if qos_policy_group:
+            lun_create.add_new_child('qos-policy-group', qos_policy_group)
         self.client.invoke_successfully(lun_create, True)
 
     def _get_iscsi_service_details(self):
@@ -682,7 +684,7 @@ class NetAppDirectISCSIDriver(driver.ISCSIDriver):
                         ' as it contains no blocks.')
                 raise exception.VolumeBackendAPIException(data=msg % name)
             new_lun = 'new-%s' % (name)
-            self._create_lun(vol_name, new_lun, new_size_bytes, metadata)
+            self.create_lun(vol_name, new_lun, new_size_bytes, metadata)
             try:
                 self._clone_lun(name, new_lun, block_count=block_count)
                 self._post_sub_clone_resize(path)
@@ -771,6 +773,9 @@ class NetAppDirectCmodeISCSIDriver(NetAppDirectISCSIDriver):
         """Creates an actual lun on filer."""
         req_size = float(size) *\
             float(self.configuration.netapp_size_multiplier)
+        qos_policy_group = None
+        if extra_specs:
+            qos_policy_group = extra_specs.pop('netapp:qos_policy_group', None)
         volumes = self._get_avl_volumes(req_size, extra_specs)
         if not volumes:
             msg = _('Failed to get vol with required'
@@ -778,14 +783,18 @@ class NetAppDirectCmodeISCSIDriver(NetAppDirectISCSIDriver):
             raise exception.VolumeBackendAPIException(data=msg % name)
         for volume in volumes:
             try:
-                self._create_lun(volume.id['name'], name, size, metadata)
+                self.create_lun(volume.id['name'], name, size, metadata,
+                                qos_policy_group=qos_policy_group)
                 metadata['Path'] = '/vol/%s/%s' % (volume.id['name'], name)
                 metadata['Volume'] = volume.id['name']
                 metadata['Qtree'] = None
                 return
-            except NaApiError:
-                LOG.warn(_("Error provisioning vol %(name)s on %(volume)s")
-                         % {'name': name, 'volume': volume.id['name']})
+            except NaApiError as ex:
+                msg = _("Error provisioning vol %(name)s on "
+                        "%(volume)s. Details: %(ex)s")
+                LOG.error(msg % {'name': name,
+                                 'volume': volume.id['name'],
+                                 'ex': ex})
             finally:
                 self._update_stale_vols(volume=volume)
 
@@ -1180,7 +1189,7 @@ class NetAppDirect7modeISCSIDriver(NetAppDirectISCSIDriver):
         if not volume:
             msg = _('Failed to get vol with required size for volume: %s')
             raise exception.VolumeBackendAPIException(data=msg % name)
-        self._create_lun(volume['name'], name, size, metadata)
+        self.create_lun(volume['name'], name, size, metadata)
         metadata['Path'] = '/vol/%s/%s' % (volume['name'], name)
         metadata['Volume'] = volume['name']
         metadata['Qtree'] = None