From a137f2ca3fe62ac9d12c565fc12a9f1ce8d4c3be Mon Sep 17 00:00:00 2001 From: Sonic Build Admin Date: Wed, 10 Sep 2025 08:47:37 +0000 Subject: [PATCH] Ignore nonexistent include in database_global.json ### why SonicDBConfig::initializeGlobalConfig aborts parsing global config if it encounters an include of a database config that doesn't exist. The global config file includes all possible includes in the switch. However, not all includes are relevant to a client. For example, in smartswitch, dash-ha container only cares NPU databases and databases of the DPU managed by the container. The current behavior requires a dash-ha container mounting all the DPU database instances. ### what this PR does 1. Add ignore_nonexistent flag, with default value false, to function parseDatabaseConfig. If it is set to true, it will not throw error if the included database config file doesn't exist. 2. initializeGlobalConfig sets ignore_nonexistent to true when calling parseDatabaseConfig. If inst_entry, db_entry and separator_entry are empty, it will ignore the key. --- common/c-api/dbconnector.cpp | 4 +-- common/c-api/dbconnector.h | 2 +- common/dbconnector.cpp | 25 ++++++++++++++----- common/dbconnector.h | 11 ++++---- crates/swss-common-testing/src/lib.rs | 4 +-- crates/swss-common/src/lib.rs | 4 +-- tests/main.cpp | 25 ++++++++++++++++++- .../database_global_with_invalid_include.json | 20 +++++++++++++++ 8 files changed, 76 insertions(+), 19 deletions(-) create mode 100644 tests/redis_multi_db_ut_config/database_global_with_invalid_include.json diff --git a/common/c-api/dbconnector.cpp b/common/c-api/dbconnector.cpp index b89b944..92e01ef 100644 --- a/common/c-api/dbconnector.cpp +++ b/common/c-api/dbconnector.cpp @@ -13,8 +13,8 @@ SWSSResult SWSSSonicDBConfig_initialize(const char *path) { SWSSTry(SonicDBConfig::initialize(path)); } -SWSSResult SWSSSonicDBConfig_initializeGlobalConfig(const char *path) { - SWSSTry(SonicDBConfig::initializeGlobalConfig(path)); +SWSSResult SWSSSonicDBConfig_initializeGlobalConfig(const char *path, uint8_t ignore_nonexistent) { + SWSSTry(SonicDBConfig::initializeGlobalConfig(path, ignore_nonexistent)); } SWSSResult SWSSDBConnector_new_tcp(int32_t dbId, const char *hostname, uint16_t port, diff --git a/common/c-api/dbconnector.h b/common/c-api/dbconnector.h index a4ab09e..b06b4a2 100644 --- a/common/c-api/dbconnector.h +++ b/common/c-api/dbconnector.h @@ -12,7 +12,7 @@ extern "C" { SWSSResult SWSSSonicDBConfig_initialize(const char *path); -SWSSResult SWSSSonicDBConfig_initializeGlobalConfig(const char *path); +SWSSResult SWSSSonicDBConfig_initializeGlobalConfig(const char *path, uint8_t ignore_nonexistent); typedef struct SWSSDBConnectorOpaque *SWSSDBConnector; diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index 47fe80d..33b3a43 100755 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include "logger.h" #include "common/dbconnector.h" @@ -22,8 +23,14 @@ using namespace swss; void SonicDBConfig::parseDatabaseConfig(const string &file, std::map &inst_entry, std::unordered_map &db_entry, - std::unordered_map &separator_entry) + std::unordered_map &separator_entry, + bool ignore_nonexistent) { + if (ignore_nonexistent && access(file.c_str(), F_OK) == -1) { + SWSS_LOG_NOTICE("Sonic database config file doesn't exist at %s\n", file.c_str()); + return; + } + ifstream i(file); if (i.good()) { @@ -75,7 +82,7 @@ void SonicDBConfig::parseDatabaseConfig(const string &file, } } -void SonicDBConfig::initializeGlobalConfig(const string &file) +void SonicDBConfig::initializeGlobalConfig(const string &file, bool ignore_nonexistent) { std::string dir_name; std::lock_guard guard(m_db_info_mutex); @@ -128,10 +135,16 @@ void SonicDBConfig::initializeGlobalConfig(const string &file) continue; } - parseDatabaseConfig(local_file, inst_entry, db_entry, separator_entry); - m_inst_info[key] = inst_entry; - m_db_info[key] = db_entry; - m_db_separator[key] = separator_entry; + parseDatabaseConfig(local_file, inst_entry, db_entry, separator_entry, ignore_nonexistent); + // all the entries are empty, then don't add them to the map. It may happen if the included + // config doesn't exist. For example, dash-ha container will only mount the redis instance + // config file for the dpu it is managing. + if (!inst_entry.empty() || !db_entry.empty() || !separator_entry.empty()) + { + m_inst_info[key] = inst_entry; + m_db_info[key] = db_entry; + m_db_separator[key] = separator_entry; + } if(key.isEmpty()) { diff --git a/common/dbconnector.h b/common/dbconnector.h index 832983e..9a40831 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -45,7 +45,7 @@ struct SonicDBKey * In our design, we allow multiple containers to share a same namespace. * So, this combination of container name and namespace is used to uniquely identify a DB instance. * If the namespace is empty, it means the DB instance is running in the default(host) namespace. - * If the container name is empty, it for adapting the old design that only one DB instance is + * If the container name is empty, it for adapting the old design that only one DB instance is * running in a namespace. */ std::string containerName; @@ -99,13 +99,13 @@ class SonicDBConfig %} #endif - static void initializeGlobalConfig(const std::string &file = DEFAULT_SONIC_DB_GLOBAL_CONFIG_FILE); + static void initializeGlobalConfig(const std::string &file = DEFAULT_SONIC_DB_GLOBAL_CONFIG_FILE, bool ignore_nonexistent = false); #if defined(SWIG) && defined(SWIGPYTHON) %pythoncode %{ ## TODO: the python function and C++ one is not on-par @staticmethod - def load_sonic_global_db_config(global_db_file_path=DEFAULT_SONIC_DB_GLOBAL_CONFIG_FILE, namespace=None): - SonicDBConfig.initializeGlobalConfig(global_db_file_path) + def load_sonic_global_db_config(global_db_file_path=DEFAULT_SONIC_DB_GLOBAL_CONFIG_FILE, namespace=None, ignore_nonexistent=False): + SonicDBConfig.initializeGlobalConfig(global_db_file_path, ignore_nonexistent) %} #endif static void reset(); @@ -157,7 +157,8 @@ class SonicDBConfig static void parseDatabaseConfig(const std::string &file, std::map &inst_entry, std::unordered_map &db_entry, - std::unordered_map &separator_entry); + std::unordered_map &separator_entry, + bool ignore_nonexistent = false); static RedisInstInfo& getRedisInfo(const std::string &dbName, const SonicDBKey &key); static SonicDBInfo& getDbInfo(const std::string &dbName, const SonicDBKey &key); }; diff --git a/crates/swss-common-testing/src/lib.rs b/crates/swss-common-testing/src/lib.rs index 7b9c587..7d29926 100644 --- a/crates/swss-common-testing/src/lib.rs +++ b/crates/swss-common-testing/src/lib.rs @@ -182,7 +182,7 @@ pub fn sonic_db_config_init_for_test() { fs::write("/tmp/db_config_test.json", DB_CONFIG_JSON).unwrap(); fs::write("/tmp/db_global_config_test.json", DB_GLOBAL_CONFIG_JSON).unwrap(); sonic_db_config_initialize("/tmp/db_config_test.json").unwrap(); - sonic_db_config_initialize_global("/tmp/db_global_config_test.json").unwrap(); + sonic_db_config_initialize_global("/tmp/db_global_config_test.json", false).unwrap(); fs::remove_file("/tmp/db_config_test.json").unwrap(); fs::remove_file("/tmp/db_global_config_test.json").unwrap(); *sonic_db_init = true; @@ -206,7 +206,7 @@ fn config_db_config_init_for_test(sock_str: &str) { fs::write("/tmp/db_config_test.json", db_config_json).unwrap(); fs::write("/tmp/db_global_config_test.json", DB_GLOBAL_CONFIG_JSON).unwrap(); sonic_db_config_initialize("/tmp/db_config_test.json").unwrap(); - sonic_db_config_initialize_global("/tmp/db_global_config_test.json").unwrap(); + sonic_db_config_initialize_global("/tmp/db_global_config_test.json", false).unwrap(); fs::remove_file("/tmp/db_config_test.json").unwrap(); fs::remove_file("/tmp/db_global_config_test.json").unwrap(); *config_db_init = true; diff --git a/crates/swss-common/src/lib.rs b/crates/swss-common/src/lib.rs index 4082429..9f1d053 100644 --- a/crates/swss-common/src/lib.rs +++ b/crates/swss-common/src/lib.rs @@ -13,9 +13,9 @@ pub fn sonic_db_config_initialize(path: &str) -> Result<(), Exception> { } /// Rust wrapper around `swss::SonicDBConfig::initializeGlobalConfig`. -pub fn sonic_db_config_initialize_global(path: &str) -> Result<(), Exception> { +pub fn sonic_db_config_initialize_global(path: &str, ignore_nonexistent: bool) -> Result<(), Exception> { let path = cstr(path); - unsafe { swss_try!(bindings::SWSSSonicDBConfig_initializeGlobalConfig(path.as_ptr())) } + unsafe { swss_try!(bindings::SWSSSonicDBConfig_initializeGlobalConfig(path.as_ptr(), ignore_nonexistent as u8)) } } /// Trait for objects that can be stored in a Sonic DB table. diff --git a/tests/main.cpp b/tests/main.cpp index bfdd9fc..fea91f5 100644 --- a/tests/main.cpp +++ b/tests/main.cpp @@ -9,7 +9,7 @@ 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"; - +string global_with_invalid_include = "./tests/redis_multi_db_ut_config/database_global_with_invalid_include.json"; #define TEST_DB "APPL_DB" #define TEST_NAMESPACE "asic0" #define INVALID_NAMESPACE "invalid" @@ -55,6 +55,29 @@ class SwsscommonEnvironment : public ::testing::Environment { EXPECT_TRUE(strstr(e.what(), "Initialize global DB config using API SonicDBConfig::initializeGlobalConfig")); } + // Test the global SonicDBConfig::initializeGlobalConfig with non-existing include + SonicDBConfig::initializeGlobalConfig(global_with_invalid_include, true); + cout<<"INIT: load global db config file with invalid include, isGlobalInit = "< db_keys = SonicDBConfig::getDbKeys(); + + // Extract containerName from SonicDBKey in db_keys and store in vector + vector cn_actual; + for (const auto& key : db_keys) { + cn_actual.push_back(key.containerName); + } + + sort (cn_actual.begin(), cn_actual.end()); + vector cn_expected = {"", "dpu0", "dpu2"}; + // verify the non-existent include is skipped + EXPECT_EQ(cn_actual.size(), cn_expected.size()); + EXPECT_TRUE(std::equal(cn_expected.begin(), cn_expected.end(), cn_actual.begin())); + // reset SonicDBConfig, init should be false + SonicDBConfig::reset(); + cout<<"RESET: isInit = "<