From 38279d7ec1d877e5c3752fce5c0f74e19f148674 Mon Sep 17 00:00:00 2001
From: Martin Schwenke <martin@meltin.net>
Date: Wed, 26 Mar 2014 16:50:59 +1100
Subject: [PATCH] ctdb-eventscripts: Make 11.natgw stateful

IP addresses and routes are only changed if either the NAT gateway
configuration or the NAT gateway master node has changed.  If running
"ip monitor" this will minimise the amount of noise seen.  It should
also be more lightweight at the expense of managing a couple of state
files.

Add a test to check that configuration changes behave correctly.
Tweak the static route result generation code so that the required
output is sorted.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>
---
 ctdb/config/events.d/11.natgw                      | 76 +++++++++++++++++++++-
 ctdb/tests/eventscripts/11.natgw.015.sh            |  4 +-
 ctdb/tests/eventscripts/11.natgw.025.sh            |  4 +-
 .../{11.natgw.025.sh => 11.natgw.031.sh}           | 34 ++++------
 ctdb/tests/eventscripts/scripts/local.sh           |  2 +
 ctdb/tests/eventscripts/stubs/ip                   |  2 +-
 6 files changed, 95 insertions(+), 27 deletions(-)
 copy ctdb/tests/eventscripts/{11.natgw.025.sh => 11.natgw.031.sh} (54%)

diff --git a/ctdb/config/events.d/11.natgw b/ctdb/config/events.d/11.natgw
index aef302c59e9..e107f47db77 100755
--- a/ctdb/config/events.d/11.natgw
+++ b/ctdb/config/events.d/11.natgw
@@ -9,11 +9,20 @@
     export CTDB_BASE=$(cd -P $(dirname "$0") ; dirname "$PWD")
 
 . $CTDB_BASE/functions
+
+service_name=natgw
+
 loadconfig
 
 [ -n "$CTDB_NATGW_NODES" ] || exit 0
 export CTDB_NATGW_NODES
 
+ctdb_setup_service_state_dir
+
+natgw_cfg_new="${service_state_dir}/cfg_new"
+natgw_cfg_old="${service_state_dir}/cfg_old"
+natgw_master_old="${service_state_dir}/master_old"
+
 natgw_check_config ()
 {
     [ -r "$CTDB_NATGW_NODES" ] || \
@@ -31,6 +40,39 @@ natgw_check_config ()
     [ -n "$CTDB_NATGW_STATIC_ROUTES" ] || CTDB_NATGW_STATIC_ROUTES="0.0.0.0/0"
 }
 
+natgw_write_config ()
+{
+    _f="$1"
+
+    cat >"$_f" <<EOF
+CTDB_NATGW_NODES="$CTDB_NATGW_NODES"
+CTDB_NATGW_PUBLIC_IP="$CTDB_NATGW_PUBLIC_IP"
+CTDB_NATGW_PUBLIC_IFACE="$CTDB_NATGW_PUBLIC_IFACE"
+CTDB_NATGW_DEFAULT_GATEWAY="$CTDB_NATGW_DEFAULT_GATEWAY"
+CTDB_NATGW_PRIVATE_NETWORK="$CTDB_NATGW_PRIVATE_NETWORK"
+CTDB_NATGW_STATIC_ROUTES="$CTDB_NATGW_STATIC_ROUTES"
+CTDB_NATGW_SLAVE_ONLY="$CTDB_NATGW_SLAVE_ONLY"
+EOF
+}
+
+natgw_config_has_changed ()
+{
+    natgw_write_config "$natgw_cfg_new"
+
+    # Non-existent old returns true, no log message
+    if [ ! -f "$natgw_cfg_old" ] ; then
+	return 0
+    fi
+
+    # Handle no change
+    if cmp "$natgw_cfg_old" "$natgw_cfg_new" >/dev/null 2>&1 ; then
+	return 1
+    fi
+
+    echo "NAT gateway configuration has changed"
+    return 0
+}
+
 natgw_set_capability ()
 {
     # Set NATGW capability depending on configuration
@@ -41,7 +83,7 @@ natgw_set_capability ()
     fi
 }
 
