THRIFT-4351: change DOCKER_ env vars for travis integration to work better,

ready for enabling job on PRs

This closes #1397

5 files changed, 38 insertions, 60 deletions

diff --git a/.travis.yml b/.travis.yml
index 0f9500c6e..9a94e3649 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -21,25 +21,8 @@
 
 #
 # Docker Integration
+# see: build/docker/README.md
 # 
-# DOCKER_REPO
-#   [required] The repository name (within your account).
-# DOCKER_PASS
-#   [optional, secure] Your docker account password.
-#   If you do not set this, each build job in the "test"
-#   stage might rebuild the docker image and extend build
-#   time by about 10 minutes per job.  If you do set this
-#   then each build job in the "docker" stage" will build
-#   the docker image if Dockerfile has changed, and then
-#   push the new tag up to your docker hub account.
-# DOCKER_USER
-#   [required] Your docker hub account name.
-#
-# The resulting tag is:
-#   $DOCKER_USER/$DOCKER_REPO:$DISTRO
-# example (and the default):
-#   apache/thrift:ubuntu-xenial
-#
 
 sudo: required
 dist: trusty
@@ -64,8 +47,10 @@ env:
     - DISTRO=ubuntu-xenial
     - BUILD_LIBS="CPP C_GLIB HASKELL JAVA PYTHON TESTING TUTORIALS"  # only meaningful for CMake builds
     - TRAVIS_BUILD_STAGE=test
-    - DEFAULT_DOCKER_USER="apache"
-    - DEFAULT_DOCKER_REPO="thrift"
+    # DOCKER_REPO (this works for all builds as a source for docker images - you can override for fork builds in your Travis settings)
+    - DOCKER_REPO="thrift/thrift-build"
+    # DOCKER_USER (provide in your Travis settings if you want to build and update docker images once, instead of on every job)
+    # DOCKER_PASS (same)
 
 jobs:
   include:
diff --git a/build/docker/README.md b/build/docker/README.md
index 81a4935d3..32fad92eb 100644
--- a/build/docker/README.md
+++ b/build/docker/README.md
@@ -11,13 +11,15 @@ The Travis CI scripts use the following environment variables and logic to deter
 | Variable | Default | Usage |
 | -------- | ----- | ------- |
 | `DISTRO` | `ubuntu-xenial` | Set by various build jobs in `.travis.yml` to run builds in different containers.  Not intended to be set externally.|
-| `DOCKER_REPO` | `thrift` | The name of the Docker Hub repository to obtain and store docker images. |
-| `DOCKER_USER` | `apache` | The Docker Hub account name containing the repository. |
+| `DOCKER_REPO` | `thrift/thrift-build` | The name of the Docker Hub repository to obtain and store docker images. |
+| `DOCKER_USER` | `<none>` | The Docker Hub account name containing the repository. |
 | `DOCKER_PASS` | `<none>` | The Docker Hub account password to use when pushing new tags. |
 
-For example, the default docker image that is used in builds if no overrides are specified would be: `apache/thrift:ubuntu-xenial`
+For example, the default docker image that is used in builds if no overrides are specified would be: `thrift/thrift-build:ubuntu-xenial`
 
-If you have forked the Apache Thrift repository and you would like to use your own Docker Hub account to store thrift build images, you can use the Travis CI web interface to set the `DOCKER_USER`, `DOCKER_PASS`, and `DOCKER_REPO` variables in a secure manner.
+### Forks ###
+
+If you have forked the Apache Thrift repository and you would like to use your own Docker Hub account to store thrift build images, you can use the Travis CI web interface to set the `DOCKER_USER`, `DOCKER_PASS`, and `DOCKER_REPO` variables in a secure manner.  Your fork builds will then pull, push, and tag the docker images in your account.
 
 ### Logic ###
 
@@ -27,36 +29,42 @@ The Travis CI build runs in two phases - first the docker images are rebuilt for
 
 The Travis CI (continuous integration) builds use the Ubuntu Trusty, Xenial, and Artful images to maximize language level coverage.
 
-### Ubuntu
+### Ubuntu ###
+
 * trusty (legacy)
 * xenial (stable)
 * artful (latest)
 
-## Unsupported Containers
+## Unsupported Containers ##
 
 These containers may be in various states, and may not build everything.
 
-### CentOS
+### CentOS ###
 * 7.3
   * make check in lib/py may hang in test_sslsocket - root cause unknown
 
-### Debian
+### Debian ###
+
 * jessie
 * stretch
   * make check in lib/cpp fails due to https://svn.boost.org/trac10/ticket/12507
 
