From 414757d8c336ef791f9654a425693918e1ecd792 Mon Sep 17 00:00:00 2001 From: Avishay Traeger Date: Fri, 21 Sep 2012 15:58:27 +0300 Subject: [PATCH] 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 --- cinder/tests/test_storwize_svc.py | 134 +++++++++++++++--------------- cinder/volume/storwize_svc.py | 67 +++++---------- 2 files changed, 88 insertions(+), 113 deletions(-) diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py index 3aff1cb7c..15bda50a0 100644 --- 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 9e0d0e7ae..a5ec8063a 100644 --- 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') -- 2.45.2