[SCM] CTDB repository - branch master updated - ctdb-1.13-257-g93c97c3

Amitay Isaacs amitay at samba.org
Tue Sep 18 02:02:02 MDT 2012


The branch, master has been updated
       via  93c97c3ba3ff714dfa0d056a91ff45010a6e2d66 (commit)
       via  acdaa04079a9827885f32a7bc078d3365c89b474 (commit)
       via  5c3be8f26dcde0b1b3d86928953e74d4a8b35958 (commit)
       via  6d41208074f0e9b56c585bca7eb39aaed653c4ca (commit)
       via  d0d0a6f19960f233224970b8d5d19b0e37222616 (commit)
       via  0ce5b079f327aba55b62800ccb22d79976fac665 (commit)
       via  30d69defa7e97ab5e3ba0492a27868dde2616494 (commit)
      from  49dd755fcd077c84eaf3d2fe5dd7757f5588d49c (commit)

http://gitweb.samba.org/?p=ctdb.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 93c97c3ba3ff714dfa0d056a91ff45010a6e2d66
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue Jul 17 20:46:58 2012 +1000

    tests/eventscripts: New policy routing test with invalid table ID
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>

commit acdaa04079a9827885f32a7bc078d3365c89b474
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue Jul 17 20:45:23 2012 +1000

    tests/eventscripts: Modify ip stub to simulate invalid table ID
    
    This involves refactoring ip_route_check_table() into a new function
    ip_check_table() which tables the operation type (i.e. rule/route) as
    an argument.
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>

commit 5c3be8f26dcde0b1b3d86928953e74d4a8b35958
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue Jul 17 20:19:37 2012 +1000

    Eventscripts: Indent error when a route delete fails in 11.per_ip_routing
    
    This puts it under the umbrella of the previous warning that should
    also have been printed.
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>

commit 6d41208074f0e9b56c585bca7eb39aaed653c4ca
Author: Martin Schwenke <martin at meltin.net>
Date:   Tue Jun 19 17:20:18 2012 +1000

    tests/eventscript: unit test for 13.per_ip_routing bogus route removal
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>

commit d0d0a6f19960f233224970b8d5d19b0e37222616
Author: Martin Schwenke <martin at meltin.net>
Date:   Fri Jun 15 17:22:02 2012 +1000

    eventscripts: 13.per_ip_routing should remove bogus routes on ipreallocated
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>

commit 0ce5b079f327aba55b62800ccb22d79976fac665
Author: Martin Schwenke <martin at meltin.net>
Date:   Wed Jun 13 13:53:18 2012 +1000

    tests/eventscripts: Add a policy routing unit test for "ip rule del" failure
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>

commit 30d69defa7e97ab5e3ba0492a27868dde2616494
Author: Martin Schwenke <martin at meltin.net>
Date:   Wed Jun 13 13:49:49 2012 +1000

    eventscripts: Print a warning on failure to delete a routing rule
    
    del_routing_for_ip() currently fails silently, which could hide real
    errors.
    
    In add_routing_for_ip() we don't want to see any error when calling
    del_routing_for_ip(), since we don't expect the rule to be there.
    
    Signed-off-by: Martin Schwenke <martin at meltin.net>

-----------------------------------------------------------------------

Summary of changes:
 config/events.d/13.per_ip_routing                  |   48 +++++++++++++++++--
 ..._ip_routing.006.sh => 13.per_ip_routing.013.sh} |   12 ++--
 ..._ip_routing.012.sh => 13.per_ip_routing.014.sh} |   17 +++----
 ..._ip_routing.006.sh => 13.per_ip_routing.015.sh} |   15 ++++--
 tests/eventscripts/stubs/ip                        |   51 ++++++++++++++------
 5 files changed, 103 insertions(+), 40 deletions(-)
 copy tests/eventscripts/{13.per_ip_routing.006.sh => 13.per_ip_routing.013.sh} (66%)
 copy tests/eventscripts/{13.per_ip_routing.012.sh => 13.per_ip_routing.014.sh} (61%)
 copy tests/eventscripts/{13.per_ip_routing.006.sh => 13.per_ip_routing.015.sh} (58%)


