From b714eff077f584b78c36a054f655dbe41f89d0bf Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 18 Apr 2019 12:20:31 +0200 Subject: [PATCH] Failure in startup with -m startup or running left running_db cleared. --- CHANGELOG.md | 2 + apps/backend/backend_main.c | 19 +++++- apps/backend/backend_startup.c | 16 +++-- test/test_startup.sh | 103 ++++++++++++++++++++++++++++----- 4 files changed, 119 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57215d1a..c6eba715 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -134,6 +134,8 @@ * Added libgen.h for baseline() ### Corrected Bugs +* Failure in startup with -m startup or running left running_db cleared. + * Running-db should not be changed on failure. Unless failure-db defined. Or if SEGV, etc. In those cases, tmp_db should include the original running-db. * Backend plugin returning NULL was still installed - is now logged and skipped. * [Parent list key is not validated if not provided via RESTCONF #83](https://github.com/clicon/clixon/issues/83), thanks achernavin22. * [Invalid JSON if GET /operations via RESTCONF #82](https://github.com/clicon/clixon/issues/82), thanks achernavin22 diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index 1cac8931..7583ea5a 100644 --- a/apps/backend/backend_main.c +++ b/apps/backend/backend_main.c @@ -638,16 +638,33 @@ main(int argc, status = STARTUP_OK; break; case SM_RUNNING: /* Use running as startup */ - /* Copy original running to startup and treat as startup */ + /* Copy original running to tmp as backup (restore if error) */ if (xmldb_copy(h, "running", "tmp") < 0) goto done; + /* [Delete and] create running db */ + if (startup_db_reset(h, "running") < 0) + goto done; ret = startup_mode_startup(h, "tmp", cbret); + /* If ret fails, copy tmp back to running */ + if (ret != 1) + if (xmldb_copy(h, "tmp", "running") < 0) + goto done; if (ret2status(ret, &status) < 0) goto done; break; case SM_STARTUP: + /* Copy original running to tmp as backup (restore if error) */ + if (xmldb_copy(h, "running", "tmp") < 0) + goto done; + /* [Delete and] create running db */ + if (startup_db_reset(h, "running") < 0) + goto done; /* Load and commit from startup */ ret = startup_mode_startup(h, "startup", cbret); + /* If ret fails, copy tmp back to running */ + if (ret != 1) + if (xmldb_copy(h, "tmp", "running") < 0) + goto done; if (ret2status(ret, &status) < 0) goto done; /* if status = STARTUP_INVALID, cbret contains info */ diff --git a/apps/backend/backend_startup.c b/apps/backend/backend_startup.c index 9c9ab382..fe0d5ba8 100644 --- a/apps/backend/backend_startup.c +++ b/apps/backend/backend_startup.c @@ -150,9 +150,6 @@ startup_mode_startup(clicon_handle h, int retval = -1; int ret; - /* [Delete and] create running db */ - if (startup_db_reset(h, "running") < 0) - goto done; /* Load plugins and call plugin_init() */ if (backend_plugin_initiate(h) != 0) goto done; @@ -299,15 +296,22 @@ startup_failsafe(clicon_handle h) clicon_err(OE_XML, errno, "cbuf_new"); goto done; } - if (startup_db_reset(h, "running") < 0) - goto done; if ((ret = xmldb_exists(h, db)) < 0) goto done; if (ret == 0){ /* No it does not exist, fail */ clicon_err(OE_DB, 0, "Startup failed and no Failsafe database found, exiting"); goto done; } - if ((ret = candidate_commit(h, db, cbret)) < 0) /* diff */ + /* Copy original running to tmp as backup (restore if error) */ + if (xmldb_copy(h, "running", "tmp") < 0) + goto done; + if (startup_db_reset(h, "running") < 0) + goto done; + ret = candidate_commit(h, db, cbret); + if (ret != 1) + if (xmldb_copy(h, "tmp", "running") < 0) + goto done; + if (ret < 0) goto done; if (ret == 0){ clicon_err(OE_DB, 0, "Startup failed, Failsafe database validation failed %s", cbuf_get(cbret)); diff --git a/test/test_startup.sh b/test/test_startup.sh index 9457e4ea..ac4be92f 100755 --- a/test/test_startup.sh +++ b/test/test_startup.sh @@ -6,6 +6,9 @@ # - An extra xml configuration file starts with an "extra" interface # - running db starts with a "run" interface # - startup db starts with a "start" interface +# There is also an "invalid" XML and a "broken" XML +# There are two steps, first run through everything OK +# Then try with invalid and borken XML and ensure the backend quits and all is untouched # Magic line must be first in script (see README.md) s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi @@ -36,27 +39,37 @@ cat < $cfg EOF -# Create running-db containin the interface "run" +# Create running-db containin the interface "run" OK runvar='runex:ethtrue' -# Create startup-db containing the interface "startup" +# Create startup-db containing the interface "startup" OK startvar='startupex:ethtrue' -# extra +# extra OK extravar='extraex:ethtrue' +# invalid (contains ), but OK XML syntax +invalidvar='invalidex:ethtrue' + +# Broken XML (contains ) +brokenvar='brokenex:ethtrue' + # Create a pre-set running, startup and (extra) config. # The configs are identified by an interface called run, startup, extra. # Depending on startup mode (init, none, running, or startup) # expect different output of an initial get-config of running testrun(){ mode=$1 - exprun=$2 # expected running_db after startup + rdb=$2 # running db at start + sdb=$3 # startup db at start + edb=$4 # extradb at start + exprun=$5 # expected running_db after startup + sudo rm -f $dir/*_db - echo "$runvar" > $dir/running_db - echo "$startvar" > $dir/startup_db - echo "$extravar" > $dir/extra_db + echo "$rdb" > $dir/running_db + echo "$sdb" > $dir/startup_db + echo "$edb" > $dir/extra_db if [ $BE -ne 0 ]; then # Bring your own backend # kill old backend (if any) @@ -81,7 +94,7 @@ testrun(){ expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' "^$exprun]]>]]>$" new "Startup test for $mode mode, check startup is untouched" - expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' "^$startvar]]>]]>$" + expecteof "$clixon_netconf -qf $cfg" 0 ']]>]]>' "^$sdb]]>]]>$" new "Kill backend" # Check if premature kill @@ -93,17 +106,79 @@ testrun(){ stop_backend -f $cfg } # testrun + +# The backend should fail with 255 and all db:s should be unaffected +testfail(){ + mode=$1 + rdb=$2 # running db at start + sdb=$3 # startup db at start + edb=$4 # extradb at start + + sudo rm -f $dir/*_db + + echo "$rdb" > $dir/running_db + echo "$sdb" > $dir/startup_db + echo "$edb" > $dir/extra_db + + # kill old backend (if any) + new "kill old backend" + sudo clixon_backend -zf $cfg + if [ $? -ne 0 ]; then + err + fi + new "start backend -f $cfg -s $mode -c $dir/extra_db" + ret=$(start_backend -1 -s $mode -f $cfg -c $dir/extra_db 2> /dev/null) + r=$? + if [ $r -ne 255 ]; then + err "Unexpected retval" $r + fi + # permission kludges + sudo chmod 666 $dir/running_db + sudo chmod 666 $dir/startup_db + new "Checking running unchanged" + ret=$(diff $dir/running_db <(echo "$rdb")) + if [ $? -ne 0 ]; then + err "$rdb" "$ret" + fi + new "Checking startup unchanged" + ret=$(diff $dir/startup_db <(echo "$sdb")) + if [ $? -ne 0 ]; then + err "$sdb" "$ret" + fi + + new "Checking extra unchanged" + ret=$(diff $dir/extra_db <(echo "$edb")) + if [ $? -ne 0 ]; then + err "$edb" "$ret" + fi +} + +# 1. Try different modes on OK running/startup/extra # Init mode: delete running and reload from scratch (just extra) -testrun init "$extravar" +testrun init "$runvar" "$startvar" "$extravar" "$extravar" # None mode: do nothing, running remains -testrun none "$runvar" +testrun none "$runvar" "$startvar" "$extravar" "$runvar" # Running mode: keep running but load also extra -testrun running 'extraex:ethtruerunex:ethtrue' +testrun running "$runvar" "$startvar" "$extravar" 'extraex:ethtruerunex:ethtrue' # Startup mode: scratch running, load startup with extra on top -testrun startup 'extraex:ethtruestartupex:ethtrue' +testrun startup "$runvar" "$startvar" "$extravar" 'extraex:ethtruestartupex:ethtrue' -echo $dir -#rm -rf $dir +# 2. Try different modes on Invalid running/startup/extra WITHOUT failsafe +# ensure all db:s are unchanged after failure. + +new "Test invalid running in running mode" +testfail running "$invalidvar" "$startvar" "$extravar" + +new "Run invalid startup in startup mode" +testfail startup "$runvar" "$invalidvar" "$extravar" + +new "Test broken running in running mode" +testfail running "$brokenvar" "$startvar" "$extravar" + +new "Run broken startup in startup mode" +testfail startup "$runvar" "$brokenvar" "$extravar" + +rm -rf $dir