Sync with nova change I135ed85a.

This patch syncs the cinder driver code with that of the nova code,
as we received comments in the review process there.  All changes are
minor: (1) removing unnecessary type checks of the flags, (2) not
using getattr to access the FLAGS, and (3) using the self.flags()
infrastructure that test.TestCase provides.

Change-Id: Id740be6441862f048c3a9267394492a7ed65db84

diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py
index 3aff1cb7cbf84fa3207660149c56eef308280f39..15bda50a0d03c6ca0e18d674684436bbf9c6e443 100644 (file)
--- a/cinder/tests/test_storwize_svc.py
+++ b/cinder/tests/test_storwize_svc.py
@@ -28,11 +28,14 @@ import random
 import socket
 
 from cinder import exception
+from cinder import flags
 from cinder.openstack.common import excutils
 from cinder.openstack.common import log as logging
 from cinder import test
 from cinder.volume import storwize_svc
 
+FLAGS = flags.FLAGS
+
 LOG = logging.getLogger(__name__)
 
 
@@ -914,30 +917,29 @@ class StorwizeSVCDriverTestCase(test.TestCase):
         super(StorwizeSVCDriverTestCase, self).setUp()
         self.USESIM = 1
         if self.USESIM == 1:
+            self.flags(
+                san_ip="hostname",
+                san_login="user",
+                san_password="pass",
+                storwize_svc_flashcopy_timeout="20",
+            )
             self.sim = StorwizeSVCManagementSimulator("volpool")
-            driver = StorwizeSVCFakeDriver()
-            driver.set_fake_storage(self.sim)
-            storwize_svc.FLAGS.san_ip = "hostname"
-            storwize_svc.FLAGS.san_login = "user"
-            storwize_svc.FLAGS.san_password = "pass"
-            storwize_svc.FLAGS.storwize_svc_flashcopy_timeout = "20"
+            self.driver = StorwizeSVCFakeDriver()
+            self.driver.set_fake_storage(self.sim)
         else:
-            driver = storwize_svc.StorwizeSVCDriver()
-            storwize_svc.FLAGS.san_ip = "-1.-1.-1.-1"
-            storwize_svc.FLAGS.san_login = "user"
-            storwize_svc.FLAGS.san_password = "password"
-            storwize_svc.FLAGS.storwize_svc_volpool_name = "pool"
+            self.flags(
+                san_ip="-1.-1.-1.-1",
+                san_login="user",
+                san_password="password",
+                storwize_svc_volpool_name="pool",
+            )
+            self.driver = storwize_svc.StorwizeSVCDriver()
 
-        self.driver = driver
         self.driver.do_setup(None)
         self.driver.check_for_setup_error()
 
-    def _revert_flags(self):
-        for flag in storwize_svc.storwize_svc_opts:
-            setattr(storwize_svc.FLAGS, flag.name, flag.default)
-
     def test_storwize_svc_volume_tests(self):
-        storwize_svc.FLAGS.storwize_svc_vol_rsize = "-1"
+        self.flags(storwize_svc_vol_rsize="-1")
         volume = {}
         volume["name"] = "test1_volume%s" % random.randint(10000, 99999)
         volume["size"] = 10
@@ -949,42 +951,38 @@ class StorwizeSVCDriverTestCase(test.TestCase):
         self.driver.delete_volume(volume)
 
         if self.USESIM == 1:
-            storwize_svc.FLAGS.storwize_svc_vol_rsize = "2%"
-            storwize_svc.FLAGS.storwize_svc_vol_compression = True
+            self.flags(storwize_svc_vol_rsize="2%")
+            self.flags(storwize_svc_vol_compression=True)
             self.driver.create_volume(volume)
             is_volume_defined = self.driver._is_volume_defined(volume["name"])
             self.assertEqual(is_volume_defined, True)
             self.driver.delete_volume(volume)
-            self._revert_flags()
+            FLAGS.reset()
 
     def test_storwize_svc_ip_connectivity(self):
         # Check for missing san_ip
-        storwize_svc.FLAGS.san_ip = None
+        self.flags(san_ip=None)
         self.assertRaises(exception.InvalidInput,
                 self.driver._check_flags)
-        self._revert_flags()
 
         if self.USESIM != 1:
             # Check for invalid ip
-            storwize_svc.FLAGS.san_ip = "-1.-1.-1.-1"
+            self.flags(san_ip="-1.-1.-1.-1")
             self.assertRaises(socket.gaierror,
                         self.driver.check_for_setup_error)
 
             # Check for unreachable IP
