Skip to content

Commit 62443b0

Browse files
zbud-msftmssonicbld
authored andcommitted
[eventd]: Close rsyslog plugin when rsyslog SIGTERM and EOF is sent to stream (#18835)
Fix #18771 Microsoft ADO (number only):27882794 How I did it Add signalOnClose for omprog as well as close rsyslog plugin when receives an EOF. How to verify it Verify rsyslog_plugin is running inside bgp or swss container Run docker exec -it bgp supervisorctl restart rsyslogd Before change: This will not kill current rsyslog_plugin process but instead rsyslogd will now break off its end of writing to cin and send EOF to rsyslog_plugin, however will not send a signal SIGTERM or SIGKILL to rsyslog_plugin. Therefore, rsyslog plugin will run in an infinite loop forever, constantly calling getline raising CPU to 100% inside docker. After change of adding signalOnClose="on" to conf file inside omprog, rsyslogd will now send SIGTERM to rsyslog_plugin process running inside container, and rsyslog_plugin will die. ? ( ): rsyslog_plugin/578637 ... [continued]: read()) = -1 (unknown) (INTERNAL ERROR: strerror_r(512, [buf], 128)=22) UT (will add sonic-mgmt testcase for storming events with logs) RCA: 1. When rsyslogd is terminated, no signal is sent to child process of rsyslog_plugin meaning that rsyslog_plugin will be constantly trying to read from cin with no writer on the other end of the pipe. This leads to rsyslog_plugin process will constantly be reading via getline infinitely. 2. Because rsyslog is terminated and the spawned rsyslog_plugin is still alive, when rsyslog starts backup again, and log is triggered, a new rsyslog_plugin will be spawned for that rsyslog process, which can lead to many "ghost" rsyslog_plugin processes that will be at high CPU usage.
1 parent 4fb785e commit 62443b0

File tree

4 files changed

+50
-12
lines changed

4 files changed

+50
-12
lines changed

files/build_templates/rsyslog_plugin.conf.j2

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ if re_match($programname, "{{ proc.name }}") then {
1212
action(type="omprog"
1313
binary="/usr/bin/rsyslog_plugin -r /etc/rsyslog.d/{{ proc.parse_json }} -m {{ yang_module }}"
1414
output="/var/log/rsyslog_plugin.log"
15+
signalOnClose="on"
1516
template="prog_msg")
1617
}
1718
{% endfor %}

src/sonic-eventd/rsyslog_plugin/rsyslog_plugin.cpp

+15-12
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
using json = nlohmann::json;
1111

12+
bool RsyslogPlugin::g_running;
13+
1214
bool RsyslogPlugin::onMessage(string msg, lua_State* luaState) {
1315
string tag;
1416
event_params_t paramDict;
@@ -30,18 +32,18 @@ void parseParams(vector<string> params, vector<EventParam>& eventParams) {
3032
if(params[i].empty()) {
3133
SWSS_LOG_ERROR("Empty param provided in regex file\n");
3234
continue;
33-
}
35+
}
3436
EventParam ep = EventParam();
3537
auto delimPos = params[i].find(':');
3638
if(delimPos == string::npos) { // no lua code
3739
ep.paramName = params[i];
3840
} else {
3941
ep.paramName = params[i].substr(0, delimPos);
40-
ep.luaCode = params[i].substr(delimPos + 1);
42+
ep.luaCode = params[i].substr(delimPos + 1);
4143
if(ep.luaCode.empty()) {
4244
SWSS_LOG_ERROR("Lua code missing after :\n");
4345
}
44-
}
46+
}
4547
eventParams.push_back(ep);
4648
}
4749
}
@@ -71,25 +73,25 @@ bool RsyslogPlugin::createRegexList() {
7173
vector<EventParam> eventParams;
7274
try {
7375
string eventRegex = jsonList[i]["regex"];
74-
regexString = timestampRegex + eventRegex;
76+
regexString = timestampRegex + eventRegex;
7577
string tag = jsonList[i]["tag"];
7678
vector<string> params = jsonList[i]["params"];
77-
vector<string> timestampParams = { "month", "day", "time" };
78-
params.insert(params.begin(), timestampParams.begin(), timestampParams.end());
79+
vector<string> timestampParams = { "month", "day", "time" };
80+
params.insert(params.begin(), timestampParams.begin(), timestampParams.end());
7981
regex expr(regexString);
8082
expression = expr;
8183
parseParams(params, eventParams);
8284
rs.params = eventParams;
8385
rs.tag = tag;
8486
rs.regexExpression = expression;
8587
regexList.push_back(rs);
86-
} catch (nlohmann::detail::type_error& deException) {
88+
} catch (nlohmann::detail::type_error& deException) {
8789
SWSS_LOG_ERROR("Missing required key, throws exception: %s\n", deException.what());
8890
return false;
8991
} catch (regex_error& reException) {
9092
SWSS_LOG_ERROR("Invalid regex, throws exception: %s\n", reException.what());
91-
return false;
92-
}
93+
return false;
94+
}
9395
}
9496