-natgw_clear ()
+_natgw_clear ()
 {
     _ip="${CTDB_NATGW_PUBLIC_IP%/*}"
     _maskbits="${CTDB_NATGW_PUBLIC_IP#*/}"
@@ -62,6 +104,15 @@ natgw_clear ()
     iptables -D INPUT -p tcp --syn -d $_ip/32 -j REJECT 2>/dev/null
 }
 
+natgw_clear ()
+{
+    if [ -r "$natgw_cfg_old" ] ; then
+	(. $natgw_cfg_old ; _natgw_clear)
+    else
+	_natgw_clear
+    fi
+}
+
 natgw_set_master ()
 {
     set_proc sys/net/ipv4/ip_forward 1
@@ -116,6 +167,24 @@ natgw_ensure_master ()
     fi
 }
 
+natgw_master_has_changed ()
+{
+    if [ -r "$natgw_master_old" ] ; then
+	read _old_natgwmaster <"$natgw_master_old"
+    else
+	_old_natgwmaster=""
+    fi
+    [ "$_old_natgwmaster" != "$natgwmaster" ]
+}
+
+natgw_save_state ()
+{
+    echo "$natgwmaster" >"$natgw_master_old"
+    # Created by natgw_config_has_changed()
+    mv "$natgw_cfg_new" "$natgw_cfg_old"
+}
+
+
 case "$1" in 
     setup)
 	natgw_check_config
@@ -144,6 +213,8 @@ case "$1" in
 	natgw_set_capability
 	natgw_ensure_master
 
+	natgw_config_has_changed || natgw_master_has_changed || exit 0
+
 	natgw_clear
 
 	if [ "$mypnn" = "$natgwmaster" ]; then
@@ -154,6 +225,9 @@ case "$1" in
 
 	# flush our route cache
 	set_proc sys/net/ipv4/route/flush 1
+
+	# Only update saved state when NATGW successfully updated
+	natgw_save_state
 	;;
 
     shutdown|removenatgw)
diff --git a/ctdb/tests/eventscripts/11.natgw.015.sh b/ctdb/tests/eventscripts/11.natgw.015.sh
index d202ae670bc..c7e0c15262f 100755
--- a/ctdb/tests/eventscripts/11.natgw.015.sh
+++ b/ctdb/tests/eventscripts/11.natgw.015.sh
@@ -33,7 +33,7 @@ setup_ctdb_natgw <<EOF
 192.168.1.24
 EOF
 
-ok_null
+ok "NAT gateway configuration has changed"
 simple_test_event "ipreallocated"
 
 ok "default via ${FAKE_CTDB_NATGW_MASTER} dev ethXXX  metric 10 "
@@ -51,7 +51,7 @@ setup_ctdb_natgw <<EOF
 192.168.1.24
 EOF
 
-ok_null
+ok "NAT gateway configuration has changed"
 simple_test_event "ipreallocated"
 
 ok "default via ${CTDB_NATGW_DEFAULT_GATEWAY} dev ethXXX  metric 10 "
diff --git a/ctdb/tests/eventscripts/11.natgw.025.sh b/ctdb/tests/eventscripts/11.natgw.025.sh
index 60602d3ead9..456fc3c030f 100755
--- a/ctdb/tests/eventscripts/11.natgw.025.sh
+++ b/ctdb/tests/eventscripts/11.natgw.025.sh
@@ -35,7 +35,7 @@ setup_ctdb_natgw <<EOF
 192.168.1.24
 EOF
 
-ok_null
+ok "NAT gateway configuration has changed"
 simple_test_event "ipreallocated"
 
 ok_natgw_slave_static_routes
@@ -53,7 +53,7 @@ setup_ctdb_natgw <<EOF
 192.168.1.24
 EOF
 
-ok_null
+ok "NAT gateway configuration has changed"
 simple_test_event "ipreallocated"
 
 ok_natgw_master_static_routes
diff --git a/ctdb/tests/eventscripts/11.natgw.025.sh b/ctdb/tests/eventscripts/11.natgw.031.sh
similarity index 54%
copy from ctdb/tests/eventscripts/11.natgw.025.sh
copy to ctdb/tests/eventscripts/11.natgw.031.sh
index 60602d3ead9..a8575a57937 100755
--- a/ctdb/tests/eventscripts/11.natgw.025.sh
+++ b/ctdb/tests/eventscripts/11.natgw.031.sh
@@ -2,13 +2,14 @@
 
 . "${TEST_SCRIPTS_DIR}/unit.sh"
 
