diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index f1e16882..1e8ef39c 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -195,11 +195,6 @@ startup_common(clicon_handle h, /* Clear flags xpath for get */ xml_apply0(xt, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)(XML_FLAG_MARK|XML_FLAG_CHANGE)); - if (xml_child_nr(xt) == 0){ /* If empty skip */ - td->td_target = xt; - xt = NULL; - goto ok; - } /* Here xt is old syntax */ /* General purpose datastore upgrade */ if (clixon_plugin_datastore_upgrade_all(h, db, xt, msdiff) < 0) @@ -211,6 +206,12 @@ startup_common(clicon_handle h, if (ret == 0) goto fail; } + /* If empty skip. Note upgrading can add children, so it may be empty before that. */ + if (xml_child_nr(xt) == 0){ + td->td_target = xt; + xt = NULL; + goto ok; + } if ((yspec = clicon_dbspec_yang(h)) == NULL){ clicon_err(OE_YANG, 0, "Yang spec not set"); goto done; diff --git a/example/main/example_backend.c b/example/main/example_backend.c index 86d78ebc..9663c3d5 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -840,6 +840,11 @@ upgrade_interfaces(clicon_handle h, { int retval = -1; + if (_module_upgrade) /* For testing */ + clicon_log(LOG_NOTICE, "%s %s op:%s from:%d to:%d", + __FUNCTION__, ns, + (op&XML_FLAG_ADD)?"ADD":(op&XML_FLAG_DEL)?"DEL":"CHANGE", + from, to); if (from <= 20140508){ if ((retval = upgrade_2014_to_2016(h, xt, ns, op, from, to, arg, cbret)) < 0) goto done; @@ -1084,15 +1089,8 @@ clixon_plugin_init(clicon_handle h) * test interface example. Otherwise the auto-upgrade feature is enabled. */ if (_module_upgrade){ -#if 1 if (upgrade_callback_register(h, upgrade_interfaces, "urn:example:interfaces", NULL) < 0) goto done; -#else - if (upgrade_callback_register(h, upgrade_2014_to_2016, "urn:example:interfaces", NULL) < 0) - goto done; - if (upgrade_callback_register(h, upgrade_2016_to_2018, "urn:example:interfaces", NULL) < 0) - goto done; -#endif } else if (upgrade_callback_register(h, xml_changelog_upgrade, NULL, NULL) < 0) diff --git a/lib/src/clixon_yang_module.c b/lib/src/clixon_yang_module.c index 38632e48..0cf52626 100644 --- a/lib/src/clixon_yang_module.c +++ b/lib/src/clixon_yang_module.c @@ -417,7 +417,7 @@ mod_ns_upgrade(clicon_handle h, goto done; } if ((ret = upgrade_callback_call(h, xt, ns, - xml_flag(xmod, (XML_FLAG_CHANGE|XML_FLAG_ADD|XML_FLAG_CHANGE)), + xml_flag(xmod, (XML_FLAG_ADD|XML_FLAG_DEL|XML_FLAG_CHANGE)), from, to, cbret)) < 0) goto done; if (ret == 0) /* XXX ignore and continue? */ diff --git a/lib/src/clixon_yang_parse_lib.c b/lib/src/clixon_yang_parse_lib.c index f1b69180..8b7fcd06 100644 --- a/lib/src/clixon_yang_parse_lib.c +++ b/lib/src/clixon_yang_parse_lib.c @@ -1208,8 +1208,7 @@ yang_spec_load_dir(clicon_handle h, if((ndp = clicon_file_dirent(dir, &dp, "(.yang)$", S_IFREG)) < 0) goto done; if (ndp == 0) - clicon_log(LOG_WARNING, "%s: No yang files found in %s", - __FUNCTION__, dir); + goto ok; /* Apply post steps on new modules, ie ones after modnr. */ modnr = yang_len_get(yspec); /* Load all yang files in dir */ @@ -1270,6 +1269,7 @@ yang_spec_load_dir(clicon_handle h, } if (yang_parse_post(h, yspec, modnr) < 0) goto done; + ok: retval = 0; done: if (dp) diff --git a/test/test_upgrade_module.sh b/test/test_upgrade_module.sh new file mode 100755 index 00000000..42b2af8b --- /dev/null +++ b/test/test_upgrade_module.sh @@ -0,0 +1,315 @@ +#!/usr/bin/env bash +# +# An effort to simplify the upgrading logic. +# The following simplifications have been made: +# 1) Just a single module defined with namespace "urn:example:interfaces" +# 2) Dont perform any upgrading actions, only look at modstate and revision dates +# Try to explore all possible cases, and ensure that the right upgrade functions are called +# +# Whats in the startup file (FROM) . There may be a revision date in the file called Y0. +# Whats loaded in the clixon backend (TO). There may be a revision date loaded called Y1. + +# FROM: x-axis: this is how the module is marked in a startup file: +# no : there is no modstate in the file (1) +# - : there is modstate but module is not present (2) +# Y0Y1 : there is modstate and revision is later than Y1 (5) +# +# TO: y-axis: the loaded YANG module in the backend: +# no : The module is not loaded +# Y1 : The module is loaded and has revision Y1 +# +# A state diagram showing the different cases: +#-----------+--------+-------+-------+-------+-------+ +# TOv FROM: | no | - | Y0Y1 | +#-----------+--------+-------+-------+-------+-------+ +# no | | | x | +#-----------+--------+-------+-------+-------+-------+ +# Y1 | | | x | | x | +#-----------+--------+-------+-------+-------+-------+ +# TO + +# 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/A@2016-01-01.yang +fyangb=$dir/B@2016-01-01.yang +log=$dir/backend.log +touch $log + +# XXX Seems like clixon cant handle no Yang files (fix that seperately) which means +# there needs to be some "background" Yangs in the case when A is removed. +cat < $fyangb +module B{ + prefix b; + revision 2016-01-01; + namespace "urn:example:b"; + container dummy{ + } +} +EOF + +# Create configuration +cat < $cfg + + $cfg + ietf-netconf:startup + /usr/local/share/clixon + $dir + /usr/local/var/$APPNAME/$APPNAME.sock + /usr/local/lib/example/backend + /usr/local/var/$APPNAME/$APPNAME.pidfile + $dir + true + /usr/local/lib/$APPNAME/clispec + /usr/local/lib/$APPNAME/cli + $APPNAME + +EOF + + +# Create 5 startup files 1-5 according to the 5 cases above (columns in the matrix) +# Argument: +# 1: payload, eg whats in the config apart from modstate +createstartups() +{ + payload=$1 + + # no : there is no modstate in the file (1) + cat < $dir/startup1.xml + + $payload + +EOF + + # Create startup datastore: + # - : there is modstate but module is not present (2) + cat < $dir/startup2.xml + + + 42 + + $payload + +EOF + + # Create startup datastore: + # $dir/startup3.xml + + + 42 + + A + 0814-01-28 + urn:example:interfaces + + + $payload + +EOF + + # Create startup datastore: + # =Y : there is modstate and revision is exactly Y (4) + cat < $dir/startup4.xml + + + 42 + + A + 2016-01-01 + urn:example:interfaces + + + $payload + +EOF + + # Create startup datastore: + # >Y : there is modstate and revision is exactly Y (5) + cat < $dir/startup5.xml + + + 42 + + A + 2018-01-01 + urn:example:interfaces + + + $payload + +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 on line $l0" + t=$(grep -n "$s" $log) + 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 +} + +# Check statements are not in log +# arg1: a statement to look for +checknolog(){ + s=$1 # statement + new "Check $s not in log" +# echo "grep -n "$s" $log" + t=$(grep -n "$s" $log) +# echo "t:$t" + if [ -n "$t" ]; then + echo -e "\e[31m\nError in Test$testnr [$testname]:" + if [ $# -gt 0 ]; then + echo "$s found in log" + echo + fi + echo -e "\e[0m" + exit -1 + fi +} + +# Arguments: +# 1: from usecase 1-5 +# 2: v: verb: true or false. The next statement should be there or not +# 3: what to look for in log (if v=true it should be there, if v=false it should not be there) +# 4: Linenr in log +testrun(){ + i=$1 + flag=$2 + match=$3 + line=$4 + + cp $dir/startup$i.xml $dir/startup_db + : > $log # truncate log + new "test params: -f $cfg" + # 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 startup -f $cfg -l f$log -- -u" + start_backend -s startup -f $cfg -l f$log -- -u + + new "waiting" + wait_backend + fi + + if $flag; then + checklog "$match" $line + else + checknolog "$match" + fi + + if [ $BE -ne 0 ]; then + 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 + fi +} + +# Run through all tests with five different startups, with 5 tests with loaded YANG module A +# and then three tests without loaded YANG module A +# Input is different payloads +# Arguments: +# 1 : payload with loaded YANG +# 2 : payload without loaded YANG +testall() +{ + payload1=$1 + payload2=$2 + + # Yang module with revision Y="2016-01-01" + cat < $fyang +module A{ + prefix a; + revision 2016-01-01; + namespace "urn:example:interfaces"; + container dummy{ + } +} +EOF + createstartups "$payload1" + + # Y1 : The module is loaded and has revision Y + new "1. module loaded, no modstate" + testrun 1 false upgrade_interfaces + + new "2. module loaded, modstate but module absent" + testrun 2 true "upgrade_interfaces urn:example:interfaces op:ADD from:0 to:20160101" 1 + + new "3. module loaded, modstate with earlier date Y0Y1" + testrun 5 true "upgrade_interfaces urn:example:interfaces op:CHANGE from:20180101 to:20160101" 1 + + # LOAD: no : The module is not loaded + # Yang module with revision Y="2016-01-01" + rm -f $fyang + createstartups "$payload2" + + new "1. module not loaded, no modstate" + testrun 1 false upgrade_interfaces + + new "2. module not loaded, modstate but module absent" + testrun 2 false upgrade_interfaces + + new "3. module not loaded, modstate with date Y1" + testrun 3 true "upgrade_interfaces urn:example:interfaces op:DEL from:8140128 to:0" 1 +} + +# There is some issue with having different payloads in the config file +# That is why there are tests with different payloads + +new "b payload only---------" +testall '' '' + +new "b payload and interfaces payload---------" +testall '' '' + +new "a payload only---------" +testall '' '' + +new "empty payload---------" +testall '' '' + +#rm -rf $dir +