From 523bc0ab71a4d976782a03c2beb46e5e8a25706b Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Fri, 17 Jun 2016 15:03:25 -0700 Subject: [PATCH] Always set swift processes to use UTC Previously, we would set the TZ environment variable to the result of time.strftime("%z", time.gmtime()) This has a few problems. 1. The "%z" format does not appear in the table of formatting directives for strftime [1]. While it *does* appear in a footnote [2] for that section, it is described as "not supported by all ANSI C libraries." This may explain the next point. 2. On the handful of Linux platforms I've tested, the above produces "+0000" regardless of the system's timezone. This seems to run counter to the intent of the patches that introduced the TZ mangling. (See the first two related changes.) 3. The above does not produce a valid Posix TZ format, which expects (at minimum) a name consisting of three or more alphabetic characters followed by the offset to be added to the local time to get Coordinated Universal Time (UTC). Further, while we would change os.environ['TZ'], we would *not* call time.tzset like it says in the docs [3], which seems like a Bad Thing. Some combination of the above has the net effect of changing some of the functions in the time module to use UTC. (Maybe all of them? At the very least, time.localtime and time.mktime.) However, it does *not* change the offset stored in time.timezone, which causes bad behavior when dealing with local timestamps [4]. Now, set TZ to "UTC+0" and call tzset. Apparently we don't have a good way of getting local timezone info, we were (unintentionally?) using UTC before, and you should probably be running your servers in UTC anyway. [1] https://docs.python.org/2/library/time.html#time.strftime [2] https://docs.python.org/2/library/time.html#id2 [3] https://docs.python.org/2/library/time.html#time.tzset [4] Like in email.utils.mktime_tz, prior to being fixed in https://hg.python.org/cpython/rev/a283563c8cc4 Change-Id: I007425301914144e228b9cfece5533443e851b6e Related-Change: Ifc78236a99ed193a42389e383d062b38f57a5a31 Related-Change: I8ec80202789707f723abfe93ccc9cf1e677e4dc6 Related-Change: Iee7488d03ab404072d3d0c1a262f004bb0f2da26 --- swift/common/daemon.py | 7 +++---- swift/common/wsgi.py | 7 +++---- test/unit/common/test_daemon.py | 31 ++++++++++++++++++++++++++++--- test/unit/common/test_wsgi.py | 13 ++++++++----- 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/swift/common/daemon.py b/swift/common/daemon.py index 39fc66c805..67004fe51d 100644 --- a/swift/common/daemon.py +++ b/swift/common/daemon.py @@ -109,10 +109,9 @@ def run_daemon(klass, conf_file, section_name='', once=False, **kwargs): eventlet.debug.hub_exceptions(eventlet_debug) # Ensure TZ environment variable exists to avoid stat('/etc/localtime') on - # some platforms. This locks in reported times to the timezone in which - # the server first starts running in locations that periodically change - # timezones. - os.environ['TZ'] = time.strftime("%z", time.gmtime()) + # some platforms. This locks in reported times to UTC. + os.environ['TZ'] = 'UTC+0' + time.tzset() try: klass(conf).run(once=once, **kwargs) diff --git a/swift/common/wsgi.py b/swift/common/wsgi.py index fb33fdb720..341f360e02 100644 --- a/swift/common/wsgi.py +++ b/swift/common/wsgi.py @@ -398,10 +398,9 @@ def loadapp(conf_file, global_conf=None, allow_modify_pipeline=True): def run_server(conf, logger, sock, global_conf=None): # Ensure TZ environment variable exists to avoid stat('/etc/localtime') on - # some platforms. This locks in reported times to the timezone in which - # the server first starts running in locations that periodically change - # timezones. - os.environ['TZ'] = time.strftime("%z", time.gmtime()) + # some platforms. This locks in reported times to UTC. + os.environ['TZ'] = 'UTC+0' + time.tzset() wsgi.HttpProtocol.default_request_version = "HTTP/1.0" # Turn off logging requests by the underlying WSGI software. diff --git a/test/unit/common/test_daemon.py b/test/unit/common/test_daemon.py index 92376ad8b2..24220c1729 100644 --- a/test/unit/common/test_daemon.py +++ b/test/unit/common/test_daemon.py @@ -18,6 +18,7 @@ import os from six import StringIO from six.moves import reload_module +import time import unittest from getpass import getuser import logging @@ -104,9 +105,11 @@ class TestRunDaemon(unittest.TestCase): sample_conf = "[my-daemon]\nuser = %s\n" % getuser() with tmpfile(sample_conf) as conf_file: with mock.patch.dict('os.environ', {'TZ': ''}): - daemon.run_daemon(MyDaemon, conf_file) - self.assertEqual(MyDaemon.forever_called, True) - self.assertTrue(os.environ['TZ'] is not '') + with mock.patch('time.tzset') as mock_tzset: + daemon.run_daemon(MyDaemon, conf_file) + self.assertTrue(MyDaemon.forever_called) + self.assertEqual(os.environ['TZ'], 'UTC+0') + self.assertEqual(mock_tzset.mock_calls, [mock.call()]) daemon.run_daemon(MyDaemon, conf_file, once=True) self.assertEqual(MyDaemon.once_called, True) @@ -133,6 +136,28 @@ class TestRunDaemon(unittest.TestCase): daemon.run_daemon, MyDaemon, conf_file, once=True) + def test_run_daemon_diff_tz(self): + old_tz = os.environ.get('TZ', '') + try: + os.environ['TZ'] = 'EST+05EDT,M4.1.0,M10.5.0' + time.tzset() + self.assertEqual((1970, 1, 1, 0, 0, 0), time.gmtime(0)[:6]) + self.assertEqual((1969, 12, 31, 19, 0, 0), time.localtime(0)[:6]) + self.assertEqual(18000, time.timezone) + + sample_conf = "[my-daemon]\nuser = %s\n" % getuser() + with tmpfile(sample_conf) as conf_file: + daemon.run_daemon(MyDaemon, conf_file) + self.assertFalse(MyDaemon.once_called) + self.assertTrue(MyDaemon.forever_called) + + self.assertEqual((1970, 1, 1, 0, 0, 0), time.gmtime(0)[:6]) + self.assertEqual((1970, 1, 1, 0, 0, 0), time.localtime(0)[:6]) + self.assertEqual(0, time.timezone) + finally: + os.environ['TZ'] = old_tz + time.tzset() + if __name__ == '__main__': unittest.main() diff --git a/test/unit/common/test_wsgi.py b/test/unit/common/test_wsgi.py index d63e0975d6..93e7725cda 100644 --- a/test/unit/common/test_wsgi.py +++ b/test/unit/common/test_wsgi.py @@ -469,11 +469,14 @@ class TestWSGI(unittest.TestCase): with mock.patch('swift.common.wsgi.eventlet') as _eventlet: with mock.patch.dict('os.environ', {'TZ': ''}): with mock.patch('swift.common.wsgi.inspect'): - conf = wsgi.appconfig(conf_dir) - logger = logging.getLogger('test') - sock = listen(('localhost', 0)) - wsgi.run_server(conf, logger, sock) - self.assertTrue(os.environ['TZ'] is not '') + with mock.patch('time.tzset') as mock_tzset: + conf = wsgi.appconfig(conf_dir) + logger = logging.getLogger('test') + sock = listen(('localhost', 0)) + wsgi.run_server(conf, logger, sock) + self.assertEqual(os.environ['TZ'], 'UTC+0') + self.assertEqual(mock_tzset.mock_calls, + [mock.call()]) self.assertEqual('HTTP/1.0', _wsgi.HttpProtocol.default_request_version)