Skip to content

Dbconnector namespace support #376

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.

Sign up for GitHub

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

Merged
judyjoseph merged 8 commits into from
Aug 19, 2020
Merged

Dbconnector namespace support #376

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
074ad7b
Adding back PR#364
judyjoseph Aug 3, 2020
4324228
Additional changes to overcome the issue mentioned in https://github.…
judyjoseph Aug 3, 2020
d287cad
Further updates based on comments
judyjoseph Aug 4, 2020
eb26c99
Fix to take care of case where certain SWSS tests where failing as th…
judyjoseph Aug 14, 2020
5408b10
Additional check to handle backward compatibility when namespace not …
judyjoseph Aug 16, 2020
67981ce
Change the variable naming --> netns, use the old logic in newConnect…
judyjoseph Aug 19, 2020
80cf409
Add some more testcases.
judyjoseph Aug 19, 2020
a98ba20
Couple of more testcase scenarios added.
judyjoseph Aug 19, 2020
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
299 changes: 242 additions & 57 deletions common/dbconnector.cpp
View file
Open in desktop

Large diffs are not rendered by default.

48 changes: 35 additions & 13 deletions common/dbconnector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,20 @@
#include <utility>

#include <hiredis/hiredis.h>
#define EMPTY_NAMESPACE std::string()

namespace swss {

class DBConnector;

class RedisInstInfo
{
public:
std::string unixSocketPath;
std::string hostname;
int port;
};

class SonicDBInfo
{
public:
Expand All @@ -24,25 +33,35 @@ class SonicDBConfig
{
public:
static void initialize(const std::string &file = DEFAULT_SONIC_DB_CONFIG_FILE);
static std::string getDbInst(const std::string &dbName);
static int getDbId(const std::string &dbName);
static std::string getSeparator(const std::string &dbName);
static std::string getSeparator(int dbId);
static void initializeGlobalConfig(const std::string &file = DEFAULT_SONIC_DB_GLOBAL_CONFIG_FILE);
static void validateNamespace(const std::string &nameSpace);
static std::string getDbInst(const std::string &dbName, const std::string &nameSpace = EMPTY_NAMESPACE);
static int getDbId(const std::string &dbName, const std::string &nameSpace = EMPTY_NAMESPACE);
static std::string getSeparator(const std::string &dbName, const std::string &nameSpace = EMPTY_NAMESPACE);
static std::string getSeparator(int dbId, const std::string &nameSpace = EMPTY_NAMESPACE);
static std::string getSeparator(const DBConnector* db);
static std::string getDbSock(const std::string &dbName);
static std::string getDbHostname(const std::string &dbName);
static int getDbPort(const std::string &dbName);
static std::string getDbSock(const std::string &dbName, const std::string &nameSpace = EMPTY_NAMESPACE);
static std::string getDbHostname(const std::string &dbName, const std::string &nameSpace = EMPTY_NAMESPACE);
static int getDbPort(const std::string &dbName, const std::string &nameSpace = EMPTY_NAMESPACE);
static std::vector<std::string> getNamespaces();
static bool isInit() { return m_init; };
static bool isGlobalInit() { return m_global_init; };

private:
static constexpr const char *DEFAULT_SONIC_DB_CONFIG_FILE = "/var/run/redis/sonic-db/database_config.json";
// { instName, { unix_socket_path, {hostname, port} } }
static std::unordered_map<std::string, std::pair<std::string, std::pair<std::string, int>>> m_inst_info;
// { dbName, {instName, dbId} }
static std::unordered_map<std::string, SonicDBInfo> m_db_info;
// { dbIp, separator }
static std::unordered_map<int, std::string> m_db_separator;
static constexpr const char *DEFAULT_SONIC_DB_GLOBAL_CONFIG_FILE = "/var/run/redis/sonic-db/database_global.json";
// { namespace { instName, { unix_socket_path, hostname, port } } }
static std::unordered_map<std::string, std::unordered_map<std::string, RedisInstInfo>> m_inst_info;
// { namespace, { dbName, {instName, dbId, separator} } }
static std::unordered_map<std::string, std::unordered_map<std::string, SonicDBInfo>> m_db_info;
// { namespace, { dbId, separator } }
static std::unordered_map<std::string, std::unordered_map<int, std::string>> m_db_separator;
static bool m_init;
static bool m_global_init;
static void parseDatabaseConfig(const std::string &file,
std::unordered_map<std::string, RedisInstInfo> &inst_entry,
std::unordered_map<std::string, SonicDBInfo> &db_entry,
std::unordered_map<int, std::string> &separator_entry);
};

class DBConnector
Expand All @@ -60,12 +79,14 @@ class DBConnector
DBConnector(int dbId, const std::string &hostname, int port, unsigned int timeout);
DBConnector(int dbId, const std::string &unixPath, unsigned int timeout);
DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn = false);
DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn, const std::string &nameSpace);
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Aug 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

DBConnector [](start = 4, length = 11)

Add test cases for new code #Closed

