]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Deprecate / obsolete NetApp volume extra specs
authorClinton Knight <cknight@netapp.com>
Fri, 26 Sep 2014 16:07:44 +0000 (12:07 -0400)
committerClinton Knight <cknight@netapp.com>
Wed, 1 Oct 2014 14:49:55 +0000 (10:49 -0400)
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
cinder/tests/volume/drivers/netapp/test_iscsi.py
cinder/volume/drivers/netapp/iscsi.py
cinder/volume/drivers/netapp/nfs.py
cinder/volume/drivers/netapp/utils.py

index f059b627a9c63307460a804dc1218db3c2770dbd..e9d27e016464324db25abf2e4dcb15d516982dad 100644 (file)
@@ -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
index 40e077872d64a17140aeb2982629a540763965d9..836a9e08b520e3a6138c56a0ae0a91956a543bfe 100644 (file)
@@ -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)
 
index edaadfa0c7a1e7503a0902c6621e0e5033a91009..70405b0e3618f25a73f0e64c7aff49f2f6bc7b44 100644 (file)
@@ -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()
index 5980c4a949182b27c1ec1da58d698307d1d56f63..c74f8551effbbda5bf9c68d57789fc8edac3aee1 100644 (file)
@@ -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()
index 10cea03e6dfd1c1ca44fa81911922a01849a2018..6a6bfc0e1d5eac74b4b0c21511e856ecd67ae613 100644 (file)
@@ -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)