Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UT] [Portsyncd] Added Unit Tests for portsyncd #2297

Merged
merged 29 commits into from
Sep 1, 2022
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
1c6cd64
Refactored portsyncd structure to improve testability
vivekrnv Apr 27, 2022
0ce3d92
Merge branch 'Azure:master' into ut_infra_improv
vivekrnv Apr 27, 2022
7395544
Skeleton for portsyncd ut added
vivekrnv Apr 27, 2022
b98badf
Merge branch 'ut_infra_improv' of https://github.com/vivekreddynv/son…
vivekrnv Apr 27, 2022
dc384cd
Minor update
vivekrnv Apr 27, 2022
bc7528a
Class Init and reading from cfg_db ut's added
vivekrnv Apr 28, 2022
e052e20
Added missing comment
vivekrnv Apr 28, 2022
e267fd9
onMsg test added
vivekrnv Apr 28, 2022
67a086d
2 new test cases added
vivekrnv Apr 29, 2022
64a8ee1
Another case added
vivekrnv Apr 29, 2022
6909333
Push infra modifications
vivekrnv Apr 29, 2022
7f26e83
Moved common methods/ds to a different file
vivekrnv May 3, 2022
0402330
bullseye link issue handled
vivekrnv May 9, 2022
0523b8c
Merge branch 'master' of https://github.com/Azure/sonic-swss into ut_…
vivekrnv May 20, 2022
8a3c138
Update UT to include ASAN changes
vivekrnv May 20, 2022
dfcba64
Restored original del func on mock_table
vivekrnv May 20, 2022
1bc2d97
Refactored mock shell cmd
vivekrnv May 21, 2022
6134e80
Minor fix
vivekrnv May 21, 2022
f757668
Merge branch 'master' of https://github.com/Azure/sonic-swss into ut_…
vivekrnv Aug 2, 2022
06edeb9
Refactored mock_tests/ folder
vivekrnv Aug 2, 2022
98b6358
p4orch tests updated
vivekrnv Aug 2, 2022
d436281
Revert "p4orch tests updated"
vivekrnv Aug 5, 2022
70099b4
Revert "Refactored mock_tests/ folder"
vivekrnv Aug 5, 2022
41d31e8
Moved the fun/global vars to orig location and removed tests
vivekrnv Aug 27, 2022
94b9e30
Minor deviations
vivekrnv Aug 27, 2022
aa8e7e7
Minor deviations
vivekrnv Aug 27, 2022
1edb07c
Merge branch 'sonic-net:master' into ut_infra_improv
vivekrnv Aug 28, 2022
42cbbcf
Merge branch 'sonic-net:master' into ut_infra_improv
vivekrnv Aug 29, 2022
e974c82
Merge branch 'sonic-net:master' into ut_infra_improv
vivekrnv Aug 30, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion portsyncd/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ else
DBGFLAGS = -g
endif

portsyncd_SOURCES = $(top_srcdir)/lib/gearboxutils.cpp portsyncd.cpp linksync.cpp $(top_srcdir)/cfgmgr/shellcmd.h
portsyncd_SOURCES = $(top_srcdir)/lib/gearboxutils.cpp portsyncd.cpp linksync.cpp portsyncd_helper.cpp $(top_srcdir)/cfgmgr/shellcmd.h

portsyncd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_ASAN)
portsyncd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_ASAN)
Expand Down
5 changes: 2 additions & 3 deletions portsyncd/linksync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
#include <netlink/route/link.h>
#include "logger.h"
#include "netmsg.h"
#include "dbconnector.h"
#include "producerstatetable.h"
#include "tokenize.h"
#include "exec.h"

Expand Down Expand Up @@ -90,7 +88,7 @@ LinkSync::LinkSync(DBConnector *appl_db, DBConnector *state_db) :

if (!WarmStart::isWarmStart())
{
/* See the comments for g_portSet in portsyncd.cpp */
/* See the comments for g_portSet */
for (auto port_iter = g_portSet.begin(); port_iter != g_portSet.end();)
{
string port = *port_iter;
Expand Down Expand Up @@ -260,3 +258,4 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
SWSS_LOG_NOTICE("Cannot find %s in port table", key.c_str());
}
}

