From 4cb4be4122a44dc99d6f29f065cdd32ae86273ce Mon Sep 17 00:00:00 2001 From: Clinton Knight Date: Fri, 26 Sep 2014 12:07:44 -0400 Subject: [PATCH] Deprecate / obsolete NetApp volume extra specs The NetApp Data ONTAP (Cluster-mode) NFS & iSCSI drivers for Juno support the Cinder pools feature, but the drivers are reporting two qualified extra specs that must be converted to unqualified extra specs in order to be used by the Cinder scheduler's capability filter. Furthermore, there are four extra specs that must be deprecated due to having the pools feature. Warnings will be logged during volume creation if any of the obsolete or deprecated extra specs are seen in the volume type. Change-Id: I4dbd667610e481356304a12b8dae84cff61aa9d9 Closes-bug: 1374630 --- cinder/tests/test_netapp_nfs.py | 35 +++++++++++++++++++ .../tests/volume/drivers/netapp/test_iscsi.py | 35 ++++++++++++++++++- cinder/volume/drivers/netapp/iscsi.py | 8 +++-- cinder/volume/drivers/netapp/nfs.py | 8 +++-- cinder/volume/drivers/netapp/utils.py | 21 +++++++++++ 5 files changed, 100 insertions(+), 7 deletions(-) diff --git a/cinder/tests/test_netapp_nfs.py b/cinder/tests/test_netapp_nfs.py index f059b627a..e9d27e016 100644 --- a/cinder/tests/test_netapp_nfs.py +++ b/cinder/tests/test_netapp_nfs.py @@ -819,6 +819,7 @@ class NetappDirectCmodeNfsDriverOnlyTestCase(test.TestCase): self._driver.ssc_enabled = True self._driver.configuration.netapp_copyoffload_tool_path = 'cof_path' + @mock.patch.object(utils, 'LOG', mock.Mock()) @mock.patch.object(netapp_nfs, 'get_volume_extra_specs') def test_create_volume(self, mock_volume_extra_specs): drv = self._driver @@ -832,6 +833,40 @@ class NetappDirectCmodeNfsDriverOnlyTestCase(test.TestCase): volume_info = self._driver.create_volume(FakeVolume(host, 1)) self.assertEqual(volume_info.get('provider_location'), fake_share) + self.assertEqual(0, utils.LOG.warn.call_count) + + @mock.patch.object(utils, 'LOG', mock.Mock()) + @mock.patch.object(netapp_nfs, 'get_volume_extra_specs') + def test_create_volume_obsolete_extra_spec(self, mock_volume_extra_specs): + drv = self._driver + drv.ssc_enabled = False + extra_specs = {'netapp:raid_type': 'raid4'} + mock_volume_extra_specs.return_value = extra_specs + fake_share = 'localhost:myshare' + host = 'hostname@backend#' + fake_share + with mock.patch.object(drv, '_ensure_shares_mounted'): + with mock.patch.object(drv, '_do_create_volume'): + self._driver.create_volume(FakeVolume(host, 1)) + warn_msg = 'Extra spec netapp:raid_type is obsolete. ' \ + 'Use netapp_raid_type instead.' + utils.LOG.warn.assert_called_once_with(warn_msg) + + @mock.patch.object(utils, 'LOG', mock.Mock()) + @mock.patch.object(netapp_nfs, 'get_volume_extra_specs') + def test_create_volume_deprecated_extra_spec(self, + mock_volume_extra_specs): + drv = self._driver + drv.ssc_enabled = False + extra_specs = {'netapp_thick_provisioned': 'true'} + mock_volume_extra_specs.return_value = extra_specs + fake_share = 'localhost:myshare' + host = 'hostname@backend#' + fake_share + with mock.patch.object(drv, '_ensure_shares_mounted'): + with mock.patch.object(drv, '_do_create_volume'): + self._driver.create_volume(FakeVolume(host, 1)) + warn_msg = 'Extra spec netapp_thick_provisioned is ' \ + 'deprecated. Use netapp_thin_provisioned instead.' + utils.LOG.warn.assert_called_once_with(warn_msg) def test_create_volume_no_pool_specified(self): drv = self._driver diff --git a/cinder/tests/volume/drivers/netapp/test_iscsi.py b/cinder/tests/volume/drivers/netapp/test_iscsi.py index 40e077872..836a9e08b 100644 --- a/cinder/tests/volume/drivers/netapp/test_iscsi.py +++ b/cinder/tests/volume/drivers/netapp/test_iscsi.py @@ -82,7 +82,8 @@ class NetAppDirectISCSIDriverTestCase(test.TestCase): @mock.patch.object(iscsiDriver, 'create_lun', mock.Mock()) @mock.patch.object(iscsiDriver, '_create_lun_handle', mock.Mock()) @mock.patch.object(iscsiDriver, '_add_lun_to_table', mock.Mock()) - @mock.patch.object(na_utils, 'get_volume_extra_specs', + @mock.patch.object(ntap_iscsi, 'LOG', mock.Mock()) + @mock.patch.object(ntap_iscsi, 'get_volume_extra_specs', mock.Mock(return_value=None)) def test_create_volume(self): self.driver.create_volume({'name': 'lun1', 'size': 100, @@ -90,6 +91,7 @@ class NetAppDirectISCSIDriverTestCase(test.TestCase): 'host': 'hostname@backend#vol1'}) self.driver.create_lun.assert_called_once_with( 'vol1', 'lun1', 107374182400, mock.ANY, None) + self.assertEqual(0, ntap_iscsi.LOG.warn.call_count) def test_create_volume_no_pool_provided_by_scheduler(self): self.assertRaises(exception.InvalidHost, self.driver.create_volume, @@ -97,6 +99,37 @@ class NetAppDirectISCSIDriverTestCase(test.TestCase): 'id': uuid.uuid4(), 'host': 'hostname@backend'}) # missing pool + @mock.patch.object(iscsiDriver, 'create_lun', mock.Mock()) + @mock.patch.object(iscsiDriver, '_create_lun_handle', mock.Mock()) + @mock.patch.object(iscsiDriver, '_add_lun_to_table', mock.Mock()) + @mock.patch.object(na_utils, 'LOG', mock.Mock()) + @mock.patch.object(ntap_iscsi, 'get_volume_extra_specs', + mock.Mock(return_value={'netapp:raid_type': 'raid4'})) + def test_create_volume_obsolete_extra_spec(self): + + self.driver.create_volume({'name': 'lun1', 'size': 100, + 'id': uuid.uuid4(), + 'host': 'hostname@backend#vol1'}) + warn_msg = 'Extra spec netapp:raid_type is obsolete. ' \ + 'Use netapp_raid_type instead.' + na_utils.LOG.warn.assert_called_once_with(warn_msg) + + @mock.patch.object(iscsiDriver, 'create_lun', mock.Mock()) + @mock.patch.object(iscsiDriver, '_create_lun_handle', mock.Mock()) + @mock.patch.object(iscsiDriver, '_add_lun_to_table', mock.Mock()) + @mock.patch.object(na_utils, 'LOG', mock.Mock()) + @mock.patch.object(ntap_iscsi, 'get_volume_extra_specs', + mock.Mock(return_value={'netapp_thick_provisioned': + 'true'})) + def test_create_volume_deprecated_extra_spec(self): + + self.driver.create_volume({'name': 'lun1', 'size': 100, + 'id': uuid.uuid4(), + 'host': 'hostname@backend#vol1'}) + warn_msg = 'Extra spec netapp_thick_provisioned is deprecated. ' \ + 'Use netapp_thin_provisioned instead.' + na_utils.LOG.warn.assert_called_once_with(warn_msg) + def test_create_lun(self): expected_path = '/vol/%s/%s' % (self.fake_volume, self.fake_lun) diff --git a/cinder/volume/drivers/netapp/iscsi.py b/cinder/volume/drivers/netapp/iscsi.py index edaadfa0c..70405b0e3 100644 --- a/cinder/volume/drivers/netapp/iscsi.py +++ b/cinder/volume/drivers/netapp/iscsi.py @@ -190,6 +190,9 @@ class NetAppDirectISCSIDriver(driver.ISCSIDriver): qos_policy_group = extra_specs.pop('netapp:qos_policy_group', None) \ if extra_specs else None + # warn on obsolete extra specs + na_utils.log_extra_spec_warnings(extra_specs) + self.create_lun(ontap_volume_name, lun_name, size, metadata, qos_policy_group) LOG.debug('Created LUN with name %s' % lun_name) @@ -1105,9 +1108,8 @@ class NetAppDirectCmodeISCSIDriver(NetAppDirectISCSIDriver): free /= units.Gi pool['free_capacity_gb'] = round_down(free, '0.01') - pool['netapp:raid_type'] = vol.aggr['raid_type'] - pool['netapp:disk_type'] = vol.aggr['disk_type'] - pool['netapp:qos_policy_group'] = vol.qos['qos_policy_group'] + pool['netapp_raid_type'] = vol.aggr['raid_type'] + pool['netapp_disk_type'] = vol.aggr['disk_type'] mirrored = vol in self.ssc_vols['mirrored'] pool['netapp_mirrored'] = six.text_type(mirrored).lower() diff --git a/cinder/volume/drivers/netapp/nfs.py b/cinder/volume/drivers/netapp/nfs.py index 5980c4a94..c74f8551e 100644 --- a/cinder/volume/drivers/netapp/nfs.py +++ b/cinder/volume/drivers/netapp/nfs.py @@ -812,6 +812,9 @@ class NetAppDirectCmodeNfsDriver (NetAppDirectNfsDriver): qos_policy_group = extra_specs.pop('netapp:qos_policy_group', None) \ if extra_specs else None + # warn on obsolete extra specs + na_utils.log_extra_spec_warnings(extra_specs) + try: volume['provider_location'] = share LOG.info(_('casted to %s') % volume['provider_location']) @@ -994,9 +997,8 @@ class NetAppDirectCmodeNfsDriver (NetAppDirectNfsDriver): # add SSC content if available vol = self._get_vol_for_share(nfs_share) if vol and self.ssc_vols: - pool['netapp:raid_type'] = vol.aggr['raid_type'] - pool['netapp:disk_type'] = vol.aggr['disk_type'] - pool['netapp:qos_policy_group'] = vol.qos['qos_policy_group'] + pool['netapp_raid_type'] = vol.aggr['raid_type'] + pool['netapp_disk_type'] = vol.aggr['disk_type'] mirrored = vol in self.ssc_vols['mirrored'] pool['netapp_mirrored'] = six.text_type(mirrored).lower() diff --git a/cinder/volume/drivers/netapp/utils.py b/cinder/volume/drivers/netapp/utils.py index 10cea03e6..6a6bfc0e1 100644 --- a/cinder/volume/drivers/netapp/utils.py +++ b/cinder/volume/drivers/netapp/utils.py @@ -45,6 +45,14 @@ from cinder.volume import volume_types LOG = logging.getLogger(__name__) +OBSOLETE_SSC_SPECS = {'netapp:raid_type': 'netapp_raid_type', + 'netapp:disk_type': 'netapp_disk_type'} +DEPRECATED_SSC_SPECS = {'netapp_unmirrored': 'netapp_mirrored', + 'netapp_nodedup': 'netapp_dedup', + 'netapp_nocompression': 'netapp_compression', + 'netapp_thick_provisioned': 'netapp_thin_provisioned'} + + def provide_ems(requester, server, stats, netapp_backend, server_type="cluster"): """Provide ems with volume stats for the requester. @@ -364,3 +372,16 @@ def convert_es_fmt_to_uuid(es_label): def round_down(value, precision): return float(decimal.Decimal(six.text_type(value)).quantize( decimal.Decimal(precision), rounding=decimal.ROUND_DOWN)) + + +def log_extra_spec_warnings(extra_specs): + for spec in (set(extra_specs.keys() if extra_specs else []) & + set(OBSOLETE_SSC_SPECS.keys())): + msg = _('Extra spec %(old)s is obsolete. Use %(new)s instead.') + args = {'old': spec, 'new': OBSOLETE_SSC_SPECS[spec]} + LOG.warn(msg % args) + for spec in (set(extra_specs.keys() if extra_specs else []) & + set(DEPRECATED_SSC_SPECS.keys())): + msg = _('Extra spec %(old)s is deprecated. Use %(new)s instead.') + args = {'old': spec, 'new': DEPRECATED_SSC_SPECS[spec]} + LOG.warn(msg % args) -- 2.45.2