ctdb-takeover: NoIPHostOnAllDisabled 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>

7 files changed, 34 insertions, 223 deletions

diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c
index cd13953d186..883951ffc6c 100644
--- a/ctdb/server/ctdb_takeover.c
+++ b/ctdb/server/ctdb_takeover.c
@@ -1236,118 +1236,6 @@ ctdb_fetch_remote_public_ips(struct ctdb_context *ctdb,
 	return public_ips;
 }
 
-struct get_tunable_callback_data {
-	const char *tunable;
-	uint32_t *out;
-	bool fatal;
-};
-
-static void get_tunable_callback(struct ctdb_context *ctdb, uint32_t pnn,
-				 int32_t res, TDB_DATA outdata,
-				 void *callback)
-{
-	struct get_tunable_callback_data *cd =
-		(struct get_tunable_callback_data *)callback;
-	int size;
-
-	if (res != 0) {
-		/* Already handled in fail callback */
-		return;
-	}
-
-	if (outdata.dsize != sizeof(uint32_t)) {
-		DEBUG(DEBUG_ERR,("Wrong size of returned data when reading \"%s\" tunable from node %d. Expected %d bytes but received %d bytes\n",
-				 cd->tunable, pnn, (int)sizeof(uint32_t),
-				 (int)outdata.dsize));
-		cd->fatal = true;
-		return;
-	}
-
-	size = talloc_array_length(cd->out);
-	if (pnn >= size) {
-		DEBUG(DEBUG_ERR,("Got %s reply from node %d but nodemap only has %d entries\n",
-				 cd->tunable, pnn, size));
-		return;
-	}
-
-		
-	cd->out[pnn] = *(uint32_t *)outdata.dptr;
-}
-
-static void get_tunable_fail_callback(struct ctdb_context *ctdb, uint32_t pnn,
-				       int32_t res, TDB_DATA outdata,
-				       void *callback)
-{
-	struct get_tunable_callback_data *cd =
-		(struct get_tunable_callback_data *)callback;
-
-	switch (res) {
-	case -ETIME:
-		DEBUG(DEBUG_ERR,
-		      ("Timed out getting tunable \"%s\" from node %d\n",
-		       cd->tunable, pnn));
-		cd->fatal = true;
-		break;
-	case -EINVAL:
-	case -1:
-		DEBUG(DEBUG_WARNING,
-		      ("Tunable \"%s\" not implemented on node %d\n",
-		       cd->tunable, pnn));
-		break;
-	default:
-		DEBUG(DEBUG_ERR,
-		      ("Unexpected error getting tunable \"%s\" from node %d\n",
-		       cd->tunable, pnn));
-		cd->fatal = true;
-	}
-}
-
-static uint32_t *get_tunable_from_nodes(struct ctdb_context *ctdb,
-					TALLOC_CTX *tmp_ctx,
-					struct ctdb_node_map_old *nodemap,
-					const char *tunable,
-					uint32_t default_value)
-{
-	TDB_DATA data;
-	struct ctdb_control_get_tunable *t;
-	uint32_t *nodes;
-	uint32_t *tvals;
-	struct get_tunable_callback_data callback_data;
-	int i;
-
-	tvals = talloc_array(tmp_ctx, uint32_t, nodemap->num);
-	CTDB_NO_MEMORY_NULL(ctdb, tvals);
-	for (i=0; i<nodemap->num; i++) {
-		tvals[i] = default_value;
-	}
-		
-	callback_data.out = tvals;
-	callback_data.tunable = tunable;
-	callback_data.fatal = false;
-
-	data.dsize = offsetof(struct ctdb_control_get_tunable, name) + strlen(tunable) + 1;
-	data.dptr  = talloc_size(tmp_ctx, data.dsize);
-	t = (struct ctdb_control_get_tunable *)data.dptr;
-	t->length = strlen(tunable)+1;
-	memcpy(t->name, tunable, t->length);
-	nodes = list_of_connected_nodes(ctdb, nodemap, tmp_ctx, true);
-	if (ctdb_client_async_control(ctdb, CTDB_CONTROL_GET_TUNABLE,
-				      nodes, 0, TAKEOVER_TIMEOUT(),
-				      false, data,
-				      get_tunable_callback,
-				      get_tunable_fail_callback,
-				      &callback_data) != 0) {
-		if (callback_data.fatal) {
-			talloc_free(tvals);
-			tvals = NULL;
-		}
-	}
-	talloc_free(nodes);
-	talloc_free(data.dptr);
-
-	return tvals;
-}
-
 static struct ctdb_node_map *
 ctdb_node_map_old_to_new(TALLOC_CTX *mem_ctx,
 			 const struct ctdb_node_map_old *old)
