Skip to content

Commit

Permalink
What I did
Browse files Browse the repository at this point in the history
Fix sonic-net/sonic-buildimage#8162

Moved sairedis record file rotation logic out of flush() to fix issue.

Why I did it
Sairedis record file was not releasing the file handle on rotation. This is because the file handle release was inside the flush() which was only being called if a select timeout was triggered. Moved the logic to its own function which is called in the start() loop.

How I verified it
Ran a script to fill log and verified that rotation was happening correctly.

Signed-off-by: Bryan Crossland [email protected]
  • Loading branch information
bacrossland committed Sep 29, 2022
1 parent 4e0e66b commit 98c44d3
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 18 deletions.
1 change: 0 additions & 1 deletion orchagent/orchdaemon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,6 @@ void OrchDaemon::logRotate() {
attr.id = SAI_REDIS_SWITCH_ATTR_PERFORM_LOG_ROTATE;
attr.value.booldata = true;
sai_status_t status = sai_switch_api->set_switch_attribute(gSwitchId, &attr);
return;
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to release the file handle on sairedis log %d", status);
Expand Down
1 change: 1 addition & 0 deletions orchagent/orchdaemon.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include "bfdorch.h"
#include "srv6orch.h"
#include "nvgreorch.h"
#include <sairedis.h>

using namespace swss;
extern bool gSaiRedisLogRotate;
Expand Down
7 changes: 4 additions & 3 deletions tests/mock_tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ LDADD_GTEST = -L/usr/src/gtest

## Orchagent Unit Tests

tests_INCLUDES = -I $(FLEX_CTR_DIR) -I $(DEBUG_CTR_DIR) -I $(top_srcdir)/lib -I$(top_srcdir)/cfgmgr -I$(top_srcdir)/orchagent
tests_INCLUDES = -I $(FLEX_CTR_DIR) -I $(DEBUG_CTR_DIR) -I $(top_srcdir)/lib -I$(top_srcdir)/cfgmgr -I$(top_srcdir)/orchagent -I$(P4_ORCH_DIR)/tests

tests_SOURCES = aclorch_ut.cpp \
portsorch_ut.cpp \
Expand Down Expand Up @@ -121,12 +121,13 @@ tests_SOURCES += $(P4_ORCH_DIR)/p4orch.cpp \
$(P4_ORCH_DIR)/wcmp_manager.cpp \
$(P4_ORCH_DIR)/mirror_session_manager.cpp \
$(P4_ORCH_DIR)/gre_tunnel_manager.cpp \
$(P4_ORCH_DIR)/l3_admit_manager.cpp
$(P4_ORCH_DIR)/l3_admit_manager.cpp \
$(P4_ORCH_DIR)/tests/mock_sai_switch.cpp

tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI)
tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) $(tests_INCLUDES)
tests_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis -lpthread \
-lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3
-lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lgmock -lgmock_main

## portsyncd unit tests

Expand Down
35 changes: 21 additions & 14 deletions tests/mock_tests/orchdaemon_ut.cpp
Original file line number Diff line number Diff line change
@@ -1,34 +1,40 @@
#include "orchdaemon.h"
#include "dbconnector.h"
#include <gtest/gtest.h>
//#include <gmock/gmock.h>
//#include "mock_sai_switch.h"
#include <gmock/gmock.h>
#include "mock_sai_switch.h"

extern sai_switch_api_t* sai_switch_api;
sai_switch_api_t test_sai_switch;

namespace orchdaemon_test
{

using ::testing::_;
using ::testing::Return;
using ::testing::StrictMock;

DBConnector appl_db("APPL_DB", 0);
DBConnector state_db("STATE_DB", 0);
DBConnector config_db("CONFIG_DB", 0);
DBConnector counters_db("COUNTERS_DB", 0);
DBConnector *gAppDb = &appl_db;
DBConnector *gStateDb = &state_db;
DBConnector *gConfigDb = &config_db;
DBConnector *gCountersDb = &counters_db;

struct OrchDaemonTest : public ::testing::Test
class OrchDaemonTest : public ::testing::Test
{
protected:
// StrictMock<MockSaiSwitch> mock_sai_switch_;
public:
StrictMock<MockSaiSwitch> mock_sai_switch_;

OrchDaemon* orchd;

OrchDaemonTest()
{
// mock_sai_switch = &mock_sai_switch_;
mock_sai_switch = &mock_sai_switch_;
sai_switch_api = &test_sai_switch;
sai_switch_api->get_switch_attribute = &mock_get_switch_attribute;
sai_switch_api->set_switch_attribute = &mock_set_switch_attribute;

orchd = new OrchDaemon(gAppDb, gConfigDb, gStateDb, gCountersDb);
orchd = new OrchDaemon(&appl_db, &config_db, &state_db, &counters_db);

// orchd = new OrchDaemon(appl_db, config_db, state_db, counters_db);
};

~OrchDaemonTest()
Expand All @@ -39,7 +45,8 @@ namespace orchdaemon_test

TEST_F(OrchDaemonTest, logRotate)
{
// EXPECT_CALL(mock_sai_switch_, set_switch_attribute( _, _)).WillOnce(Return(SAI_STATUS_SUCCESS));
EXPECT_NO_THROW(orchd->logRotate());
EXPECT_CALL(mock_sai_switch_, set_switch_attribute( _, _)).WillOnce(Return(SAI_STATUS_SUCCESS));

orchd->logRotate();
}
}

0 comments on commit 98c44d3

Please sign in to comment.