Copy link
Copy Markdown
Collaborator Author

@judyjoseph judyjoseph Aug 17, 2020

Choose a reason for hiding this comment

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

Had already added redis_multi_ns_ut.cpp which covers the cases with namespace, will add couple of negative cases for db_name not exists.

Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Aug 19, 2020

Choose a reason for hiding this comment

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

This is still applicable. There is no test case for explicit netns.

In reply to: 471706833 [](ancestors = 471706833)

Copy link
Copy Markdown
Collaborator Author

@judyjoseph judyjoseph Aug 19, 2020

Choose a reason for hiding this comment

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

Thanks, have added explicit testcase TEST(DBConnector, multi_ns_table_test) which does table/entry creation in a DB in a particular namespace.


~DBConnector();

redisContext *getContext() const;
int getDbId() const;
std::string getDbName() const;
std::string getNamespace() const;

static void select(DBConnector *db);

Expand All @@ -84,6 +105,7 @@ class DBConnector
redisContext *m_conn;
int m_dbId;
std::string m_dbName;
std::string m_namespace;
};

}
Expand Down
9 changes: 9 additions & 0 deletions common/redisselect.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@ int RedisSelect::getFd()
{
return m_subscribe->getContext()->fd;
}
int RedisSelect::getDbConnectorId()
{
return m_subscribe->getDbId();
}

std::string RedisSelect::getDbNamespace()
{
return m_subscribe->getNamespace();
}

uint64_t RedisSelect::readData()
{
Expand Down
2 changes: 2 additions & 0 deletions common/redisselect.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ class RedisSelect : public Selectable
bool hasCachedData() override;
bool initializedWithData() override;
void updateAfterRead() override;
int getDbConnectorId() override;
std::string getDbNamespace() override;
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Aug 29, 2020

Choose a reason for hiding this comment

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

Why they are added? How are they used?

Copy link
Copy Markdown
Contributor

@abdosi abdosi Aug 29, 2020

Choose a reason for hiding this comment

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

@qiluo-msft
getDbNamespace() was added to find Namespace when select returns the object.
eg: https://github.com/Azure/sonic-buildimage/blob/master/files/image_config/caclmgrd/caclmgrd#L484

getDbConnectorId() is not being used as of now but it is useful if we want to check on DBId of select return object.

Copy link
Copy Markdown
Collaborator Author

@judyjoseph judyjoseph Aug 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

another similar use case is in platform ledd daemon
https://github.com/Azure/sonic-platform-daemons/blob/master/sonic-ledd/scripts/ledd#L104


/* Create a new redisContext, SELECT DB and SUBSCRIBE */
void subscribe(DBConnector* db, const std::string &channelName);
Expand Down
10 changes: 10 additions & 0 deletions common/selectable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ class Selectable
return m_priority;
}

virtual int getDbConnectorId()
{
return 0;
}

virtual std::string getDbNamespace()
{
return std::string();
}
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Aug 29, 2020

Choose a reason for hiding this comment

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

In concept, Selectable has nothing todo with Redis. We need to remove these virtual functions.

Copy link
Copy Markdown
Contributor

@abdosi abdosi Aug 29, 2020

Choose a reason for hiding this comment

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

@qiluo-msft will check and update.


private:

friend class Select;
Expand Down
1 change: 1 addition & 0 deletions tests/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ tests_SOURCES = redis_ut.cpp \
warm_restart_ut.cpp \
redis_multi_db_ut.cpp \
logger_ut.cpp \
redis_multi_ns_ut.cpp \
fdb_flush.cpp \
main.cpp

Expand Down
38 changes: 38 additions & 0 deletions tests/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ using namespace swss;

string existing_file = "./tests/redis_multi_db_ut_config/database_config.json";
string nonexisting_file = "./tests/redis_multi_db_ut_config/database_config_nonexisting.json";
string global_existing_file = "./tests/redis_multi_db_ut_config/database_global.json";

#define TEST_DB "APPL_DB"
#define TEST_NAMESPACE "asic0"
#define INVALID_NAMESPACE "invalid"

class SwsscommonEnvironment : public ::testing::Environment {
public:
Expand All @@ -32,6 +37,39 @@ class SwsscommonEnvironment : public ::testing::Environment {
SonicDBConfig::initialize(existing_file);
cout<<"INIT: load local db config file, isInit = "<<SonicDBConfig::isInit()<<endl;
EXPECT_TRUE(SonicDBConfig::isInit());

// Test the database_global.json file
// by default , global_init should be false
cout<<"Default : isGlobalInit = "<<SonicDBConfig::isGlobalInit()<<endl;
EXPECT_FALSE(SonicDBConfig::isGlobalInit());

// Call an API which actually needs the data populated by SonicDBConfig::initializeGlobalConfig
try
{
cout<<"INIT: Invoking SonicDBConfig::getDbId(APPL_DB, asic0)"<<endl;
SonicDBConfig::getDbId(TEST_DB, TEST_NAMESPACE);
}
catch (exception &e)
{
EXPECT_TRUE(strstr(e.what(), "Initialize global DB config using API SonicDBConfig::initializeGlobalConfig"));
}

// load local global file, init should be true
SonicDBConfig::initializeGlobalConfig(global_existing_file);
cout<<"INIT: load global db config file, isInit = "<<SonicDBConfig::isGlobalInit()<<endl;
EXPECT_TRUE(SonicDBConfig::isGlobalInit());

// Call an API with wrong namespace passed
try
{
cout<<"INIT: Invoking SonicDBConfig::getDbId(APPL_DB, invalid)"<<endl;
SonicDBConfig::getDbId(TEST_DB, INVALID_NAMESPACE);
}
catch (exception &e)
{
EXPECT_TRUE(strstr(e.what(), "Namespace invalid is not a valid namespace name in config file"));
}

}
};

