Rework datastores
DatastoreAccess now has "targets". The target determines what datastores
are used for writing and reading data. There are three targets:
Operational:
- reads from operational, writes to running
Startup:
- reads from startup, writes to startup
Running:
- reads from running, writes to running
(the datastores types above correspond to the NMDA datastore types)
The need to define targets instead of just calling them "datastores" is
that the operational target doesn't actually use the operational
datastore for both writing and reading.
Change-Id: Iffb7034c27aba42d10d715e23a2c970031b9bd1b
diff --git a/src/cli.cpp b/src/cli.cpp
index 3805dbc..0635c28 100644
--- a/src/cli.cpp
+++ b/src/cli.cpp
@@ -22,12 +22,12 @@
static const auto usage = R"(CLI interface to sysrepo
Usage:
- sysrepo-cli [-d <datastore>]
+ sysrepo-cli [-d <datastore_target>]
sysrepo-cli (-h | --help)
sysrepo-cli --version
Options:
- -d <datastore> can be "running" or "startup" [default: running])";
+ -d <datastore_target> can be "running", "startup" or "operational" [default: operational])";
#elif defined(YANG_CLI)
#include <boost/spirit/home/x3.hpp>
#include <filesystem>
@@ -54,15 +54,15 @@
static const auto usage = R"(CLI interface for NETCONF
Usage:
- netconf-cli [-v] [-p <port>] <host>
- netconf-cli [-v] --socket <path>
+ netconf-cli [-v] [-d <datastore_target>] [-p <port>] <host>
+ netconf-cli [-v] [-d <datastore_target>] --socket <path>
netconf-cli (-h | --help)
netconf-cli --version
Options:
-v enable verbose mode
-p <port> port number [default: 830]
-)";
+ -d <datastore_target> can be "running", "startup" or "operational" [default: operational])";
#include "cli-netconf.hpp"
#include "netconf_access.hpp"
#define PROGRAM_NAME "netconf-cli"
@@ -95,20 +95,26 @@
Replxx lineEditor;
std::atomic<int> backendReturnCode = 0;
-#if defined(SYSREPO_CLI)
- auto datastoreType = Datastore::Running;
+ // For some reason, GCC10 still needs [[maybe_unused]] because of conditional compilation...
+ [[maybe_unused]] auto datastoreTarget = DatastoreTarget::Operational;
if (const auto& ds = args["-d"]) {
if (ds.asString() == "startup") {
- datastoreType = Datastore::Startup;
+ datastoreTarget = DatastoreTarget::Startup;
} else if (ds.asString() == "running") {
- datastoreType = Datastore::Running;
+ datastoreTarget = DatastoreTarget::Running;
+ } else if (ds.asString() == "operational") {
+ datastoreTarget = DatastoreTarget::Operational;
} else {
- std::cerr << PROGRAM_NAME << ": unknown datastore: " << ds.asString() << "\n";
+ std::cerr << PROGRAM_NAME << ": unknown datastore target: " << ds.asString() << "\n";
return 1;
}
}
- auto datastore = std::make_shared<SysrepoAccess>(datastoreType);
- std::cout << "Connected to sysrepo [datastore: " << (datastoreType == Datastore::Startup ? "startup" : "running") << "]" << std::endl;
+
+ auto datastoreTargetString = args["-d"] ? args["-d"].asString() : std::string("operational");
+
+#if defined(SYSREPO_CLI)
+ auto datastore = std::make_shared<SysrepoAccess>();
+ std::cout << "Connected to sysrepo [datastore target: " << datastoreTargetString << "]" << std::endl;
#elif defined(YANG_CLI)
auto datastore = std::make_shared<YangAccess>();
if (args["--configonly"].asBool()) {
@@ -189,10 +195,13 @@
return 1;
}
}
+ std::cout << "Connected via NETCONF [datastore target: " << datastoreTargetString << "]" << std::endl;
#else
#error "Unknown CLI backend"
#endif
+ datastore->setTarget(datastoreTarget);
+
#if defined(SYSREPO_CLI) || defined(NETCONF_CLI)
auto createTemporaryDatastore = [](const std::shared_ptr<DatastoreAccess>& datastore) {
return std::make_shared<YangAccess>(std::static_pointer_cast<YangSchema>(datastore->schema()));
diff --git a/src/datastore_access.cpp b/src/datastore_access.cpp
index 9ca931f..eba90e0 100644
--- a/src/datastore_access.cpp
+++ b/src/datastore_access.cpp
@@ -31,6 +31,11 @@
}
}
+void DatastoreAccess::setTarget(const DatastoreTarget target)
+{
+ m_target = target;
+}
+
const char* DatastoreException::what() const noexcept
{
return m_what.c_str();
diff --git a/src/datastore_access.hpp b/src/datastore_access.hpp
index b29e152..3012a7e 100644
--- a/src/datastore_access.hpp
+++ b/src/datastore_access.hpp
@@ -37,6 +37,12 @@
std::string m_what;
};
+enum class DatastoreTarget {
+ Startup,
+ Running,
+ Operational
+};
+
class Schema;
struct dataPath_;
@@ -51,6 +57,7 @@
virtual void deleteItem(const std::string& path) = 0;
virtual void moveItem(const std::string& path, std::variant<yang::move::Absolute, yang::move::Relative> move) = 0;
virtual Tree execute(const std::string& path, const Tree& input) = 0;
+ void setTarget(const DatastoreTarget target);
virtual std::shared_ptr<Schema> schema() = 0;
@@ -59,6 +66,9 @@
virtual void copyConfig(const Datastore source, const Datastore destination) = 0;
[[nodiscard]] virtual std::string dump(const DataFormat format) const = 0;
+protected:
+ DatastoreTarget m_target = DatastoreTarget::Operational;
+
private:
friend class DataQuery;
virtual std::vector<ListInstance> listInstances(const std::string& path) = 0;
diff --git a/src/netconf_access.cpp b/src/netconf_access.cpp
index ecd337e..3405fe3 100644
--- a/src/netconf_access.cpp
+++ b/src/netconf_access.cpp
@@ -15,11 +15,38 @@
NetconfAccess::~NetconfAccess() = default;
+namespace {
+auto targetToDs_get(const DatastoreTarget target)
+{
+ switch (target) {
+ case DatastoreTarget::Operational:
+ return libnetconf::NmdaDatastore::Operational;
+ case DatastoreTarget::Running:
+ return libnetconf::NmdaDatastore::Running;
+ case DatastoreTarget::Startup:
+ return libnetconf::NmdaDatastore::Startup;
+ }
+ __builtin_unreachable();
+}
+
+auto targetToDs_set(const DatastoreTarget target)
+{
+ switch (target) {
+ case DatastoreTarget::Operational:
+ case DatastoreTarget::Running:
+ return libnetconf::NmdaDatastore::Candidate;
+ case DatastoreTarget::Startup:
+ return libnetconf::NmdaDatastore::Startup;
+ }
+
+ __builtin_unreachable();
+}
+}
DatastoreAccess::Tree NetconfAccess::getItems(const std::string& path) const
{
Tree res;
- auto config = m_session->getData(libnetconf::NmdaDatastore::Operational, (path != "/") ? std::optional{path} : std::nullopt);
+ auto config = m_session->getData(targetToDs_get(m_target), (path != "/") ? std::optional{path} : std::nullopt);
if (config) {
lyNodesToTree(res, config->tree_for());
@@ -119,7 +146,7 @@
void NetconfAccess::doEditFromDataNode(std::shared_ptr<libyang::Data_Node> dataNode)
{
auto data = dataNode->print_mem(LYD_XML, 0);
- m_session->editData(libnetconf::NmdaDatastore::Candidate, data);
+ m_session->editData(targetToDs_set(m_target), data);
}
void NetconfAccess::commitChanges()
diff --git a/src/sysrepo_access.cpp b/src/sysrepo_access.cpp
index 6bcb6f0..30a3e25 100644
--- a/src/sysrepo_access.cpp
+++ b/src/sysrepo_access.cpp
@@ -77,28 +77,56 @@
__builtin_unreachable();
}
-SysrepoAccess::SysrepoAccess(const Datastore datastore)
+SysrepoAccess::SysrepoAccess()
: m_connection(std::make_shared<sysrepo::Connection>())
, m_session(std::make_shared<sysrepo::Session>(m_connection))
, m_schema(std::make_shared<YangSchema>(m_session->get_context()))
{
try {
- m_session = std::make_shared<sysrepo::Session>(m_connection, toSrDatastore(datastore));
+ m_session = std::make_shared<sysrepo::Session>(m_connection);
} catch (sysrepo::sysrepo_exception& ex) {
reportErrors();
}
}
+namespace {
+auto targetToDs_get(const DatastoreTarget target)
+{
+ switch (target) {
+ case DatastoreTarget::Operational:
+ return SR_DS_OPERATIONAL;
+ case DatastoreTarget::Running:
+ return SR_DS_RUNNING;
+ case DatastoreTarget::Startup:
+ return SR_DS_STARTUP;
+ }
+
+ __builtin_unreachable();
+}
+
+auto targetToDs_set(const DatastoreTarget target)
+{
+ switch (target) {
+ case DatastoreTarget::Operational:
+ case DatastoreTarget::Running:
+ // TODO: Doing candidate here doesn't work, why?
+ return SR_DS_RUNNING;
+ case DatastoreTarget::Startup:
+ return SR_DS_STARTUP;
+ }
+
+ __builtin_unreachable();
+}
+}
+
DatastoreAccess::Tree SysrepoAccess::getItems(const std::string& path) const
{
using namespace std::string_literals;
Tree res;
try {
- auto oldDs = m_session->session_get_ds();
- m_session->session_switch_ds(SR_DS_OPERATIONAL);
+ m_session->session_switch_ds(targetToDs_get(m_target));
auto config = m_session->get_data(((path == "/") ? "/*" : path).c_str());
- m_session->session_switch_ds(oldDs);
if (config) {
lyNodesToTree(res, config->tree_for());
}
@@ -111,6 +139,7 @@
void SysrepoAccess::setLeaf(const std::string& path, leaf_data_ value)
{
try {
+ m_session->session_switch_ds(targetToDs_set(m_target));
m_session->set_item(path.c_str(), boost::apply_visitor(valFromValue(), value));
} catch (sysrepo::sysrepo_exception& ex) {
reportErrors();
@@ -120,6 +149,7 @@
void SysrepoAccess::createItem(const std::string& path)
{
try {
+ m_session->session_switch_ds(targetToDs_set(m_target));
m_session->set_item(path.c_str());
} catch (sysrepo::sysrepo_exception& ex) {
reportErrors();
@@ -131,6 +161,7 @@
try {
// Have to use SR_EDIT_ISOLATE, because deleting something that's been set without committing is not supported
// https://github.com/sysrepo/sysrepo/issues/1967#issuecomment-625085090
+ m_session->session_switch_ds(targetToDs_set(m_target));
m_session->delete_item(path.c_str(), SR_EDIT_ISOLATE);
} catch (sysrepo::sysrepo_exception& ex) {
reportErrors();
@@ -164,12 +195,14 @@
destination = instanceToString(relative.m_path);
}
}
+ m_session->session_switch_ds(targetToDs_set(m_target));
m_session->move_item(source.c_str(), toSrMoveOp(move), destination.c_str(), destination.c_str());
}
void SysrepoAccess::commitChanges()
{
try {
+ m_session->session_switch_ds(targetToDs_set(m_target));
m_session->apply_changes(OPERATION_TIMEOUT_MS, 1);
} catch (sysrepo::sysrepo_exception& ex) {
reportErrors();
@@ -179,6 +212,7 @@
void SysrepoAccess::discardChanges()
{
try {
+ m_session->session_switch_ds(targetToDs_set(m_target));
m_session->discard_changes();
} catch (sysrepo::sysrepo_exception& ex) {
reportErrors();
@@ -188,16 +222,15 @@
DatastoreAccess::Tree SysrepoAccess::execute(const std::string& path, const Tree& input)
{
auto inputNode = treeToRpcInput(m_session->get_context(), path, input);
+ m_session->session_switch_ds(targetToDs_set(m_target));
auto output = m_session->rpc_send(inputNode);
return rpcOutputToTree(path, output);
}
void SysrepoAccess::copyConfig(const Datastore source, const Datastore destination)
{
- auto oldDs = m_session->session_get_ds();
m_session->session_switch_ds(toSrDatastore(destination));
m_session->copy_config(toSrDatastore(source), nullptr, OPERATION_TIMEOUT_MS, 1);
- m_session->session_switch_ds(oldDs);
}
std::shared_ptr<Schema> SysrepoAccess::schema()
diff --git a/src/sysrepo_access.hpp b/src/sysrepo_access.hpp
index 55e8ac6..7a02d8b 100644
--- a/src/sysrepo_access.hpp
+++ b/src/sysrepo_access.hpp
@@ -27,7 +27,7 @@
class SysrepoAccess : public DatastoreAccess {
public:
~SysrepoAccess() override;
- SysrepoAccess(const Datastore datastore);
+ SysrepoAccess();
[[nodiscard]] Tree getItems(const std::string& path) const override;
void setLeaf(const std::string& path, leaf_data_ value) override;
void createItem(const std::string& path) override;
diff --git a/tests/data_query.cpp b/tests/data_query.cpp
index eb8e838..64250da 100644
--- a/tests/data_query.cpp
+++ b/tests/data_query.cpp
@@ -37,7 +37,7 @@
SysrepoSubscription subscriptionOther("other-module");
#ifdef sysrepo_BACKEND
- SysrepoAccess datastore(Datastore::Running);
+ SysrepoAccess datastore;
#elif defined(netconf_BACKEND)
const auto NETOPEER_SOCKET = getenv("NETOPEER_SOCKET");
NetconfAccess datastore(NETOPEER_SOCKET);
diff --git a/tests/datastore_access.cpp b/tests/datastore_access.cpp
index 84c6705..76e4f7f 100644
--- a/tests/datastore_access.cpp
+++ b/tests/datastore_access.cpp
@@ -124,7 +124,7 @@
SysrepoSubscription subStartup("example-schema", &mockStartup, SR_DS_STARTUP);
#ifdef sysrepo_BACKEND
- SysrepoAccess datastore(Datastore::Running);
+ SysrepoAccess datastore;
#elif defined(netconf_BACKEND)
const auto NETOPEER_SOCKET = getenv("NETOPEER_SOCKET");
NetconfAccess datastore(NETOPEER_SOCKET);
@@ -915,7 +915,7 @@
trompeloeil::sequence seq1;
#ifdef sysrepo_BACKEND
- auto datastore = std::make_shared<SysrepoAccess>(Datastore::Running);
+ auto datastore = std::make_shared<SysrepoAccess>();
#elif defined(netconf_BACKEND)
const auto NETOPEER_SOCKET = getenv("NETOPEER_SOCKET");
auto datastore = std::make_shared<NetconfAccess>(NETOPEER_SOCKET);
@@ -1069,3 +1069,72 @@
waitForCompletionAndBitMore(seq1);
}
+
+#if not defined(yang_BACKEND)
+TEST_CASE("datastore targets")
+{
+ const auto testNode = "/example-schema:leafInt32";
+ {
+ auto conn = std::make_shared<sysrepo::Connection>();
+ auto sess = std::make_shared<sysrepo::Session>(conn);
+ sess->delete_item(testNode);
+ sess->apply_changes(1000, 1);
+ sess->session_switch_ds(SR_DS_STARTUP);
+ sess->delete_item(testNode);
+ sess->apply_changes(1000, 1);
+ }
+ MockRecorder mockRunning;
+ MockRecorder mockStartup;
+
+#ifdef sysrepo_BACKEND
+ SysrepoAccess datastore;
+#elif defined(netconf_BACKEND)
+ const auto NETOPEER_SOCKET = getenv("NETOPEER_SOCKET");
+ NetconfAccess datastore(NETOPEER_SOCKET);
+#else
+#error "Unknown backend"
+#endif
+
+ auto testGetItems = [&datastore] (const auto& path, const DatastoreAccess::Tree& expected) {
+ REQUIRE(datastore.getItems(path) == expected);
+ };
+
+ SECTION("subscriptions change operational target")
+ {
+ // Default target is operational, so setting a value and reading it should return no values as there are no
+ // subscriptions yet.
+ datastore.setLeaf(testNode, 10);
+ datastore.commitChanges();
+ testGetItems(testNode, {});
+
+ // Now we create a subscription and try again.
+ SysrepoSubscription subRunning("example-schema", &mockRunning);
+ testGetItems(testNode, {{testNode, 10}});
+ }
+
+ SECTION("running shows stuff even without subscriptions")
+ {
+ datastore.setTarget(DatastoreTarget::Running);
+ datastore.setLeaf(testNode, 10);
+ datastore.commitChanges();
+ testGetItems(testNode, {{testNode, 10}});
+
+ // Actually creating a subscription shouldn't make a difference.
+ SysrepoSubscription subRunning("example-schema", &mockRunning);
+ testGetItems(testNode, {{testNode, 10}});
+ }
+
+ SECTION("startup changes only affect startup")
+ {
+ datastore.setTarget(DatastoreTarget::Startup);
+ datastore.setLeaf(testNode, 10);
+ datastore.commitChanges();
+ testGetItems(testNode, {{testNode, 10}});
+ datastore.setTarget(DatastoreTarget::Running);
+ testGetItems(testNode, {});
+ datastore.setTarget(DatastoreTarget::Operational);
+ testGetItems(testNode, {});
+ }
+
+}
+#endif