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

@@ -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

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)
 

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()

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()

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)