From db9a17fa8ef9e52e1a8e30537c49c2a8240143ab Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 21 Apr 2022 08:27:07 +0000 Subject: [PATCH 01/20] Add sonic-db-cli files --- sonic-db-cli/Makefile.am | 24 +++ sonic-db-cli/configure.ac | 70 ++++++++ sonic-db-cli/debian/changelog | 5 + sonic-db-cli/debian/control | 14 ++ sonic-db-cli/debian/copyright | 1 + sonic-db-cli/debian/rules | 25 +++ sonic-db-cli/sonic-db-cli.cpp | 240 ++++++++++++++++++++++++++++ sonic-db-cli/sonic-db-cli.h | 42 +++++ sonic-db-cli/tests/Makefile.am | 15 ++ sonic-db-cli/tests/help_output.txt | 18 +++ sonic-db-cli/tests/main.cpp | 71 ++++++++ sonic-db-cli/tests/mock_connector.h | 0 12 files changed, 525 insertions(+) create mode 100755 sonic-db-cli/Makefile.am create mode 100755 sonic-db-cli/configure.ac create mode 100755 sonic-db-cli/debian/changelog create mode 100755 sonic-db-cli/debian/control create mode 100755 sonic-db-cli/debian/copyright create mode 100755 sonic-db-cli/debian/rules create mode 100755 sonic-db-cli/sonic-db-cli.cpp create mode 100755 sonic-db-cli/sonic-db-cli.h create mode 100755 sonic-db-cli/tests/Makefile.am create mode 100755 sonic-db-cli/tests/help_output.txt create mode 100755 sonic-db-cli/tests/main.cpp create mode 100755 sonic-db-cli/tests/mock_connector.h diff --git a/sonic-db-cli/Makefile.am b/sonic-db-cli/Makefile.am new file mode 100755 index 000000000..a3569af87 --- /dev/null +++ b/sonic-db-cli/Makefile.am @@ -0,0 +1,24 @@ +########################################################################### +## +## File: ./Makefile.am +## Versions: $Id: Makefile.am,v 1.0 2022/04/02 12:04:29 liuh@microsoft.com Exp $ +## Created: 2022/04/02 +## +########################################################################### +ACLOCAL_AMFLAGS = -I m4 --install +ACLOCAL_AMFLAGS = -I config +AUTOMAKE_OPTIONS = subdir-objects +SUBDIRS = tests + +bin_PROGRAMS = sonic-db-cli +sonic_db_cli_SOURCES = sonic-db-cli.h sonic-db-cli.cpp +sonic_db_cli_CPPFLAGS = $(AM_CPPFLAGS) -std=c++17 +sonic_db_cli_LDFLAGS = -lswsscommon $(BOOST_PROGRAM_OPTIONS_LIB) + +EXTRA_DIST = bootstrap sonic-db-cli.spec + +MAINTAINERCLEANFILES = Makefile.in config.h.in configure aclocal.m4 \ + config/config.guess config/config.sub config/depcomp \ + config/install-sh config/ltmain.sh config/missing + +pkgconfigdir = $(libdir)/pkgconfig diff --git a/sonic-db-cli/configure.ac b/sonic-db-cli/configure.ac new file mode 100755 index 000000000..b407135e2 --- /dev/null +++ b/sonic-db-cli/configure.ac @@ -0,0 +1,70 @@ +dnl +dnl File: configure.in +dnl Revision: $Id: configure.ac,v 1.0 2022/04/02 12:04:29 liuh@microsoft.com Exp $ +dnl Created: 2022/04/02 +dnl Author: Liu Hua +dnl +dnl Process this file with autoconf to produce a configure script +dnl You need autoconf 2.59 or better! +dnl +dnl --------------------------------------------------------------------------- + +AC_PREREQ(2.59) +AC_COPYRIGHT([ +See the included file: COPYING for copyright information. +]) +AC_INIT(sonic-db-cli, 1.0.0, [liuh@microsoft.com]) + +AC_CONFIG_AUX_DIR(config) +AM_INIT_AUTOMAKE([foreign]) +AC_CONFIG_SRCDIR([sonic-db-cli.cpp]) +AC_CONFIG_HEADER([config.h]) +AC_CONFIG_MACRO_DIR([config]) +AC_CONFIG_MACRO_DIR([m4]) + +dnl -------------------------------------------------------------------- +dnl Checks for programs. +AC_LANG_C +AC_LANG([C++]) +AC_PROG_CC +AC_PROG_CXX +AM_PROG_CC_C_O +AC_PROG_INSTALL +AC_PROG_LN_S +AC_PROG_MAKE_SET +AC_ENABLE_SHARED +AC_DISABLE_STATIC +AM_PROG_LIBTOOL + +dnl -------------------------------------------------------------------- +dnl Checks for libraries. +AC_CHECK_LIB(swsscommon, conn) +dnl Check boost libs: https://www.gnu.org/software/autoconf-archive/The-Macros.html#The-Macros +AX_BOOST_BASE(,, [AC_MSG_ERROR([boost lib missing])]) +AX_BOOST_PROGRAM_OPTIONS + +dnl -------------------------------------------------------------------- +dnl Checks for header files. +AC_HEADER_STDC +AC_CHECK_HEADER([swss/sonicv2connector.h], [], [AC_MSG_ERROR([swsscommon lib missing. ])] ) + +dnl -------------------------------------------------------------------- +dnl Checks for typedefs, structures, and compiler characteristics. +AC_C_CONST +AC_TYPE_SIZE_T +AC_HEADER_TIME + +dnl -------------------------------------------------------------------- +dnl Checks for library functions. +AC_FUNC_REALLOC +AC_FUNC_SELECT_ARGTYPES +AC_TYPE_SIGNAL +AC_CHECK_FUNCS([bzero gethostbyname gettimeofday inet_ntoa select socket logwtmp getrandom]) + +dnl -------------------------------------------------------------------- +dnl Generate made files +AC_CONFIG_FILES([ + Makefile + tests/Makefile +]) +AC_OUTPUT diff --git a/sonic-db-cli/debian/changelog b/sonic-db-cli/debian/changelog new file mode 100755 index 000000000..48c2cce85 --- /dev/null +++ b/sonic-db-cli/debian/changelog @@ -0,0 +1,5 @@ +sonic-db-cli (1.0.0) unstable; urgency=medium + + * Initial release (Closes: #nnnn) + + -- Liu Hua Wed, 06 Apr 2022 09:21:18 +0000 diff --git a/sonic-db-cli/debian/control b/sonic-db-cli/debian/control new file mode 100755 index 000000000..6a526bedb --- /dev/null +++ b/sonic-db-cli/debian/control @@ -0,0 +1,14 @@ +Source: sonic-db-cli +Section: unknown +Priority: optional +Maintainer: Liu Hua +Build-Depends: debhelper-compat (= 13) +Standards-Version: 4.5.1 +Homepage: +Rules-Requires-Root: no + +Package: sonic-db-cli +Architecture: any +Depends: ${shlibs:Depends}, ${misc:Depends} +Description: + diff --git a/sonic-db-cli/debian/copyright b/sonic-db-cli/debian/copyright new file mode 100755 index 000000000..7cf565eff --- /dev/null +++ b/sonic-db-cli/debian/copyright @@ -0,0 +1 @@ +Copyright (C) 2022 Microsoft, Inc \ No newline at end of file diff --git a/sonic-db-cli/debian/rules b/sonic-db-cli/debian/rules new file mode 100755 index 000000000..59ea751e6 --- /dev/null +++ b/sonic-db-cli/debian/rules @@ -0,0 +1,25 @@ +#!/usr/bin/make -f +# See debhelper(7) (uncomment to enable) +# output every command that modifies files on the build system. +#export DH_VERBOSE = 1 + + +# see FEATURE AREAS in dpkg-buildflags(1) +#export DEB_BUILD_MAINT_OPTIONS = hardening=+all + +# see ENVIRONMENT in dpkg-buildflags(1) +# package maintainers to append CFLAGS +#export DEB_CFLAGS_MAINT_APPEND = -Wall -pedantic +# package maintainers to append LDFLAGS +#export DEB_LDFLAGS_MAINT_APPEND = -Wl,--as-needed + + +%: + dh $@ + + +# dh_make generated override targets +# This is example for Cmake (See https://bugs.debian.org/641051 ) +#override_dh_auto_configure: +# dh_auto_configure -- \ +# -DCMAKE_LIBRARY_PATH=$(DEB_HOST_MULTIARCH) diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp new file mode 100755 index 000000000..3ba7e56ee --- /dev/null +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -0,0 +1,240 @@ +#include +#include + +#include +#include + +#include "sonic-db-cli.h" + +static std::string name_space; +static std::string db_or_operation; +const char* empty_str = ""; + +void printUsage(const po::options_description &all_options) +{ + std::cout << "usage: sonic-db-cli [-h] [-s] [-n NAMESPACE] db_or_op [cmd ...]" << std::endl; + std::cout << all_options << std::endl; + + std::cout << "**sudo** needed for commands accesing a different namespace [-n], or using unixsocket connection [-s]" << std::endl; + std::cout << "" << std::endl; + std::cout << "Example 1: sonic-db-cli -n asic0 CONFIG_DB keys \\*" << std::endl; + std::cout << "Example 2: sonic-db-cli -n asic2 APPL_DB HGETALL VLAN_TABLE:Vlan10" << std::endl; + std::cout << "Example 3: sonic-db-cli APPL_DB HGET VLAN_TABLE:Vlan10 mtu" << std::endl; + std::cout << "Example 4: sonic-db-cli -n asic3 APPL_DB EVAL \"return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}\" 2 k1 k2 v1 v2" << std::endl; + std::cout << "Example 5: sonic-db-cli PING | sonic-db-cli -s PING" << std::endl; + std::cout << "Example 6: sonic-db-cli SAVE | sonic-db-cli -s SAVE" << std::endl; + std::cout << "Example 7: sonic-db-cli FLUSHALL | sonic-db-cli -s FLUSHALL" << std::endl; +} + +void printRedisReply(redisReply* reply) +{ + switch(reply->type) + { + case REDIS_REPLY_INTEGER: + std::cout << reply->integer; + break; + case REDIS_REPLY_STRING: + case REDIS_REPLY_ERROR: + case REDIS_REPLY_STATUS: + case REDIS_REPLY_NIL: + std::cout << std::string(reply->str, reply->len); + break; + case REDIS_REPLY_ARRAY: + for (size_t i = 0; i < reply->elements; i++) + { + printRedisReply(reply->element[i]); + } + break; + default: + std::cerr << reply->type << std::endl; + throw std::runtime_error("Unexpected reply type"); + } + + std::cout << std::endl; +} + +int executeCommands( + const std::string& db_name, + std::vector& commands, + const std::string& name_space, + bool use_unix_socket) +{ + std::unique_ptr dbconn; + if (name_space.compare("None") == 0) { + dbconn = std::make_unique(use_unix_socket, empty_str); + } + else { + dbconn = std::make_unique(true, name_space.c_str()); + } + + try { + dbconn->connect(db_name); + } + catch (const std::exception& e) + { + std::cerr << "Invalid database name input: " << db_name << std::endl; + std::cerr << e.what() << std::endl; + return 1; + } + + auto& client = dbconn->get_redis_client(db_name); + + size_t argc = commands.size(); + const char** argv = new const char*[argc]; + size_t* argvc = new size_t[argc]; + for (size_t i = 0; i < argc; i++) + { + argv[i] = strdup(commands[i].c_str()); + argvc[i] = commands[i].size(); + } + + swss::RedisCommand command; + command.formatArgv(argc, argv, argvc); + swss::RedisReply reply(&client, command); + + auto redisReply = reply.getContext(); + printRedisReply(redisReply); + + return 0; +} + +void handleSingleOperation( + const std::string& name_space, + const std::string& db_name, + const std::string& operation, + bool use_unix_socket) +{ + swss::SonicV2Connector_Native conn(use_unix_socket, name_space.c_str()); + conn.connect(db_name); + auto& client = conn.get_redis_client(db_name); + + swss::RedisReply reply(&client, operation); + auto redisReply = reply.getContext(); + printRedisReply(redisReply); +} + +void handleAllInstances( + const std::string& name_space, + const std::string& operation, + bool use_unix_socket) +{ + // Use the tcp connectivity if namespace is local and unixsocket cmd_option is present. + if (name_space.compare("None") == 0) { + use_unix_socket = true; + } + + auto dbNames = swss::SonicDBConfig::getDbList(name_space); + // Operate All Redis Instances in Parallel + // TODO: if one of the operations failed, it could fail quickly and not necessary to wait all other operations + std::for_each( + std::execution::par, + dbNames.begin(), + dbNames.end(), + [=](auto&& db_name) + { + handleSingleOperation(name_space, db_name, operation, use_unix_socket); + }); +} + +int handleOperation( + const po::options_description &all_options, + const po::variables_map &variables_map) +{ + if (variables_map.count("db_or_op")) { + auto db_or_operation = variables_map["db_or_op"].as(); + auto name_space = variables_map["--namespace"].as(); + bool useUnixSocket = variables_map.count("--unixsocket"); + if (name_space.compare("None") != 0) { + swss::SonicDBConfig::initializeGlobalConfig(name_space); + } + + if (variables_map.count("cmd")) { + auto commands = variables_map["cmd"].as< std::vector >(); + return executeCommands(db_or_operation, commands, name_space, useUnixSocket); + } + else if (name_space.compare("PING") == 0 || name_space.compare("SAVE") == 0 || name_space.compare("FLUSHALL") == 0) { + // redis-cli doesn't depend on database_config.json which could raise some exceptions + // sonic-db-cli catch all possible exceptions and handle it as a failure case which not return 'OK' or 'PONG' + handleAllInstances(name_space, db_or_operation, useUnixSocket); + } + else { + printUsage(all_options); + } + } + else { + printUsage(all_options); + } + + return 0; +} + +void parseCliArguments( + int argc, + char** argv, + po::options_description &all_options, + po::variables_map &variables_map) +{ + all_options.add_options() + ("--help,-h", "Help message") + ("--unixsocket,-s", "Override use of tcp_port and use unixsocket") + ("--namespace,-n", po::value(&name_space)->default_value("None"), "Namespace string to use asic0/asic1.../asicn") + ("db_or_op", po::value(&db_or_operation)->default_value(empty_str), "Database name Or Unary operation(only PING/SAVE/FLUSHALL supported)") + ("cmd", po::value< std::vector >(), "Command to execute in database") + ; + + po::positional_options_description positional_opts; + positional_opts.add("db_or_op", 1); + positional_opts.add("cmd", -1); + + po::store(po::command_line_parser(argc, argv) + .options(all_options) + .positional(positional_opts) + .run(), + variables_map); + po::notify(variables_map); +} + +#ifdef TESTING +int sonic_db_cli(int argc, char** argv) +#else +int main(int argc, char** argv) +#endif +{ + po::options_description all_options("SONiC DB CLI"); + po::variables_map variables_map; + + try { + parseCliArguments(argc, argv, all_options, variables_map); + } + catch (po::error_with_option_name& e) { + std::cerr << "Command Line Syntax Error: " << e.what() << std::endl; + printUsage(all_options); + return -1; + } + catch (po::error& e) { + std::cerr << "Command Line Error: " << e.what() << std::endl; + printUsage(all_options); + return -1; + } + + if (variables_map.count("--help")) { + printUsage(all_options); + return 0; + } + + try + { + return handleOperation(all_options, variables_map); + } + catch (const std::exception& e) + { + std::cerr << "An exception of type " << e.what() << " occurred. Arguments:" << std::endl; + for (int idx = 0; idx < argc; idx++) { + std::cerr << argv[idx] << " "; + } + std::cerr << std::endl; + return 1; + } + + return 0; +} \ No newline at end of file diff --git a/sonic-db-cli/sonic-db-cli.h b/sonic-db-cli/sonic-db-cli.h new file mode 100755 index 000000000..effda9507 --- /dev/null +++ b/sonic-db-cli/sonic-db-cli.h @@ -0,0 +1,42 @@ +#pragma once + +#include +#include +#include + +namespace po = boost::program_options; + +void printUsage(const po::options_description &all_options); + +void printRedisReply(redisReply* reply); + +int executeCommands( + const std::string& db_name, + std::vector& commands, + const std::string& name_space, + bool use_unix_socket); + +void handleSingleOperation( + const std::string& name_space, + const std::string& db_name, + const std::string& operation, + bool use_unix_socket); + +void handleAllInstances( + const std::string& name_space, + const std::string& operation, + bool use_unix_socket); + +int handleOperation( + const po::options_description &all_options, + const po::variables_map &variables_map); + +void parseCliArguments( + int argc, + char** argv, + po::options_description &all_options, + po::variables_map &variables_map); + +#ifdef TESTING +int sonic_db_cli(int argc, char** argv); +#endif \ No newline at end of file diff --git a/sonic-db-cli/tests/Makefile.am b/sonic-db-cli/tests/Makefile.am new file mode 100755 index 000000000..428aeeb1a --- /dev/null +++ b/sonic-db-cli/tests/Makefile.am @@ -0,0 +1,15 @@ +#INCLUDES = -I $(top_srcdir) + +bin_PROGRAMS = tests +DBGFLAGS = -ggdb -DDEBUG + +CFLAGS_GTEST = -DTESTING -Wno-write-strings +LDADD_GTEST = -L/usr/src/gtest -lgtest -lgtest_main -lgmock -lgmock_main + +tests_SOURCES = main.cpp \ + ../sonic-db-cli.cpp \ + ../sonic-db-cli.h + +tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) +tests_CPPFLAGS = $(tests_CFLAGS) -std=c++17 +tests_LDADD = $(LDADD_GTEST) -lpthread -lswsscommon $(CODE_COVERAGE_LIBS) $(BOOST_PROGRAM_OPTIONS_LIB) \ No newline at end of file diff --git a/sonic-db-cli/tests/help_output.txt b/sonic-db-cli/tests/help_output.txt new file mode 100755 index 000000000..ff2ea2826 --- /dev/null +++ b/sonic-db-cli/tests/help_output.txt @@ -0,0 +1,18 @@ +usage: sonic-db-cli [-h] [-s] [-n NAMESPACE] db_or_op [cmd ...] +SONiC DB CLI: + ----help Help message + ----unixsocket Override use of tcp_port and use unixsocket + ----namespace arg (=None) Namespace string to use asic0/asic1.../asicn + --db_or_op arg Database name Or Unary operation(only + PING/SAVE/FLUSHALL supported) + --cmd arg Command to execute in database + +**sudo** needed for commands accesing a different namespace [-n], or using unixsocket connection [-s] + +Example 1: sonic-db-cli -n asic0 CONFIG_DB keys \* +Example 2: sonic-db-cli -n asic2 APPL_DB HGETALL VLAN_TABLE:Vlan10 +Example 3: sonic-db-cli APPL_DB HGET VLAN_TABLE:Vlan10 mtu +Example 4: sonic-db-cli -n asic3 APPL_DB EVAL "return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}" 2 k1 k2 v1 v2 +Example 5: sonic-db-cli PING | sonic-db-cli -s PING +Example 6: sonic-db-cli SAVE | sonic-db-cli -s SAVE +Example 7: sonic-db-cli FLUSHALL | sonic-db-cli -s FLUSHALL diff --git a/sonic-db-cli/tests/main.cpp b/sonic-db-cli/tests/main.cpp new file mode 100755 index 000000000..a70f482cb --- /dev/null +++ b/sonic-db-cli/tests/main.cpp @@ -0,0 +1,71 @@ +#include "gtest/gtest.h" +#include +#include +#include +#include +#include "sonic-db-cli.h" + +std::string db_file = "./database_config.json"; +std::string global_db_file = "./database_global.json"; + +#define TEST_DB "APPL_DB" +#define TEST_NAMESPACE "asic0" +#define INVALID_NAMESPACE "invalid" + +class SwsscommonEnvironment : public ::testing::Environment { +public: + void SetUp() override { + // load local config file + //SonicDBConfig::initialize(db_file); + //EXPECT_TRUE(SonicDBConfig::isInit()); + + // load local global file + swss::SonicDBConfig::initializeGlobalConfig(global_db_file); + EXPECT_TRUE(swss::SonicDBConfig::isGlobalInit()); + } +}; + +std::string readFileContent(std::string file_name) { + std::ifstream help_output_file(file_name); + std::stringstream buffer; + buffer << help_output_file.rdbuf(); + return buffer.str(); +} + +TEST(sonic_db_cli, test_cli_help) { + char *args[2]; + args[0] = "sonic-db-cli"; + args[1] = "-h"; + + testing::internal::CaptureStdout(); + EXPECT_EQ(0, sonic_db_cli(1, args)); + auto output = testing::internal::GetCapturedStdout(); + auto expected_output = readFileContent("help_output.txt"); + EXPECT_EQ(expected_output, output); +} + +TEST(sonic_db_cli, test_cli_default_ns_run_cmd) { + char *args[5]; + args[0] = "sonic-db-cli"; + args[1] = "APPL_DB"; + args[2] = "HGET"; + args[3] = "VLAN_TABLE:Vlan10"; + args[4] = "mtu"; + + testing::internal::CaptureStdout(); + EXPECT_EQ(0, sonic_db_cli(4, args)); + auto output = testing::internal::GetCapturedStdout(); + std::cout << output < Date: Thu, 21 Apr 2022 08:35:47 +0000 Subject: [PATCH 02/20] Add make file --- configure.ac | 1 + 1 file changed, 1 insertion(+) diff --git a/configure.ac b/configure.ac index ff909b1cc..27e2bc0a6 100644 --- a/configure.ac +++ b/configure.ac @@ -99,6 +99,7 @@ AC_CONFIG_FILES([ pyext/py2/Makefile pyext/py3/Makefile tests/Makefile + sonic-db-cli/Makefile ]) AC_OUTPUT From 0da2adb37408b7da5f334fc1001a11ca698f8027 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Fri, 22 Apr 2022 09:39:24 +0000 Subject: [PATCH 03/20] Improve build issue --- Makefile.am | 2 +- configure.ac | 6 ++- debian/control | 6 +++ debian/sonic-db-cli.dirs | 1 + debian/sonic-db-cli.install | 1 + sonic-db-cli/Makefile.am | 28 ++++---------- sonic-db-cli/configure.ac | 70 ----------------------------------- sonic-db-cli/debian/changelog | 5 --- sonic-db-cli/debian/control | 14 ------- sonic-db-cli/debian/copyright | 1 - sonic-db-cli/debian/rules | 25 ------------- sonic-db-cli/sonic-db-cli.cpp | 4 +- 12 files changed, 24 insertions(+), 139 deletions(-) create mode 100755 debian/sonic-db-cli.dirs create mode 100755 debian/sonic-db-cli.install delete mode 100755 sonic-db-cli/configure.ac delete mode 100755 sonic-db-cli/debian/changelog delete mode 100755 sonic-db-cli/debian/control delete mode 100755 sonic-db-cli/debian/copyright delete mode 100755 sonic-db-cli/debian/rules diff --git a/Makefile.am b/Makefile.am index 5140185f8..a77d6b9a4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,3 +1,3 @@ -SUBDIRS = common pyext tests +SUBDIRS = common pyext sonic-db-cli tests ACLOCAL_AMFLAGS = -I m4 diff --git a/configure.ac b/configure.ac index 27e2bc0a6..66e515bf2 100644 --- a/configure.ac +++ b/configure.ac @@ -20,6 +20,10 @@ PKG_CHECK_MODULES([LIBNL], [libnl-3.0 libnl-genl-3.0 libnl-route-3.0 libnl-nf-3. CFLAGS="$CFLAGS $LIBNL_CFLAGS" LIBS="$LIBS $LIBNL_LIBS" +dnl Check boost libs: https://www.gnu.org/software/autoconf-archive/The-Macros.html#The-Macros +AX_BOOST_BASE(,, [AC_MSG_ERROR([boost lib missing])]) +AX_BOOST_PROGRAM_OPTIONS + LDFLAGS="-Wl,--no-undefined $LDFLAGS" AC_SUBST([LDFLAGS]) @@ -98,8 +102,8 @@ AC_CONFIG_FILES([ pyext/Makefile pyext/py2/Makefile pyext/py3/Makefile - tests/Makefile sonic-db-cli/Makefile + tests/Makefile ]) AC_OUTPUT diff --git a/debian/control b/debian/control index 215c1093d..e7ae8d1d3 100644 --- a/debian/control +++ b/debian/control @@ -35,3 +35,9 @@ Architecture: any Depends: ${shlibs:Depends}, ${misc:Pre-Depends} Section: libs Description: This package contains Switch State Service common Python3 library. + +Package: sonic-db-cli +Architecture: any +Depends: ${shlibs:Depends}, ${misc:Pre-Depends} +Section: libs +Description: This package contains SONiC DB cli. diff --git a/debian/sonic-db-cli.dirs b/debian/sonic-db-cli.dirs new file mode 100755 index 000000000..e77248175 --- /dev/null +++ b/debian/sonic-db-cli.dirs @@ -0,0 +1 @@ +usr/bin diff --git a/debian/sonic-db-cli.install b/debian/sonic-db-cli.install new file mode 100755 index 000000000..6e8e0ec68 --- /dev/null +++ b/debian/sonic-db-cli.install @@ -0,0 +1 @@ +usr/bin/sonic-db-cli diff --git a/sonic-db-cli/Makefile.am b/sonic-db-cli/Makefile.am index a3569af87..668e9bdc7 100755 --- a/sonic-db-cli/Makefile.am +++ b/sonic-db-cli/Makefile.am @@ -1,24 +1,12 @@ -########################################################################### -## -## File: ./Makefile.am -## Versions: $Id: Makefile.am,v 1.0 2022/04/02 12:04:29 liuh@microsoft.com Exp $ -## Created: 2022/04/02 -## -########################################################################### -ACLOCAL_AMFLAGS = -I m4 --install -ACLOCAL_AMFLAGS = -I config -AUTOMAKE_OPTIONS = subdir-objects -SUBDIRS = tests +INCLUDES = -I $(top_srcdir) + +if DEBUG +DBGFLAGS = -ggdb -DDEBUG +else +DBGFLAGS = -g -DNDEBUG +endif bin_PROGRAMS = sonic-db-cli sonic_db_cli_SOURCES = sonic-db-cli.h sonic-db-cli.cpp sonic_db_cli_CPPFLAGS = $(AM_CPPFLAGS) -std=c++17 -sonic_db_cli_LDFLAGS = -lswsscommon $(BOOST_PROGRAM_OPTIONS_LIB) - -EXTRA_DIST = bootstrap sonic-db-cli.spec - -MAINTAINERCLEANFILES = Makefile.in config.h.in configure aclocal.m4 \ - config/config.guess config/config.sub config/depcomp \ - config/install-sh config/ltmain.sh config/missing - -pkgconfigdir = $(libdir)/pkgconfig +sonic_db_cli_LDFLAGS = -L$(top_srcdir)/common -lswsscommon $(BOOST_PROGRAM_OPTIONS_LIB) diff --git a/sonic-db-cli/configure.ac b/sonic-db-cli/configure.ac deleted file mode 100755 index b407135e2..000000000 --- a/sonic-db-cli/configure.ac +++ /dev/null @@ -1,70 +0,0 @@ -dnl -dnl File: configure.in -dnl Revision: $Id: configure.ac,v 1.0 2022/04/02 12:04:29 liuh@microsoft.com Exp $ -dnl Created: 2022/04/02 -dnl Author: Liu Hua -dnl -dnl Process this file with autoconf to produce a configure script -dnl You need autoconf 2.59 or better! -dnl -dnl --------------------------------------------------------------------------- - -AC_PREREQ(2.59) -AC_COPYRIGHT([ -See the included file: COPYING for copyright information. -]) -AC_INIT(sonic-db-cli, 1.0.0, [liuh@microsoft.com]) - -AC_CONFIG_AUX_DIR(config) -AM_INIT_AUTOMAKE([foreign]) -AC_CONFIG_SRCDIR([sonic-db-cli.cpp]) -AC_CONFIG_HEADER([config.h]) -AC_CONFIG_MACRO_DIR([config]) -AC_CONFIG_MACRO_DIR([m4]) - -dnl -------------------------------------------------------------------- -dnl Checks for programs. -AC_LANG_C -AC_LANG([C++]) -AC_PROG_CC -AC_PROG_CXX -AM_PROG_CC_C_O -AC_PROG_INSTALL -AC_PROG_LN_S -AC_PROG_MAKE_SET -AC_ENABLE_SHARED -AC_DISABLE_STATIC -AM_PROG_LIBTOOL - -dnl -------------------------------------------------------------------- -dnl Checks for libraries. -AC_CHECK_LIB(swsscommon, conn) -dnl Check boost libs: https://www.gnu.org/software/autoconf-archive/The-Macros.html#The-Macros -AX_BOOST_BASE(,, [AC_MSG_ERROR([boost lib missing])]) -AX_BOOST_PROGRAM_OPTIONS - -dnl -------------------------------------------------------------------- -dnl Checks for header files. -AC_HEADER_STDC -AC_CHECK_HEADER([swss/sonicv2connector.h], [], [AC_MSG_ERROR([swsscommon lib missing. ])] ) - -dnl -------------------------------------------------------------------- -dnl Checks for typedefs, structures, and compiler characteristics. -AC_C_CONST -AC_TYPE_SIZE_T -AC_HEADER_TIME - -dnl -------------------------------------------------------------------- -dnl Checks for library functions. -AC_FUNC_REALLOC -AC_FUNC_SELECT_ARGTYPES -AC_TYPE_SIGNAL -AC_CHECK_FUNCS([bzero gethostbyname gettimeofday inet_ntoa select socket logwtmp getrandom]) - -dnl -------------------------------------------------------------------- -dnl Generate made files -AC_CONFIG_FILES([ - Makefile - tests/Makefile -]) -AC_OUTPUT diff --git a/sonic-db-cli/debian/changelog b/sonic-db-cli/debian/changelog deleted file mode 100755 index 48c2cce85..000000000 --- a/sonic-db-cli/debian/changelog +++ /dev/null @@ -1,5 +0,0 @@ -sonic-db-cli (1.0.0) unstable; urgency=medium - - * Initial release (Closes: #nnnn) - - -- Liu Hua Wed, 06 Apr 2022 09:21:18 +0000 diff --git a/sonic-db-cli/debian/control b/sonic-db-cli/debian/control deleted file mode 100755 index 6a526bedb..000000000 --- a/sonic-db-cli/debian/control +++ /dev/null @@ -1,14 +0,0 @@ -Source: sonic-db-cli -Section: unknown -Priority: optional -Maintainer: Liu Hua -Build-Depends: debhelper-compat (= 13) -Standards-Version: 4.5.1 -Homepage: -Rules-Requires-Root: no - -Package: sonic-db-cli -Architecture: any -Depends: ${shlibs:Depends}, ${misc:Depends} -Description: - diff --git a/sonic-db-cli/debian/copyright b/sonic-db-cli/debian/copyright deleted file mode 100755 index 7cf565eff..000000000 --- a/sonic-db-cli/debian/copyright +++ /dev/null @@ -1 +0,0 @@ -Copyright (C) 2022 Microsoft, Inc \ No newline at end of file diff --git a/sonic-db-cli/debian/rules b/sonic-db-cli/debian/rules deleted file mode 100755 index 59ea751e6..000000000 --- a/sonic-db-cli/debian/rules +++ /dev/null @@ -1,25 +0,0 @@ -#!/usr/bin/make -f -# See debhelper(7) (uncomment to enable) -# output every command that modifies files on the build system. -#export DH_VERBOSE = 1 - - -# see FEATURE AREAS in dpkg-buildflags(1) -#export DEB_BUILD_MAINT_OPTIONS = hardening=+all - -# see ENVIRONMENT in dpkg-buildflags(1) -# package maintainers to append CFLAGS -#export DEB_CFLAGS_MAINT_APPEND = -Wall -pedantic -# package maintainers to append LDFLAGS -#export DEB_LDFLAGS_MAINT_APPEND = -Wl,--as-needed - - -%: - dh $@ - - -# dh_make generated override targets -# This is example for Cmake (See https://bugs.debian.org/641051 ) -#override_dh_auto_configure: -# dh_auto_configure -- \ -# -DCMAKE_LIBRARY_PATH=$(DEB_HOST_MULTIARCH) diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index 3ba7e56ee..404e601bd 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -1,4 +1,4 @@ -#include +//#include #include #include @@ -127,7 +127,7 @@ void handleAllInstances( // Operate All Redis Instances in Parallel // TODO: if one of the operations failed, it could fail quickly and not necessary to wait all other operations std::for_each( - std::execution::par, + //std::execution::par, dbNames.begin(), dbNames.end(), [=](auto&& db_name) From 593e2beca483bd1da771bbe3e336aa63150d9f90 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Sun, 24 Apr 2022 05:17:15 +0000 Subject: [PATCH 04/20] Fix build deb issue --- debian/sonic-db-cli.dirs | 0 debian/sonic-db-cli.install | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 debian/sonic-db-cli.dirs mode change 100755 => 100644 debian/sonic-db-cli.install diff --git a/debian/sonic-db-cli.dirs b/debian/sonic-db-cli.dirs old mode 100755 new mode 100644 diff --git a/debian/sonic-db-cli.install b/debian/sonic-db-cli.install old mode 100755 new mode 100644 From 3f9a015ef71a1bbe4b43da4b8b268eb20960b174 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Sun, 24 Apr 2022 08:54:10 +0000 Subject: [PATCH 05/20] Fix build issue --- sonic-db-cli/Makefile.am | 4 +- sonic-db-cli/sonic-db-cli.cpp | 294 +++++++++++++++++++--------------- sonic-db-cli/sonic-db-cli.h | 29 ++-- 3 files changed, 189 insertions(+), 138 deletions(-) diff --git a/sonic-db-cli/Makefile.am b/sonic-db-cli/Makefile.am index 668e9bdc7..cedace807 100755 --- a/sonic-db-cli/Makefile.am +++ b/sonic-db-cli/Makefile.am @@ -8,5 +8,7 @@ endif bin_PROGRAMS = sonic-db-cli sonic_db_cli_SOURCES = sonic-db-cli.h sonic-db-cli.cpp -sonic_db_cli_CPPFLAGS = $(AM_CPPFLAGS) -std=c++17 + +sonic_db_cli_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) +sonic_db_cli_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON) sonic_db_cli_LDFLAGS = -L$(top_srcdir)/common -lswsscommon $(BOOST_PROGRAM_OPTIONS_LIB) diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index 404e601bd..013a7effc 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -1,29 +1,27 @@ -//#include #include - -#include -#include - #include "sonic-db-cli.h" -static std::string name_space; -static std::string db_or_operation; -const char* empty_str = ""; +using namespace swss; +using namespace std; -void printUsage(const po::options_description &all_options) +static string g_netns; +static string g_dbOrOperation; +const char* emptyStr = ""; + +void printUsage(const po::options_description &allOptions) { - std::cout << "usage: sonic-db-cli [-h] [-s] [-n NAMESPACE] db_or_op [cmd ...]" << std::endl; - std::cout << all_options << std::endl; + cout << "usage: sonic-db-cli [-h] [-s] [-n NAMESPACE] db_or_op [cmd ...]" << endl; + cout << allOptions << endl; - std::cout << "**sudo** needed for commands accesing a different namespace [-n], or using unixsocket connection [-s]" << std::endl; - std::cout << "" << std::endl; - std::cout << "Example 1: sonic-db-cli -n asic0 CONFIG_DB keys \\*" << std::endl; - std::cout << "Example 2: sonic-db-cli -n asic2 APPL_DB HGETALL VLAN_TABLE:Vlan10" << std::endl; - std::cout << "Example 3: sonic-db-cli APPL_DB HGET VLAN_TABLE:Vlan10 mtu" << std::endl; - std::cout << "Example 4: sonic-db-cli -n asic3 APPL_DB EVAL \"return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}\" 2 k1 k2 v1 v2" << std::endl; - std::cout << "Example 5: sonic-db-cli PING | sonic-db-cli -s PING" << std::endl; - std::cout << "Example 6: sonic-db-cli SAVE | sonic-db-cli -s SAVE" << std::endl; - std::cout << "Example 7: sonic-db-cli FLUSHALL | sonic-db-cli -s FLUSHALL" << std::endl; + cout << "**sudo** needed for commands accesing a different namespace [-n], or using unixsocket connection [-s]" << endl; + cout << "" << endl; + cout << "Example 1: sonic-db-cli -n asic0 CONFIG_DB keys \\*" << endl; + cout << "Example 2: sonic-db-cli -n asic2 APPL_DB HGETALL VLAN_TABLE:Vlan10" << endl; + cout << "Example 3: sonic-db-cli APPL_DB HGET VLAN_TABLE:Vlan10 mtu" << endl; + cout << "Example 4: sonic-db-cli -n asic3 APPL_DB EVAL \"return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}\" 2 k1 k2 v1 v2" << endl; + cout << "Example 5: sonic-db-cli PING | sonic-db-cli -s PING" << endl; + cout << "Example 6: sonic-db-cli SAVE | sonic-db-cli -s SAVE" << endl; + cout << "Example 7: sonic-db-cli FLUSHALL | sonic-db-cli -s FLUSHALL" << endl; } void printRedisReply(redisReply* reply) @@ -31,13 +29,13 @@ void printRedisReply(redisReply* reply) switch(reply->type) { case REDIS_REPLY_INTEGER: - std::cout << reply->integer; + cout << reply->integer; break; case REDIS_REPLY_STRING: case REDIS_REPLY_ERROR: case REDIS_REPLY_STATUS: case REDIS_REPLY_NIL: - std::cout << std::string(reply->str, reply->len); + cout << string(reply->str, reply->len); break; case REDIS_REPLY_ARRAY: for (size_t i = 0; i < reply->elements; i++) @@ -46,123 +44,158 @@ void printRedisReply(redisReply* reply) } break; default: - std::cerr << reply->type << std::endl; - throw std::runtime_error("Unexpected reply type"); + cerr << reply->type << endl; + throw runtime_error("Unexpected reply type"); } - std::cout << std::endl; + cout << endl; } -int executeCommands( - const std::string& db_name, - std::vector& commands, - const std::string& name_space, - bool use_unix_socket) +string buildRedisOperation(vector& commands) { - std::unique_ptr dbconn; - if (name_space.compare("None") == 0) { - dbconn = std::make_unique(use_unix_socket, empty_str); + int argc = (int)commands.size(); + const char** argv = new const char*[argc]; + size_t* argvc = new size_t[argc]; + for (int i = 0; i < argc; i++) + { + argv[i] = strdup(commands[i].c_str()); + argvc[i] = commands[i].size(); } - else { - dbconn = std::make_unique(true, name_space.c_str()); + + RedisCommand command; + command.formatArgv(argc, argv, argvc); + + for (int i = 0; i < argc; i++) + { + free(const_cast(argv[i])); } + delete[] argv; + delete[] argvc; + + return string(command.c_str()); +} + +int connectDbInterface( + DBInterface& dbintf, + const string& db_name, + const string& netns, + bool isTcpConn) +{ + try + { + if (isTcpConn) + { + auto db_hostname = SonicDBConfig::getDbHostname(db_name, netns); + auto db_port = SonicDBConfig::getDbPort(db_name, netns); + dbintf.set_redis_kwargs("", db_hostname, db_port); + } + else + { + auto db_socket = SonicDBConfig::getDbSock(db_name); + dbintf.set_redis_kwargs(db_socket, "", 0); + } - try { - dbconn->connect(db_name); + int db_id = SonicDBConfig::getDbId(db_name, netns); + dbintf.connect(db_id, db_name, true); } - catch (const std::exception& e) + catch (const exception& e) { - std::cerr << "Invalid database name input: " << db_name << std::endl; - std::cerr << e.what() << std::endl; + cerr << "Invalid database name input: " << db_name << endl; + cerr << e.what() << endl; return 1; } - auto& client = dbconn->get_redis_client(db_name); + return 0; +} - size_t argc = commands.size(); - const char** argv = new const char*[argc]; - size_t* argvc = new size_t[argc]; - for (size_t i = 0; i < argc; i++) +int handleSingleOperation( + const string& netns, + const string& db_name, + const string& operation, + bool isTcpConn) +{ + DBInterface dbintf; + // Need connect DBInterface first then get redis client, because redis client is data member of DBInterface + if (connectDbInterface(dbintf, db_name, netns, isTcpConn) != 0) { - argv[i] = strdup(commands[i].c_str()); - argvc[i] = commands[i].size(); + return 1; } + auto& client = dbintf.get_redis_client(db_name); - swss::RedisCommand command; - command.formatArgv(argc, argv, argvc); - swss::RedisReply reply(&client, command); - - auto redisReply = reply.getContext(); - printRedisReply(redisReply); + RedisReply reply(&client, operation); + auto replyContext = reply.getContext(); + printRedisReply(replyContext); return 0; } -void handleSingleOperation( - const std::string& name_space, - const std::string& db_name, - const std::string& operation, - bool use_unix_socket) +int executeCommands( + const string& db_name, + vector& commands, + const string& netns, + bool isTcpConn) { - swss::SonicV2Connector_Native conn(use_unix_socket, name_space.c_str()); - conn.connect(db_name); - auto& client = conn.get_redis_client(db_name); - - swss::RedisReply reply(&client, operation); - auto redisReply = reply.getContext(); - printRedisReply(redisReply); + auto operation = buildRedisOperation(commands); + return handleSingleOperation(db_name, operation, netns, isTcpConn); } -void handleAllInstances( - const std::string& name_space, - const std::string& operation, - bool use_unix_socket) +int handleAllInstances( + const string& netns, + const string& operation, + bool isTcpConn) { - // Use the tcp connectivity if namespace is local and unixsocket cmd_option is present. - if (name_space.compare("None") == 0) { - use_unix_socket = true; - } - - auto dbNames = swss::SonicDBConfig::getDbList(name_space); - // Operate All Redis Instances in Parallel - // TODO: if one of the operations failed, it could fail quickly and not necessary to wait all other operations - std::for_each( - //std::execution::par, - dbNames.begin(), - dbNames.end(), - [=](auto&& db_name) + auto db_names = SonicDBConfig::getDbList(netns); + for (auto& db_name : db_names) + { + int result = handleSingleOperation(netns, db_name, operation, isTcpConn); + if (result != 0) { - handleSingleOperation(name_space, db_name, operation, use_unix_socket); - }); + // Stop when any operation failed + return result; + } + } + + return 0; } int handleOperation( - const po::options_description &all_options, - const po::variables_map &variables_map) + const po::options_description &allOptions, + const po::variables_map &variablesMap) { - if (variables_map.count("db_or_op")) { - auto db_or_operation = variables_map["db_or_op"].as(); - auto name_space = variables_map["--namespace"].as(); - bool useUnixSocket = variables_map.count("--unixsocket"); - if (name_space.compare("None") != 0) { - swss::SonicDBConfig::initializeGlobalConfig(name_space); + if (variablesMap.count("db_or_op")) + { + auto dbOrOperation = variablesMap["db_or_op"].as(); + auto netns = variablesMap["--namespace"].as(); + bool isTcpConn = variablesMap.count("--unixsocket") == 0; + if (netns != "None") + { + SonicDBConfig::initializeGlobalConfig(netns); + } + else + { + // Use the tcp connectivity if namespace is local and unixsocket cmd_option is present. + isTcpConn = true; } - if (variables_map.count("cmd")) { - auto commands = variables_map["cmd"].as< std::vector >(); - return executeCommands(db_or_operation, commands, name_space, useUnixSocket); + if (variablesMap.count("cmd")) + { + auto commands = variablesMap["cmd"].as< vector >(); + return executeCommands(dbOrOperation, commands, netns, isTcpConn); } - else if (name_space.compare("PING") == 0 || name_space.compare("SAVE") == 0 || name_space.compare("FLUSHALL") == 0) { + else if (netns.compare("PING") == 0 || netns.compare("SAVE") == 0 || netns.compare("FLUSHALL") == 0) + { // redis-cli doesn't depend on database_config.json which could raise some exceptions // sonic-db-cli catch all possible exceptions and handle it as a failure case which not return 'OK' or 'PONG' - handleAllInstances(name_space, db_or_operation, useUnixSocket); + return handleAllInstances(netns, dbOrOperation, isTcpConn); } - else { - printUsage(all_options); + else + { + printUsage(allOptions); } } - else { - printUsage(all_options); + else + { + printUsage(allOptions); } return 0; @@ -171,15 +204,15 @@ int handleOperation( void parseCliArguments( int argc, char** argv, - po::options_description &all_options, - po::variables_map &variables_map) + po::options_description &allOptions, + po::variables_map &variablesMap) { - all_options.add_options() + allOptions.add_options() ("--help,-h", "Help message") ("--unixsocket,-s", "Override use of tcp_port and use unixsocket") - ("--namespace,-n", po::value(&name_space)->default_value("None"), "Namespace string to use asic0/asic1.../asicn") - ("db_or_op", po::value(&db_or_operation)->default_value(empty_str), "Database name Or Unary operation(only PING/SAVE/FLUSHALL supported)") - ("cmd", po::value< std::vector >(), "Command to execute in database") + ("--namespace,-n", po::value(&g_netns)->default_value("None"), "Namespace string to use asic0/asic1.../asicn") + ("db_or_op", po::value(&g_dbOrOperation)->default_value(emptyStr), "Database name Or Unary operation(only PING/SAVE/FLUSHALL supported)") + ("cmd", po::value< vector >(), "Command to execute in database") ; po::positional_options_description positional_opts; @@ -187,11 +220,11 @@ void parseCliArguments( positional_opts.add("cmd", -1); po::store(po::command_line_parser(argc, argv) - .options(all_options) + .options(allOptions) .positional(positional_opts) .run(), - variables_map); - po::notify(variables_map); + variablesMap); + po::notify(variablesMap); } #ifdef TESTING @@ -200,39 +233,44 @@ int sonic_db_cli(int argc, char** argv) int main(int argc, char** argv) #endif { - po::options_description all_options("SONiC DB CLI"); - po::variables_map variables_map; + po::options_description allOptions("SONiC DB CLI"); + po::variables_map variablesMap; - try { - parseCliArguments(argc, argv, all_options, variables_map); + try + { + parseCliArguments(argc, argv, allOptions, variablesMap); } - catch (po::error_with_option_name& e) { - std::cerr << "Command Line Syntax Error: " << e.what() << std::endl; - printUsage(all_options); + catch (po::error_with_option_name& e) + { + cerr << "Command Line Syntax Error: " << e.what() << endl; + printUsage(allOptions); return -1; } - catch (po::error& e) { - std::cerr << "Command Line Error: " << e.what() << std::endl; - printUsage(all_options); + catch (po::error& e) + { + cerr << "Command Line Error: " << e.what() << endl; + printUsage(allOptions); return -1; } - if (variables_map.count("--help")) { - printUsage(all_options); + if (variablesMap.count("--help")) + { + printUsage(allOptions); return 0; } try { - return handleOperation(all_options, variables_map); + return handleOperation(allOptions, variablesMap); } - catch (const std::exception& e) + catch (const exception& e) { - std::cerr << "An exception of type " << e.what() << " occurred. Arguments:" << std::endl; - for (int idx = 0; idx < argc; idx++) { - std::cerr << argv[idx] << " "; + cerr << "An exception of type " << e.what() << " occurred. Arguments:" << endl; + for (int idx = 0; idx < argc; idx++) + { + cerr << argv[idx] << " "; } - std::cerr << std::endl; + cerr << endl; return 1; } diff --git a/sonic-db-cli/sonic-db-cli.h b/sonic-db-cli/sonic-db-cli.h index effda9507..f4b6f01a6 100755 --- a/sonic-db-cli/sonic-db-cli.h +++ b/sonic-db-cli/sonic-db-cli.h @@ -3,29 +3,40 @@ #include #include #include +#include "common/dbconnector.h" +#include "common/dbinterface.h" +#include "common/redisreply.h" namespace po = boost::program_options; void printUsage(const po::options_description &all_options); -void printRedisReply(redisReply* reply); +void printRedisReply(swss::RedisReply& reply); + +std::string buildRedisOperation(std::vector& commands); + +int connectDbInterface( + swss::DBInterface& dbintf, + const std::string& db_name, + const std::string& netns, + bool isTcpConn); int executeCommands( const std::string& db_name, std::vector& commands, - const std::string& name_space, - bool use_unix_socket); + const std::string& netns, + bool isTcpConn); -void handleSingleOperation( - const std::string& name_space, +int handleSingleOperation( + const std::string& netns, const std::string& db_name, const std::string& operation, - bool use_unix_socket); + bool isTcpConn); -void handleAllInstances( - const std::string& name_space, +int handleAllInstances( + const std::string& netns, const std::string& operation, - bool use_unix_socket); + bool isTcpConn); int handleOperation( const po::options_description &all_options, From 5110fdc6fe390daf3693a0ddef337e6348045a01 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Sun, 24 Apr 2022 09:13:00 +0000 Subject: [PATCH 06/20] Fix build issue on amd64 --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 66e515bf2..78f569754 100644 --- a/configure.ac +++ b/configure.ac @@ -21,7 +21,7 @@ PKG_CHECK_MODULES([LIBNL], [libnl-3.0 libnl-genl-3.0 libnl-route-3.0 libnl-nf-3. LIBS="$LIBS $LIBNL_LIBS" dnl Check boost libs: https://www.gnu.org/software/autoconf-archive/The-Macros.html#The-Macros -AX_BOOST_BASE(,, [AC_MSG_ERROR([boost lib missing])]) +AX_BOOST_BASE(,, AC_MSG_ERROR([boost lib missing])) AX_BOOST_PROGRAM_OPTIONS LDFLAGS="-Wl,--no-undefined $LDFLAGS" From e88e1cb5ed681c730daea1d4685ade5934096130 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Sun, 24 Apr 2022 09:24:34 +0000 Subject: [PATCH 07/20] Fix build issue on amd64 --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 78f569754..317641fdc 100644 --- a/configure.ac +++ b/configure.ac @@ -21,7 +21,7 @@ PKG_CHECK_MODULES([LIBNL], [libnl-3.0 libnl-genl-3.0 libnl-route-3.0 libnl-nf-3. LIBS="$LIBS $LIBNL_LIBS" dnl Check boost libs: https://www.gnu.org/software/autoconf-archive/The-Macros.html#The-Macros -AX_BOOST_BASE(,, AC_MSG_ERROR([boost lib missing])) +AX_BOOST_BASE([1.71], [], [AC_MSG_ERROR([boost lib missing])]) AX_BOOST_PROGRAM_OPTIONS LDFLAGS="-Wl,--no-undefined $LDFLAGS" From 5a5a9321b53b0295da25e8373936f1cada677d48 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Sun, 24 Apr 2022 09:32:48 +0000 Subject: [PATCH 08/20] Fix build issue on amd64 --- azure-pipelines.yml | 2 +- configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 97247b26e..67c389028 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -33,7 +33,7 @@ stages: sudo apt-get update sudo apt-get install -y make libtool m4 autoconf dh-exec debhelper cmake pkg-config \ libhiredis-dev libnl-3-dev libnl-genl-3-dev libnl-route-3-dev libnl-nf-3-dev swig3.0 \ - libpython2.7-dev libboost-dev libboost1.71-dev + libpython2.7-dev libboost-dev libboost1.71-dev autoconf-archive automake sudo apt-get install -y sudo sudo apt-get install -y redis-server redis-tools sudo apt-get install -y python3-pip diff --git a/configure.ac b/configure.ac index 317641fdc..de3fe1485 100644 --- a/configure.ac +++ b/configure.ac @@ -21,7 +21,7 @@ PKG_CHECK_MODULES([LIBNL], [libnl-3.0 libnl-genl-3.0 libnl-route-3.0 libnl-nf-3. LIBS="$LIBS $LIBNL_LIBS" dnl Check boost libs: https://www.gnu.org/software/autoconf-archive/The-Macros.html#The-Macros -AX_BOOST_BASE([1.71], [], [AC_MSG_ERROR([boost lib missing])]) +AX_BOOST_BASE([1.71], [], AC_MSG_ERROR([boost lib missing])) AX_BOOST_PROGRAM_OPTIONS LDFLAGS="-Wl,--no-undefined $LDFLAGS" From f33addf14d1f506fe042d805930c9d197d391e6e Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Sun, 24 Apr 2022 09:49:40 +0000 Subject: [PATCH 09/20] Fix build issue on amd64 --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 67c389028..f749aca39 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -33,7 +33,7 @@ stages: sudo apt-get update sudo apt-get install -y make libtool m4 autoconf dh-exec debhelper cmake pkg-config \ libhiredis-dev libnl-3-dev libnl-genl-3-dev libnl-route-3-dev libnl-nf-3-dev swig3.0 \ - libpython2.7-dev libboost-dev libboost1.71-dev autoconf-archive automake + libpython2.7-dev libboost-dev libboost1.71-dev autoconf-archive automake libboost-program-options-dev sudo apt-get install -y sudo sudo apt-get install -y redis-server redis-tools sudo apt-get install -y python3-pip From e2cc76388617a43d82ca66933bf4de4e606c70f8 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 11 May 2022 09:15:22 +0000 Subject: [PATCH 10/20] Improve code by PR comments --- sonic-db-cli/Makefile.am | 11 +- sonic-db-cli/main.cpp | 6 + sonic-db-cli/sonic-db-cli.cpp | 186 ++++++++++-------- sonic-db-cli/sonic-db-cli.h | 27 +-- sonic-db-cli/tests/Makefile.am | 15 -- sonic-db-cli/tests/mock_connector.h | 0 tests/Makefile.am | 3 +- .../cli_help_output.txt | 0 .../tests/main.cpp => tests/cli_ut.cpp | 33 +--- 9 files changed, 135 insertions(+), 146 deletions(-) create mode 100755 sonic-db-cli/main.cpp delete mode 100755 sonic-db-cli/tests/Makefile.am delete mode 100755 sonic-db-cli/tests/mock_connector.h rename sonic-db-cli/tests/help_output.txt => tests/cli_help_output.txt (100%) rename sonic-db-cli/tests/main.cpp => tests/cli_ut.cpp (50%) diff --git a/sonic-db-cli/Makefile.am b/sonic-db-cli/Makefile.am index cedace807..e54105e6c 100755 --- a/sonic-db-cli/Makefile.am +++ b/sonic-db-cli/Makefile.am @@ -6,9 +6,14 @@ else DBGFLAGS = -g -DNDEBUG endif -bin_PROGRAMS = sonic-db-cli -sonic_db_cli_SOURCES = sonic-db-cli.h sonic-db-cli.cpp +lib_LTLIBRARIES = libsonicdbcli.la +libsonicdbcli_la_SOURCES = sonic-db-cli.cpp +libsonicdbcli_la_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) +libsonicdbcli_la_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON) +libsonicdbcli_la_LDFLAGS = -L$(top_srcdir)/common -lswsscommon +bin_PROGRAMS = sonic-db-cli +sonic_db_cli_SOURCES = sonic-db-cli.cpp main.cpp sonic_db_cli_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) sonic_db_cli_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON) -sonic_db_cli_LDFLAGS = -L$(top_srcdir)/common -lswsscommon $(BOOST_PROGRAM_OPTIONS_LIB) +sonic_db_cli_LDFLAGS = -L$(top_srcdir)/common -lswsscommon diff --git a/sonic-db-cli/main.cpp b/sonic-db-cli/main.cpp new file mode 100755 index 000000000..5a2972a9e --- /dev/null +++ b/sonic-db-cli/main.cpp @@ -0,0 +1,6 @@ +#include "sonic-db-cli.h" + +int main(int argc, char** argv) +{ + return sonic_db_cli(argc, argv); +} \ No newline at end of file diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index 013a7effc..d4b6261a0 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -1,18 +1,19 @@ #include +#include #include "sonic-db-cli.h" using namespace swss; using namespace std; -static string g_netns; -static string g_dbOrOperation; -const char* emptyStr = ""; - -void printUsage(const po::options_description &allOptions) +void printUsage() { cout << "usage: sonic-db-cli [-h] [-s] [-n NAMESPACE] db_or_op [cmd ...]" << endl; - cout << allOptions << endl; - + cout << "SONiC DB CLI:" << endl; + cout << " --help Help message" << endl; + cout << " --unixsocket Override use of tcp_port and use unixsocket" << endl; + cout << " --namespace arg (=None) Namespace string to use asic0/asic1.../asicn" << endl; + cout << " db_or_op Database name Or Unary operation(only PING/SAVE/FLUSHALL supported)" << endl; + cout << " cmd Command to execute in database" << endl; cout << "**sudo** needed for commands accesing a different namespace [-n], or using unixsocket connection [-s]" << endl; cout << "" << endl; cout << "Example 1: sonic-db-cli -n asic0 CONFIG_DB keys \\*" << endl; @@ -53,59 +54,44 @@ void printRedisReply(redisReply* reply) string buildRedisOperation(vector& commands) { - int argc = (int)commands.size(); - const char** argv = new const char*[argc]; - size_t* argvc = new size_t[argc]; - for (int i = 0; i < argc; i++) + vector args; + for (auto& command : commands) { - argv[i] = strdup(commands[i].c_str()); - argvc[i] = commands[i].size(); + args.push_back(command.c_str()); } RedisCommand command; - command.formatArgv(argc, argv, argvc); - - for (int i = 0; i < argc; i++) - { - free(const_cast(argv[i])); - } - delete[] argv; - delete[] argvc; + command.formatArgv((int)args.size(), args.data(), NULL); return string(command.c_str()); } -int connectDbInterface( - DBInterface& dbintf, +shared_ptr connectDbInterface( const string& db_name, const string& netns, bool isTcpConn) { try { + int db_id = SonicDBConfig::getDbId(db_name, netns); if (isTcpConn) { - auto db_hostname = SonicDBConfig::getDbHostname(db_name, netns); - auto db_port = SonicDBConfig::getDbPort(db_name, netns); - dbintf.set_redis_kwargs("", db_hostname, db_port); + auto host = SonicDBConfig::getDbHostname(db_name, netns); + auto port = SonicDBConfig::getDbPort(db_name, netns); + return make_shared(db_id, host, port, 0); } else { auto db_socket = SonicDBConfig::getDbSock(db_name); - dbintf.set_redis_kwargs(db_socket, "", 0); + return make_shared(db_id, db_socket, 0); } - - int db_id = SonicDBConfig::getDbId(db_name, netns); - dbintf.connect(db_id, db_name, true); } catch (const exception& e) { cerr << "Invalid database name input: " << db_name << endl; cerr << e.what() << endl; - return 1; + return nullptr; } - - return 0; } int handleSingleOperation( @@ -114,15 +100,13 @@ int handleSingleOperation( const string& operation, bool isTcpConn) { - DBInterface dbintf; - // Need connect DBInterface first then get redis client, because redis client is data member of DBInterface - if (connectDbInterface(dbintf, db_name, netns, isTcpConn) != 0) + auto client = connectDbInterface(db_name, netns, isTcpConn); + if (nullptr == client) { return 1; } - auto& client = dbintf.get_redis_client(db_name); - RedisReply reply(&client, operation); + RedisReply reply(client.get(), operation); auto replyContext = reply.getContext(); printRedisReply(replyContext); @@ -158,15 +142,13 @@ int handleAllInstances( return 0; } -int handleOperation( - const po::options_description &allOptions, - const po::variables_map &variablesMap) +int handleOperation(Options &options) { - if (variablesMap.count("db_or_op")) + if (!options.m_db_or_op.empty()) { - auto dbOrOperation = variablesMap["db_or_op"].as(); - auto netns = variablesMap["--namespace"].as(); - bool isTcpConn = variablesMap.count("--unixsocket") == 0; + auto dbOrOperation = options.m_db_or_op; + auto netns = options.m_namespace; + bool isTcpConn = !options.m_unixsocket; if (netns != "None") { SonicDBConfig::initializeGlobalConfig(netns); @@ -177,9 +159,9 @@ int handleOperation( isTcpConn = true; } - if (variablesMap.count("cmd")) + if (options.m_cmd.size() != 0) { - auto commands = variablesMap["cmd"].as< vector >(); + auto commands = options.m_cmd; return executeCommands(dbOrOperation, commands, netns, isTcpConn); } else if (netns.compare("PING") == 0 || netns.compare("SAVE") == 0 || netns.compare("FLUSHALL") == 0) @@ -190,12 +172,12 @@ int handleOperation( } else { - printUsage(allOptions); + printUsage(); } } else { - printUsage(allOptions); + printUsage(); } return 0; @@ -204,64 +186,100 @@ int handleOperation( void parseCliArguments( int argc, char** argv, - po::options_description &allOptions, - po::variables_map &variablesMap) + Options &options) { - allOptions.add_options() - ("--help,-h", "Help message") - ("--unixsocket,-s", "Override use of tcp_port and use unixsocket") - ("--namespace,-n", po::value(&g_netns)->default_value("None"), "Namespace string to use asic0/asic1.../asicn") - ("db_or_op", po::value(&g_dbOrOperation)->default_value(emptyStr), "Database name Or Unary operation(only PING/SAVE/FLUSHALL supported)") - ("cmd", po::value< vector >(), "Command to execute in database") - ; + // Parse argument with getopt https://man7.org/linux/man-pages/man3/getopt.3.html + const char* short_options = "hsn"; + static struct option long_options[] = { + {"help", optional_argument, NULL, 'h' }, + {"unixsocket", optional_argument, NULL, 's' }, + {"namespace", optional_argument, NULL, 'n' }, + // The last element of the array has to be filled with zeros. + {0, 0, 0, 0 } + }; + + // prevent getopt_long print "invalid option" message. + opterr = 0; - po::positional_options_description positional_opts; - positional_opts.add("db_or_op", 1); - positional_opts.add("cmd", -1); + while(optind < argc) + { + int opt = getopt_long(argc, argv, short_options, long_options, NULL); + if (opt != -1) + { + switch (opt) { + case 'h': + options.m_help = true; + break; + + case 's': + options.m_unixsocket = true; + break; + + case 'n': + if (optind < argc) + { + options.m_namespace = argv[optind]; + optind++; + } + else + { + throw invalid_argument("namespace value is missing."); + } + break; + + default: + // argv contains unknown argument + throw invalid_argument("Unknown argument:" + string(argv[optind])); + } + } + else + { + // db_or_op and cmd are non-option arguments + // db_or_op argument + options.m_db_or_op = argv[optind]; + optind++; - po::store(po::command_line_parser(argc, argv) - .options(allOptions) - .positional(positional_opts) - .run(), - variablesMap); - po::notify(variablesMap); + // cmd arguments + while(optind < argc) + { + auto cmdstr = string(argv[optind]); + options.m_cmd.push_back(cmdstr); + optind++; + } + } + } } -#ifdef TESTING int sonic_db_cli(int argc, char** argv) -#else -int main(int argc, char** argv) -#endif { - po::options_description allOptions("SONiC DB CLI"); - po::variables_map variablesMap; - + Options options; try { - parseCliArguments(argc, argv, allOptions, variablesMap); + parseCliArguments(argc, argv, options); } - catch (po::error_with_option_name& e) + catch (invalid_argument const& e) { - cerr << "Command Line Syntax Error: " << e.what() << endl; - printUsage(allOptions); + cerr << "Command Line Error: " << e.what() << endl; + printUsage(); return -1; } - catch (po::error& e) + catch (logic_error const& e) { - cerr << "Command Line Error: " << e.what() << endl; - printUsage(allOptions); + // getopt_long throw logic_error when found a unknown option without value. + cerr << "Unknown option without value: " << endl; + printUsage(); return -1; } - if (variablesMap.count("--help")) + if (options.m_help) { - printUsage(allOptions); + printUsage(); return 0; } try { - return handleOperation(allOptions, variablesMap); + return handleOperation(options); } catch (const exception& e) { diff --git a/sonic-db-cli/sonic-db-cli.h b/sonic-db-cli/sonic-db-cli.h index f4b6f01a6..8cee9f803 100755 --- a/sonic-db-cli/sonic-db-cli.h +++ b/sonic-db-cli/sonic-db-cli.h @@ -2,21 +2,27 @@ #include #include -#include +#include #include "common/dbconnector.h" #include "common/dbinterface.h" #include "common/redisreply.h" -namespace po = boost::program_options; +struct Options +{ + bool m_help = false; + bool m_unixsocket = false; + std::string m_namespace; + std::string m_db_or_op; + std::vector m_cmd; +}; -void printUsage(const po::options_description &all_options); +void printUsage(); void printRedisReply(swss::RedisReply& reply); std::string buildRedisOperation(std::vector& commands); -int connectDbInterface( - swss::DBInterface& dbintf, +std::shared_ptr connectDbInterface( const std::string& db_name, const std::string& netns, bool isTcpConn); @@ -38,16 +44,11 @@ int handleAllInstances( const std::string& operation, bool isTcpConn); -int handleOperation( - const po::options_description &all_options, - const po::variables_map &variables_map); +int handleOperation(Options &options); void parseCliArguments( int argc, char** argv, - po::options_description &all_options, - po::variables_map &variables_map); + Options &options); -#ifdef TESTING -int sonic_db_cli(int argc, char** argv); -#endif \ No newline at end of file +int sonic_db_cli(int argc, char** argv); \ No newline at end of file diff --git a/sonic-db-cli/tests/Makefile.am b/sonic-db-cli/tests/Makefile.am deleted file mode 100755 index 428aeeb1a..000000000 --- a/sonic-db-cli/tests/Makefile.am +++ /dev/null @@ -1,15 +0,0 @@ -#INCLUDES = -I $(top_srcdir) - -bin_PROGRAMS = tests -DBGFLAGS = -ggdb -DDEBUG - -CFLAGS_GTEST = -DTESTING -Wno-write-strings -LDADD_GTEST = -L/usr/src/gtest -lgtest -lgtest_main -lgmock -lgmock_main - -tests_SOURCES = main.cpp \ - ../sonic-db-cli.cpp \ - ../sonic-db-cli.h - -tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) -tests_CPPFLAGS = $(tests_CFLAGS) -std=c++17 -tests_LDADD = $(LDADD_GTEST) -lpthread -lswsscommon $(CODE_COVERAGE_LIBS) $(BOOST_PROGRAM_OPTIONS_LIB) \ No newline at end of file diff --git a/sonic-db-cli/tests/mock_connector.h b/sonic-db-cli/tests/mock_connector.h deleted file mode 100755 index e69de29bb..000000000 diff --git a/tests/Makefile.am b/tests/Makefile.am index 19e04250c..dd223df9c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -36,8 +36,9 @@ tests_SOURCES = redis_ut.cpp \ status_code_util_test.cpp \ saiaclschema_ut.cpp \ timer_ut.cpp \ + cli_ut.cpp \ main.cpp tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) -tests_LDADD = $(LDADD_GTEST) -lpthread -L$(top_srcdir)/common -lswsscommon $(LIBNL_LIBS) $(CODE_COVERAGE_LIBS) +tests_LDADD = $(LDADD_GTEST) -lpthread -L$(top_srcdir)/common -lswsscommon $(LIBNL_LIBS) $(CODE_COVERAGE_LIBS) -L$(top_srcdir)/sonic-db-cli -lsonicdbcli diff --git a/sonic-db-cli/tests/help_output.txt b/tests/cli_help_output.txt similarity index 100% rename from sonic-db-cli/tests/help_output.txt rename to tests/cli_help_output.txt diff --git a/sonic-db-cli/tests/main.cpp b/tests/cli_ut.cpp similarity index 50% rename from sonic-db-cli/tests/main.cpp rename to tests/cli_ut.cpp index a70f482cb..893fe33cf 100755 --- a/sonic-db-cli/tests/main.cpp +++ b/tests/cli_ut.cpp @@ -1,30 +1,13 @@ #include "gtest/gtest.h" -#include #include #include #include -#include "sonic-db-cli.h" - -std::string db_file = "./database_config.json"; -std::string global_db_file = "./database_global.json"; +#include "sonic-db-cli/sonic-db-cli.h" #define TEST_DB "APPL_DB" #define TEST_NAMESPACE "asic0" #define INVALID_NAMESPACE "invalid" -class SwsscommonEnvironment : public ::testing::Environment { -public: - void SetUp() override { - // load local config file - //SonicDBConfig::initialize(db_file); - //EXPECT_TRUE(SonicDBConfig::isInit()); - - // load local global file - swss::SonicDBConfig::initializeGlobalConfig(global_db_file); - EXPECT_TRUE(swss::SonicDBConfig::isGlobalInit()); - } -}; - std::string readFileContent(std::string file_name) { std::ifstream help_output_file(file_name); std::stringstream buffer; @@ -40,7 +23,7 @@ TEST(sonic_db_cli, test_cli_help) { testing::internal::CaptureStdout(); EXPECT_EQ(0, sonic_db_cli(1, args)); auto output = testing::internal::GetCapturedStdout(); - auto expected_output = readFileContent("help_output.txt"); + auto expected_output = readFileContent("cli_help_output.txt"); EXPECT_EQ(expected_output, output); } @@ -56,16 +39,6 @@ TEST(sonic_db_cli, test_cli_default_ns_run_cmd) { EXPECT_EQ(0, sonic_db_cli(4, args)); auto output = testing::internal::GetCapturedStdout(); std::cout << output < Date: Wed, 11 May 2022 09:48:08 +0000 Subject: [PATCH 11/20] Improve performance with std::async --- sonic-db-cli/Makefile.am | 4 ++-- sonic-db-cli/sonic-db-cli.cpp | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/sonic-db-cli/Makefile.am b/sonic-db-cli/Makefile.am index e54105e6c..a8b52653a 100755 --- a/sonic-db-cli/Makefile.am +++ b/sonic-db-cli/Makefile.am @@ -10,10 +10,10 @@ lib_LTLIBRARIES = libsonicdbcli.la libsonicdbcli_la_SOURCES = sonic-db-cli.cpp libsonicdbcli_la_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) libsonicdbcli_la_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON) -libsonicdbcli_la_LDFLAGS = -L$(top_srcdir)/common -lswsscommon +libsonicdbcli_la_LDFLAGS = -L$(top_srcdir)/common -lswsscommon -lpthread bin_PROGRAMS = sonic-db-cli sonic_db_cli_SOURCES = sonic-db-cli.cpp main.cpp sonic_db_cli_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) sonic_db_cli_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON) -sonic_db_cli_LDFLAGS = -L$(top_srcdir)/common -lswsscommon +sonic_db_cli_LDFLAGS = -L$(top_srcdir)/common -lswsscommon -lpthread diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index d4b6261a0..718b15fb8 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -1,3 +1,4 @@ +#include #include #include #include "sonic-db-cli.h" @@ -131,12 +132,7 @@ int handleAllInstances( auto db_names = SonicDBConfig::getDbList(netns); for (auto& db_name : db_names) { - int result = handleSingleOperation(netns, db_name, operation, isTcpConn); - if (result != 0) - { - // Stop when any operation failed - return result; - } + std::async(std::launch::async, handleSingleOperation, netns, db_name, operation, isTcpConn); } return 0; From 1b46b5cd1d9694cd1b5e20c15e4718992fcd6f56 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 12 May 2022 11:09:56 +0000 Subject: [PATCH 12/20] Add UT and fix code bug, keep output same with python version --- azure-pipelines.yml | 2 +- common/redisreply.cpp | 35 +++ common/redisreply.h | 4 + configure.ac | 4 - sonic-db-cli/main.cpp | 7 +- sonic-db-cli/sonic-db-cli.cpp | 266 ++++++++++-------- sonic-db-cli/sonic-db-cli.h | 10 +- tests/{ => cli_test_data}/cli_help_output.txt | 19 +- tests/cli_test_data/cli_ping_output.txt | 1 + tests/cli_ut.cpp | 55 +++- 10 files changed, 260 insertions(+), 143 deletions(-) rename tests/{ => cli_test_data}/cli_help_output.txt (53%) create mode 100755 tests/cli_test_data/cli_ping_output.txt diff --git a/azure-pipelines.yml b/azure-pipelines.yml index b3405b7dc..43ca7391e 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -62,7 +62,7 @@ stages: sudo apt-get update sudo apt-get install -y make libtool m4 autoconf dh-exec debhelper cmake pkg-config \ libhiredis-dev libnl-3-dev libnl-genl-3-dev libnl-route-3-dev libnl-nf-3-dev swig3.0 \ - libpython2.7-dev libboost-dev libboost1.71-dev autoconf-archive automake libboost-program-options-dev + libpython2.7-dev libboost-dev libboost1.71-dev sudo apt-get install -y sudo sudo apt-get install -y redis-server redis-tools sudo apt-get install -y python3-pip diff --git a/common/redisreply.cpp b/common/redisreply.cpp index a2ef46e27..16badf624 100644 --- a/common/redisreply.cpp +++ b/common/redisreply.cpp @@ -225,4 +225,39 @@ template<> RedisMessage RedisReply::getReply() return ret; } + +string RedisReply::toString() +{ + return toString(getContext()); +} + +string RedisReply::toString(redisReply *reply) +{ + switch(reply->type) + { + case REDIS_REPLY_INTEGER: + return std::to_string(reply->integer); + + case REDIS_REPLY_STRING: + case REDIS_REPLY_ERROR: + case REDIS_REPLY_STATUS: + case REDIS_REPLY_NIL: + return string(reply->str, reply->len); + + case REDIS_REPLY_ARRAY: + { + stringstream result; + for (size_t i = 0; i < reply->elements; i++) + { + result << toString(reply->element[i]) << endl; + } + return result.str(); + } + + default: + SWSS_LOG_ERROR("invalid type %d for message", getContext()->type); + return string(); + } +} + } diff --git a/common/redisreply.h b/common/redisreply.h index b25653293..3deb504e8 100644 --- a/common/redisreply.h +++ b/common/redisreply.h @@ -2,6 +2,7 @@ #define __REDISREPLY__ #include +#include #include #include #include "rediscommand.h" @@ -92,9 +93,12 @@ class RedisReply /* Check that the status is QUEUED, throw exception otherwise */ void checkStatusQueued(); + std::string toString(); + private: void checkStatus(const char *status); void checkReply(); + std::string toString(redisReply *reply); redisReply *m_reply; }; diff --git a/configure.ac b/configure.ac index de3fe1485..eda00eaeb 100644 --- a/configure.ac +++ b/configure.ac @@ -20,10 +20,6 @@ PKG_CHECK_MODULES([LIBNL], [libnl-3.0 libnl-genl-3.0 libnl-route-3.0 libnl-nf-3. CFLAGS="$CFLAGS $LIBNL_CFLAGS" LIBS="$LIBS $LIBNL_LIBS" -dnl Check boost libs: https://www.gnu.org/software/autoconf-archive/The-Macros.html#The-Macros -AX_BOOST_BASE([1.71], [], AC_MSG_ERROR([boost lib missing])) -AX_BOOST_PROGRAM_OPTIONS - LDFLAGS="-Wl,--no-undefined $LDFLAGS" AC_SUBST([LDFLAGS]) diff --git a/sonic-db-cli/main.cpp b/sonic-db-cli/main.cpp index 5a2972a9e..a827e6fbc 100755 --- a/sonic-db-cli/main.cpp +++ b/sonic-db-cli/main.cpp @@ -1,6 +1,11 @@ #include "sonic-db-cli.h" +#include "common/dbconnector.h" int main(int argc, char** argv) { - return sonic_db_cli(argc, argv); + return sonic_db_cli( + swss::SonicDBConfig::DEFAULT_SONIC_DB_CONFIG_FILE, + swss::SonicDBConfig::DEFAULT_SONIC_DB_GLOBAL_CONFIG_FILE, + argc, + argv); } \ No newline at end of file diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index 718b15fb8..fbebb71c7 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include "sonic-db-cli.h" using namespace swss; @@ -8,15 +9,22 @@ using namespace std; void printUsage() { - cout << "usage: sonic-db-cli [-h] [-s] [-n NAMESPACE] db_or_op [cmd ...]" << endl; + cout << "usage: sonic-db-cli [-h] [-s] [-n NAMESPACE] db_or_op [cmd [cmd ...]]" << endl; + cout << endl; cout << "SONiC DB CLI:" << endl; - cout << " --help Help message" << endl; - cout << " --unixsocket Override use of tcp_port and use unixsocket" << endl; - cout << " --namespace arg (=None) Namespace string to use asic0/asic1.../asicn" << endl; - cout << " db_or_op Database name Or Unary operation(only PING/SAVE/FLUSHALL supported)" << endl; - cout << " cmd Command to execute in database" << endl; + cout << endl; + cout << "positional arguments:" << endl; + cout << " db_or_op Database name Or Unary operation(only PING/SAVE/FLUSHALL supported)" << endl; + cout << " cmd Command to execute in database" << endl; + cout << endl; + cout << "optional arguments:" << endl; + cout << " -h, --help show this help message and exit" << endl; + cout << " -s, --unixsocket Override use of tcp_port and use unixsocket" << endl; + cout << " -n NAMESPACE, --namespace NAMESPACE" << endl; + cout << " Namespace string to use asic0/asic1.../asicn" << endl; + cout << endl; cout << "**sudo** needed for commands accesing a different namespace [-n], or using unixsocket connection [-s]" << endl; - cout << "" << endl; + cout << endl; cout << "Example 1: sonic-db-cli -n asic0 CONFIG_DB keys \\*" << endl; cout << "Example 2: sonic-db-cli -n asic2 APPL_DB HGETALL VLAN_TABLE:Vlan10" << endl; cout << "Example 3: sonic-db-cli APPL_DB HGET VLAN_TABLE:Vlan10 mtu" << endl; @@ -26,33 +34,6 @@ void printUsage() cout << "Example 7: sonic-db-cli FLUSHALL | sonic-db-cli -s FLUSHALL" << endl; } -void printRedisReply(redisReply* reply) -{ - switch(reply->type) - { - case REDIS_REPLY_INTEGER: - cout << reply->integer; - break; - case REDIS_REPLY_STRING: - case REDIS_REPLY_ERROR: - case REDIS_REPLY_STATUS: - case REDIS_REPLY_NIL: - cout << string(reply->str, reply->len); - break; - case REDIS_REPLY_ARRAY: - for (size_t i = 0; i < reply->elements; i++) - { - printRedisReply(reply->element[i]); - } - break; - default: - cerr << reply->type << endl; - throw runtime_error("Unexpected reply type"); - } - - cout << endl; -} - string buildRedisOperation(vector& commands) { vector args; @@ -67,61 +48,53 @@ string buildRedisOperation(vector& commands) return string(command.c_str()); } -shared_ptr connectDbInterface( - const string& db_name, +string handleSingleOperation( const string& netns, + const string& db_name, + const string& operation, bool isTcpConn) { + shared_ptr client; + auto host = SonicDBConfig::getDbHostname(db_name, netns); + string message = "Could not connect to Redis at " + host + ":"; try { - int db_id = SonicDBConfig::getDbId(db_name, netns); - if (isTcpConn) + auto db_id = SonicDBConfig::getDbId(db_name, netns); + if (!isTcpConn && db_name != "redis_chassis.server") { - auto host = SonicDBConfig::getDbHostname(db_name, netns); - auto port = SonicDBConfig::getDbPort(db_name, netns); - return make_shared(db_id, host, port, 0); + auto db_socket = SonicDBConfig::getDbSock(db_name); + message += db_name + ": Connection refused"; + client = make_shared(db_id, db_socket, 0); } else { - auto db_socket = SonicDBConfig::getDbSock(db_name); - return make_shared(db_id, db_socket, 0); + auto port = SonicDBConfig::getDbPort(db_name, netns); + message += port + ": Connection refused"; + client = make_shared(db_id, host, port, 0); } } catch (const exception& e) { - cerr << "Invalid database name input: " << db_name << endl; - cerr << e.what() << endl; - return nullptr; + return message; } -} -int handleSingleOperation( - const string& netns, - const string& db_name, - const string& operation, - bool isTcpConn) -{ - auto client = connectDbInterface(db_name, netns, isTcpConn); - if (nullptr == client) + if (operation == "PING" + || operation == "SAVE" + || operation == "FLUSHALL") { - return 1; + RedisReply reply(client.get(), operation); + auto response = reply.getContext(); + if (nullptr != response) + { + return string(); + } + } + else + { + throw std::invalid_argument("Operation " + operation +" is not supported"); } - RedisReply reply(client.get(), operation); - auto replyContext = reply.getContext(); - printRedisReply(replyContext); - - return 0; -} - -int executeCommands( - const string& db_name, - vector& commands, - const string& netns, - bool isTcpConn) -{ - auto operation = buildRedisOperation(commands); - return handleSingleOperation(db_name, operation, netns, isTcpConn); + return message; } int handleAllInstances( @@ -130,52 +103,80 @@ int handleAllInstances( bool isTcpConn) { auto db_names = SonicDBConfig::getDbList(netns); + // Operate All Redis Instances in Parallel + // TODO: if one of the operations failed, it could fail quickly and not necessary to wait all other operations + list> responses; for (auto& db_name : db_names) { - std::async(std::launch::async, handleSingleOperation, netns, db_name, operation, isTcpConn); + future response = std::async(std::launch::async, handleSingleOperation, netns, db_name, operation, isTcpConn); + responses.push_back(std::move(response)); + } + + bool operation_failed = false; + for (auto& response : responses) + { + if (response.get() != "") + { + cout << response.get() << endl; + operation_failed = true; + } + } + + if (operation_failed) + { + return 1; + } + + if (operation == "PING") + { + cout << "PONG" << endl; + } + else + { + cout << "OK" << endl; } return 0; } -int handleOperation(Options &options) +int executeCommands( + const string& db_name, + vector& commands, + const string& netns, + bool isTcpConn) { - if (!options.m_db_or_op.empty()) + shared_ptr client = nullptr; + try { - auto dbOrOperation = options.m_db_or_op; - auto netns = options.m_namespace; - bool isTcpConn = !options.m_unixsocket; - if (netns != "None") - { - SonicDBConfig::initializeGlobalConfig(netns); - } - else - { - // Use the tcp connectivity if namespace is local and unixsocket cmd_option is present. - isTcpConn = true; - } - - if (options.m_cmd.size() != 0) - { - auto commands = options.m_cmd; - return executeCommands(dbOrOperation, commands, netns, isTcpConn); - } - else if (netns.compare("PING") == 0 || netns.compare("SAVE") == 0 || netns.compare("FLUSHALL") == 0) + int db_id = SonicDBConfig::getDbId(db_name, netns); + if (isTcpConn) { - // redis-cli doesn't depend on database_config.json which could raise some exceptions - // sonic-db-cli catch all possible exceptions and handle it as a failure case which not return 'OK' or 'PONG' - return handleAllInstances(netns, dbOrOperation, isTcpConn); + auto host = SonicDBConfig::getDbHostname(db_name, netns); + auto port = SonicDBConfig::getDbPort(db_name, netns); + client = make_shared(db_id, host, port, 0); } else { - printUsage(); + auto db_socket = SonicDBConfig::getDbSock(db_name); + client = make_shared(db_id, db_socket, 0); } } - else + catch (const exception& e) { - printUsage(); + cerr << "Invalid database name input : '" << db_name << "'" << endl; + cerr << e.what() << endl; + return 1; } + auto operation = buildRedisOperation(commands); + RedisReply reply(client.get(), operation); + /* + sonic-db-cli output format mimic the non-tty mode output format from redis-cli + based on our usage in SONiC, None and list type output from python API needs to be modified + with these changes, it is enough for us to mimic redis-cli in SONiC so far since no application uses tty mode redis-cli output + */ + cout << reply.toString() << endl; + return 0; } @@ -196,7 +197,6 @@ void parseCliArguments( // prevent getopt_long print "invalid option" message. opterr = 0; - while(optind < argc) { int opt = getopt_long(argc, argv, short_options, long_options, NULL); @@ -231,11 +231,9 @@ void parseCliArguments( else { // db_or_op and cmd are non-option arguments - // db_or_op argument options.m_db_or_op = argv[optind]; optind++; - // cmd arguments while(optind < argc) { auto cmdstr = string(argv[optind]); @@ -246,7 +244,11 @@ void parseCliArguments( } } -int sonic_db_cli(int argc, char** argv) +int sonic_db_cli( + const string &config_file, + const string &global_config_file, + int argc, + char** argv) { Options options; try @@ -273,19 +275,57 @@ int sonic_db_cli(int argc, char** argv) return 0; } - try - { - return handleOperation(options); - } - catch (const exception& e) + if (!options.m_db_or_op.empty()) { - cerr << "An exception of type " << e.what() << " occurred. Arguments:" << endl; - for (int idx = 0; idx < argc; idx++) + auto dbOrOperation = options.m_db_or_op; + auto netns = options.m_namespace; + bool isTcpConn = !options.m_unixsocket; + // Load the database config for the namespace + if (netns != "None") { - cerr << argv[idx] << " "; + SonicDBConfig::initializeGlobalConfig(global_config_file); } - cerr << endl; - return 1; + else + { + SonicDBConfig::initializeGlobalConfig(config_file); + // Use the tcp connectivity if namespace is local and unixsocket cmd_option is present. + isTcpConn = true; + } + + if (options.m_cmd.size() != 0) + { + auto commands = options.m_cmd; + return executeCommands(dbOrOperation, commands, netns, isTcpConn); + } + else if (dbOrOperation.compare("PING") == 0 + || dbOrOperation.compare("SAVE") == 0 + || dbOrOperation.compare("FLUSHALL") == 0) + { + // redis-cli doesn't depend on database_config.json which could raise some exceptions + // sonic-db-cli catch all possible exceptions and handle it as a failure case which not return 'OK' or 'PONG' + try + { + return handleAllInstances(netns, dbOrOperation, isTcpConn); + } + catch (const exception& e) + { + cerr << "An exception of type " << e.what() << " occurred. Arguments:" << endl; + for (int idx = 0; idx < argc; idx++) + { + cerr << argv[idx] << " "; + } + cerr << endl; + return 1; + } + } + else + { + printUsage(); + } + } + else + { + printUsage(); } return 0; diff --git a/sonic-db-cli/sonic-db-cli.h b/sonic-db-cli/sonic-db-cli.h index 8cee9f803..1c3cfbdf9 100755 --- a/sonic-db-cli/sonic-db-cli.h +++ b/sonic-db-cli/sonic-db-cli.h @@ -33,7 +33,7 @@ int executeCommands( const std::string& netns, bool isTcpConn); -int handleSingleOperation( +std::string handleSingleOperation( const std::string& netns, const std::string& db_name, const std::string& operation, @@ -44,11 +44,13 @@ int handleAllInstances( const std::string& operation, bool isTcpConn); -int handleOperation(Options &options); - void parseCliArguments( int argc, char** argv, Options &options); -int sonic_db_cli(int argc, char** argv); \ No newline at end of file +int sonic_db_cli( + const std::string &config_file, + const std::string &global_config_file, + int argc, + char** argv); \ No newline at end of file diff --git a/tests/cli_help_output.txt b/tests/cli_test_data/cli_help_output.txt similarity index 53% rename from tests/cli_help_output.txt rename to tests/cli_test_data/cli_help_output.txt index ff2ea2826..1d286df17 100755 --- a/tests/cli_help_output.txt +++ b/tests/cli_test_data/cli_help_output.txt @@ -1,11 +1,16 @@ -usage: sonic-db-cli [-h] [-s] [-n NAMESPACE] db_or_op [cmd ...] +usage: sonic-db-cli [-h] [-s] [-n NAMESPACE] db_or_op [cmd [cmd ...]] + SONiC DB CLI: - ----help Help message - ----unixsocket Override use of tcp_port and use unixsocket - ----namespace arg (=None) Namespace string to use asic0/asic1.../asicn - --db_or_op arg Database name Or Unary operation(only - PING/SAVE/FLUSHALL supported) - --cmd arg Command to execute in database + +positional arguments: + db_or_op Database name Or Unary operation(only PING/SAVE/FLUSHALL supported) + cmd Command to execute in database + +optional arguments: + -h, --help show this help message and exit + -s, --unixsocket Override use of tcp_port and use unixsocket + -n NAMESPACE, --namespace NAMESPACE + Namespace string to use asic0/asic1.../asicn **sudo** needed for commands accesing a different namespace [-n], or using unixsocket connection [-s] diff --git a/tests/cli_test_data/cli_ping_output.txt b/tests/cli_test_data/cli_ping_output.txt new file mode 100755 index 000000000..87eb7c81d --- /dev/null +++ b/tests/cli_test_data/cli_ping_output.txt @@ -0,0 +1 @@ +PONG diff --git a/tests/cli_ut.cpp b/tests/cli_ut.cpp index 893fe33cf..631bcfae4 100755 --- a/tests/cli_ut.cpp +++ b/tests/cli_ut.cpp @@ -2,12 +2,17 @@ #include #include #include +#include +#include #include "sonic-db-cli/sonic-db-cli.h" -#define TEST_DB "APPL_DB" +#define TEST_DB "STATE_DB2" #define TEST_NAMESPACE "asic0" #define INVALID_NAMESPACE "invalid" +const std::string config_file = "./tests/redis_multi_db_ut_config/database_config.json"; +const std::string global_config_file = "./tests/redis_multi_db_ut_config/database_global.json"; + std::string readFileContent(std::string file_name) { std::ifstream help_output_file(file_name); std::stringstream buffer; @@ -19,26 +24,50 @@ TEST(sonic_db_cli, test_cli_help) { char *args[2]; args[0] = "sonic-db-cli"; args[1] = "-h"; - + + optind = 0; testing::internal::CaptureStdout(); - EXPECT_EQ(0, sonic_db_cli(1, args)); + EXPECT_EQ(0, sonic_db_cli(config_file, global_config_file, 2, args)); auto output = testing::internal::GetCapturedStdout(); - auto expected_output = readFileContent("cli_help_output.txt"); + auto expected_output = readFileContent("./tests/cli_test_data/cli_help_output.txt"); EXPECT_EQ(expected_output, output); } -TEST(sonic_db_cli, test_cli_default_ns_run_cmd) { +TEST(sonic_db_cli, test_cli_ping_cmd) { + char *args[2]; + args[0] = "sonic-db-cli"; + args[1] = "PING"; + + optind = 0; + testing::internal::CaptureStdout(); + EXPECT_EQ(0, sonic_db_cli(config_file, global_config_file, 2, args)); + auto output = testing::internal::GetCapturedStdout(); + std::cout << output < Date: Thu, 12 May 2022 11:16:53 +0000 Subject: [PATCH 13/20] Fix PR comments --- sonic-db-cli/sonic-db-cli.cpp | 6 +++--- tests/Makefile.am | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index fbebb71c7..6406ebd58 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -297,9 +297,9 @@ int sonic_db_cli( auto commands = options.m_cmd; return executeCommands(dbOrOperation, commands, netns, isTcpConn); } - else if (dbOrOperation.compare("PING") == 0 - || dbOrOperation.compare("SAVE") == 0 - || dbOrOperation.compare("FLUSHALL") == 0) + else if (dbOrOperation == "PING" + || dbOrOperation == "SAVE" + || dbOrOperation == "FLUSHALL") { // redis-cli doesn't depend on database_config.json which could raise some exceptions // sonic-db-cli catch all possible exceptions and handle it as a failure case which not return 'OK' or 'PONG' diff --git a/tests/Makefile.am b/tests/Makefile.am index dd223df9c..1f0001f3a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -41,4 +41,4 @@ tests_SOURCES = redis_ut.cpp \ tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) -tests_LDADD = $(LDADD_GTEST) -lpthread -L$(top_srcdir)/common -lswsscommon $(LIBNL_LIBS) $(CODE_COVERAGE_LIBS) -L$(top_srcdir)/sonic-db-cli -lsonicdbcli +tests_LDADD = $(LDADD_GTEST) -lpthread -L$(top_srcdir)/common -lswsscommon $(LIBNL_LIBS) $(CODE_COVERAGE_LIBS) -L$(top_srcdir)/sonic-db-cli -lsonicdbcli From d09ed85b4cab93643d1322b989f7c5b21144d804 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Thu, 12 May 2022 12:17:59 +0000 Subject: [PATCH 14/20] Fix redis command build issue --- common/rediscommand.cpp | 10 ++++++++++ sonic-db-cli/sonic-db-cli.cpp | 19 +++---------------- sonic-db-cli/sonic-db-cli.h | 2 -- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/common/rediscommand.cpp b/common/rediscommand.cpp index 51fe98b9e..a24f3a6b8 100644 --- a/common/rediscommand.cpp +++ b/common/rediscommand.cpp @@ -49,6 +49,16 @@ void RedisCommand::formatArgv(int argc, const char **argv, const size_t *argvlen } } +void RedisCommand::format(const vector &commands) +{ + vector args; + for (auto& command : commands) + { + args.push_back(command.c_str()); + } + formatArgv(static_cast(args.size()), args.data(), NULL); +} + /* Format HMSET key multiple field value command */ void RedisCommand::formatHMSET(const std::string &key, const std::vector &values) diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index 6406ebd58..6a8594d08 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -34,20 +34,6 @@ void printUsage() cout << "Example 7: sonic-db-cli FLUSHALL | sonic-db-cli -s FLUSHALL" << endl; } -string buildRedisOperation(vector& commands) -{ - vector args; - for (auto& command : commands) - { - args.push_back(command.c_str()); - } - - RedisCommand command; - command.formatArgv((int)args.size(), args.data(), NULL); - - return string(command.c_str()); -} - string handleSingleOperation( const string& netns, const string& db_name, @@ -168,8 +154,9 @@ int executeCommands( return 1; } - auto operation = buildRedisOperation(commands); - RedisReply reply(client.get(), operation); + RedisCommand command; + command.format(commands); + RedisReply reply(client.get(), command); /* sonic-db-cli output format mimic the non-tty mode output format from redis-cli based on our usage in SONiC, None and list type output from python API needs to be modified diff --git a/sonic-db-cli/sonic-db-cli.h b/sonic-db-cli/sonic-db-cli.h index 1c3cfbdf9..7cbf5fe0d 100755 --- a/sonic-db-cli/sonic-db-cli.h +++ b/sonic-db-cli/sonic-db-cli.h @@ -20,8 +20,6 @@ void printUsage(); void printRedisReply(swss::RedisReply& reply); -std::string buildRedisOperation(std::vector& commands); - std::shared_ptr connectDbInterface( const std::string& db_name, const std::string& netns, From 5692aff3b89bb2fe16e0fc0b3014b5c70777f43f Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Fri, 13 May 2022 06:13:02 +0000 Subject: [PATCH 15/20] Add UT and fix code bug. --- common/rediscommand.h | 1 + sonic-db-cli/sonic-db-cli.cpp | 20 ++- tests/cli_ut.cpp | 224 ++++++++++++++++++++++++++++++++-- 3 files changed, 227 insertions(+), 18 deletions(-) diff --git a/common/rediscommand.h b/common/rediscommand.h index fb96f4584..5ea4c6122 100644 --- a/common/rediscommand.h +++ b/common/rediscommand.h @@ -25,6 +25,7 @@ class RedisCommand { void format(const char *fmt, ...); void formatArgv(int argc, const char **argv, const size_t *argvlen); + void format(const std::vector &commands); /* Format HMSET key multiple field value command */ #ifndef SWIG diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index 6a8594d08..a7d28e3a3 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -101,9 +101,10 @@ int handleAllInstances( bool operation_failed = false; for (auto& response : responses) { - if (response.get() != "") + auto respstr = response.get(); + if (respstr != "") { - cout << response.get() << endl; + cout << respstr << endl; operation_failed = true; } } @@ -268,15 +269,24 @@ int sonic_db_cli( auto netns = options.m_namespace; bool isTcpConn = !options.m_unixsocket; // Load the database config for the namespace - if (netns != "None") + if (netns != "None" && !netns.empty()) { - SonicDBConfig::initializeGlobalConfig(global_config_file); + // SonicDBConfig may initialized when run cli with UT + if (!SonicDBConfig::isGlobalInit()) + { + SonicDBConfig::initializeGlobalConfig(global_config_file); + } } else { - SonicDBConfig::initializeGlobalConfig(config_file); + // SonicDBConfig may initialized when run cli with UT + if (!SonicDBConfig::isInit()) + { + SonicDBConfig::initialize(config_file); + } // Use the tcp connectivity if namespace is local and unixsocket cmd_option is present. isTcpConn = true; + netns = ""; } if (options.m_cmd.size() != 0) diff --git a/tests/cli_ut.cpp b/tests/cli_ut.cpp index 631bcfae4..ae5c4a1a5 100755 --- a/tests/cli_ut.cpp +++ b/tests/cli_ut.cpp @@ -3,24 +3,34 @@ #include #include #include +#include #include +#include "common/dbconnector.h" #include "sonic-db-cli/sonic-db-cli.h" -#define TEST_DB "STATE_DB2" -#define TEST_NAMESPACE "asic0" -#define INVALID_NAMESPACE "invalid" - const std::string config_file = "./tests/redis_multi_db_ut_config/database_config.json"; const std::string global_config_file = "./tests/redis_multi_db_ut_config/database_global.json"; -std::string readFileContent(std::string file_name) { +std::string readFileContent(std::string file_name) +{ std::ifstream help_output_file(file_name); std::stringstream buffer; buffer << help_output_file.rdbuf(); return buffer.str(); } -TEST(sonic_db_cli, test_cli_help) { +std::string runCli(int argc, char** argv) +{ + optind = 0; + std::cout << "set capture" << std::endl; + testing::internal::CaptureStdout(); + EXPECT_EQ(0, sonic_db_cli(config_file, global_config_file, argc, argv)); + auto output = testing::internal::GetCapturedStdout(); + return output; +} + +TEST(sonic_db_cli, test_cli_help) +{ char *args[2]; args[0] = "sonic-db-cli"; args[1] = "-h"; @@ -33,7 +43,8 @@ TEST(sonic_db_cli, test_cli_help) { EXPECT_EQ(expected_output, output); } -TEST(sonic_db_cli, test_cli_ping_cmd) { +TEST(sonic_db_cli, test_cli_ping_cmd) +{ char *args[2]; args[0] = "sonic-db-cli"; args[1] = "PING"; @@ -42,11 +53,11 @@ TEST(sonic_db_cli, test_cli_ping_cmd) { testing::internal::CaptureStdout(); EXPECT_EQ(0, sonic_db_cli(config_file, global_config_file, 2, args)); auto output = testing::internal::GetCapturedStdout(); - std::cout << output <(db_name.c_str())); + } + db_names = swss::SonicDBConfig::getDbList("asic1"); + for (auto& db_name : db_names) + { + generateTestData("asic1", const_cast(db_name.c_str())); + } + + // save 2 DBs and get save DB performance + auto begin_time = clock(); + db_names = swss::SonicDBConfig::getDbList("asic0"); + args[2] = "asic0"; + for (auto& db_name : db_names) + { + args[3] = const_cast(db_name.c_str()); + optind = 0; + sonic_db_cli(config_file, global_config_file, 5, args); + } + + db_names = swss::SonicDBConfig::getDbList("asic1"); + args[2] = "asic1"; + for (auto& db_name : db_names) + { + args[3] = const_cast(db_name.c_str()); + optind = 0; + sonic_db_cli(config_file, global_config_file, 5, args); + } + + auto sequential_time = float( clock () - begin_time ); + + // prepare data + db_names = swss::SonicDBConfig::getDbList("asic0"); + for (auto& db_name : db_names) + { + generateTestData("asic0", const_cast(db_name.c_str())); + } + db_names = swss::SonicDBConfig::getDbList("asic1"); + for (auto& db_name : db_names) + { + generateTestData("asic0", const_cast(db_name.c_str())); + } + + // save 2 DBs in parallel, and get save DB performance + begin_time = clock(); + + args[1] = "SAVE"; + optind = 0; + sonic_db_cli(config_file, global_config_file, 2, args); + + auto parallen_time = float( clock () - begin_time ); + EXPECT_TRUE(parallen_time < sequential_time); } \ No newline at end of file From f3a21bd90675b4d032d15e2b9d5472c186f92301 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Fri, 13 May 2022 07:44:51 +0000 Subject: [PATCH 16/20] Fix output format issue --- common/redisreply.cpp | 7 ++++++- tests/cli_ut.cpp | 29 ++++++++--------------------- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/common/redisreply.cpp b/common/redisreply.cpp index 16badf624..9030ce830 100644 --- a/common/redisreply.cpp +++ b/common/redisreply.cpp @@ -249,7 +249,12 @@ string RedisReply::toString(redisReply *reply) stringstream result; for (size_t i = 0; i < reply->elements; i++) { - result << toString(reply->element[i]) << endl; + result << toString(reply->element[i]); + + if (i < reply->elements - 1) + { + result << endl; + } } return result.str(); } diff --git a/tests/cli_ut.cpp b/tests/cli_ut.cpp index ae5c4a1a5..63f2de713 100755 --- a/tests/cli_ut.cpp +++ b/tests/cli_ut.cpp @@ -22,7 +22,6 @@ std::string readFileContent(std::string file_name) std::string runCli(int argc, char** argv) { optind = 0; - std::cout << "set capture" << std::endl; testing::internal::CaptureStdout(); EXPECT_EQ(0, sonic_db_cli(config_file, global_config_file, argc, argv)); auto output = testing::internal::GetCapturedStdout(); @@ -35,10 +34,7 @@ TEST(sonic_db_cli, test_cli_help) args[0] = "sonic-db-cli"; args[1] = "-h"; - optind = 0; - testing::internal::CaptureStdout(); - EXPECT_EQ(0, sonic_db_cli(config_file, global_config_file, 2, args)); - auto output = testing::internal::GetCapturedStdout(); + auto output = runCli(2, args); auto expected_output = readFileContent("./tests/cli_test_data/cli_help_output.txt"); EXPECT_EQ(expected_output, output); } @@ -49,10 +45,7 @@ TEST(sonic_db_cli, test_cli_ping_cmd) args[0] = "sonic-db-cli"; args[1] = "PING"; - optind = 0; - testing::internal::CaptureStdout(); - EXPECT_EQ(0, sonic_db_cli(config_file, global_config_file, 2, args)); - auto output = testing::internal::GetCapturedStdout(); + auto output = runCli(2, args); EXPECT_EQ("PONG\n", output); } @@ -62,10 +55,7 @@ TEST(sonic_db_cli, test_cli_save_cmd) args[0] = "sonic-db-cli"; args[1] = "SAVE"; - optind = 0; - testing::internal::CaptureStdout(); - EXPECT_EQ(0, sonic_db_cli(config_file, global_config_file, 2, args)); - auto output = testing::internal::GetCapturedStdout(); + auto output = runCli(2, args); EXPECT_EQ("OK\n", output); } @@ -75,10 +65,7 @@ TEST(sonic_db_cli, test_cli_flush_cmd) args[0] = "sonic-db-cli"; args[1] = "FLUSHALL"; - optind = 0; - testing::internal::CaptureStdout(); - EXPECT_EQ(0, sonic_db_cli(config_file, global_config_file, 2, args)); - auto output = testing::internal::GetCapturedStdout(); + auto output = runCli(2, args); EXPECT_EQ("OK\n", output); } @@ -105,7 +92,7 @@ TEST(sonic_db_cli, test_cli_run_cmd) args[2] = "keys"; args[3] = "*"; output = runCli(4, args); - EXPECT_EQ("testkey\n\n", output); + EXPECT_EQ("testkey\n", output); } TEST(sonic_db_cli, test_cli_multi_ns_cmd) @@ -133,7 +120,7 @@ TEST(sonic_db_cli, test_cli_multi_ns_cmd) args[4] = "keys"; args[5] = "*"; output = runCli(6, args); - EXPECT_EQ("testkey\n\n", output); + EXPECT_EQ("testkey\n", output); } TEST(sonic_db_cli, test_cli_unix_socket_cmd) @@ -176,7 +163,7 @@ TEST(sonic_db_cli, test_cli_eval_cmd) args[9] = "v1"; args[10] = "v2"; auto output = runCli(11, args); - EXPECT_EQ("k1\nk2\nv1\nv2\n\n", output); + EXPECT_EQ("k1\nk2\nv1\nv2\n", output); } void flushDB(char* ns, char* database) @@ -201,7 +188,7 @@ void generateTestData(char* ns, char* database) args[3] = database; args[4] = "EVAL"; - args[5] = "local i=0 while (i<10000) do i=i+1 redis.call('SET', i, i) end"; + args[5] = "local i=0 while (i<100000) do i=i+1 redis.call('SET', i, i) end"; args[6] = "0"; optind = 0; sonic_db_cli(config_file, global_config_file, 7, args); From 7871bbedad0e6f907900e24204fd8ed6de5d9e70 Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 18 May 2022 07:14:35 +0000 Subject: [PATCH 17/20] Change install folder --- debian/sonic-db-cli.dirs | 2 +- debian/sonic-db-cli.install | 2 +- sonic-db-cli/Makefile.am | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/debian/sonic-db-cli.dirs b/debian/sonic-db-cli.dirs index e77248175..850aef0da 100644 --- a/debian/sonic-db-cli.dirs +++ b/debian/sonic-db-cli.dirs @@ -1 +1 @@ -usr/bin +usr/local/bin diff --git a/debian/sonic-db-cli.install b/debian/sonic-db-cli.install index 6e8e0ec68..698938b46 100644 --- a/debian/sonic-db-cli.install +++ b/debian/sonic-db-cli.install @@ -1 +1 @@ -usr/bin/sonic-db-cli +usr/local/bin/sonic-db-cli diff --git a/sonic-db-cli/Makefile.am b/sonic-db-cli/Makefile.am index a8b52653a..4caee3448 100755 --- a/sonic-db-cli/Makefile.am +++ b/sonic-db-cli/Makefile.am @@ -12,7 +12,8 @@ libsonicdbcli_la_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) libsonicdbcli_la_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON) libsonicdbcli_la_LDFLAGS = -L$(top_srcdir)/common -lswsscommon -lpthread -bin_PROGRAMS = sonic-db-cli +sonic_db_cli_instdir = $(prefix)/local/bin +sonic_db_cli_inst_PROGRAMS = sonic-db-cli sonic_db_cli_SOURCES = sonic-db-cli.cpp main.cpp sonic_db_cli_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) sonic_db_cli_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON) From 0d2101de5fc17f4e245bed3d79a332af6e7f374a Mon Sep 17 00:00:00 2001 From: azureuser Date: Tue, 24 May 2022 08:36:43 +0000 Subject: [PATCH 18/20] Fix coredump issue --- sonic-db-cli/sonic-db-cli.cpp | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index a7d28e3a3..31408f4e6 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -155,15 +155,23 @@ int executeCommands( return 1; } - RedisCommand command; - command.format(commands); - RedisReply reply(client.get(), command); - /* - sonic-db-cli output format mimic the non-tty mode output format from redis-cli - based on our usage in SONiC, None and list type output from python API needs to be modified - with these changes, it is enough for us to mimic redis-cli in SONiC so far since no application uses tty mode redis-cli output - */ - cout << reply.toString() << endl; + try + { + RedisCommand command; + command.format(commands); + RedisReply reply(client.get(), command); + /* + sonic-db-cli output format mimic the non-tty mode output format from redis-cli + based on our usage in SONiC, None and list type output from python API needs to be modified + with these changes, it is enough for us to mimic redis-cli in SONiC so far since no application uses tty mode redis-cli output + */ + cout << reply.toString() << endl; + } + catch (const std::system_error& e) + { + cerr << e.what() << endl; + return 1; + } return 0; } From f95fa138dcd8ca85094416c9de011ded007f8e4e Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Tue, 24 May 2022 09:46:38 +0000 Subject: [PATCH 19/20] Fix PR comments --- common/redisreply.cpp | 11 ++++---- common/redisreply.h | 5 ++-- debian/sonic-db-cli.dirs | 2 +- debian/sonic-db-cli.install | 2 +- sonic-db-cli/Makefile.am | 3 +- sonic-db-cli/main.cpp | 19 +++++++++++-- sonic-db-cli/sonic-db-cli.cpp | 21 +++++--------- sonic-db-cli/sonic-db-cli.h | 7 +++-- tests/cli_ut.cpp | 52 +++++++++++++++++++++++++++-------- 9 files changed, 78 insertions(+), 44 deletions(-) diff --git a/common/redisreply.cpp b/common/redisreply.cpp index 9030ce830..d5a48dc92 100644 --- a/common/redisreply.cpp +++ b/common/redisreply.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include @@ -158,7 +159,7 @@ void RedisReply::checkReplyType(int expectedType) const char *err = (m_reply->type == REDIS_REPLY_STRING || m_reply->type == REDIS_REPLY_ERROR) ? m_reply->str : "NON-STRING-REPLY"; - string errmsg = "Expected to get redis type " + to_string(expectedType) + " got type " + to_string(m_reply->type) + ", err: " + err; + string errmsg = "Expected to get redis type " + std::to_string(expectedType) + " got type " + std::to_string(m_reply->type) + ", err: " + err; SWSS_LOG_ERROR("%s", errmsg.c_str()); throw system_error(make_error_code(errc::io_error), errmsg); } @@ -226,12 +227,12 @@ template<> RedisMessage RedisReply::getReply() } -string RedisReply::toString() +string RedisReply::to_string() { - return toString(getContext()); + return to_string(getContext()); } -string RedisReply::toString(redisReply *reply) +string RedisReply::to_string(redisReply *reply) { switch(reply->type) { @@ -249,7 +250,7 @@ string RedisReply::toString(redisReply *reply) stringstream result; for (size_t i = 0; i < reply->elements; i++) { - result << toString(reply->element[i]); + result << to_string(reply->element[i]); if (i < reply->elements - 1) { diff --git a/common/redisreply.h b/common/redisreply.h index 3deb504e8..d807b51e9 100644 --- a/common/redisreply.h +++ b/common/redisreply.h @@ -2,7 +2,6 @@ #define __REDISREPLY__ #include -#include #include #include #include "rediscommand.h" @@ -93,12 +92,12 @@ class RedisReply /* Check that the status is QUEUED, throw exception otherwise */ void checkStatusQueued(); - std::string toString(); + std::string to_string(); private: void checkStatus(const char *status); void checkReply(); - std::string toString(redisReply *reply); + std::string to_string(redisReply *reply); redisReply *m_reply; }; diff --git a/debian/sonic-db-cli.dirs b/debian/sonic-db-cli.dirs index 850aef0da..e77248175 100644 --- a/debian/sonic-db-cli.dirs +++ b/debian/sonic-db-cli.dirs @@ -1 +1 @@ -usr/local/bin +usr/bin diff --git a/debian/sonic-db-cli.install b/debian/sonic-db-cli.install index 698938b46..6e8e0ec68 100644 --- a/debian/sonic-db-cli.install +++ b/debian/sonic-db-cli.install @@ -1 +1 @@ -usr/local/bin/sonic-db-cli +usr/bin/sonic-db-cli diff --git a/sonic-db-cli/Makefile.am b/sonic-db-cli/Makefile.am index 4caee3448..a8b52653a 100755 --- a/sonic-db-cli/Makefile.am +++ b/sonic-db-cli/Makefile.am @@ -12,8 +12,7 @@ libsonicdbcli_la_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) libsonicdbcli_la_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON) libsonicdbcli_la_LDFLAGS = -L$(top_srcdir)/common -lswsscommon -lpthread -sonic_db_cli_instdir = $(prefix)/local/bin -sonic_db_cli_inst_PROGRAMS = sonic-db-cli +bin_PROGRAMS = sonic-db-cli sonic_db_cli_SOURCES = sonic-db-cli.cpp main.cpp sonic_db_cli_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) sonic_db_cli_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON) diff --git a/sonic-db-cli/main.cpp b/sonic-db-cli/main.cpp index a827e6fbc..b0fde5ea5 100755 --- a/sonic-db-cli/main.cpp +++ b/sonic-db-cli/main.cpp @@ -1,11 +1,24 @@ #include "sonic-db-cli.h" #include "common/dbconnector.h" +using namespace swss; +using namespace std; + int main(int argc, char** argv) { + auto initializeGlobalConfig = []() + { + SonicDBConfig::initializeGlobalConfig(SonicDBConfig::DEFAULT_SONIC_DB_GLOBAL_CONFIG_FILE); + }; + + auto initializeConfig = []() + { + SonicDBConfig::initialize(SonicDBConfig::DEFAULT_SONIC_DB_CONFIG_FILE); + }; + return sonic_db_cli( - swss::SonicDBConfig::DEFAULT_SONIC_DB_CONFIG_FILE, - swss::SonicDBConfig::DEFAULT_SONIC_DB_GLOBAL_CONFIG_FILE, argc, - argv); + argv, + initializeGlobalConfig, + initializeConfig); } \ No newline at end of file diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index 31408f4e6..044ddcde4 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -165,7 +165,7 @@ int executeCommands( based on our usage in SONiC, None and list type output from python API needs to be modified with these changes, it is enough for us to mimic redis-cli in SONiC so far since no application uses tty mode redis-cli output */ - cout << reply.toString() << endl; + cout << reply.to_string() << endl; } catch (const std::system_error& e) { @@ -241,10 +241,10 @@ void parseCliArguments( } int sonic_db_cli( - const string &config_file, - const string &global_config_file, int argc, - char** argv) + char** argv, + function initializeGlobalConfig, + function initializeConfig) { Options options; try @@ -260,7 +260,7 @@ int sonic_db_cli( catch (logic_error const& e) { // getopt_long throw logic_error when found a unknown option without value. - cerr << "Unknown option without value: " << endl; + cerr << "Unknown option without value: " << e.what() << endl; printUsage(); return -1; } @@ -279,19 +279,12 @@ int sonic_db_cli( // Load the database config for the namespace if (netns != "None" && !netns.empty()) { - // SonicDBConfig may initialized when run cli with UT - if (!SonicDBConfig::isGlobalInit()) - { - SonicDBConfig::initializeGlobalConfig(global_config_file); - } + initializeGlobalConfig(); } else { // SonicDBConfig may initialized when run cli with UT - if (!SonicDBConfig::isInit()) - { - SonicDBConfig::initialize(config_file); - } + initializeConfig(); // Use the tcp connectivity if namespace is local and unixsocket cmd_option is present. isTcpConn = true; netns = ""; diff --git a/sonic-db-cli/sonic-db-cli.h b/sonic-db-cli/sonic-db-cli.h index 7cbf5fe0d..be4fcd3a0 100755 --- a/sonic-db-cli/sonic-db-cli.h +++ b/sonic-db-cli/sonic-db-cli.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -48,7 +49,7 @@ void parseCliArguments( Options &options); int sonic_db_cli( - const std::string &config_file, - const std::string &global_config_file, int argc, - char** argv); \ No newline at end of file + char** argv, + std::function initializeGlobalConfig, + std::function initializeConfig); \ No newline at end of file diff --git a/tests/cli_ut.cpp b/tests/cli_ut.cpp index 63f2de713..a3516963b 100755 --- a/tests/cli_ut.cpp +++ b/tests/cli_ut.cpp @@ -8,22 +8,50 @@ #include "common/dbconnector.h" #include "sonic-db-cli/sonic-db-cli.h" -const std::string config_file = "./tests/redis_multi_db_ut_config/database_config.json"; -const std::string global_config_file = "./tests/redis_multi_db_ut_config/database_global.json"; +using namespace swss; +using namespace std; -std::string readFileContent(std::string file_name) +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"; + +int sonic_db_cli(int argc, char** argv) +{ + auto initializeGlobalConfig = []() + { + if (!SonicDBConfig::isGlobalInit()) + { + SonicDBConfig::initializeGlobalConfig(global_config_file); + } + }; + + auto initializeConfig = []() + { + if (!SonicDBConfig::isInit()) + { + SonicDBConfig::initialize(config_file); + } + }; + + return sonic_db_cli( + argc, + argv, + initializeGlobalConfig, + initializeConfig); +} + +string readFileContent(string file_name) { - std::ifstream help_output_file(file_name); - std::stringstream buffer; + ifstream help_output_file(file_name); + stringstream buffer; buffer << help_output_file.rdbuf(); return buffer.str(); } -std::string runCli(int argc, char** argv) +string runCli(int argc, char** argv) { optind = 0; testing::internal::CaptureStdout(); - EXPECT_EQ(0, sonic_db_cli(config_file, global_config_file, argc, argv)); + EXPECT_EQ(0, sonic_db_cli(argc, argv)); auto output = testing::internal::GetCapturedStdout(); return output; } @@ -175,7 +203,7 @@ void flushDB(char* ns, char* database) args[3] = database; args[4] = "FLUSHDB"; optind = 0; - sonic_db_cli(config_file, global_config_file, 5, args); + sonic_db_cli(5, args); } void generateTestData(char* ns, char* database) @@ -191,7 +219,7 @@ void generateTestData(char* ns, char* database) args[5] = "local i=0 while (i<100000) do i=i+1 redis.call('SET', i, i) end"; args[6] = "0"; optind = 0; - sonic_db_cli(config_file, global_config_file, 7, args); + sonic_db_cli(7, args); } TEST(sonic_db_cli, test_parallel_cmd) { @@ -220,7 +248,7 @@ TEST(sonic_db_cli, test_parallel_cmd) { { args[3] = const_cast(db_name.c_str()); optind = 0; - sonic_db_cli(config_file, global_config_file, 5, args); + sonic_db_cli(5, args); } db_names = swss::SonicDBConfig::getDbList("asic1"); @@ -229,7 +257,7 @@ TEST(sonic_db_cli, test_parallel_cmd) { { args[3] = const_cast(db_name.c_str()); optind = 0; - sonic_db_cli(config_file, global_config_file, 5, args); + sonic_db_cli(5, args); } auto sequential_time = float( clock () - begin_time ); @@ -251,7 +279,7 @@ TEST(sonic_db_cli, test_parallel_cmd) { args[1] = "SAVE"; optind = 0; - sonic_db_cli(config_file, global_config_file, 2, args); + sonic_db_cli(2, args); auto parallen_time = float( clock () - begin_time ); EXPECT_TRUE(parallen_time < sequential_time); From 0b1d293a82f67316b183aa4aa112b088342c795b Mon Sep 17 00:00:00 2001 From: liuh-80 Date: Wed, 25 May 2022 01:31:05 +0000 Subject: [PATCH 20/20] Fix PR comments --- common/redisreply.cpp | 2 +- common/redisreply.h | 3 ++- sonic-db-cli/sonic-db-cli.cpp | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/common/redisreply.cpp b/common/redisreply.cpp index d5a48dc92..ca4ba1315 100644 --- a/common/redisreply.cpp +++ b/common/redisreply.cpp @@ -261,7 +261,7 @@ string RedisReply::to_string(redisReply *reply) } default: - SWSS_LOG_ERROR("invalid type %d for message", getContext()->type); + SWSS_LOG_ERROR("invalid type %d for message", reply->type); return string(); } } diff --git a/common/redisreply.h b/common/redisreply.h index d807b51e9..a8f3458de 100644 --- a/common/redisreply.h +++ b/common/redisreply.h @@ -94,10 +94,11 @@ class RedisReply std::string to_string(); + static std::string to_string(redisReply *reply); + private: void checkStatus(const char *status); void checkReply(); - std::string to_string(redisReply *reply); redisReply *m_reply; }; diff --git a/sonic-db-cli/sonic-db-cli.cpp b/sonic-db-cli/sonic-db-cli.cpp index 044ddcde4..a69502600 100755 --- a/sonic-db-cli/sonic-db-cli.cpp +++ b/sonic-db-cli/sonic-db-cli.cpp @@ -277,7 +277,7 @@ int sonic_db_cli( auto netns = options.m_namespace; bool isTcpConn = !options.m_unixsocket; // Load the database config for the namespace - if (netns != "None" && !netns.empty()) + if (!netns.empty()) { initializeGlobalConfig(); }