* CLI reconnects to backend if backend restarts with a warning

* Note that edits to the candidate database or locks will be lost
  * To force the CLI to exit if backend restarts, undef `PROTO_RESTART_RECONNECT`
  * This is an effect of the fix of [Broken pipe error seen in client (cli) when backend restarts and CLICON_SOCK is recreated](https://github.com/clicon/clixon/issues/312), the CLI behavior on backend restart is changed.
* Added `eof` parameter to `clicon_rpc()` and `clicon_rpc1()` and error handling modified
This commit is contained in:
Olof hagsand 2022-03-16 18:30:21 +01:00
parent 03f667e2ea
commit dfeb7cef75
10 changed files with 176 additions and 56 deletions

View file

@ -37,11 +37,27 @@
## 5.7.0 ## 5.7.0
Expected: May 2022 Expected: May 2022
### API changes on existing protocol/config features
Users may have to change how they access the system
* CLI reconnects to backend if backend restarts with a warning
* Note that edits to the candidate database or locks will be lost
* To force the CLI to exit if backend restarts, undef `PROTO_RESTART_RECONNECT`
* This is an effect of the fix of [Broken pipe error seen in client (cli) when backend restarts and CLICON_SOCK is recreated](https://github.com/clicon/clixon/issues/312), the CLI behavior on backend restart is changed.
### Corrected Bugs ### Corrected Bugs
* Fixed: [Broken pipe error seen in client (cli) when backend restarts and CLICON_SOCK is recreated](https://github.com/clicon/clixon/issues/312)
* Fixed: [Xpath API do not support filter data by wildcard](https://github.com/clicon/clixon/issues/313) * Fixed: [Xpath API do not support filter data by wildcard](https://github.com/clicon/clixon/issues/313)
* Fixed: SEGV in cli show yang * Fixed: SEGV in cli show yang
### C/CLI-API changes on existing features
Developers may need to change their code
* Added `eof` parameter to `clicon_rpc()` and `clicon_rpc1()` and error handling modified
## 5.6.0 ## 5.6.0
8 March 2022 8 March 2022

View file

@ -204,11 +204,23 @@ cli_sig_term(int arg)
/*! Setup signal handlers /*! Setup signal handlers
*/ */
static void static int
cli_signal_init (clicon_handle h) cli_signal_init (clicon_handle h)
{ {
int retval = -1;
cli_signal_block(h); cli_signal_block(h);
set_signal(SIGTERM, cli_sig_term, NULL); if (set_signal(SIGTERM, cli_sig_term, NULL) < 0){
clicon_err(OE_UNIX, errno, "Setting SIGTERM signal");
goto done;
}
if (set_signal(SIGPIPE, SIG_IGN, NULL) < 0){
clicon_err(OE_UNIX, errno, "Setting DIGPIPE signal");
goto done;
}
retval = 0;
done:
return retval;
} }
/*! Interactive CLI command loop /*! Interactive CLI command loop
@ -653,7 +665,8 @@ main(int argc,
clicon_log_string_limit_set(nr); clicon_log_string_limit_set(nr);
/* Setup signal handlers */ /* Setup signal handlers */
cli_signal_init(h); if (cli_signal_init(h) < 0)
goto done;
/* Backward compatible mode, do not include keys in cgv-arrays in callbacks. /* Backward compatible mode, do not include keys in cgv-arrays in callbacks.
Should be 0 but default is 1 since all legacy apps use 1 Should be 0 but default is 1 since all legacy apps use 1

View file

@ -606,8 +606,11 @@ clicon_parse(clicon_handle h,
cli_output_reset(); cli_output_reset();
if (!cligen_exiting(ch)) { if (!cligen_exiting(ch)) {
clicon_err_reset(); clicon_err_reset();
if ((ret = cligen_eval(ch, match_obj, cvv, callbacks)) < 0) if ((ret = cligen_eval(ch, match_obj, cvv, callbacks)) < 0) {
cli_handler_err(stdout); cli_handler_err(stdout);
if (clicon_suberrno == ESHUTDOWN)
goto done;
}
} }
else else
ret = 0; ret = 0;

View file

@ -602,6 +602,23 @@ netconf_terminate(clicon_handle h)
return 0; return 0;
} }
/*! Setup signal handlers
*/
static int
netconf_signal_init (clicon_handle h)
{
int retval = -1;
if (set_signal(SIGPIPE, SIG_IGN, NULL) < 0){
clicon_err(OE_UNIX, errno, "Setting DIGPIPE signal");
goto done;
}
retval = 0;
done:
return retval;
}
static int static int
timeout_fn(int s, timeout_fn(int s,
void *arg) void *arg)
@ -804,6 +821,10 @@ main(int argc,
if (netconf_module_features(h) < 0) if (netconf_module_features(h) < 0)
goto done; goto done;
/* Setup signal handlers, int particular PIPE that occurs if backend closes / restarts */
if (netconf_signal_init(h) < 0)
goto done;
/* Initialize plugin module by creating a handle holding plugin and callback lists */ /* Initialize plugin module by creating a handle holding plugin and callback lists */
if (clixon_plugin_module_init(h) < 0) if (clixon_plugin_module_init(h) < 0)
goto done; goto done;

View file

@ -149,3 +149,10 @@
* See test/fuzz/http1 * See test/fuzz/http1
*/ */
#undef RESTCONF_HTTP1_UNITTEST #undef RESTCONF_HTTP1_UNITTEST
/*! If backend is restarted, cli and netconf client will retry (once) and reconnect
* Note, if client has locked or had edits in progress, these will be lost
* A warning will be printed
* If not set, client will exit
*/
#define PROTO_RESTART_RECONNECT

View file

@ -79,9 +79,9 @@ int clicon_rpc_connect_inet(clicon_handle h,
uint16_t port, uint16_t port,
int *sock0); int *sock0);
int clicon_rpc(int sock, struct clicon_msg *msg, char **xret); int clicon_rpc(int sock, struct clicon_msg *msg, char **xret, int *eof);
int clicon_rpc1(int sock, cbuf *msgin, cbuf *msgret); int clicon_rpc1(int sock, cbuf *msgin, cbuf *msgret, int *eof);
int clicon_msg_send(int s, struct clicon_msg *msg); int clicon_msg_send(int s, struct clicon_msg *msg);

View file

@ -156,6 +156,7 @@ clixon_client_lock(int sock,
cxobj *xd; cxobj *xd;
cbuf *msg = NULL; cbuf *msg = NULL;
cbuf *msgret = NULL; cbuf *msgret = NULL;
int eof = 0;
clicon_debug(1, "%s", __FUNCTION__); clicon_debug(1, "%s", __FUNCTION__);
if (db == NULL){ if (db == NULL){
@ -175,8 +176,13 @@ clixon_client_lock(int sock,
NETCONF_BASE_NAMESPACE, NETCONF_BASE_NAMESPACE,
NETCONF_MESSAGE_ID_ATTR, NETCONF_MESSAGE_ID_ATTR,
lock?"":"un", db, lock?"":"un"); lock?"":"un", db, lock?"":"un");
if (clicon_rpc1(sock, msg, msgret) < 0) if (clicon_rpc1(sock, msg, msgret, &eof) < 0)
goto done; goto done;
if (eof){
close(sock);
clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed.");
goto done;
}
if (clixon_xml_parse_string(cbuf_get(msgret), YB_NONE, NULL, &xret, NULL) < 0) if (clixon_xml_parse_string(cbuf_get(msgret), YB_NONE, NULL, &xret, NULL) < 0)
goto done; goto done;
if ((xd = xpath_first(xret, NULL, "/rpc-reply/rpc-error")) != NULL){ if ((xd = xpath_first(xret, NULL, "/rpc-reply/rpc-error")) != NULL){
@ -410,6 +416,7 @@ clixon_client_get_xdata(int sock,
cbuf *msgret = NULL; cbuf *msgret = NULL;
const char *db = "running"; const char *db = "running";
cvec *nsc = NULL; cvec *nsc = NULL;
int eof = 0;
clicon_debug(1, "%s", __FUNCTION__); clicon_debug(1, "%s", __FUNCTION__);
if ((msg = cbuf_new()) == NULL){ if ((msg = cbuf_new()) == NULL){
@ -437,8 +444,13 @@ clixon_client_get_xdata(int sock,
cprintf(msg, "/>"); cprintf(msg, "/>");
} }
cprintf(msg, "</get-config></rpc>"); cprintf(msg, "</get-config></rpc>");
if (clicon_rpc1(sock, msg, msgret) < 0) if (clicon_rpc1(sock, msg, msgret, &eof) < 0)
goto done; goto done;
if (eof){
close(sock);
clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed.");
goto done;
}
if (clixon_xml_parse_string(cbuf_get(msgret), YB_NONE, NULL, &xret, NULL) < 0) if (clixon_xml_parse_string(cbuf_get(msgret), YB_NONE, NULL, &xret, NULL) < 0)
goto done; goto done;
if ((xd = xpath_first(xret, NULL, "/rpc-reply/rpc-error")) != NULL){ if ((xd = xpath_first(xret, NULL, "/rpc-reply/rpc-error")) != NULL){

View file

@ -605,42 +605,40 @@ clicon_rpc_connect_inet(clicon_handle h,
* *
* TBD: timeout, interrupt? * TBD: timeout, interrupt?
* retval may be -1 and * retval may be -1 and
* errno set to ENOTCONN which means that socket is now closed probably * errno set to ENOTCONN/ESHUTDOWN which means that socket is now closed probably
* due to remote peer disconnecting. The caller may have to do something,... * due to remote peer disconnecting. The caller may have to do something,...
* *
* @param[in] sock Socket / file descriptor * @param[in] sock Socket / file descriptor
* @param[in] msg CLICON msg data structure. It has fixed header and variable body. * @param[in] msg CLICON msg data structure. It has fixed header and variable body.
* @param[out] xret Returned data as netconf xml tree. * @param[out] xret Returned data as netconf xml tree.
* @retval 0 OK * @param[out] eof Set if eof encountered
* @retval 0 OK (check eof)
* @retval -1 Error * @retval -1 Error
* @see clicon_rpc1 using plain NETCONF XML * @see clicon_rpc1 using plain NETCONF XML
*/ */
int int
clicon_rpc(int sock, clicon_rpc(int sock,
struct clicon_msg *msg, struct clicon_msg *msg,
char **ret) char **ret,
int *eof)
{ {
int retval = -1; int retval = -1;
struct clicon_msg *reply = NULL; struct clicon_msg *reply = NULL;
int eof;
char *data = NULL; char *data = NULL;
if (clicon_msg_send(sock, msg) < 0) if (clicon_msg_send(sock, msg) < 0)
goto done; goto done;
if (clicon_msg_rcv(sock, &reply, &eof) < 0) if (clicon_msg_rcv(sock, &reply, eof) < 0)
goto done; goto done;
if (eof){ if (*eof)
clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed."); goto ok;
close(sock); /* assume socket */
errno = ESHUTDOWN;
goto done;
}
data = reply->op_body; /* assume string */ data = reply->op_body; /* assume string */
if (ret && data) if (ret && data)
if ((*ret = strdup(data)) == NULL){ if ((*ret = strdup(data)) == NULL){
clicon_err(OE_UNIX, errno, "strdup"); clicon_err(OE_UNIX, errno, "strdup");
goto done; goto done;
} }
ok:
retval = 0; retval = 0;
done: done:
if (reply) if (reply)
@ -655,6 +653,7 @@ clicon_rpc(int sock,
* @param[in] sock Socket / file descriptor * @param[in] sock Socket / file descriptor
* @param[in] msgin CLICON msg data structure. It has fixed header and variable body. * @param[in] msgin CLICON msg data structure. It has fixed header and variable body.
* @param[out] msgret Returned data as netconf xml tree. * @param[out] msgret Returned data as netconf xml tree.
* @param[out] eof Set if eof encountered
* @retval 0 OK * @retval 0 OK
* @retval -1 Error * @retval -1 Error
* @see clicon_rpc using clicon_msg protocol header * @see clicon_rpc using clicon_msg protocol header
@ -662,22 +661,16 @@ clicon_rpc(int sock,
int int
clicon_rpc1(int sock, clicon_rpc1(int sock,
cbuf *msg, cbuf *msg,
cbuf *msgret) cbuf *msgret,
int *eof)
{ {
int retval = -1; int retval = -1;
int eof;
clicon_debug(1, "%s", __FUNCTION__); clicon_debug(1, "%s", __FUNCTION__);
if (clicon_msg_send1(sock, msg) < 0) if (clicon_msg_send1(sock, msg) < 0)
goto done; goto done;
if (clicon_msg_rcv1(sock, msgret, &eof) < 0) if (clicon_msg_rcv1(sock, msgret, eof) < 0)
goto done; goto done;
if (eof){
clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed.");
close(sock);
errno = ESHUTDOWN;
goto done;
}
retval = 0; retval = 0;
done: done:
clicon_debug(1, "%s retval:%d", __FUNCTION__, retval); clicon_debug(1, "%s retval:%d", __FUNCTION__, retval);

View file

@ -127,6 +127,41 @@ clicon_rpc_connect(clicon_handle h,
return retval; return retval;
} }
/*! Connect to backend or use cached socket and send RPC
* @param[in] h Clixonhandle
* @param[in] msg Encoded message
* @param[out] xret Returned data as netconf xml tree.
* @param[out] eof Set if eof encountered
*/
static int
clicon_rpc_msg_once(clicon_handle h,
struct clicon_msg *msg,
char **retdata,
int *eof,
int *sp)
{
int retval = -1;
int s;
if ((s = clicon_client_socket_get(h)) < 0){
if (clicon_rpc_connect(h, &s) < 0)
goto done;
clicon_client_socket_set(h, s);
}
if (clicon_rpc(s, msg, retdata, eof) < 0){
/* 2. check socket shutdown AFTER rpc */
close(s);
s = -1;
clicon_client_socket_set(h, -1);
goto done;
}
if (sp)
*sp = s;
retval = 0;
done:
return retval;
}
/*! Send internal netconf rpc from client to backend /*! Send internal netconf rpc from client to backend
* @param[in] h CLICON handle * @param[in] h CLICON handle
* @param[in] msg Encoded message. Deallocate with free * @param[in] msg Encoded message. Deallocate with free
@ -145,20 +180,37 @@ clicon_rpc_msg(clicon_handle h,
char *retdata = NULL; char *retdata = NULL;
cxobj *xret = NULL; cxobj *xret = NULL;
int s = -1; int s = -1;
int eof = 0;
#ifdef RPC_USERNAME_ASSERT #ifdef RPC_USERNAME_ASSERT
assert(strstr(msg->op_body, "username")!=NULL); /* XXX */ assert(strstr(msg->op_body, "username")!=NULL); /* XXX */
#endif #endif
clicon_debug(1, "%s request:%s", __FUNCTION__, msg->op_body); clicon_debug(1, "%s request:%s", __FUNCTION__, msg->op_body);
/* Create a socket and connect to it, either UNIX, IPv4 or IPv6 per config options */ /* Create a socket and connect to it, either UNIX, IPv4 or IPv6 per config options */
if ((s = clicon_client_socket_get(h)) < 0){ if (clicon_rpc_msg_once(h, msg, &retdata, &eof, NULL) < 0)
if (clicon_rpc_connect(h, &s) < 0) goto done;
if (eof){
/* 2. check socket shutdown AFTER rpc */
close(s);
s = -1;
clicon_client_socket_set(h, -1);
#ifdef PROTO_RESTART_RECONNECT
if (clicon_rpc_msg_once(h, msg, &retdata, &eof, NULL) < 0)
goto done;
if (eof){
close(s);
s = -1;
clicon_client_socket_set(h, -1);
clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed.");
goto done; goto done;
clicon_client_socket_set(h, s);
} }
if (clicon_rpc(s, msg, &retdata) < 0) /* To disable this restart, unset PROTO_RESTART_RECONNECT */
clicon_log(LOG_WARNING, "The backend was probably restarted and the client has reconnected to the backend. Any locks or candidate edits are lost.");
#else
clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed.");
goto done; goto done;
#endif
}
clicon_debug(1, "%s retdata:%s", __FUNCTION__, retdata); clicon_debug(1, "%s retdata:%s", __FUNCTION__, retdata);
if (retdata){ if (retdata){
@ -174,10 +226,6 @@ clicon_rpc_msg(clicon_handle h,
} }
retval = 0; retval = 0;
done: done:
if (retval < 0 && s >= 0){
close(s);
clicon_client_socket_set(h, -1);
}
if (retdata) if (retdata)
free(retdata); free(retdata);
if (xret) if (xret)
@ -203,6 +251,7 @@ clicon_rpc_msg_persistent(clicon_handle h,
char *retdata = NULL; char *retdata = NULL;
cxobj *xret = NULL; cxobj *xret = NULL;
int s = -1; int s = -1;
int eof = 0;
if (sock0 == NULL){ if (sock0 == NULL){
clicon_err(OE_NETCONF, EINVAL, "Missing socket pointer"); clicon_err(OE_NETCONF, EINVAL, "Missing socket pointer");
@ -213,11 +262,16 @@ clicon_rpc_msg_persistent(clicon_handle h,
#endif #endif
clicon_debug(1, "%s request:%s", __FUNCTION__, msg->op_body); clicon_debug(1, "%s request:%s", __FUNCTION__, msg->op_body);
/* Create a socket and connect to it, either UNIX, IPv4 or IPv6 per config options */ /* Create a socket and connect to it, either UNIX, IPv4 or IPv6 per config options */
if (clicon_rpc_connect(h, &s) < 0) if (clicon_rpc_msg_once(h, msg, &retdata, &eof, &s) < 0)
goto done; goto done;
if (clicon_rpc(s, msg, &retdata) < 0) if (eof){
/* 2. check socket shutdown AFTER rpc */
close(s);
s = -1;
clicon_client_socket_set(h, -1);
clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed.");
goto done; goto done;
}
clicon_debug(1, "%s retdata:%s", __FUNCTION__, retdata); clicon_debug(1, "%s retdata:%s", __FUNCTION__, retdata);
if (retdata){ if (retdata){

View file

@ -100,6 +100,7 @@ main(int argc,
clicon_handle h; clicon_handle h;
int dbg = 0; int dbg = 0;
int s; int s;
int eof = 0;
/* In the startup, logs to stderr & debug flag set later */ /* In the startup, logs to stderr & debug flag set later */
clicon_log_init(__FILE__, LOG_INFO, CLICON_LOG_STDERR); clicon_log_init(__FILE__, LOG_INFO, CLICON_LOG_STDERR);
@ -177,7 +178,7 @@ main(int argc,
else else
if (clicon_rpc_connect_inet(h, sockpath, 4535, &s) < 0) if (clicon_rpc_connect_inet(h, sockpath, 4535, &s) < 0)
goto done; goto done;
if (clicon_rpc(s, msg, &retdata) < 0) if (clicon_rpc(s, msg, &retdata, &eof) < 0)
goto done; goto done;
close(s); close(s);
fprintf(stdout, "%s\n", retdata); fprintf(stdout, "%s\n", retdata);