BGP-aware fboss2 config session (ConfigSession infra)

Make the fboss2 ConfigSession and `config session` lifecycle BGP-aware. This is
the base for the `config protocol bgp` commands, which stack on top.

- ConfigSession owns the BGP config as a typed bgp::thrift::BgpConfig, exactly
  the way it owns cfg::AgentConfig for the agent. getBgpConfig()/saveBgpConfig()
  expose the WHOLE typed config and stage it to ~/.fboss2/bgp_config.json.
  ConfigSession is BGP-aware but agnostic about the config's internal structure:
  it has no notion of "global" vs "peer" vs "peer-group" -- a command mutates
  whichever typed fields it needs, mirroring how interface commands mutate
  getAgentConfig().sw()->ports(). Because the whole config round-trips through
  the typed struct, no surgical merge / preserve-list is needed.
- commit() promotes bgp_config.json to /etc/coop/bgpcpp/bgpcpp.conf, restarts
  bgp_pp, and versions it in the same /etc/coop git repo as agent.conf.
- config session clear/diff/rebase/rollback are BGP-aware: clear removes a
  staged BGP session, diff shows staged BGP changes, rebase 3-way-merges
  bgpcpp.conf, rollback restores bgpcpp.conf + restarts bgp_pp (using metadata
  history so BGP-only commits are reachable).
- FbossServiceUtil + cli_metadata gain the bgp_pp (BGP_PP) service/action; the
  config lib gains the bgp_config cpp2 dependency.
- Unit tests in CmdConfigSession{,Diff}Test cover the typed round-trip, commit,
  rebase, and rollback behavior.

Test Plan:
- bazel test //fboss/cli/fboss2/test/config:cmd_config_test //fboss/cli/fboss2/test:cmd_test
- fboss2_integration_test on a DUT (ConfigConcurrentSessionsTest.ConflictAndRebase,
  ConfigSessionClearTest, ConfigInterfaceMtuTest, ConfigInterfaceDescriptionTest): 5/5 pass

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: hillol-nexthop

cmake/CliFboss2.cmake

