[Nexthop][fboss2-dev] Add a "fboss2 config session diff" command.

cmake/CliFboss2.cmake

@@ -577,6 +577,8 @@ add_library(fboss2_config_lib
   fboss/cli/fboss2/commands/config/CmdConfigReload.cpp
   fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.h
   fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.cpp
+  fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h
+  fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.cpp
   fboss/cli/fboss2/session/ConfigSession.h
   fboss/cli/fboss2/session/ConfigSession.cpp
   fboss/cli/fboss2/CmdListConfig.cpp

cmake/CliFboss2Test.cmake

@@ -5,6 +5,7 @@ add_executable(fboss2_cmd_test
   fboss/cli/fboss2/test/TestMain.cpp
   fboss/cli/fboss2/test/CmdConfigAppliedInfoTest.cpp
   fboss/cli/fboss2/test/CmdConfigReloadTest.cpp
+  fboss/cli/fboss2/test/CmdConfigSessionDiffTest.cpp
   fboss/cli/fboss2/test/CmdConfigSessionTest.cpp
   fboss/cli/fboss2/test/CmdSetPortStateTest.cpp
   fboss/cli/fboss2/test/CmdShowAclTest.cpp

fboss/cli/fboss2/BUCK

@@ -772,12 +772,14 @@ cpp_library(
         "commands/config/CmdConfigAppliedInfo.cpp",
         "commands/config/CmdConfigReload.cpp",
         "commands/config/session/CmdConfigSessionCommit.cpp",
+        "commands/config/session/CmdConfigSessionDiff.cpp",
         "session/ConfigSession.cpp",
     ],
     headers = [
         "commands/config/CmdConfigAppliedInfo.h",
         "commands/config/CmdConfigReload.h",
         "commands/config/session/CmdConfigSessionCommit.h",
+        "commands/config/session/CmdConfigSessionDiff.h",
         "session/ConfigSession.h",
     ],
     exported_deps = [

fboss/cli/fboss2/CmdHandlerImplConfig.cpp

@@ -13,6 +13,7 @@
 #include "fboss/cli/fboss2/commands/config/CmdConfigAppliedInfo.h"
 #include "fboss/cli/fboss2/commands/config/CmdConfigReload.h"
 #include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.h"
+#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h"
 
 namespace facebook::fboss {
 
@@ -21,5 +22,7 @@ CmdHandler<CmdConfigAppliedInfo, CmdConfigAppliedInfoTraits>::run();
 template void CmdHandler<CmdConfigReload, CmdConfigReloadTraits>::run();
 template void
 CmdHandler<CmdConfigSessionCommit, CmdConfigSessionCommitTraits>::run();
+template void
+CmdHandler<CmdConfigSessionDiff, CmdConfigSessionDiffTraits>::run();
 
 } // namespace facebook::fboss

fboss/cli/fboss2/CmdListConfig.cpp

@@ -14,6 +14,7 @@
 #include "fboss/cli/fboss2/commands/config/CmdConfigAppliedInfo.h"
 #include "fboss/cli/fboss2/commands/config/CmdConfigReload.h"
 #include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.h"
+#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h"
 
 namespace facebook::fboss {
 
@@ -30,11 +31,17 @@ const CommandTree& kConfigCommandTree() {
           "session",
           "Manage config session",
           {{
-              "commit",
-              "Commit the current config session",
-              commandHandler<CmdConfigSessionCommit>,
-              argTypeHandler<CmdConfigSessionCommitTraits>,
-          }},
+               "commit",
+               "Commit the current config session",
+               commandHandler<CmdConfigSessionCommit>,
+               argTypeHandler<CmdConfigSessionCommitTraits>,
+           },
+           {
+               "diff",
+               "Show diff between configs (session vs live, session vs revision, or revision vs revision)",
+               commandHandler<CmdConfigSessionDiff>,
+               argTypeHandler<CmdConfigSessionDiffTraits>,
+           }},
       },
 
       {"config",

fboss/cli/fboss2/CmdSubcommands.cpp

@@ -219,6 +219,10 @@ CLI::App* CmdSubcommands::addCommand(
         case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_FAN_PWM:
           subCmd->add_option("pwm", args, "Fan PWM (0..100) or 'disable'");
           break;
+        case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_REVISION_LIST:
+          subCmd->add_option(
+              "revisions", args, "Revision(s) in the form 'rN' or 'current'");
+          break;
         case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_UNINITIALIZE:
         case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_NONE:
           break;

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

@@ -0,0 +1,149 @@
+/*
+ *  Copyright (c) 2004-present, Facebook, Inc.
+ *  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 "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h"
+#include "fboss/cli/fboss2/session/ConfigSession.h"
+
+#include <folly/Subprocess.h>
+#include <filesystem>
+
+namespace fs = std::filesystem;
+
+namespace facebook::fboss {
+
+namespace {
+
+// Helper function to resolve a revision specifier to a file path
+// Note: Revision format validation is done in RevisionList constructor
+std::string resolveRevisionPath(
+    const std::string& revision,
+    const std::string& cliConfigDir,
+    const std::string& systemConfigPath) {
+  if (revision == "current") {
+    return systemConfigPath;
+  }
+
+  // Build the path (revision is already validated to be in "rN" format)
+  std::string revisionPath = cliConfigDir + "/agent-" + revision + ".conf";
+
+  // Check if the file exists
+  if (!fs::exists(revisionPath)) {
+    throw std::invalid_argument(
+        "Revision " + revision + " does not exist at " + revisionPath);
+  }
+
+  return revisionPath;
+}
+
+// Helper function to execute diff and return the result
+std::string executeDiff(
+    const std::string& path1,
+    const std::string& path2,
+    const std::string& label1,
+    const std::string& label2) {
+  try {
+    folly::Subprocess proc(
+        std::vector<std::string>{
+            "/usr/bin/diff",
+            "-u",
+            "--label",
+            label1,
+            "--label",
+            label2,
+            path1,
+            path2},
+        folly::Subprocess::Options().pipeStdout().pipeStderr());
+
+    auto result = proc.communicate();
+    int returnCode = proc.wait().exitStatus();
+
+    // diff returns 0 if files are identical, 1 if different, 2 on error
+    if (returnCode == 0) {
+      return "No differences between " + label1 + " and " + label2 + ".";
+    } else if (returnCode == 1) {
+      // Files differ - return the diff output
+      return result.first;
+    } else {
+      // Error occurred
+      throw std::runtime_error("diff command failed: " + result.second);
+    }
+  } catch (const std::exception& ex) {
+    throw std::runtime_error(
+        "Failed to execute diff command: " + std::string(ex.what()));
+  }
+}
+
+} // 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();
+  std::string cliConfigDir = session.getCliConfigDir();
+
+  // Mode 1: No arguments - diff session vs current live config
+  if (revisions.empty()) {
+    if (!session.sessionExists()) {
+      return "No config session exists. Make a config change first.";
+    }
+
+    return executeDiff(
+        systemConfigPath,
+        sessionConfigPath,
+        "current live config",
+        "session config");
+  }
+
+  // Mode 2: One argument - diff session vs specified revision
+  if (revisions.size() == 1) {
+    if (!session.sessionExists()) {
+      return "No config session exists. Make a config change first.";
+    }
+
+    std::string revisionPath =
+        resolveRevisionPath(revisions[0], cliConfigDir, systemConfigPath);
+    std::string label =
+        revisions[0] == "current" ? "current live config" : revisions[0];
+
+    return executeDiff(
+        revisionPath, sessionConfigPath, label, "session config");
+  }
+
+  // Mode 3: Two arguments - diff between two revisions
+  if (revisions.size() == 2) {
+    std::string path1 =
+        resolveRevisionPath(revisions[0], cliConfigDir, systemConfigPath);
+    std::string path2 =
+        resolveRevisionPath(revisions[1], cliConfigDir, systemConfigPath);
+
+    std::string label1 =
+        revisions[0] == "current" ? "current live config" : revisions[0];
+    std::string label2 =
+        revisions[1] == "current" ? "current live config" : revisions[1];
+
+    return executeDiff(path1, path2, label1, label2);
+  }
+
+  // More than 2 arguments is an error
+  throw std::invalid_argument(
+      "Too many arguments. Expected 0, 1, or 2 revision specifiers.");
+}
+
+void CmdConfigSessionDiff::printOutput(const RetType& diffOutput) {
+  std::cout << diffOutput;
+  if (!diffOutput.empty() && diffOutput.back() != '\n') {
+    std::cout << std::endl;
+  }
+}
+
+} // namespace facebook::fboss

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

@@ -0,0 +1,39 @@
+/*
+ *  Copyright (c) 2004-present, Facebook, Inc.
+ *  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.
+ *
+ */
+
+#pragma once
+
+#include "fboss/cli/fboss2/CmdHandler.h"
+#include "fboss/cli/fboss2/utils/CmdClientUtils.h"
+#include "fboss/cli/fboss2/utils/CmdUtils.h"
+
+namespace facebook::fboss {
+
+struct CmdConfigSessionDiffTraits : public WriteCommandTraits {
+  static constexpr utils::ObjectArgTypeId ObjectArgTypeId =
+      utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_REVISION_LIST;
+  using ObjectArgType = utils::RevisionList;
+  using RetType = std::string;
+};
+
+class CmdConfigSessionDiff
+    : public CmdHandler<CmdConfigSessionDiff, CmdConfigSessionDiffTraits> {
+ public:
+  using ObjectArgType = CmdConfigSessionDiffTraits::ObjectArgType;
+  using RetType = CmdConfigSessionDiffTraits::RetType;
+
+  RetType queryClient(
+      const HostInfo& hostInfo,
+      const utils::RevisionList& revisions);
+
+  void printOutput(const RetType& diffOutput);
+};
+
+} // namespace facebook::fboss

fboss/cli/fboss2/session/ConfigSession.cpp

@@ -197,10 +197,24 @@ ConfigSession::ConfigSession(
   initializeSession();
 }
 
-ConfigSession& ConfigSession::getInstance() {
-  static ConfigSession instance;
+namespace {
+std::unique_ptr<ConfigSession>& getInstancePtr() {
+  static std::unique_ptr<ConfigSession> instance;
   return instance;
 }
+} // namespace
+
+ConfigSession& ConfigSession::getInstance() {
+  auto& instance = getInstancePtr();
+  if (!instance) {
+    instance = std::make_unique<ConfigSession>();
+  }
+  return *instance;
+}
+
+void ConfigSession::setInstance(std::unique_ptr<ConfigSession> newInstance) {
+  getInstancePtr() = std::move(newInstance);
+}
 
 std::string ConfigSession::getSessionConfigPath() const {
   return sessionConfigPath_;
@@ -210,6 +224,10 @@ std::string ConfigSession::getSystemConfigPath() const {
   return systemConfigPath_;
 }
 
+std::string ConfigSession::getCliConfigDir() const {
+  return cliConfigDir_;
+}
+
 bool ConfigSession::sessionExists() const {
   return fs::exists(sessionConfigPath_);
 }

fboss/cli/fboss2/session/ConfigSession.h

@@ -93,6 +93,9 @@ class ConfigSession {
   // Get the path to the system config file (/etc/coop/agent.conf)
   std::string getSystemConfigPath() const;
 
+  // Get the path to the CLI config directory (/etc/coop/cli)
+  std::string getCliConfigDir() const;
+
   // Atomically commit the session to /etc/coop/cli/agent-rN.conf,
   // update the symlink /etc/coop/agent.conf to point to it, and reload config.
   // Returns the revision number that was committed if the commit was
@@ -124,6 +127,9 @@ class ConfigSession {
       const std::string& systemConfigPath,
       const std::string& cliConfigDir);
 
+  // Set the singleton instance (for testing only)
+  static void setInstance(std::unique_ptr<ConfigSession> instance);
+
  private:
   std::string sessionConfigPath_;
   std::string systemConfigPath_;

fboss/cli/fboss2/test/BUCK

@@ -57,6 +57,7 @@ cpp_unittest(
     srcs = [
         "CmdConfigAppliedInfoTest.cpp",
         "CmdConfigReloadTest.cpp",
+        "CmdConfigSessionDiffTest.cpp",
         "CmdConfigSessionTest.cpp",
         "CmdGetPcapTest.cpp",
         "CmdSetPortStateTest.cpp",