From 14b5c5a97ebb0633896f3fb152da5802c266f887 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Mon, 12 Sep 2016 15:41:06 +1000 Subject: [PATCH 1/8] ctdb-ipalloc: Store known public IPs in IP allocation state This was dropped because it wasn't used, but it will be needed again. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12254 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit 0e5c62d8fc7fa51b4fb1dd435fe9c776586e5947) --- ctdb/server/ipalloc.c | 1 + ctdb/server/ipalloc_private.h | 1 + 2 files changed, 2 insertions(+) diff --git a/ctdb/server/ipalloc.c b/ctdb/server/ipalloc.c index dd88f81..4f86d6e 100644 --- a/ctdb/server/ipalloc.c +++ b/ctdb/server/ipalloc.c @@ -230,6 +230,7 @@ bool ipalloc_set_public_ips(struct ipalloc_state *ipalloc_state, struct ctdb_public_ip_list *available_ips) { ipalloc_state->available_public_ips = available_ips; + ipalloc_state->known_public_ips = known_ips; ipalloc_state->all_ips = create_merged_ip_list(ipalloc_state, known_ips); diff --git a/ctdb/server/ipalloc_private.h b/ctdb/server/ipalloc_private.h index 2328687..485f627 100644 --- a/ctdb/server/ipalloc_private.h +++ b/ctdb/server/ipalloc_private.h @@ -31,6 +31,7 @@ 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; -- 2.7.4 From 37521a6d1213bba85ff53a89a3a25b0fd3f90387 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Mon, 12 Sep 2016 15:49:03 +1000 Subject: [PATCH 2/8] ctdb-ipalloc: Whether IPs can be hosted need not depend on merged IP list Merged IP list won't be available here... BUG: https://bugzilla.samba.org/show_bug.cgi?id=12254 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit dfc3b8855d0a740d4f5218fcda4b3f64d4d6cd17) --- ctdb/server/ipalloc.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/ctdb/server/ipalloc.c b/ctdb/server/ipalloc.c index 4f86d6e..ccd12e7 100644 --- a/ctdb/server/ipalloc.c +++ b/ctdb/server/ipalloc.c @@ -245,14 +245,18 @@ bool ipalloc_set_public_ips(struct ipalloc_state *ipalloc_state, bool ipalloc_can_host_ips(struct ipalloc_state *ipalloc_state) { int i; - struct public_ip_list *ip_list; - - for (ip_list = ipalloc_state->all_ips; - ip_list != NULL; - ip_list = ip_list->next) { - if (ip_list->pnn != -1) { - return true; + for (i=0; i < ipalloc_state->num; i++) { + struct ctdb_public_ip_list *ips = + ipalloc_state->known_public_ips; + if (ips[i].num != 0) { + int j; + /* Succeed if an address is hosted on node i */ + for (j=0; j < ips[i].num; j++) { + if (ips[i].ip[j].pnn == i) { + return true; + } + } } } -- 2.7.4 From 3766170d64c4f1d39024ecb1a097dbce174c305c Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Mon, 12 Sep 2016 15:51:58 +1000 Subject: [PATCH 3/8] ctdb-ipalloc: Optimise check to see if IPs can be hosted Add an early return if there are no known IP addresses. Also add an extra comment for clarification. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12254 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit fed251726facf30225a131d43658975d5f4befb5) --- ctdb/server/ipalloc.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ctdb/server/ipalloc.c b/ctdb/server/ipalloc.c index ccd12e7..ffec5e9 100644 --- a/ctdb/server/ipalloc.c +++ b/ctdb/server/ipalloc.c @@ -245,12 +245,14 @@ bool ipalloc_set_public_ips(struct ipalloc_state *ipalloc_state, bool ipalloc_can_host_ips(struct ipalloc_state *ipalloc_state) { int i; + bool have_ips = false; for (i=0; i < ipalloc_state->num; i++) { struct ctdb_public_ip_list *ips = ipalloc_state->known_public_ips; if (ips[i].num != 0) { int j; + have_ips = true; /* Succeed if an address is hosted on node i */ for (j=0; j < ips[i].num; j++) { if (ips[i].ip[j].pnn == i) { @@ -260,6 +262,14 @@ bool ipalloc_can_host_ips(struct ipalloc_state *ipalloc_state) } } + if (! have_ips) { + return false; + } + + /* At this point there are known addresses but none are + * hosted. Need to check if cluster can now host some + * addresses. + */ for (i=0; i < ipalloc_state->num; i++) { if (ipalloc_state->available_public_ips[i].num != 0) { return true; -- 2.7.4 From 143d3e586431e7c8ec65bd150adfe9adb28b0c4e Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Mon, 12 Sep 2016 15:57:23 +1000 Subject: [PATCH 4/8] ctdb-ipalloc: Drop known_ips argument from merged IP list creation This is available in the IP allocation state. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12254 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit c1efb801a41f5b870317b14703b8a1cfecc5c68c) --- ctdb/server/ipalloc.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/ctdb/server/ipalloc.c b/ctdb/server/ipalloc.c index ffec5e9..e416c74 100644 --- a/ctdb/server/ipalloc.c +++ b/ctdb/server/ipalloc.c @@ -106,8 +106,7 @@ static int getips_count_callback(void *param, void *data) * merged list of all public addresses needs to be built so that IP * allocation can be done. */ static struct public_ip_list * -create_merged_ip_list(struct ipalloc_state *ipalloc_state, - struct ctdb_public_ip_list *known_ips) +create_merged_ip_list(struct ipalloc_state *ipalloc_state) { int i, j; struct public_ip_list *ip_list; @@ -116,14 +115,14 @@ create_merged_ip_list(struct ipalloc_state *ipalloc_state, ip_tree = trbt_create(ipalloc_state, 0); - if (known_ips == NULL) { + if (ipalloc_state->known_public_ips == NULL) { DEBUG(DEBUG_ERR, ("Known public IPs not set\n")); return NULL; } for (i=0; i < ipalloc_state->num; i++) { - public_ips = &known_ips[i]; + public_ips = &ipalloc_state->known_public_ips[i]; for (j=0; j < public_ips->num; j++) { struct public_ip_list *tmp_ip; @@ -232,8 +231,7 @@ bool ipalloc_set_public_ips(struct ipalloc_state *ipalloc_state, ipalloc_state->available_public_ips = available_ips; ipalloc_state->known_public_ips = known_ips; - ipalloc_state->all_ips = create_merged_ip_list(ipalloc_state, - known_ips); + ipalloc_state->all_ips = create_merged_ip_list(ipalloc_state); return (ipalloc_state->all_ips != NULL); } -- 2.7.4 From fd382bd5e8d0a375d642c82278437212b95f4f75 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Mon, 12 Sep 2016 15:59:09 +1000 Subject: [PATCH 5/8] ctdb-ipalloc: Move merged IP list creation to ipalloc() BUG: https://bugzilla.samba.org/show_bug.cgi?id=12254 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit 7522a7aee8f4756639311d2450b723debd674c6b) --- ctdb/server/ipalloc.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/ctdb/server/ipalloc.c b/ctdb/server/ipalloc.c index e416c74..7fd422a 100644 --- a/ctdb/server/ipalloc.c +++ b/ctdb/server/ipalloc.c @@ -231,9 +231,7 @@ bool ipalloc_set_public_ips(struct ipalloc_state *ipalloc_state, ipalloc_state->available_public_ips = available_ips; ipalloc_state->known_public_ips = known_ips; - ipalloc_state->all_ips = create_merged_ip_list(ipalloc_state); - - return (ipalloc_state->all_ips != NULL); + return true; } /* This can only return false if there are no available IPs *and* @@ -282,6 +280,11 @@ struct public_ip_list *ipalloc(struct ipalloc_state *ipalloc_state) { bool ret = false; + ipalloc_state->all_ips = create_merged_ip_list(ipalloc_state); + if (ipalloc_state->all_ips == NULL) { + return NULL; + } + switch (ipalloc_state->algorithm) { case IPALLOC_LCP2: ret = ipalloc_lcp2(ipalloc_state); -- 2.7.4 From c32158ef7f80264573ca2541d45a67934c0361e1 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Mon, 12 Sep 2016 16:04:18 +1000 Subject: [PATCH 6/8] ctdb-ipalloc: ipalloc_set_public_ips() can't fail So make it a void function. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12254 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit 7ec7d4f3c08799fa128401f08f2f810672b98ae8) --- ctdb/server/ctdb_takeover.c | 6 +----- ctdb/server/ipalloc.c | 4 +--- ctdb/server/ipalloc.h | 2 +- ctdb/tests/src/ctdb_takeover_tests.c | 5 +---- 4 files changed, 4 insertions(+), 13 deletions(-) diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index d10ffef..882ee49 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -1607,11 +1607,7 @@ int ctdb_takeover_run(struct ctdb_context *ctdb, struct ctdb_node_map_old *nodem return -1; } - if (! ipalloc_set_public_ips(ipalloc_state, known_ips, available_ips)) { - DEBUG(DEBUG_ERR, ("Failed to set public IPs\n")); - talloc_free(tmp_ctx); - return -1; - } + ipalloc_set_public_ips(ipalloc_state, known_ips, available_ips); if (! ipalloc_can_host_ips(ipalloc_state)) { DEBUG(DEBUG_WARNING,("No nodes available to host public IPs yet\n")); diff --git a/ctdb/server/ipalloc.c b/ctdb/server/ipalloc.c index 7fd422a..37804ea 100644 --- a/ctdb/server/ipalloc.c +++ b/ctdb/server/ipalloc.c @@ -224,14 +224,12 @@ void ipalloc_set_node_flags(struct ipalloc_state *ipalloc_state, } } -bool ipalloc_set_public_ips(struct ipalloc_state *ipalloc_state, +void ipalloc_set_public_ips(struct ipalloc_state *ipalloc_state, struct ctdb_public_ip_list *known_ips, struct ctdb_public_ip_list *available_ips) { ipalloc_state->available_public_ips = available_ips; ipalloc_state->known_public_ips = known_ips; - - return true; } /* This can only return false if there are no available IPs *and* diff --git a/ctdb/server/ipalloc.h b/ctdb/server/ipalloc.h index 66a5e75..d116426 100644 --- a/ctdb/server/ipalloc.h +++ b/ctdb/server/ipalloc.h @@ -56,7 +56,7 @@ void ipalloc_set_node_flags(struct ipalloc_state *ipalloc_state, uint32_t *tval_noiptakeover, uint32_t *tval_noiphostonalldisabled); -bool ipalloc_set_public_ips(struct ipalloc_state *ipalloc_state, +void ipalloc_set_public_ips(struct ipalloc_state *ipalloc_state, struct ctdb_public_ip_list *known_ips, struct ctdb_public_ip_list *available_ips); diff --git a/ctdb/tests/src/ctdb_takeover_tests.c b/ctdb/tests/src/ctdb_takeover_tests.c index a8f7841..bbeb241 100644 --- a/ctdb/tests/src/ctdb_takeover_tests.c +++ b/ctdb/tests/src/ctdb_takeover_tests.c @@ -300,10 +300,7 @@ static void ctdb_test_init(TALLOC_CTX *mem_ctx, read_ips_for_multiple_nodes, &known, &avail); - if (! ipalloc_set_public_ips(*ipalloc_state, known, avail)) { - DEBUG(DEBUG_ERR, ("Failed to set public IPs\n")); - exit(1); - } + ipalloc_set_public_ips(*ipalloc_state, known, avail); tval_noiptakeover = get_tunable_values(mem_ctx, nodemap->num, "CTDB_SET_NoIPTakeover"); -- 2.7.4 From 45c0e520ccd4ea38eb7a5da8ae5f68f3983023ec Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 13 Sep 2016 13:44:04 +1000 Subject: [PATCH 7/8] ctdb-tests: Factor out new local daemons functions ps_ctdbd Useful for being able to ensure that tests are doing what is expected. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12254 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit 3adf9cad47532d9ebf7ae3e4e618dfe7100008fb) --- ctdb/tests/simple/28_zero_eventscripts.sh | 7 +------ ctdb/tests/simple/scripts/local_daemons.bash | 10 ++++++++++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/ctdb/tests/simple/28_zero_eventscripts.sh b/ctdb/tests/simple/28_zero_eventscripts.sh index 7c03ae4..fc31c4a 100755 --- a/ctdb/tests/simple/28_zero_eventscripts.sh +++ b/ctdb/tests/simple/28_zero_eventscripts.sh @@ -35,11 +35,6 @@ CTDB_EVENT_SCRIPT_DIR="$empty_dir" daemons_start wait_until_ready -# If this fails to find processes then the tests fails, so look at -# full command-line so this will work with valgrind. Note that the -# output could be generated with pgrep's -a option but it doesn't -# exist in older versions. -ps -p $(pgrep -f '\' | xargs | sed -e 's| |,|g') -o args ww +ps_ctdbd -echo echo "Good, that seems to work!" diff --git a/ctdb/tests/simple/scripts/local_daemons.bash b/ctdb/tests/simple/scripts/local_daemons.bash index fb1e7e1..2d6ec56 100644 --- a/ctdb/tests/simple/scripts/local_daemons.bash +++ b/ctdb/tests/simple/scripts/local_daemons.bash @@ -173,3 +173,13 @@ _restart_ctdb_all () daemons_stop daemons_start } + +ps_ctdbd () +{ + # If this fails to find processes then the tests fails, so + # look at full command-line so this will work with valgrind. + # Note that the output could be generated with pgrep's -a + # option but it doesn't exist in older versions. + ps -p $(pgrep -f '\' | xargs | sed -e 's| |,|g') -o args ww + echo +} -- 2.7.4 From b9a9cac757f36fb0253e7ac354aa87e818bbe190 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 13 Sep 2016 13:48:19 +1000 Subject: [PATCH 8/8] ctdb-tests: Add new public IP takeover no-op test Test with DisableIPFailover=1 and with no public IP addresses configured. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12254 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs Autobuild-User(master): Martin Schwenke Autobuild-Date(master): Wed Sep 14 12:30:30 CEST 2016 on sn-devel-144 (cherry picked from commit 9f6015f8a61f71db7239fb8dce94781629b1d4cc) --- ctdb/tests/simple/19_ip_takeover_noop.sh | 71 ++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100755 ctdb/tests/simple/19_ip_takeover_noop.sh diff --git a/ctdb/tests/simple/19_ip_takeover_noop.sh b/ctdb/tests/simple/19_ip_takeover_noop.sh new file mode 100755 index 0000000..4cfedb0 --- /dev/null +++ b/ctdb/tests/simple/19_ip_takeover_noop.sh @@ -0,0 +1,71 @@ +#!/bin/bash + +test_info() +{ + cat <