-## Usage
+## Building Locally ##
+
 From the Apache Thrift code base root:
 
-* Build
+* Build the image
 
 	docker build -t thrift build/docker/ubuntu-xenial
 
-* Run
+* Open a command prompt in the image
 
 	docker run -v $(pwd):/thrift/src -it thrift /bin/bash
 
-## Core Tool Versions per Dockerfile
+## Core Tool Versions per Dockerfile ##
+
+Last updated: October 1, 2017
+
 | Tool      | ubuntu-trusty | ubuntu-xenial | ubuntu-artful | Notes |
 | :-------- | :------------ | :------------ | :------------ | :---- |
 | ant       | 1.9.3         | 1.9.6         | 1.9.9         |       |
@@ -74,7 +82,10 @@ From the Apache Thrift code base root:
 | openssl   | 1.0.1f        | 1.0.2g        | 1.0.2g        |       |
 | qt5       | 5.2.1         | 5.5.1         | 5.9.1         |       |
 
-## Compiler/Language Versions per Dockerfile
+## Compiler/Language Versions per Dockerfile ##
+
+Last updated: October 1, 2017
+
 | Language  | ubuntu-trusty | ubuntu-xenial | ubuntu-artful | Notes |
 | :-------- | :------------ | :------------ | :------------ | :---- |
 | as3       |               |               |               | Not in CI |
diff --git a/build/docker/refresh.sh b/build/docker/refresh.sh
index 20a443b35..08cbc91e1 100755
--- a/build/docker/refresh.sh
+++ b/build/docker/refresh.sh
@@ -31,7 +31,7 @@
 set -e
 
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
-. $SCRIPT_DIR/vars.sh
+DOCKER_TAG=$DOCKER_REPO:$DISTRO
 
 function dockerfile_changed {
   # image may not exist yet, so we have to let it fail silently:
@@ -63,15 +63,20 @@ fi
 popd
 
 #
-# Dockerfile has changed
+# Dockerfile has changed - rebuild it for the current build job.
+# If it is a "docker" stage build then we want to push it back
+# to the DOCKER_REPO.  If it is a "test" stage build then we do
+# not.  If nobody defined a DOCKER_PASS then it doesn't matter.
 #
 
 echo Rebuilding docker image $DISTRO
 docker build --tag $DOCKER_TAG build/docker/$DISTRO
 
-if [[ ! -z "$DOCKER_PASS" ]]; then 
+if [[ "$TRAVIS_BUILD_STAGE" == "docker" ]] && [[ ! -z "$DOCKER_USER" ]] && [[ ! -z "$DOCKER_PASS" ]]; then 
   echo Pushing docker image $DOCKER_TAG
   docker login -u $DOCKER_USER -p $DOCKER_PASS
   docker push $DOCKER_TAG
+else
+  echo Not pushing docker image: either not a docker stage build job, or one of DOCKER_USER or DOCKER_PASS is undefined.
 fi
 
diff --git a/build/docker/run.sh b/build/docker/run.sh
index b54924bc9..1fe19d563 100755
--- a/build/docker/run.sh
+++ b/build/docker/run.sh
@@ -21,7 +21,7 @@
 set -e
 
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
-. $SCRIPT_DIR/vars.sh
+DOCKER_TAG=$DOCKER_REPO:$DISTRO
 
 printenv | sort
 
diff --git a/build/docker/vars.sh b/build/docker/vars.sh
deleted file mode 100755
index 752e2825a..000000000
--- a/build/docker/vars.sh
+++ /dev/null
@@ -1,23 +0,0 @@
-#!/bin/bash
-#
-# Licensed to the Apache Software Foundation (ASF) under one
-# or more contributor license agreements. See the NOTICE file
-# distributed with this work for additional information
-# regarding copyright ownership. The ASF licenses this file
-# to you under the Apache License, Version 2.0 (the
-# "License"); you may not use this file except in compliance
-# with the License. You may obtain a copy of the License at
-#
-#   http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing,
-# software distributed under the License is distributed on an
-# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-# KIND, either express or implied. See the License for the
-# specific language governing permissions and limitations
-# under the License.
-#
-
-[[ ! -z "$DOCKER_USER" ]] || export DOCKER_USER=$DEFAULT_DOCKER_USER
-[[ ! -z "$DOCKER_REPO" ]] || export DOCKER_REPO=$DEFAULT_DOCKER_REPO
-[[ ! -z "$DOCKER_TAG" ]]  || export DOCKER_TAG=$DOCKER_USER/$DOCKER_REPO:$DISTRO