From 7f920206854a294d65fc1fefe8810a1b5ea8b901 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Fri, 13 Sep 2024 16:21:24 +1000 Subject: [PATCH 1/5] ctdb-scripts: Reformat with "shfmt -w -p -i 0 -fn" Massage a couple of lines manually so they're formatted sanely given the new indentation. Re-run shfmt to ensure no further changes. Best reviewed with "git show -w". Signed-off-by: Martin Schwenke Reviewed-by: Volker Lendecke Reviewed-by: Jerry Heyman (cherry picked from commit 3410eddd932b430acc687c81a5dc6e62a0a420a6) --- ctdb/config/events/legacy/10.interface.script | 80 +++++++++---------- 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/ctdb/config/events/legacy/10.interface.script b/ctdb/config/events/legacy/10.interface.script index dfd796563fd..137d558b3b7 100755 --- a/ctdb/config/events/legacy/10.interface.script +++ b/ctdb/config/events/legacy/10.interface.script @@ -5,7 +5,7 @@ # this adds/removes IPs from your # public interface -[ -n "$CTDB_BASE" ] || \ +[ -n "$CTDB_BASE" ] || CTDB_BASE=$(d=$(dirname "$0") && cd -P "$d" && dirname "$PWD") . "${CTDB_BASE}/functions" @@ -13,7 +13,7 @@ load_script_options if ! have_public_addresses; then - if [ "$1" = "init" ] ; then + if [ "$1" = "init" ]; then echo "No public addresses file found" fi exit 0 @@ -32,8 +32,8 @@ monitor_interfaces() # # public_ifaces set by get_public_ifaces() above # shellcheck disable=SC2154 - for _iface in $public_ifaces ; do - if interface_monitor "$_iface" ; then + for _iface in $public_ifaces; do + if interface_monitor "$_iface"; then up_interfaces_found=true $CTDB setifacelink "$_iface" up >/dev/null 2>&1 else @@ -42,11 +42,11 @@ monitor_interfaces() fi done - if ! $down_interfaces_found ; then + if ! $down_interfaces_found; then return 0 fi - if ! $up_interfaces_found ; then + if ! $up_interfaces_found; then return 1 fi @@ -58,63 +58,61 @@ monitor_interfaces() } # Sets: iface, ip, maskbits -get_iface_ip_maskbits () +get_iface_ip_maskbits() { - _iface_in="$1" - ip="$2" - _maskbits_in="$3" - - # Intentional word splitting here - # shellcheck disable=SC2046 - set -- $(ip_maskbits_iface "$ip") - if [ -n "$1" ] ; then - maskbits="$1" - iface="$2" - - if [ "$iface" != "$_iface_in" ] ; then - printf \ - 'WARNING: Public IP %s hosted on interface %s but VNN says %s\n' \ - "$ip" "$iface" "$_iface_in" - fi - if [ "$maskbits" != "$_maskbits_in" ] ; then - printf \ - 'WARNING: Public IP %s has %s bit netmask but VNN says %s\n' \ - "$ip" "$maskbits" "$_maskbits_in" + _iface_in="$1" + ip="$2" + _maskbits_in="$3" + + # Intentional word splitting here + # shellcheck disable=SC2046 + set -- $(ip_maskbits_iface "$ip") + if [ -n "$1" ]; then + maskbits="$1" + iface="$2" + + if [ "$iface" != "$_iface_in" ]; then + printf 'WARNING: Public IP %s hosted on interface %s but VNN says %s\n' \ + "$ip" "$iface" "$_iface_in" + fi + if [ "$maskbits" != "$_maskbits_in" ]; then + printf 'WARNING: Public IP %s has %s bit netmask but VNN says %s\n' \ + "$ip" "$maskbits" "$_maskbits_in" + fi + else + die "ERROR: Unable to determine interface for IP ${ip}" fi - else - die "ERROR: Unable to determine interface for IP ${ip}" - fi } -ip_block () +ip_block() { _ip="$1" _iface="$2" case "$_ip" in *:*) _family="inet6" ;; - *) _family="inet" ;; + *) _family="inet" ;; esac # Extra delete copes with previously killed script iptables_wrapper "$_family" \ - -D INPUT -i "$_iface" -d "$_ip" -j DROP 2>/dev/null + -D INPUT -i "$_iface" -d "$_ip" -j DROP 2>/dev/null iptables_wrapper "$_family" \ - -I INPUT -i "$_iface" -d "$_ip" -j DROP + -I INPUT -i "$_iface" -d "$_ip" -j DROP } -ip_unblock () +ip_unblock() { _ip="$1" _iface="$2" case "$_ip" in *:*) _family="inet6" ;; - *) _family="inet" ;; + *) _family="inet" ;; esac iptables_wrapper "$_family" \ - -D INPUT -i "$_iface" -d "$_ip" -j DROP 2>/dev/null + -D INPUT -i "$_iface" -d "$_ip" -j DROP 2>/dev/null } ctdb_check_args "$@" @@ -128,8 +126,8 @@ init) } _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" + get_proc "$_promote" >/dev/null 2>&1 || + die "Public IPs only supported if promote_secondaries is available" # make sure we drop any ips that might still be held if # previous instance of ctdb got killed with -9 or similar @@ -152,7 +150,7 @@ takeip) update_my_public_ip_addresses "takeip" "$ip" add_ip_to_iface "$iface" "$ip" "$maskbits" || { - exit 1; + exit 1 } # In case a previous "releaseip" for this IP was killed... @@ -213,7 +211,7 @@ updateip) # Could check maskbits too. However, that should never change # so we want to notice if it does. - if [ "$oiface" = "$niface" ] ; then + if [ "$oiface" = "$niface" ]; then echo "Redundant \"updateip\" - ${ip} already on ${niface}" exit 0 fi -- 2.47.3 From 457bc85c0c0b478a61920061984904f38675b210 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 16 Oct 2025 08:17:44 +1100 Subject: [PATCH 2/5] ctdb-daemon: Fix a crash due to a failed updateip This should really be a takeip. However, CTDB's weak check of the IP address state (using bind(2)) incorrectly indicates that the IP address is assigned to an interface so it is converted to an updateip. After commit 0536d7a98b832fc00d26b09c26bf14fb63dbf5fb (which improves IP address state checking), this will almost certainly not occur on platforms with getifaddrs(3) (e.g. Linux). This means it is only likely to occur in 4.21 when net.ipv4.ip_nonlocal_bind=1. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15935 Reported-by: Bailey Allison Signed-off-by: Martin Schwenke Reviewed-by: Anoop C S (cherry picked from commit d08f9ebd2755671d30c73a4e979029d353848828) --- ctdb/server/ctdb_takeover.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index b9196e3ff63..f1b3119bf34 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -613,7 +613,15 @@ static void ctdb_do_updateip_callback(struct ctdb_context *ctdb, int status, */ ctdb_vnn_unassign_iface(ctdb, state->vnn); state->vnn->iface = state->old; - state->vnn->iface->references++; + /* + * state->old (above) can be NULL if the IP wasn't + * recorded as held by this node but the system thinks + * the IP was assigned. In that case, a move could + * still be desirable.. + */ + if (state->vnn->iface != NULL) { + state->vnn->iface->references++; + } ctdb_request_control_reply(ctdb, state->c, NULL, status, NULL); talloc_free(state); -- 2.47.3 From 39c5317e984182c1160f40fca47d1edcf1883da5 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 16 Oct 2025 10:42:22 +1100 Subject: [PATCH 3/5] ctdb-tests: Add an event script unit test for updateip This illustrates the current failure where an unassigned public IP address causes updateip to fail. After commit 0536d7a98b832fc00d26b09c26bf14fb63dbf5fb (which improves IP address state checking), this will almost certainly not occur on platforms with getifaddrs(3) (e.g. Linux). This means it is only likely to occur in 4.21 when net.ipv4.ip_nonlocal_bind=1. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15935 Reported-by: Bailey Allison Signed-off-by: Martin Schwenke Reviewed-by: Anoop C S (cherry picked from commit a98ffb96efc4a9ea2110c654860a4ba3896ab3d5) --- .../eventscripts/10.interface.updateip.001.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100755 ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh diff --git a/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh b/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh new file mode 100755 index 00000000000..af87bc14326 --- /dev/null +++ b/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh @@ -0,0 +1,16 @@ +#!/bin/sh + +. "${TEST_SCRIPTS_DIR}/unit.sh" + +define_test "error - update a non-existent ip" + +setup + +public_address=$(ctdb_get_1_public_address) +ip="${public_address% *}" +ip="${ip#* }" + +required_result 1 "ERROR: Unable to determine interface for IP ${ip}" +# Want separate words from public_address: interface IP maskbits +# shellcheck disable=SC2086 +simple_test "__none__" $public_address -- 2.47.3 From 061d017d7739ab1839e83bc70a47ec6e1180433b Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 16 Oct 2025 13:51:27 +1100 Subject: [PATCH 4/5] ctdb-scripts: Avoid printing a message if no connections BUG: https://bugzilla.samba.org/show_bug.cgi?id=15935 Signed-off-by: Martin Schwenke Reviewed-by: Anoop C S (cherry picked from commit 01d3d25c0139a3dd49a2322a9416698d08733377) --- ctdb/config/functions | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ctdb/config/functions b/ctdb/config/functions index 4139059a3d3..d61852a8161 100755 --- a/ctdb/config/functions +++ b/ctdb/config/functions @@ -594,6 +594,10 @@ tickle_tcp_connections() _conns=$(get_tcp_connections_for_ip "$_ip" | awk '{ print $1, $2 ; print $2, $1 }') + if [ -z "$_conns" ]; then + return + fi + echo "$_conns" | awk '{ print "Tickle TCP connection", $1, $2 }' echo "$_conns" | ctdb tickle } -- 2.47.3 From 445f37243dee3fa905744ca93f1c5362d1286d66 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 16 Oct 2025 13:54:22 +1100 Subject: [PATCH 5/5] ctdb-scripts: Avoid failing updateip when IP is not assigned There is no use failing this when it could behave more like takeip. Use old interface of "__none__" as a hint that ctdbd doesn't think the IP is assigned either. In this case print a warning instead of an error. Take some care to avoid spurious errors in updateip. After commit 0536d7a98b832fc00d26b09c26bf14fb63dbf5fb (which improves IP address state checking), this will almost certainly not occur on platforms with getifaddrs(3) (e.g. Linux). This means it is only likely to occur in 4.21 when net.ipv4.ip_nonlocal_bind=1. Update test to match. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15935 Reported-by: Bailey Allison Signed-off-by: Martin Schwenke Reviewed-by: Anoop C S Autobuild-User(master): Anoop C S Autobuild-Date(master): Fri Oct 17 06:28:30 UTC 2025 on atb-devel-224 (cherry picked from commit 0e73781bf84a1e8e596d8be3f55eeb5f8f927990) --- ctdb/config/events/legacy/10.interface.script | 17 +++++++++++++---- .../eventscripts/10.interface.updateip.001.sh | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/ctdb/config/events/legacy/10.interface.script b/ctdb/config/events/legacy/10.interface.script index 137d558b3b7..f0545a40455 100755 --- a/ctdb/config/events/legacy/10.interface.script +++ b/ctdb/config/events/legacy/10.interface.script @@ -80,6 +80,11 @@ get_iface_ip_maskbits() "$ip" "$maskbits" "$_maskbits_in" fi else + if [ "$_iface_in" = "__none__" ]; then + echo "WARNING: Unable to determine interface for IP ${ip}" + iface="$_iface_in" + return + fi die "ERROR: Unable to determine interface for IP ${ip}" fi } @@ -216,10 +221,14 @@ updateip) exit 0 fi - ip_block "$ip" "$oiface" - - delete_ip_from_iface "$oiface" "$ip" "$maskbits" 2>/dev/null - delete_ip_from_iface "$niface" "$ip" "$maskbits" 2>/dev/null + # Behave more like takeip when the IP is not assigned. No + # need for a similar condition around ip_unblock()s because + # they will silently fail. + if [ "$oiface" != "__none__" ]; then + ip_block "$ip" "$oiface" + delete_ip_from_iface "$oiface" "$ip" "$maskbits" >/dev/null 2>&1 + fi + delete_ip_from_iface "$niface" "$ip" "$maskbits" >/dev/null 2>&1 add_ip_to_iface "$niface" "$ip" "$maskbits" || { ip_unblock "$ip" "$oiface" diff --git a/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh b/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh index af87bc14326..e9567a8d114 100755 --- a/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh +++ b/ctdb/tests/UNIT/eventscripts/10.interface.updateip.001.sh @@ -10,7 +10,7 @@ public_address=$(ctdb_get_1_public_address) ip="${public_address% *}" ip="${ip#* }" -required_result 1 "ERROR: Unable to determine interface for IP ${ip}" +ok "WARNING: Unable to determine interface for IP ${ip}" # Want separate words from public_address: interface IP maskbits # shellcheck disable=SC2086 simple_test "__none__" $public_address -- 2.47.3