From 58c3dd2b3a8dc2d86998286bb61dd2dd24b7b18f Mon Sep 17 00:00:00 2001 From: Seshagirirao Lekkala <54128277+selekkala@users.noreply.github.com> Date: Mon, 21 Sep 2026 14:19:46 -0700 Subject: [PATCH 1/2] [fboss2] Support BGP version in OSS builds --- cmake/CliFboss2Test.cmake | 1 + .../commands/show/bgp/CmdShowVersionBgp.cpp | 18 +++++-- .../cli/fboss2/test/CmdShowBgpVersionTest.cpp | 50 +++++++++++++++++++ fboss/cli/fboss2/test/bgp/MockBgpClient.h | 1 + neteng/fboss/bgp/if/bgp_thrift.thrift | 5 ++ 5 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 fboss/cli/fboss2/test/CmdShowBgpVersionTest.cpp diff --git a/cmake/CliFboss2Test.cmake b/cmake/CliFboss2Test.cmake index 1454beec1f4e4..553a444b52a39 100644 --- a/cmake/CliFboss2Test.cmake +++ b/cmake/CliFboss2Test.cmake @@ -126,6 +126,7 @@ add_executable(fboss2_cmd_test fboss/cli/fboss2/test/CmdShowBgpStreamSummaryTest.cpp fboss/cli/fboss2/test/CmdShowBgpSummaryEgressTest.cpp fboss/cli/fboss2/test/CmdShowBgpSummaryTest.cpp + fboss/cli/fboss2/test/CmdShowBgpVersionTest.cpp fboss/cli/fboss2/test/CmdShowBgpTableCommunityTest.cpp fboss/cli/fboss2/test/CmdShowBgpTableDetailTest.cpp fboss/cli/fboss2/test/CmdShowBgpTableMoreSpecificsTest.cpp diff --git a/fboss/cli/fboss2/commands/show/bgp/CmdShowVersionBgp.cpp b/fboss/cli/fboss2/commands/show/bgp/CmdShowVersionBgp.cpp index 58aff1eb2f304..0388c85242b2a 100644 --- a/fboss/cli/fboss2/commands/show/bgp/CmdShowVersionBgp.cpp +++ b/fboss/cli/fboss2/commands/show/bgp/CmdShowVersionBgp.cpp @@ -14,6 +14,7 @@ #include "fboss/cli/fboss2/utils/CmdClientUtilsCommon.h" // NOLINT(misc-include-cleaner) #include "fboss/cli/fboss2/utils/HostInfo.h" #include "neteng/fboss/bgp/if/gen-cpp2/TBgpService.h" // NOLINT(misc-include-cleaner) +#include "thrift/lib/cpp/TApplicationException.h" namespace facebook::fboss { @@ -27,11 +28,18 @@ CmdShowVersionBgp::RetType CmdShowVersionBgp::queryClient( client->sync_getRegexExportedValues(retVal, "build_.*"); #else - // OSS: getRegexExportedValues method not available in TBgpService - // This command requires Meta-internal fb303 extensions - throw std::runtime_error( - "BGP version command not supported in OSS build - " - "getRegexExportedValues method not available"); + auto client = utils::createClient>(hostInfo); + try { + client->sync_getBuildInfo(retVal); + } catch (const apache::thrift::TApplicationException& ex) { + if (ex.getType() != apache::thrift::TApplicationException::UNKNOWN_METHOD) { + throw; + } + throw std::runtime_error( + "The running BGP daemon does not support getBuildInfo; upgrade the " + "BGP package and restart the service before using this command"); + } #endif return retVal; diff --git a/fboss/cli/fboss2/test/CmdShowBgpVersionTest.cpp b/fboss/cli/fboss2/test/CmdShowBgpVersionTest.cpp new file mode 100644 index 0000000000000..f21c33f62308c --- /dev/null +++ b/fboss/cli/fboss2/test/CmdShowBgpVersionTest.cpp @@ -0,0 +1,50 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + */ + +#include +#include + +#include "fboss/cli/fboss2/commands/show/bgp/CmdShowVersionBgp.h" +#include "fboss/cli/fboss2/test/CmdHandlerTestBase.h" +#include "thrift/lib/cpp/TApplicationException.h" + +using namespace ::testing; + +namespace facebook::fboss { + +class CmdShowBgpVersionTestFixture : public CmdHandlerTestBase {}; + +TEST_F(CmdShowBgpVersionTestFixture, QueryClientReturnsRunningBuildInfo) { + const CmdShowVersionBgp::RetType expected = { + {"build_package_name", "bgp"}, + {"build_package_version", "0530bb98"}, + {"build_revision", "0530bb98"}, + }; + setupMockedBgpServer(); + EXPECT_CALL(getMockBgp(), getBuildInfo(_)).WillOnce(Invoke([&](auto& info) { + info = expected; + })); + + EXPECT_EQ(CmdShowVersionBgp().queryClient(localhost()), expected); +} + +TEST_F(CmdShowBgpVersionTestFixture, QueryClientRejectsOldDaemon) { + setupMockedBgpServer(); + EXPECT_CALL(getMockBgp(), getBuildInfo(_)) + .WillOnce(Throw( + apache::thrift::TApplicationException( + apache::thrift::TApplicationException::UNKNOWN_METHOD, + "Unknown function getBuildInfo"))); + + EXPECT_THROW( + CmdShowVersionBgp().queryClient(localhost()), std::runtime_error); +} + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/test/bgp/MockBgpClient.h b/fboss/cli/fboss2/test/bgp/MockBgpClient.h index eb9928efe52f5..3ab94a01180d0 100644 --- a/fboss/cli/fboss2/test/bgp/MockBgpClient.h +++ b/fboss/cli/fboss2/test/bgp/MockBgpClient.h @@ -30,6 +30,7 @@ using facebook::neteng::routing::policy::thrift::TPolicyStats; class MockBgpClient : public apache::thrift::ServiceHandler { public: + MOCK_METHOD(void, getBuildInfo, ((std::map&))); MOCK_METHOD(void, getBgpSessions, (std::vector&)); MOCK_METHOD(void, getBgpLocalConfig, (TBgpLocalConfig&)); MOCK_METHOD(void, getDrainState, (TBgpDrainState&)); diff --git a/neteng/fboss/bgp/if/bgp_thrift.thrift b/neteng/fboss/bgp/if/bgp_thrift.thrift index 1340a5a9a3683..ff3fa50e1562e 100644 --- a/neteng/fboss/bgp/if/bgp_thrift.thrift +++ b/neteng/fboss/bgp/if/bgp_thrift.thrift @@ -1384,6 +1384,11 @@ struct THealthReport { // @lint-ignore THRIFTCHECKS facebook-service-deprecated existing service inheritance is out of scope for this API addition service TBgpService extends fb303.FacebookService { + /** + * Return build metadata for the running BGP process. + */ + map getBuildInfo(); + /** * [Logging] * From 8661e8a6158c41403d72d3d24945f241d98a0c24 Mon Sep 17 00:00:00 2001 From: Seshagirirao Lekkala <54128277+selekkala@users.noreply.github.com> Date: Mon, 28 Sep 2026 17:58:23 -0700 Subject: [PATCH 2/2] [fboss2] Fix old BGP daemon compatibility test --- fboss/cli/fboss2/test/CmdShowBgpVersionTest.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/fboss/cli/fboss2/test/CmdShowBgpVersionTest.cpp b/fboss/cli/fboss2/test/CmdShowBgpVersionTest.cpp index f21c33f62308c..c283691e7e329 100644 --- a/fboss/cli/fboss2/test/CmdShowBgpVersionTest.cpp +++ b/fboss/cli/fboss2/test/CmdShowBgpVersionTest.cpp @@ -13,7 +13,6 @@ #include "fboss/cli/fboss2/commands/show/bgp/CmdShowVersionBgp.h" #include "fboss/cli/fboss2/test/CmdHandlerTestBase.h" -#include "thrift/lib/cpp/TApplicationException.h" using namespace ::testing; @@ -36,12 +35,14 @@ TEST_F(CmdShowBgpVersionTestFixture, QueryClientReturnsRunningBuildInfo) { } TEST_F(CmdShowBgpVersionTestFixture, QueryClientRejectsOldDaemon) { - setupMockedBgpServer(); - EXPECT_CALL(getMockBgp(), getBuildInfo(_)) - .WillOnce(Throw( - apache::thrift::TApplicationException( - apache::thrift::TApplicationException::UNKNOWN_METHOD, - "Unknown function getBuildInfo"))); + // Use a generated processor without getBuildInfo so Thrift emits the same + // UNKNOWN_METHOD response as a pre-API BGP daemon. + auto legacyBgpService = std::make_shared(); + auto legacyBgpServer = + std::make_unique( + legacyBgpService, "::1", 0, createFastMockServerConfig()); + CmdGlobalOptions::getInstance()->setBgpThriftPort( + legacyBgpServer->getAddress().getPort()); EXPECT_THROW( CmdShowVersionBgp().queryClient(localhost()), std::runtime_error);