@@ -1050,6 +1050,7 @@ target_link_libraries(fboss2_config_lib
   agent_dir_util
   common_file_utils
   switch_config_cpp2
+  bgp_config_cpp2
   switchinfo_utils
   platform_mapping
   Folly::folly

fboss/cli/fboss2/BUCK

@@ -1292,6 +1292,7 @@ cpp_library(
         ":fboss2-lib",
         ":table-utils",
         "//common/network/if:if-cpp2-types",
+        "//configerator/structs/neteng/fboss/bgp:bgp_config-cpp2-types",
         "//fboss/agent:agent_config-cpp2-types",
         "//fboss/agent:agent_dir_util",
         "//fboss/agent:fboss-error",

fboss/cli/fboss2/cli_metadata.thrift

@@ -19,11 +19,13 @@ enum ConfigActionLevel {
   HITLESS = 0, // Can be applied with reloadConfig() - default
   AGENT_WARMBOOT = 1, // Requires agent warmboot restart
   AGENT_COLDBOOT = 2, // Requires agent coldboot restart (clears ASIC state)
+  BGP_PP_RESTART = 3, // Requires a restart of the bgp_pp (BGP++) service
 }
 
 // Identifier for different services that can be configured
 enum ServiceType {
   AGENT = 1,
+  BGP_PP = 2, // The bgp_pp (BGP++) routing daemon
 }
 
 // Metadata stored alongside the session configuration file.

fboss/cli/fboss2/commands/config/session/CmdConfigSessionClear.cpp

@@ -28,10 +28,12 @@ CmdConfigSessionClearTraits::RetType CmdConfigSessionClear::queryClient(
   // getInstance(), which would create a session if one doesn't exist
   std::string sessionConfigPath = ConfigSession::getSessionConfigPathStatic();
   std::string metadataPath = ConfigSession::getSessionMetadataPathStatic();
+  std::string bgpConfigPath = ConfigSession::getBgpSessionConfigPathStatic();
 
   std::error_code ec;
   bool removedConfig = false;
   bool removedMetadata = false;
+  bool removedBgpConfig = false;
 
   // Remove session config file (~/.fboss2/agent.conf)
   if (fs::exists(sessionConfigPath)) {
@@ -60,7 +62,23 @@ CmdConfigSessionClearTraits::RetType CmdConfigSessionClear::queryClient(
     removedMetadata = true;
   }
 
-  if (removedConfig || removedMetadata) {
+  // Remove staged BGP config file (~/.fboss2/bgp_config.json). BGP global edits
+  // are staged here (alongside any peer edits from BgpConfigSession), so a
+  // BGP-only session must be cleared too.
+  if (fs::exists(bgpConfigPath)) {
+    ec.clear();
+    fs::remove(bgpConfigPath, ec);
+    if (ec) {
+      throw std::runtime_error(
+          fmt::format(
+              "Failed to remove BGP session config file {}: {}",
+              bgpConfigPath,
+              ec.message()));
+    }
+    removedBgpConfig = true;
+  }
+
+  if (removedConfig || removedMetadata || removedBgpConfig) {
     return "Config session cleared successfully.";
   }
   return "No config session exists. Nothing to clear.";

fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.cpp

@@ -23,7 +23,7 @@ CmdConfigSessionCommitTraits::RetType CmdConfigSessionCommit::queryClient(
     const HostInfo& hostInfo) {
   auto& session = ConfigSession::getInstance();
 
-  if (!session.sessionExists()) {
+  if (!session.hasActiveSession()) {
     return "No config session exists. Make a config change first.";
   }
 
@@ -59,6 +59,11 @@ CmdConfigSessionCommitTraits::RetType CmdConfigSessionCommit::queryClient(
               fmt::format("{} (warmboot)", serviceName));
         }
         break;
+      case cli::ConfigActionLevel::BGP_PP_RESTART:
+        for (const auto& serviceName : serviceNamesList) {
+          restartedServices.push_back(fmt::format("{} (restart)", serviceName));
+        }
+        break;
       case cli::ConfigActionLevel::HITLESS:
         for (const auto& serviceName : serviceNamesList) {
           reloadedServices.push_back(serviceName);

fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.cpp

@@ -17,11 +17,13 @@
 #include "fboss/cli/fboss2/utils/CmdUtils.h"
 #include "fboss/cli/fboss2/utils/HostInfo.h"
 
+#include <fmt/format.h>
 #include <folly/FileUtil.h>
 #include <folly/Subprocess.h>
 #include <unistd.h>
 #include <cstdlib>
 #include <exception>
+#include <functional>
 #include <iostream>
 #include <stdexcept>
 #include <string>
@@ -32,27 +34,68 @@ namespace facebook::fboss {
 
 namespace {
 
-// Helper function to get config content from a revision specifier
-// Returns the content and a label for the revision
+// Git-relative paths of the two config files tracked in the /etc/coop repo.
+constexpr auto kAgentGitRelPath = "cli/agent.conf";
+constexpr auto kBgpGitRelPath = "bgpcpp/bgpcpp.conf";
+
+// A diffable config domain. The agent config and the BGP config are tracked in
+// the same /etc/coop git repo but live in different files; `config session
+// diff` shows whichever domain(s) are staged/relevant.
+struct DiffDomain {
+  std::string name; // "Agent" / "BGP" (section header when >1 domain shown)
+  std::string gitRelPath; // path in the git repo (e.g. cli/agent.conf)
+  std::string systemPath; // current live file
+  std::string sessionPath; // staged session file (~/.fboss2/...)
+  bool staged; // a session edit is staged for this domain
+};
+
+std::vector<DiffDomain> allDomains(ConfigSession& session) {
+  return {
+      DiffDomain{
+          "Agent",
+          kAgentGitRelPath,
+          session.getSystemConfigPath(),
+          session.getSessionConfigPath(),
+          session.sessionExists()},
+      DiffDomain{
+          "BGP",
+          kBgpGitRelPath,
+          session.getBgpSystemConfigPath(),
+          session.getBgpSessionConfigPath(),
+          session.bgpSessionExists()},
+  };
+}
+
+// Read a file, returning empty content (not an error) when it doesn't exist.
+std::string readFileOrEmpty(const std::string& path) {
+  std::string content;
+  folly::readFile(path.c_str(), content);
+  return content;
+}
+
+// Get config content from a revision specifier for a specific domain file.
+// "current" reads the live system file. A path absent at the given revision
+// (e.g. a commit predating BGP config) is treated as empty content.
 std::pair<std::string, std::string> getRevisionContent(
     const std::string& revision,
-    ConfigSession& session) {
-  auto& git = session.getGit();
-  std::string cliConfigPath = session.getCliConfigPath();
-
+    const DiffDomain& domain,
+    Git& git) {
   if (revision == "current") {
-    // Read the current live config (via the symlink or directly from cli path)
-    std::string content;
-    if (!folly::readFile(cliConfigPath.c_str(), content)) {
-      throw std::runtime_error(
-          "Failed to read current config from " + cliConfigPath);
-    }
-    return {content, "current live config"};
+    return {readFileOrEmpty(domain.systemPath), "current live config"};
   }
-
-  // Resolve the commit SHA and get the content from Git
   std::string resolvedSha = git.resolveRef(revision);
-  std::string content = git.fileAtRevision(resolvedSha, "cli/agent.conf");
+  // Verify the revision is real before treating a missing domain path as empty.
+  // cli/agent.conf is present in every commit (including the initial one), so a
+  // genuinely invalid revision throws here and propagates; only a path absent
+  // at an otherwise-valid revision (e.g. bgpcpp.conf before BGP existed) is
+  // treated as empty.
+  git.fileAtRevision(resolvedSha, kAgentGitRelPath);
+  std::string content;
+  try {
+    content = git.fileAtRevision(resolvedSha, domain.gitRelPath);
+  } catch (const std::exception&) {
+    content = "";
+  }
   return {content, Git::shortSha1(revision)};
 }
 
@@ -116,64 +159,108 @@ std::string executeDiff(
   }
 }
 
+// Append a (optionally headered) diff section to the combined output.
+void appendSection(
+    std::string& out,
+    const std::string& name,
+    const std::string& body,
+    bool withHeader) {
+  if (withHeader) {
+    if (!out.empty()) {
+      out += "\n";
+    }
+    out += fmt::format("===== {} config =====\n", name);
+  }
+  out += body;
+  if (!body.empty() && body.back() != '\n') {
+    out += "\n";
+  }
+}
+
 } // namespace
 
 CmdConfigSessionDiffTraits::RetType CmdConfigSessionDiff::queryClient(
     const HostInfo& /* hostInfo */,
     const utils::RevisionList& revisions) {
   auto& session = ConfigSession::getInstance();
-
-  std::string systemConfigPath = session.getSystemConfigPath();
-  std::string sessionConfigPath = session.getSessionConfigPath();
-
-  // Mode 1: No arguments - diff session vs current live config
+  auto& git = session.getGit();
+  auto domains = allDomains(session);
+
+  // Modes 1 and 2 both diff each staged domain's session file against some
+  // "base" (current live config for mode 1; a revision for mode 2). The only
+  // difference is how the base content+label is obtained, so share the loop.
+  auto diffStagedDomains =
+      [&](const std::function<std::pair<std::string, std::string>(
+              const DiffDomain&)>& getBase) {
+        int stagedCount = 0;
+        for (const auto& d : domains) {
+          stagedCount += d.staged ? 1 : 0;
+        }
+        std::string out;
+        for (const auto& d : domains) {
+          if (!d.staged) {
+            continue;
+          }
+          auto [baseContent, baseLabel] = getBase(d);
+          std::string sessionContent;
+          if (!folly::readFile(d.sessionPath.c_str(), sessionContent)) {
+            throw std::runtime_error(
+                "Failed to read session config from " + d.sessionPath);
+          }
+          appendSection(
+              out,
+              d.name,
+              executeDiff(
+                  baseContent, sessionContent, baseLabel, "session config"),
+              stagedCount > 1);
+        }
+        return out;
+      };
+
+  // Mode 1: No arguments - diff each staged domain's session vs current live.
   if (revisions.empty()) {
-    if (!session.sessionExists()) {
+    if (!session.hasActiveSession()) {
       return "No config session exists. Make a config change first.";
     }
-
-    std::string currentContent;
-    if (!folly::readFile(systemConfigPath.c_str(), currentContent)) {
-      throw std::runtime_error(
-          "Failed to read current config from " + systemConfigPath);
-    }
-
-    std::string sessionContent;
-    if (!folly::readFile(sessionConfigPath.c_str(), sessionContent)) {
-      throw std::runtime_error(
-          "Failed to read session config from " + sessionConfigPath);
-    }
-
-    return executeDiff(
-        currentContent,
-        sessionContent,
-        "current live config",
-        "session config");
+    return diffStagedDomains([&](const DiffDomain& d) {
+      return std::make_pair(
+          readFileOrEmpty(d.systemPath), std::string("current live config"));
+    });
   }
 
-  // Mode 2: One argument - diff session vs specified revision
+  // Mode 2: One argument - diff each staged domain's session vs a revision.
   if (revisions.size() == 1) {
-    if (!session.sessionExists()) {
+    if (!session.hasActiveSession()) {
       return "No config session exists. Make a config change first.";
     }
-
-    auto [revContent, revLabel] = getRevisionContent(revisions[0], session);
-
-    std::string sessionContent;
-    if (!folly::readFile(sessionConfigPath.c_str(), sessionContent)) {
-      throw std::runtime_error(
-          "Failed to read session config from " + sessionConfigPath);
-    }
-
-    return executeDiff(revContent, sessionContent, revLabel, "session config");
+    return diffStagedDomains([&](const DiffDomain& d) {
+      return getRevisionContent(revisions[0], d, git);
+    });
   }
 
-  // Mode 3: Two arguments - diff between two revisions
+  // Mode 3: Two arguments - diff between two revisions for each domain that
+  // has content at either revision.
   if (revisions.size() == 2) {
-    auto [content1, label1] = getRevisionContent(revisions[0], session);
-    auto [content2, label2] = getRevisionContent(revisions[1], session);
-
-    return executeDiff(content1, content2, label1, label2);
+    // Pre-compute each domain's rendered diff section so headers are only added
+    // when more than one domain is shown.
+    std::vector<std::pair<std::string, std::string>> sections; // {name, body}
+    for (const auto& d : domains) {
+      auto [c1, l1] = getRevisionContent(revisions[0], d, git);
+      auto [c2, l2] = getRevisionContent(revisions[1], d, git);
+      if (c1.empty() && c2.empty()) {
+        continue; // domain absent at both revisions
+      }
+      sections.emplace_back(d.name, executeDiff(c1, c2, l1, l2));
+    }
+    if (sections.empty()) {
+      return "No config found at the given revisions.";
+    }
+    std::string out;
+    const bool multi = sections.size() > 1;
+    for (const auto& [name, body] : sections) {
+      appendSection(out, name, body, multi);
+    }
+    return out;
   }
 
   // More than 2 arguments is an error