From bdd0f7d3906fd147b8d8a5f965034a6e03e422fa Mon Sep 17 00:00:00 2001 From: Amitay Isaacs Date: Fri, 25 Aug 2017 15:00:59 +1000 Subject: [PATCH 1/6] ctdb-daemon: Fix implementation of process_exists control Only check processes that are CTDB clients. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012 Signed-off-by: Amitay Isaacs Reviewed-by: Martin Schwenke (cherry picked from commit d0a20baf43834c7290dfd8f256d9521724202f0c) --- ctdb/server/ctdb_daemon.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c index d0d86a0c703..122d884a0c9 100644 --- a/ctdb/server/ctdb_daemon.c +++ b/ctdb/server/ctdb_daemon.c @@ -1800,12 +1800,16 @@ int32_t ctdb_control_process_exists(struct ctdb_context *ctdb, pid_t pid) { struct ctdb_client *client; - if (ctdb->nodes[ctdb->pnn]->flags & (NODE_FLAGS_BANNED|NODE_FLAGS_STOPPED)) { - client = ctdb_find_client_by_pid(ctdb, pid); - if (client != NULL) { - DEBUG(DEBUG_NOTICE,(__location__ " Killing client with pid:%d on banned/stopped node\n", (int)pid)); - talloc_free(client); - } + client = ctdb_find_client_by_pid(ctdb, pid); + if (client == NULL) { + return -1; + } + + if (ctdb->nodes[ctdb->pnn]->flags & NODE_FLAGS_INACTIVE) { + DEBUG(DEBUG_NOTICE, + ("Killing client with pid:%d on banned/stopped node\n", + (int)pid)); + talloc_free(client); return -1; } -- 2.13.5 From 7b64d2c6ae63357d1bfa39968fdb199e96a24d28 Mon Sep 17 00:00:00 2001 From: Amitay Isaacs Date: Fri, 25 Aug 2017 16:54:47 +1000 Subject: [PATCH 2/6] ctdb-tests: Fix the implementation of process-exists in fake daemon Keep track of clients and their pids. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012 Signed-off-by: Amitay Isaacs Reviewed-by: Martin Schwenke (cherry picked from commit 7dec80a7c042d83f9d48c75a8717c3d1b59b1fbf) --- ctdb/tests/src/fake_ctdbd.c | 81 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 2 deletions(-) diff --git a/ctdb/tests/src/fake_ctdbd.c b/ctdb/tests/src/fake_ctdbd.c index 7aa3ca229a3..9e2062a5084 100644 --- a/ctdb/tests/src/fake_ctdbd.c +++ b/ctdb/tests/src/fake_ctdbd.c @@ -108,6 +108,13 @@ struct fake_control_failure { const char *comment; }; +struct ctdb_client { + struct ctdb_client *prev, *next; + struct ctdbd_context *ctdb; + pid_t pid; + void *state; +}; + struct ctdbd_context { struct node_map *node_map; struct interface_map *iface_map; @@ -126,6 +133,7 @@ struct ctdbd_context { char *reclock; struct ctdb_public_ip_list *known_ips; struct fake_control_failure *control_failures; + struct ctdb_client *client_list; }; /* @@ -820,6 +828,48 @@ fail: } /* + * Manage clients + */ + +static int ctdb_client_destructor(struct ctdb_client *client) +{ + DLIST_REMOVE(client->ctdb->client_list, client); + return 0; +} + +static int client_add(struct ctdbd_context *ctdb, pid_t client_pid, + void *client_state) +{ + struct ctdb_client *client; + + client = talloc_zero(client_state, struct ctdb_client); + if (client == NULL) { + return ENOMEM; + } + + client->ctdb = ctdb; + client->pid = client_pid; + client->state = client_state; + + DLIST_ADD(ctdb->client_list, client); + talloc_set_destructor(client, ctdb_client_destructor); + return 0; +} + +static void *client_find(struct ctdbd_context *ctdb, pid_t client_pid) +{ + struct ctdb_client *client; + + for (client=ctdb->client_list; client != NULL; client=client->next) { + if (client->pid == client_pid) { + return client->state; + } + } + + return NULL; +} + +/* * CTDB context setup */ @@ -1143,6 +1193,7 @@ struct client_state { int fd; struct ctdbd_context *ctdb; int pnn; + pid_t pid; struct comm_context *comm; struct srvid_register_state *rstate; int status; @@ -1256,11 +1307,22 @@ static void control_process_exists(TALLOC_CTX *mem_ctx, struct ctdb_req_header *header, struct ctdb_req_control *request) { + struct client_state *state = tevent_req_data( + req, struct client_state); + struct ctdbd_context *ctdb = state->ctdb; + struct client_state *cstate; struct ctdb_reply_control reply; reply.rdata.opcode = request->opcode; - reply.status = kill(request->rdata.data.pid, 0); - reply.errmsg = NULL; + + cstate = client_find(ctdb, request->rdata.data.pid); + if (cstate == NULL) { + reply.status = -1; + reply.errmsg = "No client for PID"; + } else { + reply.status = kill(request->rdata.data.pid, 0); + reply.errmsg = NULL; + } client_send_control(req, header, &reply); } @@ -2975,6 +3037,8 @@ static struct tevent_req *client_send(TALLOC_CTX *mem_ctx, { struct tevent_req *req; struct client_state *state; + struct ucred cr; + socklen_t crl = sizeof(struct ucred); int ret; req = tevent_req_create(mem_ctx, &state, struct client_state); @@ -2987,6 +3051,13 @@ static struct tevent_req *client_send(TALLOC_CTX *mem_ctx, state->ctdb = ctdb; state->pnn = pnn; + ret = getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &cr, &crl); + if (ret != 0) { + tevent_req_error(req, ret); + return tevent_req_post(req, ev); + } + state->pid = cr.pid; + ret = comm_setup(state, ev, fd, client_read_handler, req, client_dead_handler, req, &state->comm); if (ret != 0) { @@ -2994,6 +3065,12 @@ static struct tevent_req *client_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } + ret = client_add(ctdb, state->pid, state); + if (ret != 0) { + tevent_req_error(req, ret); + return tevent_req_post(req, ev); + } + DEBUG(DEBUG_INFO, ("New client fd=%d\n", fd)); return req; -- 2.13.5 From c81639c5744c5359a9f152112d87dce08b75e94a Mon Sep 17 00:00:00 2001 From: Amitay Isaacs Date: Wed, 30 Aug 2017 13:05:32 +1000 Subject: [PATCH 3/6] ctdb-tests: Add a dummy ctdb client for testing BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012 Signed-off-by: Amitay Isaacs Reviewed-by: Martin Schwenke (cherry picked from commit 3067db5b50162fdae288aaad8e75beb924fc9494) --- ctdb/tests/src/dummy_client.c | 148 ++++++++++++++++++++++++++++++++++++++++++ ctdb/wscript | 3 +- 2 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 ctdb/tests/src/dummy_client.c diff --git a/ctdb/tests/src/dummy_client.c b/ctdb/tests/src/dummy_client.c new file mode 100644 index 00000000000..6af41f3b1f9 --- /dev/null +++ b/ctdb/tests/src/dummy_client.c @@ -0,0 +1,148 @@ +/* + Dummy CTDB client for testing + + Copyright (C) Amitay Isaacs 2017 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, see . +*/ + +#include "replace.h" +#include "system/network.h" + +#include +#include +#include + +#include "common/logging.h" + +#include "client/client.h" + +static struct { + const char *sockpath; + const char *debuglevel; + int timelimit; + const char *srvidstr; +} options; + +static struct poptOption cmdline_options[] = { + POPT_AUTOHELP + { "socket", 's', POPT_ARG_STRING, &options.sockpath, 0, + "Unix domain socket path", "filename" }, + { "debug", 'd', POPT_ARG_STRING, &options.debuglevel, 0, + "debug level", "ERR|WARNING|NOTICE|INFO|DEBUG" } , + { "timelimit", 't', POPT_ARG_INT, &options.timelimit, 0, + "time limit", "seconds" }, + { "srvid", 'S', POPT_ARG_STRING, &options.srvidstr, 0, + "srvid to register", "srvid" }, + POPT_TABLEEND +}; + +static void dummy_handler(uint64_t srvid, TDB_DATA data, void *private_data) +{ + bool *done = (bool *)private_data; + + *done = true; +} + +int main(int argc, const char *argv[]) +{ + TALLOC_CTX *mem_ctx; + struct tevent_context *ev; + struct ctdb_client_context *client; + const char *ctdb_socket; + poptContext pc; + int opt, ret; + int log_level; + bool status, done; + + /* Set default options */ + options.sockpath = CTDB_SOCKET; + options.debuglevel = "ERR"; + options.timelimit = 60; + options.srvidstr = NULL; + + ctdb_socket = getenv("CTDB_SOCKET"); + if (ctdb_socket != NULL) { + options.sockpath = ctdb_socket; + } + + pc = poptGetContext(argv[0], argc, argv, cmdline_options, + POPT_CONTEXT_KEEP_FIRST); + while ((opt = poptGetNextOpt(pc)) != -1) { + fprintf(stderr, "Invalid option %s\n", poptBadOption(pc, 0)); + exit(1); + } + + if (options.sockpath == NULL) { + fprintf(stderr, "Please specify socket path\n"); + poptPrintHelp(pc, stdout, 0); + exit(1); + } + + mem_ctx = talloc_new(NULL); + if (mem_ctx == NULL) { + fprintf(stderr, "Memory allocation error\n"); + exit(1); + } + + ev = tevent_context_init(mem_ctx); + if (ev == NULL) { + fprintf(stderr, "Memory allocation error\n"); + exit(1); + } + + status = debug_level_parse(options.debuglevel, &log_level); + if (! status) { + fprintf(stderr, "Invalid debug level\n"); + poptPrintHelp(pc, stdout, 0); + exit(1); + } + + setup_logging("dummy_client", DEBUG_STDERR); + DEBUGLEVEL = log_level; + + ret = ctdb_client_init(mem_ctx, ev, options.sockpath, &client); + if (ret != 0) { + D_ERR("Failed to initialize client, ret=%d\n", ret); + exit(1); + } + + done = false; + if (options.srvidstr != NULL) { + uint64_t srvid; + + srvid = strtoull(options.srvidstr, NULL, 0); + + ret = ctdb_client_set_message_handler(ev, client, srvid, + dummy_handler, &done); + if (ret != 0) { + D_ERR("Failed to register srvid, ret=%d\n", ret); + talloc_free(client); + exit(1); + } + + D_INFO("Registered SRVID 0x%"PRIx64"\n", srvid); + } + + ret = ctdb_client_wait_timeout(ev, &done, + tevent_timeval_current_ofs(options.timelimit, 0)); + if (ret != 0 && ret == ETIME) { + D_ERR("client_wait_timeout() failed, ret=%d\n", ret); + talloc_free(client); + exit(1); + } + + talloc_free(client); + exit(0); +} diff --git a/ctdb/wscript b/ctdb/wscript index fe7d7122308..754cdf3c01f 100644 --- a/ctdb/wscript +++ b/ctdb/wscript @@ -793,7 +793,8 @@ def build(bld): 'transaction_loop', 'update_record', 'update_record_persistent', - 'lock_tdb' + 'lock_tdb', + 'dummy_client' ] for target in ctdb_tests: -- 2.13.5 From 3682a500d1639cd3ca37a556ba897e9255f6d7a6 Mon Sep 17 00:00:00 2001 From: Amitay Isaacs Date: Fri, 25 Aug 2017 16:55:34 +1000 Subject: [PATCH 4/6] ctdb-tests: Fix ctdb process-exist tests BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012 Signed-off-by: Amitay Isaacs Reviewed-by: Martin Schwenke Autobuild-User(master): Amitay Isaacs Autobuild-Date(master): Sat Sep 9 14:44:57 CEST 2017 on sn-devel-144 (cherry picked from commit 87f7d32a906799e83cb9b023978e689a630de017) --- ctdb/tests/simple/07_ctdb_process_exists.sh | 34 ++++++++++++++++++----------- ctdb/tests/tool/ctdb.process-exists.001.sh | 12 +++++----- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/ctdb/tests/simple/07_ctdb_process_exists.sh b/ctdb/tests/simple/07_ctdb_process_exists.sh index b7492a84f19..c44924b0fe6 100755 --- a/ctdb/tests/simple/07_ctdb_process_exists.sh +++ b/ctdb/tests/simple/07_ctdb_process_exists.sh @@ -15,11 +15,10 @@ Prerequisites: Steps: 1. Verify that the status on all of the ctdb nodes is 'OK'. -2. On one of the cluster nodes, get the PID of an existing process - (using ps wax). +2. On one of the cluster nodes, get the PID of a ctdb client. 3. Run 'ctdb process-exists ' on the node and verify that the correct output is shown. -4. Run 'ctdb process-exists ' with a pid of a non-existent +4. Run 'ctdb process-exists ' with a pid of ctdb daemon process and verify that the correct output is shown. Expected results: @@ -38,15 +37,25 @@ cluster_is_healthy test_node=1 -# Create a background process on $test_node that will last for 60 seconds. +# Execute a ctdb client on $test_node that will last for 60 seconds. # It should still be there when we check. -try_command_on_node $test_node 'sleep 60 >/dev/null 2>&1 & echo $!' -pid="$out" +try_command_on_node -v $test_node \ + "$CTDB_TEST_WRAPPER exec dummy_client >/dev/null 2>&1 & echo \$!" +client_pid="$out" -echo "Checking for PID $pid on node $test_node" -# set -e is good, but avoid it here +cleanup () +{ + if [ -n "$client_pid" ] ; then + onnode $test_node kill -9 "$client_pid" + fi +} + +ctdb_test_exit_hook_add cleanup + +echo "Checking for PID $client_pid on node $test_node" status=0 -try_command_on_node $test_node "$CTDB process-exists ${pid}" || status=$? +try_command_on_node $test_node \ + "$CTDB process-exists ${client_pid}" || status=$? echo "$out" if [ $status -eq 0 ] ; then @@ -56,10 +65,9 @@ else testfailures=1 fi -# Now just echo the PID of the shell from the onnode process on node -# 2. This PID will disappear and PIDs shouldn't roll around fast -# enough to trick the test... but there is a chance that will happen! -try_command_on_node $test_node 'echo $$' +# Now just echo the PID of the ctdb daemon on test node. +# This is not a ctdb client and process-exists should return error. +try_command_on_node $test_node "ctdb getpid" pid="$out" echo "Checking for PID $pid on node $test_node" diff --git a/ctdb/tests/tool/ctdb.process-exists.001.sh b/ctdb/tests/tool/ctdb.process-exists.001.sh index b153da1e1fe..2339344fec5 100755 --- a/ctdb/tests/tool/ctdb.process-exists.001.sh +++ b/ctdb/tests/tool/ctdb.process-exists.001.sh @@ -11,12 +11,14 @@ NODEMAP 2 192.168.20.43 0x0 EOF -pid=$(ctdbd_getpid) +dummy_client -s $ctdbd_socket & +pid=$! ok "PID $pid exists" simple_test "$pid" -# Use a PID that is probably impossible. It must fit into 32 bits but -# should be larger than most settings for pid_max. -required_result 1 "PID 99999999 does not exist" -simple_test "99999999" +kill -9 $pid + +pid=$(ctdbd_getpid) +required_result 1 "PID $pid does not exist" +simple_test "$pid" -- 2.13.5 From 06dfcc9d8b33e46f9b19bfac9f6a0f415b48600e Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Tue, 12 Sep 2017 11:51:19 +1000 Subject: [PATCH 5/6] ctdb-tests: Wait up to 30 seconds for process to be registered in ctdbd BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012 This avoids a potential race where the client is not properly registered before "ctdb process-exists" is called. Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit ff75f0836aef56476ec45a3bc8f3ca22c118e3a4) --- ctdb/tests/simple/07_ctdb_process_exists.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ctdb/tests/simple/07_ctdb_process_exists.sh b/ctdb/tests/simple/07_ctdb_process_exists.sh index c44924b0fe6..f24e93a47ad 100755 --- a/ctdb/tests/simple/07_ctdb_process_exists.sh +++ b/ctdb/tests/simple/07_ctdb_process_exists.sh @@ -52,9 +52,9 @@ cleanup () ctdb_test_exit_hook_add cleanup -echo "Checking for PID $client_pid on node $test_node" +echo "Waiting until PID $client_pid is registered on node $test_node" status=0 -try_command_on_node $test_node \ +wait_until 30 try_command_on_node $test_node \ "$CTDB process-exists ${client_pid}" || status=$? echo "$out" -- 2.13.5 From f6f07f0026adbf859439c603645482e423906ffc Mon Sep 17 00:00:00 2001 From: Amitay Isaacs Date: Tue, 5 Sep 2017 13:52:47 +1000 Subject: [PATCH 6/6] ctdb-tests: Fix ctdb test binary name in path testing BUG: https://bugzilla.samba.org/show_bug.cgi?id=13012 Signed-off-by: Amitay Isaacs Reviewed-by: Martin Schwenke (cherry picked from commit 96aef2371c6c1e0c6bd13874a71583eb9609959b) --- ctdb/tests/scripts/test_wrap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ctdb/tests/scripts/test_wrap b/ctdb/tests/scripts/test_wrap index 176310e9a15..3db3180ae98 100755 --- a/ctdb/tests/scripts/test_wrap +++ b/ctdb/tests/scripts/test_wrap @@ -10,7 +10,7 @@ TEST_SCRIPTS_DIR=$(dirname $0) # We need the test binaries (i.e. tests/bin/) to be in $PATH. If they # aren't already in $PATH then we know that tests/bin/ sits alongside # tests/scripts/. -f="ctdb_bench" +f="fetch_ring" if [ ! $(which $f >/dev/null 2>&1) ] ; then d=$(dirname "$TEST_SCRIPTS_DIR")/bin [ -x "$d/$f" ] && PATH="$d:$PATH" -- 2.13.5