From 39326a04a3b8747416c910409fd7e677b27de32c Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Fri, 15 Apr 2016 10:39:35 -0400 Subject: [PATCH 1/2] Refactored stacktrace implementation for bash scripts This changes the prior implementation of the stacktrace to use the new stacktrace implementation as well as the newly-implemented Bash trap framework for the ERR signal. In order to make the stacktrace output more useful, especially for someone attempting to re-run the commands used, we evaluate all of the variables in the $BASH_COMMAND as best we can. We will not be able to ever correctly evaluate positional variables ($1, $2, $@) or variables created in the local scopes of the trap handler or in the stacktrace function such as $return_code, $last_command, or $errexit_set. For this reason, we print both $BASH_COMMAND and our attempt to substitute variables. Signed-off-by: Steve Kuznetsov --- hack/build-base-images.sh | 2 +- hack/build-cross.sh | 2 +- hack/build-go.sh | 2 +- hack/build-images.sh | 2 +- hack/build-release.sh | 2 +- hack/cherry-pick.sh | 2 +- hack/gen-bootstrap-bindata.sh | 2 +- hack/gen-swagger-docs.sh | 2 +- hack/install-etcd.sh | 2 +- hack/lib/log/stacktrace.sh | 100 ++++++++++++++++++ hack/lib/util/misc.sh | 96 +++++++++++++++++ hack/lib/util/trap.sh | 99 +++++++++++++++++ hack/move-upstream.sh | 2 +- hack/test-cmd.sh | 2 +- hack/test-end-to-end.sh | 2 +- hack/test-go.sh | 2 +- hack/test-integration.sh | 2 +- hack/test-lib.sh | 2 +- hack/test-lib/test/junit.sh | 2 +- hack/test-tools.sh | 2 +- hack/test-util.sh | 2 +- hack/update-generated-swagger-descriptions.sh | 2 +- hack/update-generated-swagger-spec.sh | 2 +- hack/util.sh | 73 ------------- hack/verify-open-ports.sh | 2 +- test/cmd/admin.sh | 2 +- test/cmd/authentication.sh | 2 +- test/cmd/basicresources.sh | 2 +- test/cmd/builds.sh | 2 +- test/cmd/debug.sh | 2 +- test/cmd/deployments.sh | 2 +- test/cmd/diagnostics.sh | 2 +- test/cmd/edit.sh | 2 +- test/cmd/export.sh | 2 +- test/cmd/help.sh | 2 +- test/cmd/images.sh | 2 +- test/cmd/newapp.sh | 2 +- test/cmd/policy.sh | 2 +- test/cmd/process.sh | 2 +- test/cmd/router.sh | 2 +- test/cmd/secrets.sh | 2 +- test/cmd/templates.sh | 2 +- test/cmd/triggers.sh | 2 +- test/cmd/volumes.sh | 2 +- test/end-to-end/core.sh | 2 +- test/extended/alternate_certs.sh | 2 +- test/extended/alternate_launches.sh | 2 +- test/extended/cmd.sh | 2 +- test/extended/ldap_groups.sh | 2 +- test/extended/networking.sh | 2 +- test/extended/setup.sh | 2 +- 51 files changed, 342 insertions(+), 120 deletions(-) create mode 100644 hack/lib/log/stacktrace.sh create mode 100644 hack/lib/util/misc.sh create mode 100644 hack/lib/util/trap.sh diff --git a/hack/build-base-images.sh b/hack/build-base-images.sh index 7e639c6e0a94..0e210d7c3eb1 100755 --- a/hack/build-base-images.sh +++ b/hack/build-base-images.sh @@ -12,7 +12,7 @@ set -o pipefail STARTTIME=$(date +%s) OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install # Go to the top of the tree. cd "${OS_ROOT}" diff --git a/hack/build-cross.sh b/hack/build-cross.sh index 277117181189..1885454f306f 100755 --- a/hack/build-cross.sh +++ b/hack/build-cross.sh @@ -9,7 +9,7 @@ set -o pipefail STARTTIME=$(date +%s) OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install platforms=( "${OS_CROSS_COMPILE_PLATFORMS[@]}" ) if [[ -n "${OS_ONLY_BUILD_PLATFORMS-}" ]]; then diff --git a/hack/build-go.sh b/hack/build-go.sh index 2885cd62daf2..cfa9ab10ddd3 100755 --- a/hack/build-go.sh +++ b/hack/build-go.sh @@ -9,7 +9,7 @@ set -o pipefail STARTTIME=$(date +%s) OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install # only works on Linux for now, all other platforms must build binaries themselves if [[ -z "$@" ]]; then diff --git a/hack/build-images.sh b/hack/build-images.sh index ea2f6cef5ee6..bb39d0821e22 100755 --- a/hack/build-images.sh +++ b/hack/build-images.sh @@ -15,7 +15,7 @@ STARTTIME=$(date +%s) OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" source "${OS_ROOT}/contrib/node/install-sdn.sh" -os::log::install_errexit +os::log::stacktrace::install # Go to the top of the tree. cd "${OS_ROOT}" diff --git a/hack/build-release.sh b/hack/build-release.sh index befa4853e209..815f287301b7 100755 --- a/hack/build-release.sh +++ b/hack/build-release.sh @@ -12,7 +12,7 @@ set -o pipefail STARTTIME=$(date +%s) OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install # Go to the top of the tree. cd "${OS_ROOT}" diff --git a/hack/cherry-pick.sh b/hack/cherry-pick.sh index bf28393567ec..e62ebb3f0ccd 100755 --- a/hack/cherry-pick.sh +++ b/hack/cherry-pick.sh @@ -8,7 +8,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install # Go to the top of the tree. cd "${OS_ROOT}" diff --git a/hack/gen-bootstrap-bindata.sh b/hack/gen-bootstrap-bindata.sh index 0d7b9394e0dc..d34890d2c7af 100755 --- a/hack/gen-bootstrap-bindata.sh +++ b/hack/gen-bootstrap-bindata.sh @@ -7,7 +7,7 @@ set -o pipefail STARTTIME=$(date +%s) OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install EXAMPLES=examples OUTPUT_PARENT=${OUTPUT_ROOT:-$OS_ROOT} diff --git a/hack/gen-swagger-docs.sh b/hack/gen-swagger-docs.sh index 501a8e9eb5bd..3d0d4ee67fc5 100755 --- a/hack/gen-swagger-docs.sh +++ b/hack/gen-swagger-docs.sh @@ -9,7 +9,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/.. cd "${OS_ROOT}" source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install pushd "${OS_ROOT}/hack/swagger-doc" > /dev/null gradle gendocs --info diff --git a/hack/install-etcd.sh b/hack/install-etcd.sh index 328c6e3c01b9..b74baf4334b4 100755 --- a/hack/install-etcd.sh +++ b/hack/install-etcd.sh @@ -8,7 +8,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install etcd_version=$(go run ${OS_ROOT}/tools/godepversion/godepversion.go ${OS_ROOT}/Godeps/Godeps.json github.com/coreos/etcd/etcdserver) diff --git a/hack/lib/log/stacktrace.sh b/hack/lib/log/stacktrace.sh new file mode 100644 index 000000000000..798c10e3b2d0 --- /dev/null +++ b/hack/lib/log/stacktrace.sh @@ -0,0 +1,100 @@ +#!/bin/bash +# +# This library contains an implementation of a stack trace for Bash scripts. + +# os::log::stacktrace::install installs the stacktrace as a handler for the ERR signal if one +# has not already been installed and sets `set -o errtrace` in order to propagate the handler +# If the ERR trap is not initialized, installing this plugin will initialize it. +# +# Globals: +# None +# Arguments: +# None +# Returns: +# - export OS_USE_STACKTRACE +function os::log::stacktrace::install() { + # setting 'errtrace' propagates our ERR handler to functions, expansions and subshells + set -o errtrace + + # OS_USE_STACKTRACE is read by os::util::trap at runtime to request a stacktrace + export OS_USE_STACKTRACE=true + + os::util::trap::init_err +} +readonly -f os::log::stacktrace::install + +# os::log::stacktrace::print prints the stacktrace and exits with the return code from the script that +# called for a stack trace. This function will always return 0 if it is not handling the signal, and if it +# is handling the signal, this function will always `exit`, not return, the return code it receives as +# its first argument. +# +# Globals: +# - BASH_SOURCE +# - BASH_LINENO +# - FUNCNAME +# Arguments: +# - 1: the return code of the command in the script that generated the ERR signal +# - 2: the last command that ran before handlers were invoked +# - 3: whether or not `set -o errexit` was set in the script that generated the ERR signal +# Returns: +# None +function os::log::stacktrace::print() { + local return_code=$1 + local last_command=$2 + local errexit_set=${3:-} + set -- + + # evaulate all of the variables in the last command literal so we have a useful stacktrace + # this will *not* be able to capture positional variables ($1, $@, $*) or variables declared + # in this scope or in the trap handler ($return_code, $last_command, $errexit_set) + # Furthermore, since it is possible that the failure in the last command itself was an unset + # variable, we need to turn off that check to ensure we're not re-triggering it here. + set +o nounset + local -r last_command_with_vars="$( eval "echo \"${last_command}\"" )" + set -o nounset + + if [[ "${return_code}" = "0" ]]; then + # we're not supposed to respond when no error has occurred + return 0 + fi + + if [[ -z "${errexit_set}" ]]; then + # if errexit wasn't set in the shell when the ERR signal was issued, then we can ignore the signal + # as this is not cause for failure + return 0 + fi + + # iterate backwards through the stack until we leave library files, so we can be sure we start logging + # actual script code and not this handler's call + local stack_begin_index + for (( stack_begin_index = 0; stack_begin_index < ${#BASH_SOURCE[@]}; stack_begin_index++ )); do + if [[ ! "${BASH_SOURCE[${stack_begin_index}]}" =~ hack/lib/(log/stacktrace|util/trap)\.sh ]]; then + break + fi + done + + local preamble_finished + local stack_index=1 + local i + for (( i = stack_begin_index; i < ${#BASH_SOURCE[@]}; i++ )); do + local bash_source + bash_source="$( os::util::repository_relative_path "${BASH_SOURCE[$i]}" )" + if [[ -z "${preamble_finished:-}" ]]; then + preamble_finished=true + os::log::error "PID ${BASHPID:-$$}: ${bash_source}:${BASH_LINENO[$i-1]}: \`${last_command}\` exited with status ${return_code}." >&2 + os::log::error "Command with variables substituted is: " + os::log::error $'\t'"${last_command_with_vars}" + os::log::info $'\t\t'"Stack Trace: " >&2 + os::log::info $'\t\t'" ${stack_index}: ${bash_source}:${BASH_LINENO[$i-1]}: \`${last_command}\`" >&2 + else + os::log::info $'\t\t'" ${stack_index}: ${bash_source}:${BASH_LINENO[$i-1]}: ${FUNCNAME[$i-1]}" >&2 + fi + stack_index=$(( stack_index + 1 )) + done + + # we know we're the privileged handler in this chain, so we can safely exit the shell without + # starving another handler of the privilege of reacting to this signal + os::log::info " Exiting with code ${return_code}." >&2 + exit "${return_code}" +} +readonly -f os::log::stacktrace::print \ No newline at end of file diff --git a/hack/lib/util/misc.sh b/hack/lib/util/misc.sh new file mode 100644 index 000000000000..2caaf836d516 --- /dev/null +++ b/hack/lib/util/misc.sh @@ -0,0 +1,96 @@ +#!/bin/bash +# +# This library holds miscellaneous utility functions. If there begin to be groups of functions in this +# file that share intent or are thematically similar, they should be split into their own files. + +# os::util::describe_return_code describes an exit code +# +# Globals: +# - OS_SCRIPT_START_TIME +# Arguments: +# - 1: exit code to describe +# Returns: +# None +function os::util::describe_return_code() { + local return_code=$1 + + if [[ "${return_code}" = "0" ]]; then + echo -n "[INFO] $0 succeeded " + else + echo -n "[ERROR] $0 failed " + fi + + if [[ -n "${OS_SCRIPT_START_TIME:-}" ]]; then + local end_time + end_time="$(date +%s)" + local elapsed_time + elapsed_time="$(( end_time - OS_SCRIPT_START_TIME ))" + local formatted_time + formatted_time="$( os::util::format_seconds "${elapsed_time}" )" + echo "after ${formatted_time}" + else + echo + fi +} +readonly -f os::util::describe_return_code + +# os::util::install_describe_return_code installs the return code describer for the EXIT trap +# If the EXIT trap is not initialized, installing this plugin will initialize it. +# +# Globals: +# None +# Arguments: +# None +# Returns: +# - export OS_DESCRIBE_RETURN_CODE +# - export OS_SCRIPT_START_TIME +function os::util::install_describe_return_code() { + export OS_DESCRIBE_RETURN_CODE="true" + OS_SCRIPT_START_TIME="$( date +%s )"; export OS_SCRIPT_START_TIME + os::util::trap::init_exit +} +readonly -f os::util::install_describe_return_code + +# os::util::repository_relative_path returns the relative path from the $OS_ROOT directory to the +# given file, if the file is inside of the $OS_ROOT directory. If the file is outside of $OS_ROOT, +# this function will return the absolute path to the file +# +# Globals: +# - OS_ROOT +# Arguments: +# - 1: the path to relativize +# Returns: +# None +function os::util::repository_relative_path() { + local filename=$1 + + if which realpath >/dev/null 2>&1; then + local trim_path + trim_path="$( realpath "${OS_ROOT}" )/" + filename="$( realpath "${filename}" )" + filename="${filename##*${trim_path}}" + fi + + echo "${filename}" +} +readonly -f os::util::repository_relative_path + +# os::util::format_seconds formats a duration of time in seconds to print in HHh MMm SSs +# +# Globals: +# None +# Arguments: +# - 1: time in seconds to format +# Return: +# None +function os::util::format_seconds() { + local raw_seconds=$1 + + local hours minutes seconds + (( hours=raw_seconds/3600 )) + (( minutes=(raw_seconds%3600)/60 )) + (( seconds=raw_seconds%60 )) + + printf '%02dh %02dm %02ds' "${hours}" "${minutes}" "${seconds}" +} +readonly -f os::util::format_seconds \ No newline at end of file diff --git a/hack/lib/util/trap.sh b/hack/lib/util/trap.sh new file mode 100644 index 000000000000..6f19bb6aaaf4 --- /dev/null +++ b/hack/lib/util/trap.sh @@ -0,0 +1,99 @@ +#!/bin/bash +# +# This library defines the trap handlers for the ERR and EXIT signals. Any new handler for these signals +# must be added to these handlers and activated by the environment variable mechanism that the rest use. +# These functions ensure that no handler can ever alter the exit code that was emitted by a command +# in a test script. + +# os::util::trap::init_err initializes the privileged handler for the ERR signal if it hasn't +# been registered already. This will overwrite any other handlers registered on the signal. +# +# Globals: +# None +# Arguments: +# None +# Returns: +# None +function os::util::trap::init_err() { + if ! trap -p ERR | grep -q 'os::util::trap::err_handler'; then + trap 'os::util::trap::err_handler;' ERR + fi +} +readonly -f os::util::trap::init_err + +# os::util::trap::init_exit initializes the privileged handler for the EXIT signal if it hasn't +# been registered already. This will overwrite any other handlers registered on the signal. +# +# Globals: +# None +# Arguments: +# None +# Returns: +# None +function os::util::trap::init_exit() { + if ! trap -p EXIT | grep -q 'os::util::trap::exit_handler'; then + trap 'os::util::trap::exit_handler;' EXIT + fi +} +readonly -f os::util::trap::init_exit + +# os::util::trap::err_handler is the handler for the ERR signal. +# +# Globals: +# - OS_TRAP_DEBUG +# - OS_USE_STACKTRACE +# Arguments: +# None +# Returns: +# - returns original return code, allows privileged handler to exit if necessary +function os::util::trap::err_handler() { + local -r return_code=$? + local -r last_command="${BASH_COMMAND}" + + if set +o | grep -q '\-o errexit'; then + local -r errexit_set=true + fi + + if [[ "${OS_TRAP_DEBUG:-}" = "true" ]]; then + echo "[DEBUG] Error handler executing with return code \`${return_code}\`, last command \`${last_command}\`, and errexit set \`${errexit_set:-}\`" + fi + + if [[ "${OS_USE_STACKTRACE:-}" = "true" ]]; then + # the OpenShift stacktrace function is treated as a privileged handler for this signal + # and is therefore allowed to run outside of a subshell in order to allow it to `exit` + # if necessary + os::log::stacktrace::print "${return_code}" "${last_command}" "${errexit_set:-}" + fi + + return "${return_code}" +} +readonly -f os::util::trap::err_handler + +# os::util::trap::exit_handler is the handler for the EXIT signal. +# +# Globals: +# - OS_TRAP_DEBUG +# - OS_DESCRIBE_RETURN_CODE +# Arguments: +# None +# Returns: +# - original exit code of the script that exited +function os::util::trap::exit_handler() { + local -r return_code=$? + + # we do not want these traps to be able to trigger more errors, we can let them fail silently + set +o errexit + + if [[ "${OS_TRAP_DEBUG:-}" = "true" ]]; then + echo "[DEBUG] Exit handler executing with return code \`${return_code}\`" + fi + + # the following envars selectively enable optional exit traps, all of which are run inside of + # a subshell in order to sandbox them and not allow them to influence how this script will exit + if [[ "${OS_DESCRIBE_RETURN_CODE:-}" = "true" ]]; then + ( os::util::describe_return_code "${return_code}" ) + fi + + exit "${return_code}" +} +readonly -f os::util::trap::exit_handler \ No newline at end of file diff --git a/hack/move-upstream.sh b/hack/move-upstream.sh index 1d06b2f87cf5..ab8758d872ad 100755 --- a/hack/move-upstream.sh +++ b/hack/move-upstream.sh @@ -13,7 +13,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install # Go to the top of the tree. cd "${OS_ROOT}" diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index 745578b7868d..ba9d11342e98 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -11,7 +11,7 @@ STARTTIME=$(date +%s) OS_ROOT=$(dirname "${BASH_SOURCE}")/.. cd "${OS_ROOT}" source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install os::util::environment::setup_time_vars function cleanup() diff --git a/hack/test-end-to-end.sh b/hack/test-end-to-end.sh index faf3b9c25663..96da64412e08 100755 --- a/hack/test-end-to-end.sh +++ b/hack/test-end-to-end.sh @@ -20,7 +20,7 @@ if [[ "${TEST_END_TO_END:-}" != "direct" ]]; then fi source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install ensure_iptables_or_die diff --git a/hack/test-go.sh b/hack/test-go.sh index 2e15f4e3fb94..318f6ada4dd9 100755 --- a/hack/test-go.sh +++ b/hack/test-go.sh @@ -44,7 +44,7 @@ start_time=$(date +%s) OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" cd "${OS_ROOT}" -os::log::install_errexit +os::log::stacktrace::install os::build::setup_env os::util::environment::setup_tmpdir_vars "test-go" diff --git a/hack/test-integration.sh b/hack/test-integration.sh index 379aa2723361..dfde0547488a 100755 --- a/hack/test-integration.sh +++ b/hack/test-integration.sh @@ -7,7 +7,7 @@ set -o pipefail STARTTIME=$(date +%s) OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install # Go to the top of the tree. cd "${OS_ROOT}" diff --git a/hack/test-lib.sh b/hack/test-lib.sh index 3c17f2d245b6..3080bfb355e9 100755 --- a/hack/test-lib.sh +++ b/hack/test-lib.sh @@ -26,7 +26,7 @@ trap exit_trap EXIT start_time=$(date +%s) OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install os::util::environment::setup_tmpdir_vars "test-lib" cd "${OS_ROOT}" diff --git a/hack/test-lib/test/junit.sh b/hack/test-lib/test/junit.sh index ae62cc98a671..622025ef2e60 100755 --- a/hack/test-lib/test/junit.sh +++ b/hack/test-lib/test/junit.sh @@ -26,7 +26,7 @@ trap exit_trap EXIT start_time=$(date +%s) OS_ROOT="$( dirname "${BASH_SOURCE}" )"/../../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install # envars used to track these interactions are not propagated out of the subshells used to run these commands # therefore each os::cmd call is its own sandbox and complicated scenarios need to play out inside one call diff --git a/hack/test-tools.sh b/hack/test-tools.sh index 80e57913ee58..5765c1d81150 100755 --- a/hack/test-tools.sh +++ b/hack/test-tools.sh @@ -10,7 +10,7 @@ STARTTIME=$(date +%s) OS_ROOT=$(dirname "${BASH_SOURCE}")/.. cd "${OS_ROOT}" source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install os::test::junit::declare_suite_start 'tools' diff --git a/hack/test-util.sh b/hack/test-util.sh index 67ba68f418b5..c747112ae67c 100755 --- a/hack/test-util.sh +++ b/hack/test-util.sh @@ -8,7 +8,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT BASETMPDIR="${TMPDIR:-/tmp}/openshift/test-tools" diff --git a/hack/update-generated-swagger-descriptions.sh b/hack/update-generated-swagger-descriptions.sh index f030ffe0cdfc..bf4bb28fc103 100755 --- a/hack/update-generated-swagger-descriptions.sh +++ b/hack/update-generated-swagger-descriptions.sh @@ -15,7 +15,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/.. cd "${OS_ROOT}" source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install # read in envar options verify="${VERIFY:-}" diff --git a/hack/update-generated-swagger-spec.sh b/hack/update-generated-swagger-spec.sh index 68fe10ff48c1..a185f50c088a 100755 --- a/hack/update-generated-swagger-spec.sh +++ b/hack/update-generated-swagger-spec.sh @@ -8,7 +8,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install function cleanup() { diff --git a/hack/util.sh b/hack/util.sh index ee35b2737f1d..42b4366d828e 100644 --- a/hack/util.sh +++ b/hack/util.sh @@ -722,79 +722,6 @@ readonly -f disable-selinux # end of common functions for extended test group's run.sh scripts ###### -# Handler for when we exit automatically on an error. -# Borrowed from https://gist.github.com/ahendrix/7030300 -function os::log::errexit() { - local err="${PIPESTATUS[@]}" - - # If the shell we are in doesn't have errexit set (common in subshells) then - # don't dump stacks. - set +o | grep -qe "-o errexit" || return - - set +o xtrace - local code="${1:-1}" - os::log::error_exit "'${BASH_COMMAND}' exited with status $err" "${1:-1}" 1 -} -readonly -f os::log::errexit - -function os::log::install_errexit() { - # trap ERR to provide an error handler whenever a command exits nonzero this - # is a more verbose version of set -o errexit - trap 'os::log::errexit' ERR - - # setting errtrace allows our ERR trap handler to be propagated to functions, - # expansions and subshells - set -o errtrace -} -readonly -f os::log::install_errexit - -# Print out the stack trace -# -# Args: -# $1 The number of stack frames to skip when printing. -function os::log::stack() { - local stack_skip=${1:-0} - stack_skip=$((stack_skip + 1)) - if [[ ${#FUNCNAME[@]} -gt $stack_skip ]]; then - echo "Call stack:" >&2 - local i - for ((i=1 ; i <= ${#FUNCNAME[@]} - $stack_skip ; i++)) - do - local frame_no=$((i - 1 + stack_skip)) - local source_file=${BASH_SOURCE[$frame_no]} - local source_lineno=${BASH_LINENO[$((frame_no - 1))]} - local funcname=${FUNCNAME[$frame_no]} - echo " $i: ${source_file}:${source_lineno} ${funcname}(...)" >&2 - done - fi -} -readonly -f os::log::stack - -# Log an error and exit. -# Args: -# $1 Message to log with the error -# $2 The error code to return -# $3 The number of stack frames to skip when printing. -function os::log::error_exit() { - local message="${1:-}" - local code="${2:-1}" - local stack_skip="${3:-0}" - stack_skip=$((stack_skip + 1)) - - local source_file=${BASH_SOURCE[$stack_skip]} - local source_line=${BASH_LINENO[$((stack_skip - 1))]} - echo "!!! Error in ${source_file}:${source_line}" >&2 - [[ -z ${1-} ]] || { - echo " ${1}" >&2 - } - - os::log::stack $stack_skip - - echo "Exiting with status ${code}" >&2 - exit "${code}" -} -readonly -f os::log::error_exit - function os::log::with-severity() { local msg=$1 local severity=$2 diff --git a/hack/verify-open-ports.sh b/hack/verify-open-ports.sh index d7a0b3168d03..492ffc3f73a9 100755 --- a/hack/verify-open-ports.sh +++ b/hack/verify-open-ports.sh @@ -9,7 +9,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install # Open port scanning echo "[INFO] Checking open ports ('sudo openshift start' should already be running)" diff --git a/test/cmd/admin.sh b/test/cmd/admin.sh index 216c3cc39357..5ec0ef9d028b 100755 --- a/test/cmd/admin.sh +++ b/test/cmd/admin.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/authentication.sh b/test/cmd/authentication.sh index 706a262325cb..016ff6442026 100755 --- a/test/cmd/authentication.sh +++ b/test/cmd/authentication.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT os::test::junit::declare_suite_start "cmd/authentication" diff --git a/test/cmd/basicresources.sh b/test/cmd/basicresources.sh index 96b1b26ad5bf..fc6cea0e5809 100755 --- a/test/cmd/basicresources.sh +++ b/test/cmd/basicresources.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/builds.sh b/test/cmd/builds.sh index 76864d41aa10..bb1d6f230a4b 100755 --- a/test/cmd/builds.sh +++ b/test/cmd/builds.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/debug.sh b/test/cmd/debug.sh index 06f5e0399234..2b175875c99f 100755 --- a/test/cmd/debug.sh +++ b/test/cmd/debug.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/deployments.sh b/test/cmd/deployments.sh index 0b63207930ec..d64c1d0f5e7d 100755 --- a/test/cmd/deployments.sh +++ b/test/cmd/deployments.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/diagnostics.sh b/test/cmd/diagnostics.sh index ab01787b210f..db0275988ab6 100755 --- a/test/cmd/diagnostics.sh +++ b/test/cmd/diagnostics.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # This test validates the diagnostics command diff --git a/test/cmd/edit.sh b/test/cmd/edit.sh index fa59591ecbb8..8f3a043a9fc7 100755 --- a/test/cmd/edit.sh +++ b/test/cmd/edit.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/export.sh b/test/cmd/export.sh index 0e732547c784..252e37d5ab39 100755 --- a/test/cmd/export.sh +++ b/test/cmd/export.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/help.sh b/test/cmd/help.sh index c76b31d756cf..6119971a3093 100755 --- a/test/cmd/help.sh +++ b/test/cmd/help.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT os::test::junit::declare_suite_start "cmd/help" diff --git a/test/cmd/images.sh b/test/cmd/images.sh index 233edca97a32..654c4efe4f9b 100755 --- a/test/cmd/images.sh +++ b/test/cmd/images.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/newapp.sh b/test/cmd/newapp.sh index 78c312883b1a..ecb4371818eb 100755 --- a/test/cmd/newapp.sh +++ b/test/cmd/newapp.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/policy.sh b/test/cmd/policy.sh index e3863a931352..89785e139de0 100755 --- a/test/cmd/policy.sh +++ b/test/cmd/policy.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT os::test::junit::declare_suite_start "cmd/policy" diff --git a/test/cmd/process.sh b/test/cmd/process.sh index b1f990fb26fb..c5dfea91828e 100755 --- a/test/cmd/process.sh +++ b/test/cmd/process.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install # Cleanup cluster resources created by this test ( diff --git a/test/cmd/router.sh b/test/cmd/router.sh index f56ff9e6b0c9..605ae0ba65bc 100755 --- a/test/cmd/router.sh +++ b/test/cmd/router.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/secrets.sh b/test/cmd/secrets.sh index 6e8eb3e24228..0b131cdbd012 100755 --- a/test/cmd/secrets.sh +++ b/test/cmd/secrets.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/templates.sh b/test/cmd/templates.sh index 4a5be1e79401..1cb4e934ebe9 100755 --- a/test/cmd/templates.sh +++ b/test/cmd/templates.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/triggers.sh b/test/cmd/triggers.sh index 5adb1d5da187..bfeea58d1320 100755 --- a/test/cmd/triggers.sh +++ b/test/cmd/triggers.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/cmd/volumes.sh b/test/cmd/volumes.sh index e2032644ef39..533885fff474 100755 --- a/test/cmd/volumes.sh +++ b/test/cmd/volumes.sh @@ -6,7 +6,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install trap os::test::junit::reconcile_output EXIT # Cleanup cluster resources created by this test diff --git a/test/end-to-end/core.sh b/test/end-to-end/core.sh index d2aa68a3f37c..ac2e1e1975d4 100755 --- a/test/end-to-end/core.sh +++ b/test/end-to-end/core.sh @@ -9,7 +9,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install os::util::environment::setup_time_vars trap os::test::junit::reconcile_output EXIT diff --git a/test/extended/alternate_certs.sh b/test/extended/alternate_certs.sh index f684c9d8851c..e214ddf693d6 100755 --- a/test/extended/alternate_certs.sh +++ b/test/extended/alternate_certs.sh @@ -9,7 +9,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. cd "${OS_ROOT}" source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install os::util::environment::setup_all_server_vars "test-extended-alternate-certs/" reset_tmp_dir diff --git a/test/extended/alternate_launches.sh b/test/extended/alternate_launches.sh index 45c77418e14f..c326767830f1 100755 --- a/test/extended/alternate_launches.sh +++ b/test/extended/alternate_launches.sh @@ -11,7 +11,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. cd "${OS_ROOT}" source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install os::util::environment::setup_all_server_vars "test-extended-alternate-launches/" reset_tmp_dir diff --git a/test/extended/cmd.sh b/test/extended/cmd.sh index a459a291c971..f19a88396f73 100755 --- a/test/extended/cmd.sh +++ b/test/extended/cmd.sh @@ -11,7 +11,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install os::util::environment::setup_time_vars cd "${OS_ROOT}" diff --git a/test/extended/ldap_groups.sh b/test/extended/ldap_groups.sh index ad18a6c53942..3b7ebdb2400c 100755 --- a/test/extended/ldap_groups.sh +++ b/test/extended/ldap_groups.sh @@ -10,7 +10,7 @@ set -o pipefail OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install os::util::environment::setup_time_vars cd "${OS_ROOT}" diff --git a/test/extended/networking.sh b/test/extended/networking.sh index b6d33336ae87..7b265a8681b0 100755 --- a/test/extended/networking.sh +++ b/test/extended/networking.sh @@ -17,7 +17,7 @@ export SHELLOPTS OS_ROOT=$(dirname "${BASH_SOURCE}")/../.. source "${OS_ROOT}/hack/lib/init.sh" -os::log::install_errexit +os::log::stacktrace::install NETWORKING_DEBUG=${NETWORKING_DEBUG:-false} diff --git a/test/extended/setup.sh b/test/extended/setup.sh index 79f16ddfcf7d..f2c609076143 100644 --- a/test/extended/setup.sh +++ b/test/extended/setup.sh @@ -22,7 +22,7 @@ function os::test::extended::focus { # be done in other contexts. function os::test::extended::setup { source "${OS_ROOT}/hack/lib/init.sh" - os::log::install_errexit + os::log::stacktrace::install # build binaries if [[ -z $(os::build::find-binary ginkgo) ]]; then From 62e2e0aba421407e15ed8f67b660251657d53a6f Mon Sep 17 00:00:00 2001 From: Steve Kuznetsov Date: Fri, 29 Apr 2016 15:42:22 -0400 Subject: [PATCH 2/2] Refactored binary build shell invocation By changing the amount of code that is literally contained in the sub-shell `()` braces, we allow the stacktrace that is generated to contain much less text on any one line, so that it is read-able and useful. Furthermore, by naming the positional variables we pass around we allow the new stack trace code to correctly evaluate them and show us what went wrong, exactly. Signed-off-by: Steve Kuznetsov --- hack/common.sh | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/hack/common.sh b/hack/common.sh index 663105fa7f47..7fae813eac3a 100755 --- a/hack/common.sh +++ b/hack/common.sh @@ -249,7 +249,7 @@ function os::build::build_static_binaries() { } readonly -f os::build::build_static_binaries -# Build binaries targets specified +# Build binary targets specified # # Input: # $@ - targets and go flags. If no targets are set then all binaries targets @@ -257,8 +257,19 @@ readonly -f os::build::build_static_binaries # OS_BUILD_PLATFORMS - Incoming variable of targets to build for. If unset # then just the host architecture is built. function os::build::build_binaries() { + local -a binaries=( "$@" ) # Create a sub-shell so that we don't pollute the outer environment - ( + ( os::build::internal::build_binaries "${binaries[@]+"${binaries[@]}"}" ) +} + +# Build binary targets specified. Should always be run in a sub-shell so we don't leak GOBIN +# +# Input: +# $@ - targets and go flags. If no targets are set then all binaries targets +# are built. +# OS_BUILD_PLATFORMS - Incoming variable of targets to build for. If unset +# then just the host architecture is built. +os::build::internal::build_binaries() { # Check for `go` binary and set ${GOPATH}. os::build::setup_env @@ -325,7 +336,6 @@ function os::build::build_binaries() { "$(dirname ${test})" done done - ) } readonly -f os::build::build_binaries