The branch, master has been updated via 79f5d05 ctdb-tools: Remove duplicate code via b71becc ctdb-scripts: Ignore shellcheck SC2181 warning (use of $?) via 2b0e266 ctdb-tools: Avoid shellcheck SC2181 warnings (use of $?) in onnode via aa12ea7 ctdb-tools: Use a clear and readable if-statement via 4dc41cd ctdb-tools: Reformat and explain complex code via 3654694 ctdb-tools: Avoid shellcheck SC2188 warning (redirect without command) via db014a5 ctdb-scripts: Avoid shellcheck warning SC2188 (redirect without command) via b171c09 ctdb-tests: Indentation fixups via e8c5d0e ctdb-tests: Fix logic to handle PATH additions for tests via 661426d ctdb-tests: Move die() function to top of script via ac1b1d8 ctdb-tests: run_tests.sh sets evironment variable CTDB_TEST_DIR from e2c0fd3 blackbox: Add test for 'net ads changetrustpw'
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 79f5d058469c62b92b5e6227230597a30b41353a Author: Martin Schwenke <mar...@meltin.net> Date: Thu Aug 10 20:23:09 2017 +1000 ctdb-tools: Remove duplicate code These lines are duplicates of those above. It has always been this way... Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> Autobuild-User(master): Amitay Isaacs <ami...@samba.org> Autobuild-Date(master): Mon Aug 14 09:00:45 CEST 2017 on sn-devel-144 commit b71becc1501f70f26ab854460d6f833d8f4b5302 Author: Martin Schwenke <mar...@meltin.net> Date: Fri Aug 11 12:49:32 2017 +1000 ctdb-scripts: Ignore shellcheck SC2181 warning (use of $?) Given the size of the command substitutions it would be less clear to embed the assignments and substitutions inside a conditional. It is clearer if the exit code is checked afterwards. However, do fix some untidy uses of != instead of -ne when comparing with $?. Make the code easier to understand by reversing the logic and using -eq and ||. Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> commit 2b0e266d07cf62c1cbbda182aae6cdc93777cb3b Author: Martin Schwenke <mar...@meltin.net> Date: Thu Jul 13 12:58:33 2017 +1000 ctdb-tools: Avoid shellcheck SC2181 warnings (use of $?) in onnode Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> commit aa12ea77937b17c2c84cd5fce94f946d0e55f91f Author: Martin Schwenke <mar...@meltin.net> Date: Fri Aug 11 14:06:30 2017 +1000 ctdb-tools: Use a clear and readable if-statement This is consistent with the if-statement above. Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> commit 4dc41cd2b636030e21991685788480a0ea0cdaa1 Author: Martin Schwenke <mar...@meltin.net> Date: Wed Aug 9 17:11:18 2017 +1000 ctdb-tools: Reformat and explain complex code There are multiple command groups and redirects on very long lines. Reformat the long lines to break them up and add a comment to explain what is happening. Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> commit 3654694a092b15733bd04631786e60627c392c6a Author: Martin Schwenke <mar...@meltin.net> Date: Thu Jul 13 13:08:39 2017 +1000 ctdb-tools: Avoid shellcheck SC2188 warning (redirect without command) Shellcheck found a bug! Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> commit db014a51295ca66f4aee058dd8ee63f0beea5527 Author: Martin Schwenke <mar...@meltin.net> Date: Thu Jul 13 12:52:39 2017 +1000 ctdb-scripts: Avoid shellcheck warning SC2188 (redirect without command) This makes the code look deliberate instead like something has been accidentally omitted. Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> commit b171c090ec9236f1e23d71fdce13656121d21ce6 Author: Martin Schwenke <mar...@meltin.net> Date: Thu Aug 3 21:01:59 2017 +1000 ctdb-tests: Indentation fixups The rest of the code in this file now matches the coding guidelines, so clean up the rest. Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> commit e8c5d0e25e850d9a5f2fbd6b95b38deabedc0c64 Author: Martin Schwenke <mar...@meltin.net> Date: Wed Mar 15 15:50:46 2017 +1100 ctdb-tests: Fix logic to handle PATH additions for tests When using non-standard test subdirectories, the current code can fail to find the test bin directory and stupidly just adds /bin to PATH. Switch to using CTDB_TESTS_ARE_INSTALLED along with some sanity checks to determine the mode of operation. With this change, test directories can now be created as subdirectories of arbitrary component directories. Tests can then be run directly, either by specifying the subdirectory or individual test cases. Integration into the top-level tests/ directory is then done via a symbolic link, which enables 2 things: * Ability to run a directory of test cases from top level by simply specifying the link name. * Ease of installation - the installation code just works with the symbolic link. Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> commit 661426df40abea0960ab3a0f55063dbb009ae6b6 Author: Martin Schwenke <mar...@meltin.net> Date: Thu Aug 3 20:57:47 2017 +1000 ctdb-tests: Move die() function to top of script So it can be called within the script instead of just by scripts that include it. Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> commit ac1b1d8c8a65e048c605d73f58dc22785f78181d Author: Martin Schwenke <mar...@meltin.net> Date: Thu Aug 3 20:36:57 2017 +1000 ctdb-tests: run_tests.sh sets evironment variable CTDB_TEST_DIR Instead of just local variable test_dir. The environment variable can be accessed from other test infrastructure scripts. Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> ----------------------------------------------------------------------- Summary of changes: ctdb/config/debug-hung-script.sh | 2 ++ ctdb/config/functions | 6 ++-- ctdb/tests/run_tests.sh | 6 ++-- ctdb/tests/scripts/common.sh | 60 +++++++++++++++++++++------------------- ctdb/tools/ctdb_diagnostics | 9 +++--- ctdb/tools/onnode | 47 +++++++++++++++++++++---------- ctdb/wscript | 5 ++-- 7 files changed, 80 insertions(+), 55 deletions(-) Changeset truncated at 500 lines: diff --git a/ctdb/config/debug-hung-script.sh b/ctdb/config/debug-hung-script.sh index da988c2..2e3792c 100755 --- a/ctdb/config/debug-hung-script.sh +++ b/ctdb/config/debug-hung-script.sh @@ -39,6 +39,8 @@ fi sed -r -n "s@.*-(.*(${pat}).*),([0-9]*).*@\3 \1@p" | while read pid name ; do trace=$(cat "/proc/${pid}/stack" 2>/dev/null) + # No! Checking the exit code afterwards is actually clearer... + # shellcheck disable=SC2181 if [ $? -eq 0 ] ; then echo "---- Stack trace of interesting process ${pid}[${name}] ----" echo "$trace" diff --git a/ctdb/config/functions b/ctdb/config/functions index 5df52c2..1131229 100755 --- a/ctdb/config/functions +++ b/ctdb/config/functions @@ -701,7 +701,7 @@ _ctdb_counter_common () { ctdb_counter_init () { _ctdb_counter_common "$1" - >"$_counter_file" + : >"$_counter_file" } ctdb_counter_incr () { _ctdb_counter_common "$1" @@ -747,7 +747,7 @@ ctdb_service_needs_reconfigure () ctdb_service_set_reconfigure () { _ctdb_service_reconfigure_common - >"$_ctdb_service_reconfigure_flag" + : >"$_ctdb_service_reconfigure_flag" } ctdb_service_unset_reconfigure () @@ -868,7 +868,7 @@ if ! type mktemp >/dev/null 2>&1 ; then if $_dir ; then mkdir "$_t" else - >"$_t" + : >"$_t" fi ) echo "$_t" diff --git a/ctdb/tests/run_tests.sh b/ctdb/tests/run_tests.sh index d5f3116..ffc81d4 100755 --- a/ctdb/tests/run_tests.sh +++ b/ctdb/tests/run_tests.sh @@ -232,13 +232,13 @@ find_and_run_one_test () # Following 2 lines may be modified by installation script export CTDB_TESTS_ARE_INSTALLED=false -test_dir=$(dirname "$0") +export CTDB_TEST_DIR=$(dirname "$0") if [ -z "$TEST_VAR_DIR" ] ; then if $CTDB_TESTS_ARE_INSTALLED ; then TEST_VAR_DIR=$(mktemp -d) else - TEST_VAR_DIR="${test_dir}/var" + TEST_VAR_DIR="${CTDB_TEST_DIR}/var" fi fi mkdir -p "$TEST_VAR_DIR" @@ -252,7 +252,7 @@ if $socket_wrapper ; then mkdir -p "$SOCKET_WRAPPER_DIR" fi -export TEST_SCRIPTS_DIR="${test_dir}/scripts" +export TEST_SCRIPTS_DIR="${CTDB_TEST_DIR}/scripts" # If no tests specified then run some defaults if [ -z "$1" ] ; then diff --git a/ctdb/tests/scripts/common.sh b/ctdb/tests/scripts/common.sh index 287fb71..93db417 100644 --- a/ctdb/tests/scripts/common.sh +++ b/ctdb/tests/scripts/common.sh @@ -2,44 +2,48 @@ # Common variables and functions for all CTDB tests. +# Print a message and exit. +die () +{ + echo "$1" >&2 ; exit ${2:-1} +} + # This expands the most probable problem cases like "." and "..". TEST_SUBDIR=$(dirname "$0") if [ $(dirname "$TEST_SUBDIR") = "." ] ; then - TEST_SUBDIR=$(cd "$TEST_SUBDIR" ; pwd) + TEST_SUBDIR=$(cd "$TEST_SUBDIR" ; pwd) fi -_test_dir=$(dirname "$TEST_SUBDIR") - # If we are running from within the source tree then, depending on the # tests that we're running, we may need to add the top level bin/ and -# tools/ subdirectories to $PATH. This means we need a way of -# determining if we're running from within the source tree. There is -# no use looking outside the tests/ subdirectory because anything -# above that level may be meaningless and outside our control. -# Therefore, we'll use existence of $_test_dir/run_tests.sh to -# indicate that we're running in-tree - on a system where the tests -# have been installed, this file will be absent (renamed and placed in -# some bin/ directory). -if [ -f "${_test_dir}/run_tests.sh" ] ; then - ctdb_dir=$(dirname "$_test_dir") - - _tools_dir="${ctdb_dir}/tools" - if [ -d "$_tools_dir" ] ; then - PATH="${_tools_dir}:$PATH" - fi +# tools/ subdirectories to $PATH. In this case, sanity check that +# run_tests.sh is in the expected place. If the tests are installed +# then sanity check that TEST_BIN_DIR is set. +if $CTDB_TESTS_ARE_INSTALLED ; then + if [ -z "$TEST_BIN_DIR" ] ; then + die "CTDB_TESTS_ARE_INSTALLED but TEST_BIN_DIR not set" + fi + + _test_bin_dir="$TEST_BIN_DIR" +else + if [ ! -f "${CTDB_TEST_DIR}/run_tests.sh" ] ; then + die "Tests not installed but can't find run_tests.sh" + fi + + ctdb_dir=$(dirname "$CTDB_TEST_DIR") + + _tools_dir="${ctdb_dir}/tools" + if [ -d "$_tools_dir" ] ; then + PATH="${_tools_dir}:$PATH" + fi + + _test_bin_dir="${ctdb_dir}/bin" fi -_test_bin_dir="${TEST_BIN_DIR:-${ctdb_dir}/bin}" case "$_test_bin_dir" in - /*) : ;; - *) _test_bin_dir="${PWD}/${_test_bin_dir}" ;; +/*) : ;; +*) _test_bin_dir="${PWD}/${_test_bin_dir}" ;; esac if [ -d "$_test_bin_dir" ] ; then - PATH="${_test_bin_dir}:$PATH" + PATH="${_test_bin_dir}:$PATH" fi - -# Print a message and exit. -die () -{ - echo "$1" >&2 ; exit ${2:-1} -} diff --git a/ctdb/tools/ctdb_diagnostics b/ctdb/tools/ctdb_diagnostics index e72c23f..ccaf859 100755 --- a/ctdb/tools/ctdb_diagnostics +++ b/ctdb/tools/ctdb_diagnostics @@ -26,7 +26,9 @@ parse_options () { temp=$(getopt -n "ctdb_diagnostics" -o "n:cwh" -l no-ads,help -- "$@") - [ $? != 0 ] && usage + # No! Checking the exit code afterwards is actually clearer... + # shellcheck disable=SC2181 + [ $? -eq 0 ] || usage eval set -- "$temp" @@ -81,7 +83,7 @@ fi # are the same on the nodes CONFIG_FILES_MAY="/usr/local/etc/ctdb/public_addresses /usr/local/etc/ctdb/static-routes" -2>&1 +exec 2>&1 cat <<EOF -------------------------------------------------------------------- @@ -249,9 +251,6 @@ show_all "ctdb -X getdbmap | awk -F'|' 'NR > 1 {print \$3}' | sort | xargs -n 1 echo "Showing log.ctdb" show_all "test -f /usr/local/var/log/log.ctdb && tail -100 /usr/local/var/log/log.ctdb" -echo "Showing log.ctdb" -show_all "test -f /usr/local/var/log/log.ctdb && tail -100 /usr/local/var/log/log.ctdb" - show_all "tail -200 /var/log/messages" show_all "ls -lRs /usr/local/var/lib/ctdb" show_all "ls -lRs /usr/local/etc/ctdb" diff --git a/ctdb/tools/onnode b/ctdb/tools/onnode index b3efe91..ca9673a 100755 --- a/ctdb/tools/onnode +++ b/ctdb/tools/onnode @@ -81,7 +81,9 @@ parse_options () # Not on the previous line - local returns 0! temp=$(POSIXLY_CORRECT=1 getopt -n "$prog" -o "cf:hno:pqvPi" -l help -- "$@") - [ $? != 0 ] && usage + # No! Checking the exit code afterwards is actually clearer... + # shellcheck disable=SC2181 + [ $? -eq 0 ] || usage eval set -- "$temp" @@ -147,6 +149,8 @@ get_nodes_with_status () if [ -z "$ctdb_status_output" ] ; then ctdb_status_output=$(ctdb -X status 2>&1) + # No! Checking the exit code afterwards is actually clearer... + # shellcheck disable=SC2181 if [ $? -ne 0 ] ; then echo "${prog}: unable to get status of CTDB nodes" >&2 echo "$ctdb_status_output" >&2 @@ -238,10 +242,9 @@ get_nodes () all_nodes=$(sed -e 's@#.*@@g' -e 's@ *@@g' -e 's@^$@#DEAD@' "$f") fi - local nodes="" - local n - for n in $(parse_nodespec "$1") ; do - [ $? != 0 ] && exit 1 # Required to catch exit in above subshell. + local n nodes + nodes=$(parse_nodespec "$1") || exit $? + for n in $nodes ; do case "$n" in all) echo "${all_nodes//#DEAD/}" @@ -337,8 +340,7 @@ fi ###################################################################### -nodes=$(get_nodes "$nodespec") -[ $? != 0 ] && exit 1 # Required to catch exit in above subshell. +nodes=$(get_nodes "$nodespec") || exit $? if $quiet ; then verbose=false @@ -359,24 +361,41 @@ trap 'kill -TERM $pids 2>/dev/null' INT TERM retcode=0 for n in $nodes ; do set -o pipefail 2>/dev/null + + # The following code applies stdout_filter and stderr_filter to + # the relevant streams. Both filters are at the end of pipes so + # they read from stdin and (by default) write to stdout. To allow + # the filters to operate independently, the output of + # stdout_filter is sent to a temporary file descriptor (3), which + # is redirected back to stdout at the outermost level. if $parallel ; then - { exec 3>&1 ; { $SSH $ssh_opts $EXTRA_SSH_OPTS "$n" "$command" | stdout_filter >&3 ; } 2>&1 | stderr_filter ; } & + { + exec 3>&1 + { + $SSH $ssh_opts $EXTRA_SSH_OPTS "$n" "$command" | + stdout_filter >&3 + } 2>&1 | stderr_filter + } & pids="${pids} $!" else if $verbose ; then echo >&2 ; echo ">> NODE: $n <<" >&2 fi - { exec 3>&1 ; { $SSH $ssh_opts $EXTRA_SSH_OPTS "$n" "$command" | stdout_filter >&3 ; } 2>&1 | stderr_filter ; } - [ $? = 0 ] || retcode=$? + { + exec 3>&1 + { + $SSH $ssh_opts $EXTRA_SSH_OPTS "$n" "$command" | + stdout_filter >&3 + } 2>&1 | stderr_filter + } || retcode=$? fi done -$parallel && { +if $parallel ; then for p in $pids; do - wait "$p" - [ $? = 0 ] || retcode=$? + wait "$p" || retcode=$? done -} +fi exit $retcode diff --git a/ctdb/wscript b/ctdb/wscript index 7197b2a..08b3f5b 100644 --- a/ctdb/wscript +++ b/ctdb/wscript @@ -887,8 +887,9 @@ def build(bld): 'script_install_paths.sh', destname='script_install_paths.sh', chmod=0644) - sed_expr1 = 's@^test_dir=.*@test_dir=%s\\nexport TEST_BIN_DIR=\"%s\"@' % ( - bld.env.CTDB_TEST_DATADIR, bld.env.CTDB_TEST_LIBEXECDIR) + sed_expr1 = 's@^\(export %s\)=.*@\\1=%s\\nexport %s=\"%s\"@''' % ( + 'CTDB_TEST_DIR', bld.env.CTDB_TEST_DATADIR, + 'TEST_BIN_DIR', bld.env.CTDB_TEST_LIBEXECDIR) sed_expr2 = 's@^\(export CTDB_TESTS_ARE_INSTALLED\)=false@\\1=true@' bld.SAMBA_GENERATOR('ctdb-test-runner', source='tests/run_tests.sh', -- Samba Shared Repository