-            storwize_svc.FLAGS.san_ip = "1.1.1.1"
+            self.flags(san_ip="1.1.1.1")
             self.assertRaises(socket.error,
                         self.driver.check_for_setup_error)
 
-            self._revert_flags()
-
     def test_storwize_svc_connectivity(self):
         # Make sure we detect if the pool doesn't exist
-        orig_pool = getattr(storwize_svc.FLAGS, "storwize_svc_volpool_name")
         no_exist_pool = "i-dont-exist-%s" % random.randint(10000, 99999)
-        storwize_svc.FLAGS.storwize_svc_volpool_name = no_exist_pool
+        self.flags(storwize_svc_volpool_name=no_exist_pool)
         self.assertRaises(exception.InvalidInput,
                 self.driver.check_for_setup_error)
-        storwize_svc.FLAGS.storwize_svc_volpool_name = orig_pool
+        FLAGS.reset()
 
         # Check the case where the user didn't configure IP addresses
         # as well as receiving unexpected results from the storage
@@ -1006,42 +1004,42 @@ class StorwizeSVCDriverTestCase(test.TestCase):
                     self.driver.check_for_setup_error)
 
         # Check with bad parameters
-        storwize_svc.FLAGS.san_password = None
-        storwize_svc.FLAGS.san_private_key = None
+        self.flags(san_password=None)
+        self.flags(san_private_key=None)
         self.assertRaises(exception.InvalidInput,
                 self.driver._check_flags)
-        self._revert_flags()
+        FLAGS.reset()
 
-        storwize_svc.FLAGS.storwize_svc_vol_rsize = "invalid"
+        self.flags(storwize_svc_vol_rsize="invalid")
         self.assertRaises(exception.InvalidInput,
                 self.driver._check_flags)
-        self._revert_flags()
+        FLAGS.reset()
 
-        storwize_svc.FLAGS.storwize_svc_vol_warning = "invalid"
+        self.flags(storwize_svc_vol_warning="invalid")
         self.assertRaises(exception.InvalidInput,
                 self.driver._check_flags)
-        self._revert_flags()
+        FLAGS.reset()
 
-        storwize_svc.FLAGS.storwize_svc_vol_autoexpand = "invalid"
+        self.flags(storwize_svc_vol_autoexpand="invalid")
         self.assertRaises(exception.InvalidInput,
                 self.driver._check_flags)
-        self._revert_flags()
+        FLAGS.reset()
 
-        storwize_svc.FLAGS.storwize_svc_vol_grainsize = str(42)
+        self.flags(storwize_svc_vol_grainsize=str(42))
         self.assertRaises(exception.InvalidInput,
                 self.driver._check_flags)
-        self._revert_flags()
+        FLAGS.reset()
 
-        storwize_svc.FLAGS.storwize_svc_flashcopy_timeout = str(601)
+        self.flags(storwize_svc_flashcopy_timeout=str(601))
         self.assertRaises(exception.InvalidInput,
                 self.driver._check_flags)
-        self._revert_flags()
+        FLAGS.reset()
 
-        storwize_svc.FLAGS.storwize_svc_vol_compression = True
-        storwize_svc.FLAGS.storwize_svc_vol_rsize = "-1"
+        self.flags(storwize_svc_vol_compression=True)
+        self.flags(storwize_svc_vol_rsize="-1")
         self.assertRaises(exception.InvalidInput,
                 self.driver._check_flags)
-        self._revert_flags()
+        FLAGS.reset()
 
         # Finally, check with good parameters
         self.driver.check_for_setup_error()
@@ -1058,12 +1056,12 @@ class StorwizeSVCDriverTestCase(test.TestCase):
         snapshot["volume_name"] = volume1["name"]
 
         # Test timeout and volume cleanup
-        storwize_svc.FLAGS.storwize_svc_flashcopy_timeout = str(1)
+        self.flags(storwize_svc_flashcopy_timeout=str(1))
         self.assertRaises(exception.InvalidSnapshot,
                 self.driver.create_snapshot, snapshot)
         is_volume_defined = self.driver._is_volume_defined(snapshot["name"])
         self.assertEqual(is_volume_defined, False)
-        self._revert_flags()
+        FLAGS.reset()
 
         # Test bogus statuses
         if self.USESIM == 1:
@@ -1174,7 +1172,7 @@ class StorwizeSVCDriverTestCase(test.TestCase):
         attributes = self.driver._get_volume_attributes(volume["name"])
         attr_size = float(attributes["capacity"]) / 1073741824  # bytes to GB
         self.assertEqual(attr_size, float(volume["size"]))
