Merge pull request #14534 from stevekuznetsov/skuznets/cleanup-image-…

…bash Bring image building logic into the Bash library
openshift · Jun 15, 2017 · 250f921 · 250f921
2 parents 22f308e + 9753bf5
commit 250f921
Show file tree

Hide file tree

Showing 5 changed files with 155 additions and 120 deletions.
diff --git a/hack/build-base-images.sh b/hack/build-base-images.sh
@@ -11,7 +11,7 @@ tag_prefix="${OS_IMAGE_PREFIX:-"openshift/origin"}"
 os::util::ensure::gopath_binary_exists imagebuilder
 
 # Build the base image without the default image args
-OS_BUILD_IMAGE_ARGS="${OS_BUILD_IMAGE_BASE_ARGS-}" os::build::image "${OS_ROOT}/images/source" "${tag_prefix}-source"
-OS_BUILD_IMAGE_ARGS="${OS_BUILD_IMAGE_BASE_ARGS-}" os::build::image "${OS_ROOT}/images/base" "${tag_prefix}-base"
+os::build::image "${tag_prefix}-source" "${OS_ROOT}/images/source"
+os::build::image "${tag_prefix}-base"  "${OS_ROOT}/images/base"
 
 ret=$?; ENDTIME=$(date +%s); echo "$0 took $(($ENDTIME - $STARTTIME)) seconds"; exit "$ret"
diff --git a/hack/build-dind-images.sh b/hack/build-dind-images.sh
@@ -8,8 +8,8 @@
 STARTTIME=$(date +%s)
 source "$(dirname "${BASH_SOURCE}")/lib/init.sh"
 
-os::build::image "${OS_ROOT}/images/dind" openshift/dind
-os::build::image "${OS_ROOT}/images/dind/node" openshift/dind-node
-os::build::image "${OS_ROOT}/images/dind/master" openshift/dind-master
+os::build::image openshift/dind "${OS_ROOT}/images/dind"
+os::build::image openshift/dind-node "${OS_ROOT}/images/dind/node"
+os::build::image openshift/dind-master "${OS_ROOT}/images/dind/master"
 
 ret=$?; ENDTIME=$(date +%s); echo "$0 took $(($ENDTIME - $STARTTIME)) seconds"; exit "$ret"
diff --git a/hack/build-images.sh b/hack/build-images.sh
@@ -6,9 +6,15 @@
 # NOTE: you only need to run this script if your code changes are part of
 # any images OpenShift runs internally such as origin-sti-builder, origin-docker-builder,
 # origin-deployer, etc.
-STARTTIME=$(date +%s)
 source "$(dirname "${BASH_SOURCE}")/lib/init.sh"
 
+function cleanup() {
+ return_code=$?
+ os::util::describe_return_code "${return_code}"
+ exit "${return_code}"
+}
+trap "cleanup" EXIT
+
 os::util::ensure::gopath_binary_exists imagebuilder
 # image builds require RPMs to have been built
 os::build::release::check_for_rpms
@@ -32,54 +38,6 @@ function ln_or_cp {
  fi
 }
 
-
-# image-build is wrapped to allow output to be captured
-function image-build() {
- local tag=$1
- local dir=$2
- local dest="${tag}"
- local extra=
- if [[ ! "${tag}" == *":"* ]]; then
- dest="${tag}:latest"
- # tag to release commit unless we specified a hardcoded tag
- extra="${tag}:${OS_RELEASE_COMMIT}"
- fi
-
- local STARTTIME
- local ENDTIME
- STARTTIME="$(date +%s)"
-
- # build the image
- if ! os::build::image "${dir}" "${dest}" "" "${extra}"; then
- os::log::warning "Retrying build once"
- if ! os::build::image "${dir}" "${dest}" "" "${extra}"; then
- return 1
- fi
- fi
-
- # ensure the temporary contents are cleaned up
- git clean -fdx "${dir}"
-
- ENDTIME="$(date +%s)"
- echo "Finished in $(($ENDTIME - $STARTTIME)) seconds"
-}
-
-# builds an image and tags it two ways - with latest, and with the release tag
-function image() {
- local tag=$1
- local dir=$2
- local out
- mkdir -p "${BASETMPDIR}"
- out="$( mktemp "${BASETMPDIR}/imagelogs.XXXXX" )"
- if ! image-build "${tag}" "${dir}" > "${out}" 2>&1; then
- sed -e "s|^|$1: |" "${out}" 1>&2
- os::log::error "Failed to build $1"
- return 1
- fi
- sed -e "s|^|$1: |" "${out}"
- return 0
-}
-
 # Link or copy image binaries to the appropriate locations.
 ln_or_cp "${OS_OUTPUT_BINPATH}/linux/amd64/hello-openshift" examples/hello-openshift/bin
 ln_or_cp "${OS_OUTPUT_BINPATH}/linux/amd64/gitserver" examples/gitserver/bin