Expand Down
82 changes: 82 additions & 0 deletions tests/redis_multi_db_ut_config/database_config0.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
{
"INSTANCES": {
"redis":{
"hostname" : "127.0.0.1",
"port": 6379,
"unix_socket_path": "/var/run/redis0/redis.sock"
}
},
"DATABASES" : {
"APPL_DB" : {
"id" : 0,
"separator": ":",
"instance" : "redis"
},
"ASIC_DB" : {
"id" : 1,
"separator": ":",
"instance" : "redis"
},
"COUNTERS_DB" : {
"id" : 2,
"separator": ":",
"instance" : "redis"
},
"LOGLEVEL_DB" : {
"id" : 3,
"separator": ":",
"instance" : "redis"
},
"CONFIG_DB" : {
"id" : 4,
"separator": "|",
"instance" : "redis"
},
"PFC_WD_DB" : {
"id" : 5,
"separator": ":",
"instance" : "redis"
},
"FLEX_COUNTER_DB" : {
"id" : 5,
"separator": ":",
"instance" : "redis"
},
"STATE_DB" : {
"id" : 6,
"separator": "|",
"instance" : "redis"
},
"SNMP_OVERLAY_DB" : {
"id" : 7,
"separator": "|",
"instance" : "redis"
},
"ASIC_DB2" : {
"id" : 10,
"separator": ":",
"instance" : "redis"
},
"COUNTERS_DB2" : {
"id" : 11,
"separator": ":",
"instance" : "redis"
},
"FLEX_COUNTER_DB2" : {
"id" : 12,
"separator": ":",
"instance" : "redis"
},
"STATE_DB2" : {
"id" : 13,
"separator": "|",
"instance" : "redis"
},
"TEST_DB" : {
"id" : 15,
"separator": ":",
"instance" : "redis"
}
},
"VERSION" : "1.0"
}
82 changes: 82 additions & 0 deletions tests/redis_multi_db_ut_config/database_config1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
{
"INSTANCES": {
"redis":{
"hostname" : "127.0.0.1",
"port": 6379,
"unix_socket_path": "/var/run/redis1/redis.sock"
}
},
"DATABASES" : {
"APPL_DB" : {
"id" : 0,
"separator": ":",
"instance" : "redis"
},
"ASIC_DB" : {
"id" : 1,
"separator": ":",
"instance" : "redis"
},
"COUNTERS_DB" : {
"id" : 2,
"separator": ":",
"instance" : "redis"
},
"LOGLEVEL_DB" : {
"id" : 3,
"separator": ":",
"instance" : "redis"
},
"CONFIG_DB" : {
"id" : 4,
"separator": "|",
"instance" : "redis"
},
"PFC_WD_DB" : {
"id" : 5,
"separator": ":",
"instance" : "redis"
},
"FLEX_COUNTER_DB" : {
"id" : 5,
"separator": ":",
"instance" : "redis"
},
"STATE_DB" : {
"id" : 6,
"separator": "|",
"instance" : "redis"
},
"SNMP_OVERLAY_DB" : {
"id" : 7,
"separator": "|",
"instance" : "redis"
},
"ASIC_DB2" : {
"id" : 10,
"separator": ":",
"instance" : "redis"
},
"COUNTERS_DB2" : {
"id" : 11,
"separator": ":",
"instance" : "redis"
},
"FLEX_COUNTER_DB2" : {
"id" : 12,
"separator": ":",
"instance" : "redis"
},
"STATE_DB2" : {
"id" : 13,
"separator": "|",
"instance" : "redis"
},
"TEST_DB" : {
"id" : 15,
"separator": ":",
"instance" : "redis"
}
},
"VERSION" : "1.0"
}
16 changes: 16 additions & 0 deletions tests/redis_multi_db_ut_config/database_global.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"INCLUDES" : [
{
"include" : "database_config.json"
},
{
"namespace" : "asic0",
"include" : "../redis_multi_db_ut_config/database_config0.json"
},
{
"namespace" : "asic1",
"include" : "../redis_multi_db_ut_config/database_config1.json"
}
],
"VERSION" : "1.0"
}
Loading