From 98c44d34cf5e2c36c5aa45a1e13744ca40ec729d Mon Sep 17 00:00:00 2001 From: Bryan Crossland Date: Thu, 29 Sep 2022 14:37:02 -0500 Subject: [PATCH] What I did Fix Azure/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 bryan.crossland@target.com --- orchagent/orchdaemon.cpp | 1 - orchagent/orchdaemon.h | 1 + tests/mock_tests/Makefile.am | 7 +++--- tests/mock_tests/orchdaemon_ut.cpp | 35 ++++++++++++++++++------------ 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 8655c662a9..cd6c94c213 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -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); diff --git a/orchagent/orchdaemon.h b/orchagent/orchdaemon.h index 20a68be3de..998e72335a 100644 --- a/orchagent/orchdaemon.h +++ b/orchagent/orchdaemon.h @@ -45,6 +45,7 @@ #include "bfdorch.h" #include "srv6orch.h" #include "nvgreorch.h" +#include using namespace swss; extern bool gSaiRedisLogRotate; diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index 97e5a17448..51b694554b 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -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 \ @@ -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 diff --git a/tests/mock_tests/orchdaemon_ut.cpp b/tests/mock_tests/orchdaemon_ut.cpp index eaff7c8c6e..ab3e4a226c 100644 --- a/tests/mock_tests/orchdaemon_ut.cpp +++ b/tests/mock_tests/orchdaemon_ut.cpp @@ -1,34 +1,40 @@ #include "orchdaemon.h" #include "dbconnector.h" #include -//#include -//#include "mock_sai_switch.h" +#include +#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 mock_sai_switch_; + public: + StrictMock 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() @@ -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(); } }