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 <alistairncoles@gmail.com>
Change-Id: I32f979f068592eaac39dcc6807b3114caeaaa814

doc/saio/swift/object-server/1.conf

@@ -30,3 +30,5 @@ rsync_module = {replication_ip}::object{replication_port}
 [object-updater]
 
 [object-auditor]
+
+[object-relinker]

doc/saio/swift/object-server/2.conf

@@ -30,3 +30,5 @@ rsync_module = {replication_ip}::object{replication_port}
 [object-updater]
 
 [object-auditor]
+
+[object-relinker]

doc/saio/swift/object-server/3.conf

@@ -30,3 +30,5 @@ rsync_module = {replication_ip}::object{replication_port}
 [object-updater]
 
 [object-auditor]
+
+[object-relinker]

doc/saio/swift/object-server/4.conf

@@ -30,3 +30,5 @@ rsync_module = {replication_ip}::object{replication_port}
 [object-updater]
 
 [object-auditor]
+
+[object-relinker]

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

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, \
+from swift.common.utils import replace_partition_in_path, config_true_value, \
-    audit_location_generator
+    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,
+    parser.add_argument('--logfile', default=None, dest='logfile',
-                        dest='logfile', help='Set log file name')
+                        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(
+    swift_dir = args.swift_dir or conf.get('swift_dir', '/etc/swift')
-        format='%(message)s',
+    devices = args.devices or conf.get('devices', '/srv/node')
-        level=logging.DEBUG if args.debug else logging.INFO,
+    skip_mount_check = args.skip_mount_check or not config_true_value(
-        filename=args.logfile)
+        conf.get('mount_check', 'true'))
-
-    logger = logging.getLogger()
 
     if args.action == 'relink':
         return relink(
-            args.swift_dir, args.devices, args.skip_mount_check, logger,
+            swift_dir, devices, skip_mount_check, logger, device=args.device)
-            device=args.device)
 
     if args.action == 'cleanup':
         return cleanup(
-            args.swift_dir, args.devices, args.skip_mount_check, logger,
+            swift_dir, devices, skip_mount_check, logger, device=args.device)
-            device=args.device)

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.conf_files = Server('object').conf_files()
-            self.device_dir({'ip': ip, 'port': port, 'device': ''})
-            for ip, port in {(dev['ip'], dev['port'])
-                             for dev in self.object_ring.devs}]
 
     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:
+        for conf in self.conf_files:
-            self.assertEqual(0, relinker_main([
+            self.assertEqual(0, relinker_main(['relink', conf]))
-                'relink', '--skip-mount-check',
-                '--devices', device]))
 
         # 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:
+        for conf in self.conf_files:
-            self.assertEqual(0, relinker_main([
+            self.assertEqual(0, relinker_main(['cleanup', conf]))
-                'cleanup', '--skip-mount-check',
-                '--devices', device]))
 
         # Ensure objects are still accessible
         client.head_object(self.url, self.token, container, obj)

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()