From f713bb135224b45b16f054a9d90448fd0bebce67 Mon Sep 17 00:00:00 2001 From: Sachin Patil Date: Sat, 30 Jul 2016 10:40:03 +0000 Subject: [PATCH] Fix swift-get-nodes arg parsing for missing ring - Verify .ring.gz path exist if ring file is the first argument. - Code Refactoring: - swift/cli/info.parse_get_node_args() - Respective test cases for info.parse_get_node_args() Closes-Bug: #1539275 Change-Id: I0a41936d6b75c60336be76f8702fd616d74f1545 Signed-off-by: Sachin Patil --- bin/swift-get-nodes | 45 ++--- swift/cli/info.py | 32 +++ test/unit/cli/test_info.py | 388 ++++++++++++++++++++++++++++++++++++- 3 files changed, 427 insertions(+), 38 deletions(-) mode change 100755 => 100644 bin/swift-get-nodes diff --git a/bin/swift-get-nodes b/bin/swift-get-nodes old mode 100755 new mode 100644 index b25002ad..9d095174 --- a/bin/swift-get-nodes +++ b/bin/swift-get-nodes @@ -15,11 +15,12 @@ # limitations under the License. import sys -import os from optparse import OptionParser +from os.path import basename from swift.common.ring import Ring -from swift.cli.info import print_item_locations, InfoSystemExit +from swift.cli.info import (parse_get_node_args, print_item_locations, + InfoSystemExit) if __name__ == '__main__': @@ -27,8 +28,9 @@ if __name__ == '__main__': usage = ''' Shows the nodes responsible for the item specified. Usage: %prog [-a] [ []] - Or: %prog [-a] -p partition - Or: %prog [-a] -P policy_name + Or: %prog [-a] -p partition + Or: %prog [-a] -P policy_name [ []] + Or: %prog [-a] -P policy_name -p partition Note: account, container, object can also be a single arg separated by / Example: $ %prog -a /etc/swift/account.ring.gz MyAccount @@ -49,33 +51,16 @@ if __name__ == '__main__': parser.add_option('-d', '--swift-dir', default='/etc/swift', dest='swift_dir', help='Path to swift directory') options, args = parser.parse_args() + try: + ring_path, args = parse_get_node_args(options, args) + except InfoSystemExit as e: + parser.print_help() + sys.exit('ERROR: %s' % e) - # swift-get-nodes -P nada -p 1 - if len(args) == 0: - if not options.policy_name or not options.partition: - sys.exit(parser.print_help()) - elif len(args) > 4 or len(args) < 1: - sys.exit(parser.print_help()) - - # Parse single path arg, as noted in above help text. - # if len(args) == 1 and options.policy_name and '/' in args[0]: - if len(args) == 1 and not args[0].endswith('ring.gz'): - path = args[0].lstrip('/') - args = [p for p in path.split('/', 2) if p] - if len(args) == 2 and '/' in args[1]: - path = args[1].lstrip('/') - args = [args[0]] + [p for p in path.split('/', 2) if p] - - ring = None - ring_name = None - - if len(args) >= 1 and args[0].endswith('ring.gz'): - if os.path.exists(args[0]): - ring_name = args[0].rsplit('/', 1)[-1].split('.', 1)[0] - ring = Ring(args[0]) - else: - print('Ring file does not exist') - args.pop(0) + ring = ring_name = None + if ring_path: + ring_name = basename(ring_path)[:len('ring.gz')] + ring = Ring(ring_path) try: print_item_locations(ring, ring_name, *args, **vars(options)) diff --git a/swift/cli/info.py b/swift/cli/info.py index 55bfb536..c61ee2fe 100644 --- a/swift/cli/info.py +++ b/swift/cli/info.py @@ -38,6 +38,38 @@ class InfoSystemExit(Exception): pass +def parse_get_node_args(options, args): + """ + Parse the get_nodes commandline args + + :returns: a tuple, (ring_path, args) + """ + ring_path = None + + if options.policy_name: + if POLICIES.get_by_name(options.policy_name) is None: + raise InfoSystemExit('No policy named %r' % options.policy_name) + elif args and args[0].endswith('ring.gz'): + if os.path.exists(args[0]): + ring_path = args.pop(0) + else: + raise InfoSystemExit('Ring file does not exist') + + if len(args) == 1: + args = args[0].strip('/').split('/', 2) + + if not ring_path and not options.policy_name: + raise InfoSystemExit('Need to specify policy_name or ') + + if not (args or options.partition): + raise InfoSystemExit('No target specified') + + if len(args) > 3: + raise InfoSystemExit('Invalid arguments') + + return ring_path, args + + def curl_head_command(ip, port, device, part, target, policy_index): """ Provide a string that is a well formatted curl command to HEAD an object diff --git a/test/unit/cli/test_info.py b/test/unit/cli/test_info.py index 58af8d79..835029a9 100644 --- a/test/unit/cli/test_info.py +++ b/test/unit/cli/test_info.py @@ -12,6 +12,7 @@ """Tests for swift.cli.info""" +from argparse import Namespace import os import unittest import mock @@ -24,9 +25,10 @@ from test.unit import patch_policies, write_fake_ring from swift.common import ring, utils from swift.common.swob import Request from swift.common.storage_policy import StoragePolicy, POLICIES -from swift.cli.info import print_db_info_metadata, print_ring_locations, \ - print_info, print_obj_metadata, print_obj, InfoSystemExit, \ - print_item_locations +from swift.cli.info import (print_db_info_metadata, print_ring_locations, + print_info, print_obj_metadata, print_obj, + InfoSystemExit, print_item_locations, + parse_get_node_args) from swift.account.server import AccountController from swift.container.server import ContainerController from swift.obj.diskfile import write_metadata @@ -86,12 +88,9 @@ class TestCliInfoBase(unittest.TestCase): rmtree(os.path.dirname(self.testdir)) def assertRaisesMessage(self, exc, msg, func, *args, **kwargs): - try: + with self.assertRaises(exc) as ctx: func(*args, **kwargs) - except Exception as e: - self.assertIn(msg, str(e), "Expected %r in %r" % (msg, str(e))) - self.assertIsInstance(e, exc, - "Expected %s, got %s" % (exc, type(e))) + self.assertIn(msg, str(ctx.exception)) class TestCliInfo(TestCliInfoBase): @@ -483,6 +482,379 @@ No user metadata found in db file''' % POLICIES[0].name else: self.fail("Expected an InfoSystemExit exception to be raised") + def test_parse_get_node_args(self): + # Capture error messages + # (without any parameters) + options = Namespace(policy_name=None, partition=None) + args = '' + self.assertRaisesMessage(InfoSystemExit, + 'Need to specify policy_name or ', + parse_get_node_args, options, args.split()) + # a + options = Namespace(policy_name=None, partition=None) + args = 'a' + self.assertRaisesMessage(InfoSystemExit, + 'Need to specify policy_name or ', + parse_get_node_args, options, args.split()) + # a c + options = Namespace(policy_name=None, partition=None) + args = 'a c' + self.assertRaisesMessage(InfoSystemExit, + 'Need to specify policy_name or ', + parse_get_node_args, options, args.split()) + # a c o + options = Namespace(policy_name=None, partition=None) + args = 'a c o' + self.assertRaisesMessage(InfoSystemExit, + 'Need to specify policy_name or ', + parse_get_node_args, options, args.split()) + + # a/c + options = Namespace(policy_name=None, partition=None) + args = 'a/c' + self.assertRaisesMessage(InfoSystemExit, + 'Need to specify policy_name or ', + parse_get_node_args, options, args.split()) + # a/c/o + options = Namespace(policy_name=None, partition=None) + args = 'a/c/o' + self.assertRaisesMessage(InfoSystemExit, + 'Need to specify policy_name or ', + parse_get_node_args, options, args.split()) + + # account container junk/test.ring.gz + options = Namespace(policy_name=None, partition=None) + args = 'account container junk/test.ring.gz' + self.assertRaisesMessage(InfoSystemExit, + 'Need to specify policy_name or ', + parse_get_node_args, options, args.split()) + + # account container object junk/test.ring.gz + options = Namespace(policy_name=None, partition=None) + args = 'account container object junk/test.ring.gz' + self.assertRaisesMessage(InfoSystemExit, + 'Need to specify policy_name or ', + parse_get_node_args, options, args.split()) + + # object.ring.gz(without any arguments i.e. a c o) + options = Namespace(policy_name=None, partition=None) + args = 'object.ring.gz' + self.assertRaisesMessage(InfoSystemExit, + 'Ring file does not exist', + parse_get_node_args, options, args.split()) + + # Valid policy + # -P zero + options = Namespace(policy_name='zero', partition=None) + args = '' + self.assertRaisesMessage(InfoSystemExit, + 'No target specified', + parse_get_node_args, options, args.split()) + # -P one a/c/o + options = Namespace(policy_name='one', partition=None) + args = 'a/c/o' + ring_path, args = parse_get_node_args(options, args.split()) + self.assertIsNone(ring_path) + self.assertEqual(args, ['a', 'c', 'o']) + # -P one account container photos/cat.jpg + options = Namespace(policy_name='one', partition=None) + args = 'account container photos/cat.jpg' + ring_path, args = parse_get_node_args(options, args.split()) + self.assertIsNone(ring_path) + self.assertEqual(args, ['account', 'container', 'photos/cat.jpg']) + # -P one account/container/photos/cat.jpg + options = Namespace(policy_name='one', partition=None) + args = 'account/container/photos/cat.jpg' + ring_path, args = parse_get_node_args(options, args.split()) + self.assertIsNone(ring_path) + self.assertEqual(args, ['account', 'container', 'photos/cat.jpg']) + # -P one account/container/junk/test.ring.gz(object endswith 'ring.gz') + options = Namespace(policy_name='one', partition=None) + args = 'account/container/junk/test.ring.gz' + ring_path, args = parse_get_node_args(options, args.split()) + self.assertIsNone(ring_path) + self.assertEqual(args, ['account', 'container', 'junk/test.ring.gz']) + # -P two a c o hooya + options = Namespace(policy_name='two', partition=None) + args = 'a c o hooya' + self.assertRaisesMessage(InfoSystemExit, + 'Invalid arguments', + parse_get_node_args, options, args.split()) + # -P zero -p 1 + options = Namespace(policy_name='zero', partition='1') + args = '' + ring_path, args = parse_get_node_args(options, args.split()) + self.assertIsNone(ring_path) + self.assertFalse(args) + # -P one -p 1 a/c/o + options = Namespace(policy_name='one', partition='1') + args = 'a/c/o' + ring_path, args = parse_get_node_args(options, args.split()) + self.assertIsNone(ring_path) + self.assertEqual(args, ['a', 'c', 'o']) + # -P two -p 1 a c o hooya + options = Namespace(policy_name='two', partition='1') + args = 'a c o hooya' + self.assertRaisesMessage(InfoSystemExit, + 'Invalid arguments', + parse_get_node_args, options, args.split()) + + # Invalid policy + # -P undefined + options = Namespace(policy_name='undefined') + args = '' + self.assertRaisesMessage(InfoSystemExit, + "No policy named 'undefined'", + parse_get_node_args, options, args.split()) + # -P undefined -p 1 + options = Namespace(policy_name='undefined', partition='1') + args = '' + self.assertRaisesMessage(InfoSystemExit, + "No policy named 'undefined'", + parse_get_node_args, options, args.split()) + # -P undefined a + options = Namespace(policy_name='undefined') + args = 'a' + self.assertRaisesMessage(InfoSystemExit, + "No policy named 'undefined'", + parse_get_node_args, options, args.split()) + # -P undefined a c + options = Namespace(policy_name='undefined') + args = 'a c' + self.assertRaisesMessage(InfoSystemExit, + "No policy named 'undefined'", + parse_get_node_args, options, args.split()) + # -P undefined a c o + options = Namespace(policy_name='undefined') + args = 'a c o' + self.assertRaisesMessage(InfoSystemExit, + "No policy named 'undefined'", + parse_get_node_args, options, args.split()) + # -P undefined a/c + options = Namespace(policy_name='undefined') + args = 'a/c' + # ring_path, args = parse_get_node_args(options, args.split()) + self.assertRaisesMessage(InfoSystemExit, + "No policy named 'undefined'", + parse_get_node_args, options, args) + # -P undefined a/c/o + options = Namespace(policy_name='undefined') + args = 'a/c/o' + # ring_path, args = parse_get_node_args(options, args.split()) + self.assertRaisesMessage(InfoSystemExit, + "No policy named 'undefined'", + parse_get_node_args, options, args) + + # Mock tests + # /etc/swift/object.ring.gz(without any arguments i.e. a c o) + options = Namespace(policy_name=None, partition=None) + args = '/etc/swift/object.ring.gz' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + self.assertRaisesMessage( + InfoSystemExit, + 'No target specified', + parse_get_node_args, options, args.split()) + # Similar ring_path and arguments + # /etc/swift/object.ring.gz /etc/swift/object.ring.gz + options = Namespace(policy_name=None, partition=None) + args = '/etc/swift/object.ring.gz /etc/swift/object.ring.gz' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, '/etc/swift/object.ring.gz') + self.assertEqual(args, ['etc', 'swift', 'object.ring.gz']) + # /etc/swift/object.ring.gz a/c/etc/swift/object.ring.gz + options = Namespace(policy_name=None, partition=None) + args = '/etc/swift/object.ring.gz a/c/etc/swift/object.ring.gz' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, '/etc/swift/object.ring.gz') + self.assertEqual(args, ['a', 'c', 'etc/swift/object.ring.gz']) + # Invalid path as mentioned in BUG#1539275 + # /etc/swift/object.tar.gz account container object + options = Namespace(policy_name=None, partition=None) + args = '/etc/swift/object.tar.gz account container object' + self.assertRaisesMessage( + InfoSystemExit, + 'Need to specify policy_name or ', + parse_get_node_args, options, args.split()) + + # object.ring.gz a/ + options = Namespace(policy_name=None) + args = 'object.ring.gz a/' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a']) + # object.ring.gz a/c + options = Namespace(policy_name=None) + args = 'object.ring.gz a/c' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a', 'c']) + # object.ring.gz a/c/o + options = Namespace(policy_name=None) + args = 'object.ring.gz a/c/o' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a', 'c', 'o']) + # object.ring.gz a/c/o/junk/test.ring.gz + options = Namespace(policy_name=None) + args = 'object.ring.gz a/c/o/junk/test.ring.gz' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a', 'c', 'o/junk/test.ring.gz']) + # object.ring.gz a + options = Namespace(policy_name=None) + args = 'object.ring.gz a' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a']) + # object.ring.gz a c + options = Namespace(policy_name=None) + args = 'object.ring.gz a c' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a', 'c']) + # object.ring.gz a c o + options = Namespace(policy_name=None) + args = 'object.ring.gz a c o' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a', 'c', 'o']) + # object.ring.gz a c o blah blah + options = Namespace(policy_name=None) + args = 'object.ring.gz a c o blah blah' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + self.assertRaisesMessage( + InfoSystemExit, + 'Invalid arguments', + parse_get_node_args, options, args.split()) + # object.ring.gz a/c/o/blah/blah + options = Namespace(policy_name=None) + args = 'object.ring.gz a/c/o/blah/blah' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a', 'c', 'o/blah/blah']) + + # object.ring.gz -p 1 + options = Namespace(policy_name=None, partition='1') + args = 'object.ring.gz' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertFalse(args) + # object.ring.gz -p 1 a c o + options = Namespace(policy_name=None, partition='1') + args = 'object.ring.gz a c o' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a', 'c', 'o']) + # object.ring.gz -p 1 a c o forth_arg + options = Namespace(policy_name=None, partition='1') + args = 'object.ring.gz a c o forth_arg' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + self.assertRaisesMessage( + InfoSystemExit, + 'Invalid arguments', + parse_get_node_args, options, args.split()) + # object.ring.gz -p 1 a/c/o + options = Namespace(policy_name=None, partition='1') + args = 'object.ring.gz a/c/o' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a', 'c', 'o']) + # object.ring.gz -p 1 a/c/junk/test.ring.gz + options = Namespace(policy_name=None, partition='1') + args = 'object.ring.gz a/c/junk/test.ring.gz' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a', 'c', 'junk/test.ring.gz']) + # object.ring.gz -p 1 a/c/photos/cat.jpg + options = Namespace(policy_name=None, partition='1') + args = 'object.ring.gz a/c/photos/cat.jpg' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a', 'c', 'photos/cat.jpg']) + + # --all object.ring.gz a + options = Namespace(all=True, policy_name=None) + args = 'object.ring.gz a' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a']) + # --all object.ring.gz a c + options = Namespace(all=True, policy_name=None) + args = 'object.ring.gz a c' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a', 'c']) + # --all object.ring.gz a c o + options = Namespace(all=True, policy_name=None) + args = 'object.ring.gz a c o' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['a', 'c', 'o']) + # object.ring.gz account container photos/cat.jpg + options = Namespace(policy_name=None, partition=None) + args = 'object.ring.gz account container photos/cat.jpg' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['account', 'container', 'photos/cat.jpg']) + # object.ring.gz /account/container/photos/cat.jpg + options = Namespace(policy_name=None, partition=None) + args = 'object.ring.gz account/container/photos/cat.jpg' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['account', 'container', 'photos/cat.jpg']) + # Object name ends with 'ring.gz' + # object.ring.gz /account/container/junk/test.ring.gz + options = Namespace(policy_name=None, partition=None) + args = 'object.ring.gz account/container/junk/test.ring.gz' + with mock.patch('swift.cli.info.os.path.exists') as exists: + exists.return_value = True + ring_path, args = parse_get_node_args(options, args.split()) + self.assertEqual(ring_path, 'object.ring.gz') + self.assertEqual(args, ['account', 'container', 'junk/test.ring.gz']) + class TestPrintObj(TestCliInfoBase):