From 1b7dd34d38c26c3d26c98341499973cc87bad2e1 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Mon, 1 Feb 2021 21:04:36 -0800 Subject: [PATCH] relinker: Allow conf files for configuration Swap out the standard logger stuff in place of --logfile. Keep --device as a CLI-only option. Everything else is pretty standard stuff that ought to be in [DEFAULT]. Co-Authored-By: Alistair Coles Change-Id: I32f979f068592eaac39dcc6807b3114caeaaa814 --- doc/saio/swift/object-server/1.conf | 2 + doc/saio/swift/object-server/2.conf | 2 + doc/saio/swift/object-server/3.conf | 2 + doc/saio/swift/object-server/4.conf | 2 + etc/object-server.conf-sample | 7 ++ swift/cli/relinker.py | 42 +++++++---- test/probe/test_object_partpower_increase.py | 19 ++--- test/unit/cli/test_relinker.py | 78 ++++++++++++++++++++ 8 files changed, 125 insertions(+), 29 deletions(-) diff --git a/doc/saio/swift/object-server/1.conf b/doc/saio/swift/object-server/1.conf index 7405dddf91..ecd5ff01c9 100644 --- a/doc/saio/swift/object-server/1.conf +++ b/doc/saio/swift/object-server/1.conf @@ -30,3 +30,5 @@ rsync_module = {replication_ip}::object{replication_port} [object-updater] [object-auditor] + +[object-relinker] diff --git a/doc/saio/swift/object-server/2.conf b/doc/saio/swift/object-server/2.conf index 2b6bc0358f..456f7d5586 100644 --- a/doc/saio/swift/object-server/2.conf +++ b/doc/saio/swift/object-server/2.conf @@ -30,3 +30,5 @@ rsync_module = {replication_ip}::object{replication_port} [object-updater] [object-auditor] + +[object-relinker] diff --git a/doc/saio/swift/object-server/3.conf b/doc/saio/swift/object-server/3.conf index 6ef04ee59b..9a0ebbdca0 100644 --- a/doc/saio/swift/object-server/3.conf +++ b/doc/saio/swift/object-server/3.conf @@ -30,3 +30,5 @@ rsync_module = {replication_ip}::object{replication_port} [object-updater] [object-auditor] + +[object-relinker] diff --git a/doc/saio/swift/object-server/4.conf b/doc/saio/swift/object-server/4.conf index 3fbf83c6fd..1c0db1ff51 100644 --- a/doc/saio/swift/object-server/4.conf +++ b/doc/saio/swift/object-server/4.conf @@ -30,3 +30,5 @@ rsync_module = {replication_ip}::object{replication_port} [object-updater] [object-auditor] + +[object-relinker] diff --git a/etc/object-server.conf-sample b/etc/object-server.conf-sample index 1e1d3b6eed..2bb53c2192 100644 --- a/etc/object-server.conf-sample +++ b/etc/object-server.conf-sample @@ -602,3 +602,10 @@ use = egg:swift#xprofile # # unwind the iterator of applications # unwind = false + +[object-relinker] +# You can override the default log routing for this app here (don't use set!): +# log_name = object-relinker +# log_facility = LOG_LOCAL0 +# log_level = INFO +# log_address = /dev/log diff --git a/swift/cli/relinker.py b/swift/cli/relinker.py index 6c74cc5312..76079140db 100644 --- a/swift/cli/relinker.py +++ b/swift/cli/relinker.py @@ -24,8 +24,8 @@ from functools import partial from swift.common.storage_policy import POLICIES from swift.common.exceptions import DiskFileDeleted, DiskFileNotExist, \ DiskFileQuarantined -from swift.common.utils import replace_partition_in_path, \ - audit_location_generator +from swift.common.utils import replace_partition_in_path, config_true_value, \ + audit_location_generator, get_logger, readconf from swift.obj import diskfile @@ -327,35 +327,45 @@ def main(args): parser = argparse.ArgumentParser( description='Relink and cleanup objects to increase partition power') parser.add_argument('action', choices=['relink', 'cleanup']) - parser.add_argument('--swift-dir', default='/etc/swift', + parser.add_argument('conf_file', nargs='?', help=( + 'Path to config file with [object-relinker] section')) + parser.add_argument('--swift-dir', default=None, dest='swift_dir', help='Path to swift directory') - parser.add_argument('--devices', default='/srv/node', + parser.add_argument('--devices', default=None, dest='devices', help='Path to swift device directory') parser.add_argument('--device', default=None, dest='device', help='Device name to relink (default: all)') parser.add_argument('--skip-mount-check', default=False, help='Don\'t test if disk is mounted', action="store_true", dest='skip_mount_check') - parser.add_argument('--logfile', default=None, - dest='logfile', help='Set log file name') + parser.add_argument('--logfile', default=None, dest='logfile', + help='Set log file name. Ignored if using conf_file.') parser.add_argument('--debug', default=False, action='store_true', help='Enable debug mode') args = parser.parse_args(args) + if args.conf_file: + conf = readconf(args.conf_file, 'object-relinker') + if args.debug: + conf['log_level'] = 'DEBUG' + logger = get_logger(conf) + else: + conf = {} + logging.basicConfig( + format='%(message)s', + level=logging.DEBUG if args.debug else logging.INFO, + filename=args.logfile) + logger = logging.getLogger() - 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')) if args.action == 'relink': return relink( - args.swift_dir, args.devices, args.skip_mount_check, logger, - device=args.device) + swift_dir, devices, skip_mount_check, logger, device=args.device) if args.action == 'cleanup': return cleanup( - args.swift_dir, args.devices, args.skip_mount_check, logger, - device=args.device) + swift_dir, devices, skip_mount_check, logger, device=args.device) diff --git a/test/probe/test_object_partpower_increase.py b/test/probe/test_object_partpower_increase.py index 60425c0846..569d2ece2a 100755 --- a/test/probe/test_object_partpower_increase.py +++ b/test/probe/test_object_partpower_increase.py @@ -25,7 +25,7 @@ from uuid import uuid4 from swiftclient import client from swift.cli.relinker import main as relinker_main -from swift.common.manager import Manager +from swift.common.manager import Manager, Server from swift.common.ring import RingBuilder from swift.common.utils import replace_partition_in_path from swift.obj.diskfile import get_data_dir @@ -49,10 +49,7 @@ class TestPartPowerIncrease(ProbeTest): # Ensure the test object will be erasure coded self.data = ' ' * getattr(self.policy, 'ec_segment_size', 1) - self.devices = [ - self.device_dir({'ip': ip, 'port': port, 'device': ''}) - for ip, port in {(dev['ip'], dev['port']) - for dev in self.object_ring.devs}] + self.conf_files = Server('object').conf_files() def tearDown(self): # Keep a backup copy of the modified .builder file @@ -116,10 +113,8 @@ class TestPartPowerIncrease(ProbeTest): client.head_object(self.url, self.token, container, obj) # Relink existing objects - for device in self.devices: - self.assertEqual(0, relinker_main([ - 'relink', '--skip-mount-check', - '--devices', device])) + for conf in self.conf_files: + self.assertEqual(0, relinker_main(['relink', conf])) # Create second object after relinking and ensure it is accessible client.put_object(self.url, self.token, container, obj2, self.data) @@ -165,10 +160,8 @@ class TestPartPowerIncrease(ProbeTest): client.put_object(self.url, self.token, container, obj, self.data) # Cleanup old objects in the wrong location - for device in self.devices: - self.assertEqual(0, relinker_main([ - 'cleanup', '--skip-mount-check', - '--devices', device])) + for conf in self.conf_files: + self.assertEqual(0, relinker_main(['cleanup', conf])) # Ensure objects are still accessible client.head_object(self.url, self.token, container, obj) diff --git a/test/unit/cli/test_relinker.py b/test/unit/cli/test_relinker.py index 3d9b8e909b..24186cd54b 100644 --- a/test/unit/cli/test_relinker.py +++ b/test/unit/cli/test_relinker.py @@ -15,6 +15,9 @@ import binascii import errno import fcntl import json +import logging +from textwrap import dedent + import mock import os import shutil @@ -92,6 +95,81 @@ class TestRelinker(unittest.TestCase): shutil.rmtree(self.testdir, ignore_errors=1) storage_policy.reload_storage_policies() + def test_conf_file(self): + config = """ + [DEFAULT] + swift_dir = test/swift/dir + devices = /test/node + mount_check = false + + [object-relinker] + log_level = WARNING + log_name = test-relinker + """ + conf_file = os.path.join(self.testdir, 'relinker.conf') + with open(conf_file, 'w') as f: + f.write(dedent(config)) + + # 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') + logger = mock_relink.call_args[0][3] + # --debug overrides conf file + self.assertEqual(logging.DEBUG, logger.getEffectiveLevel()) + self.assertEqual('test-relinker', logger.logger.name) + + # flip mount_check, no --debug... + config = """ + [DEFAULT] + swift_dir = test/swift/dir + devices = /test/node + mount_check = true + + [object-relinker] + log_level = WARNING + log_name = test-relinker + """ + with open(conf_file, 'w') as f: + 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') + logger = mock_relink.call_args[0][3] + self.assertEqual(logging.WARNING, logger.getEffectiveLevel()) + self.assertEqual('test-relinker', logger.logger.name) + + # override with cli options... + with mock.patch('swift.cli.relinker.relink') as mock_relink: + relinker.main(['relink', conf_file, '--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') + + 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') + mock_logging_config.assert_called_once_with( + format='%(message)s', level=logging.INFO, filename=None) + + with mock.patch('swift.cli.relinker.relink') as mock_relink, \ + mock.patch('logging.basicConfig') as mock_logging_config: + 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') + # --debug is now effective + mock_logging_config.assert_called_once_with( + format='%(message)s', level=logging.DEBUG, filename=None) + def test_relink(self): self.rb.prepare_increase_partition_power() self._save_ring()