From 5b6842c8b703d5734e2c68ef90f80dad51205f6b Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 23 Aug 2016 10:20:51 +1000 Subject: [PATCH 1/5] ctdb-tests: Drop function _ctdb_hack_options() It does something special if the --start-as-stopped option is given and is not used in any tests. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12170 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit fd443b8cce4dedfe9a72f25e955274f66da41290) --- ctdb/tests/scripts/integration.bash | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/ctdb/tests/scripts/integration.bash b/ctdb/tests/scripts/integration.bash index 12deb75..0f12c1f 100644 --- a/ctdb/tests/scripts/integration.bash +++ b/ctdb/tests/scripts/integration.bash @@ -524,20 +524,8 @@ wait_until_node_has_some_ips () ####################################### -_ctdb_hack_options () -{ - local ctdb_options="$*" - - case "$ctdb_options" in - *--start-as-stopped*) - export CTDB_START_AS_STOPPED="yes" - esac -} - restart_ctdb_1 () { - _ctdb_hack_options "$@" - if [ -e /etc/redhat-release ] ; then service ctdb restart else -- 2.9.3 From df7b5f17432aae3c90bb1b228ca51936bf502cbe Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 23 Aug 2016 10:23:56 +1000 Subject: [PATCH 2/5] ctdb-tests: Drop attempts to pass arguments to ctdbd on (re)start This is not used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12170 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit eb38d02eb77dd654c8ed79fe985f738431918c9f) --- ctdb/tests/scripts/integration.bash | 6 ++---- ctdb/tests/simple/scripts/local_daemons.bash | 7 ++----- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/ctdb/tests/scripts/integration.bash b/ctdb/tests/scripts/integration.bash index 0f12c1f..b2a3451 100644 --- a/ctdb/tests/scripts/integration.bash +++ b/ctdb/tests/scripts/integration.bash @@ -536,7 +536,7 @@ restart_ctdb_1 () # Restart CTDB on all nodes. Override for local daemons. _restart_ctdb_all () { - onnode -p all $CTDB_TEST_WRAPPER restart_ctdb_1 "$@" + onnode -p all $CTDB_TEST_WRAPPER restart_ctdb_1 } # Nothing needed for a cluster. Override for local daemons. @@ -547,8 +547,6 @@ setup_ctdb () restart_ctdb () { - # "$@" is passed to restart_ctdb_all. - echo -n "Restarting CTDB" if $ctdb_test_restart_scheduled ; then echo -n " (scheduled)" @@ -557,7 +555,7 @@ restart_ctdb () local i for i in $(seq 1 5) ; do - _restart_ctdb_all "$@" || { + _restart_ctdb_all || { echo "Restart failed. Trying again in a few seconds..." sleep_for 5 continue diff --git a/ctdb/tests/simple/scripts/local_daemons.bash b/ctdb/tests/simple/scripts/local_daemons.bash index 765655e..0ad7eb8 100644 --- a/ctdb/tests/simple/scripts/local_daemons.bash +++ b/ctdb/tests/simple/scripts/local_daemons.bash @@ -95,7 +95,6 @@ setup_ctdb () daemons_start_1 () { local pnn="$1" - shift # "$@" gets passed to ctdbd local public_addresses_all="${TEST_VAR_DIR}/public_addresses_all" local public_addresses_mine="${TEST_VAR_DIR}/public_addresses.${pnn}" @@ -147,12 +146,10 @@ EOF daemons_start () { - # "$@" gets passed to ctdbd - echo "Starting $TEST_LOCAL_DAEMONS ctdb daemons..." for i in $(seq 0 $(($TEST_LOCAL_DAEMONS - 1))) ; do - daemons_start_1 $i "$@" + daemons_start_1 $i done } @@ -166,5 +163,5 @@ maybe_stop_ctdb () _restart_ctdb_all () { daemons_stop - daemons_start "$@" + daemons_start } -- 2.9.3 From fd305e287ce4f3df48fbf16055eef4e25ab30f88 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 23 Aug 2016 10:59:25 +1000 Subject: [PATCH 3/5] ctdb-tests: Move local daemon configuration creation into setup_ctdb() These files don't need to be re-generated on every restart. They can be generated once when CTDB is first started. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12170 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit 3c87868d208de8928e25db1dd34266830f017a07) --- ctdb/tests/simple/scripts/local_daemons.bash | 53 ++++++++++++++-------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/ctdb/tests/simple/scripts/local_daemons.bash b/ctdb/tests/simple/scripts/local_daemons.bash index 0ad7eb8..24c0ee1 100644 --- a/ctdb/tests/simple/scripts/local_daemons.bash +++ b/ctdb/tests/simple/scripts/local_daemons.bash @@ -49,14 +49,12 @@ setup_ctdb () mkdir -p "${TEST_VAR_DIR}/test.db/persistent" local public_addresses_all="${TEST_VAR_DIR}/public_addresses_all" - local no_public_addresses="${TEST_VAR_DIR}/no_public_addresses.txt" - rm -f $CTDB_NODES $public_addresses_all $no_public_addresses + rm -f $CTDB_NODES $public_addresses_all # If there are (strictly) greater than 2 nodes then we'll randomly # choose a node to have no public addresses. local no_public_ips=-1 [ $TEST_LOCAL_DAEMONS -gt 2 ] && no_public_ips=$(($RANDOM % $TEST_LOCAL_DAEMONS)) - echo "$no_public_ips" >$no_public_addresses # When running certain tests we add and remove eventscripts, so we # need to be able to modify the events.d/ directory. Therefore, @@ -90,33 +88,25 @@ setup_ctdb () fi fi done -} - -daemons_start_1 () -{ - local pnn="$1" - local public_addresses_all="${TEST_VAR_DIR}/public_addresses_all" - local public_addresses_mine="${TEST_VAR_DIR}/public_addresses.${pnn}" - local no_public_addresses="${TEST_VAR_DIR}/no_public_addresses.txt" - local public_addresses + local pnn + for pnn in $(seq 0 $(($TEST_LOCAL_DAEMONS - 1))) ; do + local public_addresses_mine="${TEST_VAR_DIR}/public_addresses.${pnn}" + local public_addresses - local no_public_ips=-1 - [ -r $no_public_addresses ] && read no_public_ips <$no_public_addresses - - if [ "$no_public_ips" = $pnn ] ; then + if [ "$no_public_ips" = $pnn ] ; then echo "Node $no_public_ips will have no public IPs." public_addresses="/dev/null" - else + else cp "$public_addresses_all" "$public_addresses_mine" public_addresses="$public_addresses_mine" - fi + fi - local node_ip=$(sed -n -e "$(($pnn + 1))p" "$CTDB_NODES") + local node_ip=$(sed -n -e "$(($pnn + 1))p" "$CTDB_NODES") - local pidfile="${TEST_VAR_DIR}/ctdbd.${pnn}.pid" - local conf="${TEST_VAR_DIR}/ctdbd.${pnn}.conf" - cat >"$conf" <"$conf" <>"$conf" + # Override from the environment. This would be easier if env was + # guaranteed to quote its output so it could be reused. + env | + grep '^CTDB_' | + sed -e 's@=\([^"]\)@="\1@' -e 's@[^"]$@&"@' -e 's@="$@&"@' >>"$conf" + done +} + +daemons_start_1 () +{ + local pnn="$1" + + local pidfile="${TEST_VAR_DIR}/ctdbd.${pnn}.pid" + local conf="${TEST_VAR_DIR}/ctdbd.${pnn}.conf" # We'll use "pkill -f" to kill the daemons with # "ctdbd --sloppy-start --nopublicipcheck" as context. -- 2.9.3 From d2188242c9c369370a39fd3864b36cf5bff275a9 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 23 Aug 2016 11:07:25 +1000 Subject: [PATCH 4/5] ctdb-tests: Remove function daemons_start_1() This function doesn't do anything significant, so just unroll the body into the loop in daemons_start(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=12170 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit bcb33c46dfe3d6c06eb6b3b5e7f8625538772d5e) --- ctdb/tests/simple/scripts/local_daemons.bash | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/ctdb/tests/simple/scripts/local_daemons.bash b/ctdb/tests/simple/scripts/local_daemons.bash index 24c0ee1..a53a2f2 100644 --- a/ctdb/tests/simple/scripts/local_daemons.bash +++ b/ctdb/tests/simple/scripts/local_daemons.bash @@ -129,26 +129,20 @@ EOF done } -daemons_start_1 () -{ - local pnn="$1" - - local pidfile="${TEST_VAR_DIR}/ctdbd.${pnn}.pid" - local conf="${TEST_VAR_DIR}/ctdbd.${pnn}.conf" - - # We'll use "pkill -f" to kill the daemons with - # "ctdbd --sloppy-start --nopublicipcheck" as context. - CTDBD="${VALGRIND} ctdbd --sloppy-start --nopublicipcheck" \ - CTDBD_CONF="$conf" \ - ctdbd_wrapper "$pidfile" start -} - daemons_start () { echo "Starting $TEST_LOCAL_DAEMONS ctdb daemons..." - for i in $(seq 0 $(($TEST_LOCAL_DAEMONS - 1))) ; do - daemons_start_1 $i + local pnn + for pnn in $(seq 0 $(($TEST_LOCAL_DAEMONS - 1))) ; do + local pidfile="${TEST_VAR_DIR}/ctdbd.${pnn}.pid" + local conf="${TEST_VAR_DIR}/ctdbd.${pnn}.conf" + + # We'll use "pkill -f" to kill the daemons with + # "ctdbd --sloppy-start --nopublicipcheck" as context. + CTDBD="${VALGRIND} ctdbd --sloppy-start --nopublicipcheck" \ + CTDBD_CONF="$conf" \ + ctdbd_wrapper "$pidfile" start done } -- 2.9.3 From 48cf0e679c59f435975d628ad0aafb1a7493e50f Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 23 Aug 2016 11:49:54 +1000 Subject: [PATCH 5/5] ctdb-tests: Reimplement daemons_stop() using ctdbd_wrapper The current daemons_stop() implementation uses very loose matching to decide which processes to kill if "ctdb shutdown" hasn't worked within 1 second. This can cause ctdbd processes from other test runs to be killed. Instead, use ctdbd_wrapper, which uses the PID file as a last resort. This has the advantage of never killing ctdbd processes from other test runs. However, this also has the obvious consequence that an interrupted test run in one directory can not have its daemons cleaned up from a new test run in a different directory. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12170 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs Autobuild-User(master): Amitay Isaacs Autobuild-Date(master): Tue Aug 23 21:17:46 CEST 2016 on sn-devel-144 (cherry picked from commit 17dfd8b96bc1750a2f435caa1d208766257346f3) --- ctdb/tests/simple/scripts/local_daemons.bash | 40 +++++++++++----------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/ctdb/tests/simple/scripts/local_daemons.bash b/ctdb/tests/simple/scripts/local_daemons.bash index a53a2f2..ecb64f9 100644 --- a/ctdb/tests/simple/scripts/local_daemons.bash +++ b/ctdb/tests/simple/scripts/local_daemons.bash @@ -22,28 +22,6 @@ export CTDB_NODES="${TEST_VAR_DIR}/nodes.txt" ####################################### -daemons_stop () -{ - echo "Attempting to politely shutdown daemons..." - onnode -q all $CTDB shutdown || true - - echo "Sleeping for a while..." - sleep_for 1 - - local pat="ctdbd --sloppy-start --nopublicipcheck" - if pgrep -f "$pat" >/dev/null ; then - echo "Killing remaining daemons..." - pkill -f "$pat" - - if pgrep -f "$pat" >/dev/null ; then - echo "Once more with feeling.." - pkill -9 -f "$pat" - fi - fi - - rm -rf "${TEST_VAR_DIR}/test.db" -} - setup_ctdb () { mkdir -p "${TEST_VAR_DIR}/test.db/persistent" @@ -138,14 +116,28 @@ daemons_start () local pidfile="${TEST_VAR_DIR}/ctdbd.${pnn}.pid" local conf="${TEST_VAR_DIR}/ctdbd.${pnn}.conf" - # We'll use "pkill -f" to kill the daemons with - # "ctdbd --sloppy-start --nopublicipcheck" as context. CTDBD="${VALGRIND} ctdbd --sloppy-start --nopublicipcheck" \ CTDBD_CONF="$conf" \ ctdbd_wrapper "$pidfile" start done } +daemons_stop () +{ + echo "Stopping $TEST_LOCAL_DAEMONS ctdb daemons..." + + local pnn + for pnn in $(seq 0 $(($TEST_LOCAL_DAEMONS - 1))) ; do + local pidfile="${TEST_VAR_DIR}/ctdbd.${pnn}.pid" + local conf="${TEST_VAR_DIR}/ctdbd.${pnn}.conf" + + CTDBD_CONF="$conf" \ + ctdbd_wrapper "$pidfile" stop + done + + rm -rf "${TEST_VAR_DIR}/test.db" +} + maybe_stop_ctdb () { if $TEST_CLEANUP ; then -- 2.9.3