Skip to content

Commit

Permalink
Added multi-datastore
Browse files Browse the repository at this point in the history
Fixed: [connection open does local commit on services](#119)
Fixed problem with optimization of moving per-dev commits to single per-transition
  • Loading branch information
olofhagsand committed May 2, 2024
1 parent b0dbb1c commit 971e2e9
Show file tree
Hide file tree
Showing 10 changed files with 660 additions and 177 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Expected: June 2024

### Corrected Bugs

* Fixed: [connection open does local commit on services](https://github.com/clicon/clixon-controller/issues/119)
* Fixed: [CLI: Can't change configuration on Juniper QFX devices](https://github.com/clicon/clixon-controller/issues/109)
* Fixed: [Commit to device(s) in OPEN state even if we have other devices in CLOSED state](https://github.com/clicon/clixon-controller/issues/95)

Expand Down
2 changes: 2 additions & 0 deletions src/controller.xml.in
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
<CLICON_RESTCONF_INSTALLDIR>@SBINDIR@</CLICON_RESTCONF_INSTALLDIR>
<CLICON_RESTCONF_USER>@CLICON_USER@</CLICON_RESTCONF_USER>
<CLICON_RESTCONF_PRIVILEGES>drop_perm</CLICON_RESTCONF_PRIVILEGES>
<!-- Cannot use drop because XMLDB_MULTI creates new files -->
<CLICON_BACKEND_PRIVILEGES>none</CLICON_BACKEND_PRIVILEGES>
<CLICON_VALIDATE_STATE_XML>false</CLICON_VALIDATE_STATE_XML>
<CLICON_CLI_HELPSTRING_TRUNCATE>true</CLICON_CLI_HELPSTRING_TRUNCATE>
<CLICON_CLI_HELPSTRING_LINES>1</CLICON_CLI_HELPSTRING_LINES>
Expand Down
22 changes: 19 additions & 3 deletions src/controller_device_recv.c
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ device_state_recv_config(clixon_handle h,
controller_transaction *ct;
int merge = 0;
int transient = 0;
cxobj *xt1 = NULL;

clixon_debug(CLIXON_DBG_CTRL | CLIXON_DBG_DETAIL, "%s", __FUNCTION__);
if ((ret = rpc_reply_sanity(dh, xmsg, rpcname, conn_state)) < 0)
Expand Down Expand Up @@ -325,10 +326,23 @@ device_state_recv_config(clixon_handle h,
}
goto ok;
}
/* 2. Why not just cp? */
if ((ret = xmldb_put(h, "candidate", OP_NONE, xt, NULL, cbret)) < 0)
/* Must make a copy: xmldb_put strips attributes */
if ((xt1 = xml_dup(xt)) == NULL)
goto done;
if ((ret = device_config_write(h, name, "SYNCED", xt, cbret)) < 0)
/* 1. Put device config change to tmp */
if ((ret = xmldb_put(h, "tmpdev", OP_NONE, xt, NULL, cbret)) < 0)
goto done;
if (ret == 0){ /* discard */
clixon_debug(CLIXON_DBG_CTRL, "%s", cbuf_get(cbret));
if (device_close_connection(dh, "Failed to commit: %s", cbuf_get(cbret)) < 0)
goto done;
goto closed;
}
device_handle_sync_time_set(dh, NULL);
/* 2. Put same to candidate */
if (ret && (ret = xmldb_put(h, "candidate", OP_NONE, xt1, NULL, cbret)) < 0)
goto done;
if (ret && (ret = device_config_write(h, name, "SYNCED", xt, cbret)) < 0)
goto done;
if (ret == 0){
if (device_close_connection(dh, "%s", cbuf_get(cbret)) < 0)
Expand All @@ -339,6 +353,8 @@ device_state_recv_config(clixon_handle h,
ok:
retval = 1;
done:
if (xt1)
xml_free(xt1);
if (xt)
xml_free(xt);
if (xerr)
Expand Down
26 changes: 19 additions & 7 deletions src/controller_device_state.c
Original file line number Diff line number Diff line change
Expand Up @@ -1013,14 +1013,20 @@ device_state_check_sanity(device_handle dh,
return 1;
}

/*! Commit candidate
/*! Utility function to Commit a db to running
*
* @param[in] h Clixon handle
* @param[in] dh Clixon device handle.
* @param[in] ct Transaction
* @param[in] db Database name
* @retval 0 OK
* @retval -1 Error
*/
static int
device_commit_when_done(clixon_handle h,
device_handle dh,
char *name,
controller_transaction *ct,
uint64_t tid)
char *db)
{
int retval = -1;
cbuf *cbret = NULL;
Expand All @@ -1030,7 +1036,7 @@ device_commit_when_done(clixon_handle h,
clixon_err(OE_UNIX, errno, "cbuf_new");
goto done;
}
if ((ret = candidate_commit(h, NULL, "candidate", 0, 0, cbret)) < 0){
if ((ret = candidate_commit(h, NULL, db, 0, 0, cbret)) < 0){
/* Handle that candidate_commit can return < 0 if transaction ongoing */
cprintf(cbret, "%s", clixon_err_reason());
ret = 0;
Expand All @@ -1039,7 +1045,9 @@ device_commit_when_done(clixon_handle h,
clixon_debug(CLIXON_DBG_CTRL, "%s", cbuf_get(cbret));
if (device_close_connection(dh, "Failed to commit: %s", cbuf_get(cbret)) < 0)
goto done;
if (controller_transaction_failed(h, tid, ct, dh, TR_FAILED_DEV_LEAVE, name, device_handle_logmsg_get(dh)) < 0)
if (controller_transaction_failed(h, ct->ct_id, ct, dh, TR_FAILED_DEV_LEAVE,
device_handle_name_get(dh),
device_handle_logmsg_get(dh)) < 0)
goto done;
goto failed;
}
Expand Down Expand Up @@ -1281,11 +1289,15 @@ device_state_handler(clixon_handle h,
/* The device is OK */
if (device_state_check_ok(h, dh, ct) < 0)
goto done;
if (ct->ct_state == TS_DONE && ct->ct_result == TR_SUCCESS){
if ((ret = device_commit_when_done(h, dh, name, ct, tid)) < 0)
if (ct->ct_state == TS_DONE &&
ct->ct_result == TR_SUCCESS &&
!ct->ct_pull_transient){
/* See puts from each device in device_state_recv_config() */
if ((ret = device_commit_when_done(h, dh, ct, "tmpdev")) < 0)
goto done;
if (ret == 0)
break;
xmldb_delete(h, "tmpdev");
}
break;
case CS_PUSH_LOCK:
Expand Down
8 changes: 8 additions & 0 deletions src/controller_rpc.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,10 @@ controller_connect(clixon_handle h,
goto done;
}
}
if (xmldb_db_reset(h, "tmpdev") < 0) /* Requires root access */
goto done;
if (xmldb_copy(h, "running", "tmpdev") < 0)
goto done;
/* Point of no return: assume errors handled in device_input_cb */
device_handle_tid_set(dh, ct->ct_id);
if (connect_netconf_ssh(h, dh, user, addr, ssh_stricthostkey) < 0) /* match */
Expand Down Expand Up @@ -462,6 +466,10 @@ rpc_config_pull(clixon_handle h,
if (ret == 0) /* Failed but cbret set */
goto ok;
} /* for */
if (xmldb_db_reset(h, "tmpdev") < 0) /* Requires root access */
goto done;
if (xmldb_copy(h, "running", "tmpdev") < 0)
goto done;
cprintf(cbret, "<rpc-reply xmlns=\"%s\">", NETCONF_BASE_NAMESPACE);
cprintf(cbret, "<tid xmlns=\"%s\">%" PRIu64"</tid>", CONTROLLER_NAMESPACE, ct->ct_id);
cprintf(cbret, "</rpc-reply>");
Expand Down
4 changes: 2 additions & 2 deletions test/reset-controller.sh
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,10 @@ ret=$(${clixon_netconf} -q0 -f $CFG <<EOF
</rpc>]]>]]>
EOF
)

#echo "ret:$ret"
match=$(echo "$ret" | grep --null -Eo "<rpc-error>") || true
if [ -n "$match" ]; then
err1 "netconf open rpc-error detected"
err "OK" "$ret"
fi

sleep $sleep
Expand Down
206 changes: 206 additions & 0 deletions test/test-c-editconfig.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
#!/usr/bin/env bash
#
# Simple C / non-python service checking shared object create and delete
# Uses util/controller_service.c as C-based server
# Keep non-device config after pull, see https://github.com/clicon/clixon-controller/issues/119
#

# Magic line must be first in script (see README.md)
s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi

if [ $nr -lt 2 ]; then
echo "Test requires nr=$nr to be greater than 1"
if [ "$s" = $0 ]; then exit 0; else return 0; fi
fi

dir=/var/tmp/$0
CFG=$dir/controller.xml
CFD=$dir/confdir
test -d $dir || mkdir -p $dir
test -d $CFD || mkdir -p $CFD

fyang=$dir/myyang.yang

: ${clixon_controller_xpath:=clixon_controller_xpath}

# source IMG/USER etc
. ./site.sh

# Common NACM scripts
. ./nacm.sh

cat<<EOF > $CFG
<clixon-config xmlns="http://clicon.org/config">
<CLICON_CONFIGFILE>$CFG</CLICON_CONFIGFILE>
<CLICON_CONFIGDIR>$CFD</CLICON_CONFIGDIR>
<CLICON_FEATURE>ietf-netconf:startup</CLICON_FEATURE>
<CLICON_FEATURE>clixon-restconf:allow-auth-none</CLICON_FEATURE>
<CLICON_CONFIG_EXTEND>clixon-controller-config</CLICON_CONFIG_EXTEND>
<CLICON_YANG_DIR>${YANG_INSTALLDIR}</CLICON_YANG_DIR>
<CLICON_YANG_MAIN_DIR>$dir</CLICON_YANG_MAIN_DIR>
<CLICON_CLI_MODE>operation</CLICON_CLI_MODE>
<CLICON_CLI_DIR>${LIBDIR}/controller/cli</CLICON_CLI_DIR>
<CLICON_CLISPEC_DIR>${LIBDIR}/controller/clispec</CLICON_CLISPEC_DIR>
<CLICON_BACKEND_DIR>${LIBDIR}/controller/backend</CLICON_BACKEND_DIR>
<CLICON_SOCK>${LOCALSTATEDIR}/run/controller.sock</CLICON_SOCK>
<CLICON_SOCK_GROUP>${CLICON_GROUP}</CLICON_SOCK_GROUP>
<CLICON_SOCK_PRIO>true</CLICON_SOCK_PRIO>
<CLICON_BACKEND_PIDFILE>${LOCALSTATEDIR}/run/controller.pid</CLICON_BACKEND_PIDFILE>
<CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
<CLICON_XMLDB_MULTI>true</CLICON_XMLDB_MULTI>
<CLICON_STARTUP_MODE>init</CLICON_STARTUP_MODE>
<CLICON_STREAM_DISCOVERY_RFC5277>true</CLICON_STREAM_DISCOVERY_RFC5277>
<CLICON_RESTCONF_USER>${CLICON_USER}</CLICON_RESTCONF_USER>
<CLICON_RESTCONF_PRIVILEGES>drop_perm</CLICON_RESTCONF_PRIVILEGES>
<CLICON_RESTCONF_INSTALLDIR>${SBINDIR}</CLICON_RESTCONF_INSTALLDIR>
<CLICON_BACKEND_USER>${CLICON_USER}</CLICON_BACKEND_USER>
<CLICON_VALIDATE_STATE_XML>true</CLICON_VALIDATE_STATE_XML>
<CLICON_CLI_HELPSTRING_TRUNCATE>true</CLICON_CLI_HELPSTRING_TRUNCATE>
<CLICON_CLI_HELPSTRING_LINES>1</CLICON_CLI_HELPSTRING_LINES>
<CLICON_CLI_OUTPUT_FORMAT>text</CLICON_CLI_OUTPUT_FORMAT>
<CLICON_YANG_SCHEMA_MOUNT>true</CLICON_YANG_SCHEMA_MOUNT>
<CLICON_NACM_CREDENTIALS>exact</CLICON_NACM_CREDENTIALS>
<CLICON_NACM_MODE>internal</CLICON_NACM_MODE>
<CLICON_NACM_DISABLED_ON_EMPTY>true</CLICON_NACM_DISABLED_ON_EMPTY>
</clixon-config>
EOF

cat<<EOF > $CFD/autocli.xml
<clixon-config xmlns="http://clicon.org/config">
<autocli>
<module-default>false</module-default>
<list-keyword-default>kw-nokey</list-keyword-default>
<treeref-state-default>true</treeref-state-default>
<grouping-treeref>true</grouping-treeref>
<rule>
<name>include controller</name>
<module-name>clixon-controller</module-name>
<operation>enable</operation>
</rule>
<rule>
<name>include openconfig</name>
<module-name>openconfig*</module-name>
<operation>enable</operation>
</rule>
<!-- there are many more arista/openconfig top-level modules -->
</autocli>
</clixon-config>
EOF

cat <<EOF > $fyang
module myyang {
yang-version 1.1;
namespace "urn:example:test";
prefix test;
import clixon-controller {
prefix ctrl;
}
revision 2023-03-22{
description "Initial prototype";
}
augment "/ctrl:services" {
list testA {
description "Test A service";
key a_name;
leaf a_name {
description "Test A instance";
type string;
}
leaf-list params{
type string;
min-elements 1; /* For validate fail*/
}
uses ctrl:created-by-service;
}
}
augment "/ctrl:services" {
list testB {
description "Test B service";
key b_name;
leaf b_name {
description "Test B instance";
type string;
}
leaf-list params{
type string;
}
uses ctrl:created-by-service;
}
}
}
EOF

RULES=$(cat <<EOF
<nacm xmlns="urn:ietf:params:xml:ns:yang:ietf-netconf-acm">
<enable-nacm>true</enable-nacm>
<read-default>permit</read-default>
<write-default>permit</write-default>
<exec-default>permit</exec-default>
$NGROUPS
$NADMIN
</nacm>
EOF
)

# Start from startup which by default should start it
# First disable services process
cat <<EOF > $dir/startup_db
<config>
<processes xmlns="http://clicon.org/controller">
<services>
<enabled>true</enabled>
</services>
</processes>
$RULES
</config>
EOF

cat<<EOF > $CFD/action-command.xml
<clixon-config xmlns="http://clicon.org/config">
<CONTROLLER_ACTION_COMMAND xmlns="http://clicon.org/controller-config">${BINDIR}/clixon_controller_service -f $CFG</CONTROLLER_ACTION_COMMAND>
</clixon-config>
EOF

# Reset devices with initial config
. ./reset-devices.sh

if $BE; then
new "Kill old backend"
stop_backend -f $CFG
fi
if $BE; then
new "Start new backend -s startup -f $CFG -D $DBG"
sudo clixon_backend -s startup -f $CFG -D $DBG
fi

new "Wait backend 1"
wait_backend

# Reset controller by initiating with clixon/openconfig devices and a pull
. ./reset-controller.sh

new "close connections"
expectpart "$(${clixon_cli} -1f $CFG connect close)" 0 ""

new "Set service"
expectpart "$(${clixon_cli} -1f $CFG -m configure set services testA bar params AA)" 0 ""

new "Compare service"
expectpart "$($clixon_cli -1 -f $CFG -m configure show compare xml)" 0 "^+\ *<testA xmlns=\"urn:example:test\">" "^+\ *<a_name>bar</a_name>" "^+\ *<params>AA</params>"

new "open connections"
expectpart "$(${clixon_cli} -1f $CFG connect open wait)" 0 ""

new "Compare service again"
expectpart "$($clixon_cli -1 -f $CFG -m configure show compare xml)" 0 "^+\ *<testA xmlns=\"urn:example:test\">" "^+\ *<a_name>bar</a_name>" "^+\ *<params>AA</params>"

if $BE; then
new "Kill old backend"
stop_backend -f $CFG
fi

endtest
Loading

0 comments on commit 971e2e9

Please sign in to comment.