Changeset truncated at 500 lines:

diff --git a/config/events.d/13.per_ip_routing b/config/events.d/13.per_ip_routing
index cfde403..06b21b9 100755
--- a/config/events.d/13.per_ip_routing
+++ b/config/events.d/13.per_ip_routing
@@ -217,7 +217,7 @@ add_routing_for_ip ()
     _pref="$CTDB_PER_IP_ROUTING_RULE_PREF"
     _table_id="${table_id_prefix}${_ip}"
 
-    del_routing_for_ip "$_ip"
+    del_routing_for_ip "$_ip" >/dev/null 2>&1
 
     ip rule add from "$_ip" pref "$_pref" table "$_table_id" || \
 	die "add_routing_for_ip: failed to add rule for $_ip"
@@ -238,9 +238,23 @@ del_routing_for_ip ()
     _pref="$CTDB_PER_IP_ROUTING_RULE_PREF"
     _table_id="${table_id_prefix}${_ip}"
 
-    # Do this unconditionally since we own any matching table ids...
-    ip rule del from $_ip pref $_pref table $_table_id 2>/dev/null
-    ip route flush table $_table_id 2>/dev/null
+    # Do this unconditionally since we own any matching table ids.
+    # However, print a meaningful message if something goes wrong.
+    _cmd="ip rule del from $_ip pref $_pref table $_table_id"
+    _out=$($_cmd 2>&1) || \
+	cat <<EOF
+WARNING: Failed to delete policy routing rule
+  Command "$_cmd" failed:
+  $_out
+EOF
+    # This should never usually fail, so don't redirect output.
+    # However, it can fail when deleting a rogue IP, since there will
+    # be no routes for that IP.  In this case it should only fail when
+    # the rule deletion above has already failed because the table id
+    # is invalid.  Therefore, go to a little bit of trouble to indent
+    # the failure message so that it is associated with the above
+    # warning message and doesn't look too nasty.
+    ip route flush table $_table_id 2>&1 | sed -e 's@^.@  &@'
 }
 
 ######################################################################
@@ -285,6 +299,31 @@ add_missing_routes ()
     } || exit $?
 }
 
+# Remove rules/routes for addresses that we're not hosting.  If a
+# releaseip event failed in an earlier script then we might not have
+# had a chance to remove the corresponding rules/routes.
+remove_bogus_routes ()
+{
+    # Get a IPs current hosted by this node, each anchored with '@'.
+    _ips=$(ctdb ip -v -Y | awk -F: 'NR > 1 && $4 != "" {printf "@%s@\n", $2}')
+
+    ip rule show |
+    while read _p _x _i _x _t ; do
+	# Remove trailing colon after priority/preference.
+	_p="${_p%:}"
+	# Only remove rules that match our priority/preference.
+	[ "$CTDB_PER_IP_ROUTING_RULE_PREF" = "$_p" ] || continue
+	# Only remove rules for which we don't have an IP.  This could
+	# be done with grep, but let's do it with shell prefix removal
+	# to avoid unnecessary processes.  This falls through if
+	# "@${_i}@" isn't present in $_ips.
+	[ "$_ips" = "${_ips#*@${_i}@}" ] || continue
+
+	echo "Removing ip rule/routes for unhosted public address $_i"
+	del_routing_for_ip "$_i"
+    done
+}
+
 ######################################################################
 
 ctdb_check_args "$@"
@@ -346,6 +385,7 @@ case "$1" in
 
     ipreallocated)
 	add_missing_routes
+	remove_bogus_routes
 	;;
 
     *)
