From 6b30d9e818f87a6eabbd6a3c199ce34c3e23c070 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Fri, 12 Feb 2021 21:25:43 -0800 Subject: [PATCH] relinker: Pass whole conf dicts around This lets the diskfile managers have access to things like reclaim age, which it important now that we're doing rehashing. While we're at it, pass better log_level values around. Change-Id: Ibe1d18d94f9d5af7654a41e2086ea1a7b42e0750 --- swift/cli/relinker.py | 70 ++++++++++++++-------------------- test/unit/cli/test_relinker.py | 62 +++++++++++++++++++++--------- 2 files changed, 74 insertions(+), 58 deletions(-) diff --git a/swift/cli/relinker.py b/swift/cli/relinker.py index 470f377de9..1eee000ebf 100644 --- a/swift/cli/relinker.py +++ b/swift/cli/relinker.py @@ -245,14 +245,7 @@ def determine_exit_code(logger, found_policy, processed, action, action_errors, return EXIT_SUCCESS -def relink(swift_dir='/etc/swift', - devices='/srv/node', - skip_mount_check=False, - logger=logging.getLogger(), - device=None, - files_per_second=0): - mount_check = not skip_mount_check - conf = {'devices': devices, 'mount_check': mount_check} +def relink(conf, logger, device): diskfile_router = diskfile.DiskFileRouter(conf, logger) found_policy = False relinked = errors = 0 @@ -260,13 +253,13 @@ def relink(swift_dir='/etc/swift', for policy in POLICIES: diskfile_mgr = diskfile_router[policy] policy.object_ring = None # Ensure it will be reloaded - policy.load_ring(swift_dir) + policy.load_ring(conf['swift_dir']) part_power = policy.object_ring.part_power next_part_power = policy.object_ring.next_part_power if not next_part_power or next_part_power == part_power: continue logger.info('Relinking files for policy %s under %s', - policy.name, devices) + policy.name, conf['devices']) found_policy = True datadir = diskfile.get_data_dir(policy) @@ -287,9 +280,9 @@ def relink(swift_dir='/etc/swift', relink_hashes_filter = partial(hashes_filter, next_part_power) locations = audit_location_generator( - devices, + conf['devices'], datadir, - mount_check=mount_check, + mount_check=conf['mount_check'], devices_filter=relink_devices_filter, hook_pre_device=relink_hook_pre_device, hook_post_device=relink_hook_post_device, @@ -297,8 +290,9 @@ def relink(swift_dir='/etc/swift', hook_post_partition=relink_hook_post_partition, hashes_filter=relink_hashes_filter, logger=logger, error_counter=error_counter) - if files_per_second > 0: - locations = RateLimitedIterator(locations, files_per_second) + if conf['files_per_second'] > 0: + locations = RateLimitedIterator( + locations, conf['files_per_second']) for fname, _, _ in locations: newfname = replace_partition_in_path(fname, next_part_power) try: @@ -319,14 +313,7 @@ def relink(swift_dir='/etc/swift', ) -def cleanup(swift_dir='/etc/swift', - devices='/srv/node', - skip_mount_check=False, - logger=logging.getLogger(), - device=None, - files_per_second=0): - mount_check = not skip_mount_check - conf = {'devices': devices, 'mount_check': mount_check} +def cleanup(conf, logger, device): diskfile_router = diskfile.DiskFileRouter(conf, logger) errors = cleaned_up = 0 error_counter = {} @@ -334,13 +321,13 @@ def cleanup(swift_dir='/etc/swift', for policy in POLICIES: diskfile_mgr = diskfile_router[policy] policy.object_ring = None # Ensure it will be reloaded - policy.load_ring(swift_dir) + policy.load_ring(conf['swift_dir']) part_power = policy.object_ring.part_power next_part_power = policy.object_ring.next_part_power if not next_part_power or next_part_power != part_power: continue logger.info('Cleaning up files for policy %s under %s', - policy.name, devices) + policy.name, conf['devices']) found_policy = True datadir = diskfile.get_data_dir(policy) @@ -361,9 +348,9 @@ def cleanup(swift_dir='/etc/swift', cleanup_hashes_filter = partial(hashes_filter, next_part_power) locations = audit_location_generator( - devices, + conf['devices'], datadir, - mount_check=mount_check, + mount_check=conf['mount_check'], devices_filter=cleanup_devices_filter, hook_pre_device=cleanup_hook_pre_device, hook_post_device=cleanup_hook_post_device, @@ -371,8 +358,9 @@ def cleanup(swift_dir='/etc/swift', hook_post_partition=cleanup_hook_post_partition, hashes_filter=cleanup_hashes_filter, logger=logger, error_counter=error_counter) - if files_per_second > 0: - locations = RateLimitedIterator(locations, files_per_second) + if conf['files_per_second'] > 0: + locations = RateLimitedIterator( + locations, conf['files_per_second']) for fname, device, partition in locations: expected_fname = replace_partition_in_path(fname, part_power) if fname == expected_fname: @@ -463,29 +451,29 @@ def main(args): drop_privileges(user) logger = get_logger(conf) else: - conf = {} + conf = {'log_level': 'DEBUG' if args.debug else 'INFO'} if args.user: # Drop privs before creating log file drop_privileges(args.user) + conf['user'] = args.user logging.basicConfig( format='%(message)s', level=logging.DEBUG if args.debug else logging.INFO, filename=args.logfile) logger = logging.getLogger() - swift_dir = args.swift_dir or conf.get('swift_dir', '/etc/swift') - devices = args.devices or conf.get('devices', '/srv/node') - skip_mount_check = args.skip_mount_check or not config_true_value( - conf.get('mount_check', 'true')) - files_per_second = non_negative_float( - args.files_per_second or conf.get('files_per_second', '0')) + conf.update({ + 'swift_dir': args.swift_dir or conf.get('swift_dir', '/etc/swift'), + 'devices': args.devices or conf.get('devices', '/srv/node'), + 'mount_check': (config_true_value(conf.get('mount_check', 'true')) + and not args.skip_mount_check), + 'files_per_second': ( + args.files_per_second if args.files_per_second is not None + else non_negative_float(conf.get('files_per_second', '0'))), + }) if args.action == 'relink': - return relink( - swift_dir, devices, skip_mount_check, logger, device=args.device, - files_per_second=files_per_second) + return relink(conf, logger, device=args.device) if args.action == 'cleanup': - return cleanup( - swift_dir, devices, skip_mount_check, logger, device=args.device, - files_per_second=files_per_second) + return cleanup(conf, logger, device=args.device) diff --git a/test/unit/cli/test_relinker.py b/test/unit/cli/test_relinker.py index 1b3bc51855..35d49e9912 100644 --- a/test/unit/cli/test_relinker.py +++ b/test/unit/cli/test_relinker.py @@ -170,6 +170,7 @@ class TestRelinker(unittest.TestCase): swift_dir = test/swift/dir devices = /test/node mount_check = false + reclaim_age = 5184000 [object-relinker] log_level = WARNING @@ -182,10 +183,17 @@ class TestRelinker(unittest.TestCase): # cite conf file on command line with mock.patch('swift.cli.relinker.relink') as mock_relink: relinker.main(['relink', conf_file, '--device', 'sdx', '--debug']) - mock_relink.assert_called_once_with( - 'test/swift/dir', '/test/node', True, mock.ANY, device='sdx', - files_per_second=0.0) - logger = mock_relink.call_args[0][3] + mock_relink.assert_called_once_with({ + '__file__': mock.ANY, + 'swift_dir': 'test/swift/dir', + 'devices': '/test/node', + 'mount_check': False, + 'reclaim_age': '5184000', + 'files_per_second': 0.0, + 'log_name': 'test-relinker', + 'log_level': 'DEBUG', + }, mock.ANY, device='sdx') + logger = mock_relink.call_args[0][1] # --debug overrides conf file self.assertEqual(logging.DEBUG, logger.getEffectiveLevel()) self.assertEqual('test-relinker', logger.logger.name) @@ -206,10 +214,16 @@ class TestRelinker(unittest.TestCase): f.write(dedent(config)) with mock.patch('swift.cli.relinker.relink') as mock_relink: relinker.main(['relink', conf_file, '--device', 'sdx']) - mock_relink.assert_called_once_with( - 'test/swift/dir', '/test/node', False, mock.ANY, device='sdx', - files_per_second=11.1) - logger = mock_relink.call_args[0][3] + mock_relink.assert_called_once_with({ + '__file__': mock.ANY, + 'swift_dir': 'test/swift/dir', + 'devices': '/test/node', + 'mount_check': True, + 'files_per_second': 11.1, + 'log_name': 'test-relinker', + 'log_level': 'WARNING', + }, mock.ANY, device='sdx') + logger = mock_relink.call_args[0][1] self.assertEqual(logging.WARNING, logger.getEffectiveLevel()) self.assertEqual('test-relinker', logger.logger.name) @@ -219,18 +233,28 @@ class TestRelinker(unittest.TestCase): 'relink', conf_file, '--device', 'sdx', '--debug', '--swift-dir', 'cli-dir', '--devices', 'cli-devs', '--skip-mount-check', '--files-per-second', '2.2']) - mock_relink.assert_called_once_with( - 'cli-dir', 'cli-devs', True, mock.ANY, device='sdx', - files_per_second=2.2) + mock_relink.assert_called_once_with({ + '__file__': mock.ANY, + 'swift_dir': 'cli-dir', + 'devices': 'cli-devs', + 'mount_check': False, + 'files_per_second': 2.2, + 'log_level': 'DEBUG', + 'log_name': 'test-relinker', + }, mock.ANY, device='sdx') with mock.patch('swift.cli.relinker.relink') as mock_relink, \ mock.patch('logging.basicConfig') as mock_logging_config: relinker.main(['relink', '--device', 'sdx', '--swift-dir', 'cli-dir', '--devices', 'cli-devs', '--skip-mount-check']) - mock_relink.assert_called_once_with( - 'cli-dir', 'cli-devs', True, mock.ANY, device='sdx', - files_per_second=0.0) + mock_relink.assert_called_once_with({ + 'swift_dir': 'cli-dir', + 'devices': 'cli-devs', + 'mount_check': False, + 'files_per_second': 0.0, + 'log_level': 'INFO', + }, mock.ANY, device='sdx') mock_logging_config.assert_called_once_with( format='%(message)s', level=logging.INFO, filename=None) @@ -239,9 +263,13 @@ class TestRelinker(unittest.TestCase): relinker.main(['relink', '--device', 'sdx', '--debug', '--swift-dir', 'cli-dir', '--devices', 'cli-devs', '--skip-mount-check']) - mock_relink.assert_called_once_with( - 'cli-dir', 'cli-devs', True, mock.ANY, device='sdx', - files_per_second=0.0) + mock_relink.assert_called_once_with({ + 'swift_dir': 'cli-dir', + 'devices': 'cli-devs', + 'mount_check': False, + 'files_per_second': 0.0, + 'log_level': 'DEBUG', + }, mock.ANY, device='sdx') # --debug is now effective mock_logging_config.assert_called_once_with( format='%(message)s', level=logging.DEBUG, filename=None)