From: Alex Meade Date: Wed, 19 Mar 2014 18:03:15 +0000 (-0400) Subject: NetApp cmode iscsi: Fix QOS extra spec X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=7c1fe4a0625274421a62dd14f9957dcc98c4e247;p=openstack-build%2Fcinder-build.git 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 007201cfa..dd9e83f2a 100644 --- 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 1dca09de9..24540c1ce 100644 --- 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