-        pool = getattr(storwize_svc.FLAGS, "storwize_svc_volpool_name")
+        pool = storwize_svc.FLAGS.storwize_svc_volpool_name
         self.assertEqual(attributes["mdisk_grp_name"], pool)
 
         # Try to create the volume again (should fail)
@@ -1218,20 +1216,20 @@ class StorwizeSVCDriverTestCase(test.TestCase):
         # easytier      False   2
 
         # Test 1
-        storwize_svc.FLAGS.storwize_svc_vol_rsize = "-1"
-        storwize_svc.FLAGS.storwize_svc_vol_easytier = True
+        self.flags(storwize_svc_vol_rsize="-1")
+        self.flags(storwize_svc_vol_easytier=True)
         attrs = self._create_test_vol()
         self.assertEquals(attrs["free_capacity"], "0")
         self.assertEquals(attrs["easy_tier"], "on")
-        self._revert_flags()
+        FLAGS.reset()
 
         # Test 2
-        storwize_svc.FLAGS.storwize_svc_vol_rsize = "2%"
-        storwize_svc.FLAGS.storwize_svc_vol_compression = False
-        storwize_svc.FLAGS.storwize_svc_vol_warning = "0"
-        storwize_svc.FLAGS.storwize_svc_vol_autoexpand = True
-        storwize_svc.FLAGS.storwize_svc_vol_grainsize = "32"
-        storwize_svc.FLAGS.storwize_svc_vol_easytier = False
+        self.flags(storwize_svc_vol_rsize="2%")
+        self.flags(storwize_svc_vol_compression=False)
+        self.flags(storwize_svc_vol_warning="0")
+        self.flags(storwize_svc_vol_autoexpand=True)
+        self.flags(storwize_svc_vol_grainsize="32")
+        self.flags(storwize_svc_vol_easytier=False)
         attrs = self._create_test_vol()
         self.assertNotEqual(attrs["capacity"], attrs["real_capacity"])
         self.assertEquals(attrs["compressed_copy"], "no")
@@ -1239,15 +1237,15 @@ class StorwizeSVCDriverTestCase(test.TestCase):
         self.assertEquals(attrs["autoexpand"], "on")
         self.assertEquals(attrs["grainsize"], "32")
         self.assertEquals(attrs["easy_tier"], "off")
-        self._revert_flags()
+        FLAGS.reset()
 
         # Test 3
-        storwize_svc.FLAGS.storwize_svc_vol_rsize = "2%"
-        storwize_svc.FLAGS.storwize_svc_vol_compression = False
-        storwize_svc.FLAGS.storwize_svc_vol_warning = "80%"
-        storwize_svc.FLAGS.storwize_svc_vol_autoexpand = False
-        storwize_svc.FLAGS.storwize_svc_vol_grainsize = "256"
-        storwize_svc.FLAGS.storwize_svc_vol_easytier = True
+        self.flags(storwize_svc_vol_rsize="2%")
+        self.flags(storwize_svc_vol_compression=False)
+        self.flags(storwize_svc_vol_warning="80%")
+        self.flags(storwize_svc_vol_autoexpand=False)
+        self.flags(storwize_svc_vol_grainsize="256")
+        self.flags(storwize_svc_vol_easytier=True)
         attrs = self._create_test_vol()
         self.assertNotEqual(attrs["capacity"], attrs["real_capacity"])
         self.assertEquals(attrs["compressed_copy"], "no")
@@ -1255,11 +1253,11 @@ class StorwizeSVCDriverTestCase(test.TestCase):
         self.assertEquals(attrs["autoexpand"], "off")
         self.assertEquals(attrs["grainsize"], "256")
         self.assertEquals(attrs["easy_tier"], "on")
-        self._revert_flags()
+        FLAGS.reset()
 
         # Test 4
-        storwize_svc.FLAGS.storwize_svc_vol_rsize = "2%"
-        storwize_svc.FLAGS.storwize_svc_vol_compression = True
+        self.flags(storwize_svc_vol_rsize="2%")
+        self.flags(storwize_svc_vol_compression=True)
         try:
             attrs = self._create_test_vol()
             self.assertNotEqual(attrs["capacity"], attrs["real_capacity"])
@@ -1274,7 +1272,7 @@ class StorwizeSVCDriverTestCase(test.TestCase):
             self.sim.error_injection("mkvdisk", "no_compression")
             self.assertRaises(exception.ProcessExecutionError,
                     self._create_test_vol)
-        self._revert_flags()
+        FLAGS.reset()
 
     def test_storwize_svc_unicode_host_and_volume_names(self):
         volume1 = {}