10 changes: 10 additions & 0 deletions portsyncd/linksync.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,16 @@
#include "dbconnector.h"
#include "producerstatetable.h"
#include "netmsg.h"
#include "exec.h"
#include "warm_restart.h"
#include "shellcmd.h"

#include <map>
#include <set>
#include <vector>
#include <set>
#include <map>
#include <list>

namespace swss {

Expand All @@ -28,4 +36,6 @@ class LinkSync : public NetMsg

}

void handlePortConfigFromConfigDB(swss::ProducerStateTable &, swss::DBConnector &, bool );

#endif
72 changes: 2 additions & 70 deletions portsyncd/portsyncd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,22 @@
#include <iostream>
#include <fstream>
#include <sstream>
#include <vector>
#include <set>
#include <map>
#include <list>
#include <sys/stat.h>
#include "dbconnector.h"
#include "select.h"
#include "netdispatcher.h"
#include "netlink.h"
#include "producerstatetable.h"
#include "portsyncd/linksync.h"
#include "subscriberstatetable.h"
#include "exec.h"
#include "warm_restart.h"

using namespace std;
using namespace swss;

#define DEFAULT_SELECT_TIMEOUT 1000 /* ms */

/*
* This g_portSet contains all the front panel ports that the corresponding
* host interfaces needed to be created. When this LinkSync class is
* initialized, we check the database to see if some of the ports' host
* interfaces are already created and remove them from this set. We will
* remove the rest of the ports in the set when receiving the first netlink
* message indicating that the host interfaces are created. After the set
* is empty, we send out the signal PortInitDone. g_init is used to limit the
* command to be run only once.
*/
set<string> g_portSet;
bool g_init = false;
extern set<string> g_portSet;
extern bool g_init;

void usage()
{
Expand All @@ -42,11 +26,6 @@ void usage()
cout << " this program will exit if configDB does not contain that info" << endl;
}

void handlePortConfigFile(ProducerStateTable &p, string file, bool warm);
void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb, bool warm);
void handleVlanIntfFile(string file);
void checkPortInitDone(DBConnector *appl_db);

int main(int argc, char **argv)
{
Logger::linkToDbNative("portsyncd");
Expand Down Expand Up @@ -152,50 +131,3 @@ int main(int argc, char **argv)

return 1;
}

static void notifyPortConfigDone(ProducerStateTable &p)
{
/* Notify that all ports added */
FieldValueTuple finish_notice("count", to_string(g_portSet.size()));
vector<FieldValueTuple> attrs = { finish_notice };
p.set("PortConfigDone", attrs);
}

void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb, bool warm)
{
SWSS_LOG_ENTER();

SWSS_LOG_NOTICE("Getting port configuration from ConfigDB...");

Table table(&cfgDb, CFG_PORT_TABLE_NAME);
std::vector<FieldValueTuple> ovalues;
std::vector<string> keys;
table.getKeys(keys);

if (keys.empty())
{
SWSS_LOG_NOTICE("ConfigDB does not have port information, "
"however ports can be added later on, continuing...");
}

for ( auto &k : keys )
{
table.get(k, ovalues);
vector<FieldValueTuple> attrs;
for ( auto &v : ovalues )
{
FieldValueTuple attr(v.first, v.second);
attrs.push_back(attr);
}
if (!warm)
{
p.set(k, attrs);
}
g_portSet.insert(k);
}
if (!warm)
{
notifyPortConfigDone(p);
}

}
64 changes: 64 additions & 0 deletions portsyncd/portsyncd_helper.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
#include "portsyncd/linksync.h"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we discussed it last time, please move this to portsyncd itself. I don't see a reason why it has to be part of helper file and cause merge conflicts for future bug fixes.

Copy link
Contributor Author

@vivekrnv vivekrnv Aug 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these are moved back to portsyncd.cpp, i won't be able to test those since i can't link my test binary with portsyncd.cpp. Because with gtest we already link it with -lgtest_main so the linker wouldn't allow another main() function to be present in any other files. Thus i need a seperate file to test handlePortConfigFromConfigDB method and g_portSet


using namespace std;
using namespace swss;

