diff --git a/cmake/CliFboss2TestIntegrationTest.cmake b/cmake/CliFboss2TestIntegrationTest.cmake index 7bfff504c46a8..f7a28e44e0792 100644 --- a/cmake/CliFboss2TestIntegrationTest.cmake +++ b/cmake/CliFboss2TestIntegrationTest.cmake @@ -64,6 +64,8 @@ add_executable(fboss2_integration_test fboss/cli/fboss2/utils/oss/CmdInitUtils.cpp fboss/cli/fboss2/test/integration_test/ConfigIpRouteTest.cpp fboss/cli/fboss2/test/integration_test/ConfigCoppTrafficPolicyTest.cpp + fboss/cli/fboss2/test/integration_test/ShowConfigRunningAgentTest.cpp + fboss/cli/fboss2/test/integration_test/ShowConfigRunningBgpTest.cpp fboss/cli/fboss2/test/integration_test/ShowRouteFilterTest.cpp ) diff --git a/docs/docs/debugging/fboss_cli.md b/docs/docs/debugging/fboss_cli.md index be93335eb6cce..bc61a74e1406e 100644 --- a/docs/docs/debugging/fboss_cli.md +++ b/docs/docs/debugging/fboss_cli.md @@ -1284,7 +1284,7 @@ fboss2-dev config session commit | Command | Description | |---------|-------------| -| `show config running` | Show the current in-memory configuration of the agent | +| `show config running agent`, `show config running bgp` | Show the current in-memory configuration of the agent or BGP daemon | | `config session diff` | Show pending changes (session vs live, session vs revision, or revision vs revision) | | `config session commit` | Apply staged changes to all affected services | | `config session rebase` | Reapply uncommitted changes on top of current startup config | diff --git a/fboss/cli/fboss2/BUCK b/fboss/cli/fboss2/BUCK index f0bd6249cb184..09e3f2001c7d3 100644 --- a/fboss/cli/fboss2/BUCK +++ b/fboss/cli/fboss2/BUCK @@ -154,6 +154,7 @@ cpp_library( "//folly/coro:blocking_wait", "//folly/coro:task", "//folly/executors:cpu_thread_pool_executor", + "//folly/json:dynamic", "//folly/logging:logging", "//thrift/lib/cpp/util:enum_utils", "//thrift/lib/cpp2/op:get", diff --git a/fboss/cli/fboss2/CmdHandler.cpp b/fboss/cli/fboss2/CmdHandler.cpp index e3112d8d081cb..0e894b8f52f6a 100644 --- a/fboss/cli/fboss2/CmdHandler.cpp +++ b/fboss/cli/fboss2/CmdHandler.cpp @@ -25,6 +25,8 @@ #include #include #include +#include +#include #include #include #include @@ -94,9 +96,20 @@ void printJson( } } - out << apache::thrift::SimpleJSONSerializer::serialize( - hostResults) - << std::endl; + if constexpr (std::is_same_v) { + // The thrift serializer cannot handle folly::dynamic (it throws + // "expected dynamic type 'array', but had type 'object'"), so render the + // per-host map with folly's own JSON writer. + folly::dynamic hostObj = folly::dynamic::object; + for (auto& [host, data] : hostResults) { + hostObj[host] = std::move(data); + } + out << folly::toJson(hostObj) << std::endl; + } else { + out << apache::thrift::SimpleJSONSerializer::serialize( + hostResults) + << std::endl; + } } template diff --git a/fboss/cli/fboss2/commands/show/bgp/config/CmdShowConfigTraits.h b/fboss/cli/fboss2/commands/show/bgp/config/CmdShowConfigTraits.h index 7eeae9554d3d7..217fddc890595 100644 --- a/fboss/cli/fboss2/commands/show/bgp/config/CmdShowConfigTraits.h +++ b/fboss/cli/fboss2/commands/show/bgp/config/CmdShowConfigTraits.h @@ -48,9 +48,9 @@ struct CmdShowConfigRunningBgpTraits : public ReadCommandTraits { "The command is reached as 'show bgp config' in the internal build, " "where 'show bgp config raw' gives the config-file rendering instead " "and 'show config running bgp' is an equivalent alias; in the " - "open-source build it is spelled 'show bgp config running' and " - "neither the raw variant nor the 'show config running bgp' alias is " - "registered. " + "open-source build it is spelled 'show config running bgp', with " + "'show bgp config running' kept as an equivalent alias, and the raw " + "variant is not registered. " // The two builds also differ in the SHAPE of what they return. "The two builds also differ in what they render: the internal build " "returns the structured BgpConfig document illustrated below, while " diff --git a/fboss/cli/fboss2/oss/CmdList.cpp b/fboss/cli/fboss2/oss/CmdList.cpp index c24df0274fec4..2ccf49e9f01d4 100644 --- a/fboss/cli/fboss2/oss/CmdList.cpp +++ b/fboss/cli/fboss2/oss/CmdList.cpp @@ -264,6 +264,10 @@ const CommandTree& kBaseAdditionalCommandTree() { "Show Agent running config", commandHandler, argRegistrar}, + {"bgp", + "Show BGP running config", + commandHandler, + argTypeHandler}, }}, {"history", "Show history of changes to the current config", diff --git a/fboss/cli/fboss2/test/integration_test/BUCK b/fboss/cli/fboss2/test/integration_test/BUCK index 358dedfb6fc65..d3fe6e6a74b47 100644 --- a/fboss/cli/fboss2/test/integration_test/BUCK +++ b/fboss/cli/fboss2/test/integration_test/BUCK @@ -71,6 +71,8 @@ cpp_binary( "DeleteQosPolicyTest.cpp", "DeleteVlanTest.cpp", "Fboss2IntegrationTest.cpp", + "ShowConfigRunningAgentTest.cpp", + "ShowConfigRunningBgpTest.cpp", "ShowRouteFilterTest.cpp", "TunnelIpInIpTest.cpp", "//fboss/cli/fboss2:cmd-list-config-impl", diff --git a/fboss/cli/fboss2/test/integration_test/ShowConfigRunningAgentTest.cpp b/fboss/cli/fboss2/test/integration_test/ShowConfigRunningAgentTest.cpp new file mode 100644 index 0000000000000..d4f7b103af719 --- /dev/null +++ b/fboss/cli/fboss2/test/integration_test/ShowConfigRunningAgentTest.cpp @@ -0,0 +1,23 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +#include +#include + +#include "fboss/cli/fboss2/test/integration_test/Fboss2IntegrationTest.h" + +using namespace facebook::fboss; + +class ShowConfigRunningAgentTest : public Fboss2IntegrationTest {}; + +TEST_F(ShowConfigRunningAgentTest, JsonMatchesDaemonRunningConfig) { + auto result = runCli({"show", "config", "running", "agent"}); + ASSERT_EQ(result.exitCode, 0) << result.stderr; + ASSERT_TRUE(result.stderr.empty()) << result.stderr; + auto hosts = folly::parseJson(result.stdout); + ASSERT_TRUE(hosts.isObject()); + ASSERT_EQ(hosts.size(), 1); + const auto& config = hosts.items().begin()->second; + ASSERT_TRUE(config.isObject()); + EXPECT_EQ(config, getRunningConfig()) + << "'show config running agent' must match agent getRunningConfig"; +} diff --git a/fboss/cli/fboss2/test/integration_test/ShowConfigRunningBgpTest.cpp b/fboss/cli/fboss2/test/integration_test/ShowConfigRunningBgpTest.cpp new file mode 100644 index 0000000000000..7add41f23646c --- /dev/null +++ b/fboss/cli/fboss2/test/integration_test/ShowConfigRunningBgpTest.cpp @@ -0,0 +1,58 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * End-to-end test for 'show config running bgp'. + * + * 'show config running ' is the canonical spelling for running-config + * views, with 'show bgp config running' kept as an alias. Both must reach the + * same handler and return the config bgpd itself reports over its + * getRunningConfig thrift RPC. + * + * Requirements: + * - bgpd must be running and answering its thrift RPC; otherwise the tests + * are skipped (ConfigBgpTestBase::SetUp). + */ + +#include +#include +#include +#include +#include + +#include "fboss/cli/fboss2/test/integration_test/ConfigBgpTestBase.h" + +using namespace facebook::fboss; + +class ShowConfigRunningBgpTest : public ConfigBgpTestBase { + protected: + // --fmt json output is keyed by host; these tests only query localhost. + folly::dynamic showRunningBgp(const std::vector& args) const { + auto result = runCli(args); + EXPECT_EQ(result.exitCode, 0) << result.stderr; + EXPECT_TRUE(result.stderr.empty()) << result.stderr; + if (result.exitCode != 0 || !result.stderr.empty()) { + return nullptr; + } + auto json = folly::parseJson(result.stdout); + EXPECT_EQ(json.size(), 1) << "expected exactly one host entry"; + if (json.size() != 1) { + return nullptr; + } + return json.items().begin()->second; + } +}; + +TEST_F(ShowConfigRunningBgpTest, MatchesDaemonRunningConfig) { + auto cliConfig = showRunningBgp({"show", "config", "running", "bgp"}); + ASSERT_TRUE(cliConfig.isObject()); + EXPECT_EQ(cliConfig, readRunningBgpConfigViaRpc()) + << "'show config running bgp' must match bgpd getRunningConfig"; +} + +TEST_F(ShowConfigRunningBgpTest, MatchesShowBgpConfigRunningAlias) { + auto canonical = showRunningBgp({"show", "config", "running", "bgp"}); + auto alias = showRunningBgp({"show", "bgp", "config", "running"}); + ASSERT_TRUE(canonical.isObject()); + EXPECT_EQ(canonical, alias) + << "'show config running bgp' must match 'show bgp config running'"; +}