From a3f28e29a2922f0a045a1c98dfa2cac7708066b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Lis=C3=A1k?= Date: Mon, 11 Jul 2016 10:15:19 +0200 Subject: [PATCH] Add a jitter for check of time synchronization. If a request time of check is smaller than a time diff (between local and remote server) and that time diff is tolerable small then it still prints: !! http://server:port/recon/time current time is 2016-07-05 06:24:02, but remote is 2016-07-05 06:24:02, differs by 0.00 sec So you can use a jitter option to consider it as time synchronized: swift-recon -T --jitter 0.01 Change-Id: I4891397e7cc0817819201b361f79f8fe7fdcfc89 --- swift/cli/recon.py | 14 ++++--- test/unit/cli/test_recon.py | 77 +++++++++++++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/swift/cli/recon.py b/swift/cli/recon.py index cd96c60c8f..33f8a2799e 100644 --- a/swift/cli/recon.py +++ b/swift/cli/recon.py @@ -878,14 +878,16 @@ class SwiftRecon(object): host = urlparse(url).netloc.split(':')[0] print('%.02f%% %s' % (used, '%-15s %s' % (host, device))) - def time_check(self, hosts): + def time_check(self, hosts, jitter=0.0): """ Check a time synchronization of hosts with current time :param hosts: set of hosts to check. in the format of: set([('127.0.0.1', 6020), ('127.0.0.2', 6030)]) + :param jitter: Maximal allowed time jitter """ + jitter = abs(jitter) matches = 0 errors = 0 recon = Scout("time", self.verbose, self.suppress_errors, @@ -896,13 +898,13 @@ class SwiftRecon(object): if status != 200: errors = errors + 1 continue - if (ts_remote < ts_start or ts_remote > ts_end): + if (ts_remote + jitter < ts_start or ts_remote - jitter > ts_end): diff = abs(ts_end - ts_remote) ts_end_f = self._ptime(ts_end) ts_remote_f = self._ptime(ts_remote) print("!! %s current time is %s, but remote is %s, " - "differs by %.2f sec" % ( + "differs by %.4f sec" % ( url, ts_end_f, ts_remote_f, @@ -987,6 +989,8 @@ class SwiftRecon(object): help="Get drive audit error stats") args.add_option('--time', '-T', action="store_true", help="Check time synchronization") + args.add_option('--jitter', type="float", default=0.0, + help="Maximal allowed time jitter") args.add_option('--top', type='int', metavar='COUNT', default=0, help='Also show the top COUNT entries in rank order.') args.add_option('--lowest', type='int', metavar='COUNT', default=0, @@ -1064,7 +1068,7 @@ class SwiftRecon(object): self.socket_usage(hosts) self.server_type_check(hosts) self.driveaudit_check(hosts) - self.time_check(hosts) + self.time_check(hosts, options.jitter) else: if options.async: if self.server_type == 'object': @@ -1113,7 +1117,7 @@ class SwiftRecon(object): if options.driveaudit: self.driveaudit_check(hosts) if options.time: - self.time_check(hosts) + self.time_check(hosts, options.jitter) def main(): diff --git a/test/unit/cli/test_recon.py b/test/unit/cli/test_recon.py index 4532c680ba..925d8cfe75 100644 --- a/test/unit/cli/test_recon.py +++ b/test/unit/cli/test_recon.py @@ -894,12 +894,12 @@ class TestReconCommands(unittest.TestCase): def dummy_request(*args, **kwargs): return [ - ('http://127.0.0.1:6010/recon/load', + ('http://127.0.0.1:6010/recon/time', now, 200, now - 0.5, now + 0.5), - ('http://127.0.0.1:6020/recon/load', + ('http://127.0.0.1:6020/recon/time', now, 200, now, @@ -944,7 +944,7 @@ class TestReconCommands(unittest.TestCase): default_calls = [ mock.call("!! http://127.0.0.1:6010/recon/time current time is " "2015-04-25 22:13:21, but remote is " - "2015-04-25 22:13:20, differs by 1.30 sec"), + "2015-04-25 22:13:20, differs by 1.3000 sec"), mock.call('1/2 hosts matched, 0 error[s] while checking hosts.'), ] @@ -954,6 +954,77 @@ class TestReconCommands(unittest.TestCase): # that is returned from the recon middleware, thus can't rely on it mock_print.assert_has_calls(default_calls, any_order=True) + @mock.patch('six.moves.builtins.print') + @mock.patch('time.time') + def test_time_check_jitter(self, mock_now, mock_print): + now = 1430000000.0 + mock_now.return_value = now + + def dummy_request(*args, **kwargs): + return [ + ('http://127.0.0.1:6010/recon/time', + now - 2, + 200, + now, + now + 3), + ('http://127.0.0.1:6020/recon/time', + now + 2, + 200, + now - 3, + now), + ] + + cli = recon.SwiftRecon() + cli.pool.imap = dummy_request + + default_calls = [ + mock.call('2/2 hosts matched, 0 error[s] while checking hosts.') + ] + + cli.time_check([('127.0.0.1', 6010), ('127.0.0.1', 6020)], 3) + # We need any_order=True because the order of calls depends on the dict + # that is returned from the recon middleware, thus can't rely on it + mock_print.assert_has_calls(default_calls, any_order=True) + + @mock.patch('six.moves.builtins.print') + @mock.patch('time.time') + def test_time_check_jitter_mismatch(self, mock_now, mock_print): + now = 1430000000.0 + mock_now.return_value = now + + def dummy_request(*args, **kwargs): + return [ + ('http://127.0.0.1:6010/recon/time', + now - 4, + 200, + now, + now + 2), + ('http://127.0.0.1:6020/recon/time', + now + 4, + 200, + now - 2, + now), + ] + + cli = recon.SwiftRecon() + cli.pool.imap = dummy_request + + default_calls = [ + mock.call("!! http://127.0.0.1:6010/recon/time current time is " + "2015-04-25 22:13:22, but remote is " + "2015-04-25 22:13:16, differs by 6.0000 sec"), + mock.call("!! http://127.0.0.1:6020/recon/time current time is " + "2015-04-25 22:13:20, but remote is " + "2015-04-25 22:13:24, differs by 4.0000 sec"), + mock.call('0/2 hosts matched, 0 error[s] while checking hosts.'), + ] + + cli.time_check([('127.0.0.1', 6010), ('127.0.0.1', 6020)], 3) + + # We need any_order=True because the order of calls depends on the dict + # that is returned from the recon middleware, thus can't rely on it + mock_print.assert_has_calls(default_calls, any_order=True) + @mock.patch('six.moves.builtins.print') @mock.patch('swift.cli.recon.SwiftRecon.get_hosts') def test_multiple_server_types(self, mock_get_hosts, mock_print):