[Nexthop] [fboss2-dev] Add show config running bgp

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
 )
 

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 |

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",

fboss/cli/fboss2/CmdHandler.cpp

@@ -25,6 +25,8 @@
 #include <folly/ScopeGuard.h>
 #include <folly/Traits.h>
 #include <folly/executors/CPUThreadPoolExecutor.h>
+#include <folly/json/dynamic.h>
+#include <folly/json/json.h>
 #include <folly/logging/xlog.h>
 #include <folly/stop_watch.h>
 #include <cstddef>
@@ -94,9 +96,20 @@ void printJson(
     }
   }
 
-  out << apache::thrift::SimpleJSONSerializer::serialize<std::string>(
-             hostResults)
-      << std::endl;
+  if constexpr (std::is_same_v<typename CmdTypeT::RetType, folly::dynamic>) {
+    // 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<std::string>(
+               hostResults)
+        << std::endl;
+  }
 }
 
 template <typename CmdTypeT>

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 "

fboss/cli/fboss2/oss/CmdList.cpp

@@ -264,6 +264,10 @@ const CommandTree& kBaseAdditionalCommandTree() {
               "Show Agent running config",
               commandHandler<CmdShowConfigRunningAgent>,
               argRegistrar<CmdShowConfigDynamicTraits>},
+             {"bgp",
+              "Show BGP running config",
+              commandHandler<CmdShowConfigRunningBgp>,
+              argTypeHandler<CmdShowConfigRunningBgpTraits>},
          }},
         {"history",
          "Show history of changes to the current config",

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",

fboss/cli/fboss2/test/integration_test/ShowConfigRunningAgentTest.cpp

@@ -0,0 +1,23 @@
+// Copyright (c) Meta Platforms, Inc. and affiliates.
+
+#include <folly/json/json.h>
+#include <gtest/gtest.h>
+
+#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";
+}

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 <daemon>' 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 <folly/json/dynamic.h>
+#include <folly/json/json.h>
+#include <gtest/gtest.h>
+#include <string>
+#include <vector>
+
+#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<std::string>& 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'";
+}