diff --git a/CHANGELOG.md b/CHANGELOG.md index 22d0f6ce..7c1c5a1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,16 @@ ## 5.1.0 Expected: April +### Minor features + +* Introduced a delay before making process start/stop/restart processes for race conditions when configuring eg restconf +* For restconf `CLICON_BACKEND_RESTCONF_PROCESS`, restart restconf if restconf is edited. + +### Corrected Bugs + +* Reverted blocked signal behavior introduced in 5.0. + + ## 5.0.0 27 February 2021 diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index 315a28e5..c12d41f2 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -1580,16 +1580,18 @@ from_client_process_control(clicon_handle h, int retval = -1; cxobj *x; char *name = NULL; - char *operation = NULL; + char *opstr = NULL; uint32_t pid = 0; + proc_operation op = PROC_OP_NONE; - clicon_debug(1, "%s", __FUNCTION__); if ((x = xml_find_type(xe, NULL, "name", CX_ELMNT)) != NULL) name = xml_body(x); - if ((x = xml_find_type(xe, NULL, "operation", CX_ELMNT)) != NULL) - operation = xml_body(x); + if ((x = xml_find_type(xe, NULL, "operation", CX_ELMNT)) != NULL){ + opstr = xml_body(x); + op = clixon_process_op_str2int(opstr); + } /* Make the actual process operation (with wrap function enabled) */ - if (clixon_process_operation(h, name, operation, 1, &pid) < 0) + if (clixon_process_operation(h, name, op, 1, &pid) < 0) goto done; cprintf(cbret, "%u", NETCONF_BASE_NAMESPACE, CLIXON_LIB_NS, pid); diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index e19595fd..571f71f2 100644 --- a/apps/backend/backend_main.c +++ b/apps/backend/backend_main.c @@ -172,6 +172,7 @@ backend_sig_child(int arg) clicon_debug(1, "%s", __FUNCTION__); if ((pid = waitpid(-1, &status, 0)) != -1 && WIFEXITED(status)){ } + clicon_sig_ignore_set(1); } /*! Create backend server socket and register callback @@ -936,8 +937,8 @@ main(int argc, */ if (status == STARTUP_OK){ if (startup_mode == SM_NONE){ - /* Special case for mode none: no commits are made therefore need to run - * start all processes + /* Special case for mode none: no commits are made therefore need to go through all processes + * registered in plugins (specifically the restconf pseudo plugin) */ if (clixon_process_start_all(h) < 0) goto done; diff --git a/apps/backend/backend_plugin_restconf.c b/apps/backend/backend_plugin_restconf.c index b469e273..17af6d61 100644 --- a/apps/backend/backend_plugin_restconf.c +++ b/apps/backend/backend_plugin_restconf.c @@ -69,24 +69,28 @@ int restconf_rpc_wrapper(clicon_handle h, process_entry_t *pe, - char **operation) + proc_operation *operation) { int retval = -1; cxobj *xt = NULL; clicon_debug(1, "%s", __FUNCTION__); - if (strcmp(*operation, "stop") == 0){ + switch (*operation){ + case PROC_OP_STOP: /* if RPC op is stop, stop the service */ - } - else if (strcmp(*operation, "start") == 0){ + break; + case PROC_OP_START: /* RPC op is start & enable is true, then start the service, & enable is false, error or ignore it */ if (xmldb_get(h, "running", NULL, "/restconf", &xt) < 0) goto done; if (xt != NULL && xpath_first(xt, NULL, "/restconf[enable='false']") != NULL) { - *operation = "none"; + *operation = PROC_OP_NONE; } + break; + default: + break; } retval = 0; done: @@ -188,12 +192,25 @@ restconf_pseudo_process_commit(clicon_handle h, xtarget = transaction_target(td); if (xpath_first(xtarget, NULL, "/restconf[enable='true']") != NULL) enabled++; + /* Toggle start/stop */ if ((cx = xpath_first(xtarget, NULL, "/restconf/enable")) != NULL && xml_flag(cx, XML_FLAG_CHANGE|XML_FLAG_ADD)){ if (clixon_process_operation(h, RESTCONF_PROCESS, - enabled?"start":"stop", 0, NULL) < 0) + enabled?PROC_OP_START:PROC_OP_STOP, 0, NULL) < 0) goto done; } + else if (enabled){ /* If something changed and running, restart process */ + if (transaction_dlen(xtarget) != 0 || + transaction_alen(xtarget) != 0 || + transaction_clen(xtarget) != 0){ + /* A restart can terminate a restconf connection (cut the tree limb you are sitting on) + * Specifically, the socket is terminated where the reply is sent, which will + * cause the curl to fail. + */ + if (clixon_process_operation(h, RESTCONF_PROCESS, PROC_OP_RESTART, 0, NULL) < 0) + goto done; + } + } retval = 0; done: return retval; @@ -211,8 +228,9 @@ backend_plugin_restconf_register(clicon_handle h, if (clixon_pseudo_plugin(h, "restconf pseudo plugin", &cp) < 0) goto done; - cp->cp_api.ca_trans_commit = restconf_pseudo_process_commit; + cp->cp_api.ca_trans_validate = restconf_pseudo_process_validate; + cp->cp_api.ca_trans_commit = restconf_pseudo_process_commit; /* Register generic process-control of restconf daemon, ie start/stop restconf */ if (restconf_pseudo_process_control(h) < 0) diff --git a/apps/restconf/restconf_main_evhtp.c b/apps/restconf/restconf_main_evhtp.c index eb16e27d..474f363b 100644 --- a/apps/restconf/restconf_main_evhtp.c +++ b/apps/restconf/restconf_main_evhtp.c @@ -169,6 +169,7 @@ restconf_sig_child(int arg) if ((pid = waitpid(-1, &status, 0)) != -1 && WIFEXITED(status)){ } + clicon_sig_ignore_set(1); } static char* diff --git a/lib/clixon/clixon_event.h b/lib/clixon/clixon_event.h index 82712b82..4a741126 100644 --- a/lib/clixon/clixon_event.h +++ b/lib/clixon/clixon_event.h @@ -49,6 +49,10 @@ int clicon_exit_reset(void); int clicon_exit_get(void); +int clicon_sig_ignore_set(int val); + +int clicon_sig_ignore_get(void); + int clixon_event_reg_fd(int fd, int (*fn)(int, void*), void *arg, char *str); int clixon_event_unreg_fd(int s, int (*fn)(int, void*)); diff --git a/lib/clixon/clixon_plugin.h b/lib/clixon/clixon_plugin.h index 01fa28e4..8c932da3 100644 --- a/lib/clixon/clixon_plugin.h +++ b/lib/clixon/clixon_plugin.h @@ -114,17 +114,20 @@ typedef enum clixon_auth_type clixon_auth_type_t; /* Called when application is "started", (almost) all initialization is complete * Backend: daemon is in the background. If daemon privileges are dropped * this callback is called *before* privileges are dropped. + * @param[in] h Clixon handle */ typedef int (plgstart_t)(clicon_handle); /* Plugin start */ /* Called just before or after a server has "daemonized", ie put in background. * Backend: If daemon privileges are dropped this callback is called *before* privileges are dropped. - * If daemon is started in foreground (-F): pre-daemon is not called, but post-daemon called + * If daemon is started in foreground (-F): pre-daemon is not called, but daemon called + * @param[in] h Clixon handle */ typedef int (plgdaemon_t)(clicon_handle); /* Plugin pre/post daemonized */ /* Called just before plugin unloaded. + * @param[in] h Clixon handle */ typedef int (plgexit_t)(clicon_handle); /* Plugin exit */ @@ -263,8 +266,8 @@ struct clixon_plugin_api{ struct { /* netconf-specific */ } cau_netconf; struct { /* backend-specific */ - plgdaemon_t *cb_pre_daemon; /* Plugin just before daemonization */ - plgdaemon_t *cb_daemon; /* Plugin daemonized */ + plgdaemon_t *cb_pre_daemon; /* Plugin just before daemonization (only daemon) */ + plgdaemon_t *cb_daemon; /* Plugin daemonized (always called) */ plgreset_t *cb_reset; /* Reset system status */ plgstatedata_t *cb_statedata; /* Get state data from plugin (backend only) */ trans_cb_t *cb_trans_begin; /* Transaction start */ diff --git a/lib/clixon/clixon_proc.h b/lib/clixon/clixon_proc.h index 3e6b536f..5eb7edd4 100644 --- a/lib/clixon/clixon_proc.h +++ b/lib/clixon/clixon_proc.h @@ -43,8 +43,17 @@ */ typedef struct process_entry_t process_entry_t; +/* Process operations */ +typedef enum proc_operation { + PROC_OP_NONE = 0, + PROC_OP_START, + PROC_OP_STOP, + PROC_OP_RESTART, + PROC_OP_STATUS +} proc_operation; + /* Process RPC callback function */ -typedef int (proc_cb_t)(clicon_handle h, process_entry_t *pe, char **operation); +typedef int (proc_cb_t)(clicon_handle h, process_entry_t *pe, proc_operation *operation); /* * Prototypes @@ -52,9 +61,11 @@ typedef int (proc_cb_t)(clicon_handle h, process_entry_t *pe, char **operation); int clixon_proc_socket(char **argv, pid_t *pid, int *sock); int clixon_proc_socket_close(pid_t pid, int sock); int clixon_proc_background(char **argv, const char *netns, pid_t *pid); +proc_operation clixon_process_op_str2int(char *opstr); int clixon_process_register(clicon_handle h, const char *name, const char *netns, proc_cb_t *callback, char **argv, int argc); int clixon_process_delete_all(clicon_handle h); -int clixon_process_operation(clicon_handle h, const char *name, char *op, const int wrapit, uint32_t *pid); +int clixon_process_operation(clicon_handle h, const char *name, proc_operation op, const int wrapit, uint32_t *pid); int clixon_process_start_all(clicon_handle h); +int clixon_process_sched_register(clicon_handle h); #endif /* _CLIXON_PROC_H_ */ diff --git a/lib/src/clixon_event.c b/lib/src/clixon_event.c index 1e573e62..c1f01741 100644 --- a/lib/src/clixon_event.c +++ b/lib/src/clixon_event.c @@ -86,8 +86,12 @@ static struct event_data *ee_timers = NULL; /* Set if element in ee is deleted (clixon_event_unreg_fd). Check in ee loops */ static int _ee_unreg = 0; +/* If set (eg by signal handler) exit select loop on next run and return 0 */ static int _clicon_exit = 0; +/* If set (eg by signal handler) ignore EINTR and continue select loop */ +static int _clicon_sig_ignore = 0; + /*! For signal handlers: instead of doing exit, set a global variable to exit * Status is then checked in event_loop. * Note it maybe would be better to do use on a handle basis, but a signal @@ -117,6 +121,19 @@ clicon_exit_get(void) return _clicon_exit; } +int +clicon_sig_ignore_set(int val) +{ + _clicon_sig_ignore = val; + return 0; +} + +int +clicon_sig_ignore_get(void) +{ + return _clicon_sig_ignore; +} + /*! Register a callback function to be called on input on a file descriptor. * * @param[in] fd File descriptor @@ -320,18 +337,26 @@ clixon_event_loop(void) if (clicon_exit_get()) break; if (n == -1) { - /* Signals either set clixon_exit, then the function returns 0 - * Typically for set_signal() of SIGTERM,SIGINT, etc - * Other signals are ignored, and the select is rerun, eg SIGCHLD - */ if (errno == EINTR){ + /* Signals are checked and are in three classes: + * (1) Signals that exit gracefully, the function returns 0 + * Must be registered such as by set_signal() of SIGTERM,SIGINT, etc with a handler that calls + * clicon_exit_set(). + * (2) Signals that are ignored, and the select is rerun, eg SIGCHLD if handler calls clicon_sig_ignore() + * New select loop is called + * (3) Other signals result in an error and return -1. + */ clicon_debug(1, "%s select: %s", __FUNCTION__, strerror(errno)); if (clicon_exit_get()){ clicon_err(OE_EVENTS, errno, "select"); retval = 0; } - else + else if (clicon_sig_ignore_get()){ + clicon_sig_ignore_set(0); continue; + } + else + clicon_err(OE_EVENTS, errno, "select"); } else clicon_err(OE_EVENTS, errno, "select"); diff --git a/lib/src/clixon_plugin.c b/lib/src/clixon_plugin.c index 13f808ed..f3491145 100644 --- a/lib/src/clixon_plugin.c +++ b/lib/src/clixon_plugin.c @@ -334,7 +334,7 @@ clixon_pseudo_plugin(clicon_handle h, int retval = -1; clixon_plugin *cp = NULL; - clicon_debug(1, "%s", __FUNCTION__); + clicon_debug(1, "%s %s", __FUNCTION__, name); /* Create a pseudo plugins */ /* Note: sizeof clixon_plugin_api which is largest of clixon_plugin_api:s */ diff --git a/lib/src/clixon_proc.c b/lib/src/clixon_proc.c index 663a697a..e173336f 100644 --- a/lib/src/clixon_proc.c +++ b/lib/src/clixon_proc.c @@ -40,7 +40,6 @@ #include "clixon_config.h" #endif - #ifdef HAVE_SETNS /* linux network namespaces */ #define _GNU_SOURCE #endif @@ -51,6 +50,7 @@ #include #include #include +#include #include #include #ifdef HAVE_SETNS /* linux network namespaces */ @@ -69,6 +69,7 @@ #include "clixon_err.h" #include "clixon_log.h" #include "clixon_queue.h" +#include "clixon_event.h" #include "clixon_hash.h" #include "clixon_handle.h" #include "clixon_sig.h" @@ -79,6 +80,7 @@ /* * Types */ + /* Process entry list */ struct process_entry_t { qelem_t pe_qelem; /* List header */ @@ -86,23 +88,14 @@ struct process_entry_t { char *pe_netns; /* Network namespace */ char **pe_argv; /* argv with command as element 0 and NULL-terminated */ pid_t pe_pid; /* Running process id (state) or 0 if dead */ + proc_operation pe_op; /* Operation pending? */ proc_cb_t *pe_callback; /* Wrapper function, may be called from process_operation */ }; -/* - * Child process ID - * XXX Really shouldn't be a global variable - */ -static int _clicon_proc_child = 0; - -/* - * Make sure child is killed by ctrl-C - */ static void clixon_proc_sigint(int sig) { - if (_clicon_proc_child > 0) - kill(_clicon_proc_child, SIGINT); + /* XXX does nothing */ } /*! Fork a child, exec a child and setup socket to child and return to caller @@ -200,7 +193,7 @@ clixon_proc_socket_close(pid_t pid, * * @param[in] argv NULL-terminated Argument vector * @param[in] netns Network namespace (or NULL) - * @param[out] pid + * @param[out] pid Process id * @retval 0 OK * @retval -1 Error. */ @@ -290,12 +283,25 @@ clixon_proc_background(char **argv, * Process management: start/stop registered processes for internal use */ -/* - * Types +/* Process operations */ +static const map_str2int proc_operation_map[] = { + {"none", PROC_OP_NONE}, + {"start", PROC_OP_START}, + {"stop", PROC_OP_STOP}, + {"restart", PROC_OP_RESTART}, + {"status", PROC_OP_STATUS}, + {NULL, -1} +}; -/* List of process callback entries */ -static process_entry_t *proc_entry_list = NULL; +/* List of process callback entries XXX move to handle */ +static process_entry_t *_proc_entry_list = NULL; + +proc_operation +clixon_process_op_str2int(char *opstr) +{ + return clicon_str2int(proc_operation_map, opstr); +} /*! Register an internal process * @@ -353,7 +359,7 @@ clixon_process_register(clicon_handle h, } } pe->pe_callback = callback; - ADDQ(pe, proc_entry_list); + ADDQ(pe, _proc_entry_list); retval = 0; done: return retval; @@ -367,8 +373,8 @@ clixon_process_delete_all(clicon_handle h) process_entry_t *pe; char **pa; - while((pe = proc_entry_list) != NULL) { - DELQ(pe, proc_entry_list, process_entry_t *); + while((pe = _proc_entry_list) != NULL) { + DELQ(pe, _proc_entry_list, process_entry_t *); if (pe->pe_name) free(pe->pe_name); if (pe->pe_netns) @@ -416,12 +422,16 @@ proc_op_run(pid_t pid0, /*! Perform process operation * + * @param[in] op One of: start, stop, restart, status + * @param[in] netns Network namespace + * @param[in] argv NULL-terminated Argument vector + * @param[out] pidp Process-id */ static int -clixon_process_operation_one(const char *op, - const char *netns, - char **argv, - pid_t *pidp) +clixon_process_operation_one(const proc_operation op, + const char *netns, + char **argv, + pid_t *pidp) { int retval = -1; int run = 0; @@ -429,37 +439,33 @@ clixon_process_operation_one(const char *op, /* Check if running */ if (proc_op_run(*pidp, &run) < 0) goto done; - if (strcmp(op, "stop") == 0 || - strcmp(op, "restart") == 0){ - if (run) + if (op == PROC_OP_STOP || op == PROC_OP_RESTART){ + if (run){ pidfile_zapold(*pidp); /* Ensures its dead */ + } *pidp = 0; /* mark as dead */ run = 0; } - if (strcmp(op, "start") == 0 || - strcmp(op, "restart") == 0){ + if (op == PROC_OP_START || op == PROC_OP_RESTART){ if (run == 1){ ; /* Already runs */ } else{ if (clixon_proc_background(argv, netns, pidp) < 0) goto done; + clicon_debug(1, "%s started pid:%d", __FUNCTION__, *pidp); } } - else if (strcmp(op, "status") == 0){ - ; /* status already set */ - } - retval = 0; done: - return retval; + return retval; } /*! Find process operation entry given name and op and perform operation if found * * @param[in] h clicon handle * @param[in] name Name of process - * @param[in] op start, stop. + * @param[in] op start, stop, restart, status * @param[in] wrapit If set, call potential callback, if false, dont call it * @param[out] status true if process is running / false if not running on entry * @retval -1 Error @@ -467,41 +473,46 @@ clixon_process_operation_one(const char *op, * @see upgrade_callback_reg_fn which registers the callbacks */ int -clixon_process_operation(clicon_handle h, - const char *name, - char *op, - int wrapit, - uint32_t *pid) +clixon_process_operation(clicon_handle h, + const char *name, + proc_operation op, + int wrapit, + uint32_t *pid) { int retval = -1; process_entry_t *pe; + int sched = 0; /* If set, process action should be scheduled, register a timeout */ - clicon_debug(1, "%s name:%s op:%s", __FUNCTION__, name, op); - if (proc_entry_list == NULL) + clicon_debug(1, "%s name:%s op:%s", __FUNCTION__, name, clicon_int2str(proc_operation_map, op)); + if (_proc_entry_list == NULL) goto ok; - pe = proc_entry_list; + pe = _proc_entry_list; do { if (strcmp(pe->pe_name, name) == 0){ /* Call wrapper function that eg changes op based on config */ if (wrapit && pe->pe_callback != NULL) if (pe->pe_callback(h, pe, &op) < 0) goto done; - if (clixon_process_operation_one(op, pe->pe_netns, pe->pe_argv, &pe->pe_pid) < 0) - goto done; + if (op == PROC_OP_START || op == PROC_OP_STOP || op == PROC_OP_RESTART){ + pe->pe_op = op; + sched++; + } if (pid) *pid = pe->pe_pid; break; /* hit break here */ } pe = NEXTQ(process_entry_t *, pe); - } while (pe != proc_entry_list); + } while (pe != _proc_entry_list); + if (sched && clixon_process_sched_register(h) < 0) + goto done; ok: retval = 0; done: - clicon_debug(1, "%s retval:%d", __FUNCTION__, retval); + clicon_debug(2, "%s retval:%d", __FUNCTION__, retval); return retval; } -/*! Start all processes that are enabled +/*! Go through process list and start all processes that are enabled via config wrap function * @param[in] h Clixon handle * Commit rules should have done this, but there are some cases such as backend -s none mode * where commits are not made. @@ -511,26 +522,87 @@ clixon_process_start_all(clicon_handle h) { int retval = -1; process_entry_t *pe; - char *op; + proc_operation op; + int sched = 0; /* If set, process action should be scheduled, register a timeout */ clicon_debug(1, "%s",__FUNCTION__); - if (proc_entry_list == NULL) + if (_proc_entry_list == NULL) goto ok; - pe = proc_entry_list; + pe = _proc_entry_list; do { - op = "start"; + op = PROC_OP_START; /* Call wrapper function that eg changes op based on config */ if (pe->pe_callback != NULL) if (pe->pe_callback(h, pe, &op) < 0) goto done; - if (strcmp(op, "start") == 0) - if (clixon_process_operation_one("start", pe->pe_netns, pe->pe_argv, &pe->pe_pid) < 0) - goto done; + if (op == PROC_OP_START){ + pe->pe_op = op; + sched++; + } pe = NEXTQ(process_entry_t *, pe); - } while (pe != proc_entry_list); + } while (pe != _proc_entry_list); + if (sched && clixon_process_sched_register(h) < 0) + goto done; ok: retval = 0; done: clicon_debug(1, "%s retval:%d", __FUNCTION__, retval); return retval; } + +/*! Traverse all processes and check pending start/stop/restarts + * @param[in] h Clixon handle + * Typical cases where postponing process start/stop is necessary: + * (1) at startup, if started before deamoninization, process will get as child of 1 + * (2) edit changes or rpc restart especially of restconf where you may saw of your arm and terminate + * return socket. + */ +static int +clixon_process_sched(int fd, + clicon_handle h) +{ + int retval = -1; + process_entry_t *pe; + proc_operation op; + + clicon_debug(2, "%s",__FUNCTION__); + if (_proc_entry_list == NULL) + goto ok; + pe = _proc_entry_list; + do { + if ((op = pe->pe_op) != PROC_OP_NONE){ + if (clixon_process_operation_one(op, pe->pe_netns, pe->pe_argv, &pe->pe_pid) < 0) + goto done; + clicon_debug(1, "%s op:%s pid:%d", __FUNCTION__, clicon_int2str(proc_operation_map, op), pe->pe_pid); + pe->pe_op = PROC_OP_NONE; + } + pe = NEXTQ(process_entry_t *, pe); + } while (pe != _proc_entry_list); + ok: + retval = 0; + done: + clicon_debug(2, "%s retval:%d", __FUNCTION__, retval); + return retval; +} + +/*! Register scheduling of process start/stop/restart + * After a delay t1, schedule the process + * @note The delay is for mitigating a race condition if a process is restarted that is used in the session + * restarting it. In this way, the process "should have" time to exit. + */ +int +clixon_process_sched_register(clicon_handle h) +{ + int retval = -1; + struct timeval t; + struct timeval t1 = {0, 1000}; /* XXX See discussion ^*/ + + gettimeofday(&t, NULL); + timeradd(&t, &t1, &t); + if (clixon_event_reg_timeout(t, clixon_process_sched, h, "process") < 0) + goto done; + retval = 0; + done: + clicon_debug(2, "%s retval:%d", __FUNCTION__, retval); + return retval; +} diff --git a/lib/src/clixon_proto.c b/lib/src/clixon_proto.c index 663bf51f..3c664a18 100644 --- a/lib/src/clixon_proto.c +++ b/lib/src/clixon_proto.c @@ -278,6 +278,7 @@ atomicio(ssize_t (*fn) (int, void *, size_t), continue; else if (errno == ECONNRESET)/* Connection reset by peer */ res = 0; + /* SIGPIPE if client is killed */ case 0: /* fall thru */ return (res); default: diff --git a/test/test_restconf_rpc.sh b/test/test_restconf_rpc.sh index 2925a0f5..c25448b9 100755 --- a/test/test_restconf_rpc.sh +++ b/test/test_restconf_rpc.sh @@ -81,19 +81,21 @@ EOF if [ -z "$pid" ]; then err "Running process" "$ret" fi + new "check restconf retvalue" - if [ $expectret -eq 0 ]; then - if [ $pid -ne 0 ]; then - err "No process" "$pid" - fi - else - if [ $pid -eq 0 ]; then - err "Running process" + if [ $operation = "status" ]; then + if [ $expectret -eq 0 ]; then + if [ $pid -ne 0 ]; then + err "No process" "$pid" + fi + else + if [ $pid -eq 0 ]; then + err "Running process" + fi fi fi - - echo $pid # cant use return that only uses 0-255 + echo "$pid" # cant use return that only uses 0-255 } new "ENABLE true" @@ -124,34 +126,68 @@ if [ $BE -ne 0 ]; then wait_backend fi +# For debug +#>&2 echo "curl $CURLOPTS -X POST -H \"Content-Type: application/yang-data+json\" $RCPROTO://localhost/restconf/operations/clixon-lib:process-control -d '{\"clixon-lib:input\":{\"name\":\"restconf\",\"operation\":\"status\"}}'" + # Get pid of running process and check return xml new "1. Get rpc status" pid0=$(testrpc status 1) # Save pid0 if [ $? -ne 0 ]; then echo "$pid0";exit -1; fi -new "check restconf process running using ps pid0:$pid0" +new "check restconf process runnng using ps pid:$pid0" ps=$(ps -hp $pid0) if [ -z "$ps" ]; then - err "A restconf running" + err "Restconf $pid0 not found" +fi + +new "check parent process of pid:$pid0" +ppid=$(ps -o ppid= -p $pid0) +if [ "$ppid" -eq 1 -o "$ppid" -eq "$pid0" ]; then + err "Restconf parent pid of $pid0 is $ppid is wrong" fi new "wait restconf" wait_restconf -new "try restconf rpc" +new "try restconf rpc status" expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/operations/clixon-lib:process-control -d '{"clixon-lib:input":{"name":"restconf","operation":"status"}}')" 0 "HTTP/1.1 200 OK" '{"clixon-lib:output":{"pid":' +new "1.1. Get status" +pid1=$(testrpc status 1) +if [ $? -ne 0 ]; then echo "$pid1";exit -1; fi + +new "Check same pid" +if [ "$pid0" -ne "$pid1" ]; then + err "$pid0" "$pid1" +fi + +new "try restconf rpc restart" +expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/operations/clixon-lib:process-control -d '{"clixon-lib:input":{"name":"restconf","operation":"restart"}}')" 0 "HTTP/1.1 200 OK" '{"clixon-lib:output":{"pid":' + +new "1.1. Get status" +pid1=$(testrpc status 1) +if [ $? -ne 0 ]; then echo "$pid1";exit -1; fi + +new "check different pids" +if [ "$pid0" -eq "$pid1" ]; then + err "not $pid0" +fi + new "2. stop restconf RPC" -pid=$(testrpc stop 0) -if [ $? -ne 0 ]; then echo "$pid";exit -1; fi +testrpc stop 0 +if [ $? -ne 0 ]; then exit -1; fi new "3. Get rpc status stopped" pid=$(testrpc status 0) if [ $? -ne 0 ]; then echo "$pid";exit -1; fi new "4. Start rpc again" -pid3=$(testrpc start 1) # Save pid3 +testrpc start 0 +if [ $? -ne 0 ]; then exit -1; fi + +new "4.1. Get rpc status" +pid3=$(testrpc status 1) if [ $? -ne 0 ]; then echo "$pid3";exit -1; fi new "check restconf process running using ps" @@ -161,44 +197,32 @@ if [ -z "$ps" ]; then fi if [ $pid0 -eq $pid3 ]; then - err "A different pid" "$pid3" + err "A different pid" "same pid: $pid3" fi new "kill restconf" stop_restconf_pre new "5. start restconf RPC" -pid=$(testrpc start 1) -if [ $? -ne 0 ]; then echo "$pid";exit -1; fi +testrpc start 0 +if [ $? -ne 0 ]; then exit -1; fi new "6. check status RPC on" pid5=$(testrpc status 1) # Save pid5 if [ $? -ne 0 ]; then echo "$pid5";exit -1; fi new "7. restart restconf RPC" -pid=$(testrpc restart 1) -if [ $? -ne 0 ]; then echo "$pid";exit -1; fi +testrpc restart 0 +if [ $? -ne 0 ]; then exit -1; fi new "8. Get restconf status rpc" pid7=$(testrpc status 1) # Save pid7 if [ $? -ne 0 ]; then echo "$pid7";exit -1; fi if [ $pid5 -eq $pid7 ]; then - err "A different pid" "$pid7" + err "A different pid" "samepid: $pid7" fi -#if [ $valgrindtest -eq 0 ]; then # Cant get pgrep to work properly -# new "check new pid" -# sleep $DEMWAIT # Slows the tests down considerably, but needed in eg docker test -# pid1=$(pgrep clixon_restconf) -# if [ -z "$pid0" -o -z "$pid1" ]; then -# err "Pids expected" "pid0:$pid0 = pid1:$pid1" -# fi -# if [ $pid0 -eq $pid1 ]; then# -# err "Different pids" "pid0:$pid0 = pid1:$pid1" -# fi -#fi - if [ $BE -ne 0 ]; then new "Kill backend" # Check if premature kill @@ -214,6 +238,7 @@ fi # Start backend with -s none should start restconf too via ca_reset rule new "Restart backend -s none" + if [ $BE -ne 0 ]; then new "kill old backend" sudo clixon_backend -z -f $cfg @@ -277,8 +302,8 @@ pid=$(testrpc status 0) if [ $? -ne 0 ]; then echo "$pid";exit -1; fi new "11. start restconf RPC" -pid=$(testrpc start 0) -if [ $? -ne 0 ]; then echo "$pid";exit -1; fi +testrpc start 0 +if [ $? -ne 0 ]; then exit -1; fi new "12. check status RPC off" pid=$(testrpc status 0) @@ -294,6 +319,18 @@ new "13. check status RPC on" pid=$(testrpc status 1) if [ $? -ne 0 ]; then echo "$pid";exit -1; fi +# Edit a field, eg debug +new "Edit a field via restconf" +expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/clixon-restconf:restconf/debug -d '{"clixon-restconf:debug":1}' )" 0 "HTTP/1.1 201 Created" + +new "check status RPC new pid" +pid1=$(testrpc status 1) + +if [ $? -ne 0 ]; then echo "$pid1";exit -1; fi +if [ $pid -eq $pid1 ]; then + err "A different pid" "Same pid: $pid" +fi + new "Disable restconf" expecteof "$clixon_netconf -qf $cfg" 0 "mergefalse]]>]]>" "^]]>]]>$" @@ -327,7 +364,7 @@ fi #Start backend -s none should start unset pid -sleep $DEMWAIT # Lots of processes need to die before next test +sleep $DEMSLEEP # Lots of processes need to die before next test new "endtest" endtest diff --git a/yang/clixon/clixon-lib@2020-12-30.yang b/yang/clixon/clixon-lib@2020-12-30.yang index 7a62559e..c3780d1e 100644 --- a/yang/clixon/clixon-lib@2020-12-30.yang +++ b/yang/clixon/clixon-lib@2020-12-30.yang @@ -171,11 +171,8 @@ module clixon-lib { } output { leaf pid { - description "Process-id of running process if operation is start, - restart or status. 0 if not running. - This is for two reasons: - - to check process running for status - - get the actual pid for targeting the actual process"; + description "Process-id of running process or 0 if not running + Value is only valid for operation status"; type uint32; } }