From cf5de09761c21aee4f3d571a94fbee5bda306a97 Mon Sep 17 00:00:00 2001 From: Philip Zeyliger Date: Mon, 30 Jul 2018 16:43:09 -0700 Subject: [PATCH] IMPALA-7385: Fix test-with-docker errors having to do with time zones. ExprTest.TimestampFunctions, query_test.test_scanners.TestOrc.test_type_conversions, and query_test.test_queries.TestHdfsQueries.test_hdfs_scan_node were all failing when using test-with-docker with mismatched dates. As it turns out, there is code that calls readlink(/etc/localtime) and parses the output to identify the current timezone name. This is described in localtime(5) on Ubuntu16: It should be an absolute or relative symbolic link pointing to /usr/share/zoneinfo/, followed by a timezone identifier such as "Europe/Berlin" or "Etc/UTC". ... Because the timezone identifier is extracted from the symlink target name of /etc/localtime, this file may not be a normal file or hardlink." To honor this requirement, and to make the tests pass, I re-jiggered how I pass the time zone information from the host into the container. The previously failing tests now pass. Change-Id: Ia9facfd9741806e7dbb868d8d06d9296bf86e52f Reviewed-on: http://gerrit.cloudera.org:8080/11106 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- docker/entrypoint.sh | 28 +++++++++------------------- docker/test-with-docker.py | 10 ++++++++-- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 382ca6535..0e192c98d 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -349,27 +349,17 @@ function test_suite() { return $ret } -# Ubuntu's tzdata package is very finnicky, and if you -# mount /etc/localtime from the host to the container directly, -# it fails to install. However, if you make it a symlink -# and configure /etc/timezone to something that's not an -# empty string, you'll get the right behavior. -# -# The post installation script is findable by looking for "tzdata.postinst" -# -# Use this command to reproduce the Ubuntu issue: -# docker run -v /etc/localtime:/mnt/localtime -ti ubuntu:16.04 bash -c ' -# date -# ln -sf /mnt/localtime /etc/localtime -# date +%Z > /etc/timezone -# date -# apt-get update > /dev/null -# apt-get install tzdata -# date' +# It's convenient (for log files to be legible) for the container +# to have the host timezone. However, /etc/localtime is finnicky +# (see localtime(5)) and mounting it to the host /etc/localtime or +# symlinking it there doesn't always work. Instead, we expect +# $LOCALTIME_LINK_TARGET to be set to a path in /usr/share/zoneinfo. function configure_timezone() { - if ! diff -q /etc/localtime /mnt/localtime 2> /dev/null; then - ln -sf /mnt/localtime /etc/localtime + if [ -e "${LOCALTIME_LINK_TARGET}" ]; then + ln -sf "${LOCALTIME_LINK_TARGET}" /etc/localtime date +%Z > /etc/timezone + else + echo '$LOCALTIME_LINK_TARGET not configured.' 1>&2 fi } diff --git a/docker/test-with-docker.py b/docker/test-with-docker.py index 3da700f7d..dc3bf956b 100755 --- a/docker/test-with-docker.py +++ b/docker/test-with-docker.py @@ -499,6 +499,12 @@ class TestWithDocker(object): if self.test_mode: extras = ["-e", "TEST_TEST_WITH_DOCKER=true"] + extras + # According to localtime(5), /etc/localtime is supposed + # to be a symlink to somewhere inside /usr/share/zoneinfo + assert os.path.islink("/etc/localtime") + localtime_link_target = os.path.realpath("/etc/localtime") + assert localtime_link_target.startswith("/usr/share/zoneinfo") + container_id = _check_output([ "docker", "create", # Required for some of the ntp handling in bootstrap and Kudu; @@ -526,9 +532,9 @@ class TestWithDocker(object): "-v", self.git_root + ":/repo:ro", "-v", self.git_common_dir + ":/git_common_dir:ro", "-e", "GIT_HEAD_REV=" + self.git_head_rev, - "-v", self.ccache_dir + ":/ccache", # Share timezone between host and container - "-v", "/etc/localtime:/mnt/localtime", + "-e", "LOCALTIME_LINK_TARGET=" + localtime_link_target, + "-v", self.ccache_dir + ":/ccache", "-v", _make_dir_if_not_exist(self.log_dir, logdir) + ":/logs", "-v", base + ":/mnt/base:ro"]