/*
* This g_portSet contains all the front panel ports that the corresponding
* host interfaces needed to be created. When this LinkSync class is
* initialized, we check the database to see if some of the ports' host
* interfaces are already created and remove them from this set. We will
* remove the rest of the ports in the set when receiving the first netlink
* message indicating that the host interfaces are created. After the set
* is empty, we send out the signal PortInitDone. g_init is used to limit the
* command to be run only once.
*/
set<string> g_portSet;
bool g_init;

static void notifyPortConfigDone(ProducerStateTable &p)
{
/* Notify that all ports added */
FieldValueTuple finish_notice("count", to_string(g_portSet.size()));
vector<FieldValueTuple> attrs = { finish_notice };
p.set("PortConfigDone", attrs);
}

void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb, bool warm)
{
SWSS_LOG_ENTER();

SWSS_LOG_NOTICE("Getting port configuration from ConfigDB...");

Table table(&cfgDb, CFG_PORT_TABLE_NAME);
std::vector<FieldValueTuple> ovalues;
std::vector<string> keys;
table.getKeys(keys);

if (keys.empty())
{
SWSS_LOG_NOTICE("ConfigDB does not have port information, "
"however ports can be added later on, continuing...");
}

for ( auto &k : keys )
{
table.get(k, ovalues);
vector<FieldValueTuple> attrs;
for ( auto &v : ovalues )
{
FieldValueTuple attr(v.first, v.second);
attrs.push_back(attr);
}
if (!warm)
{
p.set(k, attrs);
}
g_portSet.insert(k);
}
if (!warm)
{
notifyPortConfigDone(p);
}

}
58 changes: 40 additions & 18 deletions tests/mock_tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ FLEX_CTR_DIR = $(top_srcdir)/orchagent/flex_counter
DEBUG_CTR_DIR = $(top_srcdir)/orchagent/debug_counter
P4_ORCH_DIR = $(top_srcdir)/orchagent/p4orch

INCLUDES = -I $(FLEX_CTR_DIR) -I $(DEBUG_CTR_DIR) -I $(top_srcdir)/lib -I $(top_srcdir)/cfgmgr

CFLAGS_SAI = -I /usr/include/sai

TESTS = tests tests_intfmgrd
TESTS = tests tests_intfmgrd tests_portsyncd

noinst_PROGRAMS = tests tests_intfmgrd
noinst_PROGRAMS = tests tests_intfmgrd tests_portsyncd

LDADD_SAI = -lsaimeta -lsaimetadata -lsaivs -lsairedis

Expand All @@ -21,6 +19,10 @@ endif
CFLAGS_GTEST =
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_SOURCES = aclorch_ut.cpp \
portsorch_ut.cpp \
routeorch_ut.cpp \
Expand All @@ -36,10 +38,10 @@ tests_SOURCES = aclorch_ut.cpp \
mock_orchagent_main.cpp \
mock_dbconnector.cpp \
mock_consumerstatetable.cpp \
common/mock_shell_command.cpp \
mock_table.cpp \
mock_hiredis.cpp \
mock_redisreply.cpp \
mock_shell_command.cpp \
bulker_ut.cpp \
portmgr_ut.cpp \
fake_response_publisher.cpp \
Expand Down Expand Up @@ -118,26 +120,46 @@ tests_SOURCES += $(P4_ORCH_DIR)/p4orch.cpp \
$(P4_ORCH_DIR)/wcmp_manager.cpp \
$(P4_ORCH_DIR)/mirror_session_manager.cpp

tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI)
tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) -I$(top_srcdir)/orchagent
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

## portsyncd unit tests

tests_portsyncd_SOURCES = portsyncd/portsyncd_ut.cpp \
$(top_srcdir)/portsyncd/linksync.cpp \
$(top_srcdir)/portsyncd/portsyncd_helper.cpp \
mock_dbconnector.cpp \
common/mock_shell_command.cpp \
mock_table.cpp \
mock_hiredis.cpp \
mock_redisreply.cpp

