* Added check of old config in upgrade scenarios.

* Added to clixon-config.yang `CLICON_XMLDB_UPGRADE_CHECKOLD`, set to false to keep less strict check

CHANGELOG.md

@@ -33,9 +33,10 @@ Expected: April
 
 ### New features
 
-* Add multiple yang support also for obsolete versions
+* Add multiple yang support also for old/previous versions
-  * This means that files and datastores supporting modstate also looks for deleted or updated yang modules.
+  * This means that files and datastores supporting modstate also look for deleted or updated yang modules.
   * A stricter binding which gives error if loading outdated YANG file does not exist.
+  * Keep old behavior: dont check old config file: set `CLICON_XMLDB_UPGRADE_CHECKOLD` to false.
 	
 ### API changes on existing protocol/config features
 
@@ -55,6 +56,7 @@ Expected: April
 * New clixon-config@2020-03-08.yang revision
   * Added: `CLICON_NETCONF_HELLO_OPTIONAL`
   * Added: `CLICON_CLI_AUTOCLI_EXCLUDE`
+  * Added: `CLICON_XMLDB_UPGRADE_CHECKOLD`
 
 ### C/CLI-API changes on existing features
 

apps/backend/backend_commit.c

@@ -192,6 +192,7 @@ startup_common(clicon_handle       h,
     cxobj              *xt = NULL;
     cxobj              *x;
     cxobj              *xret = NULL;
+    cxobj              *xerr = NULL;
 
     /* If CLICON_XMLDB_MODSTATE is enabled, then get the db XML with 
      * potentially non-matching module-state in msdiff
@@ -203,9 +204,19 @@ startup_common(clicon_handle       h,
     /* Get the startup datastore WITHOUT binding to YANG, sorting and default setting. 
      * It is done below, later in this function
      */
-    if ((ret = xmldb_get0(h, db, YB_NONE, NULL, "/", 0, &xt, msdiff, NULL)) < 0)
+    if (clicon_option_bool(h, "CLICON_XMLDB_UPGRADE_CHECKOLD")){
+	if ((ret = xmldb_get0(h, db, YB_MODULE, NULL, "/", 0, &xt, msdiff, &xerr)) < 0)
 	    goto done;
-    /* ret should not be 0 */
+	if (ret == 0){     /* ret should not be 0 */
+	    if (clicon_xml2cbuf(cbret, xerr, 0, 0, -1) < 0)
+		goto done;
+	    goto fail;
+	}
+    }
+    else {
+	if (xmldb_get0(h, db, YB_NONE, NULL, "/", 0, &xt, msdiff, &xerr) < 0)
+	    goto done;
+    }
     if ((yspec = clicon_dbspec_yang(h)) == NULL){
 	clicon_err(OE_YANG, 0, "Yang spec not set");
 	goto done;
@@ -315,6 +326,8 @@ startup_common(clicon_handle       h,
  ok:
     retval = 1;
  done:
+    if (xerr)
+	xml_free(xerr);
     if (xret)
 	xml_free(xret);
     if (xt)

lib/src/clixon_datastore_read.c

@@ -475,7 +475,12 @@ xmldb_readfile(clicon_handle    h,
 	clicon_err(OE_UNIX, errno, "open(%s)", dbfile);
 	goto done;
     }    
-    /* ret == 0 should not happen with YB_NONE. Binding is done later */
+    /* Read whole datastore file on the form:
+     * <config>
+     *   modstate*  # this is analyzed, stripped and returned as msdiff in text_read_modstate
+     *   config*
+     * </config>
+     * ret == 0 should not happen with YB_NONE. Binding is done later */
     if (strcmp(format, "json")==0){
 	if (clixon_json_parse_file(fp, YB_NONE, yspec, &x0, xerr) < 0) 
 	    goto done;
@@ -502,17 +507,17 @@ xmldb_readfile(clicon_handle    h,
     xml_flag_set(x0, XML_FLAG_TOP);
     if (xml_child_nr(x0) == 0 && de)
 	de->de_empty = 1;
-
+    /* Check if we support modstate */
-    /* Datastore files may contain module-state defining
-     * which modules are used in the file. 
-     * Strip module-state and return msdiff
-     */
     if (clicon_option_bool(h, "CLICON_XMLDB_MODSTATE"))
 	if ((msdiff = modstate_diff_new()) == NULL)
 	    goto done;
     if ((x = xml_find_type(x0, NULL, "modules-state", CX_ELMNT)) != NULL)
 	if ((xmodfile = xml_dup(x)) == NULL)
 	    goto done;
+    /* Datastore files may contain module-state defining
+     * which modules are used in the file. 
+     * Strip module-state, analyze it with CHANGE/ADD/RM and return msdiff
+     */
     if (text_read_modstate(h, yspec, x0, msdiff) < 0)
 	goto done;
     if (yb == YB_MODULE){
@@ -534,6 +539,7 @@ xmldb_readfile(clicon_handle    h,
 		/* Extract revision */
 		if ((rev = xml_find_body(xmsd, "revision")) == NULL)
 		    continue;
+		/* Add old/deleted yangs not present in the loaded/running yangspec. */
 		if ((ymod = yang_find_module_by_namespace_revision(yspec, ns, rev)) == NULL){
 		    /* Append it */
 		    if (yang_spec_parse_module(h, name, rev, yspec) < 0){
@@ -575,13 +581,15 @@ xmldb_readfile(clicon_handle    h,
 			goto done;
 		}
 	    }
-	}
+	} /* if msdiff */
 	/* xml looks like: <top><config><x>... actually YB_MODULE_NEXT 
 	 */
 	if ((ret = xml_bind_yang(x0, YB_MODULE, yspec1?yspec1:yspec, xerr)) < 0)
 	    goto done;
 	if (ret == 0)
 	    goto fail;
+	if (xml_sort_recurse(x0) < 0)
+	    goto done;
     }
     if (xp){
 	*xp = x0;
@@ -1026,11 +1034,16 @@ xmldb_get(clicon_handle    h,
  * @note Use of 1 for OK
  * @code
  *   cxobj   *xt;
- *   if (xmldb_get0(h, "running", YB_MODULE, nsc, "/interface[name="eth"]", 0, &xt, NULL, NULL) < 0)
+ *   cxobj   *xerr = NULL;
+ *   if (xmldb_get0(h, "running", YB_MODULE, nsc, "/interface[name="eth"]", 0, &xt, NULL, &xerr) < 0)
  *      err;
+ *   if (ret == 0){ # Not if YB_NONE
+ *      # Error handling
+ *   }
  *   ...
  *   xmldb_get0_clear(h, xt);   # Clear tree from default values and flags 
  *   xmldb_get0_free(h, &xt);   # Free tree
+ *   xml_free(xerr);
  * @endcode
  * @see xml_nsctx_node  to get a XML namespace context from XML tree
  * @see xmldb_get for a copy version (old-style)

lib/src/clixon_yang_module.c

@@ -376,11 +376,8 @@ yang_modules_state_get(clicon_handle    h,
 /*! For single module state with namespace, get revisions and send upgrade callbacks
  * @param[in]  h        Clicon handle
  * @param[in]  xt       Top-level XML tree to be updated (includes other ns as well)
- * @param[in]  xd       XML module state diff (for one yang module)
+ * @param[in]  xmod     XML module state diff (for one yang module)
- * @param[in]  xvec     Help vector where to store XML child nodes (??)
+ * @param[in]  ns       Namespace of module state we are looking for
- * @param[in]  xlen     Length of xvec
- * @param[in]  ns0      Namespace of module state we are looking for
- * @param[in]  op       add,del, or mod
  * @param[out] cbret    Netconf error message if invalid
  * @retval     1        OK
  * @retval     0        Validation failed (cbret set)

test/test_cli.sh

@@ -116,11 +116,11 @@ new "cli check load"
 expectpart "$($clixon_cli -1 -f $cfg -l o show conf cli)" 0 "interfaces interface eth/0/0 ipv4 enabled true"
 
 new "cli debug set"
-expectpart "$($clixon_cli -1 -f $cfg -l o debug level 1)" 0 "^$"
+expectpart "$($clixon_cli -1 -f $cfg -l o debug cli 1)" 0 "^$"
 
 # How to test this?
 new "cli debug reset"
-expectpart "$($clixon_cli -1 -f $cfg -l o debug level 0)" 0 "^$"
+expectpart "$($clixon_cli -1 -f $cfg -l o debug cli 0)" 0 "^$"
 
 new "cli rpc"
 # We dont know which message-id the cli app uses

test/test_datastore_repair.sh

@@ -2,7 +2,7 @@
 # Test of the general-purpose (raw) upgrade mechanism.
 # Input is a startup db without mod-state info.
 # It has wrong namespace bindings and needs to remove some nodes
-# Output is a valid config woith correct namespaces and removed nods
+# Output is a valid config with correct namespaces and removed nods
 # The code for this is in the main example backend plugin.
 
 # Magic line must be first in script (see README.md)
@@ -26,6 +26,7 @@ cat <<EOF > $cfg
   <CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
   <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
   <CLICON_XMLDB_MODSTATE>true</CLICON_XMLDB_MODSTATE>
+  <CLICON_XMLDB_UPGRADE_CHECKOLD>false</CLICON_XMLDB_UPGRADE_CHECKOLD>
   <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
   <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
   <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>

test/test_nacm_default.sh

@@ -31,6 +31,7 @@ cat <<EOF > $cfg
   <CLICON_BACKEND_DIR>/usr/local/lib/$APPNAME/backend</CLICON_BACKEND_DIR>
   <CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
   <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
+  <CLICON_XMLDB_UPGRADE_CHECKOLD>true</CLICON_XMLDB_UPGRADE_CHECKOLD>
   <CLICON_NACM_MODE>internal</CLICON_NACM_MODE>
   <CLICON_NACM_DISABLED_ON_EMPTY>true</CLICON_NACM_DISABLED_ON_EMPTY>
   <CLICON_XMLDB_FORMAT>$format</CLICON_XMLDB_FORMAT>

test/test_restconf_basic_auth.sh

@@ -46,29 +46,7 @@ cat <<EOF > $cfg
   <CLICON_SOCK>/usr/local/var/$APPNAME/$APPNAME.sock</CLICON_SOCK>
   <CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
   <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
-  <CLICON_MODULE_LIBRARY_RFC7895>true</CLICON_MODULE_LIBRARY_RFC7895>
+  <CLICON_XMLDB_UPGRADE_CHECKOLD>true</CLICON_XMLDB_UPGRADE_CHECKOLD>
-  <CLICON_NACM_MODE>internal</CLICON_NACM_MODE>
-  <CLICON_ANONYMOUS_USER>$anonymous</CLICON_ANONYMOUS_USER>
-</clixon-config>
-EOF
-
-# Start with common config, then append fcgi/native specific config
-cat <<EOF > $cfg
-<clixon-config xmlns="http://clicon.org/config">
-  <CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
-  <CLICON_FEATURE>ietf-netconf:startup</CLICON_FEATURE>
-  <CLICON_YANG_DIR>/usr/local/share/clixon</CLICON_YANG_DIR>
-  <CLICON_YANG_DIR>$IETFRFC</CLICON_YANG_DIR>
-  <CLICON_YANG_MAIN_FILE>$fyang</CLICON_YANG_MAIN_FILE>
-  <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
-  <CLICON_BACKEND_DIR>/usr/local/lib/$APPNAME/backend</CLICON_BACKEND_DIR>
-  <CLICON_BACKEND_REGEXP>example_backend.so$</CLICON_BACKEND_REGEXP>
-  <CLICON_RESTCONF_DIR>/usr/local/lib/$APPNAME/restconf</CLICON_RESTCONF_DIR>
-  <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
-  <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
-  <CLICON_SOCK>/usr/local/var/$APPNAME/$APPNAME.sock</CLICON_SOCK>
-  <CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
-  <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
   <CLICON_MODULE_LIBRARY_RFC7895>true</CLICON_MODULE_LIBRARY_RFC7895>
   <CLICON_NACM_MODE>internal</CLICON_NACM_MODE>
   <CLICON_ANONYMOUS_USER>$anonymous</CLICON_ANONYMOUS_USER>

test/test_restconf_ssl_certs.sh

@@ -173,6 +173,7 @@ cat <<EOF > $cfg
   <CLICON_SOCK>/usr/local/var/$APPNAME/$APPNAME.sock</CLICON_SOCK>
   <CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
   <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
+  <CLICON_XMLDB_UPGRADE_CHECKOLD>true</CLICON_XMLDB_UPGRADE_CHECKOLD>
   <CLICON_MODULE_LIBRARY_RFC7895>true</CLICON_MODULE_LIBRARY_RFC7895>
   <CLICON_NACM_MODE>internal</CLICON_NACM_MODE>
   <restconf>

test/test_upgrade_auto.sh

@@ -183,6 +183,7 @@ cat <<EOF > $cfg
   <CLICON_XMLDB_MODSTATE>true</CLICON_XMLDB_MODSTATE>
   <CLICON_XML_CHANGELOG>true</CLICON_XML_CHANGELOG>
   <CLICON_XML_CHANGELOG_FILE>$changelog</CLICON_XML_CHANGELOG_FILE>
+  <CLICON_XMLDB_UPGRADE_CHECKOLD>false</CLICON_XMLDB_UPGRADE_CHECKOLD>
   <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
   <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
   <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>

test/test_upgrade_failsafe.sh

@@ -105,6 +105,7 @@ cat <<EOF > $cfg
   <CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
   <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
   <CLICON_XMLDB_MODSTATE>true</CLICON_XMLDB_MODSTATE>
+  <CLICON_XMLDB_UPGRADE_CHECKOLD>false</CLICON_XMLDB_UPGRADE_CHECKOLD>
   <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
   <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
   <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>

test/test_upgrade_interfaces.sh

@@ -243,6 +243,7 @@ cat <<EOF > $cfg
   <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
   <CLICON_XMLDB_MODSTATE>true</CLICON_XMLDB_MODSTATE>
   <CLICON_XML_CHANGELOG>false</CLICON_XML_CHANGELOG>
+  <CLICON_XMLDB_UPGRADE_CHECKOLD>false</CLICON_XMLDB_UPGRADE_CHECKOLD>
   <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
   <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
   <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>

test/test_upgrade_module.sh

@@ -65,6 +65,7 @@ cat <<EOF > $cfg
   <CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
   <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
   <CLICON_XMLDB_MODSTATE>true</CLICON_XMLDB_MODSTATE>
+  <CLICON_XMLDB_UPGRADE_CHECKOLD>false</CLICON_XMLDB_UPGRADE_CHECKOLD>
   <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
   <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
   <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>

test/test_upgrade_quit.sh

@@ -242,6 +242,7 @@ cat <<EOF > $cfg
   <CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
   <CLICON_FEATURE>ietf-netconf:startup</CLICON_FEATURE>
   <CLICON_YANG_DIR>/usr/local/share/clixon</CLICON_YANG_DIR>
+  <CLICON_YANG_DIR>$dir</CLICON_YANG_DIR>
   <CLICON_FEATURE>interfaces:if-mib</CLICON_FEATURE>
   <CLICON_YANG_MAIN_DIR>$dir</CLICON_YANG_MAIN_DIR>
   <CLICON_SOCK>/usr/local/var/$APPNAME/$APPNAME.sock</CLICON_SOCK>
@@ -251,6 +252,7 @@ cat <<EOF > $cfg
   <CLICON_XMLDB_MODSTATE>true</CLICON_XMLDB_MODSTATE>
   <CLICON_XMLDB_PRETTY>false</CLICON_XMLDB_PRETTY>
   <CLICON_XML_CHANGELOG>false</CLICON_XML_CHANGELOG>
+  <CLICON_XMLDB_UPGRADE_CHECKOLD>true</CLICON_XMLDB_UPGRADE_CHECKOLD>
   <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
   <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
   <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>

test/test_upgrade_simple.sh

@@ -25,6 +25,7 @@ cat <<EOF > $cfg
   <CLICON_BACKEND_PIDFILE>/usr/local/var/hello.pidfile</CLICON_BACKEND_PIDFILE>
   <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
   <CLICON_XMLDB_MODSTATE>true</CLICON_XMLDB_MODSTATE>
+  <CLICON_XMLDB_UPGRADE_CHECKOLD>false</CLICON_XMLDB_UPGRADE_CHECKOLD>
   <CLICON_STARTUP_MODE>init</CLICON_STARTUP_MODE>
   <CLICON_MODULE_LIBRARY_RFC7895>false</CLICON_MODULE_LIBRARY_RFC7895>
 </clixon-config>

yang/clixon/clixon-config@2021-03-08.yang

@@ -46,8 +46,9 @@ module clixon-config {
     revision 2021-03-08 {
 	description
 	    "Added option:
-                   CLICON_NETCONF_HELLO_OPTIONAL;
+                   CLICON_NETCONF_HELLO_OPTIONAL
-		   CLICON_CLI_AUTOCLI_EXCLUDE";
+		   CLICON_CLI_AUTOCLI_EXCLUDE
+	           CLICON_XMLDB_UPGRADE_CHECKOLD";
     }
     revision 2020-12-30 {
 	description
@@ -779,6 +780,16 @@ module clixon-config {
                  yang modules match.
                  See also CLICON_MODULE_LIBRARY_RFC7895";
 	}
+	leaf CLICON_XMLDB_UPGRADE_CHECKOLD {
+	    type boolean;
+	    default true;
+	    description
+		"Controls behavior of check of startup in upgrade scenarios.
+                 If set, yang bind and check datastore syntax against the old Yang. 
+                 The old yang must be accessible via YANG_DIR.
+                 Will fail startup if old yang not found or if old config does not match.
+                 If not set, no yang check of old config is made until it is upgraded to new yang.";
+	}
 	leaf CLICON_XML_CHANGELOG {
 	    type boolean;
 	    default false;