ctdb-takeover: NoIPTakeover is global across cluster

Instead of gathering the value from all nodes, just use the value on
the recovery master and have it affect all nodes.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c
index 40c2b5d1baa4ca8af390e925a30607642987a19f..cd13953d18652e311fb00dff2b690fee1eec15f4 100644 (file)
--- a/ctdb/server/ctdb_takeover.c
+++ b/ctdb/server/ctdb_takeover.c
@@ -1373,16 +1373,9 @@ static bool set_ipflags(struct ctdb_context *ctdb,
                        struct ipalloc_state *ipalloc_state,
                        struct ctdb_node_map_old *nodemap)
 {
-       uint32_t *tval_noiptakeover;
        uint32_t *tval_noiphostonalldisabled;
        struct ctdb_node_map *new;
 
-       tval_noiptakeover = get_tunable_from_nodes(ctdb, ipalloc_state, nodemap,
-                                                  "NoIPTakeover", 0);
-       if (tval_noiptakeover == NULL) {
-               return false;
-       }
-
        tval_noiphostonalldisabled =
                get_tunable_from_nodes(ctdb, ipalloc_state, nodemap,
                                       "NoIPHostOnAllDisabled", 0);
@@ -1397,10 +1390,8 @@ static bool set_ipflags(struct ctdb_context *ctdb,
        }
 
        ipalloc_set_node_flags(ipalloc_state, new,
-                            tval_noiptakeover,
                             tval_noiphostonalldisabled);
 
-       talloc_free(tval_noiptakeover);
        talloc_free(tval_noiphostonalldisabled);
        talloc_free(new);
 
@@ -1573,6 +1564,7 @@ int ctdb_takeover_run(struct ctdb_context *ctdb, struct ctdb_node_map_old *nodem
 
        ipalloc_state = ipalloc_state_init(tmp_ctx, ctdb->num_nodes,
                                           determine_algorithm(&ctdb->tunable),
+                                          (ctdb->tunable.no_ip_takeover != 0),
                                           (ctdb->tunable.no_ip_failback != 0),
                                           force_rebalance_nodes);
        if (ipalloc_state == NULL) {

diff --git a/ctdb/server/ipalloc.c b/ctdb/server/ipalloc.c
index 819add164b0ba071d2929be71bab628fbd385b61..2a3cbc6d3857db6c02f9d7f421237dbe6fa5f59b 100644 (file)
--- a/ctdb/server/ipalloc.c
+++ b/ctdb/server/ipalloc.c
@@ -38,6 +38,7 @@ struct ipalloc_state *
 ipalloc_state_init(TALLOC_CTX *mem_ctx,
                   uint32_t num_nodes,
                   enum ipalloc_algorithm algorithm,
+                  bool no_ip_takeover,
                   bool no_ip_failback,
                   uint32_t *force_rebalance_nodes)
 {
@@ -50,14 +51,6 @@ ipalloc_state_init(TALLOC_CTX *mem_ctx,
 
        ipalloc_state->num = num_nodes;
 
-       ipalloc_state->noiptakeover =
-               talloc_zero_array(ipalloc_state,
-                                 bool,
-                                 ipalloc_state->num);
-       if (ipalloc_state->noiptakeover == NULL) {
-               DEBUG(DEBUG_ERR, (__location__ " Out of memory\n"));
-               goto fail;
-       }
        ipalloc_state->noiphost =
                talloc_zero_array(ipalloc_state,
                                  bool,
@@ -68,6 +61,7 @@ ipalloc_state_init(TALLOC_CTX *mem_ctx,
        }
 
        ipalloc_state->algorithm = algorithm;
+       ipalloc_state->no_ip_takeover = no_ip_takeover;
        ipalloc_state->no_ip_failback = no_ip_failback;
        ipalloc_state->force_rebalance_nodes = force_rebalance_nodes;
 
@@ -210,7 +204,6 @@ static bool all_nodes_are_disabled(struct ctdb_node_map *nodemap)
 
 /* Set internal flags for IP allocation:
  *   Clear ip flags
- *   Set NOIPTAKOVER ip flags from per-node NoIPTakeover tunable
  *   Set NOIPHOST ip flag for each INACTIVE node
  *   if all nodes are disabled:
  *     Set NOIPHOST ip flags from per-node NoIPHostOnAllDisabled tunable
@@ -219,17 +212,11 @@ static bool all_nodes_are_disabled(struct ctdb_node_map *nodemap)
  */
 void ipalloc_set_node_flags(struct ipalloc_state *ipalloc_state,
                            struct ctdb_node_map *nodemap,
-                           uint32_t *tval_noiptakeover,
                            uint32_t *tval_noiphostonalldisabled)
 {
        int i;
 
        for (i=0;i<nodemap->num;i++) {
-               /* Can not take IPs on node with NoIPTakeover set */
-               if (tval_noiptakeover[i] != 0) {
-                       ipalloc_state->noiptakeover[i] = true;
-               }
-
                /* Can not host IPs on INACTIVE node */
                if (nodemap->node[i].flags & NODE_FLAGS_INACTIVE) {
                        ipalloc_state->noiphost[i] = true;

diff --git a/ctdb/server/ipalloc.h b/ctdb/server/ipalloc.h
index 4413b2a0d19e574bbe30d57e5044148fb91b1329..26932466da27787147e2145c083d42e66208aa45 100644 (file)
--- a/ctdb/server/ipalloc.h
+++ b/ctdb/server/ipalloc.h
@@ -49,12 +49,12 @@ struct ipalloc_state;
 struct ipalloc_state * ipalloc_state_init(TALLOC_CTX *mem_ctx,
                                          uint32_t num_nodes,
                                          enum ipalloc_algorithm algorithm,
+                                         bool no_ip_takeover,
                                          bool no_ip_failback,
                                          uint32_t *force_rebalance_nodes);
 
 void ipalloc_set_node_flags(struct ipalloc_state *ipalloc_state,
                            struct ctdb_node_map *nodemap,
-                           uint32_t *tval_noiptakeover,
                            uint32_t *tval_noiphostonalldisabled);
 
 void ipalloc_set_public_ips(struct ipalloc_state *ipalloc_state,

diff --git a/ctdb/server/ipalloc_common.c b/ctdb/server/ipalloc_common.c
index 9ccba71012ece4d2804142e9fc6072307b92446e..6886a8183da0775f9375feafac2dfa1ce4cf6b32 100644 (file)
--- a/ctdb/server/ipalloc_common.c
+++ b/ctdb/server/ipalloc_common.c
@@ -72,7 +72,7 @@ bool can_node_takeover_ip(struct ipalloc_state *ipalloc_state,
                          int32_t pnn,
                          struct public_ip_list *ip)
 {
-       if (ipalloc_state->noiptakeover[pnn]) {
+       if (ipalloc_state->no_ip_takeover) {
                return false;
        }
 

diff --git a/ctdb/server/ipalloc_private.h b/ctdb/server/ipalloc_private.h
index 485f62738553d5deaf7afe9db36cbeb8e1aa25f9..f5b61c384c8bce37dfc1d574ebbd527c8e7a32ef 100644 (file)
--- a/ctdb/server/ipalloc_private.h
+++ b/ctdb/server/ipalloc_private.h
@@ -32,12 +32,12 @@ struct ipalloc_state {
        /* Arrays with data for each node */
        struct ctdb_public_ip_list *available_public_ips;
        struct ctdb_public_ip_list *known_public_ips;
-       bool *noiptakeover;
        bool *noiphost;
 
        struct public_ip_list *all_ips;
        enum ipalloc_algorithm algorithm;
        bool no_ip_failback;
+       bool no_ip_takeover;
        uint32_t *force_rebalance_nodes;
 };
 

diff --git a/ctdb/tests/simple/73_tunable_NoIPTakeover.sh b/ctdb/tests/simple/73_tunable_NoIPTakeover.sh
index 43fd8b267c57ee801d8f32a5f8ad5a50b2ec9ff1..62a307709be008c8642fae6e78e6f32cf2b78df5 100755 (executable)
--- a/ctdb/tests/simple/73_tunable_NoIPTakeover.sh
+++ b/ctdb/tests/simple/73_tunable_NoIPTakeover.sh
@@ -51,8 +51,8 @@ num=`try_command_on_node -v 1 "$CTDB ip" | grep -v Public | egrep " 1$" | wc -l`
 echo "Number of addresses on node 1 : $num"
 
 
-echo "Turning on NoIPTakeover on node 1"
-try_command_on_node 1 "$CTDB setvar NoIPTakeover 1"
+echo "Turning on NoIPTakeover on all nodes"
+try_command_on_node all "$CTDB setvar NoIPTakeover 1"
 try_command_on_node 1 "$CTDB ipreallocate"
 
 echo Disable node 1

diff --git a/ctdb/tests/src/ctdb_takeover_tests.c b/ctdb/tests/src/ctdb_takeover_tests.c
index 1f463300aecbfff3748d4ddbbf3035e0f2fb692d..ee99e2ae9a16cec2960dde80b2b20f798c1c4ec4 100644 (file)
--- a/ctdb/tests/src/ctdb_takeover_tests.c
+++ b/ctdb/tests/src/ctdb_takeover_tests.c
@@ -154,9 +154,10 @@ static void ctdb_test_init(TALLOC_CTX *mem_ctx,
 {
        struct ctdb_public_ip_list *known;
        struct ctdb_public_ip_list *avail;
-       char *tok, *ns, *t;
+       char *tok, *ns;
+       const char *t;
        struct ctdb_node_map *nodemap;
-       uint32_t *tval_noiptakeover;
+       uint32_t noiptakeover;
        uint32_t *tval_noiptakeoverondisabled;
        ctdb_sock_addr sa_zero = { .ip = { 0 } };
        enum ipalloc_algorithm algorithm;
@@ -194,8 +195,16 @@ static void ctdb_test_init(TALLOC_CTX *mem_ctx,
                }
        }
 
+       t = getenv("CTDB_SET_NoIPTakeover");
+       if (t != NULL) {
+               noiptakeover = (uint32_t) strtol(t, NULL, 0);
+       } else {
+               noiptakeover = 0;
+       }
+
        *ipalloc_state = ipalloc_state_init(mem_ctx, nodemap->num,
                                            algorithm,
+                                           (noiptakeover != 0),
                                            false, NULL);
        assert(*ipalloc_state != NULL);
 
@@ -205,16 +214,12 @@ static void ctdb_test_init(TALLOC_CTX *mem_ctx,
 
        ipalloc_set_public_ips(*ipalloc_state, known, avail);
 
-       tval_noiptakeover = get_tunable_values(mem_ctx, nodemap->num,
-                                              "CTDB_SET_NoIPTakeover");
-       assert(tval_noiptakeover != NULL);
        tval_noiptakeoverondisabled =
                get_tunable_values(mem_ctx, nodemap->num,
                                   "CTDB_SET_NoIPHostOnAllDisabled");
        assert(tval_noiptakeoverondisabled != NULL);
 
        ipalloc_set_node_flags(*ipalloc_state, nodemap,
-                              tval_noiptakeover,
                               tval_noiptakeoverondisabled);
 }
 

diff --git a/ctdb/tests/takeover/lcp2.012.sh b/ctdb/tests/takeover/lcp2.012.sh
index 6f95f0bef02b4f58769eedb97144f3a4b113fb9d..074cdcc5634c5df50788ea149595aad6a810daf9 100755 (executable)
--- a/ctdb/tests/takeover/lcp2.012.sh
+++ b/ctdb/tests/takeover/lcp2.012.sh
@@ -2,23 +2,23 @@
 
 . "${TEST_SCRIPTS_DIR}/unit.sh"
 
-define_test "Node with NODE_FLAGS_NOIPTAKEOVER doesn't gain IPs"
+define_test "NoIPTakeover - nodes don't gain IPs"
 
 export CTDB_TEST_LOGLEVEL=ERR
 
 required_result <<EOF
 192.168.21.254 1
-192.168.21.253 2
+192.168.21.253 1
 192.168.21.252 1
-192.168.20.254 2
-192.168.20.253 2
+192.168.20.254 1
+192.168.20.253 1
 192.168.20.252 1
-192.168.20.251 2
+192.168.20.251 1
 192.168.20.250 1
 192.168.20.249 1
 EOF
 
-export CTDB_SET_NoIPTakeover="1,0,0"
+export CTDB_SET_NoIPTakeover=1
 
 simple_test 0,0,0 <<EOF
 192.168.20.249 1

diff --git a/ctdb/tests/takeover/lcp2.013.sh b/ctdb/tests/takeover/lcp2.013.sh
index bc2116aff6b6e7b519f2d7667ef26676f698a94a..091a23538fa46ec2f16341a4a13fd1a6fdefae3d 100755 (executable)
--- a/ctdb/tests/takeover/lcp2.013.sh
+++ b/ctdb/tests/takeover/lcp2.013.sh
@@ -2,7 +2,7 @@
 
 . "${TEST_SCRIPTS_DIR}/unit.sh"
 
-define_test "Node with NODE_FLAGS_NOIPTAKEOVER doesn't lose IPs"
+define_test "NoIPTakeover: nodes don't lose IPs"
 
 export CTDB_TEST_LOGLEVEL=ERR
 
@@ -18,7 +18,7 @@ required_result <<EOF
 192.168.20.249 0
 EOF
 
-export CTDB_SET_NoIPTakeover="1,0,0"
+export CTDB_SET_NoIPTakeover=1
 
 simple_test 0,0,0 <<EOF
 192.168.20.249 0

diff --git a/ctdb/tests/takeover/lcp2.022.sh b/ctdb/tests/takeover/lcp2.022.sh
index 77089b03cebb1e50f208d6565b7874143a088947..52ac6ea4ce346dbac865d294018852dc1b2eba78 100755 (executable)
--- a/ctdb/tests/takeover/lcp2.022.sh
+++ b/ctdb/tests/takeover/lcp2.022.sh
@@ -2,25 +2,26 @@
 
 . "${TEST_SCRIPTS_DIR}/unit.sh"
 
-define_test "3 nodes, 2->3 unhealthy, all IPs assigned, split NoIPTakeover"
+define_test "3 nodes, 2->3 unhealthy, all IPs assigned, NoIPTakeover"
 
 export CTDB_TEST_LOGLEVEL=ERR
 
-# We expect 1/2 the IPs to move, but the rest to stay (as opposed to
-# NoIPHostOnAllDisabled)
+# We expect the IPs stay where they are (as opposed to
+# NoIPHostOnAllDisabled).  IPs are hosted when all nodes are disabled,
+# but they have nowhere else to go because of NoIPTakeover.
 required_result <<EOF
 192.168.21.254 2
-192.168.21.253 0
+192.168.21.253 2
 192.168.21.252 2
-192.168.20.254 0
-192.168.20.253 0
+192.168.20.254 2
+192.168.20.253 2
 192.168.20.252 2
-192.168.20.251 0
+192.168.20.251 2
 192.168.20.250 2
 192.168.20.249 2
 EOF
 
-export CTDB_SET_NoIPTakeover=0,1,1
+export CTDB_SET_NoIPTakeover=1
 
 simple_test 2,2,2 <<EOF
 192.168.21.254 2