From bc0eb921d0d5fb6e5e4a50367221da1b98e00ca2 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Mon, 4 May 2020 14:54:57 +0200 Subject: [PATCH] Added functionality to restart an individual plugin.: New clixon-lib:restart-plugin RPC --- CHANGELOG.md | 2 + apps/backend/backend_commit.c | 71 +++++-------- example/main/example_backend.c | 11 +- example/main/example_backend_nacm.c | 1 + include/clixon_custom.h | 2 +- test/test_transaction.sh | 10 +- test/test_transaction_restart.sh | 149 ++++++++++++++++++++++++++++ 7 files changed, 190 insertions(+), 56 deletions(-) create mode 100755 test/test_transaction_restart.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 11165081..a24ae9b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,8 @@ Expected: May 2020 ### Minor changes +* Added functionality to restart an individual plugin. + * New clixon-lib:restart-plugin RPC * Added option `CLICON_YANG_UNKNOWN_ANYDATA` to treat unknown XML (wrt YANG) as anydata. * This is to be (very) forgiving but you need to accept eg unsynchronized YANG and XML * Compile-time option: `USE_CLIGEN44` for running clixon-45 with cligen-44. diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index 2e9e6fe2..7f917fad 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -811,6 +811,10 @@ from_client_validate(clicon_handle h, } /* from_client_validate */ #ifdef RESTART_PLUGIN_RPC +/*! Restart specific backend plugins without full backend restart + * Note, depending on plugin callbacks, there may be other dependencies which may make this + * difficult in the general case. + */ int from_client_restart_one(clicon_handle h, clixon_plugin *cp, @@ -820,7 +824,6 @@ from_client_restart_one(clicon_handle h, char *db = "tmp"; transaction_data_t *td = NULL; plgreset_t *resetfn; /* Plugin auth */ - trans_cb_t *fn; int ret; cxobj *xerr = NULL; yang_stmt *yspec; @@ -889,17 +892,10 @@ from_client_restart_one(clicon_handle h, xml_apply_ancestor(xn, (xml_applyfn_t*)xml_flag_set, (void*)XML_FLAG_CHANGE); } - /* 4. Call plugin transaction start callbacks */ - if ((fn = cp->cp_api.ca_trans_begin) != NULL){ - if ((retval = fn(h, (transaction_data)td)) < 0){ - if (!clicon_errno) /* sanity: log if clicon_err() is not called ! */ - clicon_log(LOG_NOTICE, "%s: Plugin '%s' transaction_begin callback does not make clicon_err call on error", - __FUNCTION__, cp->cp_name); - - goto fail; - } - } - /* 5. Make generic validation on all new or changed data. + /* Call plugin transaction start callbacks */ + if (plugin_transaction_begin_one(cp, h, td) < 0) + goto fail; + /* Make generic validation on all new or changed data. Note this is only call that uses 3-values */ if ((ret = generic_validate(h, yspec, td, &xerr)) < 0) goto done; @@ -908,42 +904,25 @@ from_client_restart_one(clicon_handle h, goto done; goto fail; } - if ((fn = cp->cp_api.ca_trans_validate) != NULL){ - if ((retval = fn(h, (transaction_data)td)) < 0){ - if (!clicon_errno) /* sanity: log if clicon_err() is not called ! */ - clicon_log(LOG_NOTICE, "%s: Plugin '%s' transaction_validate callback does not make clicon_err call on error", - __FUNCTION__, cp->cp_name); - goto fail; - } - } - if ((fn = cp->cp_api.ca_trans_complete) != NULL){ - if ((retval = fn(h, (transaction_data)td)) < 0){ - if (!clicon_errno) /* sanity: log if clicon_err() is not called ! */ - clicon_log(LOG_NOTICE, "%s: Plugin '%s' trans_complete callback does not make clicon_err call on error", - __FUNCTION__, cp->cp_name); - - goto fail; - } - } - - if ((fn = cp->cp_api.ca_trans_commit) != NULL){ - if (fn(h, (transaction_data)td) < 0){ - if (!clicon_errno) /* sanity: log if clicon_err() is not called ! */ - clicon_log(LOG_NOTICE, "%s: Plugin '%s' trans_commit callback does not make clicon_err call on error", - __FUNCTION__, cp->cp_name); - goto fail; - } - } - if ((fn = cp->cp_api.ca_trans_end) != NULL){ - if ((retval = fn(h, (transaction_data)td)) < 0){ - if (!clicon_errno) /* sanity: log if clicon_err() is not called ! */ - clicon_log(LOG_NOTICE, "%s: Plugin '%s' trans_end callback does not make clicon_err call on error", - __FUNCTION__, cp->cp_name); - goto fail; - } - } + /* Call validate callback in this plugin */ + if (plugin_transaction_validate_one(cp, h, td) < 0) + goto fail; + if (plugin_transaction_complete_one(cp, h, td) < 0) + goto fail; + /* Call commit callback in this plugin */ + if (plugin_transaction_commit_one(cp, h, td) < 0) + goto fail; + if (plugin_transaction_commit_done_one(cp, h, td) < 0) + goto fail; + /* Finalize */ + if (plugin_transaction_end_one(cp, h, td) < 0) + goto fail; retval = 1; done: + if (td){ + xmldb_get0_free(h, &td->td_target); + transaction_free(td); + } return retval; fail: retval = 0; diff --git a/example/main/example_backend.c b/example/main/example_backend.c index 571446db..fbf6f448 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -65,23 +65,25 @@ /*! Variable to control if reset code is run. * The reset code inserts "extra XML" which assumes ietf-interfaces is * loaded, and this is not always the case. - * Therefore, the backend must be started with -- -r to enable the reset function + * Start backend with -- -r */ static int _reset = 0; /*! Variable to control if state code is run * The state code adds extra non-config data - * Therefore, the backend must be started with -- -s to enable the state function + * Start backend with -- -s */ static int _state = 0; /*! File where state XML is read from, if _state is true -- -sS * Primarily for testing + * Start backend with -- -sS */ static char *_state_file = NULL; /*! Read state file init on startup instead of on request * Primarily for testing + * Start backend with -- -siS */ static int _state_file_init = 0; static cxobj *_state_xstate = NULL; @@ -89,15 +91,18 @@ static cxobj *_state_xstate = NULL; /*! Variable to control module-specific upgrade callbacks. * If set, call test-case for upgrading ietf-interfaces, otherwise call * auto-upgrade + * Start backend with -- -u */ static int _module_upgrade = 0; /*! Variable to control general-purpose upgrade callbacks. + * Start backend with -- -U */ static int _general_upgrade = 0; /*! Variable to control transaction logging (for debug) - * If set, call syslog for every transaction callback + * If set, call syslog for every transaction callback + * Start backend with -- -t */ static int _transaction_log = 0; diff --git a/example/main/example_backend_nacm.c b/example/main/example_backend_nacm.c index 67228cd6..3e25bfce 100644 --- a/example/main/example_backend_nacm.c +++ b/example/main/example_backend_nacm.c @@ -63,6 +63,7 @@ /*! Variable to control transaction logging (for debug) * If set, call syslog for every transaction callback + * Start backend with -- -t */ static int _transaction_log = 0; diff --git a/include/clixon_custom.h b/include/clixon_custom.h index c48090fe..40dbf36f 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -105,7 +105,7 @@ * Note, depending on plugin callbacks, there may be other dependencies which may make this * difficult in the general case. */ -#undef RESTART_PLUGIN_RPC +#define RESTART_PLUGIN_RPC /*! Differentiate creating XML object body/element vs elenmet to reduce space */ diff --git a/test/test_transaction.sh b/test/test_transaction.sh index e8ba3876..13e4c597 100755 --- a/test/test_transaction.sh +++ b/test/test_transaction.sh @@ -2,6 +2,8 @@ # Transaction functionality # The test uses two backend plugins (main and nacm) that logs to a file and a # netconf client to push operation. The tests then look at the log. +# The test assumes the two plugins recognize the -- -t argument which includes +# that one of them fails at validation at one point # The tests are as follows (first five only callbacks per se; then data vector tests) # 1. Validate-only transaction # 2. Commit transaction @@ -19,9 +21,6 @@ # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi -# Which format to use as datastore format internally -: ${format:=xml} - APPNAME=example cfg=$dir/conf_yang.xml @@ -77,11 +76,12 @@ cat < $cfg $dir/$APPNAME.sock /usr/local/var/$APPNAME/$APPNAME.pidfile /usr/local/var/$APPNAME - $format EOF # Check statements in log +# arg1: a statement to look for +# arg2: expected line number checklog(){ s=$1 # statement l0=$2 # linenr @@ -321,5 +321,3 @@ stop_backend -f $cfg rm -rf $dir -# unset conditional parameters -unset format diff --git a/test/test_transaction_restart.sh b/test/test_transaction_restart.sh new file mode 100755 index 00000000..b71f1a5e --- /dev/null +++ b/test/test_transaction_restart.sh @@ -0,0 +1,149 @@ +#!/usr/bin/env bash +# Transaction functionality: restart single plugin and observe that only that plugin +# gets callbacks +# The test uses two backend plugins (main and nacm) that logs. +# nacm is then restarted, not main + +# Magic line must be first in script (see README.md) +s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi + + +APPNAME=example + +cfg=$dir/conf_yang.xml +fyang=$dir/trans.yang +flog=$dir/backend.log +touch $flog + +# Used as a trigger for user-validittion errors, eg $errnr is invalid +errnr=42 + +cat < $fyang +module trans{ + yang-version 1.1; + namespace "urn:example:clixon"; + prefix ex; + /* Generic config data */ + container table{ + list parameter{ + key name; + leaf name{ + type string; + } + leaf value{ + type string; + } + } + } +} +EOF + +cat < $cfg + + $cfg + /usr/local/share/clixon + $fyang + /usr/local/lib/$APPNAME/clispec + /usr/local/lib/$APPNAME/backend + + /usr/local/lib/$APPNAME/netconf + /usr/local/lib/$APPNAME/restconf + /usr/local/lib/$APPNAME/cli + $APPNAME + $dir/$APPNAME.sock + /usr/local/var/$APPNAME/$APPNAME.pidfile + /usr/local/var/$APPNAME + +EOF + +# Check statements in log +# arg1: a statement to look for +# arg2: expected line number +checklog(){ + s=$1 # statement + l0=$2 # linenr + new "Check $s in log" +# echo "grep \"transaction_log $s line:$l0\" $flog" + t=$(grep -n "transaction_log $s" $flog) + + if [ -z "$t" ]; then + echo -e "\e[31m\nError in Test$testnr [$testname]:" + if [ $# -gt 0 ]; then + echo "Not found in log" + echo + fi + echo -e "\e[0m" + exit -1 + fi + l1=$(echo "$t" | awk -F ":" '{print $1}') + if [ $l1 -ne $l0 ]; then + echo -e "\e[31m\nError in Test$testnr [$testname]:" + if [ $# -gt 0 ]; then + echo "Expected match on line $l0, found on $l1" + echo + fi + echo -e "\e[0m" + exit -1 + fi +} + +new "test params: -f $cfg -l f$flog -- -t" # Fail on this +# Bring your own backend +if [ $BE -ne 0 ]; then + # kill old backend (if any) + new "kill old backend" + sudo clixon_backend -zf $cfg + if [ $? -ne 0 ]; then + err + fi + new "start backend -s init -f $cfg -l f$flog -- -t /foo" + start_backend -s init -f $cfg -l f$flog -- -t /foo # -t means transaction logging (foo is dummy) + + new "waiting" + wait_backend +fi + +let nr=0 + +new "Basic transaction to add top-level x" +expecteof "$clixon_netconf -qf $cfg" 0 "$nr
]]>]]>" '^]]>]]>$' + +new "Commit base" +expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' '^]]>]]>$' + +let line=13 # Skipping basic transaction. Sanity check, find one last transaction +xml="0
" +checklog "$nr nacm_end add: $xml" $line + +new "Send restart nacm plugin" +expecteof "$clixon_netconf -qf $cfg" 0 'example_backend_nacm]]>]]>' '^]]>]]>' + +# Now analyze log: +# all transactions come from nacm plugin only. +let nr++ +let line=14 + +for op in begin validate complete commit commit_done end; do + checklog "$nr nacm_$op add: $xml" $line + let line++ +done + +# Negative test: restart a plugin that does not exist +new "Send restart to nonexistatn plugin expect fail" +expecteof "$clixon_netconf -qf $cfg" 0 'xxx]]>]]>' '^applicationbad-elementpluginerrorNo such plugin]]>]]>$' + +if [ $BE -eq 0 ]; then + exit # BE +fi + +new "Kill backend" +# Check if premature kill +pid=$(pgrep -u root -f clixon_backend) +if [ -z "$pid" ]; then + err "backend already dead" +fi +# kill backend +stop_backend -f $cfg + +rm -rf $dir +