The branch, master has been updated via a9ccdec ctdb-tests: Handle interactions with monitor events via f1a20d7 ctdb-recoverd: Fix a bug in the LCP2 rebalancing code via 50fc53d ctdb-tests: New test to ensure "ctdb reloadips" manipulates IPs correctly via a226015 ctdb-tests-eventscripts: Testing support for promote_secondaries via 176ae6c ctdb-eventscripts: Deleting IPs should use the promote_secondaries option from 6a7c420 waf:lib/replace fix iconv checks on HP/UX
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit a9ccdec008ebcb1b286eede4f43167e3e4d4cbe0 Author: Martin Schwenke <mar...@meltin.net> Date: Wed Feb 12 15:33:19 2014 +1100 ctdb-tests: Handle interactions with monitor events In the first case, reconfiguration can longer happen in a monitor event, so this is no longer a problem. Drop it. Running a monitor event by hand no longer cancels the existing monitor event. Instead the hand-run event fails. So do this differently and just wait for a monitor event before continuing. 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): Thu Feb 13 04:05:57 CET 2014 on sn-devel-104 commit f1a20d748f6ab4702be5b17047a3fbfa0f3e8d0c Author: Martin Schwenke <mar...@meltin.net> Date: Fri Feb 7 17:19:20 2014 +1100 ctdb-recoverd: Fix a bug in the LCP2 rebalancing code srcimbl gets changed on every iteration of the loop. The value that should be stored for the new imbalance of the source node is minsrcimbl. To help diagnose this, added some extra debug that can be left in. The extra debug changes the output of a couple of tests. Note that the resulting IP allocations in those tests is unchanged - only the debug output is changed. Also add some new tests that illustrates the bug. Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> commit 50fc53d7f11a3c28fd4ef5318d90f842bbc0f19c Author: Martin Schwenke <mar...@meltin.net> Date: Wed Feb 12 09:49:11 2014 +1100 ctdb-tests: New test to ensure "ctdb reloadips" manipulates IPs correctly This adds a lot of IPs (currently 100) in a new network and deletes them in a few steps. First the primary is deleted and then a check is done to ensure that the remaining IPs are all correct. Then about 1/2 of the IPs and deleted and remaining IPs are checked. Then the remaining IPs are deleted and a check is done to ensure they are all gone. Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> commit a226015990356ee989c5b9a472581bb3187de894 Author: Martin Schwenke <mar...@meltin.net> Date: Tue Jan 28 16:08:50 2014 +1100 ctdb-tests-eventscripts: Testing support for promote_secondaries Just enable this behaviour by default in the ip command stub, since 10.interface assumes/sets it. The rc.local replacement for set_proc() doesn't do anything... Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> commit 176ae6c704528c021fcc34a41878584f43a00119 Author: Martin Schwenke <mar...@meltin.net> Date: Tue Jan 28 14:41:25 2014 +1100 ctdb-eventscripts: Deleting IPs should use the promote_secondaries option If a primary IP address is being deleted from an interface, the secondaries are remembered and added back after the primary is deleted. This is done under a lock shared by the add/del script code. It is necessary because, by default, Linux deletes secondaries when the corresponding primary is deleted. There is a race here between ctdbd and the scripts, since ctdbd doesn't know about the lock. If ctdbd receives a release IP control and the IP address is not on an interface then it is regarded as a "Redundant release of IP" so no "releaseip" event is generated. This can occur if the IP address in question is a secondary that has been temporarily dropped. It is more likely if the number of secondaries is large. Since Linux 2.6.12 (i.e. 2005) Linux has supported a promote_secondaries option on interfaces. This option is currently undocumented but that will change in Linux 3.14. With promote_secondaries enabled the kernel will not drop secondaries but will promote a corresponding secondary instead. The kernel does all necessary locking. Use promote_secondaries to simplify the code, avoid re-adding secondaries, avoid re-adding routes and provide improved performance. This could be done conditionally, with a fallback to legacy secondary-re-adding code, but no supported Linux distribution is running a pre-2.6.12 kernel so this is unnecessary. Signed-off-by: Martin Schwenke <mar...@meltin.net> Reviewed-by: Amitay Isaacs <ami...@gmail.com> ----------------------------------------------------------------------- Summary of changes: ctdb/config/events.d/10.interface | 4 + ctdb/config/functions | 87 +----- ctdb/server/ctdb_takeover.c | 5 +- ctdb/tests/complex/18_ctdb_reloadips.sh | 234 ++++++++++++++ ctdb/tests/eventscripts/etc-ctdb/rc.local | 3 + ctdb/tests/eventscripts/stubs/ip | 11 +- ctdb/tests/simple/60_recoverd_missing_ip.sh | 10 +- ctdb/tests/takeover/lcp2.005.sh | 63 +++- ctdb/tests/takeover/lcp2.023.sh | 34 ++- ctdb/tests/takeover/lcp2.031.sh | 143 +++++++++ ctdb/tests/takeover/lcp2.032.sh | 450 +++++++++++++++++++++++++++ 11 files changed, 942 insertions(+), 102 deletions(-) create mode 100755 ctdb/tests/complex/18_ctdb_reloadips.sh create mode 100755 ctdb/tests/takeover/lcp2.031.sh create mode 100755 ctdb/tests/takeover/lcp2.032.sh Changeset truncated at 500 lines: diff --git a/ctdb/config/events.d/10.interface b/ctdb/config/events.d/10.interface index f44c674..32d841f 100755 --- a/ctdb/config/events.d/10.interface +++ b/ctdb/config/events.d/10.interface @@ -151,6 +151,10 @@ case "$1" in get_proc sys/net/ipv4/conf/all/arp_filter >/dev/null 2>&1 && { set_proc sys/net/ipv4/conf/all/arp_filter 1 } + + _promote="sys/net/ipv4/conf/all/promote_secondaries" + get_proc "$_promote" >/dev/null 2>&1 || \ + die "Public IPs only supported if promote_secondaries is available" ;; ############################# diff --git a/ctdb/config/functions b/ctdb/config/functions index 1aad3ae..4363d3d 100755 --- a/ctdb/config/functions +++ b/ctdb/config/functions @@ -820,34 +820,20 @@ nfs_statd_update () fi } -add_ip_to_iface() +######################################################## + +add_ip_to_iface () { _iface=$1 _ip=$2 _maskbits=$3 - _lockfile="${CTDB_VARDIR}/state/interface_modify_${_iface}.flock" - mkdir -p "${_lockfile%/*}" # dirname - [ -f "$_lockfile" ] || touch "$_lockfile" - - ( - # Note: use of return/exit/die() below only gets us out of the - # sub-shell, which is actually what we want. That is, the - # function should just return non-zero. - - flock --timeout 30 0 || \ - die "add_ip_to_iface: unable to get lock for ${_iface}" - - # Ensure interface is up - ip link set "$_iface" up || \ - die "Failed to bringup interface $_iface" + # Ensure interface is up + ip link set "$_iface" up || \ + die "Failed to bringup interface $_iface" - ip addr add "$_ip/$_maskbits" brd + dev "$_iface" || \ - die "Failed to add $_ip/$_maskbits on dev $_iface" - ) <"$_lockfile" - - # Do nothing here - return above only gets us out of the subshell - # and doing anything here will affect the return code. + ip addr add "$_ip/$_maskbits" brd + dev "$_iface" || \ + die "Failed to add $_ip/$_maskbits on dev $_iface" } delete_ip_from_iface() @@ -856,56 +842,15 @@ delete_ip_from_iface() _ip=$2 _maskbits=$3 - _lockfile="${CTDB_VARDIR}/state/interface_modify_${_iface}.flock" - mkdir -p "${_lockfile%/*}" # dirname - [ -f "$_lockfile" ] || touch "$_lockfile" - - ( - # Note: use of return/exit/die() below only gets us out of the - # sub-shell, which is actually what we want. That is, the - # function should just return non-zero. - - flock --timeout 30 0 || \ - die "delete_ip_from_iface: unable to get lock for ${_iface}" - - _im="$_ip/$_maskbits" # shorthand for readability - - # "ip addr del" will delete all secondary IPs if this is the - # primary. To work around this _very_ annoying behaviour we - # have to keep a record of the secondaries and re-add them - # afterwards. Yuck! - - _secondaries="" - if ip addr list dev "$_iface" primary | grep -Fq "inet $_im " ; then - _secondaries=$(ip addr list dev "$_iface" secondary | \ - awk '$1 == "inet" { print $2 }') - fi - - local _rc=0 - ip addr del "$_im" dev "$_iface" || { - echo "Failed to del $_ip on dev $_iface" - _rc=1 - } - - if [ -n "$_secondaries" ] ; then - for _i in $_secondaries; do - if ip addr list dev "$_iface" | grep -Fq "inet $_i" ; then - echo "Kept secondary $_i on dev $_iface" - else - echo "Re-adding secondary address $_i to dev $_iface" - ip addr add $_i brd + dev $_iface || { - echo "Failed to re-add address $_i to dev $_iface" - _rc=1 - } - fi - done - fi - - return $_rc - ) <"$_lockfile" + # This could be set globally for all interfaces but it is probably + # better to avoid surprises, so limit it the interfaces where CTDB + # has public IP addresses. There isn't anywhere else convenient + # to do this so just set it each time. This is much cheaper than + # remembering and re-adding secondaries. + set_proc "sys/net/ipv4/conf/${_iface}/promote_secondaries" 1 - # Do nothing here - return above only gets us out of the subshell - # and doing anything here will affect the return code. + ip addr del "$_ip/$_maskbits" dev "$_iface" || \ + die "Failed to del $_ip on dev $_iface" } # If the given IP is hosted then print 2 items: maskbits and iface diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index c21736e..d3a6e25 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -1958,7 +1958,7 @@ static bool lcp2_failback_candidate(struct ctdb_context *ctdb, mindstnode, mindstimbl - lcp2_imbalances[mindstnode])); - lcp2_imbalances[srcnode] = srcimbl; + lcp2_imbalances[srcnode] = minsrcimbl; lcp2_imbalances[mindstnode] = mindstimbl; minip->pnn = mindstnode; @@ -2024,10 +2024,13 @@ try_again: * iterate through candidates. Usually the 1st one will be * used, so this doesn't cost much... */ + DEBUG(DEBUG_DEBUG,("+++++++++++++++++++++++++++++++++++++++++\n")); + DEBUG(DEBUG_DEBUG,("Selecting most imbalanced node from:\n")); lips = talloc_array(ctdb, struct lcp2_imbalance_pnn, numnodes); for (i=0; i<numnodes; i++) { lips[i].imbalance = lcp2_imbalances[i]; lips[i].pnn = i; + DEBUG(DEBUG_DEBUG,(" %d [%d]\n", i, lcp2_imbalances[i])); } qsort(lips, numnodes, sizeof(struct lcp2_imbalance_pnn), lcp2_cmp_imbalance_pnn); diff --git a/ctdb/tests/complex/18_ctdb_reloadips.sh b/ctdb/tests/complex/18_ctdb_reloadips.sh new file mode 100755 index 0000000..042683c --- /dev/null +++ b/ctdb/tests/complex/18_ctdb_reloadips.sh @@ -0,0 +1,234 @@ +#!/bin/bash + +test_info() +{ + cat <<EOF +Verify that adding/deleting IPs using 'ctdb reloadips' works + +Checks that when IPs are added to and deleted from a single node then +those IPs are actually assigned and unassigned from the specified +interface. + +Prerequisites: + +* An active CTDB cluster with public IP addresses configured + +Expected results: + +* When IPs are added to a single node then they are assigned to an + interface. + +* When IPs are deleted from a single node then they disappear from an + interface. +EOF +} + +. "${TEST_SCRIPTS_DIR}/integration.bash" + +set -e + +ctdb_test_init "$@" + +ctdb_test_check_real_cluster + +cluster_is_healthy + +# Reset configuration +ctdb_restart_when_done + +select_test_node_and_ips + +#################### + +# Search for an unused 10.B.1.0/24 network on which to add public IP +# addresses. + +# The initial search is for a 10.B.0.0/16 network since some +# configurations may use a whole class B for the private network. +# Check that there are no public IP addresses (as reported by "ctdb ip +# -n -all") or other IP addresses (as reported by "ip addr show") with +# the provided prefix. Note that this is an IPv4-specific test. + +echo "Getting public IP information from CTDB..." +try_command_on_node any "$CTDB ip -Y -v -n all" +ctdb_ip_info=$(echo "$out" | awk -F: 'NR > 1 { print $2, $3, $5 }') + +echo "Getting IP information from interfaces..." +try_command_on_node all "ip addr show" +ip_addr_info=$(echo "$out" | \ + awk '$1 == "inet" { print gensub(/\/.*/, "", "", $2)}') + +prefix="" +for b in $(seq 0 255) ; do + prefix="10.${b}" + + # Does the prefix match any IP address returned by "ip addr info"? + while read ip ; do + if [ "${ip#${prefix}.}" != "$ip" ] ; then + prefix="" + continue 2 + fi + done <<<"$ip_addr_info" + + # Does the prefix match any public IP address "ctdb ip -n all"? + while read ip pnn iface ; do + if [ "${ip#${prefix}.}" != "$ip" ] ; then + prefix="" + continue 2 + fi + done <<<"$ctdb_ip_info" + + # Got through the IPs without matching prefix - done! + break +done + +[ -n "$prefix" ] || die "Unable to find a usable IP address prefix" + +# We really want a class C: 10.B.1.0/24 +prefix="${prefix}.1" + +#################### + +iface=$(echo "$ctdb_ip_info" | awk -v pnn=$test_node '$2 == pnn { print $3 ; exit }') + +#################### + +new_takeover_timeout=90 +echo "Setting TakeoverTimeout=${new_takeover_timeout} to avoid potential bans" +try_command_on_node $test_node "$CTDB setvar TakeoverTimeout ${new_takeover_timeout}" + +#################### + +addresses=$(get_ctdbd_command_line_option $test_node "public-addresses") +echo "Public addresses file on node $test_node is \"$addresses\"" +backup="${addresses}.$$" + +backup_public_addresses () +{ + try_command_on_node $test_node "cp -a $addresses $backup" +} + +restore_public_addresses () +{ + try_command_on_node $test_node "mv $backup $addresses >/dev/null 2>&1 || true" +} +ctdb_test_exit_hook_add restore_public_addresses + +# Now create that backup +backup_public_addresses + +#################### + +add_ips_to_original_config () +{ + local test_node="$1" + local addresses="$2" + local iface="$3" + local prefix="$4" + local first="$5" + local last="$6" + + echo "Adding new public IPs to original config on node ${test_node}..." + echo "IPs will be ${prefix}.${first}/24..${prefix}.${last}/24" + + # Implement this by completely rebuilding the public_addresses + # file. This is easier than deleting entries on a remote node. + restore_public_addresses + backup_public_addresses + + # Note that tee is a safe way of creating a file on a remote node. + # This avoids potential fragility with quoting or redirection. + for i in $(seq $first $last) ; do + echo "${prefix}.${i}/24 ${iface}" + done | + try_command_on_node -i $test_node "tee -a $addresses" +} + +check_ips () +{ + local test_node="$1" + local iface="$2" + local prefix="$3" + local first="$4" + local last="$5" + + # If just 0 specified then this is an empty range + local public_ips_file=$(mktemp) + if [ "$first" = 0 -a -z "$last" ] ; then + echo "Checking that there are no IPs in ${prefix}.0/24" + else + local prefix_regexp="inet *${prefix//./\.}" + + echo "Checking IPs in range ${prefix}.${first}/24..${prefix}.${last}/24" + + local i + for i in $(seq $first $last) ; do + echo "${prefix}.${i}" + done | sort >"$public_ips_file" + fi + + try_command_on_node $test_node "ip addr show dev ${iface}" + local ip_addrs_file=$(mktemp) + echo "$out" | \ + sed -n -e "s@.*inet * \(${prefix//./\.}\.[0-9]*\)/.*@\1@p" | \ + sort >"$ip_addrs_file" + + local diffs=$(diff "$public_ips_file" "$ip_addrs_file") || true + rm -f "$ip_addrs_file" "$public_ips_file" + + if [ -z "$diffs" ] ; then + echo "GOOD: IP addresses are as expected" + else + echo "BAD: IP addresses are incorrect:" + echo "$diffs" + exit 1 + fi +} + +#################### + +new_ip_max=100 + +#################### + +add_ips_to_original_config \ + $test_node "$addresses" "$iface" "$prefix" 1 $new_ip_max + +try_command_on_node $test_node "$CTDB reloadips" + +check_ips $test_node "$iface" "$prefix" 1 $new_ip_max + +#################### + +# This should be the primary. Ensure that no other IPs are lost +echo "Using 'ctdb reloadips' to remove the 1st address just added..." + +add_ips_to_original_config \ + $test_node "$addresses" "$iface" "$prefix" 2 $new_ip_max + +try_command_on_node $test_node "$CTDB reloadips" + +check_ips $test_node "$iface" "$prefix" 2 $new_ip_max + +#################### + +# Get rid of about 1/2 the IPs +start=$(($new_ip_max / 2 + 1)) +echo "Updating to include only about 1/2 of the new IPs..." + +add_ips_to_original_config \ + $test_node "$addresses" "$iface" "$prefix" $start $new_ip_max + +try_command_on_node $test_node "$CTDB reloadips" + +check_ips $test_node "$iface" "$prefix" $start $new_ip_max + +#################### + +# Delete the rest +echo "Restoring original IP configuration..." +restore_public_addresses + +try_command_on_node $test_node "$CTDB reloadips" + +check_ips $test_node "$iface" "$prefix" 0 diff --git a/ctdb/tests/eventscripts/etc-ctdb/rc.local b/ctdb/tests/eventscripts/etc-ctdb/rc.local index 6052d87..0dc531f 100755 --- a/ctdb/tests/eventscripts/etc-ctdb/rc.local +++ b/ctdb/tests/eventscripts/etc-ctdb/rc.local @@ -33,6 +33,9 @@ get_proc () sys/net/ipv4/conf/all/arp_filter) echo 1 ;; + sys/net/ipv4/conf/all/promote_secondaries) + echo 1 + ;; fs/nfsd/threads) echo "$FAKE_NFSD_THREAD_PIDS" | wc -w ;; diff --git a/ctdb/tests/eventscripts/stubs/ip b/ctdb/tests/eventscripts/stubs/ip index f076cb9..860f6a5 100755 --- a/ctdb/tests/eventscripts/stubs/ip +++ b/ctdb/tests/eventscripts/stubs/ip @@ -1,5 +1,7 @@ #!/bin/sh +promote_secondaries=true + not_implemented () { echo "ip stub command: \"$1\" not implemented" @@ -305,8 +307,13 @@ ip_addr_del () echo "RTNETLINK answers: Cannot assign requested address" >&2 exit 254 elif grep -Fq "$local" "$pf" ; then - # Remove primaries AND SECONDARIES. - rm -f "$pf" "$sf" + if $promote_secondaries && [ -s "$sf" ] ; then + head -n 1 "$sf" >"$pf" + sed -i -e '1d' "$sf" + else + # Remove primaries AND SECONDARIES. + rm -f "$pf" "$sf" + fi elif [ -f "$sf" ] && grep -Fq "$local" "$sf" ; then grep -Fv "$local" "$sf" >"${sf}.new" mv "${sf}.new" "$sf" diff --git a/ctdb/tests/simple/60_recoverd_missing_ip.sh b/ctdb/tests/simple/60_recoverd_missing_ip.sh index 0734aee..88e4502 100755 --- a/ctdb/tests/simple/60_recoverd_missing_ip.sh +++ b/ctdb/tests/simple/60_recoverd_missing_ip.sh @@ -37,10 +37,6 @@ fi echo "$test_ip/$mask is on $iface" -# Push out the next monitor event so it is less likely to be cancelled -# and result in services not being restarted properly. -try_command_on_node $test_node $CTDB eventscript monitor - echo "Deleting IP $test_ip from all nodes" try_command_on_node -v $test_node $CTDB delip -n all $test_ip @@ -61,9 +57,9 @@ ctdb_test_exit_hook_add my_exit_hook # delips is complete. try_command_on_node $test_node $CTDB sync -# This effectively cancels any monitor event that is in progress and -# runs a new one -try_command_on_node $test_node $CTDB eventscript monitor +# Wait for a monitor event. Then the next steps are unlikely to occur +# in the middle of a monitor event and will have the expected effect. +wait_for_monitor_event $test_node if [ -z "$TEST_LOCAL_DAEMONS" ] ; then # Stop monitor events from bringing up the link status of an interface diff --git a/ctdb/tests/takeover/lcp2.005.sh b/ctdb/tests/takeover/lcp2.005.sh index 113e52f..4e0bed8 100755 --- a/ctdb/tests/takeover/lcp2.005.sh +++ b/ctdb/tests/takeover/lcp2.005.sh @@ -10,6 +10,11 @@ required_result <<EOF DATE TIME [PID]: ---------------------------------------- DATE TIME [PID]: CONSIDERING MOVES (UNASSIGNED) DATE TIME [PID]: ---------------------------------------- +DATE TIME [PID]: +++++++++++++++++++++++++++++++++++++++++ +DATE TIME [PID]: Selecting most imbalanced node from: +DATE TIME [PID]: 0 [0] +DATE TIME [PID]: 1 [539166] +DATE TIME [PID]: 2 [0] DATE TIME [PID]: ---------------------------------------- DATE TIME [PID]: CONSIDERING MOVES FROM 1 [539166] DATE TIME [PID]: 1 [-116718] -> 192.168.21.254 -> 0 [+0] @@ -32,8 +37,13 @@ DATE TIME [PID]: 1 [-121110] -> 192.168.20.249 -> 0 [+0] DATE TIME [PID]: 1 [-121110] -> 192.168.20.249 -> 2 [+0] DATE TIME [PID]: ---------------------------------------- DATE TIME [PID]: 1 [-121363] -> 192.168.20.253 -> 0 [+0] +DATE TIME [PID]: +++++++++++++++++++++++++++++++++++++++++ +DATE TIME [PID]: Selecting most imbalanced node from: +DATE TIME [PID]: 0 [0] +DATE TIME [PID]: 1 [417803] +DATE TIME [PID]: 2 [0] DATE TIME [PID]: ---------------------------------------- -DATE TIME [PID]: CONSIDERING MOVES FROM 1 [418056] +DATE TIME [PID]: CONSIDERING MOVES FROM 1 [417803] DATE TIME [PID]: 1 [-102557] -> 192.168.21.254 -> 0 [+14161] DATE TIME [PID]: 1 [-102557] -> 192.168.21.254 -> 2 [+0] DATE TIME [PID]: 1 [-102810] -> 192.168.21.253 -> 0 [+14161] @@ -52,8 +62,13 @@ DATE TIME [PID]: 1 [-105485] -> 192.168.20.249 -> 0 [+15625] DATE TIME [PID]: 1 [-105485] -> 192.168.20.249 -> 2 [+0] DATE TIME [PID]: ---------------------------------------- DATE TIME [PID]: 1 [-105738] -> 192.168.20.251 -> 2 [+0] +DATE TIME [PID]: +++++++++++++++++++++++++++++++++++++++++ -- Samba Shared Repository