Commit e5a6c2b4 authored by Thomas Markwalder's avatar Thomas Markwalder

[5046] CommandMgr dups the connection socket prior to executing command

src/lib/config/command_mgr.cc
    CommandMgr::commandReader(int sockfd) - duplicates the connection
    socket to use for repsonding in case the command closes the channel.

src/lib/testutils/io_utils.cc
    fileExists() - now uses stat() function so one can use it on any
    type of file, like a unix socket

updated unit tests accordingly
parent 7289e654
......@@ -16,6 +16,7 @@
#include <hooks/hooks_manager.h>
#include <log/logger_support.h>
#include <stats/stats_mgr.h>
#include <testutils/io_utils.h>
#include <testutils/unix_control_client.h>
#include "marker_file.h"
......@@ -592,7 +593,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, set_config) {
EXPECT_EQ(1, subnets->size());
// Create a valid config with two subnets and no command channel.
// It should succeed but client will not receive a the response
// It should succeed, client should still receive the response
os.str("");
os << set_config_txt << ","
<< args_txt
......@@ -604,11 +605,18 @@ TEST_F(CtrlChannelDhcpv4SrvTest, set_config) {
<< "}\n" // close dhcp4
<< "}}";
/* Verify the control channel socket exists */
ASSERT_TRUE(fileExists(socket_path_));
// Send the set-config command
sendUnixCommand(os.str(), response);
// With no command channel, no response
EXPECT_EQ("", response);
/* Verify the control channel socket no longer exists */
EXPECT_FALSE(fileExists(socket_path_));
// With no command channel, should still receive the response.
EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration successful.\" }",
response);
// Check that the config was not lost
subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll();
......
......@@ -18,6 +18,7 @@
#include <log/logger_support.h>
#include <stats/stats_mgr.h>
#include <testutils/unix_control_client.h>
#include <testutils/io_utils.h>
#include "marker_file.h"
#include "test_libraries.h"
......@@ -26,6 +27,7 @@
#include <gtest/gtest.h>
#include <sys/select.h>
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <cstdlib>
......@@ -468,11 +470,18 @@ TEST_F(CtrlChannelDhcpv6SrvTest, set_config) {
<< "}\n" // close dhcp6
<< "}}";
/* Verify the control channel socket exists */
ASSERT_TRUE(fileExists(socket_path_));
// Send the set-config command
sendUnixCommand(os.str(), response);
// With no command channel, no response
EXPECT_EQ("", response);
/* Verify the control channel socket no longer exists */
EXPECT_FALSE(fileExists(socket_path_));
// With no command channel, should still receive the response.
EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration successful.\" }",
response);
// Check that the config was not lost
subnets = CfgMgr::instance().getCurrentCfg()->getCfgSubnets6()->getAll();
......
......@@ -11,6 +11,7 @@
#include <dhcp/iface_mgr.h>
#include <config/config_log.h>
#include <boost/bind.hpp>
#include <unistd.h>
using namespace isc::data;
......@@ -145,6 +146,19 @@ CommandMgr::commandReader(int sockfd) {
return;
}
// Duplicate the connection's socket in the event, the command causes the
// channel to close (like a reconfig). This permits us to always have
// a socket on which to respond. If for some reason we can't fall back
// to the connection socket.
int rsp_fd = dup(sockfd);
if (rsp_fd < 0 ) {
// Highly unlikely
const char* errmsg = strerror(errno);
LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_DUP_WARN)
.arg(errmsg);
rsp_fd = sockfd;
}
LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_READ).arg(rval).arg(sockfd);
// Ok, we received something. Let's see if we can make any sense of it.
......@@ -163,6 +177,11 @@ CommandMgr::commandReader(int sockfd) {
if (!rsp) {
LOG_WARN(command_logger, COMMAND_RESPONSE_ERROR);
// Only close the dupped socket if it's different (should be)
if (rsp_fd != sockfd) {
close(rsp_fd);
}
return;
}
......@@ -179,7 +198,8 @@ CommandMgr::commandReader(int sockfd) {
}
// Send the data back over socket.
rval = write(sockfd, txt.c_str(), len);
rval = write(rsp_fd, txt.c_str(), len);
int saverr = errno;
LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_WRITE).arg(len).arg(sockfd);
......@@ -187,7 +207,13 @@ CommandMgr::commandReader(int sockfd) {
// Response transmission failed. Since the response failed, it doesn't
// make sense to send any status codes. Let's log it and be done with
// it.
LOG_ERROR(command_logger, COMMAND_SOCKET_WRITE_FAIL).arg(len).arg(sockfd);
LOG_ERROR(command_logger, COMMAND_SOCKET_WRITE_FAIL)
.arg(len).arg(sockfd).arg(strerror(saverr));
}
// Only close the dupped socket if it's different (should be)
if (rsp_fd != sockfd) {
close(rsp_fd);
}
}
......
......@@ -57,6 +57,13 @@ This error message indicates that the server failed to set non-blocking mode
on just created socket. That socket was created for accepting specific
incoming connection. Additional information may be provided as third parameter.
% COMMAND_SOCKET_DUP_WARN Failed to duplicate socket for response: %1
This debug message indicates that the commandReader was unable to duplicate
the connection socket prior to executing the command. This is most likely a
system resource issue. The command should still be processed and the response
sent, unless the command caused the command channel to be closed (e.g. a
reconfiguration command).
% COMMAND_SOCKET_READ Received %1 bytes over command socket %2
This debug message indicates that specified number of bytes was received
over command socket identified by specified file descriptor.
......@@ -86,6 +93,6 @@ descriptor and path specified.
This debug message indicates that the specified number of bytes was sent
over command socket identifier by the specified file descriptor.
% COMMAND_SOCKET_WRITE_FAIL Error while writing %1 bytes to command socket %2
% COMMAND_SOCKET_WRITE_FAIL Error while writing %1 bytes to command socket %2 : %3
This error message indicates that an error was encountered while
attempting to send a response to the command socket.
......@@ -15,10 +15,8 @@ namespace dhcp {
namespace test {
bool fileExists(const std::string& file_path) {
std::ifstream fs(file_path.c_str());
const bool file_exists = fs.good();
fs.close();
return (file_exists);
struct stat statbuf;
return(stat(file_path.c_str(), &statbuf) == 0);
}
std::string readFile(const std::string& file_path) {
......
......@@ -8,6 +8,7 @@
#define TEST_IO_UTILS_H
#include <string>
#include <sys/stat.h>
namespace isc {
namespace dhcp {
......
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