diff --git a/tests/eventscripts/13.per_ip_routing.006.sh b/tests/eventscripts/13.per_ip_routing.013.sh
similarity index 66%
copy from tests/eventscripts/13.per_ip_routing.006.sh
copy to tests/eventscripts/13.per_ip_routing.013.sh
index ed9df84..5c07632 100755
--- a/tests/eventscripts/13.per_ip_routing.006.sh
+++ b/tests/eventscripts/13.per_ip_routing.013.sh
@@ -2,7 +2,7 @@
 
 . "${TEST_SCRIPTS_DIR}/unit.sh"
 
-define_test "1 IP configured, takeip, releaseip"
+define_test "1 IP configured, releaseip of unassigned"
 
 setup_ctdb
 setup_ctdb_policy_routing
@@ -19,11 +19,11 @@ $ip $net
 $ip 0.0.0.0/0 $gw
 EOF
 
-    ok_null
-
-    simple_test_event "takeip" $dev $ip $bits
-
-    ok_null
+    ok <<EOF
+WARNING: Failed to delete policy routing rule
+  Command "ip rule del from $ip pref $CTDB_PER_IP_ROUTING_RULE_PREF table ctdb.$ip" failed:
+  RTNETLINK answers: No such file or directory
+EOF
 
     simple_test_event "releaseip" $dev $ip $bits
 
diff --git a/tests/eventscripts/13.per_ip_routing.012.sh b/tests/eventscripts/13.per_ip_routing.014.sh
similarity index 61%
copy from tests/eventscripts/13.per_ip_routing.012.sh
copy to tests/eventscripts/13.per_ip_routing.014.sh
index 198551f..9327264 100755
--- a/tests/eventscripts/13.per_ip_routing.012.sh
+++ b/tests/eventscripts/13.per_ip_routing.014.sh
@@ -2,11 +2,10 @@
 
 . "${TEST_SCRIPTS_DIR}/unit.sh"
 
-define_test "1 IP configured, takeip, releaseip, ipreallocated"
+define_test "1 IP configured, takeip, moveip, ipreallocated"
 
-# This partly tests the test infrastructure.  If the (stub) "ctdb
-# moveip" doesn't do anything then the IP being released will still be
-# on the node and the ipreallocated event will add the routes back.
+# We move the IP to another node but don't run releaseip.
+# ipreallocated should remove the bogus routes.
 
 setup_ctdb
 setup_ctdb_policy_routing
@@ -25,16 +24,16 @@ EOF
 
     ok_null
 
+    # Set up the routes for an IP that we have
     simple_test_event "takeip" $dev $ip $bits
 
-    ok_null
-
+    # Now move that IPs but don't run the associated "releaseip"
     ctdb moveip $ip 1
-    simple_test_event "releaseip" $dev $ip $bits
 
-    ok_null
+    ok <<EOF
+Removing ip rule/routes for unhosted public address 10.0.0.3
+EOF
 
-    # This will cause any
     simple_test_event "ipreallocated"
 
     ok <<EOF
diff --git a/tests/eventscripts/13.per_ip_routing.006.sh b/tests/eventscripts/13.per_ip_routing.015.sh
similarity index 58%
copy from tests/eventscripts/13.per_ip_routing.006.sh
copy to tests/eventscripts/13.per_ip_routing.015.sh
index ed9df84..5258438 100755
--- a/tests/eventscripts/13.per_ip_routing.006.sh
+++ b/tests/eventscripts/13.per_ip_routing.015.sh
@@ -2,11 +2,13 @@
 
 . "${TEST_SCRIPTS_DIR}/unit.sh"
 
-define_test "1 IP configured, takeip, releaseip"
+define_test "1 IP configured, releaseip of unassigned"
 
 setup_ctdb
 setup_ctdb_policy_routing
 
+export IP_ROUTE_BAD_TABLE_ID=true
+
 ctdb_get_1_public_address |
 {
     read dev ip bits
@@ -19,11 +21,12 @@ $ip $net
 $ip 0.0.0.0/0 $gw
 EOF
 
-    ok_null
-
-    simple_test_event "takeip" $dev $ip $bits
-
-    ok_null
+    ok <<EOF
+WARNING: Failed to delete policy routing rule
+  Command "ip rule del from $ip pref $CTDB_PER_IP_ROUTING_RULE_PREF table ctdb.$ip" failed:
+  Error: argument ctdb.$ip is wrong: invalid table ID
+  Error: argument ctdb.$ip is wrong: table id value is invalid
+EOF
 
     simple_test_event "releaseip" $dev $ip $bits
 
diff --git a/tests/eventscripts/stubs/ip b/tests/eventscripts/stubs/ip
index 31d5b4d..709c379 100755
--- a/tests/eventscripts/stubs/ip
+++ b/tests/eventscripts/stubs/ip
@@ -34,6 +34,39 @@ ip_link_set_down ()
     touch "${FAKE_IP_STATE}/interfaces-down/$3"
 }
 
+# This is incomplete because it doesn't actually look up table ids in
+# /etc/iproute2/rt_tables.  The rules/routes are actually associated
+# with the name instead of the number.  However, we include a variable
+# to fake a bad table id.
+[ -n "$IP_ROUTE_BAD_TABLE_ID" ] || IP_ROUTE_BAD_TABLE_ID=false
+
+ip_check_table ()
+{
+    _cmd="$1"
+
+    [ -n "$_table" ] || not_implemented "ip rule/route without \"table\""
+
+    # Only allow tables names from 13.per_ip_routing.  This is a cheap
+    # way of avoiding implementing the default/main/local tables.
+    case "$_table" in
+	ctdb.*)
+	    if $IP_ROUTE_BAD_TABLE_ID ; then
+		# Ouch.  Simulate inconsistent errors from ip.  :-(
+		case "$_cmd" in
+		    route)
+			echo "Error: argument "${_table}" is wrong: table id value is invalid" >&2
+			
+			;;
+		    *)
+			echo "Error: argument "${_table}" is wrong: invalid table ID" >&2
+		esac
+		exit 255
+	    fi
+	    ;;
+	*) not_implemented "table=${_table} in ${_args}" ;;
+    esac
+}
+
 ######################################################################
 
 ip_addr ()
@@ -288,7 +321,7 @@ ip_rule_common ()
     done
 
     [ -n "$_pre" ]   || not_implemented "ip rule without \"pref\""
-    [ -n "$_table" ] || not_implemented "ip rule without \"table\""
+    ip_check_table "rule"
     # Relax this if more selectors added later...
     [ -n "$_from" ]  || not_implemented "ip rule without \"from\""
 }
@@ -349,18 +382,6 @@ ip_route ()
     esac
 }
 
-ip_route_check_table ()
-{
-    [ -n "$_table" ] || not_implemented "ip rule without \"table\""
-
-    # Only allow tables names from 13.per_ip_routing.  This is a cheap
-    # way of avoiding implementing the default/main/local tables.
-    case "$_table" in
-	ctdb.*) : ;;
-	*) not_implemented "table=${_table} in ${_args}" ;;
-    esac
-}
-
 ip_route_common ()
 {
     _args="$*"
@@ -368,7 +389,7 @@ ip_route_common ()
     [ "$1" = table ] || not_implemented "$1 in \"$_args\""
     _table="$2"
 
-    ip_route_check_table
+    ip_check_table "route"
 }
 
 # Routes are in a file per table in the directory
@@ -413,7 +434,7 @@ ip_route_add ()
 	esac
     done
 
-    ip_route_check_table
+    ip_check_table "route"
     [ -n "$_prefix" ] || not_implemented "ip route without inet prefix in \"$_args\""
     [ -n "$_dev" ] || not_implemented "ip route without \"dev\" in \"$_args\""
 


-- 
CTDB repository


More information about the samba-cvs mailing list