You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/08/07 00:56:10 UTC

[3/5] impala git commit: IMPALA-7385: Fix test-with-docker errors having to do with time zones.

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 <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/cf5de097
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/cf5de097
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/cf5de097

Branch: refs/heads/master
Commit: cf5de09761c21aee4f3d571a94fbee5bda306a97
Parents: e6bf4dc
Author: Philip Zeyliger <ph...@cloudera.com>
Authored: Mon Jul 30 16:43:09 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Mon Aug 6 22:41:02 2018 +0000

----------------------------------------------------------------------
 docker/entrypoint.sh       | 28 +++++++++-------------------
 docker/test-with-docker.py | 10 ++++++++--
 2 files changed, 17 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/cf5de097/docker/entrypoint.sh
----------------------------------------------------------------------
diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh
index 382ca65..0e192c9 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
 }
 

http://git-wip-us.apache.org/repos/asf/impala/blob/cf5de097/docker/test-with-docker.py
----------------------------------------------------------------------
diff --git a/docker/test-with-docker.py b/docker/test-with-docker.py
index 3da700f..dc3bf95 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"]