diff --git a/cinder/volume/storwize_svc.py b/cinder/volume/storwize_svc.py
index 9e0d0e7ae9d1e1f93132f5d6f39011c22fb8f6d5..a5ec8063a07c51f4f41202656fb5025ae983d4d2 100644 (file)
--- a/cinder/volume/storwize_svc.py
+++ b/cinder/volume/storwize_svc.py
@@ -79,7 +79,7 @@ storwize_svc_opts = [
                help='Enable Easy Tier for volumes'),
     cfg.StrOpt('storwize_svc_flashcopy_timeout',
                default='120',
-               help='Maximum number of seconds to wait for FlashCopy to be'
+               help='Maximum number of seconds to wait for FlashCopy to be '
                     'prepared. Maximum value is 600 seconds (10 minutes).'),
 ]
 
@@ -149,11 +149,11 @@ class StorwizeSVCDriver(san.SanISCSIDriver):
                 % {'cmd': ssh_cmd,
                    'out': str(out),
                    'err': str(err)})
-        search_text = '!%s!' % getattr(FLAGS, 'storwize_svc_volpool_name')
+        search_text = '!%s!' % FLAGS.storwize_svc_volpool_name
         if search_text not in out:
             raise exception.InvalidInput(
                     reason=(_('pool %s doesn\'t exist')
-                        % getattr(FLAGS, 'storwize_svc_volpool_name')))
+                        % FLAGS.storwize_svc_volpool_name))
 
         storage_nodes = {}
         # Get the iSCSI names of the Storwize/SVC nodes
@@ -326,38 +326,29 @@ class StorwizeSVCDriver(san.SanISCSIDriver):
                         reason=_('%s is not set') % flag)
 
         # Ensure that either password or keyfile were set
-        if not (getattr(FLAGS, 'san_password', None)
-                or getattr(FLAGS, 'san_private_key', None)):
+        if not (FLAGS.san_password or FLAGS.san_private_key):
             raise exception.InvalidInput(
                 reason=_('Password or SSH private key is required for '
                          'authentication: set either san_password or '
                          'san_private_key option'))
 
         # Check that rsize is a number or percentage
-        rsize = getattr(FLAGS, 'storwize_svc_vol_rsize')
+        rsize = FLAGS.storwize_svc_vol_rsize
         if not self._check_num_perc(rsize) and (rsize != '-1'):
             raise exception.InvalidInput(
                 reason=_('Illegal value specified for storwize_svc_vol_rsize: '
                          'set to either a number or a percentage'))
 
         # Check that warning is a number or percentage
-        warning = getattr(FLAGS, 'storwize_svc_vol_warning')
+        warning = FLAGS.storwize_svc_vol_warning
         if not self._check_num_perc(warning):
             raise exception.InvalidInput(
                 reason=_('Illegal value specified for '
                          'storwize_svc_vol_warning: '
                          'set to either a number or a percentage'))
 
-        # Check that autoexpand is a boolean
-        autoexpand = getattr(FLAGS, 'storwize_svc_vol_autoexpand')
-        if type(autoexpand) != type(True):
-            raise exception.InvalidInput(
-                reason=_('Illegal value specified for '
-                         'storwize_svc_vol_autoexpand: set to either '
-                         'True or False'))
-
         # Check that grainsize is 32/64/128/256
-        grainsize = getattr(FLAGS, 'storwize_svc_vol_grainsize')
+        grainsize = FLAGS.storwize_svc_vol_grainsize
         if grainsize not in ['32', '64', '128', '256']:
             raise exception.InvalidInput(
                 reason=_('Illegal value specified for '
@@ -365,7 +356,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver):
                          '\'32\', \'64\', \'128\', or \'256\''))
 
         # Check that flashcopy_timeout is numeric and 32/64/128/256
-        flashcopy_timeout = getattr(FLAGS, 'storwize_svc_flashcopy_timeout')
+        flashcopy_timeout = FLAGS.storwize_svc_flashcopy_timeout
         if not (flashcopy_timeout.isdigit() and int(flashcopy_timeout) > 0 and
             int(flashcopy_timeout) <= 600):
             raise exception.InvalidInput(
@@ -374,27 +365,14 @@ class StorwizeSVCDriver(san.SanISCSIDriver):
                          'valid values are between 0 and 600')
                                          % flashcopy_timeout)
 
