Skip to content

GitLab

Commit 5b8d7823 authored by Francis Dupont's avatar Francis Dupont
Browse files
Options

[5150a] Merged review improvements

parent 8de4264d
Hide whitespace changes
Inline Side-by-side
Showing with 43 additions and 32 deletions
src/bin/dhcp4/ctrl_dhcp4_srv.cc
View file @ 5b8d7823
......@@ -6,6 +6,7 @@
#include <config.h>
#include <cc/data.h>
#include <cc/command_interpreter.h>
#include <dhcp4/ctrl_dhcp4_srv.h>
#include <dhcp4/dhcp4_log.h>
#include <dhcp4/dhcp4to6_ipc.h>
......@@ -148,7 +149,7 @@ ControlledDhcpv4Srv::commandConfigWriteHandler(const string&,
ConstElementPtr
ControlledDhcpv4Srv::commandSetConfigHandler(const string&,
ConstElementPtr args) {
const int status_code = 1; // 1 indicates an error
const int status_code = CONTROL_RESULT_ERROR; // 1 indicates an error
ConstElementPtr dhcp4;
string message;
......@@ -209,7 +210,7 @@ ControlledDhcpv4Srv::commandSetConfigHandler(const string&,
ConstElementPtr
ControlledDhcpv4Srv::commandConfigTestHandler(const string&,
ConstElementPtr args) {
const int status_code = 1; // 1 indicates an error
const int status_code = CONTROL_RESULT_ERROR; // 1 indicates an error
ConstElementPtr dhcp4;
string message;
......@@ -266,7 +267,7 @@ ControlledDhcpv4Srv::commandBuildReportHandler(const string&,
ConstElementPtr
ControlledDhcpv4Srv::commandLeasesReclaimHandler(const string&,
ConstElementPtr args) {
int status_code = 1;
int status_code = CONTROL_RESULT_ERROR;
string message;
// args must be { "remove": <bool> }
......
src/bin/dhcp4/ctrl_dhcp4_srv.h
View file @ 5b8d7823
......@@ -60,9 +60,10 @@ public:
/// in them.
///
/// Currently supported commands are:
/// - config-reload
/// - config-test
/// - shutdown
/// - libreload
/// - config-reload
/// - leases-reclaim
/// ...
///
......@@ -94,7 +95,7 @@ public:
///
/// This is a method for checking incoming configuration.
///
/// @param new_config textual representation of the new configuration
/// @param new_config JSON representation of the new configuration
///
/// @return status of the config check
isc::data::ConstElementPtr
......
src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
View file @ 5b8d7823
......@@ -696,7 +696,8 @@ TEST_F(CtrlChannelDhcpv4SrvTest, set_config) {
// Should fail with a syntax error
EXPECT_EQ("{ \"result\": 1, "
"\"text\": \"subnet configuration failed: mandatory 'subnet' parameter is missing for a subnet being configured (<string>:20:17)\" }",
"\"text\": \"subnet configuration failed: mandatory 'subnet' "
"parameter is missing for a subnet being configured (<string>:20:17)\" }",
response);
// Check that the config was not lost
......@@ -716,13 +717,13 @@ TEST_F(CtrlChannelDhcpv4SrvTest, set_config) {
<< "}\n" // close dhcp4
<< "}}";
/* Verify the control channel socket exists */
// Verify the control channel socket exists.
ASSERT_TRUE(fileExists(socket_path_));
// Send the set-config command
// Send the set-config command.
sendUnixCommand(os.str(), response);
/* Verify the control channel socket no longer exists */
// Verify the control channel socket no longer exists.
EXPECT_FALSE(fileExists(socket_path_));
// With no command channel, should still receive the response.
......@@ -893,7 +894,8 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configTest) {
// Should fail with a syntax error
EXPECT_EQ("{ \"result\": 1, "
"\"text\": \"subnet configuration failed: mandatory 'subnet' parameter is missing for a subnet being configured (<string>:20:17)\" }",
"\"text\": \"subnet configuration failed: mandatory 'subnet' "
"parameter is missing for a subnet being configured (<string>:20:17)\" }",
response);
// Check that the config was not lost
......@@ -912,17 +914,19 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configTest) {
<< "}\n" // close dhcp4
<< "}}";
/* Verify the control channel socket exists */
// Verify the control channel socket exists.
ASSERT_TRUE(fileExists(socket_path_));
// Send the config-test command
sendUnixCommand(os.str(), response);
/* Verify the control channel socket still exists */
// Verify the control channel socket still exists.
EXPECT_TRUE(fileExists(socket_path_));
// Verify the configuration was successful.
EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration seems sane. Control-socket, hook-libraries, and D2 configuration were sanity checked, but not applied.\" }",
EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration seems sane. "
"Control-socket, hook-libraries, and D2 configuration were "
"sanity checked, but not applied.\" }",
response);
// Check that the config was not applied
......
src/bin/dhcp6/ctrl_dhcp6_srv.cc
View file @ 5b8d7823
......@@ -6,6 +6,7 @@
#include <config.h>
#include <cc/data.h>
#include <cc/command_interpreter.h>
#include <config/command_mgr.h>
#include <dhcp/libdhcp++.h>
#include <dhcpsrv/cfgmgr.h>
......@@ -153,7 +154,7 @@ ControlledDhcpv6Srv::commandConfigWriteHandler(const string&, ConstElementPtr ar
ConstElementPtr
ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
ConstElementPtr args) {
const int status_code = 1; // 1 indicates an error
const int status_code = CONTROL_RESULT_ERROR;
ConstElementPtr dhcp6;
string message;
......@@ -201,7 +202,7 @@ ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
// the logging first in case there's a configuration failure.
int rcode = 0;
isc::config::parseAnswer(rcode, result);
if (rcode == 0) {
if (rcode == CONTROL_RESULT_SUCCESS) {
CfgMgr::instance().getStagingCfg()->applyLoggingCfg();
// Use new configuration.
......@@ -214,7 +215,7 @@ ControlledDhcpv6Srv::commandSetConfigHandler(const string&,
ConstElementPtr
ControlledDhcpv6Srv::commandConfigTestHandler(const string&,
ConstElementPtr args) {
const int status_code = 1; // 1 indicates an error
const int status_code = CONTROL_RESULT_ERROR; // 1 indicates an error
ConstElementPtr dhcp6;
string message;
......@@ -359,8 +360,9 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) {
ControlledDhcpv6Srv* srv = ControlledDhcpv6Srv::getInstance();
if (!srv) {
ConstElementPtr no_srv = isc::config::createAnswer(1,
"Server object not initialized, can't process config.");
ConstElementPtr no_srv = isc::config::createAnswer(
CONTROL_RESULT_ERROR,
"Server object not initialized, can't process config.");
return (no_srv);
}
......
src/bin/dhcp6/ctrl_dhcp6_srv.h
View file @ 5b8d7823
......@@ -60,10 +60,11 @@ public:
/// in them.
///
/// Currently supported commands are:
/// - shutdown
/// - libreload
/// - config-reload
/// - config-test
/// - leases-reclaim
/// - libreload
/// - shutdown
/// ...
///
/// @note It never throws.
......@@ -94,7 +95,7 @@ public:
///
/// This is a method for checking incoming configuration.
///
/// @param new_config textual representation of the new configuration
/// @param new_config JSON representation of the new configuration
///
/// @return status of the config check
isc::data::ConstElementPtr
......
src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
View file @ 5b8d7823
......@@ -430,7 +430,7 @@ TEST_F(CtrlDhcpv6SrvTest, configReload) {
}
// Check that the "set-config" command will replace current configuration
TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
TEST_F(CtrlChannelDhcpv6SrvTest, configSet) {
createUnixChannelServer();
// Define strings to permutate the config arguments
......@@ -552,13 +552,13 @@ TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
<< "}\n" // close dhcp6
<< "}}";
/* Verify the control channel socket exists */
// Verify the control channel socket exists.
ASSERT_TRUE(fileExists(socket_path_));
// Send the set-config command
// Send the set-config command.
sendUnixCommand(os.str(), response);
/* Verify the control channel socket no longer exists */
// Verify the control channel socket no longer exists.
EXPECT_FALSE(fileExists(socket_path_));
// With no command channel, should still receive the response.
......@@ -574,7 +574,7 @@ TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
}
// Verify that the "config-test" command will do what we expect.
TEST_F(CtrlChannelDhcpv6SrvTest, config_test) {
TEST_F(CtrlChannelDhcpv6SrvTest, configTest) {
createUnixChannelServer();
// Define strings to permutate the config arguments
......@@ -677,7 +677,8 @@ TEST_F(CtrlChannelDhcpv6SrvTest, config_test) {
// Should fail with a syntax error
EXPECT_EQ("{ \"result\": 1, "
"\"text\": \"subnet configuration failed: mandatory 'subnet' parameter is missing for a subnet being configured (<string>:21:17)\" }",
"\"text\": \"subnet configuration failed: mandatory 'subnet' parameter "
"is missing for a subnet being configured (<string>:21:17)\" }",
response);
// Check that the config was not lost
......@@ -696,17 +697,19 @@ TEST_F(CtrlChannelDhcpv6SrvTest, config_test) {
<< "}\n" // close dhcp6
<< "}}";
/* Verify the control channel socket exists */
// Verify the control channel socket exists.
ASSERT_TRUE(fileExists(socket_path_));
// Send the config-test command
// Send the config-test command.
sendUnixCommand(os.str(), response);
/* Verify the control channel socket still exists */
// Verify the control channel socket still exists.
EXPECT_TRUE(fileExists(socket_path_));
// Verify the configuration was successful.
EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration seems sane. Control-socket, hook-libraries, and D2 configuration were sanity checked, but not applied.\" }",
EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration seems sane. "
"Control-socket, hook-libraries, and D2 configuration were "
"sanity checked, but not applied.\" }",
response);
// Check that the config was not applied.
......@@ -717,7 +720,6 @@ TEST_F(CtrlChannelDhcpv6SrvTest, config_test) {
CfgMgr::instance().clear();
}
typedef std::map<std::string, isc::data::ConstElementPtr> ElementMap;
// This test checks which commands are registered by the DHCPv4 server.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment