diff --git a/etc/object-server.conf-sample b/etc/object-server.conf-sample index 762037178c..545554ddd3 100644 --- a/etc/object-server.conf-sample +++ b/etc/object-server.conf-sample @@ -638,3 +638,6 @@ use = egg:swift#xprofile # tombstones from their old locations, causing duplicate tombstones with # different inodes to be relinked to the next partition power location. # link_check_limit = 2 +# +# stats_interval = 300.0 +# recon_cache_path = /var/cache/swift diff --git a/swift/cli/relinker.py b/swift/cli/relinker.py index d65d684ec5..08c11dbd8f 100644 --- a/swift/cli/relinker.py +++ b/swift/cli/relinker.py @@ -22,12 +22,14 @@ import json import logging import os import time +from collections import defaultdict from swift.common.storage_policy import POLICIES from swift.common.utils import replace_partition_in_path, config_true_value, \ audit_location_generator, get_logger, readconf, drop_privileges, \ RateLimitedIterator, lock_path, PrefixLoggerAdapter, distribute_evenly, \ - non_negative_float, non_negative_int, config_auto_int_value + non_negative_float, non_negative_int, config_auto_int_value, \ + dump_recon_cache from swift.obj import diskfile @@ -39,6 +41,13 @@ STEP_CLEANUP = 'cleanup' EXIT_SUCCESS = 0 EXIT_NO_APPLICABLE_POLICY = 2 EXIT_ERROR = 1 +RECON_FILE = 'relinker.recon' +DEFAULT_RECON_CACHE_PATH = '/var/cache/swift' +DEFAULT_STATS_INTERVAL = 300.0 + + +def recursive_defaultdict(): + return defaultdict(recursive_defaultdict) def policy(policy_name_or_index): @@ -48,9 +57,50 @@ def policy(policy_name_or_index): return value +def _aggregate_stats(base_stats, update_stats): + for key, value in update_stats.items(): + base_stats.setdefault(key, 0) + base_stats[key] += value + + return base_stats + + +def _aggregate_recon_stats(base_stats, updated_stats): + for k, v in updated_stats.items(): + if k == 'stats': + base_stats['stats'] = _aggregate_stats(base_stats['stats'], v) + elif k == "start_time": + base_stats[k] = min(base_stats.get(k, v), v) + elif k in ("timestamp", "total_time"): + base_stats[k] = max(base_stats.get(k, 0), v) + elif k in ('parts_done', 'total_parts'): + base_stats[k] += v + + return base_stats + + +def _zero_stats(): + return { + 'hash_dirs': 0, + 'files': 0, + 'linked': 0, + 'removed': 0, + 'errors': 0} + + +def _zero_collated_stats(): + return { + 'parts_done': 0, + 'total_parts': 0, + 'total_time': 0, + 'stats': _zero_stats()} + + class Relinker(object): def __init__(self, conf, logger, device_list=None, do_cleanup=False): self.conf = conf + self.recon_cache = os.path.join(self.conf['recon_cache_path'], + RECON_FILE) self.logger = logger self.device_list = device_list or [] self.do_cleanup = do_cleanup @@ -60,26 +110,65 @@ class Relinker(object): self.part_power = self.next_part_power = None self.diskfile_mgr = None self.dev_lock = None + self._last_recon_update = time.time() + self.stats_interval = float(conf.get( + 'stats_interval', DEFAULT_STATS_INTERVAL)) self.diskfile_router = diskfile.DiskFileRouter(self.conf, self.logger) - self._zero_stats() + self.stats = _zero_stats() + self.devices_data = recursive_defaultdict() + self.policy_count = 0 + self.pid = os.getpid() - def _zero_stats(self): - self.stats = { - 'hash_dirs': 0, - 'files': 0, - 'linked': 0, - 'removed': 0, - 'errors': 0, - 'policies': 0, - } + def _aggregate_dev_policy_stats(self): + for dev_data in self.devices_data.values(): + dev_data.update(_zero_collated_stats()) + for policy_data in dev_data.get('policies', {}).values(): + _aggregate_recon_stats(dev_data, policy_data) + + def _update_recon(self, device=None, force_dump=False): + if not force_dump and self._last_recon_update + self.stats_interval \ + > time.time(): + # not time yet! + return + if device: + # dump recon stats for the device + num_parts_done = sum( + 1 for part_done in self.states["state"].values() + if part_done) + num_total_parts = len(self.states["state"]) + step = STEP_CLEANUP if self.do_cleanup else STEP_RELINK + policy_dev_progress = {'step': step, + 'parts_done': num_parts_done, + 'total_parts': num_total_parts, + 'timestamp': time.time()} + self.devices_data[device]['policies'][self.policy.idx].update( + policy_dev_progress) + + # aggregate device policy level values into device level + self._aggregate_dev_policy_stats() + + # We want to periodically update the worker recon timestamp so we know + # it's still running + recon_data = self._update_worker_stats(recon_dump=False) + + recon_data.update({'devices': self.devices_data}) + if device: + self.logger.debug("Updating recon for %s", device) + else: + self.logger.debug("Updating recon") + self._last_recon_update = time.time() + dump_recon_cache(recon_data, self.recon_cache, self.logger) @property def total_errors(self): - return sum([ - self.stats['errors'], - self.stats.get('unmounted', 0), - self.stats.get('unlistable_partitions', 0), - ]) + # first make sure the policy data is aggregated down to the device + # level + self._aggregate_dev_policy_stats() + return sum([sum([ + dev.get('stats', {}).get('errors', 0), + dev.get('stats', {}).get('unmounted', 0), + dev.get('stats', {}).get('unlistable_partitions', 0)]) + for dev in self.devices_data.values()]) def devices_filter(self, _, devices): if self.device_list: @@ -117,9 +206,24 @@ class Relinker(object): if err.errno != errno.ENOENT: raise - def hook_post_device(self, _): + # initialise the device in recon. + device = os.path.basename(device_path) + self.devices_data[device]['policies'][self.policy.idx] = { + 'start_time': time.time(), 'stats': _zero_stats(), + 'part_power': self.states["part_power"], + 'next_part_power': self.states["next_part_power"]} + self.stats = \ + self.devices_data[device]['policies'][self.policy.idx]['stats'] + self._update_recon(device) + + def hook_post_device(self, device_path): os.close(self.dev_lock) self.dev_lock = None + device = os.path.basename(device_path) + pol_stats = self.devices_data[device]['policies'][self.policy.idx] + total_time = time.time() - pol_stats['start_time'] + pol_stats.update({'total_time': total_time, 'stats': self.stats}) + self._update_recon(device, force_dump=True) def partitions_filter(self, datadir_path, partitions): # Remove all non partitions first (eg: auditor_status_ALL.json) @@ -265,8 +369,10 @@ class Relinker(object): if part) step = STEP_CLEANUP if self.do_cleanup else STEP_RELINK num_total_parts = len(self.states["state"]) - self.logger.info("Step: %s Device: %s Policy: %s Partitions: %d/%d" % ( - step, device, self.policy.name, num_parts_done, num_total_parts)) + self.logger.info( + "Step: %s Device: %s Policy: %s Partitions: %d/%d", + step, device, self.policy.name, num_parts_done, num_total_parts) + self._update_recon(device) def hashes_filter(self, suff_path, hashes): hashes = list(hashes) @@ -431,6 +537,11 @@ class Relinker(object): 'Error invalidating suffix for %s: %r', hash_path, exc) + def place_policy_stat(self, dev, policy, stat, value): + stats = self.devices_data[dev]['policies'][policy.idx].setdefault( + "stats", _zero_stats()) + stats[stat] = stats.get(stat, 0) + value + def process_policy(self, policy): self.logger.info( 'Processing files for policy %s under %s (cleanup=%s)', @@ -444,6 +555,7 @@ class Relinker(object): "next_part_power": self.next_part_power, "state": {}, } + audit_stats = {} locations = audit_location_generator( self.conf['devices'], @@ -457,7 +569,7 @@ class Relinker(object): hook_post_partition=self.hook_post_partition, hashes_filter=self.hashes_filter, logger=self.logger, - error_counter=self.stats, + error_counter=audit_stats, yield_hash_dirs=True ) if self.conf['files_per_second'] > 0: @@ -471,8 +583,30 @@ class Relinker(object): continue self.process_location(hash_path, new_hash_path) + # any unmounted devices don't trigger the pre_device trigger. + # so we'll deal with them here. + for dev in audit_stats.get('unmounted', []): + self.place_policy_stat(dev, policy, 'unmounted', 1) + + # Further unlistable_partitions doesn't trigger the post_device, so + # we also need to deal with them here. + for datadir in audit_stats.get('unlistable_partitions', []): + device_path, _ = os.path.split(datadir) + device = os.path.basename(device_path) + self.place_policy_stat(device, policy, 'unlistable_partitions', 1) + + def _update_worker_stats(self, recon_dump=True, return_code=None): + worker_stats = {'devices': self.device_list, + 'timestamp': time.time(), + 'return_code': return_code} + worker_data = {"workers": {str(self.pid): worker_stats}} + if recon_dump: + dump_recon_cache(worker_data, self.recon_cache, self.logger) + return worker_data + def run(self): - self._zero_stats() + num_policies = 0 + self._update_worker_stats() for policy in self.conf['policies']: self.policy = policy policy.object_ring = None # Ensure it will be reloaded @@ -484,13 +618,16 @@ class Relinker(object): if self.do_cleanup != part_power_increased: continue - self.stats['policies'] += 1 + num_policies += 1 self.process_policy(policy) - policies = self.stats.pop('policies') - if not policies: + # Some stat collation happens during _update_recon and we want to force + # this to happen at the end of the run + self._update_recon(force_dump=True) + if not num_policies: self.logger.warning( "No policy found to increase the partition power.") + self._update_worker_stats(return_code=EXIT_NO_APPLICABLE_POLICY) return EXIT_NO_APPLICABLE_POLICY if self.total_errors > 0: @@ -502,34 +639,49 @@ class Relinker(object): log_method = self.logger.info status = EXIT_SUCCESS - hash_dirs = self.stats.pop('hash_dirs') - files = self.stats.pop('files') - linked = self.stats.pop('linked') - removed = self.stats.pop('removed') - action_errors = self.stats.pop('errors') - unmounted = self.stats.pop('unmounted', 0) + stats = _zero_stats() + for dev_stats in self.devices_data.values(): + stats = _aggregate_stats(stats, dev_stats.get('stats', {})) + hash_dirs = stats.pop('hash_dirs') + files = stats.pop('files') + linked = stats.pop('linked') + removed = stats.pop('removed') + action_errors = stats.pop('errors') + unmounted = stats.pop('unmounted', 0) if unmounted: self.logger.warning('%d disks were unmounted', unmounted) - listdir_errors = self.stats.pop('unlistable_partitions', 0) + listdir_errors = stats.pop('unlistable_partitions', 0) if listdir_errors: self.logger.warning( 'There were %d errors listing partition directories', listdir_errors) - if self.stats: + if stats: self.logger.warning( 'There were unexpected errors while enumerating disk ' - 'files: %r', self.stats) + 'files: %r', stats) log_method( '%d hash dirs processed (cleanup=%s) (%d files, %d linked, ' '%d removed, %d errors)', hash_dirs, self.do_cleanup, files, linked, removed, action_errors + listdir_errors) + self._update_worker_stats(return_code=status) return status +def _reset_recon(recon_cache, logger): + device_progress_recon = {'devices': {}, 'workers': {}} + dump_recon_cache(device_progress_recon, recon_cache, logger) + + def parallel_process(do_cleanup, conf, logger=None, device_list=None): logger = logger or logging.getLogger() + + # initialise recon dump for collection + # Lets start by always deleting last run's stats + recon_cache = os.path.join(conf['recon_cache_path'], RECON_FILE) + _reset_recon(recon_cache, logger) + device_list = sorted(set(device_list or os.listdir(conf['devices']))) workers = conf['workers'] if workers == 'auto': @@ -636,6 +788,10 @@ def main(args): type=non_negative_float, dest='files_per_second', help='Used to limit I/O. Zero implies no limit ' '(default: no limit).') + parser.add_argument('--stats-interval', default=None, + type=non_negative_float, dest='stats_interval', + help='Emit stats to recon roughly every N seconds. ' + '(default: %d).' % DEFAULT_STATS_INTERVAL) parser.add_argument( '--workers', default=None, type=auto_or_int, help=( 'Process devices across N workers ' @@ -689,6 +845,11 @@ def main(args): 'link_check_limit': ( args.link_check_limit if args.link_check_limit is not None else non_negative_int(conf.get('link_check_limit', 2))), + 'recon_cache_path': conf.get('recon_cache_path', + DEFAULT_RECON_CACHE_PATH), + 'stats_interval': non_negative_float( + args.stats_interval or conf.get('stats_interval', + DEFAULT_STATS_INTERVAL)), }) return parallel_process( args.action == 'cleanup', conf, logger, args.device_list) diff --git a/swift/common/middleware/recon.py b/swift/common/middleware/recon.py index 12f39a14ac..d19edd2d2c 100644 --- a/swift/common/middleware/recon.py +++ b/swift/common/middleware/recon.py @@ -56,6 +56,8 @@ class ReconMiddleware(object): 'account.recon') self.drive_recon_cache = os.path.join(self.recon_cache_path, 'drive.recon') + self.relink_recon_cache = os.path.join(self.recon_cache_path, + "relinker.recon") self.account_ring_path = os.path.join(swift_dir, 'account.ring.gz') self.container_ring_path = os.path.join(swift_dir, 'container.ring.gz') @@ -65,20 +67,26 @@ class ReconMiddleware(object): self.rings.append(os.path.join(swift_dir, policy.ring_name + '.ring.gz')) - def _from_recon_cache(self, cache_keys, cache_file, openr=open): + def _from_recon_cache(self, cache_keys, cache_file, openr=open, + ignore_missing=False): """retrieve values from a recon cache file :params cache_keys: list of cache items to retrieve :params cache_file: cache file to retrieve items from. :params openr: open to use [for unittests] + :params ignore_missing: Some recon stats are very temporary, in this + case it would be better to not log if things are missing. :return: dict of cache items and their values or none if not found """ try: with openr(cache_file, 'r') as f: recondata = json.load(f) return dict((key, recondata.get(key)) for key in cache_keys) - except IOError: - self.logger.exception(_('Error reading recon cache file')) + except IOError as err: + if err.errno == errno.ENOENT and ignore_missing: + pass + else: + self.logger.exception(_('Error reading recon cache file')) except ValueError: self.logger.exception(_('Error parsing recon cache file')) except Exception: @@ -334,6 +342,14 @@ class ReconMiddleware(object): return time.time() + def get_relinker_info(self): + """get relinker info, if any""" + + stat_keys = ['devices', 'workers'] + return self._from_recon_cache(stat_keys, + self.relink_recon_cache, + ignore_missing=True) + def GET(self, req): root, rcheck, rtype = req.split_path(1, 3, True) all_rtypes = ['account', 'container', 'object'] @@ -378,6 +394,8 @@ class ReconMiddleware(object): content = self.get_time() elif rcheck == "sharding": content = self.get_sharding_info() + elif rcheck == "relinker": + content = self.get_relinker_info() else: content = "Invalid path: %s" % req.path return Response(request=req, status="404 Not Found", diff --git a/swift/common/utils.py b/swift/common/utils.py index 8a7be261f7..ad157e58ab 100644 --- a/swift/common/utils.py +++ b/swift/common/utils.py @@ -3311,8 +3311,8 @@ def audit_location_generator(devices, datadir, suffix='', for device in device_dir: if mount_check and not ismount(os.path.join(devices, device)): if error_counter is not None: - error_counter.setdefault('unmounted', 0) - error_counter['unmounted'] += 1 + error_counter.setdefault('unmounted', []) + error_counter['unmounted'].append(device) if logger: logger.warning( _('Skipping %s as it is not mounted'), device) @@ -3325,8 +3325,8 @@ def audit_location_generator(devices, datadir, suffix='', except OSError as e: # NB: listdir ignores non-existent datadir_path if error_counter is not None: - error_counter.setdefault('unlistable_partitions', 0) - error_counter['unlistable_partitions'] += 1 + error_counter.setdefault('unlistable_partitions', []) + error_counter['unlistable_partitions'].append(datadir_path) if logger: logger.warning(_('Skipping %(datadir)s because %(err)s'), {'datadir': datadir_path, 'err': e}) diff --git a/test/unit/__init__.py b/test/unit/__init__.py index 4c3067cb11..1b005cbaa7 100644 --- a/test/unit/__init__.py +++ b/test/unit/__init__.py @@ -225,6 +225,7 @@ class FakeRing(Ring): # 9 total nodes (6 more past the initial 3) is the cap, no matter if # this is set higher, or R^2 for R replicas self.set_replicas(replicas) + self._next_part_power = None self._reload() def has_changed(self): diff --git a/test/unit/cli/test_relinker.py b/test/unit/cli/test_relinker.py index ca47cdf927..7abde673cd 100644 --- a/test/unit/cli/test_relinker.py +++ b/test/unit/cli/test_relinker.py @@ -23,6 +23,7 @@ import os import pickle import shutil import tempfile +import time import unittest import uuid @@ -33,7 +34,8 @@ from swift.common import ring, utils from swift.common import storage_policy from swift.common.exceptions import PathNotDir from swift.common.storage_policy import ( - StoragePolicy, StoragePolicyCollection, POLICIES, ECStoragePolicy) + StoragePolicy, StoragePolicyCollection, POLICIES, ECStoragePolicy, + get_policy_string) from swift.obj.diskfile import write_metadata, DiskFileRouter, \ DiskFileManager, relink_paths @@ -47,14 +49,21 @@ PART_POWER = 8 class TestRelinker(unittest.TestCase): + + maxDiff = None + def setUp(self): skip_if_no_xattrs() self.logger = debug_logger() self.testdir = tempfile.mkdtemp() self.devices = os.path.join(self.testdir, 'node') + self.recon_cache_path = os.path.join(self.testdir, 'cache') + self.recon_cache = os.path.join(self.recon_cache_path, + 'relinker.recon') shutil.rmtree(self.testdir, ignore_errors=True) os.mkdir(self.testdir) os.mkdir(self.devices) + os.mkdir(self.recon_cache_path) self.rb = ring.RingBuilder(PART_POWER, 6.0, 1) @@ -64,13 +73,36 @@ class TestRelinker(unittest.TestCase): 'ip': ip, 'port': 10000, 'device': 'sda1'}) self.rb.rebalance(seed=1) + self.conf_file = os.path.join(self.testdir, 'relinker.conf') + self._setup_config() + self.existing_device = 'sda1' os.mkdir(os.path.join(self.devices, self.existing_device)) self.objects = os.path.join(self.devices, self.existing_device, 'objects') self.policy = StoragePolicy(0, 'platinum', True) storage_policy._POLICIES = StoragePolicyCollection([self.policy]) - self._setup_object() + self._setup_object(policy=self.policy) + + def _setup_config(self): + config = """ + [DEFAULT] + swift_dir = {swift_dir} + devices = {devices} + mount_check = {mount_check} + + [object-relinker] + recon_cache_path = {recon_cache_path} + # update every chance we get! + stats_interval = 0 + """.format( + swift_dir=self.testdir, + devices=self.devices, + mount_check=False, + recon_cache_path=self.recon_cache_path, + ) + with open(self.conf_file, 'w') as f: + f.write(dedent(config)) def _get_object_name(self, condition=None): attempts = [] @@ -93,31 +125,40 @@ class TestRelinker(unittest.TestCase): % attempts) return _hash, part, next_part, obj_path - def _setup_object(self, condition=None): + def _create_object(self, policy, part, _hash): + objects_dir = os.path.join(self.devices, self.existing_device, + get_policy_string('objects', policy)) + shutil.rmtree(objects_dir, ignore_errors=True) + os.mkdir(objects_dir) + objdir = os.path.join(objects_dir, str(part), _hash[-3:], _hash) + os.makedirs(objdir) + timestamp = utils.Timestamp.now() + filename = timestamp.internal + ".data" + objname = os.path.join(objdir, filename) + with open(objname, "wb") as dummy: + dummy.write(b"Hello World!") + write_metadata(dummy, + {'name': self.obj_path, 'Content-Length': '12'}) + return objdir, filename, timestamp + + def _setup_object(self, condition=None, policy=None): + policy = policy or self.policy _hash, part, next_part, obj_path = self._get_object_name(condition) self._hash = _hash self.part = part self.next_part = next_part self.obj_path = obj_path + objects_dir = os.path.join(self.devices, self.existing_device, + get_policy_string('objects', policy)) - shutil.rmtree(self.objects, ignore_errors=True) - os.mkdir(self.objects) - self.objdir = os.path.join( - self.objects, str(self.part), self._hash[-3:], self._hash) - os.makedirs(self.objdir) - self.obj_ts = utils.Timestamp.now() - self.object_fname = self.obj_ts.internal + ".data" + self.objdir, self.object_fname, self.obj_ts = self._create_object( + policy, part, _hash) self.objname = os.path.join(self.objdir, self.object_fname) - with open(self.objname, "wb") as dummy: - dummy.write(b"Hello World!") - write_metadata(dummy, - {'name': self.obj_path, 'Content-Length': '12'}) - - self.part_dir = os.path.join(self.objects, str(self.part)) + self.part_dir = os.path.join(objects_dir, str(self.part)) self.suffix = self._hash[-3:] self.suffix_dir = os.path.join(self.part_dir, self.suffix) - self.next_part_dir = os.path.join(self.objects, str(self.next_part)) + self.next_part_dir = os.path.join(objects_dir, str(self.next_part)) self.next_suffix_dir = os.path.join(self.next_part_dir, self.suffix) self.expected_dir = os.path.join(self.next_suffix_dir, self._hash) self.expected_file = os.path.join(self.expected_dir, self.object_fname) @@ -162,6 +203,14 @@ class TestRelinker(unittest.TestCase): with mock.patch('swift.common.utils.listdir', mocked): yield + @contextmanager + def _mock_relinker(self): + with mock.patch.object(relinker.logging, 'getLogger', + return_value=self.logger): + with mock.patch('swift.cli.relinker.DEFAULT_RECON_CACHE_PATH', + self.recon_cache_path): + yield + def test_workers_parent(self): os.mkdir(os.path.join(self.devices, 'sda2')) self.rb.prepare_increase_partition_power() @@ -198,8 +247,7 @@ class TestRelinker(unittest.TestCase): with mock.patch('os.fork', side_effect=list(pids.keys())), \ mock.patch('os.wait', lambda: pids.popitem()), \ - mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + self._mock_relinker(): self.assertEqual(1, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -207,6 +255,7 @@ class TestRelinker(unittest.TestCase): '--skip-mount', ])) self.assertEqual(pids, {}) + self.assertEqual([], self.logger.get_lines_for_level('error')) warning_lines = self.logger.get_lines_for_level('warning') self.assertTrue( warning_lines[0].startswith('Worker (pid=5, devs=')) @@ -430,7 +479,8 @@ class TestRelinker(unittest.TestCase): @patch_policies( [StoragePolicy(0, name='gold', is_default=True), ECStoragePolicy(1, name='platinum', ec_type=DEFAULT_TEST_EC_TYPE, - ec_ndata=4, ec_nparity=2)]) + ec_ndata=4, ec_nparity=2)], + fake_ring_args=[{}, {}]) def test_conf_file(self): config = """ [DEFAULT] @@ -463,6 +513,8 @@ class TestRelinker(unittest.TestCase): 'workers': 'auto', 'partitions': set(), 'link_check_limit': 2, + 'recon_cache_path': '/var/cache/swift', + 'stats_interval': 300.0, } mock_relinker.assert_called_once_with( exp_conf, mock.ANY, ['sdx'], do_cleanup=False) @@ -490,6 +542,8 @@ class TestRelinker(unittest.TestCase): log_name = test-relinker files_per_second = 11.1 link_check_limit = 1 + recon_cache_path = /var/cache/swift-foo + stats_interval = 111 """ with open(conf_file, 'w') as f: f.write(dedent(config)) @@ -507,6 +561,8 @@ class TestRelinker(unittest.TestCase): 'partitions': set(), 'workers': 'auto', 'link_check_limit': 1, + 'recon_cache_path': '/var/cache/swift-foo', + 'stats_interval': 111.0, }, mock.ANY, ['sdx'], do_cleanup=False) logger = mock_relinker.call_args[0][1] self.assertEqual(logging.WARNING, logger.getEffectiveLevel()) @@ -520,7 +576,8 @@ class TestRelinker(unittest.TestCase): '--skip-mount-check', '--files-per-second', '2.2', '--policy', '1', '--partition', '123', '--partition', '123', '--partition', '456', - '--link-check-limit', '3', '--workers', '2' + '--link-check-limit', '3', '--workers', '2', + '--stats-interval', '222', ]) mock_relinker.assert_called_once_with({ '__file__': mock.ANY, @@ -534,6 +591,8 @@ class TestRelinker(unittest.TestCase): 'partitions': {123, 456}, 'workers': 2, 'link_check_limit': 3, + 'recon_cache_path': '/var/cache/swift-foo', + 'stats_interval': 222.0, }, mock.ANY, ['sdx'], do_cleanup=False) with mock.patch('swift.cli.relinker.Relinker') as mock_relinker, \ @@ -551,6 +610,8 @@ class TestRelinker(unittest.TestCase): 'partitions': set(), 'workers': 'auto', 'link_check_limit': 2, + 'recon_cache_path': '/var/cache/swift', + 'stats_interval': 300.0, }, mock.ANY, ['sdx'], do_cleanup=False) mock_logging_config.assert_called_once_with( format='%(message)s', level=logging.INFO, filename=None) @@ -576,7 +637,9 @@ class TestRelinker(unittest.TestCase): 'policies': set(POLICIES), 'partitions': set(), 'workers': 'auto', - 'link_check_limit': 2 + 'link_check_limit': 2, + 'recon_cache_path': '/var/cache/swift', + 'stats_interval': 300.0, }, mock.ANY, ['sdx'], do_cleanup=False) # --debug is now effective mock_logging_config.assert_called_once_with( @@ -616,6 +679,8 @@ class TestRelinker(unittest.TestCase): 'partitions': set(), 'workers': 'auto', 'link_check_limit': 1, + 'recon_cache_path': '/var/cache/swift', + 'stats_interval': 300.0, }, mock.ANY, ['sdx'], do_cleanup=False) logger = mock_relinker.call_args[0][1] self.assertEqual(logging.WARNING, logger.getEffectiveLevel()) @@ -651,6 +716,8 @@ class TestRelinker(unittest.TestCase): 'partitions': set(), 'workers': 'auto', 'link_check_limit': 1, + 'recon_cache_path': '/var/cache/swift', + 'stats_interval': 300.0, }, mock.ANY, ['sdx'], do_cleanup=False) logger = mock_relinker.call_args[0][1] self.assertEqual(logging.WARNING, logger.getEffectiveLevel()) @@ -785,8 +852,7 @@ class TestRelinker(unittest.TestCase): with mock.patch('swift.cli.relinker.diskfile.relink_paths', mock_relink_paths if mock_relink_paths else default_mock_relink_paths): - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(exp_ret_code, relinker.main([ command, '--swift-dir', self.testdir, @@ -807,6 +873,7 @@ class TestRelinker(unittest.TestCase): self.assertEqual(sorted(exp_filenames), sorted(actual_old)) else: self.assertFalse(os.path.exists(self.objdir)) + self.assertEqual([], self.logger.get_lines_for_level('error')) def _relink_test(self, old_file_specs, new_file_specs, exp_old_specs, exp_new_specs): @@ -993,8 +1060,7 @@ class TestRelinker(unittest.TestCase): pass # expect an error - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(1, relinker.main([ 'relink', '--swift-dir', self.testdir, @@ -1010,6 +1076,7 @@ class TestRelinker(unittest.TestCase): self.assertIn('1 hash dirs processed (cleanup=False) ' '(1 files, 0 linked, 0 removed, 1 errors)', warning_lines) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_relink_link_already_exists(self): self.rb.prepare_increase_partition_power() @@ -1023,8 +1090,7 @@ class TestRelinker(unittest.TestCase): return orig_relink_paths(target_path, new_target_path, **kwargs) - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): with mock.patch('swift.cli.relinker.diskfile.relink_paths', mock_relink_paths): self.assertEqual(0, relinker.main([ @@ -1042,6 +1108,7 @@ class TestRelinker(unittest.TestCase): info_lines = self.logger.get_lines_for_level('info') self.assertIn('1 hash dirs processed (cleanup=False) ' '(1 files, 0 linked, 0 removed, 0 errors)', info_lines) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_relink_link_target_disappears(self): # we need object name in lower half of current part so that there is no @@ -1058,8 +1125,7 @@ class TestRelinker(unittest.TestCase): return orig_relink_paths(target_path, new_target_path, **kwargs) - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): with mock.patch('swift.cli.relinker.diskfile.relink_paths', mock_relink_paths): self.assertEqual(0, relinker.main([ @@ -1074,12 +1140,12 @@ class TestRelinker(unittest.TestCase): info_lines = self.logger.get_lines_for_level('info') self.assertIn('1 hash dirs processed (cleanup=False) ' '(1 files, 0 linked, 0 removed, 0 errors)', info_lines) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_relink_no_applicable_policy(self): # NB do not prepare part power increase self._save_ring() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(2, relinker.main([ 'relink', '--swift-dir', self.testdir, @@ -1087,12 +1153,12 @@ class TestRelinker(unittest.TestCase): ])) self.assertEqual(self.logger.get_lines_for_level('warning'), ['No policy found to increase the partition power.']) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_relink_not_mounted(self): self.rb.prepare_increase_partition_power() self._save_ring() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(1, relinker.main([ 'relink', '--swift-dir', self.testdir, @@ -1104,12 +1170,12 @@ class TestRelinker(unittest.TestCase): '0 hash dirs processed (cleanup=False) ' '(0 files, 0 linked, 0 removed, 0 errors)', ]) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_relink_listdir_error(self): self.rb.prepare_increase_partition_power() self._save_ring() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): with self._mock_listdir(): self.assertEqual(1, relinker.main([ 'relink', @@ -1123,6 +1189,7 @@ class TestRelinker(unittest.TestCase): '0 hash dirs processed (cleanup=False) ' '(0 files, 0 linked, 0 removed, 1 errors)', ]) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_relink_device_filter(self): self.rb.prepare_increase_partition_power() @@ -1204,8 +1271,7 @@ class TestRelinker(unittest.TestCase): # restrict to a partition with no test object self.logger.clear() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'relink', '--swift-dir', self.testdir, @@ -1226,11 +1292,11 @@ class TestRelinker(unittest.TestCase): ) self.assertIn('Finished relinker (cleanup=False):', info_lines[3]) + self.assertEqual([], self.logger.get_lines_for_level('error')) # restrict to one partition with a test object self.logger.clear() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'relink', '--swift-dir', self.testdir, @@ -1256,11 +1322,11 @@ class TestRelinker(unittest.TestCase): ) self.assertIn('Finished relinker (cleanup=False):', info_lines[4]) + self.assertEqual([], self.logger.get_lines_for_level('error')) # restrict to two partitions with test objects self.logger.clear() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'relink', '--swift-dir', self.testdir, @@ -1296,6 +1362,7 @@ class TestRelinker(unittest.TestCase): ) self.assertIn('Finished relinker (cleanup=False):', info_lines[5]) + self.assertEqual([], self.logger.get_lines_for_level('error')) @patch_policies( [StoragePolicy(0, name='gold', is_default=True), @@ -1332,8 +1399,7 @@ class TestRelinker(unittest.TestCase): self.assertEqual(2, cm.exception.code) # policy with no object - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'relink', '--swift-dir', self.testdir, @@ -1355,11 +1421,11 @@ class TestRelinker(unittest.TestCase): ) self.assertIn('Finished relinker (cleanup=False):', info_lines[3]) + self.assertEqual([], self.logger.get_lines_for_level('error')) # policy with object self.logger.clear() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'relink', '--swift-dir', self.testdir, @@ -1386,11 +1452,11 @@ class TestRelinker(unittest.TestCase): ) self.assertIn('Finished relinker (cleanup=False):', info_lines[4]) + self.assertEqual([], self.logger.get_lines_for_level('error')) # policy name works, too self.logger.clear() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'relink', '--swift-dir', self.testdir, @@ -1416,6 +1482,7 @@ class TestRelinker(unittest.TestCase): ) self.assertIn('Finished relinker (cleanup=False):', info_lines[3]) + self.assertEqual([], self.logger.get_lines_for_level('error')) @patch_policies( [StoragePolicy(0, name='gold', is_default=True), @@ -1425,16 +1492,19 @@ class TestRelinker(unittest.TestCase): # verify that only policies in appropriate state are processed def do_relink(options=None): options = [] if options is None else options - with mock.patch( - 'swift.cli.relinker.Relinker.process_policy') as mocked: - res = relinker.main([ - 'relink', - '--swift-dir', self.testdir, - '--skip-mount', - '--devices', self.devices, - '--device', self.existing_device, - ] + options) - return res, mocked + with self._mock_relinker(): + with mock.patch( + 'swift.cli.relinker.Relinker.process_policy') \ + as mocked: + res = relinker.main([ + 'relink', + '--swift-dir', self.testdir, + '--skip-mount', + '--devices', self.devices, + '--device', self.existing_device, + ] + options) + self.assertEqual([], self.logger.get_lines_for_level('error')) + return res, mocked self._save_ring(POLICIES) # no ring prepared for increase res, mocked = do_relink() @@ -1902,6 +1972,7 @@ class TestRelinker(unittest.TestCase): 'mount_check': False, 'files_per_second': 0, 'policies': POLICIES, + 'recon_cache_path': self.recon_cache_path, 'workers': 0} self.assertEqual(0, relinker.Relinker( conf, logger=self.logger, device_list=[self.existing_device], @@ -2412,8 +2483,7 @@ class TestRelinker(unittest.TestCase): def test_cleanup_no_applicable_policy(self): # NB do not prepare part power increase self._save_ring() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(2, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -2421,11 +2491,11 @@ class TestRelinker(unittest.TestCase): ])) self.assertEqual(self.logger.get_lines_for_level('warning'), ['No policy found to increase the partition power.']) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_not_mounted(self): self._common_test_cleanup() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(1, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -2437,11 +2507,11 @@ class TestRelinker(unittest.TestCase): '0 hash dirs processed (cleanup=True) ' '(0 files, 0 linked, 0 removed, 0 errors)', ]) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_listdir_error(self): self._common_test_cleanup() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): with self._mock_listdir(): self.assertEqual(1, relinker.main([ 'cleanup', @@ -2455,88 +2525,224 @@ class TestRelinker(unittest.TestCase): '0 hash dirs processed (cleanup=True) ' '(0 files, 0 linked, 0 removed, 1 errors)', ]) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_device_filter(self): self._common_test_cleanup() - self.assertEqual(0, relinker.main([ - 'cleanup', - '--swift-dir', self.testdir, - '--devices', self.devices, - '--skip-mount', - '--device', self.existing_device, - ])) + with self._mock_relinker(): + self.assertEqual(0, relinker.main([ + 'cleanup', + '--swift-dir', self.testdir, + '--devices', self.devices, + '--skip-mount', + '--device', self.existing_device, + ])) # Old objectname should be removed, new should still exist self.assertTrue(os.path.isdir(self.expected_dir)) self.assertTrue(os.path.isfile(self.expected_file)) self.assertFalse(os.path.isfile( os.path.join(self.objdir, self.object_fname))) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_device_filter_invalid(self): self._common_test_cleanup() - self.assertEqual(0, relinker.main([ - 'cleanup', - '--swift-dir', self.testdir, - '--devices', self.devices, - '--skip-mount', - '--device', 'none', - ])) + with self._mock_relinker(): + self.assertEqual(0, relinker.main([ + 'cleanup', + '--swift-dir', self.testdir, + '--devices', self.devices, + '--skip-mount', + '--device', 'none', + ])) # Old objectname should still exist, new should still exist self.assertTrue(os.path.isdir(self.expected_dir)) self.assertTrue(os.path.isfile(self.expected_file)) self.assertTrue(os.path.isfile( os.path.join(self.objdir, self.object_fname))) + self.assertEqual([], self.logger.get_lines_for_level('error')) - def test_relink_cleanup(self): - state_file = os.path.join(self.devices, self.existing_device, - 'relink.objects.json') + def _time_iter(self, start): + yield start + while True: + yield start + 1 + + @patch_policies( + [StoragePolicy(0, 'platinum', True), + ECStoragePolicy( + 1, name='ec', is_default=False, ec_type=DEFAULT_TEST_EC_TYPE, + ec_ndata=4, ec_nparity=2)]) + @mock.patch('os.getpid', return_value=100) + def test_relink_cleanup(self, mock_getpid): + # setup a policy-0 object in a part in the second quartile so that its + # next part *will not* be handled during cleanup + self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1)) + # create policy-1 object in a part in the first quartile so that its + # next part *will* be handled during cleanup + _hash, pol_1_part, pol_1_next_part, objpath = self._get_object_name( + lambda part: part < 2 ** (PART_POWER - 1)) + self._create_object(POLICIES[1], pol_1_part, _hash) + + state_files = { + POLICIES[0]: os.path.join(self.devices, self.existing_device, + 'relink.objects.json'), + POLICIES[1]: os.path.join(self.devices, self.existing_device, + 'relink.objects-1.json'), + } self.rb.prepare_increase_partition_power() self._save_ring() - self.assertEqual(0, relinker.main([ - 'relink', - '--swift-dir', self.testdir, - '--devices', self.devices, - '--skip-mount', - ])) - state = {str(self.part): True} - with open(state_file, 'rt') as f: - orig_inode = os.stat(state_file).st_ino - self.assertEqual(json.load(f), { - "part_power": PART_POWER, - "next_part_power": PART_POWER + 1, - "state": state}) + ts1 = time.time() + with mock.patch('time.time', side_effect=self._time_iter(ts1)): + self.assertEqual(0, relinker.main([ + 'relink', + self.conf_file, + ])) + + orig_inodes = {} + for policy, part in zip(POLICIES, + (self.part, pol_1_part)): + state_file = state_files[policy] + orig_inodes[policy] = os.stat(state_file).st_ino + state = {str(part): True} + with open(state_files[policy], 'rt') as f: + self.assertEqual(json.load(f), { + "part_power": PART_POWER, + "next_part_power": PART_POWER + 1, + "state": state}) + recon_progress = utils.load_recon_cache(self.recon_cache) + expected_recon_data = { + 'devices': {'sda1': {'parts_done': 2, + 'policies': {'0': { + 'next_part_power': PART_POWER + 1, + 'part_power': PART_POWER, + 'parts_done': 1, + 'start_time': mock.ANY, + 'stats': {'errors': 0, + 'files': 1, + 'hash_dirs': 1, + 'linked': 1, + 'removed': 0}, + 'step': 'relink', + 'timestamp': mock.ANY, + 'total_parts': 1, + 'total_time': 0.0}, + '1': { + 'next_part_power': PART_POWER + 1, + 'part_power': PART_POWER, + 'parts_done': 1, + 'start_time': mock.ANY, + 'stats': { + 'errors': 0, + 'files': 1, + 'hash_dirs': 1, + 'linked': 1, + 'removed': 0}, + 'step': 'relink', + 'timestamp': mock.ANY, + 'total_parts': 1, + 'total_time': 0.0}}, + 'start_time': mock.ANY, + 'stats': {'errors': 0, + 'files': 2, + 'hash_dirs': 2, + 'linked': 2, + 'removed': 0}, + 'timestamp': mock.ANY, + 'total_parts': 2, + 'total_time': 0}}, + 'workers': {'100': {'devices': ['sda1'], + 'return_code': 0, + 'timestamp': mock.ANY}}} + self.assertEqual(recon_progress, expected_recon_data) self.rb.increase_partition_power() self.rb._ring = None # Force builder to reload ring self._save_ring() - with open(state_file, 'rt') as f: - # Keep the state file open during cleanup so the inode can't be + with open(state_files[0], 'rt'), open(state_files[1], 'rt'): + # Keep the state files open during cleanup so the inode can't be # released/re-used when it gets unlinked - self.assertEqual(orig_inode, os.stat(state_file).st_ino) - self.assertEqual(0, relinker.main([ - 'cleanup', - '--swift-dir', self.testdir, - '--devices', self.devices, - '--skip-mount', - ])) - self.assertNotEqual(orig_inode, os.stat(state_file).st_ino) - if self.next_part < 2 ** PART_POWER: - state[str(self.next_part)] = True - with open(state_file, 'rt') as f: - # NB: part_power/next_part_power tuple changed, so state was reset - # (though we track prev_part_power for an efficient clean up) - self.assertEqual(json.load(f), { - "prev_part_power": PART_POWER, - "part_power": PART_POWER + 1, - "next_part_power": PART_POWER + 1, - "state": state}) + self.assertEqual(orig_inodes[0], os.stat(state_files[0]).st_ino) + self.assertEqual(orig_inodes[1], os.stat(state_files[1]).st_ino) + ts1 = time.time() + with mock.patch('time.time', side_effect=self._time_iter(ts1)): + self.assertEqual(0, relinker.main([ + 'cleanup', + self.conf_file, + ])) + self.assertNotEqual(orig_inodes[0], os.stat(state_files[0]).st_ino) + self.assertNotEqual(orig_inodes[1], os.stat(state_files[1]).st_ino) + for policy, part, next_part in zip(POLICIES, + (self.part, pol_1_part), + (None, pol_1_next_part)): + state_file = state_files[policy] + state = {str(part): True} + if next_part is not None: + # cleanup will process the new partition as well as the old if + # old is in first quartile + state[str(next_part)] = True + with open(state_file, 'rt') as f: + # NB: part_power/next_part_power tuple changed, so state was + # reset (though we track prev_part_power for an efficient clean + # up) + self.assertEqual(json.load(f), { + "prev_part_power": PART_POWER, + "part_power": PART_POWER + 1, + "next_part_power": PART_POWER + 1, + "state": state}) + recon_progress = utils.load_recon_cache(self.recon_cache) + expected_recon_data = { + 'devices': {'sda1': {'parts_done': 3, + 'policies': {'0': { + 'next_part_power': PART_POWER + 1, + 'part_power': PART_POWER + 1, + 'parts_done': 1, + 'start_time': mock.ANY, + 'stats': {'errors': 0, + 'files': 1, + 'hash_dirs': 1, + 'linked': 0, + 'removed': 1}, + 'step': 'cleanup', + 'timestamp': mock.ANY, + 'total_parts': 1, + 'total_time': 0.0}, + '1': { + 'next_part_power': PART_POWER + 1, + 'part_power': PART_POWER + 1, + 'parts_done': 2, + 'start_time': mock.ANY, + 'stats': { + 'errors': 0, + 'files': 1, + 'hash_dirs': 1, + 'linked': 0, + 'removed': 1}, + 'step': 'cleanup', + 'timestamp': mock.ANY, + 'total_parts': 2, + 'total_time': 0.0}}, + 'start_time': mock.ANY, + 'stats': {'errors': 0, + 'files': 2, + 'hash_dirs': 2, + 'linked': 0, + 'removed': 2}, + 'timestamp': mock.ANY, + 'total_parts': 3, + 'total_time': 0}}, + 'workers': {'100': {'devices': ['sda1'], + 'return_code': 0, + 'timestamp': mock.ANY}}} + self.assertEqual(recon_progress, expected_recon_data) def test_devices_filter_filtering(self): # With no filtering, returns all devices r = relinker.Relinker( - {'devices': self.devices}, self.logger, self.existing_device) + {'devices': self.devices, + 'recon_cache_path': self.recon_cache_path}, + self.logger, self.existing_device) devices = r.devices_filter("", [self.existing_device]) self.assertEqual(set([self.existing_device]), devices) @@ -2551,11 +2757,15 @@ class TestRelinker(unittest.TestCase): def test_hook_pre_post_device_locking(self): r = relinker.Relinker( - {'devices': self.devices}, self.logger, self.existing_device) + {'devices': self.devices, + 'recon_cache_path': self.recon_cache_path}, + self.logger, self.existing_device) device_path = os.path.join(self.devices, self.existing_device) r.datadir = 'object' # would get set in process_policy - r.states = {"state": {}} # ditto + r.states = {"state": {}, "part_power": PART_POWER, + "next_part_power": PART_POWER + 1} # ditto lock_file = os.path.join(device_path, '.relink.%s.lock' % r.datadir) + r.policy = self.policy # The first run gets the lock r.hook_pre_device(device_path) @@ -2568,21 +2778,31 @@ class TestRelinker(unittest.TestCase): self.assertEqual(errno.EAGAIN, raised.exception.errno) # Another must not get the lock, so it must return an empty list - r.hook_post_device("") + r.hook_post_device(device_path) self.assertIsNone(r.dev_lock) with open(lock_file, 'a') as f: fcntl.flock(f.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) - def test_state_file(self): + def _test_state_file(self, pol, expected_recon_data): r = relinker.Relinker( - {'devices': self.devices}, self.logger, self.existing_device) + {'devices': self.devices, + 'recon_cache_path': self.recon_cache_path, + 'stats_interval': 0.0}, + self.logger, [self.existing_device]) device_path = os.path.join(self.devices, self.existing_device) r.datadir = 'objects' r.part_power = PART_POWER r.next_part_power = PART_POWER + 1 datadir_path = os.path.join(device_path, r.datadir) state_file = os.path.join(device_path, 'relink.%s.json' % r.datadir) + r.policy = pol + r.pid = 1234 # for recon workers stats + + recon_progress = utils.load_recon_cache(self.recon_cache) + # the progress for the current policy should be gone. So we should + # just have anything from any other process polices.. if any. + self.assertEqual(recon_progress, expected_recon_data) # Start relinking r.states = { @@ -2604,7 +2824,6 @@ class TestRelinker(unittest.TestCase): "", ['96', '227', '312', 'auditor_status.json'])) self.assertEqual(r.states["state"], {'96': False, '227': False}) - r.policy = POLICIES[0] r.diskfile_mgr = DiskFileRouter({ 'devices': self.devices, 'mount_check': False, @@ -2623,6 +2842,41 @@ class TestRelinker(unittest.TestCase): "part_power": PART_POWER, "next_part_power": PART_POWER + 1, "state": {'96': True, '227': False}}) + recon_progress = utils.load_recon_cache(self.recon_cache) + expected_recon_data.update( + {'devices': { + 'sda1': { + 'parts_done': 1, + 'policies': { + str(pol.idx): { + 'next_part_power': PART_POWER + 1, + 'part_power': PART_POWER, + 'parts_done': 1, + 'start_time': mock.ANY, + 'stats': { + 'errors': 0, + 'files': 0, + 'hash_dirs': 0, + 'linked': 0, + 'removed': 0}, + 'step': 'relink', + 'timestamp': mock.ANY, + 'total_parts': 2}}, + 'start_time': mock.ANY, + 'stats': { + 'errors': 0, + 'files': 0, + 'hash_dirs': 0, + 'linked': 0, + 'removed': 0}, + 'timestamp': mock.ANY, + 'total_parts': 2, + 'total_time': 0}}, + 'workers': { + '1234': {'timestamp': mock.ANY, + 'return_code': None, + 'devices': ['sda1']}}}) + self.assertEqual(recon_progress, expected_recon_data) # Restart relinking after only part 96 was done self.logger.clear() @@ -2664,6 +2918,37 @@ class TestRelinker(unittest.TestCase): "part_power": PART_POWER, "next_part_power": PART_POWER + 1, "state": {'96': True, '227': True}}) + recon_progress = utils.load_recon_cache(self.recon_cache) + expected_recon_data.update( + {'devices': { + 'sda1': { + 'parts_done': 2, + 'policies': { + str(pol.idx): { + 'next_part_power': PART_POWER + 1, + 'part_power': PART_POWER, + 'parts_done': 2, + 'start_time': mock.ANY, + 'stats': { + 'errors': 1, + 'files': 0, + 'hash_dirs': 0, + 'linked': 0, + 'removed': 0}, + 'step': 'relink', + 'timestamp': mock.ANY, + 'total_parts': 2}}, + 'start_time': mock.ANY, + 'stats': { + 'errors': 1, + 'files': 0, + 'hash_dirs': 0, + 'linked': 0, + 'removed': 0}, + 'timestamp': mock.ANY, + 'total_parts': 2, + 'total_time': 0}}}) + self.assertEqual(recon_progress, expected_recon_data) # If the process restarts, it reload the state r.states = { @@ -2698,6 +2983,7 @@ class TestRelinker(unittest.TestCase): self.assertEqual(['227', '96'], r.partitions_filter("", ['96', '227', '312'])) # Ack partition 227 + r.hook_pre_partition(os.path.join(datadir_path, '227')) r.hook_post_partition(os.path.join(datadir_path, '227')) self.assertIn("Step: cleanup Device: sda1 Policy: %s " "Partitions: 1/2" % r.policy.name, @@ -2710,6 +2996,37 @@ class TestRelinker(unittest.TestCase): "part_power": PART_POWER + 1, "next_part_power": PART_POWER + 1, "state": {'96': False, '227': True}}) + recon_progress = utils.load_recon_cache(self.recon_cache) + expected_recon_data.update( + {'devices': { + 'sda1': { + 'parts_done': 1, + 'policies': { + str(pol.idx): { + 'next_part_power': PART_POWER + 1, + 'part_power': PART_POWER + 1, + 'parts_done': 1, + 'start_time': mock.ANY, + 'stats': { + 'errors': 0, + 'files': 0, + 'hash_dirs': 0, + 'linked': 0, + 'removed': 0}, + 'step': 'cleanup', + 'timestamp': mock.ANY, + 'total_parts': 2}}, + 'start_time': mock.ANY, + 'stats': { + 'errors': 0, + 'files': 0, + 'hash_dirs': 0, + 'linked': 0, + 'removed': 0}, + 'timestamp': mock.ANY, + 'total_parts': 2, + 'total_time': 0}}}) + self.assertEqual(recon_progress, expected_recon_data) # Restart cleanup after only part 227 was done self.assertEqual(['96'], r.partitions_filter("", ['96', '227', '312'])) @@ -2730,6 +3047,38 @@ class TestRelinker(unittest.TestCase): "next_part_power": PART_POWER + 1, "state": {'96': True, '227': True}}) + recon_progress = utils.load_recon_cache(self.recon_cache) + expected_recon_data.update( + {'devices': { + 'sda1': { + 'parts_done': 2, + 'policies': { + str(pol.idx): { + 'next_part_power': PART_POWER + 1, + 'part_power': PART_POWER + 1, + 'parts_done': 2, + 'start_time': mock.ANY, + 'stats': { + 'errors': 0, + 'files': 0, + 'hash_dirs': 0, + 'linked': 0, + 'removed': 0}, + 'step': 'cleanup', + 'timestamp': mock.ANY, + 'total_parts': 2}}, + 'start_time': mock.ANY, + 'stats': { + 'errors': 0, + 'files': 0, + 'hash_dirs': 0, + 'linked': 0, + 'removed': 0}, + 'timestamp': mock.ANY, + 'total_parts': 2, + 'total_time': 0}}}) + self.assertEqual(recon_progress, expected_recon_data) + # At the end, the state is still accurate r.states = { "prev_part_power": PART_POWER, @@ -2751,6 +3100,38 @@ class TestRelinker(unittest.TestCase): r.hook_pre_device(device_path) self.assertEqual(r.states["state"], {}) self.assertFalse(os.path.exists(state_file)) + # this will also reset the recon stats + recon_progress = utils.load_recon_cache(self.recon_cache) + expected_recon_data.update({ + 'devices': { + 'sda1': { + 'parts_done': 0, + 'policies': { + str(pol.idx): { + 'next_part_power': PART_POWER + 2, + 'part_power': PART_POWER + 1, + 'parts_done': 0, + 'start_time': mock.ANY, + 'stats': { + 'errors': 0, + 'files': 0, + 'hash_dirs': 0, + 'linked': 0, + 'removed': 0}, + 'step': 'cleanup', + 'timestamp': mock.ANY, + 'total_parts': 0}}, + 'start_time': mock.ANY, + 'stats': { + 'errors': 0, + 'files': 0, + 'hash_dirs': 0, + 'linked': 0, + 'removed': 0}, + 'timestamp': mock.ANY, + 'total_parts': 0, + 'total_time': 0}}}) + self.assertEqual(recon_progress, expected_recon_data) os.close(r.dev_lock) # Release the lock # If the file gets corrupted, restart from scratch @@ -2764,12 +3145,56 @@ class TestRelinker(unittest.TestCase): r.hook_pre_device(device_path) self.assertEqual(r.states["state"], {}) self.assertFalse(os.path.exists(state_file)) + recon_progress = utils.load_recon_cache(self.recon_cache) + expected_recon_data.update({ + 'devices': { + 'sda1': { + 'parts_done': 0, + 'policies': { + str(pol.idx): { + 'next_part_power': PART_POWER + 1, + 'part_power': PART_POWER, + 'parts_done': 0, + 'start_time': mock.ANY, + 'stats': { + 'errors': 0, + 'files': 0, + 'hash_dirs': 0, + 'linked': 0, + 'removed': 0}, + 'step': 'cleanup', + 'timestamp': mock.ANY, + 'total_parts': 0}}, + 'start_time': mock.ANY, + 'stats': { + 'errors': 0, + 'files': 0, + 'hash_dirs': 0, + 'linked': 0, + 'removed': 0}, + 'timestamp': mock.ANY, + 'total_parts': 0, + 'total_time': 0}}}) + self.assertEqual(recon_progress, expected_recon_data) os.close(r.dev_lock) # Release the lock + return expected_recon_data + + @patch_policies( + [StoragePolicy(0, 'platinum', True), + ECStoragePolicy( + 1, name='ec', is_default=False, ec_type=DEFAULT_TEST_EC_TYPE, + ec_ndata=4, ec_nparity=2)]) + def test_state_file(self): + expected_recon_data = {} + for policy in POLICIES: + # because we specifying a device, it should be itself reset + expected_recon_data = self._test_state_file( + policy, expected_recon_data) + self.logger.clear() def test_cleanup_relinked_ok(self): self._common_test_cleanup() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -2784,13 +3209,13 @@ class TestRelinker(unittest.TestCase): info_lines = self.logger.get_lines_for_level('info') self.assertIn('1 hash dirs processed (cleanup=True) ' '(1 files, 0 linked, 1 removed, 0 errors)', info_lines) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_not_yet_relinked(self): # force rehash of new partition to not happen during cleanup self._setup_object(lambda part: part >= 2 ** (PART_POWER - 1)) self._common_test_cleanup(relink=False) - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -2814,6 +3239,7 @@ class TestRelinker(unittest.TestCase): self.assertTrue(os.path.exists(hashes_invalid)) with open(hashes_invalid, 'r') as fd: self.assertEqual(str(self.suffix), fd.read().strip()) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_same_object_different_inode_in_new_partition(self): # force rehash of new partition to not happen during cleanup @@ -2824,8 +3250,7 @@ class TestRelinker(unittest.TestCase): with open(self.expected_file, 'w') as fd: fd.write('same but different') - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): res = relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -2850,6 +3275,7 @@ class TestRelinker(unittest.TestCase): self.assertEqual('1 hash dirs processed (cleanup=True) ' '(1 files, 0 linked, 0 removed, 1 errors)', warning_lines[1]) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_older_object_in_new_partition(self): # relink of the current object failed, but there is an older version of @@ -2865,8 +3291,7 @@ class TestRelinker(unittest.TestCase): fd.write(b"Hello Olde Worlde!") write_metadata(fd, {'name': self.obj_path, 'Content-Length': '18'}) - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): res = relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -2892,6 +3317,7 @@ class TestRelinker(unittest.TestCase): self.assertTrue(os.path.exists(hashes_invalid)) with open(hashes_invalid, 'r') as fd: self.assertEqual(str(self.suffix), fd.read().strip()) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_deleted(self): # force rehash of new partition to not happen during cleanup @@ -2910,8 +3336,7 @@ class TestRelinker(unittest.TestCase): os.rename(self.expected_file, fname_ts) self.assertTrue(os.path.isfile(fname_ts)) - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -2928,6 +3353,7 @@ class TestRelinker(unittest.TestCase): info_lines = self.logger.get_lines_for_level('info') self.assertIn('1 hash dirs processed (cleanup=True) ' '(0 files, 0 linked, 1 removed, 0 errors)', info_lines) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_old_part_careful(self): self._common_test_cleanup() @@ -2935,14 +3361,16 @@ class TestRelinker(unittest.TestCase): extra_file = os.path.join(self.part_dir, 'extra') with open(extra_file, 'w'): pass - self.assertEqual(0, relinker.main([ - 'cleanup', - '--swift-dir', self.testdir, - '--devices', self.devices, - '--skip-mount', - ])) + with self._mock_relinker(): + self.assertEqual(0, relinker.main([ + 'cleanup', + '--swift-dir', self.testdir, + '--devices', self.devices, + '--skip-mount', + ])) # old partition can't be cleaned up self.assertTrue(os.path.exists(self.part_dir)) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_old_part_robust(self): self._common_test_cleanup() @@ -2967,15 +3395,17 @@ class TestRelinker(unittest.TestCase): return orig_resp with mock.patch.object(DiskFileManager, 'get_hashes', mock_get_hashes): - self.assertEqual(0, relinker.main([ - 'cleanup', - '--swift-dir', self.testdir, - '--devices', self.devices, - '--skip-mount', - ])) + with self._mock_relinker(): + self.assertEqual(0, relinker.main([ + 'cleanup', + '--swift-dir', self.testdir, + '--devices', self.devices, + '--skip-mount', + ])) self.assertEqual([True], calls) # old partition can still be cleaned up self.assertFalse(os.path.exists(self.part_dir)) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_reapable(self): # relink a tombstone @@ -2986,9 +3416,8 @@ class TestRelinker(unittest.TestCase): self._common_test_cleanup() self.assertTrue(os.path.exists(self.expected_file)) # sanity check - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger), \ - mock.patch('time.time', return_value=1e11): # far, far future + with self._mock_relinker(), \ + mock.patch('time.time', return_value=1e10 - 1): # far future self.assertEqual(0, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -3009,8 +3438,7 @@ class TestRelinker(unittest.TestCase): # cleanup: cleanup should re-create the link os.remove(self.expected_file) - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -3028,6 +3456,7 @@ class TestRelinker(unittest.TestCase): info_lines = self.logger.get_lines_for_level('info') self.assertIn('1 hash dirs processed (cleanup=True) ' '(1 files, 1 linked, 1 removed, 0 errors)', info_lines) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_new_does_not_exist_and_relink_fails(self): # force rehash of new partition to not happen during cleanup @@ -3041,8 +3470,7 @@ class TestRelinker(unittest.TestCase): os.remove(self.expected_file) with mock.patch('swift.obj.diskfile.os.link', side_effect=OSError): - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(1, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -3064,6 +3492,7 @@ class TestRelinker(unittest.TestCase): # nor in the old partition old_hashes_invalid = os.path.join(self.part_dir, 'hashes.invalid') self.assertFalse(os.path.exists(old_hashes_invalid)) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_remove_fails(self): meta_file = utils.Timestamp(int(self.obj_ts) + 1).internal + '.meta' @@ -3084,8 +3513,7 @@ class TestRelinker(unittest.TestCase): return orig_remove(path) with mock.patch('swift.obj.diskfile.os.remove', mock_remove): - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(1, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -3102,6 +3530,7 @@ class TestRelinker(unittest.TestCase): '1 hash dirs processed (cleanup=True) ' '(2 files, 0 linked, 1 removed, 1 errors)', ]) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_cleanup_two_files_need_linking(self): meta_file = utils.Timestamp(int(self.obj_ts) + 1).internal + '.meta' @@ -3114,8 +3543,7 @@ class TestRelinker(unittest.TestCase): self.assertFalse(os.path.isfile(self.expected_file)) # link missing self.assertFalse(os.path.isfile(new_meta_path)) # link missing - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -3130,6 +3558,7 @@ class TestRelinker(unittest.TestCase): info_lines = self.logger.get_lines_for_level('info') self.assertIn('1 hash dirs processed (cleanup=True) ' '(2 files, 2 linked, 2 removed, 0 errors)', info_lines) + self.assertEqual([], self.logger.get_lines_for_level('error')) @patch_policies( [ECStoragePolicy( @@ -3140,8 +3569,7 @@ class TestRelinker(unittest.TestCase): # are included in the diskfile data as 'unexpected' files and cleanup # should include them self._common_test_cleanup() - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -3166,6 +3594,7 @@ class TestRelinker(unittest.TestCase): info_lines = self.logger.get_lines_for_level('info') self.assertIn('1 hash dirs processed (cleanup=True) ' '(1 files, 0 linked, 1 removed, 0 errors)', info_lines) + self.assertEqual([], self.logger.get_lines_for_level('error')) @patch_policies( [ECStoragePolicy( @@ -3176,8 +3605,7 @@ class TestRelinker(unittest.TestCase): # Switch the policy type so all fragments raise DiskFileError: they # are included in the diskfile data as 'unexpected' files and cleanup # should include them - with mock.patch.object(relinker.logging, 'getLogger', - return_value=self.logger): + with self._mock_relinker(): self.assertEqual(0, relinker.main([ 'cleanup', '--swift-dir', self.testdir, @@ -3202,6 +3630,7 @@ class TestRelinker(unittest.TestCase): info_lines = self.logger.get_lines_for_level('info') self.assertIn('1 hash dirs processed (cleanup=True) ' '(1 files, 1 linked, 1 removed, 0 errors)', info_lines) + self.assertEqual([], self.logger.get_lines_for_level('error')) def test_rehashing(self): calls = [] @@ -3223,7 +3652,8 @@ class TestRelinker(unittest.TestCase): mock_invalidate), \ mock.patch.object(DiskFileManager, 'get_hashes', mock_get_hashes): - yield + with self._mock_relinker(): + yield with do_mocks(): self.rb.prepare_increase_partition_power() @@ -3263,6 +3693,7 @@ class TestRelinker(unittest.TestCase): POLICIES[0]), ]) self.assertEqual(calls, expected) + self.assertEqual([], self.logger.get_lines_for_level('error')) if __name__ == '__main__': diff --git a/test/unit/common/middleware/test_recon.py b/test/unit/common/middleware/test_recon.py index 71fcd0efef..a967007eea 100644 --- a/test/unit/common/middleware/test_recon.py +++ b/test/unit/common/middleware/test_recon.py @@ -15,6 +15,7 @@ import array from contextlib import contextmanager +import errno import json import mock import os @@ -29,6 +30,7 @@ from swift.common import ring, utils from swift.common.swob import Request from swift.common.middleware import recon from swift.common.storage_policy import StoragePolicy +from test.debug_logger import debug_logger from test.unit import patch_policies @@ -160,6 +162,9 @@ class FakeRecon(object): def fake_sharding(self): return {"sharding_stats": "1"} + def fake_relinker(self): + return {"relinktest": "1"} + def fake_updater(self, recon_type): self.fake_updater_rtype = recon_type return {'updatertest': "1"} @@ -205,8 +210,10 @@ class FakeRecon(object): def nocontent(self): return None - def raise_IOError(self, *args, **kwargs): - raise IOError + def raise_IOError(self, errno=None): + mock_obj = mock.MagicMock() + mock_obj.side_effect = IOError(errno, str(errno)) + return mock_obj def raise_ValueError(self, *args, **kwargs): raise ValueError @@ -235,6 +242,7 @@ class TestReconSuccess(TestCase): self.real_from_cache = self.app._from_recon_cache self.app._from_recon_cache = self.fakecache.fake_from_recon_cache self.frecon = FakeRecon() + self.app.logger = debug_logger() # replace hash md5 implementation of the md5_hash_for_file function mock_hash_for_file = mock.patch( @@ -476,11 +484,29 @@ class TestReconSuccess(TestCase): self.app._from_recon_cache = self.fakecache.fake_from_recon_cache def test_from_recon_cache_ioerror(self): - oart = self.frecon.raise_IOError + oart = self.frecon.raise_IOError() self.app._from_recon_cache = self.real_from_cache rv = self.app._from_recon_cache(['testkey1', 'notpresentkey'], 'test.cache', openr=oart) self.assertEqual(rv, {'notpresentkey': None, 'testkey1': None}) + self.assertIn('Error reading recon cache file: ', + self.app.logger.get_lines_for_level('error')) + # Now try with ignore_missing but not ENOENT + self.app.logger.clear() + rv = self.app._from_recon_cache(['testkey1', 'notpresentkey'], + 'test.cache', openr=oart, + ignore_missing=True) + self.assertEqual(rv, {'notpresentkey': None, 'testkey1': None}) + self.assertIn('Error reading recon cache file: ', + self.app.logger.get_lines_for_level('error')) + # Now try again with ignore_missing with ENOENT + self.app.logger.clear() + oart = self.frecon.raise_IOError(errno.ENOENT) + rv = self.app._from_recon_cache(['testkey1', 'notpresentkey'], + 'test.cache', openr=oart, + ignore_missing=True) + self.assertEqual(rv, {'notpresentkey': None, 'testkey1': None}) + self.assertEqual(self.app.logger.get_lines_for_level('error'), []) self.app._from_recon_cache = self.fakecache.fake_from_recon_cache def test_from_recon_cache_valueerror(self): @@ -489,6 +515,8 @@ class TestReconSuccess(TestCase): rv = self.app._from_recon_cache(['testkey1', 'notpresentkey'], 'test.cache', openr=oart) self.assertEqual(rv, {'notpresentkey': None, 'testkey1': None}) + self.assertIn('Error parsing recon cache file: ', + self.app.logger.get_lines_for_level('error')) self.app._from_recon_cache = self.fakecache.fake_from_recon_cache def test_from_recon_cache_exception(self): @@ -497,6 +525,8 @@ class TestReconSuccess(TestCase): rv = self.app._from_recon_cache(['testkey1', 'notpresentkey'], 'test.cache', openr=oart) self.assertEqual(rv, {'notpresentkey': None, 'testkey1': None}) + self.assertIn('Error retrieving recon data: ', + self.app.logger.get_lines_for_level('error')) self.app._from_recon_cache = self.fakecache.fake_from_recon_cache def test_get_mounted(self): @@ -1189,6 +1219,88 @@ class TestReconSuccess(TestCase): '/var/cache/swift/container.recon'), {})]) self.assertEqual(rv, from_cache_response) + def test_get_relinker_info(self): + from_cache_response = { + "devices": { + "sdb3": { + "parts_done": 523, + "policies": { + "1": { + "next_part_power": 11, + "start_time": 1618998724.845616, + "stats": { + "errors": 0, + "files": 1630, + "hash_dirs": 1630, + "linked": 1630, + "policies": 1, + "removed": 0 + }, + "timestamp": 1618998730.24672, + "total_parts": 1029, + "total_time": 5.400741815567017 + }}, + "start_time": 1618998724.845946, + "stats": { + "errors": 0, + "files": 836, + "hash_dirs": 836, + "linked": 836, + "removed": 0 + }, + "timestamp": 1618998730.24672, + "total_parts": 523, + "total_time": 5.400741815567017 + }, + "sdb7": { + "parts_done": 506, + "policies": { + "1": { + "next_part_power": 11, + "part_power": 10, + "parts_done": 506, + "start_time": 1618998724.845616, + "stats": { + "errors": 0, + "files": 794, + "hash_dirs": 794, + "linked": 794, + "removed": 0 + }, + "step": "relink", + "timestamp": 1618998730.166175, + "total_parts": 506, + "total_time": 5.320528984069824 + } + }, + "start_time": 1618998724.845616, + "stats": { + "errors": 0, + "files": 794, + "hash_dirs": 794, + "linked": 794, + "removed": 0 + }, + "timestamp": 1618998730.166175, + "total_parts": 506, + "total_time": 5.320528984069824 + } + }, + "workers": { + "100": { + "drives": ["sda1"], + "return_code": 0, + "timestamp": 1618998730.166175} + }} + self.fakecache.fakeout_calls = [] + self.fakecache.fakeout = from_cache_response + rv = self.app.get_relinker_info() + self.assertEqual(self.fakecache.fakeout_calls, + [((['devices', 'workers'], + '/var/cache/swift/relinker.recon'), + {'ignore_missing': True})]) + self.assertEqual(rv, from_cache_response) + class TestReconMiddleware(unittest.TestCase): @@ -1222,6 +1334,7 @@ class TestReconMiddleware(unittest.TestCase): self.app.get_driveaudit_error = self.frecon.fake_driveaudit self.app.get_time = self.frecon.fake_time self.app.get_sharding_info = self.frecon.fake_sharding + self.app.get_relinker_info = self.frecon.fake_relinker def test_recon_get_mem(self): get_mem_resp = [b'{"memtest": "1"}'] @@ -1497,6 +1610,14 @@ class TestReconMiddleware(unittest.TestCase): resp = self.app(req.environ, start_response) self.assertEqual(resp, get_sharding_resp) + def test_recon_get_relink(self): + get_recon_resp = [ + b'{"relinktest": "1"}'] + req = Request.blank('/recon/relinker', + environ={'REQUEST_METHOD': 'GET'}) + resp = self.app(req.environ, start_response) + self.assertEqual(resp, get_recon_resp) + if __name__ == '__main__': unittest.main() diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index 6dad72299d..5582edf8d4 100644 --- a/test/unit/common/test_utils.py +++ b/test/unit/common/test_utils.py @@ -6611,7 +6611,7 @@ class TestAuditLocationGenerator(unittest.TestCase): dev_dir = os.path.join(devices, 'device_is_empty_dir') os.makedirs(dev_dir) - def assert_listdir_error(devices): + def assert_listdir_error(devices, expected): logger = debug_logger() error_counter = {} locations = utils.audit_location_generator( @@ -6620,19 +6620,23 @@ class TestAuditLocationGenerator(unittest.TestCase): ) self.assertEqual([], list(locations)) self.assertEqual(1, len(logger.get_lines_for_level('warning'))) - self.assertEqual({'unlistable_partitions': 1}, error_counter) + self.assertEqual({'unlistable_partitions': expected}, + error_counter) # file under devices/ devices = os.path.join(tmpdir, 'devices3') os.makedirs(devices) with open(os.path.join(devices, 'device_is_file'), 'w'): pass - assert_listdir_error(devices) + listdir_error_data_dir = os.path.join(devices, 'device_is_file', + 'data') + assert_listdir_error(devices, [listdir_error_data_dir]) # dir under devices/ devices = os.path.join(tmpdir, 'devices4') device = os.path.join(devices, 'device') os.makedirs(device) + expected_datadir = os.path.join(devices, 'device', 'data') assert_no_errors(devices) # error for dir under devices/ @@ -6644,7 +6648,7 @@ class TestAuditLocationGenerator(unittest.TestCase): return orig_listdir(path) with mock.patch('swift.common.utils.listdir', mocked): - assert_listdir_error(devices) + assert_listdir_error(devices, [expected_datadir]) # mount check error devices = os.path.join(tmpdir, 'devices5') @@ -6669,7 +6673,7 @@ class TestAuditLocationGenerator(unittest.TestCase): ) self.assertEqual([], list(locations)) self.assertEqual(1, len(logger.get_lines_for_level('warning'))) - self.assertEqual({'unmounted': 1}, error_counter) + self.assertEqual({'unmounted': ['device']}, error_counter) class TestGreenAsyncPile(unittest.TestCase):