From 4a6327e700a0f99000fc506aee4ba640a6e68d0f Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 24 Oct 2022 02:51:50 +0000 Subject: [PATCH 1/6] Fix sonic-db-cli crash when database config file not ready issue. --- sonic-db-cli/sonic-db-cli.cpp | 6 +++-- tests/cli_ut.cpp | 42 +++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index 3e7003b8c..7ce83a03a 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -287,8 +287,6 @@ int sonic_db_cli( } else { - // SonicDBConfig may initialized when run cli with UT - initializeConfig(); // Use the tcp connectivity if namespace is local and unixsocket cmd_option is present. isTcpConn = true; netns = ""; @@ -297,6 +295,8 @@ int sonic_db_cli( if (options.m_cmd.size() != 0) { auto commands = options.m_cmd; + + initializeConfig(); return executeCommands(dbOrOperation, commands, netns, isTcpConn); } else if (dbOrOperation == "PING" @@ -307,6 +307,8 @@ int sonic_db_cli( // sonic-db-cli catch all possible exceptions and handle it as a failure case which not return 'OK' or 'PONG' try { + // When database config does not exist, sonic-db-cli will ignore exception and return 1. + initializeConfig(); return handleAllInstances(netns, dbOrOperation, isTcpConn); } catch (const exception& e) diff --git a/tests/cli_ut.cpp b/tests/cli_ut.cpp index 3957cdeac..913b8c018 100755 --- a/tests/cli_ut.cpp +++ b/tests/cli_ut.cpp @@ -11,6 +11,7 @@ using namespace swss; using namespace std; +const string not_exist_config_file = "./tests/redis_multi_db_ut_config/database_config_not_exist.json"; const string config_file = "./tests/redis_multi_db_ut_config/database_config.json"; const string global_config_file = "./tests/redis_multi_db_ut_config/database_global.json"; @@ -279,6 +280,47 @@ TEST(sonic_db_cli, test_cli_ping_cmd) EXPECT_EQ("True\n", output); } +TEST(sonic_db_cli, test_cli_ping_cmd_no_config) +{ + char *args[3]; + args[0] = "sonic-db-cli"; + args[1] = "PING"; + + auto output = runCli(2, args); + EXPECT_EQ("PONG\n", output); + + // When ping with DB name, result should be 'True' + args[0] = "sonic-db-cli"; + args[1] = "TEST_DB"; + args[2] = "PING"; + + // data base file does not exist, will throw exception + auto initializeGlobalConfig = []() + { + if (!SonicDBConfig::isGlobalInit()) + { + SonicDBConfig::initializeGlobalConfig(not_exist_config_file); + } + }; + + auto initializeConfig = []() + { + if (!SonicDBConfig::isInit()) + { + SonicDBConfig::initialize(not_exist_config_file); + } + }; + + optind = 0; + int exit_code = sonic_db_cli( + 3, + args, + initializeGlobalConfig, + initializeConfig); + + EXPECT_EQ(1, exit_code); +} + TEST(sonic_db_cli, test_cli_save_cmd) { char *args[2]; From 02d61fb8a1749c5f691af7b1e7eaad6b942294d0 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 24 Oct 2022 02:56:43 +0000 Subject: [PATCH 2/6] Fix code issue --- sonic-db-cli/sonic-db-cli.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index 7ce83a03a..f0a22cde7 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -296,7 +296,11 @@ int sonic_db_cli( { auto commands = options.m_cmd; - initializeConfig(); + if (netns.empty()) + { + initializeConfig(); + } + return executeCommands(dbOrOperation, commands, netns, isTcpConn); } else if (dbOrOperation == "PING" @@ -307,8 +311,12 @@ int sonic_db_cli( // sonic-db-cli catch all possible exceptions and handle it as a failure case which not return 'OK' or 'PONG' try { - // When database config does not exist, sonic-db-cli will ignore exception and return 1. - initializeConfig(); + if (netns.empty()) + { + // When database_config.json does not exist, sonic-db-cli will ignore exception and return 1. + initializeConfig(); + } + return handleAllInstances(netns, dbOrOperation, isTcpConn); } catch (const exception& e) From 81a3d41b76640b0d19de468e9ceb70cb39e27c3b Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 24 Oct 2022 03:46:22 +0000 Subject: [PATCH 3/6] Improve UT --- tests/cli_ut.cpp | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/cli_ut.cpp b/tests/cli_ut.cpp index 913b8c018..9154a9a89 100755 --- a/tests/cli_ut.cpp +++ b/tests/cli_ut.cpp @@ -286,14 +286,6 @@ TEST(sonic_db_cli, test_cli_ping_cmd_no_config) args[0] = "sonic-db-cli"; args[1] = "PING"; - auto output = runCli(2, args); - EXPECT_EQ("PONG\n", output); - - // When ping with DB name, result should be 'True' - args[0] = "sonic-db-cli"; - args[1] = "TEST_DB"; - args[2] = "PING"; - // data base file does not exist, will throw exception auto initializeGlobalConfig = []() { @@ -313,6 +305,19 @@ TEST(sonic_db_cli, test_cli_ping_cmd_no_config) optind = 0; int exit_code = sonic_db_cli( + 2, + args, + initializeGlobalConfig, + initializeConfig); + + EXPECT_EQ(1, exit_code); + + args[0] = "sonic-db-cli"; + args[1] = "TEST_DB"; + args[2] = "PING"; + + optind = 0; + exit_code = sonic_db_cli( 3, args, initializeGlobalConfig, From feffe04d51e00da5e7af925d56295d72c7f731e8 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 24 Oct 2022 08:08:50 +0000 Subject: [PATCH 4/6] Fix UT --- tests/cli_ut.cpp | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/tests/cli_ut.cpp b/tests/cli_ut.cpp index 9154a9a89..f26c0856f 100755 --- a/tests/cli_ut.cpp +++ b/tests/cli_ut.cpp @@ -311,19 +311,6 @@ TEST(sonic_db_cli, test_cli_ping_cmd_no_config) initializeConfig); EXPECT_EQ(1, exit_code); - - args[0] = "sonic-db-cli"; - args[1] = "TEST_DB"; - args[2] = "PING"; - - optind = 0; - exit_code = sonic_db_cli( - 3, - args, - initializeGlobalConfig, - initializeConfig); - - EXPECT_EQ(1, exit_code); } TEST(sonic_db_cli, test_cli_save_cmd) From f346b55f7544c32727709b0c216638631fd9aafc Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 24 Oct 2022 08:28:14 +0000 Subject: [PATCH 5/6] Fix UT --- tests/cli_ut.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/cli_ut.cpp b/tests/cli_ut.cpp index f26c0856f..f4d89a48b 100755 --- a/tests/cli_ut.cpp +++ b/tests/cli_ut.cpp @@ -289,18 +289,12 @@ TEST(sonic_db_cli, test_cli_ping_cmd_no_config) // data base file does not exist, will throw exception auto initializeGlobalConfig = []() { - if (!SonicDBConfig::isGlobalInit()) - { - SonicDBConfig::initializeGlobalConfig(not_exist_config_file); - } + SonicDBConfig::initializeGlobalConfig(not_exist_config_file); }; auto initializeConfig = []() { - if (!SonicDBConfig::isInit()) - { - SonicDBConfig::initialize(not_exist_config_file); - } + SonicDBConfig::initialize(not_exist_config_file); }; optind = 0; From d80d91bcd50c5cf4884616d8a1ab365ebd21cb26 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Mon, 24 Oct 2022 09:00:02 +0000 Subject: [PATCH 6/6] Improve code coverage --- tests/cli_ut.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/cli_ut.cpp b/tests/cli_ut.cpp index f4d89a48b..6782a95cc 100755 --- a/tests/cli_ut.cpp +++ b/tests/cli_ut.cpp @@ -305,6 +305,28 @@ TEST(sonic_db_cli, test_cli_ping_cmd_no_config) initializeConfig); EXPECT_EQ(1, exit_code); + + // When ping with DB name, exception will happen + args[0] = "sonic-db-cli"; + args[1] = "TEST_DB"; + args[2] = "PING"; + + bool exception_happen = false; + try + { + optind = 0; + exit_code = sonic_db_cli( + 3, + args, + initializeGlobalConfig, + initializeConfig); + } + catch (const exception& e) + { + exception_happen = true; + } + + EXPECT_EQ(true, exception_happen); } TEST(sonic_db_cli, test_cli_save_cmd)