tests_portsyncd_INCLUDES = -I $(top_srcdir)/portsyncd -I $(top_srcdir)/cfgmgr
tests_portsyncd_CXXFLAGS = -Wl,-wrap,if_nameindex -Wl,-wrap,if_freenameindex
tests_portsyncd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST)
tests_portsyncd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(tests_portsyncd_INCLUDES)
tests_portsyncd_LDADD = $(LDADD_GTEST) -lnl-genl-3 -lhiredis -lhiredis \
-lswsscommon -lswsscommon -lgtest -lgtest_main -lnl-3 -lnl-route-3 -lpthread

## intfmgrd unit tests

tests_intfmgrd_SOURCES = intfmgrd/add_ipv6_prefix_ut.cpp \
$(top_srcdir)/cfgmgr/intfmgr.cpp \
$(top_srcdir)/orchagent/orch.cpp \
$(top_srcdir)/orchagent/request_parser.cpp \
$(top_srcdir)/lib/subintf.cpp \
mock_orchagent_main.cpp \
mock_dbconnector.cpp \
mock_table.cpp \
mock_hiredis.cpp \
fake_response_publisher.cpp \
mock_redisreply.cpp
$(top_srcdir)/cfgmgr/intfmgr.cpp \
$(top_srcdir)/lib/subintf.cpp \
$(top_srcdir)/orchagent/orch.cpp \
$(top_srcdir)/orchagent/request_parser.cpp \
mock_orchagent_main.cpp \
mock_dbconnector.cpp \
mock_table.cpp \
mock_hiredis.cpp \
fake_response_publisher.cpp \
mock_redisreply.cpp \
common/mock_shell_command.cpp

tests_intfmgrd_INCLUDES = $(tests_INCLUDES) -I$(top_srcdir)/cfgmgr -I$(top_srcdir)/lib
tests_intfmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI)
tests_intfmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) -I $(top_srcdir)/cfgmgr -I $(top_srcdir)/orchagent/
tests_intfmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) $(tests_intfmgrd_INCLUDES)
tests_intfmgrd_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis \
-lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lpthread
25 changes: 25 additions & 0 deletions tests/mock_tests/common/mock_shell_command.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#include <string>
#include <vector>

/* Override this pointer for custom behavior */
int (*callback)(const std::string &cmd, std::string &stdout) = nullptr;

int mockCmdReturn = 0;
std::string mockCmdStdcout = "";
std::vector<std::string> mockCallArgs;

namespace swss {
int exec(const std::string &cmd, std::string &stdout)
{
if (callback != nullptr)
{
return callback(cmd, stdout);
}
else
{
mockCallArgs.push_back(cmd);
stdout = mockCmdStdcout;
return mockCmdReturn;
}
}
}
16 changes: 4 additions & 12 deletions tests/mock_tests/intfmgrd/add_ipv6_prefix_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,17 @@
#include <sys/stat.h>
#include "../mock_table.h"
#include "warm_restart.h"
#define private public
#define private public
#include "intfmgr.h"
#undef private

/* Override this pointer for custom behavior */
int (*callback)(const std::string &cmd, std::string &stdout) = nullptr;
std::vector<std::string> mockCallArgs;

namespace swss {
int exec(const std::string &cmd, std::string &stdout)
{
mockCallArgs.push_back(cmd);
return callback(cmd, stdout);
}
}
extern int (*callback)(const std::string &cmd, std::string &stdout);
extern std::vector<std::string> mockCallArgs;

bool Ethernet0IPv6Set = false;

int cb(const std::string &cmd, std::string &stdout){
mockCallArgs.push_back(cmd);
if (cmd == "sysctl -w net.ipv6.conf.\"Ethernet0\".disable_ipv6=0") Ethernet0IPv6Set = true;
else if (cmd.find("/sbin/ip -6 address \"add\"") == 0) {
return Ethernet0IPv6Set ? 0 : 2;
Expand Down
5 changes: 2 additions & 3 deletions tests/mock_tests/mock_table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,15 @@ namespace swss
keys.push_back(it.first);
}
}



void Table::del(const std::string &key, const std::string& /* op */, const std::string& /*prefix*/)
{
auto table = gDB[m_pipe->getDbId()].find(getTableName());
if (table != gDB[m_pipe->getDbId()].end()){
table->second.erase(key);
}
}

void ProducerStateTable::set(const std::string &key,
const std::vector<FieldValueTuple> &values,
const std::string &op,
Expand Down
Loading