@@ -88,32 +46,28 @@ ln_or_cp "${OS_OUTPUT_BINPATH}/linux/amd64/gitserver" examples/gitserver/b
 tag_prefix="${OS_IMAGE_PREFIX:-"openshift/origin"}"
 
 # images that depend on "${tag_prefix}-source"
-image "${tag_prefix}-pod" images/pod
-image "${tag_prefix}-cluster-capacity" images/cluster-capacity
-image "${tag_prefix}-service-catalog" images/service-catalog
+os::build::image "${tag_prefix}-pod" images/pod
+os::build::image "${tag_prefix}-cluster-capacity" images/cluster-capacity
+os::build::image "${tag_prefix}-service-catalog" images/service-catalog
 
 # images that depend on "${tag_prefix}-base"
-image "${tag_prefix}" images/origin
-image "${tag_prefix}-haproxy-router" images/router/haproxy
-image "${tag_prefix}-keepalived-ipfailover" images/ipfailover/keepalived
-image "${tag_prefix}-docker-registry" images/dockerregistry
-image "${tag_prefix}-egress-router" images/egress/router
-image "${tag_prefix}-egress-http-proxy" images/egress/http-proxy
-image "${tag_prefix}-federation" images/federation
+os::build::image "${tag_prefix}" images/origin
+os::build::image "${tag_prefix}-haproxy-router" images/router/haproxy
+os::build::image "${tag_prefix}-keepalived-ipfailover" images/ipfailover/keepalived
+os::build::image "${tag_prefix}-docker-registry" images/dockerregistry
+os::build::image "${tag_prefix}-egress-router" images/egress/router
+os::build::image "${tag_prefix}-egress-http-proxy" images/egress/http-proxy
+os::build::image "${tag_prefix}-federation" images/federation
 # images that depend on "${tag_prefix}
-image "${tag_prefix}-gitserver" examples/gitserver
-image "${tag_prefix}-deployer" images/deployer
-image "${tag_prefix}-recycler" images/recycler
-image "${tag_prefix}-docker-builder" images/builder/docker/docker-builder
-image "${tag_prefix}-sti-builder" images/builder/docker/sti-builder
-image "${tag_prefix}-f5-router" images/router/f5
-image "openshift/node" images/node
+os::build::image "${tag_prefix}-gitserver" examples/gitserver
+os::build::image "${tag_prefix}-deployer" images/deployer
+os::build::image "${tag_prefix}-recycler" images/recycler
+os::build::image "${tag_prefix}-docker-builder" images/builder/docker/docker-builder
+os::build::image "${tag_prefix}-sti-builder" images/builder/docker/sti-builder
+os::build::image "${tag_prefix}-f5-router" images/router/f5
+os::build::image "openshift/node" images/node
 # images that depend on "openshift/node"
-image "openshift/openvswitch" images/openvswitch
+os::build::image "openshift/openvswitch" images/openvswitch
 
 # extra images (not part of infrastructure)