-define_test "static routes, custom gateway, multiple transitions"
+define_test "master node, static routes, custom gateway, config change"
 
 setup_ctdb
 
 export CTDB_NATGW_STATIC_ROUTES="10.1.1.0/24 10.1.2.0/24@10.1.1.253"
 
-echo "*** Master node..."
+echo "##################################################"
+echo "Static routes..."
 
 setup_ctdb_natgw <<EOF
 192.168.1.21 master
@@ -26,34 +27,26 @@ simple_test_command ip route show
 ok_natgw_master_ip_addr_show
 simple_test_command ip addr show "$CTDB_NATGW_PUBLIC_IFACE"
 
-echo "*** Slave node..."
+echo "##################################################"
+echo "Default routes..."
 
-setup_ctdb_natgw <<EOF
-192.168.1.21
-192.168.1.22 master
-192.168.1.23
-192.168.1.24
-EOF
+unset CTDB_NATGW_STATIC_ROUTES
 
-ok_null
+ok "NAT gateway configuration has changed"
 simple_test_event "ipreallocated"
 
-ok_natgw_slave_static_routes
+ok "default via ${CTDB_NATGW_DEFAULT_GATEWAY} dev ethXXX  metric 10 "
 simple_test_command ip route show
 
-ok_natgw_slave_ip_addr_show
+ok_natgw_master_ip_addr_show
 simple_test_command ip addr show "$CTDB_NATGW_PUBLIC_IFACE"
 
-echo "*** Master node again..."
+echo "##################################################"
+echo "Static routes again..."
 
-setup_ctdb_natgw <<EOF
-192.168.1.21 master
-192.168.1.22
-192.168.1.23
-192.168.1.24
-EOF
+export CTDB_NATGW_STATIC_ROUTES="10.1.3.0/24 10.1.4.4/32 10.1.2.0/24@10.1.1.252"
 
-ok_null
+ok "NAT gateway configuration has changed"
 simple_test_event "ipreallocated"
 
 ok_natgw_master_static_routes
@@ -61,4 +54,3 @@ simple_test_command ip route show
 
 ok_natgw_master_ip_addr_show
 simple_test_command ip addr show "$CTDB_NATGW_PUBLIC_IFACE"
-
diff --git a/ctdb/tests/eventscripts/scripts/local.sh b/ctdb/tests/eventscripts/scripts/local.sh
index 219bbee51c9..aba53e84261 100644
--- a/ctdb/tests/eventscripts/scripts/local.sh
+++ b/ctdb/tests/eventscripts/scripts/local.sh
@@ -675,6 +675,7 @@ ok_natgw_master_static_routes ()
 	_t="${_t}${_t:+${_nl}}"
 	_t="${_t}${_net} via ${_gw} dev ethXXX  metric 10 "
     done
+    _t=$(echo "$_t" | sort)
     ok "$_t"
 }
 
@@ -694,6 +695,7 @@ ok_natgw_slave_static_routes ()
 	_t="${_t}${_t:+${_nl}}"
 	_t="${_t}${_net} via ${FAKE_CTDB_NATGW_MASTER} dev ethXXX  metric 10 "
     done
+    _t=$(echo "$_t" | sort)
     ok "$_t"
 }
 
diff --git a/ctdb/tests/eventscripts/stubs/ip b/ctdb/tests/eventscripts/stubs/ip
index e8f17d84131..696758b407e 100755
--- a/ctdb/tests/eventscripts/stubs/ip
+++ b/ctdb/tests/eventscripts/stubs/ip
@@ -479,7 +479,7 @@ ip_route_show ()
     ip_route_common "$@"
 
     # Missing file is just an empty table
-    cat "$FAKE_IP_STATE/routes/${_table}" 2>/dev/null || true
+    sort "$FAKE_IP_STATE/routes/${_table}" 2>/dev/null || true
 }
 
 ip_route_flush ()
-- 
2.11.4.GIT