[Nexthop][fboss2-dev] Add hitless/warmboot/coldboot options for config reload

cmake/AgentHwSaiStoreTest.cmake

@@ -35,7 +35,6 @@ add_executable(store_test
     fboss/agent/hw/sai/store/tests/UdfStoreTest.cpp
     fboss/agent/hw/sai/store/tests/VlanStoreTest.cpp
     fboss/agent/hw/sai/store/tests/WredStoreTest.cpp
-    fboss/agent/hw/sai/store/tests/UdfStoreTest.cpp
 )
 
 target_link_libraries(store_test

cmake/AgentState.cmake

@@ -117,7 +117,6 @@ add_library(state
   fboss/agent/state/UdfGroupMap.cpp
   fboss/agent/state/UdfPacketMatcher.cpp
   fboss/agent/state/UdfPacketMatcherMap.cpp
-  fboss/agent/state/IpTunnel.cpp
   fboss/agent/state/IpTunnelMap.cpp
   fboss/agent/state/Vlan.cpp
   fboss/agent/state/VlanMap.cpp

cmake/AgentTestAgentHwTests.cmake

@@ -215,7 +215,6 @@ add_library(agent_hw_test_src
   fboss/agent/test/agent_hw_tests/AgentRollbackTests.cpp
   fboss/agent/test/agent_hw_tests/AgentRouteRollbackTests.cpp
   fboss/agent/test/agent_hw_tests/AgentPacketStreamHandlerTests.cpp
-  fboss/agent/test/agent_hw_tests/AgentAclTableGroupTests.cpp
 )
 
 target_link_libraries(agent_hw_test_src

cmake/CliFboss2.cmake

@@ -1094,8 +1094,6 @@ add_library(fboss2_config_lib
   fboss/cli/fboss2/commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h
   fboss/cli/fboss2/commands/delete/arp/CmdDeleteArp.cpp
   fboss/cli/fboss2/commands/delete/arp/CmdDeleteArp.h
-  fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.cpp
-  fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.h
   fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.cpp
   fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicy.h
   fboss/cli/fboss2/commands/delete/qos/policy/CmdDeleteQosPolicyMap.cpp

cmake/CliFboss2TestIntegrationTest.cmake

@@ -27,6 +27,7 @@ add_executable(fboss2_integration_test
   fboss/cli/fboss2/test/integration_test/ConfigPfcTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigPortQueueConfigTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigPtpTest.cpp
+  fboss/cli/fboss2/test/integration_test/ConfigReloadTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigInterfaceFlowControlTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigInterfaceLldpExpectedValueTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigInterfaceLookupClassTest.cpp

cmake/ThriftCowVisitors.cmake

@@ -33,7 +33,6 @@ add_library(
   thrift_cow_visitors
   fboss/thrift_cow/visitors/DeltaVisitor.h
   fboss/thrift_cow/visitors/ExtendedPathVisitor.h
-  fboss/thrift_cow/visitors/ExtendedPathVisitor.h
   fboss/thrift_cow/visitors/PathVisitor.h
   fboss/thrift_cow/visitors/RecurseVisitor.h
   fboss/thrift_cow/visitors/VisitorUtils.h

fboss/cli/fboss2/BUCK

@@ -1409,6 +1409,9 @@ cpp_library(
         "//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/agent:switchinfo_utils",
         "//fboss/agent:fboss-types",
         "//fboss/agent:platform_config-cpp2-types",
         "//fboss/agent:switch_config-cpp2-types",

fboss/cli/fboss2/CmdListConfig.cpp

@@ -966,7 +966,7 @@ const CommandTree& kConfigCommandTree() {
 
       {"config",
        "reload",
-       "Reload agent configuration",
+       "Reload agent config (hitless | warmboot | coldboot)",
        commandHandler<CmdConfigReload>,
        argRegistrar<CmdConfigReloadTraits>},
 

fboss/cli/fboss2/commands/config/CmdConfigReload.cpp

@@ -10,21 +10,141 @@
 
 #include "fboss/cli/fboss2/commands/config/CmdConfigReload.h"
 
+#include "fboss/agent/AgentDirectoryUtil.h"
+#include "fboss/agent/gen-cpp2/agent_config_types.h"
 #include "fboss/agent/if/gen-cpp2/FbossCtrlAsyncClient.h"
 #include "fboss/cli/fboss2/CmdHandler.cpp"
+#include "fboss/cli/fboss2/session/ConfigSession.h"
+#include "fboss/cli/fboss2/session/FbossServiceUtil.h"
 #include "fboss/cli/fboss2/utils/CmdClientUtilsCommon.h"
 
+#include <fmt/format.h>
+#include <folly/FileUtil.h>
+#include <folly/String.h>
+#include <thrift/lib/cpp2/protocol/Serializer.h>
+#include <filesystem>
 #include <iostream>
+#include <stdexcept>
 
 namespace facebook::fboss {
 
+BootTypeArg::BootTypeArg() : BaseObjectArgType() {}
+
+BootTypeArg::BootTypeArg(std::vector<std::string> v) {
+  if (v.empty()) {
+    level_ = cli::ConfigActionLevel::HITLESS;
+    return;
+  }
+  if (v.size() != 1) {
+    throw std::invalid_argument(
+        "Expected at most one boot type (hitless, warmboot, or coldboot)");
+  }
+  std::string mode = v[0];
+  folly::toLowerAscii(mode);
+  if (mode == "hitless") {
+    level_ = cli::ConfigActionLevel::HITLESS;
+  } else if (mode == "warmboot") {
+    level_ = cli::ConfigActionLevel::AGENT_WARMBOOT;
+  } else if (mode == "coldboot") {
+    level_ = cli::ConfigActionLevel::AGENT_COLDBOOT;
+  } else {
+    throw std::invalid_argument(
+        "Invalid boot type '" + v[0] +
+        "'. Expected 'hitless', 'warmboot', or 'coldboot'");
+  }
+  data_.push_back(v[0]);
+}
+
+namespace {
+
+// Build a comma-separated list of "<service> (<bootType>)" entries used in
+// the success message for warmboot / coldboot reloads.
+std::string formatRestartedServices(
+    const std::vector<std::string>& services,
+    folly::StringPiece bootType) {
+  std::vector<std::string> labeled;
+  labeled.reserve(services.size());
+  for (const auto& service : services) {
+    labeled.push_back(fmt::format("{} ({})", service, bootType));
+  }
+  return folly::join(", ", labeled);
+}
+
+// Restart agent services locally (warmboot or coldboot) by constructing a
+// FbossServiceUtil directly from the on-disk agent config. We deserialize
+// the JSON straight into cfg::AgentConfig (mirroring ConfigSession) rather
+// than going through the AgentConfig wrapper, to avoid pulling
+// load_agent_config into fboss2-config-lib. This also keeps the command free of
+// session side effects.
+std::string performLocalRestart(cli::ConfigActionLevel level) {
+  // Derive /etc/coop/agent.conf via AgentDirectoryUtil so the
+  // FBOSS_CONFIG_BASE_DIR test override works. getConfigDirectory() returns
+  // <base>/agent; the agent reads its config from <base>/agent.conf (one level
+  // up).
+  AgentDirectoryUtil dirUtil;
+  std::string configPath = std::filesystem::path(dirUtil.getConfigDirectory())
+                               .parent_path()
+                               .string() +
+      "/agent.conf";
+
+  std::string configJson;
+  if (!folly::readFile(configPath.c_str(), configJson)) {
+    throw std::runtime_error(
+        fmt::format("Failed to read agent config file: {}", configPath));
+  }
+  cfg::AgentConfig agentConfig;
+  apache::thrift::SimpleJSONSerializer::deserialize<cfg::AgentConfig>(
+      configJson, agentConfig);
+
+  // FbossServiceUtil infers multi_switch mode and switch indices from the
+  // config; restartService() waits for each service to become active and
+  // throws std::runtime_error on timeout.
+  FbossServiceUtil serviceUtil(agentConfig);
+  auto restarted = serviceUtil.restartService(cli::ServiceType::AGENT, level);
+
+  folly::StringPiece bootType =
+      (level == cli::ConfigActionLevel::AGENT_COLDBOOT) ? "coldboot"
+                                                        : "warmboot";
+  return fmt::format(
+      "Config reloaded successfully via {} restart.",
+      formatRestartedServices(restarted, bootType));
+}
+
+} // namespace
+
 CmdConfigReloadTraits::RetType CmdConfigReload::queryClient(
-    const HostInfo& hostInfo) {
-  auto client =
-      utils::createClient<facebook::fboss::FbossCtrlAsyncClient>(hostInfo);
+    const HostInfo& hostInfo,
+    const BootTypeArg& bootType) {
+  if (bootType.level() == cli::ConfigActionLevel::HITLESS) {
+    auto client =
+        utils::createClient<facebook::fboss::FbossCtrlAsyncClient>(hostInfo);
+    client->sync_reloadConfig();
+    return "Config reloaded successfully";
+  }
+
+  // warmboot / coldboot dispatch to local systemctl, so reject non-local
+  // hosts to avoid restarting the wrong machine.
+  if (!hostInfo.isLocalHost()) {
+    throw std::invalid_argument(
+        fmt::format(
+            "config reload {} is local-only; remove --host or run on the switch.",
+            (bootType.level() == cli::ConfigActionLevel::AGENT_COLDBOOT)
+                ? "coldboot"
+                : "warmboot"));
+  }
+
+  // Warn (don't refuse) if the operator has an active session on disk; this
+  // command operates on /etc/coop/agent.conf only and will not apply session
+  // changes. Use the static path getter to avoid creating a singleton.
+  std::error_code ec;
+  if (std::filesystem::exists(
+          ConfigSession::getSessionConfigPathStatic(), ec)) {
+    std::cerr << "Warning: an active config session exists in ~/.fboss2/; this "
+              << "command operates on /etc/coop/agent.conf only and will not "
+              << "apply session changes." << std::endl;
+  }
 
-  client->sync_reloadConfig();
-  return "Config reloaded successfully";
+  return performLocalRestart(bootType.level());
 }
 
 void CmdConfigReload::printOutput(const RetType& logMsg) {

fboss/cli/fboss2/commands/config/CmdConfigReload.h

@@ -10,16 +10,46 @@
 
 #pragma once
 
-#include <folly/Conv.h>
-#include <iostream>
 #include "fboss/cli/fboss2/CmdHandler.h"
-#include "fboss/cli/fboss2/utils/CmdClientUtils.h"
-#include "fboss/cli/fboss2/utils/CmdUtils.h"
+#include "fboss/cli/fboss2/gen-cpp2/cli_metadata_types.h"
+#include "fboss/cli/fboss2/utils/CmdUtilsCommon.h"
 
 namespace facebook::fboss {
 
+// Optional positional argument controlling how the reload is applied:
+//   (omitted) or "hitless" -> sync_reloadConfig() over thrift (default)
+//   "warmboot"             -> systemctl restart agent services
+//   "coldboot"             -> create coldboot marker + systemctl restart
+class BootTypeArg : public utils::BaseObjectArgType<std::string> {
+ public:
+  /* implicit */ BootTypeArg();
+
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  /* implicit */ BootTypeArg(std::vector<std::string> v);
+
+  cli::ConfigActionLevel level() const {
+    return level_;
+  }
+
+ private:
+  cli::ConfigActionLevel level_ = cli::ConfigActionLevel::HITLESS;
+};
+
 struct CmdConfigReloadTraits : public WriteCommandTraits {
-  using ObjectArgType = std::monostate;
+  static void addCliArg(CLI::App& cmd, std::vector<std::string>& args) {
+    cmd.add_option(
+        "boot_type",
+        args,
+        "Optional boot type controlling how the reload is applied:\n"
+        "  hitless   (default) Apply config diff in-place via the running "
+        "agent.\n"
+        "  warmboot  Restart the FBOSS agent service(s); on startup the "
+        "agent reads agent.conf and the warmboot state cache from the "
+        "previous run. Local-only.\n"
+        "  coldboot  Restart the FBOSS agent service(s) and reprogram the "
+        "ASIC from scratch from agent.conf. Local-only.");
+  }
+  using ObjectArgType = BootTypeArg;
   using RetType = std::string;
 };
 
@@ -29,7 +59,7 @@ class CmdConfigReload
   using ObjectArgType = CmdConfigReloadTraits::ObjectArgType;
   using RetType = CmdConfigReloadTraits::RetType;
 
-  RetType queryClient(const HostInfo& hostInfo);
+  RetType queryClient(const HostInfo& hostInfo, const BootTypeArg& bootType);
 
   void printOutput(const RetType& logMsg);
 };

fboss/cli/fboss2/session/FbossServiceUtil.cpp

@@ -15,6 +15,8 @@
 #include <glog/logging.h>
 #include <stdexcept>
 #include "fboss/agent/AgentDirectoryUtil.h"
+#include "fboss/agent/SwitchInfoUtils.h"
+#include "fboss/agent/gen-cpp2/agent_config_types.h"
 #include "fboss/agent/if/gen-cpp2/FbossCtrl.h"
 #include "fboss/cli/fboss2/session/SystemdInterface.h"
 #include "fboss/cli/fboss2/utils/CmdClientUtilsCommon.h"
@@ -30,6 +32,20 @@ constexpr std::string_view kBgpd = "bgpd";
 
 namespace facebook::fboss {
 
+FbossServiceUtil::FbossServiceUtil(const cfg::AgentConfig& agentConfig)
+    : systemd_(std::make_unique<SystemdInterface>()) {
+  const auto& args = *agentConfig.defaultCommandLineArgs();
+  // Parse multi_switch from the on-disk config rather than calling
+  // utils::isMultiSwitchEnabled(hostInfo), so this works independently of
+  // the agent's state without requiring a live thrift connection.
+  multiSwitch_ =
+      args.count("multi_switch") && args.at("multi_switch") == "true";
+  for (const auto& [_, info] : getSwitchInfoFromConfig(&(*agentConfig.sw()))) {
+    switchIndexes_.push_back(*info.switchIndex());
+  }
+  std::sort(switchIndexes_.begin(), switchIndexes_.end());
+}
+
 FbossServiceUtil::FbossServiceUtil(
     std::vector<int> switchIndexes,
     bool multiSwitch)

fboss/cli/fboss2/session/FbossServiceUtil.h

@@ -20,6 +20,10 @@
 
 namespace facebook::fboss {
 
+namespace cfg {
+class AgentConfig;
+} // namespace cfg
+
 /**
  * FbossServiceUtil handles systemd service orchestration for FBOSS agents.
  *
@@ -31,6 +35,9 @@ namespace facebook::fboss {
  */
 class FbossServiceUtil {
  public:
+  // Construct from an AgentConfig: infers multi_switch mode and switch indices.
+  explicit FbossServiceUtil(const cfg::AgentConfig& agentConfig);
+
   // Production constructor: creates its own SystemdInterface.
   FbossServiceUtil(std::vector<int> switchIndexes, bool multiSwitch);