-image "openshift/hello-openshift" examples/hello-openshift
-
-echo
-
-ret=$?; ENDTIME=$(date +%s); echo "$0 took $(($ENDTIME - $STARTTIME)) seconds"; exit "$ret"
+os::build::image "openshift/hello-openshift" examples/hello-openshift
diff --git a/hack/dind-cluster.sh b/hack/dind-cluster.sh
@@ -526,7 +526,7 @@ function build-image() {
  local build_root=$1
  local image_name=$2
 
- os::build::image "${build_root}" "${image_name}"
+ os::build::image "${image_name}" "${build_root}"
 }
 
 function os::build::get-bin-output-path() {

diff --git a/hack/lib/build/images.sh b/hack/lib/build/images.sh
@@ -2,47 +2,128 @@
 
 # This library holds utility functions for building container images.
 
-# os::build::image builds an image from a directory, to a tag, with an optional dockerfile to
-# use as the third argument. The environment variable OS_BUILD_IMAGE_ARGS adds additional
-# options to the command. The default is to use the imagebuilder binary if it is available
-# on the path with fallback to docker build if it is not available.
+# os::build::image builds an image from a directory, to a tag or tags The default
+# behavior is to use the imagebuilder binary if it is available on the path with
+# fallback to docker build if it is not available.
+#
+# Globals:
+# - OS_BUILD_IMAGE_ARGS
+# - OS_BUILD_IMAGE_NUM_RETRIES
+# Arguments:
+# - 1: the directory in which to build
+# - 2: the tag to apply to the image
+# Returns:
+# None
 function os::build::image() {
- local directory=$1
- local tag=$2
- local dockerfile="${3-}"
- local extra_tag="${4-}"
- local options="${OS_BUILD_IMAGE_ARGS-}"
- local mode="${OS_BUILD_IMAGE_TYPE:-imagebuilder}"
-
- if [[ "${mode}" == "imagebuilder" ]]; then
- if os::util::find::system_binary 'imagebuilder'; then
- if [[ -n "${extra_tag}" ]]; then
- extra_tag="-t '${extra_tag}'"
- fi
- if [[ -n "${dockerfile}" ]]; then
- eval "imagebuilder -f '${dockerfile}' -t '${tag}' ${extra_tag} ${options} '${directory}'"
- return $?
- fi
- eval "imagebuilder -t '${tag}' ${extra_tag} ${options} '${directory}'"
- return $?
- fi
-
- os::log::warning "Unable to locate 'imagebuilder' on PATH, falling back to Docker build"
- # clear options since we were unable to select imagebuilder
- options=""
- fi
-
- if [[ -n "${dockerfile}" ]]; then
- eval "docker build -f '${dockerfile}' -t '${tag}' ${options} '${directory}'"
- if [[ -n "${extra_tag}" ]]; then
- docker tag "${tag}" "${extra_tag}"
- fi
- return $?
- fi
- eval "docker build -t '${tag}' ${options} '${directory}'"
- if [[ -n "${extra_tag}" ]]; then
- docker tag "${tag}" "${extra_tag}"
- fi
- return $?
+ local tag=$1
+ local directory=$2
+ local extra_tag
+
+ if [[ ! "${tag}" == *":"* ]]; then
+ # if no tag was specified in the image name,
+ # tag with :latest and the release commit, if
+ # available, falling back to the last commit
+ # if no release commit is recorded
+ local release_commit
+ release_commit="${OS_RELEASE_COMMIT:-"$( git log -1 --pretty=%h )"}"
+ extra_tag="${tag}:${release_commit}"
+
+ tag="${tag}:latest"
+ fi
+
+ local result='1'
+ local image_build_log
+ image_build_log="$( mktemp "${BASETMPDIR}/imagelogs.XXXXX" )"
+ for (( i = 0; i < "${OS_BUILD_IMAGE_NUM_RETRIES:-2}"; i++ )); do
+ if [[ "${i}" -gt 0 ]]; then
+ os::log::warning "Retrying image build for ${tag}, attempt ${i}..."
+ fi
+
+ if os::build::image::internal::generic "${tag}" "${directory}" "${extra_tag:-}" >>"${image_build_log}" 2>&1; then
+ result='0'
+ break
+ fi
+ done
+
+ os::log::internal::prefix_lines "[${tag%:*}]" "$( cat "${image_build_log}" )"
+ return "${result}"
+}
+readonly -f os::build::image
+
+# os::build::image::internal::generic builds a container image using either imagebuilder
+# or docker, defaulting to imagebuilder if present
+#
+# Globals:
+# - OS_BUILD_IMAGE_ARGS
+# Arguments:
+# - 1: the directory in which to build
+# - 2: the tag to apply to the image
+# - 3: optionally, extra tags to add
+# Returns:
+# None
+function os::build::image::internal::generic() {
+ local directory=$2
+
+ if os::util::find::system_binary 'imagebuilder' >/dev/null; then
+ os::build::image::internal::imagebuilder "$@"
+ else
+ os::log::warning "Unable to locate 'imagebuilder' on PATH, falling back to Docker build"
+ os::build::image::internal::docker "$@"
+ fi
+
+ # ensure the temporary contents are cleaned up
+ git clean -fdx "${directory}"
+}
+readonly -f os::build::image::internal::generic
+
+# os::build::image::internal::imagebuilder builds a container image using imagebuilder
+#
+# Globals:
+# - OS_BUILD_IMAGE_ARGS
+# Arguments:
+# - 1: the directory in which to build
+# - 2: the tag to apply to the image
+# - 3: optionally, extra tags to add
+# Returns:
+# None
+function os::build::image::internal::imagebuilder() {
+ local tag=$1
+ local directory=$2
+ local extra_tag="${3-}"
+ local options=()
+
+ if [[ -n "${OS_BUILD_IMAGE_ARGS:-}" ]]; then
+ options=( ${OS_BUILD_IMAGE_ARGS} )
+ fi
+
+ if [[ -n "${extra_tag}" ]]; then
+ options+=( -t "${extra_tag}" )
+ fi
+
+ imagebuilder "${options[@]:-}" -t "${tag}" "${directory}"
+}
+readonly -f os::build::image::internal::imagebuilder
+
+# os::build::image::internal::docker builds a container image using docker
+#
+# Globals:
+# - OS_BUILD_IMAGE_ARGS
+# Arguments:
+# - 1: the directory in which to build
+# - 2: the tag to apply to the image
+# - 3: optionally, extra tags to add
+# Returns:
+# None
+function os::build::image::internal::docker() {
+ local tag=$1
+ local directory=$2
+ local extra_tag="${3-}"
+ local options=()
+
+ docker build ${OS_BUILD_IMAGE_ARGS:-} -t "${tag}" "${directory}"
+
+ if [[ -n "${extra_tag}" ]]; then
+ docker tag "${tag}" "${extra_tag}"
+ fi
 }
-readonly -f os::build::image
+readonly -f os::build::image::internal::docker