@@ -1373,26 +1261,15 @@ static bool set_ipflags(struct ctdb_context *ctdb,
 			struct ipalloc_state *ipalloc_state,
 			struct ctdb_node_map_old *nodemap)
 {
-	uint32_t *tval_noiphostonalldisabled;
 	struct ctdb_node_map *new;
 
-	tval_noiphostonalldisabled =
-		get_tunable_from_nodes(ctdb, ipalloc_state, nodemap,
-				       "NoIPHostOnAllDisabled", 0);
-	if (tval_noiphostonalldisabled == NULL) {
-		/* Caller frees tmp_ctx */
-		return false;
-	}
-
 	new = ctdb_node_map_old_to_new(ipalloc_state, nodemap);
 	if (new == NULL) {
 		return false;
 	}
 
-	ipalloc_set_node_flags(ipalloc_state, new,
-			     tval_noiphostonalldisabled);
+	ipalloc_set_node_flags(ipalloc_state, new);
 
-	talloc_free(tval_noiphostonalldisabled);
 	talloc_free(new);
 
 	return true;
@@ -1562,11 +1439,13 @@ int ctdb_takeover_run(struct ctdb_context *ctdb, struct ctdb_node_map_old *nodem
 		goto ipreallocated;
 	}
 
-	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);
+	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),
+		(ctdb->tunable.no_ip_host_on_all_disabled != 0),
+		force_rebalance_nodes);
 	if (ipalloc_state == NULL) {
 		talloc_free(tmp_ctx);
 		return -1;
diff --git a/ctdb/server/ipalloc.c b/ctdb/server/ipalloc.c
index 2a3cbc6d385..8b1f4f04210 100644
--- a/ctdb/server/ipalloc.c
+++ b/ctdb/server/ipalloc.c
@@ -40,6 +40,7 @@ ipalloc_state_init(TALLOC_CTX *mem_ctx,
 		   enum ipalloc_algorithm algorithm,
 		   bool no_ip_takeover,
 		   bool no_ip_failback,
+		   bool no_ip_host_on_all_disabled,
 		   uint32_t *force_rebalance_nodes)
 {
 	struct ipalloc_state *ipalloc_state =
@@ -63,6 +64,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->no_ip_host_on_all_disabled = no_ip_host_on_all_disabled;
 	ipalloc_state->force_rebalance_nodes = force_rebalance_nodes;
 
 	return ipalloc_state;
@@ -211,33 +213,25 @@ static bool all_nodes_are_disabled(struct ctdb_node_map *nodemap)
  *     Set NOIPHOST ip flags for disabled nodes
  */
 void ipalloc_set_node_flags(struct ipalloc_state *ipalloc_state,
-			    struct ctdb_node_map *nodemap,
-			    uint32_t *tval_noiphostonalldisabled)
+			    struct ctdb_node_map *nodemap)
 {
 	int i;
+	bool all_disabled = all_nodes_are_disabled(nodemap);
 
 	for (i=0;i<nodemap->num;i++) {
 		/* Can not host IPs on INACTIVE node */
 		if (nodemap->node[i].flags & NODE_FLAGS_INACTIVE) {
 			ipalloc_state->noiphost[i] = true;
 		}
-	}
 
-	if (all_nodes_are_disabled(nodemap)) {
-		/* If all nodes are disabled, can not host IPs on node
-		 * with NoIPHostOnAllDisabled set
-		 */
-		for (i=0;i<nodemap->num;i++) {
-			if (tval_noiphostonalldisabled[i] != 0) {
-				ipalloc_state->noiphost[i] = true;
-			}
-		}
-	} else {
-		/* If some nodes are not disabled, then can not host
-		 * IPs on DISABLED node
+		/* If node is disabled then it can only host IPs if
+		 * all nodes are disabled and NoIPHostOnAllDisabled is
+		 * unset
 		 */
-		for (i=0;i<nodemap->num;i++) {
-			if (nodemap->node[i].flags & NODE_FLAGS_DISABLED) {
+		if (nodemap->node[i].flags & NODE_FLAGS_DISABLED) {
+			if (!(all_disabled &&
+			      ipalloc_state->no_ip_host_on_all_disabled == 0)) {
+
 				ipalloc_state->noiphost[i] = true;
 			}
 		}
diff --git a/ctdb/server/ipalloc.h b/ctdb/server/ipalloc.h
index 26932466da2..40edddacb44 100644
--- a/ctdb/server/ipalloc.h
+++ b/ctdb/server/ipalloc.h
@@ -51,11 +51,11 @@ struct ipalloc_state * ipalloc_state_init(TALLOC_CTX *mem_ctx,
 					  enum ipalloc_algorithm algorithm,
 					  bool no_ip_takeover,
 					  bool no_ip_failback,
+					  bool no_ip_host_on_all_disabled,
 					  uint32_t *force_rebalance_nodes);
 
 void ipalloc_set_node_flags(struct ipalloc_state *ipalloc_state,
-			    struct ctdb_node_map *nodemap,
-			    uint32_t *tval_noiphostonalldisabled);
+			    struct ctdb_node_map *nodemap);
 
 void ipalloc_set_public_ips(struct ipalloc_state *ipalloc_state,
 			    struct ctdb_public_ip_list *known_ips,
diff --git a/ctdb/server/ipalloc_private.h b/ctdb/server/ipalloc_private.h
index f5b61c384c8..cd74d9cc3f6 100644
--- a/ctdb/server/ipalloc_private.h
+++ b/ctdb/server/ipalloc_private.h
@@ -38,6 +38,7 @@ struct ipalloc_state {
 	enum ipalloc_algorithm algorithm;
 	bool no_ip_failback;
 	bool no_ip_takeover;
+	bool no_ip_host_on_all_disabled;
 	uint32_t *force_rebalance_nodes;
 };
 
diff --git a/ctdb/tests/src/ctdb_takeover_tests.c b/ctdb/tests/src/ctdb_takeover_tests.c
index ee99e2ae9a1..5093757adc3 100644
--- a/ctdb/tests/src/ctdb_takeover_tests.c
+++ b/ctdb/tests/src/ctdb_takeover_tests.c
@@ -158,7 +158,7 @@ static void ctdb_test_init(TALLOC_CTX *mem_ctx,
 	const char *t;
 	struct ctdb_node_map *nodemap;
 	uint32_t noiptakeover;
-	uint32_t *tval_noiptakeoverondisabled;
+	uint32_t noiphostonalldisabled;
 	ctdb_sock_addr sa_zero = { .ip = { 0 } };
 	enum ipalloc_algorithm algorithm;
 
@@ -202,10 +202,19 @@ static void ctdb_test_init(TALLOC_CTX *mem_ctx,
 		noiptakeover = 0;
 	}
 
+	t = getenv("CTDB_SET_NoIPHostOnAllDisabled");
+	if (t != NULL) {
+		noiphostonalldisabled = (uint32_t) strtol(t, NULL, 0);
+	} else {
+		noiphostonalldisabled = 0;
+	}
+
 	*ipalloc_state = ipalloc_state_init(mem_ctx, nodemap->num,
 					    algorithm,
 					    (noiptakeover != 0),
-					    false, NULL);
+					    false,
+					    (noiphostonalldisabled != 0),
+					    NULL);
 	assert(*ipalloc_state != NULL);
 
 	read_ctdb_public_ip_info(mem_ctx, nodemap->num,
@@ -214,13 +223,7 @@ static void ctdb_test_init(TALLOC_CTX *mem_ctx,
 
 	ipalloc_set_public_ips(*ipalloc_state, known, avail);
 
-	tval_noiptakeoverondisabled =
-		get_tunable_values(mem_ctx, nodemap->num,
-				   "CTDB_SET_NoIPHostOnAllDisabled");
-	assert(tval_noiptakeoverondisabled != NULL);
-
-	ipalloc_set_node_flags(*ipalloc_state, nodemap,
-			       tval_noiptakeoverondisabled);
+	ipalloc_set_node_flags(*ipalloc_state, nodemap);
 }
 
 /* IP layout is read from stdin.  See comment for ctdb_test_init() for
diff --git a/ctdb/tests/takeover/lcp2.020.sh b/ctdb/tests/takeover/lcp2.020.sh
deleted file mode 100755
index a8897e05a2a..00000000000
--- a/ctdb/tests/takeover/lcp2.020.sh
+++ /dev/null
@@ -1,33 +0,0 @@
-#!/bin/sh
-
-. "${TEST_SCRIPTS_DIR}/unit.sh"
-
-define_test "3 nodes, all IPs assigned, 2->3 unhealthy, NoIPHostOnAllDisabled on 2"
-
-export CTDB_TEST_LOGLEVEL=ERR
-
-required_result <<EOF
-192.168.21.254 2
-192.168.21.253 2
-192.168.21.252 2
-192.168.20.254 2
-192.168.20.253 2
-192.168.20.252 2
-192.168.20.251 2
-192.168.20.250 2
-192.168.20.249 2
-EOF
-
-export CTDB_SET_NoIPHostOnAllDisabled=1,1,0
-
-simple_test 2,2,2 <<EOF
-192.168.21.254 2
-192.168.21.253 2
-192.168.21.252 2
-192.168.20.254 2
-192.168.20.253 2
-192.168.20.252 2
-192.168.20.251 2
-192.168.20.250 2
-192.168.20.249 2
-EOF
diff --git a/ctdb/tests/takeover/lcp2.021.sh b/ctdb/tests/takeover/lcp2.021.sh
deleted file mode 100755
index 4d185f2477b..00000000000
--- a/ctdb/tests/takeover/lcp2.021.sh
+++ /dev/null
@@ -1,33 +0,0 @@
-#!/bin/sh
-
-. "${TEST_SCRIPTS_DIR}/unit.sh"
-
-define_test "3 nodes, no IPs assigned, 3->2 unhealthy, NoIPHostOnAllDisabled on 2 others"
-
-export CTDB_TEST_LOGLEVEL=ERR
-
-required_result <<EOF
-192.168.21.254 0
-192.168.21.253 0
-192.168.21.252 0
-192.168.20.254 0
-192.168.20.253 0
-192.168.20.252 0
-192.168.20.251 0
-192.168.20.250 0
-192.168.20.249 0
-EOF
-
-export CTDB_SET_NoIPHostOnAllDisabled=0,1,1
-
-simple_test 2,2,2 <<EOF
-192.168.21.254 2
-192.168.21.253 2
-192.168.21.252 2
-192.168.20.254 2
-192.168.20.253 2
-192.168.20.252 2
-192.168.20.251 2
-192.168.20.250 2
-192.168.20.249 2
-EOF