Dbconnector namespace support (#376)

judyjoseph · abdosi · commit a87ac4ff679c · 2020-08-19T14:41:10.000-07:00
* Adding back PR#364 * Kept the old definition of >>DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn = false) << intact , so that we don't the issue (sonic-net/sonic-swss#1363) * Also changes to allign the dbConnector class here with that of sonic-py-swssdk 1. Ignore if the database_global.json file is not present. 2. validate the namespace before using it. * Fixes to take care of case where certain SWSS tests where failing as they were creating DBConnector object using the earlier constructors before Multi-DB design where the first argument is dbid eg: conf_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) * Change the variable naming --> netns, use the old logic in newConnector() * Couple of more testcase scenarios added.
diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp
diff --git a/common/dbconnector.h b/common/dbconnector.h
@@ -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:
@@ -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 &netns);
+    static std::string getDbInst(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE);
+    static int getDbId(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE);
+    static std::string getSeparator(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE);
+    static std::string getSeparator(int dbId, const std::string &netns = 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 &netns = EMPTY_NAMESPACE);
+    static std::string getDbHostname(const std::string &dbName, const std::string &netns = EMPTY_NAMESPACE);
+    static int getDbPort(const std::string &dbName, const std::string &netns = 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
@@ -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 &netns);
 
     ~DBConnector();
 
     redisContext *getContext() const;
     int getDbId() const;
     std::string getDbName() const;
+    std::string getNamespace() const;
 
     static void select(DBConnector *db);
 
@@ -84,6 +105,7 @@ class DBConnector
     redisContext *m_conn;
     int m_dbId;
     std::string m_dbName;
+    std::string m_namespace;
 };
 
 }
diff --git a/common/redisselect.cpp b/common/redisselect.cpp
@@ -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()
 {
diff --git a/common/redisselect.h b/common/redisselect.h
@@ -20,6 +20,8 @@ class RedisSelect : public Selectable
     bool hasCachedData() override;
     bool initializedWithData() override;
     void updateAfterRead() override;
+    int getDbConnectorId() override;
+    std::string getDbNamespace() override;
 
     /* Create a new redisContext, SELECT DB and SUBSCRIBE */
     void subscribe(DBConnector* db, const std::string &channelName);
diff --git a/common/selectable.h b/common/selectable.h
@@ -58,6 +58,16 @@ class Selectable
         return m_priority;
     }
 
+    virtual int getDbConnectorId()
+    {
+        return 0;
+    }
+
+    virtual std::string getDbNamespace()
+    {
+        return std::string();
+    }
+
 private:
 
     friend class Select;
diff --git a/tests/Makefile.am b/tests/Makefile.am
@@ -27,6 +27,7 @@ tests_SOURCES = redis_ut.cpp                \
                 selectable_priority.cpp       \
                 warm_restart_ut.cpp         \
                 redis_multi_db_ut.cpp       \
+                redis_multi_ns_ut.cpp       \
                 main.cpp
 
 tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS)
diff --git a/tests/main.cpp b/tests/main.cpp
@@ -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:
@@ -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"));
+        }
+
     }
 };
 
diff --git a/tests/redis_multi_db_ut_config/database_config0.json b/tests/redis_multi_db_ut_config/database_config0.json
@@ -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"
+}
diff --git a/tests/redis_multi_db_ut_config/database_config1.json b/tests/redis_multi_db_ut_config/database_config1.json
@@ -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"
+}
diff --git a/tests/redis_multi_db_ut_config/database_global.json b/tests/redis_multi_db_ut_config/database_global.json
@@ -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"
+}
diff --git a/tests/redis_multi_ns_ut.cpp b/tests/redis_multi_ns_ut.cpp
diff --git a/tests/redis_ut.cpp b/tests/redis_ut.cpp

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,15 @@ int RedisSelect::getFd()`
`16`	`16`	`{`
`17`	`17`	`return m_subscribe->getContext()->fd;`
`18`	`18`	`}`
	`19`	`+int RedisSelect::getDbConnectorId()`
	`20`	`+{`
	`21`	`+ return m_subscribe->getDbId();`
	`22`	`+}`
	`23`	`+`
	`24`	`+std::string RedisSelect::getDbNamespace()`
	`25`	`+{`
	`26`	`+ return m_subscribe->getNamespace();`
	`27`	`+}`
`19`	`28`
`20`	`29`	`uint64_t RedisSelect::readData()`
`21`	`30`	`{`