9597
if(regexList.empty()) {
@@ -104,11 +106,11 @@ bool RsyslogPlugin::createRegexList() {
104106
}
105107

106108
void RsyslogPlugin::run() {
109+
signal(SIGTERM, RsyslogPlugin::signalHandler);
107110
lua_State* luaState = luaL_newstate();
108111
luaL_openlibs(luaState);
109-
while(true) {
110-
string line;
111-
getline(cin, line);
112+
string line;
113+
while(RsyslogPlugin::g_running && getline(cin, line)) {
112114
if(line.empty()) {
113115
continue;
114116
}
@@ -132,4 +134,5 @@ RsyslogPlugin::RsyslogPlugin(string moduleName, string regexPath) {
132134
m_parser = unique_ptr<SyslogParser>(new SyslogParser());
133135
m_moduleName = moduleName;
134136
m_regexPath = regexPath;
137+
RsyslogPlugin::g_running = true;
135138
}

src/sonic-eventd/rsyslog_plugin/rsyslog_plugin.h

+8
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ extern "C"
99
}
1010
#include <string>
1111
#include <memory>
12+
#include <csignal>
1213
#include "syslog_parser.h"
1314
#include "events.h"
1415
#include "logger.h"
@@ -24,10 +25,17 @@ using namespace swss;
2425

2526
class RsyslogPlugin {
2627
public:
28+
static bool g_running;
2729
int onInit();
2830
bool onMessage(string msg, lua_State* luaState);
2931
void run();
3032
RsyslogPlugin(string moduleName, string regexPath);
33+
static void signalHandler(int signum) {
34+
if (signum == SIGTERM) {
35+
SWSS_LOG_INFO("Rsyslog plugin received SIGTERM, shutting down");
36+
RsyslogPlugin::g_running = false;
37+
}
38+
}
3139
private:
3240
unique_ptr<SyslogParser> m_parser;
3341
event_handle_t m_eventHandle;

src/sonic-eventd/rsyslog_plugin_tests/rsyslog_plugin_ut.cpp

+26
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ extern "C"
88
#include <fstream>
99
#include <memory>
1010
#include <regex>
11+
#include <thread>
1112
#include "gtest/gtest.h"
1213
#include <nlohmann/json.hpp>
1314
#include "events.h"
@@ -253,6 +254,31 @@ TEST(rsyslog_plugin, onMessage_noParams) {
253254
infile.close();
254255
}
255256

257+
TEST(rsyslog_plugin, run) {
258+
unique_ptr<RsyslogPlugin> plugin(new RsyslogPlugin("test_mod_name", "./rsyslog_plugin_tests/test_regex_5.rc.json"));
259+
EXPECT_EQ(0, plugin->onInit());
260+
istringstream ss("");
261+
streambuf* cinbuf = cin.rdbuf();
262+
cin.rdbuf(ss.rdbuf());
263+
plugin->run();
264+
cin.rdbuf(cinbuf);
265+
}
266+
267+
TEST(rsyslog_plugin, run_SIGTERM) {
268+
unique_ptr<RsyslogPlugin> plugin(new RsyslogPlugin("test_mod_name", "./rsyslog_plugin_tests/test_regex_5.rc.json"));
269+
EXPECT_EQ(0, plugin->onInit());
270+
EXPECT_TRUE(RsyslogPlugin::g_running);
271+
thread pluginThread([&]() {
272+
plugin->run();
273+
});
274+
275+
RsyslogPlugin::signalHandler(SIGTERM);
276+
277+
pluginThread.join();
278+
279+
EXPECT_FALSE(RsyslogPlugin::g_running);
280+
}
281+
256282
TEST(timestampFormatter, changeTimestampFormat) {
257283
unique_ptr<TimestampFormatter> formatter(new TimestampFormatter());
258284

0 commit comments

Comments
 (0)