-        # Check that compression is a boolean and that rsize is set
-        volume_compression = getattr(FLAGS, 'storwize_svc_vol_compression')
-        if type(volume_compression) != type(True):
-            raise exception.InvalidInput(
-                reason=_('Illegal value specified for '
-                         'storwize_svc_vol_compression: set to either '
-                         'True or False'))
+        # Check that rsize is set
+        volume_compression = FLAGS.storwize_svc_vol_compression
         if ((volume_compression == True) and
-                (getattr(FLAGS, 'storwize_svc_vol_rsize') == '-1')):
+                (FLAGS.storwize_svc_vol_rsize == '-1')):
             raise exception.InvalidInput(
                 reason=_('If compression is set to True, rsize must '
                          'also be set (not equal to -1)'))
 
-        # Check that easytier is a boolean
-        volume_easytier = getattr(FLAGS, 'storwize_svc_vol_easytier')
-        if type(volume_easytier) != type(True):
-            raise exception.InvalidInput(
-                reason=_('Illegal value specified for '
-                         'storwize_svc_vol_easytier: set to either '
-                         'True or False'))
-
     def do_setup(self, context):
         """Validate the flags."""
         LOG.debug(_('enter: do_setup'))
@@ -415,35 +393,35 @@ class StorwizeSVCDriver(san.SanISCSIDriver):
 
         size = int(volume['size'])
 
-        if getattr(FLAGS, 'storwize_svc_vol_autoexpand') == True:
+        if FLAGS.storwize_svc_vol_autoexpand == True:
             autoex = '-autoexpand'
         else:
             autoex = ''
 
-        if getattr(FLAGS, 'storwize_svc_vol_easytier') == True:
+        if FLAGS.storwize_svc_vol_easytier == True:
             easytier = '-easytier on'
         else:
             easytier = '-easytier off'
 
         # Set space-efficient options
-        if getattr(FLAGS, 'storwize_svc_vol_rsize').strip() == '-1':
+        if FLAGS.storwize_svc_vol_rsize.strip() == '-1':
             ssh_cmd_se_opt = ''
         else:
             ssh_cmd_se_opt = ('-rsize %(rsize)s %(autoex)s -warning %(warn)s' %
-                        {'rsize': getattr(FLAGS, 'storwize_svc_vol_rsize'),
+                        {'rsize': FLAGS.storwize_svc_vol_rsize,
                          'autoex': autoex,
-                         'warn': getattr(FLAGS, 'storwize_svc_vol_warning')})
-            if getattr(FLAGS, 'storwize_svc_vol_compression'):
+                         'warn': FLAGS.storwize_svc_vol_warning})
+            if FLAGS.storwize_svc_vol_compression:
                 ssh_cmd_se_opt = ssh_cmd_se_opt + ' -compressed'
             else:
                 ssh_cmd_se_opt = ssh_cmd_se_opt + (' -grainsize %(grain)s' %
-                       {'grain': getattr(FLAGS, 'storwize_svc_vol_grainsize')})
+                       {'grain': FLAGS.storwize_svc_vol_grainsize})
 
         ssh_cmd = ('mkvdisk -name %(name)s -mdiskgrp %(mdiskgrp)s '
                     '-iogrp 0 -size %(size)s -unit '
                     '%(unit)s %(easytier)s %(ssh_cmd_se_opt)s'
                     % {'name': name,
-                    'mdiskgrp': getattr(FLAGS, 'storwize_svc_volpool_name'),
+                    'mdiskgrp': FLAGS.storwize_svc_volpool_name,
                     'size': size, 'unit': units, 'easytier': easytier,
                     'ssh_cmd_se_opt': ssh_cmd_se_opt})
         out, err = self._run_ssh(ssh_cmd)
@@ -731,8 +709,8 @@ class StorwizeSVCDriver(san.SanISCSIDriver):
         mapping_ready = False
         wait_time = 5
         # Allow waiting of up to timeout (set as parameter)
-        max_retries = (int(getattr(FLAGS,
-                        'storwize_svc_flashcopy_timeout')) / wait_time) + 1
+        max_retries = (int(FLAGS.storwize_svc_flashcopy_timeout)
+                        / wait_time) + 1
         for try_number in range(1, max_retries):
             mapping_attributes = self._get_flashcopy_mapping_attributes(
                                                             fc_map_id)
@@ -759,8 +737,7 @@ class StorwizeSVCDriver(san.SanISCSIDriver):
             exception_msg = (_('mapping %(id)s prepare failed to complete '
                                'within the alloted %(to)s seconds timeout. '
                                'Terminating') % {'id': fc_map_id,
-                               'to': getattr(
-                                FLAGS, 'storwize_svc_flashcopy_timeout')})
+                               'to': FLAGS.storwize_svc_flashcopy_timeout})
             LOG.error(_('_run_flashcopy: fail to start FlashCopy '
                         'from %(source)s to %(target)s with '
                         'exception %(ex)s')