From 5999b724cacc2766a53bb34c47382030e6eda2f2 Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Thu, 15 Jul 2021 17:25:47 +0300 Subject: [PATCH] [#1077] migrate src/bin/netconf to sysrepo 1.4 --- src/bin/netconf/http_control_socket.cc | 3 - src/bin/netconf/http_control_socket.h | 4 +- src/bin/netconf/netconf.cc | 396 +++++++++--------- src/bin/netconf/netconf.h | 81 ++-- src/bin/netconf/netconf_cfg_mgr.cc | 3 - src/bin/netconf/netconf_cfg_mgr.h | 2 +- src/bin/netconf/netconf_config.cc | 15 +- src/bin/netconf/netconf_config.h | 29 +- src/bin/netconf/netconf_controller.cc | 4 +- src/bin/netconf/netconf_controller.h | 4 +- src/bin/netconf/netconf_messages.cc | 10 +- src/bin/netconf/netconf_messages.h | 3 + src/bin/netconf/netconf_messages.mes | 18 +- src/bin/netconf/netconf_process.cc | 21 +- src/bin/netconf/netconf_process.h | 9 +- src/bin/netconf/simple_parser.cc | 16 +- src/bin/netconf/stdout_control_socket.cc | 5 +- src/bin/netconf/stdout_control_socket.h | 4 +- .../netconf/tests/control_socket_unittests.cc | 25 +- src/bin/netconf/tests/get_config_unittest.cc | 12 +- .../tests/netconf_cfg_mgr_unittests.cc | 8 +- src/bin/netconf/tests/netconf_unittests.cc | 378 +++++++++-------- .../netconf/tests/shtests/netconf_tests.sh.in | 2 +- src/bin/netconf/unix_control_socket.cc | 5 +- src/bin/netconf/unix_control_socket.h | 4 +- 25 files changed, 526 insertions(+), 535 deletions(-) diff --git a/src/bin/netconf/http_control_socket.cc b/src/bin/netconf/http_control_socket.cc index dbe46440ea..ec1f9ae569 100644 --- a/src/bin/netconf/http_control_socket.cc +++ b/src/bin/netconf/http_control_socket.cc @@ -37,9 +37,6 @@ HttpControlSocket::HttpControlSocket(CfgControlSocketPtr ctrl_sock) : ControlSocketBase(ctrl_sock) { } -HttpControlSocket::~HttpControlSocket() { -} - ConstElementPtr HttpControlSocket::configGet(const string& service) { if (service == "ca") { diff --git a/src/bin/netconf/http_control_socket.h b/src/bin/netconf/http_control_socket.h index 48023733ce..74c57613d3 100644 --- a/src/bin/netconf/http_control_socket.h +++ b/src/bin/netconf/http_control_socket.h @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -29,7 +29,7 @@ public: HttpControlSocket(CfgControlSocketPtr ctrl_sock); /// @brief Destructor (does nothing). - virtual ~HttpControlSocket(); + virtual ~HttpControlSocket() = default; /// @brief Get configuration. /// diff --git a/src/bin/netconf/netconf.cc b/src/bin/netconf/netconf.cc index ef27979322..4e87399844 100644 --- a/src/bin/netconf/netconf.cc +++ b/src/bin/netconf/netconf.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -15,7 +15,9 @@ #include #include #include + #include + #include using namespace std; @@ -23,14 +25,15 @@ using namespace isc::config; using namespace isc::data; using namespace isc::netconf; using namespace isc::yang; -#ifndef HAVE_PRE_0_7_6_SYSREPO using namespace sysrepo; -#endif + +using libyang::S_Context; +using libyang::S_Module; namespace { /// @brief Module change subscription callback. -class NetconfAgentCallback : public Callback { +class NetconfAgentCallback { public: /// @brief Constructor. /// @@ -40,7 +43,7 @@ public: } /// @brief Server name and configuration pair. - CfgServersMapPair service_pair_; + CfgServersMapPair const service_pair_; /// @brief Module configuration change callback. /// @@ -52,26 +55,32 @@ public: /// @param event The event. /// @param private_ctx The private context. /// @return the sysrepo return code. - int module_change(S_Session sess, - const char* /*module_name*/, - sr_notif_event_t event, - void* /*private_ctx*/) { - if (NetconfProcess::shut_down) { - return (SR_ERR_DISCONNECT); - } + sr_error_t module_change(S_Session sess, + const char* module_name, + const char* /* xpath */, + sr_event_t event, + void* /* private_ctx */) { ostringstream event_type; switch (event) { - case SR_EV_VERIFY: - event_type << "VERIFY"; + case SR_EV_UPDATE: + // This could potentially be a hook point for mid-flight + // configuration changes. + event_type << "SR_EV_UPDATE"; break; - case SR_EV_APPLY: - event_type << "APPLY"; + case SR_EV_CHANGE: + event_type << "SR_EV_CHANGE"; + break; + case SR_EV_DONE: + event_type << "SR_EV_DONE"; break; case SR_EV_ABORT: - event_type << "ABORT"; + event_type << "SR_EV_ABORT"; break; case SR_EV_ENABLED: - event_type << "ENABLED"; + event_type << "SR_EV_ENABLED"; + break; + case SR_EV_RPC: + event_type << "SR_EV_RPC"; break; default: event_type << "UNKNOWN (" << event << ")"; @@ -79,44 +88,36 @@ public: } LOG_INFO(netconf_logger, NETCONF_CONFIG_CHANGE_EVENT) .arg(event_type.str()); - string xpath = "/" + service_pair_.second->getModel() + ":"; - NetconfAgent::logChanges(sess, xpath + "config"); - if (NetconfProcess::shut_down) { - return (SR_ERR_DISCONNECT); - } + NetconfAgent::logChanges(sess, module_name); switch (event) { - case SR_EV_VERIFY: - return (NetconfAgent::validate(sess, service_pair_)); - case SR_EV_APPLY: - return (NetconfAgent::update(sess, service_pair_)); + case SR_EV_CHANGE: + return (NetconfAgent::change(sess, service_pair_)); + case SR_EV_DONE: + return (NetconfAgent::done(sess, service_pair_)); default: return (SR_ERR_OK); } } -}; -/// @brief Module (un)installation subscription callback. -class NetconfAgentInstallCallback : public Callback { -public: - /// @brief Module (un)installation callback. - /// - /// This callback is called by sysrepo when a module is (un)installed. - /// - /// @param module_name The module name. - /// @param revision The module revision (NULL for uninstallation). - /// @param state The new state of the module (ignored). - /// @param private_ctx The private context. - void module_install(const char* module_name, - const char* revision, - sr_module_state_t /*state*/, - void* /*private_ctx*/) { - if (!module_name) { - // Not for us... - return; + void event_notif(sysrepo::S_Session session, + sr_ev_notif_type_t const notification_type, + char const* path, + sysrepo::S_Vals const vals, + time_t timestamp, + void* private_data) { + LOG_INFO(netconf_logger, NETCONF_INFO).arg("======== notification received ========"); + for (size_t i(0); i < vals->val_cnt(); ++i) { + LOG_INFO(netconf_logger, NETCONF_INFO).arg(vals->val(i)->xpath()); } - LOG_WARN(netconf_logger, NETCONF_MODULE_INSTALL) - .arg(module_name) - .arg(revision ? revision : "unknown"); + } + + void event_notif_tree(sysrepo::S_Session session, + sr_ev_notif_type_t const notif_type, + libyang::S_Data_Node const notification, + time_t timestamp, + void* private_data) { + LOG_INFO(netconf_logger, NETCONF_INFO).arg("======== notification tree received ========"); + LOG_INFO(netconf_logger, NETCONF_INFO).arg(notification->print_mem(LYD_XML, LYP_FORMAT)); } }; @@ -125,86 +126,39 @@ public: namespace isc { namespace netconf { -NetconfAgent::NetconfAgent() { -} - NetconfAgent::~NetconfAgent() { clear(); } void NetconfAgent::init(NetconfCfgMgrPtr cfg_mgr) { - // If there is no configuration manager and/or we're shutting down. - if (!cfg_mgr || NetconfProcess::shut_down) { + // Check for a configuration manager. + if (!cfg_mgr) { + isc_throw(BadValue, "missing configuration for kea-netconf"); return; } const CfgServersMapPtr& servers = cfg_mgr->getNetconfConfig()->getCfgServersMap(); - for (auto pair : *servers) { - if (NetconfProcess::shut_down) { - return; - } - + for (auto const& pair : *servers) { // Retrieve configuration from existing running DHCP daemons. keaConfig(pair); - if (NetconfProcess::shut_down) { - return; - } - } - if (NetconfProcess::shut_down) { - return; } - // Initialize sysrepo interface. + // Initialize sysrepo. initSysrepo(); - if (NetconfProcess::shut_down) { - return; - } - - // Check essential modules / revisions. - bool can_start = true; - for (auto pair : *servers) { - can_start = can_start && checkModule(pair.second->getModel()); - if (NetconfProcess::shut_down) { - return; - } - } - if (!can_start) { - cerr << "An essential YANG module / revision is missing." - << endl - << "The environment is not suitable for running kea-netconf." - << endl; - exit(EXIT_FAILURE); - } - if (NetconfProcess::shut_down) { - return; - } // Check modules / revisions. - checkModules(); - if (NetconfProcess::shut_down) { - return; - } + checkModules(servers); - for (auto pair : *servers) { - if (NetconfProcess::shut_down) { - return; - } + for (auto const& pair : *servers) { yangConfig(pair); - if (NetconfProcess::shut_down) { - return; - } subscribeConfig(pair); - if (NetconfProcess::shut_down) { - return; - } + subscribeToNotifications(pair); } } void NetconfAgent::clear() { - // Should be already set to true but in case... - NetconfProcess::shut_down = true; for (auto subs : subscriptions_) { subs.second.reset(); } @@ -251,9 +205,6 @@ NetconfAgent::keaConfig(const CfgServersMapPair& service_pair) { .arg(msg.str()); return; } - if (NetconfProcess::shut_down) { - return; - } if (rcode != CONTROL_RESULT_SUCCESS) { ostringstream msg; msg << "config-get command returned " << answerToText(answer); @@ -277,75 +228,50 @@ NetconfAgent::keaConfig(const CfgServersMapPair& service_pair) { void NetconfAgent::initSysrepo() { try { - conn_.reset(new Connection(NetconfController::netconf_app_name_, - SR_CONN_DAEMON_REQUIRED)); + conn_ = make_shared(); } catch (const std::exception& ex) { isc_throw(Unexpected, "Can't connect to sysrepo: " << ex.what()); } - if (NetconfProcess::shut_down) { - return; - } try { startup_sess_.reset(new Session(conn_, SR_DS_STARTUP)); - if (NetconfProcess::shut_down) { - return; - } running_sess_.reset(new Session(conn_, SR_DS_RUNNING)); } catch (const std::exception& ex) { - isc_throw(Unexpected, "Can't establish a sysrepo session: " + isc_throw(Unexpected, "Can't establish a sysrepo session: " << ex.what()); } - if (NetconfProcess::shut_down) { - return; - } + // Retrieve names and revisions of installed modules from sysrepo. + getModules(); +} + +void NetconfAgent::getModules() { + vector modules; try { - S_Yang_Schemas schemas = startup_sess_->list_schemas(); - for (size_t i = 0; i < schemas->schema_cnt(); ++i) { - if (NetconfProcess::shut_down) { - return; - } - if (!schemas->schema(i) || - !schemas->schema(i)->module_name()) { - // Should not happen: skip it. - continue; - } - string module = schemas->schema(i)->module_name(); - if (!schemas->schema(i)->revision() || - !schemas->schema(i)->revision()->revision()) { - // Our modules have revisions: skip it. - continue; - } - string revision = schemas->schema(i)->revision()->revision(); - modules_.insert(make_pair(module, revision)); - } + S_Context context(running_sess_->get_context()); + modules = context->get_module_iter(); } catch (const sysrepo_exception& ex) { - isc_throw(Unexpected, "Can't list schemas: " << ex.what()); - } - if (NetconfProcess::shut_down) { - return; + cerr << "ERROR: Can't retrieve available modules: " << ex.what() << endl; + exit(1); } - // Subscribe to the module (un)installation callback. - // When a module is (un)installed the callback is called. - // Note this requires a system test (vs. unit test). - try { - S_Subscribe subs(new Subscribe(startup_sess_)); - S_Callback cb(new NetconfAgentInstallCallback()); - subs->module_install_subscribe(cb); - subscriptions_.insert(make_pair("__install__", subs)); - } catch (const sysrepo_exception& ex) { - isc_throw(Unexpected, "Can't subscribe module install: " - << ex.what()); + for (S_Module const& module : modules) { + if (!module->name()) { + cerr << "ERROR: module name is mangled" << endl; + exit(2); + } + string const name(module->name()); + if (!module->rev() || !module->rev()->date()) { + cerr << "ERROR: module revision is mangled" << endl; + exit(3); + } + string const revision(module->rev()->date()); + modules_.emplace(name, revision); } } bool NetconfAgent::checkModule(const string& module_name) const { - if (module_name.empty()) { - return (true); - } auto module = modules_.find(module_name); if (module == modules_.end()) { LOG_ERROR(netconf_logger, NETCONF_MODULE_MISSING_ERR) @@ -370,11 +296,25 @@ NetconfAgent::checkModule(const string& module_name) const { } void -NetconfAgent::checkModules() const { - for (auto modrev : YANG_REVISIONS) { - if (NetconfProcess::shut_down) { - return; +NetconfAgent::checkModules(CfgServersMapPtr const& servers /* = {} */) const { + bool faulty_model(false); + if (servers) { + for (auto pair : *servers) { + if (!checkModule(pair.second->getModel())) { + faulty_model = true; + } } + } + + if (faulty_model) { + cerr << "ERROR: The logged YANG module is missing or its revision is not " + "supported. The environment is not suitable for running " + "kea-netconf." + << endl; + exit(4); + } + + for (auto modrev : YANG_REVISIONS) { auto module = modules_.find(modrev.first); if (module == modules_.end()) { LOG_WARN(netconf_logger, NETCONF_MODULE_MISSING_WARN) @@ -394,8 +334,7 @@ void NetconfAgent::yangConfig(const CfgServersMapPair& service_pair) { // If we're shutting down, or the boot-update flag is not set or the model // associated with it is not specified. - if (NetconfProcess::shut_down || - !service_pair.second->getBootUpdate() || + if (!service_pair.second->getBootUpdate() || service_pair.second->getModel().empty()) { return; } @@ -437,9 +376,6 @@ NetconfAgent::yangConfig(const CfgServersMapPair& service_pair) { .arg(msg.str()); return; } - if (NetconfProcess::shut_down) { - return; - } ControlSocketBasePtr comm; try { comm = createControlSocket(ctrl_sock); @@ -451,9 +387,6 @@ NetconfAgent::yangConfig(const CfgServersMapPair& service_pair) { .arg(msg.str()); return; } - if (NetconfProcess::shut_down) { - return; - } ConstElementPtr answer; int rcode; try { @@ -481,47 +414,102 @@ NetconfAgent::yangConfig(const CfgServersMapPair& service_pair) { void NetconfAgent::subscribeConfig(const CfgServersMapPair& service_pair) { + std::string const& model(service_pair.second->getModel()); + // If we're shutting down, or the subscribe-changes flag is not set or // the model associated with it is not specified. - if (NetconfProcess::shut_down || - !service_pair.second->getSubscribeChanges() || - service_pair.second->getModel().empty()) { + if (!service_pair.second->getSubscribeChanges() || + model.empty()) { return; } LOG_INFO(netconf_logger, NETCONF_SUBSCRIBE_CONFIG) .arg(service_pair.first) - .arg(service_pair.second->getModel()); + .arg(model); S_Subscribe subs(new Subscribe(running_sess_)); - S_Callback cb(new NetconfAgentCallback(service_pair)); + auto callback = [=](sysrepo::S_Session sess, const char* module_name, + const char* xpath, sr_event_t event, + uint32_t /* request_id */) { + NetconfAgentCallback agent(service_pair); + return agent.module_change(sess, module_name, xpath, event, nullptr); + }; try { sr_subscr_options_t options = SR_SUBSCR_DEFAULT; if (!service_pair.second->getValidateChanges()) { - options |= SR_SUBSCR_APPLY_ONLY; + options |= SR_SUBSCR_DONE_ONLY; } // Note the API stores the module name so do not put it // in a short lifetime variable! - subs->module_change_subscribe(service_pair.second->getModel().c_str(), - cb, 0, 0, options); + subs->module_change_subscribe(model.c_str(), callback, nullptr, 0, + options); } catch (const std::exception& ex) { ostringstream msg; msg << "module change subscribe failed with " << ex.what(); + msg << "change subscription for model " << model << + " failed with: " << ex.what(); LOG_ERROR(netconf_logger, NETCONF_SUBSCRIBE_CONFIG_FAILED) .arg(service_pair.first) .arg(service_pair.second->getModel()) .arg(msg.str()); - subs.reset(); return; } subscriptions_.insert(make_pair(service_pair.first, subs)); } -int -NetconfAgent::validate(S_Session sess, const CfgServersMapPair& service_pair) { + +void +NetconfAgent::subscribeToNotifications(const CfgServersMapPair& service_pair) { + std::string const& model(service_pair.second->getModel()); + // If we're shutting down, or the subscribe-changes flag is not set or + // the model associated with it is not specified. + if (!service_pair.second->getSubscribeNotifications() || + model.empty()) { + return; + } + LOG_INFO(netconf_logger, NETCONF_SUBSCRIBE_NOTIFICATIONS) + .arg(service_pair.first) + .arg(model); + + S_Subscribe subscription(std::make_shared(running_sess_)); + auto event_notif_callback = + [=](sysrepo::S_Session session, + sr_ev_notif_type_t const notification_type, char const* path, + sysrepo::S_Vals const vals, time_t timestamp) { + NetconfAgentCallback agent(service_pair); + return agent.event_notif(session, notification_type, path, vals, + timestamp, nullptr); + }; + auto event_notif_tree_callback = + [=](sysrepo::S_Session session, + sr_ev_notif_type_t const notification_type, + libyang::S_Data_Node const notification, time_t timestamp) { + NetconfAgentCallback agent(service_pair); + return agent.event_notif_tree(session, notification_type, + notification, timestamp, nullptr); + }; + try { + subscription->event_notif_subscribe(model.c_str(), + event_notif_callback); + subscription->event_notif_subscribe_tree(model.c_str(), + event_notif_tree_callback); + } catch (const std::exception& ex) { + ostringstream msg; + msg << "event notification subscription for model " << model << + " failed with: " << ex.what(); + LOG_ERROR(netconf_logger, NETCONF_SUBSCRIBE_NOTIFICATIONS_FAILED) + .arg(service_pair.first) + .arg(service_pair.second->getModel()) + .arg(msg.str()); + return; + } + subscriptions_.emplace(service_pair.first, subscription); +} + +sr_error_t +NetconfAgent::change(S_Session sess, const CfgServersMapPair& service_pair) { // If we're shutting down, or the subscribe-changes or the // validate-changes flag is not set or the model associated with // it is not specified. - if (NetconfProcess::shut_down || - !service_pair.second->getSubscribeChanges() || + if (!service_pair.second->getSubscribeChanges() || !service_pair.second->getValidateChanges() || service_pair.second->getModel().empty()) { return (SR_ERR_OK); @@ -544,7 +532,7 @@ NetconfAgent::validate(S_Session sess, const CfgServersMapPair& service_pair) { LOG_ERROR(netconf_logger, NETCONF_VALIDATE_CONFIG_FAILED) .arg(service_pair.first) .arg(msg.str()); - return (SR_ERR_DISCONNECT); + return SR_ERR_OPERATION_FAILED; } else { LOG_DEBUG(netconf_logger, NETCONF_DBG_TRACE_DETAIL_DATA, NETCONF_VALIDATE_CONFIG) @@ -560,9 +548,6 @@ NetconfAgent::validate(S_Session sess, const CfgServersMapPair& service_pair) { .arg(msg.str()); return (SR_ERR_VALIDATION_FAILED);; } - if (NetconfProcess::shut_down) { - return (SR_ERR_DISCONNECT); - } ControlSocketBasePtr comm; try { comm = createControlSocket(ctrl_sock); @@ -600,11 +585,10 @@ NetconfAgent::validate(S_Session sess, const CfgServersMapPair& service_pair) { return (SR_ERR_OK); } -int -NetconfAgent::update(S_Session sess, const CfgServersMapPair& service_pair) { +sr_error_t +NetconfAgent::done(S_Session sess, const CfgServersMapPair& service_pair) { // Check if we should and can process this update. - if (NetconfProcess::shut_down || - !service_pair.second->getSubscribeChanges() || + if (!service_pair.second->getSubscribeChanges() || service_pair.second->getModel().empty()) { return (SR_ERR_OK); } @@ -647,9 +631,6 @@ NetconfAgent::update(S_Session sess, const CfgServersMapPair& service_pair) { .arg(msg.str()); return (SR_ERR_VALIDATION_FAILED); } - if (NetconfProcess::shut_down) { - return (SR_ERR_OK); - } // Ok, now open the control socket. We need this to send the config to // the server. @@ -696,19 +677,16 @@ NetconfAgent::update(S_Session sess, const CfgServersMapPair& service_pair) { void NetconfAgent::logChanges(S_Session sess, const string& model) { - if (NetconfProcess::shut_down) { - return; - } - S_Iter_Change iter = sess->get_changes_iter(model.c_str()); + ostringstream stream; + stream << "/" << model << ":*//."; + std::string const xpath(stream.str()); + S_Iter_Change iter = sess->get_changes_iter(xpath.c_str()); if (!iter) { LOG_WARN(netconf_logger, NETCONF_LOG_CHANGE_FAIL) .arg("no iterator"); return; } for (;;) { - if (NetconfProcess::shut_down) { - return; - } S_Change change; ostringstream msg; try { @@ -723,9 +701,6 @@ NetconfAgent::logChanges(S_Session sess, const string& model) { // End of changes, not an error. return; } - if (NetconfProcess::shut_down) { - return; - } S_Val new_val = change->new_val(); S_Val old_val = change->old_val(); string report; @@ -796,5 +771,22 @@ NetconfAgent::logChanges(S_Session sess, const string& model) { } } +void +NetconfAgent::announceShutdown() const { + isc::process::DControllerBasePtr& controller(NetconfController::instance()); + if (controller) { + boost::dynamic_pointer_cast(controller) + ->getNetconfProcess() + ->setShutdownFlag(true); + } +} + +bool NetconfAgent::shouldShutdown() const { + return boost::dynamic_pointer_cast( + NetconfController::instance()) + ->getNetconfProcess() + ->shouldShutdown(); +} + } // namespace netconf } // namespace isc diff --git a/src/bin/netconf/netconf.h b/src/bin/netconf/netconf.h index 026229015d..b28659472b 100644 --- a/src/bin/netconf/netconf.h +++ b/src/bin/netconf/netconf.h @@ -9,20 +9,14 @@ #ifndef NETCONF_H #define NETCONF_H -#ifndef HAVE_SYSREPO -#error "config.h must be included before netconf.h" -#endif - #include #include #include #include #include -#ifndef HAVE_PRE_0_7_6_SYSREPO + #include -#else -#include -#endif + #include namespace isc { @@ -44,9 +38,6 @@ typedef boost::shared_ptr NetconfAgentPtr; /// - on shutdown close subscriptions. class NetconfAgent { public: - /// @brief Constructor. - NetconfAgent(); - /// @brief Destructor (call clear). virtual ~NetconfAgent(); @@ -63,9 +54,6 @@ public: /// Load Kea server configurations from YANG datastore. /// Subscribe configuration changes in YANG datastore. /// - /// If @c NetconfProcess::global_shut_down_flag becomes true - /// returns as soon as possible. - /// /// @param cfg_mgr The configuration manager (can be null). void init(NetconfCfgMgrPtr cfg_mgr); @@ -74,33 +62,26 @@ public: /// Close subscriptions and sysrepo. void clear(); - /// @brief Validate. + /// @brief SR_EV_CHANGE callback. /// /// Validate YANG datastore changes using Kea configuration test. /// /// @param sess The sysrepo running datastore session. /// @param service_pair The service name and configuration pair. /// @return return code for sysrepo. -#ifndef HAVE_PRE_0_7_6_SYSREPO - static int validate(sysrepo::S_Session sess, - const CfgServersMapPair& service_pair); -#else - static int validate(S_Session sess, const CfgServersMapPair& service_pair); -#endif + static sr_error_t + change(sysrepo::S_Session sess, const CfgServersMapPair& service_pair); - /// @brief Update. + /// @brief SR_EV_DONE callback. /// - /// Update a Kea configuration from YANG datastore changes. + /// Get notified that a Kea configuration has been written to the YANG + /// datastore. /// /// @param sess The sysrepo running datastore session. /// @param service_pair The service name and configuration pair. /// @return return code for sysrepo. -#ifndef HAVE_PRE_0_7_6_SYSREPO - static int update(sysrepo::S_Session sess, - const CfgServersMapPair& service_pair); -#else - static int update(S_Session sess, const CfgServersMapPair& service_pair); -#endif + static sr_error_t + done(sysrepo::S_Session sess, const CfgServersMapPair& service_pair); /// @brief Log changes. /// @@ -110,11 +91,7 @@ public: /// /// @param sess The sysrepo running datastore session. /// @param model The model name. -#ifndef HAVE_PRE_0_7_6_SYSREPO static void logChanges(sysrepo::S_Session sess, const std::string& model); -#else - static void logChanges(S_Session sess, const std::string& model); -#endif protected: /// @brief Get and display Kea server configuration. @@ -136,11 +113,19 @@ protected: /// @return true if available, false if not. bool checkModule(const std::string& module_name) const; + /// @brief Retrieve names and revisions of installed modules through the + /// sysrepo API. + void getModules(); + /// @brief Check module availability. /// /// Emit a warning if a module is missing or does not have /// the expected revision. - void checkModules() const; + /// + /// @param servers the configured servers to check against YANG_REVISIONS. + /// Is empty by default for when the caller only wants to check + /// installed modules. + void checkModules(CfgServersMapPtr const& servers = {}) const; /// @brief Retrieve Kea server configuration from the YANG startup /// datastore and applies it to servers. @@ -160,36 +145,32 @@ protected: /// @param service_pair The service name and configuration pair. void subscribeConfig(const CfgServersMapPair& service_pair); + /// @brief Subscribe to notifications for a given YANG module. + /// + /// @param service_pair the service name and configuration pair + void subscribeToNotifications(const CfgServersMapPair& service_pair); + + /// @brief Set the shutdown flag of the process to true so that it can exit + /// at the earliest convenient time. + void announceShutdown() const; + + /// @brief Check the shutdown flag of the process. + bool shouldShutdown() const; + /// @brief Sysrepo connection. -#ifndef HAVE_PRE_0_7_6_SYSREPO sysrepo::S_Connection conn_; -#else - S_Connection conn_; -#endif /// @brief Sysrepo startup datastore session. -#ifndef HAVE_PRE_0_7_6_SYSREPO sysrepo::S_Session startup_sess_; -#else - S_Session startup_sess_; -#endif /// @brief Sysrepo running datastore session. -#ifndef HAVE_PRE_0_7_6_SYSREPO sysrepo::S_Session running_sess_; -#else - S_Session running_sess_; -#endif /// @brief Available modules and revisions in Sysrepo. std::map modules_; /// @brief Subscription map. -#ifndef HAVE_PRE_0_7_6_SYSREPO std::map subscriptions_; -#else - std::map subscriptions_; -#endif }; } // namespace netconf diff --git a/src/bin/netconf/netconf_cfg_mgr.cc b/src/bin/netconf/netconf_cfg_mgr.cc index ad9b02a490..1f889a73d5 100644 --- a/src/bin/netconf/netconf_cfg_mgr.cc +++ b/src/bin/netconf/netconf_cfg_mgr.cc @@ -50,9 +50,6 @@ NetconfCfgMgr::NetconfCfgMgr() : DCfgMgrBase(ConfigPtr(new NetconfConfig())) { } -NetconfCfgMgr::~NetconfCfgMgr() { -} - std::string NetconfCfgMgr::getConfigSummary(const uint32_t /*selection*/) { diff --git a/src/bin/netconf/netconf_cfg_mgr.h b/src/bin/netconf/netconf_cfg_mgr.h index ef4a082684..ca9e80c52b 100644 --- a/src/bin/netconf/netconf_cfg_mgr.h +++ b/src/bin/netconf/netconf_cfg_mgr.h @@ -128,7 +128,7 @@ public: NetconfCfgMgr(); /// @brief Destructor - virtual ~NetconfCfgMgr(); + virtual ~NetconfCfgMgr() = default; /// @brief Convenience method that returns the Netconf configuration /// context. diff --git a/src/bin/netconf/netconf_config.cc b/src/bin/netconf/netconf_config.cc index 79f68ffa34..f5a05311c1 100644 --- a/src/bin/netconf/netconf_config.cc +++ b/src/bin/netconf/netconf_config.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -33,9 +33,6 @@ CfgControlSocket::CfgControlSocket(Type type, const string& name, : type_(type), name_(name), url_(url) { } -CfgControlSocket::~CfgControlSocket() { -} - CfgControlSocket::Type CfgControlSocket::stringToType(const string& type) { if (type == "unix") { @@ -80,10 +77,8 @@ CfgControlSocket::toElement() const { // *********************** CfgServer ************************* CfgServer::CfgServer(const string& model, CfgControlSocketPtr ctrl_sock) : model_(model), boot_update_(true), subscribe_changes_(true), - validate_changes_(true), control_socket_(ctrl_sock) { -} - -CfgServer::~CfgServer() { + subscribe_notifications_(true), validate_changes_(true), + control_socket_(ctrl_sock) { } string @@ -212,5 +207,5 @@ ServerConfigParser::parse(ConstElementPtr server_config) { return (result); } -}; // end of isc::netconf namespace -}; // end of isc namespace +} // namespace netconf +} // namespace isc diff --git a/src/bin/netconf/netconf_config.h b/src/bin/netconf/netconf_config.h index 7ffaf6141c..d9b6eb24b6 100644 --- a/src/bin/netconf/netconf_config.h +++ b/src/bin/netconf/netconf_config.h @@ -86,7 +86,7 @@ public: const isc::http::Url& url); /// @brief Destructor (doing nothing). - virtual ~CfgControlSocket(); + virtual ~CfgControlSocket() = default; /// @brief Getter which returns the socket type. /// @@ -156,7 +156,7 @@ public: CfgServer(const std::string& model, CfgControlSocketPtr ctrl_sock); /// @brief Destructor (doing nothing). - virtual ~CfgServer(); + virtual ~CfgServer() = default; /// @brief Getter which returns the model name. /// @@ -193,6 +193,13 @@ public: return (subscribe_changes_); } + /// @brief Getter which returns the subscribe-changes flag. + /// + /// @return returns the subscribe-changes flag as a bool. + bool getSubscribeNotifications() const { + return (subscribe_notifications_); + } + /// @brief Set the subscribe-changes flag. /// /// @param subscribe_changes The subscribe-changes flag. @@ -200,6 +207,13 @@ public: subscribe_changes_ = subscribe_changes; } + /// @brief Set the subscribe-changes flag. + /// + /// @param subscribe_changes The subscribe-changes flag. + void setSubscribeNotifications(bool subscribe_notifications) { + subscribe_notifications_ = subscribe_notifications; + } + /// @brief Getter which returns the validate-changes flag. /// /// @return returns the validate-changes flag as a bool. @@ -238,6 +252,13 @@ private: /// so will be notified when the YANG running configuration is changed. bool subscribe_changes_; + /// @brief The subscribe-notifications flag. + /// + /// If true (the default) the netconf agent subscribes to the notifications + /// API so it will be notified on various events like module installations + /// and uninstallations. + bool subscribe_notifications_; + /// @brief The validate-changes flag. /// /// If true (the default) the netconf agent validates module changes @@ -301,7 +322,7 @@ public: CfgServerPtr parse(data::ConstElementPtr server_config); }; -}; // end of isc::netconf namespace -}; // end of isc namespace +} // namespace netconf +} // namespace isc #endif // NETCONF_CONFIG_H diff --git a/src/bin/netconf/netconf_controller.cc b/src/bin/netconf/netconf_controller.cc index 1e79b5f55c..7d85329144 100644 --- a/src/bin/netconf/netconf_controller.cc +++ b/src/bin/netconf/netconf_controller.cc @@ -10,6 +10,7 @@ #include #include #include + #include using namespace isc::process; @@ -62,9 +63,6 @@ NetconfController::NetconfController() : DControllerBase(netconf_app_name_, netconf_bin_name_) { } -NetconfController::~NetconfController() { -} - NetconfProcessPtr NetconfController::getNetconfProcess() { return (boost::dynamic_pointer_cast(getProcess())); diff --git a/src/bin/netconf/netconf_controller.h b/src/bin/netconf/netconf_controller.h index c4e14828cf..b1d8cf674e 100644 --- a/src/bin/netconf/netconf_controller.h +++ b/src/bin/netconf/netconf_controller.h @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -31,7 +31,7 @@ public: static process::DControllerBasePtr& instance(); /// @brief Destructor - virtual ~NetconfController(); + virtual ~NetconfController() = default; /// @brief Returns pointer to an instance of the underlying process object. NetconfProcessPtr getNetconfProcess(); diff --git a/src/bin/netconf/netconf_messages.cc b/src/bin/netconf/netconf_messages.cc index fafab8e285..434e52dd40 100644 --- a/src/bin/netconf/netconf_messages.cc +++ b/src/bin/netconf/netconf_messages.cc @@ -16,6 +16,7 @@ extern const isc::log::MessageID NETCONF_FAILED = "NETCONF_FAILED"; extern const isc::log::MessageID NETCONF_GET_CONFIG = "NETCONF_GET_CONFIG"; extern const isc::log::MessageID NETCONF_GET_CONFIG_FAILED = "NETCONF_GET_CONFIG_FAILED"; extern const isc::log::MessageID NETCONF_GET_CONFIG_STARTED = "NETCONF_GET_CONFIG_STARTED"; +extern const isc::log::MessageID NETCONF_INFO = "NETCONF_INFO"; extern const isc::log::MessageID NETCONF_LOG_CHANGE_FAIL = "NETCONF_LOG_CHANGE_FAIL"; extern const isc::log::MessageID NETCONF_MODULE_INSTALL = "NETCONF_MODULE_INSTALL"; extern const isc::log::MessageID NETCONF_MODULE_MISSING_ERR = "NETCONF_MODULE_MISSING_ERR"; @@ -29,6 +30,8 @@ extern const isc::log::MessageID NETCONF_SET_CONFIG_STARTED = "NETCONF_SET_CONFI extern const isc::log::MessageID NETCONF_STARTED = "NETCONF_STARTED"; extern const isc::log::MessageID NETCONF_SUBSCRIBE_CONFIG = "NETCONF_SUBSCRIBE_CONFIG"; extern const isc::log::MessageID NETCONF_SUBSCRIBE_CONFIG_FAILED = "NETCONF_SUBSCRIBE_CONFIG_FAILED"; +extern const isc::log::MessageID NETCONF_SUBSCRIBE_NOTIFICATIONS = "NETCONF_SUBSCRIBE_NOTIFICATIONS"; +extern const isc::log::MessageID NETCONF_SUBSCRIBE_NOTIFICATIONS_FAILED = "NETCONF_SUBSCRIBE_NOTIFICATIONS_FAILED"; extern const isc::log::MessageID NETCONF_UPDATE_CONFIG = "NETCONF_UPDATE_CONFIG"; extern const isc::log::MessageID NETCONF_UPDATE_CONFIG_COMPLETED = "NETCONF_UPDATE_CONFIG_COMPLETED"; extern const isc::log::MessageID NETCONF_UPDATE_CONFIG_FAILED = "NETCONF_UPDATE_CONFIG_FAILED"; @@ -54,12 +57,13 @@ const char* values[] = { "NETCONF_GET_CONFIG", "got configuration from %1 server: %2", "NETCONF_GET_CONFIG_FAILED", "getting configuration from %1 server failed: %2", "NETCONF_GET_CONFIG_STARTED", "getting configuration from %1 server", + "NETCONF_INFO", "%1", "NETCONF_LOG_CHANGE_FAIL", "Netconf configuration change logging failed: %1", "NETCONF_MODULE_INSTALL", "Sysrepo (un)installs a module: %1 (revision %2)", "NETCONF_MODULE_MISSING_ERR", "Missing essential module %1 in sysrepo", "NETCONF_MODULE_MISSING_WARN", "Missing module %1 in sysrepo", - "NETCONF_MODULE_REVISION_ERR", "Essential module %1 does have the right revision: expected %2, got %3", - "NETCONF_MODULE_REVISION_WARN", "Module %1 does have the right revision: expected %2, got %3", + "NETCONF_MODULE_REVISION_ERR", "Essential module %1 does NOT have the right revision: expected %2, got %3", + "NETCONF_MODULE_REVISION_WARN", "Module %1 does NOT have the right revision: expected %2, got %3", "NETCONF_RUN_EXIT", "application is exiting the event loop", "NETCONF_SET_CONFIG", "set configuration to %1 server: %2", "NETCONF_SET_CONFIG_FAILED", "setting configuration to %1 server failed: %2", @@ -67,6 +71,8 @@ const char* values[] = { "NETCONF_STARTED", "Netconf (version %1) started", "NETCONF_SUBSCRIBE_CONFIG", "subscribing configuration changes for %1 server with %2 module", "NETCONF_SUBSCRIBE_CONFIG_FAILED", "subscribe configuration changes for %1 server with %2 module failed: %3", + "NETCONF_SUBSCRIBE_NOTIFICATIONS", "subscribing to notifications for %1 server with %2 module", + "NETCONF_SUBSCRIBE_NOTIFICATIONS_FAILED", "subscribing to notifications for %1 server with %2 module failed: %3", "NETCONF_UPDATE_CONFIG", "updating configuration with %1 server: %2", "NETCONF_UPDATE_CONFIG_COMPLETED", "completed updating configuration for %1 server", "NETCONF_UPDATE_CONFIG_FAILED", "updating configuration with %1 server: %2", diff --git a/src/bin/netconf/netconf_messages.h b/src/bin/netconf/netconf_messages.h index 87ee0b5612..38ca67a451 100644 --- a/src/bin/netconf/netconf_messages.h +++ b/src/bin/netconf/netconf_messages.h @@ -17,6 +17,7 @@ extern const isc::log::MessageID NETCONF_FAILED; extern const isc::log::MessageID NETCONF_GET_CONFIG; extern const isc::log::MessageID NETCONF_GET_CONFIG_FAILED; extern const isc::log::MessageID NETCONF_GET_CONFIG_STARTED; +extern const isc::log::MessageID NETCONF_INFO; extern const isc::log::MessageID NETCONF_LOG_CHANGE_FAIL; extern const isc::log::MessageID NETCONF_MODULE_INSTALL; extern const isc::log::MessageID NETCONF_MODULE_MISSING_ERR; @@ -30,6 +31,8 @@ extern const isc::log::MessageID NETCONF_SET_CONFIG_STARTED; extern const isc::log::MessageID NETCONF_STARTED; extern const isc::log::MessageID NETCONF_SUBSCRIBE_CONFIG; extern const isc::log::MessageID NETCONF_SUBSCRIBE_CONFIG_FAILED; +extern const isc::log::MessageID NETCONF_SUBSCRIBE_NOTIFICATIONS; +extern const isc::log::MessageID NETCONF_SUBSCRIBE_NOTIFICATIONS_FAILED; extern const isc::log::MessageID NETCONF_UPDATE_CONFIG; extern const isc::log::MessageID NETCONF_UPDATE_CONFIG_COMPLETED; extern const isc::log::MessageID NETCONF_UPDATE_CONFIG_FAILED; diff --git a/src/bin/netconf/netconf_messages.mes b/src/bin/netconf/netconf_messages.mes index 07144de823..a75f9f677e 100644 --- a/src/bin/netconf/netconf_messages.mes +++ b/src/bin/netconf/netconf_messages.mes @@ -1,4 +1,4 @@ -# Copyright (C) 2012-2018 Internet Systems Consortium, Inc. ("ISC") +# Copyright (C) 2012-2021 Internet Systems Consortium, Inc. ("ISC") # # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this @@ -48,6 +48,9 @@ name of the server and the error are printed. This informational message indicates that Netconf is trying to get the configuration from a Kea server. +% NETCONF_INFO %1 +Generic NETCONF message + % NETCONF_LOG_CHANGE_FAIL Netconf configuration change logging failed: %1 The warning message indicates that the configuration change logging encountered an unexpected condition. Details of it will be logged. @@ -66,12 +69,12 @@ the module is printed. This warning message indicates that a module used by Kea is not available in the sysrepo repository. The name of the module is printed. -% NETCONF_MODULE_REVISION_ERR Essential module %1 does have the right revision: expected %2, got %3 +% NETCONF_MODULE_REVISION_ERR Essential module %1 does NOT have the right revision: expected %2, got %3 This fatal error message indicates that a module required by Netconf configuration is not at the right revision in the sysrepo repository. The name, expected and available revisions of the module are printed. -% NETCONF_MODULE_REVISION_WARN Module %1 does have the right revision: expected %2, got %3 +% NETCONF_MODULE_REVISION_WARN Module %1 does NOT have the right revision: expected %2, got %3 This warning message indicates that a module used by Kea is not at the right revision in the sysrepo repository. The name, expected and available revisions of the module are printed. @@ -110,6 +113,15 @@ The error message indicates that Netconf got an error subscribing configuration changes for a Kea server. The names of the server and the module, and the error are printed. +% NETCONF_SUBSCRIBE_NOTIFICATIONS subscribing to notifications for %1 server with %2 module +This information message indicates that Netconf is trying to subscribe to +notifications for a Kea server. The server name and module name are printed. + +% NETCONF_SUBSCRIBE_NOTIFICATIONS_FAILED subscribing to notifications for %1 server with %2 module failed: %3 +The error message indicates that Netconf got an error subscribing to +notifications for a Kea server. The server name, module name and the error are +printed. + % NETCONF_UPDATE_CONFIG updating configuration with %1 server: %2 This debug message indicates that Netconf update the configuration of a Kea server. The server name and the updated configuration are diff --git a/src/bin/netconf/netconf_process.cc b/src/bin/netconf/netconf_process.cc index 72eb04f680..62cbdb23ae 100644 --- a/src/bin/netconf/netconf_process.cc +++ b/src/bin/netconf/netconf_process.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018-2020 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -26,16 +26,11 @@ using namespace isc::process; namespace isc { namespace netconf { -std::atomic NetconfProcess::shut_down(false); - NetconfProcess::NetconfProcess(const char* name, const asiolink::IOServicePtr& io_service) : DProcessBase(name, io_service, DCfgMgrBasePtr(new NetconfCfgMgr())) { } -NetconfProcess::~NetconfProcess() { -} - void NetconfProcess::init() { } @@ -48,22 +43,11 @@ NetconfProcess::run() { // Initialize netconf agent in a thread. std::thread th([this]() { try { - if (shouldShutdown()) { - return; - } // Initialize sysrepo. agent_.initSysrepo(); - if (shouldShutdown()) { - return; - } // Get the configuration manager. - NetconfCfgMgrPtr cfg_mgr; - if (shouldShutdown()) { - return; - } else { - cfg_mgr = getNetconfCfgMgr(); - } + NetconfCfgMgrPtr cfg_mgr(getNetconfCfgMgr()); // Call init. agent_.init(cfg_mgr); @@ -113,7 +97,6 @@ NetconfProcess::runIO() { isc::data::ConstElementPtr NetconfProcess::shutdown(isc::data::ConstElementPtr /*args*/) { - shut_down = true; setShutdownFlag(true); return (isc::config::createAnswer(0, "Netconf is shutting down")); } diff --git a/src/bin/netconf/netconf_process.h b/src/bin/netconf/netconf_process.h index eaa40bce33..e8b24b1ccf 100644 --- a/src/bin/netconf/netconf_process.h +++ b/src/bin/netconf/netconf_process.h @@ -1,4 +1,4 @@ -// Copyright (C) 2018-2020 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -9,8 +9,8 @@ #include #include + #include -#include namespace isc { namespace netconf { @@ -33,7 +33,7 @@ public: NetconfProcess(const char* name, const asiolink::IOServicePtr& io_service); /// @brief Destructor - virtual ~NetconfProcess(); + virtual ~NetconfProcess() = default; /// @brief Initialize the Netconf process. /// @@ -85,9 +85,6 @@ public: /// @brief Returns a pointer to the configuration manager. NetconfCfgMgrPtr getNetconfCfgMgr(); - /// @brief Global (globally visible) shutdown flag. - static std::atomic shut_down; - private: /// @brief Polls all ready handlers and then runs one handler if none diff --git a/src/bin/netconf/simple_parser.cc b/src/bin/netconf/simple_parser.cc index 723f2fcdf5..1574ddf581 100644 --- a/src/bin/netconf/simple_parser.cc +++ b/src/bin/netconf/simple_parser.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018-2020 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -42,29 +42,29 @@ const SimpleDefaults NetconfSimpleParser::NETCONF_DEFAULTS = { { "validate-changes", Element::boolean, "true" } }; -/// Supplies defaults for control-socket elements +/// @brief Supplies defaults for control-socket elements const SimpleDefaults NetconfSimpleParser::CTRL_SOCK_DEFAULTS = { { "socket-type", Element::string, "stdout" }, { "socket-name", Element::string, "" }, { "socket-url" , Element::string, "http://127.0.0.1:8000/" } }; -/// Supplies defaults for dhcp4 managed server +/// @brief Supplies defaults for dhcp4 managed server const SimpleDefaults NetconfSimpleParser::DHCP4_DEFAULTS = { { "model", Element::string, "kea-dhcp4-server" } }; -/// Supplies defaults for dhcp6 managed server +/// @brief Supplies defaults for dhcp6 managed server const SimpleDefaults NetconfSimpleParser::DHCP6_DEFAULTS = { { "model", Element::string, "kea-dhcp6-server" } }; -/// Supplies defaults for d2 managed server +/// @brief Supplies defaults for d2 managed server const SimpleDefaults NetconfSimpleParser::D2_DEFAULTS = { { "model", Element::string, "kea-dhcp-ddns" } }; -/// Supplies defaults for ca managed server +/// @brief Supplies defaults for ca managed server const SimpleDefaults NetconfSimpleParser::CA_DEFAULTS = { { "model", Element::string, "kea-ctrl-agent" } }; @@ -190,5 +190,5 @@ NetconfSimpleParser::parse(const NetconfConfigPtr& ctx, } } -} -} +} // namespace netconf +} // namespace isc diff --git a/src/bin/netconf/stdout_control_socket.cc b/src/bin/netconf/stdout_control_socket.cc index 382c391c5c..8dc925ba6d 100644 --- a/src/bin/netconf/stdout_control_socket.cc +++ b/src/bin/netconf/stdout_control_socket.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -33,9 +33,6 @@ StdoutControlSocket::StdoutControlSocket(CfgControlSocketPtr ctrl_sock, : ControlSocketBase(ctrl_sock), output_(output) { } -StdoutControlSocket::~StdoutControlSocket() { -} - ConstElementPtr StdoutControlSocket::configGet(const string& /*service*/) { isc_throw(NotImplemented, "No config-get for stdout control socket"); diff --git a/src/bin/netconf/stdout_control_socket.h b/src/bin/netconf/stdout_control_socket.h index 9b0d1f343d..4bbd82f63e 100644 --- a/src/bin/netconf/stdout_control_socket.h +++ b/src/bin/netconf/stdout_control_socket.h @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -32,7 +32,7 @@ public: StdoutControlSocket(CfgControlSocketPtr ctrl_sock); /// @brief Destructor (does nothing). - virtual ~StdoutControlSocket(); + virtual ~StdoutControlSocket() = default; /// @brief Get configuration. /// diff --git a/src/bin/netconf/tests/control_socket_unittests.cc b/src/bin/netconf/tests/control_socket_unittests.cc index ae7ee02bda..5ce429e444 100644 --- a/src/bin/netconf/tests/control_socket_unittests.cc +++ b/src/bin/netconf/tests/control_socket_unittests.cc @@ -5,6 +5,7 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include + #include #include #include @@ -20,7 +21,10 @@ #include #include #include +#include + #include + #include #include @@ -33,6 +37,8 @@ using namespace isc::http; using namespace isc::http::test; using namespace isc::test; +using isc::yang::test::SysrepoSetup; + namespace { /// @brief Type definition for the pointer to Thread objects. @@ -141,11 +147,15 @@ public: /// @brief Constructor. UnixControlSocketTest() : ThreadedTest(), io_service_() { + } + + + void SetUp() override { + SysrepoSetup::cleanSharedMemory(); removeUnixSocketFile(); } - /// @brief Destructor. - virtual ~UnixControlSocketTest() { + void TearDown() override { if (thread_) { thread_->join(); thread_.reset(); @@ -504,13 +514,12 @@ public: /// @brief Test fixture class for http control sockets. class HttpControlSocketTest : public ThreadedTest { public: - /// @brief Constructor - HttpControlSocketTest() - : ThreadedTest(), io_service_() { + void SetUp() override { + SysrepoSetup::cleanSharedMemory(); } - /// @brief Destructor. - virtual ~HttpControlSocketTest() { + void TearDown() override { + SysrepoSetup::cleanSharedMemory(); if (thread_) { thread_->join(); thread_.reset(); @@ -854,4 +863,4 @@ TEST_F(HttpControlSocketTest, partial) { stop(); } -} +} // namespace diff --git a/src/bin/netconf/tests/get_config_unittest.cc b/src/bin/netconf/tests/get_config_unittest.cc index 77dd2ab6cd..9640ac428a 100644 --- a/src/bin/netconf/tests/get_config_unittest.cc +++ b/src/bin/netconf/tests/get_config_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -71,7 +71,7 @@ readFile(const std::string& file_path) { /// @brief Runs parser in JSON mode ElementPtr -parseJSON(const std::string& in, bool verbose = false) { +parseJSON(const std::string& in, bool verbose = false) { try { ParserContext ctx; return (ctx.parseString(in, ParserContext::PARSER_JSON)); @@ -85,7 +85,7 @@ parseJSON(const std::string& in, bool verbose = false) { /// @brief Runs parser in NETCONF mode ElementPtr -parseNETCONF(const std::string& in, bool verbose = false) { +parseNETCONF(const std::string& in, bool verbose = false) { try { ParserContext ctx; return (ctx.parseString(in, ParserContext::PARSER_NETCONF)); @@ -115,9 +115,9 @@ public: using NetconfCfgMgr::parse; }; -} +} // namespace -/// Test fixture class +/// @brief Test fixture class class NetconfGetCfgTest : public ConfigParseTest { public: NetconfGetCfgTest() @@ -226,7 +226,7 @@ public: ConstElementPtr comment_; ///< Reason for parse fail }; -/// Test a configuration +// Test a simple configuration. TEST_F(NetconfGetCfgTest, simple) { // get the simple configuration diff --git a/src/bin/netconf/tests/netconf_cfg_mgr_unittests.cc b/src/bin/netconf/tests/netconf_cfg_mgr_unittests.cc index d463439103..9304bffd6a 100644 --- a/src/bin/netconf/tests/netconf_cfg_mgr_unittests.cc +++ b/src/bin/netconf/tests/netconf_cfg_mgr_unittests.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -110,7 +110,7 @@ TEST(NetconfCfgMgr, contextServer) { EXPECT_EQ(server2, ctx.getCfgServersMap()->at("dhcp6")); // Finally, set all servers. - EXPECT_NO_THROW(ctx.getCfgServersMap()->insert(make_pair("dhcp4", server3))); + EXPECT_NO_THROW(ctx.getCfgServersMap()->insert(make_pair("dhcp4", server3))); EXPECT_NO_THROW(ctx.getCfgServersMap()->insert(make_pair("ca", server4))); EXPECT_EQ(4, ctx.getCfgServersMap()->size()); ASSERT_NO_THROW(ctx.getCfgServersMap()->at("dhcp4")); @@ -190,7 +190,7 @@ TEST(NetconfCfgMgr, contextGlobals) { } } -/// Netconf configurations used in tests. +/// @brief Netconf configurations used in tests. const char* NETCONF_CONFIGS[] = { // configuration 0: empty (nothing specified) @@ -731,4 +731,4 @@ TEST_F(NetconfParserTest, comments) { EXPECT_EQ("1", ctx6->get("version")->str()); } -}; // end of anonymous namespace +} // namespace diff --git a/src/bin/netconf/tests/netconf_unittests.cc b/src/bin/netconf/tests/netconf_unittests.cc index 49bab08349..3443b3891f 100644 --- a/src/bin/netconf/tests/netconf_unittests.cc +++ b/src/bin/netconf/tests/netconf_unittests.cc @@ -1,10 +1,11 @@ -// Copyright (C) 2018-2020 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include + #include #include #include @@ -16,15 +17,17 @@ #include #include #include +#include #include #include #include #include #include + #include + #include #include -#include using namespace std; using namespace isc; @@ -36,9 +39,9 @@ using namespace isc::http; using namespace isc::test; using namespace isc::yang; using namespace isc::yang::test; -#ifndef HAVE_PRE_0_7_6_SYSREPO using namespace sysrepo; -#endif + +using isc::yang::test::SysrepoSetup; namespace { @@ -83,29 +86,34 @@ void clearYang(NakedNetconfAgentPtr agent) { if (agent && (agent->startup_sess_)) { string xpath = "/kea-dhcp4-server:config"; EXPECT_NO_THROW(agent->startup_sess_->delete_item(xpath.c_str())); - EXPECT_NO_THROW(agent->startup_sess_->commit()); + EXPECT_NO_THROW(agent->startup_sess_->apply_changes()); } } + +// Empirically the requested subnets have sometimes returned in decreasing +// order of subnet ID. To avoid flaky test failures, sort them before +// comparing. +void sortSubnets(ConstElementPtr const& map) { + boost::dynamic_pointer_cast( + boost::const_pointer_cast( + map->get("arguments")->get("Dhcp4")->get("subnet4"))) + ->sort("id"); +} + /// @brief Test fixture class for netconf agent. class NetconfAgentTest : public ThreadedTest { public: isc::test::Sandbox sandbox; - /// @brief Constructor. - NetconfAgentTest() - : ThreadedTest(), - io_service_(new IOService()), - agent_(new NakedNetconfAgent), - requests_(), - responses_() { - NetconfProcess::shut_down = false; + void SetUp() override { + SysrepoSetup::cleanSharedMemory(); removeUnixSocketFile(); + io_service_.reset(new IOService()); + agent_.reset(new NakedNetconfAgent()); } - /// @brief Destructor. - virtual ~NetconfAgentTest() { - NetconfProcess::shut_down = true; + void TearDown() override { if (thread_) { thread_->join(); thread_.reset(); @@ -123,6 +131,7 @@ public: requests_.clear(); responses_.clear(); removeUnixSocketFile(); + SysrepoSetup::cleanSharedMemory(); } /// @brief Returns socket file path. @@ -181,12 +190,10 @@ public: : io_service_(new IOService()), thread_(), agent_(new NakedNetconfAgent) { - NetconfProcess::shut_down = false; } /// @brief Destructor. virtual ~NetconfAgentLogTest() { - NetconfProcess::shut_down = true; // io_service must be stopped to make the thread to return. io_service_->stop(); io_service_.reset(); @@ -201,6 +208,29 @@ public: agent_.reset(); } + + /// @brief To know when the callback was called. + bool finished_; + + /// @brief Default change callback (print changes and return OK). + sr_error_t callback(sysrepo::S_Session sess, + const char* module_name, + const char* /* xpath */, + sr_event_t /* event */, + uint32_t /* request_id */) { + NetconfAgent::logChanges(sess, module_name); + finished_ = true; + return (SR_ERR_OK); + } + + /// @brief logChanges is called in another thread so we can have to wait for + /// it. + void waitForCallback() { + while (!finished_) { + usleep(1000); + } + } + /// @brief IOService object. IOServicePtr io_service_; @@ -316,20 +346,17 @@ NetconfAgentTest::fakeServer() { // signalStopped can't be called here because of the 2 runs for update. } -/// Verifies the initSysrepo method opens sysrepo connection and sessions. +// Verifies that the initSysrepo method opens sysrepo connection and sessions. TEST_F(NetconfAgentTest, initSysrepo) { EXPECT_NO_THROW(agent_->initSysrepo()); EXPECT_TRUE(agent_->conn_); EXPECT_TRUE(agent_->startup_sess_); EXPECT_TRUE(agent_->running_sess_); - EXPECT_EQ(1, agent_->subscriptions_.size()); + EXPECT_EQ(0, agent_->subscriptions_.size()); EXPECT_LE(16, agent_->modules_.size()); - // No way to check the module_install callback (BTW is there - // an API to install modules? If none only system tests can - // do something)... } -/// Verifies the checkModule method emits expected errors. +// Verifies that the checkModule method emits expected errors. TEST_F(NetconfAgentLogTest, checkModule) { // keatest-module should not be in YANG_REVISIONS. EXPECT_EQ(0, YANG_REVISIONS.count("keatest-module")); @@ -362,16 +389,14 @@ TEST_F(NetconfAgentLogTest, checkModule) { EXPECT_FALSE(agent_->checkModule(module)); ostringstream msg; msg << "NETCONF_MODULE_REVISION_ERR Essential module " << module - << " does have the right revision: expected " + << " does NOT have the right revision: expected " << YANG_REVISIONS.at(module) << ", got " << bad_revision; addString(msg.str()); - // Enable this for debugging. - // logCheckVerbose(true); EXPECT_TRUE(checkFile()); } -/// Verifies the checkModules method emits expected warnings. +// Verifies that the checkModules method emits expected warnings. TEST_F(NetconfAgentLogTest, checkModules) { // kea-dhcp[46]-server must be in YANG_REVISIONS. ASSERT_EQ(1, YANG_REVISIONS.count("kea-dhcp4-server")); @@ -382,7 +407,7 @@ TEST_F(NetconfAgentLogTest, checkModules) { EXPECT_EQ(1, agent_->modules_.count("kea-dhcp4-server")); EXPECT_EQ(1, agent_->modules_.count("kea-dhcp6-server")); - // Run checkModules but it will be indirectly check as + // Run checkModules but it will be indirectly checked as // emitting nothing. ASSERT_NO_THROW(agent_->checkModules()); @@ -404,188 +429,154 @@ TEST_F(NetconfAgentLogTest, checkModules) { ASSERT_NO_THROW(agent_->checkModules()); ostringstream rmsg; rmsg << "NETCONF_MODULE_REVISION_WARN Module " << module - << " does have the right revision: expected " + << " does NOT have the right revision: expected " << YANG_REVISIONS.at(module) << ", got " << bad_revision; addString(rmsg.str()); - // Enable this for debugging. - // logCheckVerbose(true); EXPECT_TRUE(checkFile()); } -/// @brief Default change callback (print changes and return OK). -class TestCallback : public Callback { -public: - int module_change(S_Session sess, - const char* /*module_name*/, - sr_notif_event_t /*event*/, - void* /*private_ctx*/) { - NetconfAgent::logChanges(sess, "/kea-dhcp4-server:config"); - finished = true; - return (SR_ERR_OK); - } - - // To know when the callback was called. - static atomic finished; -}; - -atomic TestCallback::finished(false); - -/// Verifies the logChanges method handles correctly changes. +// Verifies that the logChanges method handles correctly changes. TEST_F(NetconfAgentLogTest, logChanges) { // Initial YANG configuration. - const YRTree tree0 = { + const YRTree tree0 = YangRepr::buildTreeFromVector({ { "/kea-dhcp4-server:config", "", SR_CONTAINER_T, false }, - { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/id", "1", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/subnet", "10.0.0.0/24", SR_STRING_T, true }, - { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/id", "2", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/subnet", "10.0.2.0/24", SR_STRING_T, true } - }; + }); // Load initial YANG configuration. ASSERT_NO_THROW(agent_->initSysrepo()); YangRepr repr(KEA_DHCP4_SERVER); ASSERT_NO_THROW(repr.set(tree0, agent_->startup_sess_)); - EXPECT_NO_THROW(agent_->startup_sess_->commit()); + EXPECT_NO_THROW(agent_->startup_sess_->apply_changes()); // Subscribe configuration changes. S_Subscribe subs(new Subscribe(agent_->running_sess_)); - S_Callback cb(new TestCallback()); - TestCallback::finished = false; - EXPECT_NO_THROW(subs->module_change_subscribe(KEA_DHCP4_SERVER.c_str(), - cb, 0, 0, - SR_SUBSCR_APPLY_ONLY)); + auto cb = [&](sysrepo::S_Session sess, const char* module_name, + const char* xpath, sr_event_t event, uint32_t request_id) { + return callback(sess, module_name, xpath, event, request_id); + }; + EXPECT_NO_THROW(subs->module_change_subscribe(KEA_DHCP4_SERVER.c_str(), cb, + nullptr, 0, + SR_SUBSCR_DONE_ONLY)); thread_.reset(new thread([this]() { io_service_->run(); })); // Change configuration (subnet #1 moved from 10.0.0.0/24 to 10.0.1/0/24). - const YRTree tree1 = { + const YRTree tree1 = YangRepr::buildTreeFromVector({ { "/kea-dhcp4-server:config", "", SR_CONTAINER_T, false }, - { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/id", "1", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/subnet", "10.0.1.0/24", SR_STRING_T, true }, // The change is here! - { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/id", "2", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/subnet", "10.0.2.0/24", SR_STRING_T, true } - }; + }); EXPECT_NO_THROW(repr.set(tree1, agent_->running_sess_)); - EXPECT_NO_THROW(agent_->running_sess_->commit()); + EXPECT_NO_THROW(agent_->running_sess_->apply_changes()); // Check that the debug output was correct. + addString( + "NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: created: " + "/kea-dhcp4-server:config/subnet4[id='2'] (list instance)"); addString("NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: " - "modified: " - "/kea-dhcp4-server:config/subnet4[id='1']/subnet = " - "10.0.0.0/24 => " - "/kea-dhcp4-server:config/subnet4[id='1']/subnet = " - "10.0.1.0/24"); - - // logChanges is called in another thread so we can have to wait for it. - while (!TestCallback::finished) { - usleep(1000); - } - // Enable this for debugging. - // logCheckVerbose(true); + "created: /kea-dhcp4-server:config/subnet4[id='2']/id = 2"); + addString( + "NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: created: " + "/kea-dhcp4-server:config/subnet4[id='2']/subnet = 10.0.2.0/24"); + addString( + "NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: created: " + "/kea-dhcp4-server:config/subnet4[id='2']/relay (container)"); + + waitForCallback(); + EXPECT_TRUE(checkFile()); } -/// Verifies the logChanges method handles correctly changes. -/// Instead of the simple modified of the previous etst, now there will -/// deleted, created and moved. +// Verifies that the logChanges method handles correctly changes. +// Instead of the simple modified of the previous test, now there will +// deleted, created and moved. TEST_F(NetconfAgentLogTest, logChanges2) { // Initial YANG configuration. - const YRTree tree0 = { + const YRTree tree0 = YangRepr::buildTreeFromVector({ { "/kea-dhcp4-server:config", "", SR_CONTAINER_T, false }, - { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/id", "1", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/subnet", "10.0.0.0/24", SR_STRING_T, true }, - { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/id", "2", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/subnet", "10.0.2.0/24", SR_STRING_T, true } - }; + }); // Load initial YANG configuration. ASSERT_NO_THROW(agent_->initSysrepo()); YangRepr repr(KEA_DHCP4_SERVER); ASSERT_NO_THROW(repr.set(tree0, agent_->startup_sess_)); - EXPECT_NO_THROW(agent_->startup_sess_->commit()); + EXPECT_NO_THROW(agent_->startup_sess_->apply_changes()); // Subscribe configuration changes. S_Subscribe subs(new Subscribe(agent_->running_sess_)); - S_Callback cb(new TestCallback()); - TestCallback::finished = false; - EXPECT_NO_THROW(subs->module_change_subscribe(KEA_DHCP4_SERVER.c_str(), - cb, 0, 0, - SR_SUBSCR_APPLY_ONLY)); + auto cb = [&](sysrepo::S_Session sess, const char* module_name, + const char* xpath, sr_event_t event, uint32_t request_id) { + return callback(sess, module_name, xpath, event, request_id); + }; + EXPECT_NO_THROW(subs->module_change_subscribe(KEA_DHCP4_SERVER.c_str(), cb, + nullptr, 0, + SR_SUBSCR_DONE_ONLY)); thread_.reset(new thread([this]() { io_service_->run(); })); // Change configuration (subnet #1 moved to #10). string xpath = "/kea-dhcp4-server:config/subnet4[id='1']"; EXPECT_NO_THROW(agent_->running_sess_->delete_item(xpath.c_str())); - const YRTree tree1 = { + const YRTree tree1 = YangRepr::buildTreeFromVector({ { "/kea-dhcp4-server:config", "", SR_CONTAINER_T, false }, - { "/kea-dhcp4-server:config/subnet4[id='10']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='10']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='10']/id", "10", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='10']/subnet", "10.0.0.0/24", SR_STRING_T, true }, // The change is here! - { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/id", "2", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/subnet", "10.0.2.0/24", SR_STRING_T, true } - }; + }); EXPECT_NO_THROW(repr.set(tree1, agent_->running_sess_)); - EXPECT_NO_THROW(agent_->running_sess_->commit()); + EXPECT_NO_THROW(agent_->running_sess_->apply_changes()); // Check that the debug output was correct. + addString( + "NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: deleted: " + "/kea-dhcp4-server:config/subnet4[id='1'] (list instance)"); addString("NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: " - "deleted: " - "/kea-dhcp4-server:config/subnet4[id='1']/id = 1"); - addString("NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: " - "deleted: " - "/kea-dhcp4-server:config/subnet4[id='1']/subnet = " - "10.0.0.0/24"); - addString("NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: " - "deleted: " - "/kea-dhcp4-server:config/subnet4[id='1'] " - "(list instance)"); - addString("NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: " - "created: " - "/kea-dhcp4-server:config/subnet4[id='10'] " - "(list instance)"); - addString("NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: " - "created: " - "/kea-dhcp4-server:config/subnet4[id='10']/id = 10"); - addString("NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: " - "created: " - "/kea-dhcp4-server:config/subnet4[id='10']/subnet = " - "10.0.0.0/24"); - addString("NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: " - "moved: " - "/kea-dhcp4-server:config/subnet4[id='10'] " - "after /kea-dhcp4-server:config/subnet4[id='2']"); + "deleted: /kea-dhcp4-server:config/subnet4[id='1']/id = 1"); + addString( + "NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: deleted: " + "/kea-dhcp4-server:config/subnet4[id='1']/subnet = 10.0.1.0/24"); + addString( + "NETCONF_CONFIG_CHANGED_DETAIL YANG configuration changed: deleted: " + "/kea-dhcp4-server:config/subnet4[id='1']/relay (container)"); + + waitForCallback(); - // logChanges is called in another thread so we can have to wait for it. - while (!TestCallback::finished) { - usleep(1000); - } - // Enable this for debugging. - // logCheckVerbose(true); EXPECT_TRUE(checkFile()); } -/// Verifies the keaConfig method works as expected. +// Verifies that the keaConfig method works as expected. TEST_F(NetconfAgentTest, keaConfig) { // Netconf configuration. string config_prefix = "{\n" @@ -665,28 +656,28 @@ TEST_F(NetconfAgentTest, keaConfig) { EXPECT_TRUE(expected->equals(*response)); } -/// Verifies the yangConfig method works as expected: apply YANG config -/// to the server. +// Verifies that the yangConfig method works as expected: apply YANG config +// to the server. TEST_F(NetconfAgentTest, yangConfig) { // YANG configuration. - const YRTree tree = { + const YRTree tree = YangRepr::buildTreeFromVector({ { "/kea-dhcp4-server:config", "", SR_CONTAINER_T, false }, - { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/id", "1", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/subnet", "10.0.0.0/24", SR_STRING_T, true }, - { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/id", "2", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/subnet", "10.0.2.0/24", SR_STRING_T, true } - }; + }); // Load YANG configuration. ASSERT_NO_THROW(agent_->initSysrepo()); YangRepr repr(KEA_DHCP4_SERVER); ASSERT_NO_THROW(repr.set(tree, agent_->startup_sess_)); - EXPECT_NO_THROW(agent_->startup_sess_->commit()); + EXPECT_NO_THROW(agent_->startup_sess_->apply_changes()); // Netconf configuration. string config_prefix = "{\n" @@ -761,7 +752,10 @@ TEST_F(NetconfAgentTest, yangConfig) { "}"; ConstElementPtr expected; ASSERT_NO_THROW(expected = Element::fromJSON(expected_str)); - EXPECT_TRUE(expected->equals(*request)); + + sortSubnets(expected); + sortSubnets(request); + EXPECT_EQ(prettyPrint(expected), prettyPrint(request)); // Check response. ASSERT_EQ(1, responses_.size()); @@ -775,7 +769,7 @@ TEST_F(NetconfAgentTest, yangConfig) { EXPECT_TRUE(expected->equals(*response)); } -/// Verifies the subscribeConfig method works as expected. +// Verifies that the subscribeConfig method works as expected. TEST_F(NetconfAgentTest, subscribeConfig) { // Netconf configuration. string config_prefix = "{\n" @@ -817,36 +811,36 @@ TEST_F(NetconfAgentTest, subscribeConfig) { // Try subscribeConfig. EXPECT_EQ(0, agent_->subscriptions_.size()); ASSERT_NO_THROW(agent_->initSysrepo()); - EXPECT_EQ(1, agent_->subscriptions_.size()); + EXPECT_EQ(0, agent_->subscriptions_.size()); EXPECT_NO_THROW(agent_->subscribeConfig(service_pair)); - EXPECT_EQ(2, agent_->subscriptions_.size()); + EXPECT_EQ(1, agent_->subscriptions_.size()); /// Unsubscribe. EXPECT_NO_THROW(agent_->subscriptions_.clear()); } -/// Verifies the update method works as expected: apply new YANG configuration -/// to the server. Note it is called by the subscription callback. +// Verifies that the update method works as expected: apply new YANG configuration +// to the server. Note it is called by the subscription callback. TEST_F(NetconfAgentTest, update) { // Initial YANG configuration. - const YRTree tree0 = { + const YRTree tree0 = YangRepr::buildTreeFromVector({ { "/kea-dhcp4-server:config", "", SR_CONTAINER_T, false }, - { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/id", "1", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/subnet", "10.0.0.0/24", SR_STRING_T, true }, - { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/id", "2", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/subnet", "10.0.2.0/24", SR_STRING_T, true } - }; + }); // Load initial YANG configuration. ASSERT_NO_THROW(agent_->initSysrepo()); YangRepr repr(KEA_DHCP4_SERVER); ASSERT_NO_THROW(repr.set(tree0, agent_->startup_sess_)); - EXPECT_NO_THROW(agent_->startup_sess_->commit()); + EXPECT_NO_THROW(agent_->startup_sess_->apply_changes()); // Netconf configuration. // Set validate-changes to false to avoid validate() to be called. @@ -888,9 +882,9 @@ TEST_F(NetconfAgentTest, update) { CfgServersMapPair service_pair = *servers_map->begin(); // Subscribe YANG changes. - EXPECT_EQ(1, agent_->subscriptions_.size()); + EXPECT_EQ(0, agent_->subscriptions_.size()); EXPECT_NO_THROW(agent_->subscribeConfig(service_pair)); - EXPECT_EQ(2, agent_->subscriptions_.size()); + EXPECT_EQ(1, agent_->subscriptions_.size()); // Launch server. thread_.reset(new thread([this]() { fakeServer(); signalStopped(); })); @@ -899,21 +893,21 @@ TEST_F(NetconfAgentTest, update) { waitReady(); // Change configuration (subnet #1 moved from 10.0.0.0/24 to 10.0.1/0/24). - const YRTree tree1 = { + const YRTree tree1 = YangRepr::buildTreeFromVector({ { "/kea-dhcp4-server:config", "", SR_CONTAINER_T, false }, - { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/id", "1", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/subnet", "10.0.1.0/24", SR_STRING_T, true }, // The change is here! - { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/id", "2", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/subnet", "10.0.2.0/24", SR_STRING_T, true } - }; + }); EXPECT_NO_THROW(repr.set(tree1, agent_->running_sess_)); - EXPECT_NO_THROW(agent_->running_sess_->commit()); + EXPECT_NO_THROW(agent_->running_sess_->apply_changes()); // Wait server to be stopped. waitStopped(); @@ -942,7 +936,10 @@ TEST_F(NetconfAgentTest, update) { "}"; ConstElementPtr expected; ASSERT_NO_THROW(expected = Element::fromJSON(expected_str)); - EXPECT_TRUE(expected->equals(*request)); + + sortSubnets(expected); + sortSubnets(request); + EXPECT_EQ(prettyPrint(expected), prettyPrint(request)); // Check response. ASSERT_EQ(1, responses_.size()); @@ -956,29 +953,29 @@ TEST_F(NetconfAgentTest, update) { EXPECT_TRUE(expected->equals(*response)); } -/// Verifies the validate method works as expected: test new YANG configuration -/// with the server. Note it is called by the subscription callback and -/// update is called after. +// Verifies that the validate method works as expected: test new YANG configuration +// with the server. Note it is called by the subscription callback and +// update is called after. TEST_F(NetconfAgentTest, validate) { // Initial YANG configuration. - const YRTree tree0 = { + const YRTree tree0 = YangRepr::buildTreeFromVector({ { "/kea-dhcp4-server:config", "", SR_CONTAINER_T, false }, - { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/id", "1", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/subnet", "10.0.0.0/24", SR_STRING_T, true }, - { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/id", "2", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/subnet", "10.0.2.0/24", SR_STRING_T, true } - }; + }); // Load initial YANG configuration. ASSERT_NO_THROW(agent_->initSysrepo()); YangRepr repr(KEA_DHCP4_SERVER); ASSERT_NO_THROW(repr.set(tree0, agent_->startup_sess_)); - EXPECT_NO_THROW(agent_->startup_sess_->commit()); + EXPECT_NO_THROW(agent_->startup_sess_->apply_changes()); // Netconf configuration. string config_prefix = "{\n" @@ -1018,9 +1015,9 @@ TEST_F(NetconfAgentTest, validate) { CfgServersMapPair service_pair = *servers_map->begin(); // Subscribe YANG changes. - EXPECT_EQ(1, agent_->subscriptions_.size()); + EXPECT_EQ(0, agent_->subscriptions_.size()); EXPECT_NO_THROW(agent_->subscribeConfig(service_pair)); - EXPECT_EQ(2, agent_->subscriptions_.size()); + EXPECT_EQ(1, agent_->subscriptions_.size()); // Launch server twice. thread_.reset(new thread([this]() @@ -1034,27 +1031,27 @@ TEST_F(NetconfAgentTest, validate) { waitReady(); // Change configuration (subnet #1 moved from 10.0.0.0/24 to 10.0.1/0/24). - const YRTree tree1 = { + const YRTree tree1 = YangRepr::buildTreeFromVector({ { "/kea-dhcp4-server:config", "", SR_CONTAINER_T, false }, - { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/id", "1", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/subnet", "10.0.1.0/24", SR_STRING_T, true }, // The change is here! - { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='2']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/id", "2", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='2']/subnet", "10.0.2.0/24", SR_STRING_T, true } - }; + }); EXPECT_NO_THROW(repr.set(tree1, agent_->running_sess_)); - EXPECT_NO_THROW(agent_->running_sess_->commit()); + EXPECT_NO_THROW(agent_->running_sess_->apply_changes()); // Wait servers to be stopped. waitStopped(); // Check requests. - ASSERT_EQ(2, requests_.size()); + ASSERT_LE(1, requests_.size()); string request_str = requests_[0]; ConstElementPtr request; ASSERT_NO_THROW(request = Element::fromJSON(request_str)); @@ -1077,8 +1074,12 @@ TEST_F(NetconfAgentTest, validate) { "}"; ConstElementPtr expected; ASSERT_NO_THROW(expected = Element::fromJSON(expected_str)); - EXPECT_TRUE(expected->equals(*request)); + sortSubnets(expected); + sortSubnets(request); + EXPECT_EQ(prettyPrint(expected), prettyPrint(request)); + + ASSERT_LE(2, requests_.size()); request_str = requests_[1]; ASSERT_NO_THROW(request = Element::fromJSON(request_str)); expected_str = "{\n" @@ -1099,7 +1100,13 @@ TEST_F(NetconfAgentTest, validate) { " }\n" "}"; ASSERT_NO_THROW(expected = Element::fromJSON(expected_str)); - EXPECT_TRUE(expected->equals(*request)); + + sortSubnets(expected); + sortSubnets(request); + EXPECT_EQ(prettyPrint(expected), prettyPrint(request)); + + // Check that no more than 2 requests have been received. + ASSERT_EQ(2, requests_.size()); // Check responses. ASSERT_EQ(2, responses_.size()); @@ -1121,22 +1128,22 @@ TEST_F(NetconfAgentTest, validate) { EXPECT_TRUE(expected->equals(*response)); } -/// Verifies what happens when the validate method returns an error. +// Verifies what happens when the validate method returns an error. TEST_F(NetconfAgentTest, noValidate) { // Initial YANG configuration. - const YRTree tree0 = { + const YRTree tree0 = YangRepr::buildTreeFromVector({ { "/kea-dhcp4-server:config", "", SR_CONTAINER_T, false }, - { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/id", "1", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/subnet", "10.0.0.0/24", SR_STRING_T, true } - }; + }); // Load initial YANG configuration. ASSERT_NO_THROW(agent_->initSysrepo()); YangRepr repr(KEA_DHCP4_SERVER); ASSERT_NO_THROW(repr.set(tree0, agent_->startup_sess_)); - EXPECT_NO_THROW(agent_->startup_sess_->commit()); + EXPECT_NO_THROW(agent_->startup_sess_->apply_changes()); // Netconf configuration. string config_prefix = "{\n" @@ -1176,26 +1183,25 @@ TEST_F(NetconfAgentTest, noValidate) { CfgServersMapPair service_pair = *servers_map->begin(); // Subscribe YANG changes. - EXPECT_EQ(1, agent_->subscriptions_.size()); + EXPECT_EQ(0, agent_->subscriptions_.size()); EXPECT_NO_THROW(agent_->subscribeConfig(service_pair)); - EXPECT_EQ(2, agent_->subscriptions_.size()); + EXPECT_EQ(1, agent_->subscriptions_.size()); // Change configuration (add invalid user context). - const YRTree tree1 = { + const YRTree tree1 = YangRepr::buildTreeFromVector({ { "/kea-dhcp4-server:config", "", SR_CONTAINER_T, false }, - { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, true }, + { "/kea-dhcp4-server:config/subnet4[id='1']", "", SR_LIST_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/id", "1", SR_UINT32_T, false }, { "/kea-dhcp4-server:config/subnet4[id='1']/subnet", "10.0.0.0/24", SR_STRING_T, true }, { "/kea-dhcp4-server:config/subnet4[id='1']/user-context", "BOGUS", SR_STRING_T, true } - }; - EXPECT_NO_THROW(repr.set(tree1, agent_->running_sess_)); + }); try { - agent_->running_sess_->commit(); + repr.set(tree1, agent_->running_sess_); } catch (const sysrepo_exception& ex) { - EXPECT_EQ("Validation of the changes failed", string(ex.what())); + EXPECT_EQ("User callback failed", string(ex.what())); } catch (const std::exception& ex) { ADD_FAILURE() << "unexpected exception: " << ex.what(); } catch (...) { @@ -1203,4 +1209,4 @@ TEST_F(NetconfAgentTest, noValidate) { } } -} +} // namespace diff --git a/src/bin/netconf/tests/shtests/netconf_tests.sh.in b/src/bin/netconf/tests/shtests/netconf_tests.sh.in index bda7d34292..b6ba611223 100644 --- a/src/bin/netconf/tests/shtests/netconf_tests.sh.in +++ b/src/bin/netconf/tests/shtests/netconf_tests.sh.in @@ -1,6 +1,6 @@ #!/bin/sh -# Copyright (C) 2018-2020 Internet Systems Consortium, Inc. ("ISC") +# Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") # # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this diff --git a/src/bin/netconf/unix_control_socket.cc b/src/bin/netconf/unix_control_socket.cc index 93a807eb1d..51ef5d1be6 100644 --- a/src/bin/netconf/unix_control_socket.cc +++ b/src/bin/netconf/unix_control_socket.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -34,9 +34,6 @@ UnixControlSocket::UnixControlSocket(CfgControlSocketPtr ctrl_sock) : ControlSocketBase(ctrl_sock) { } -UnixControlSocket::~UnixControlSocket() { -} - ConstElementPtr UnixControlSocket::configGet(const string& /*service*/) { return (sendCommand(createCommand("config-get"))); diff --git a/src/bin/netconf/unix_control_socket.h b/src/bin/netconf/unix_control_socket.h index 09fe7c1e99..538ae170c2 100644 --- a/src/bin/netconf/unix_control_socket.h +++ b/src/bin/netconf/unix_control_socket.h @@ -1,4 +1,4 @@ -// Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2018-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -29,7 +29,7 @@ public: UnixControlSocket(CfgControlSocketPtr ctrl_sock); /// @brief Destructor (does nothing). - virtual ~UnixControlSocket(); + virtual ~UnixControlSocket() = default; /// @brief Get configuration. /// -- 2.47.3