diff --git a/cmake/CliFboss2.cmake b/cmake/CliFboss2.cmake index d0891c04e7737..f9ead328e24fb 100644 --- a/cmake/CliFboss2.cmake +++ b/cmake/CliFboss2.cmake @@ -575,8 +575,14 @@ add_library(fboss2_config_lib fboss/cli/fboss2/commands/config/CmdConfigAppliedInfo.cpp fboss/cli/fboss2/commands/config/CmdConfigReload.h fboss/cli/fboss2/commands/config/CmdConfigReload.cpp + fboss/cli/fboss2/commands/config/history/CmdConfigHistory.h + fboss/cli/fboss2/commands/config/history/CmdConfigHistory.cpp + fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.h + fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.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 diff --git a/cmake/CliFboss2Test.cmake b/cmake/CliFboss2Test.cmake index 3d4391cb82dee..cbb89bdfd0405 100644 --- a/cmake/CliFboss2Test.cmake +++ b/cmake/CliFboss2Test.cmake @@ -4,7 +4,9 @@ add_executable(fboss2_cmd_test fboss/cli/fboss2/test/TestMain.cpp fboss/cli/fboss2/test/CmdConfigAppliedInfoTest.cpp + fboss/cli/fboss2/test/CmdConfigHistoryTest.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 diff --git a/fboss/cli/fboss2/BUCK b/fboss/cli/fboss2/BUCK index 592c7bc87e675..4df0553b12bcf 100644 --- a/fboss/cli/fboss2/BUCK +++ b/fboss/cli/fboss2/BUCK @@ -771,13 +771,19 @@ cpp_library( "CmdListConfig.cpp", "commands/config/CmdConfigAppliedInfo.cpp", "commands/config/CmdConfigReload.cpp", + "commands/config/history/CmdConfigHistory.cpp", + "commands/config/rollback/CmdConfigRollback.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/history/CmdConfigHistory.h", + "commands/config/rollback/CmdConfigRollback.h", "commands/config/session/CmdConfigSessionCommit.h", + "commands/config/session/CmdConfigSessionDiff.h", "session/ConfigSession.h", ], exported_deps = [ diff --git a/fboss/cli/fboss2/CmdHandlerImplConfig.cpp b/fboss/cli/fboss2/CmdHandlerImplConfig.cpp index d4217e43fa286..6b2f8ac117f64 100644 --- a/fboss/cli/fboss2/CmdHandlerImplConfig.cpp +++ b/fboss/cli/fboss2/CmdHandlerImplConfig.cpp @@ -12,14 +12,21 @@ #include "fboss/cli/fboss2/commands/config/CmdConfigAppliedInfo.h" #include "fboss/cli/fboss2/commands/config/CmdConfigReload.h" +#include "fboss/cli/fboss2/commands/config/history/CmdConfigHistory.h" +#include "fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.h" #include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.h" +#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h" namespace facebook::fboss { template void CmdHandler::run(); template void CmdHandler::run(); +template void CmdHandler::run(); +template void CmdHandler::run(); template void CmdHandler::run(); +template void +CmdHandler::run(); } // namespace facebook::fboss diff --git a/fboss/cli/fboss2/CmdListConfig.cpp b/fboss/cli/fboss2/CmdListConfig.cpp index e59256deb5fbc..254e99acbfcff 100644 --- a/fboss/cli/fboss2/CmdListConfig.cpp +++ b/fboss/cli/fboss2/CmdListConfig.cpp @@ -13,7 +13,10 @@ #include "fboss/cli/fboss2/CmdHandler.h" #include "fboss/cli/fboss2/commands/config/CmdConfigAppliedInfo.h" #include "fboss/cli/fboss2/commands/config/CmdConfigReload.h" +#include "fboss/cli/fboss2/commands/config/history/CmdConfigHistory.h" +#include "fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.h" #include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.h" +#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h" namespace facebook::fboss { @@ -25,16 +28,28 @@ const CommandTree& kConfigCommandTree() { commandHandler, argTypeHandler}, + {"config", + "history", + "Show history of committed config revisions", + commandHandler, + argTypeHandler}, + { "config", "session", "Manage config session", {{ - "commit", - "Commit the current config session", - commandHandler, - argTypeHandler, - }}, + "commit", + "Commit the current config session", + commandHandler, + argTypeHandler, + }, + { + "diff", + "Show diff between configs (session vs live, session vs revision, or revision vs revision)", + commandHandler, + argTypeHandler, + }}, }, {"config", @@ -42,6 +57,12 @@ const CommandTree& kConfigCommandTree() { "Reload agent configuration", commandHandler, argTypeHandler}, + + {"config", + "rollback", + "Rollback to a previous config revision", + commandHandler, + argTypeHandler}, }; sort(root.begin(), root.end()); return root; diff --git a/fboss/cli/fboss2/CmdSubcommands.cpp b/fboss/cli/fboss2/CmdSubcommands.cpp index 87485bcd711c5..199f4b00a6c6b 100644 --- a/fboss/cli/fboss2/CmdSubcommands.cpp +++ b/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; diff --git a/fboss/cli/fboss2/commands/config/history/CmdConfigHistory.cpp b/fboss/cli/fboss2/commands/config/history/CmdConfigHistory.cpp new file mode 100644 index 0000000000000..69ffaa7d1c9a1 --- /dev/null +++ b/fboss/cli/fboss2/commands/config/history/CmdConfigHistory.cpp @@ -0,0 +1,161 @@ +/* + * 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/history/CmdConfigHistory.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "fboss/cli/fboss2/session/ConfigSession.h" +#include "fboss/cli/fboss2/utils/Table.h" + +namespace fs = std::filesystem; + +namespace facebook::fboss { + +namespace { + +struct RevisionInfo { + int revisionNumber; + std::string owner; + int64_t commitTimeNsec; // Commit time in nanoseconds since epoch + std::string filePath; +}; + +// Get the username from a UID +std::string getUsername(uid_t uid) { + struct passwd* pw = getpwuid(uid); + if (pw) { + return std::string(pw->pw_name); + } + // If we can't resolve the username, return the UID as a string + return "UID:" + std::to_string(uid); +} + +// Format time as a human-readable string with milliseconds +std::string formatTime(int64_t timeNsec) { + // Convert nanoseconds to seconds and remaining nanoseconds + std::time_t timeSec = timeNsec / 1000000000; + long nsec = timeNsec % 1000000000; + + char buffer[100]; + struct tm* timeinfo = std::localtime(&timeSec); + std::strftime(buffer, sizeof(buffer), "%Y-%m-%d %H:%M:%S", timeinfo); + + // Add milliseconds + long milliseconds = nsec / 1000000; + std::ostringstream oss; + oss << buffer << '.' << std::setfill('0') << std::setw(3) << milliseconds; + return oss.str(); +} + +// Collect all revision files from the CLI config directory +std::vector collectRevisions(const std::string& cliConfigDir) { + std::vector revisions; + + std::error_code ec; + if (!fs::exists(cliConfigDir, ec) || !fs::is_directory(cliConfigDir, ec)) { + // Directory doesn't exist or is not a directory + return revisions; + } + + for (const auto& entry : fs::directory_iterator(cliConfigDir, ec)) { + if (ec) { + continue; // Skip entries we can't read + } + + if (!entry.is_regular_file(ec)) { + continue; // Skip non-regular files + } + + std::string filename = entry.path().filename().string(); + int revNum = ConfigSession::extractRevisionNumber(filename); + + if (revNum < 0) { + continue; // Skip files that don't match our pattern + } + + // Get file metadata using statx to get birth time (creation time) + struct statx stx; + if (statx( + AT_FDCWD, entry.path().c_str(), 0, STATX_BTIME | STATX_UID, &stx) != + 0) { + continue; // Skip if we can't get file stats + } + + RevisionInfo info; + info.revisionNumber = revNum; + info.owner = getUsername(stx.stx_uid); + // Use birth time (creation time) if available, otherwise fall back to mtime + if (stx.stx_mask & STATX_BTIME) { + info.commitTimeNsec = + static_cast(stx.stx_btime.tv_sec) * 1000000000 + + stx.stx_btime.tv_nsec; + } else { + info.commitTimeNsec = + static_cast(stx.stx_mtime.tv_sec) * 1000000000 + + stx.stx_mtime.tv_nsec; + } + info.filePath = entry.path().string(); + + revisions.push_back(info); + } + + // Sort by revision number (ascending) + std::sort( + revisions.begin(), + revisions.end(), + [](const RevisionInfo& a, const RevisionInfo& b) { + return a.revisionNumber < b.revisionNumber; + }); + + return revisions; +} + +} // namespace + +CmdConfigHistoryTraits::RetType CmdConfigHistory::queryClient( + const HostInfo& hostInfo) { + auto& session = ConfigSession::getInstance(); + const std::string cliConfigDir = session.getCliConfigDir(); + + auto revisions = collectRevisions(cliConfigDir); + + if (revisions.empty()) { + return "No config revisions found in " + cliConfigDir; + } + + // Build the table + utils::Table table; + table.setHeader({"Revision", "Owner", "Commit Time"}); + + for (const auto& rev : revisions) { + table.addRow( + {"r" + std::to_string(rev.revisionNumber), + rev.owner, + formatTime(rev.commitTimeNsec)}); + } + + // Convert table to string + std::ostringstream oss; + oss << table; + return oss.str(); +} + +void CmdConfigHistory::printOutput(const RetType& tableOutput) { + std::cout << tableOutput << std::endl; +} + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/config/history/CmdConfigHistory.h b/fboss/cli/fboss2/commands/config/history/CmdConfigHistory.h new file mode 100644 index 0000000000000..44008e5e28c7d --- /dev/null +++ b/fboss/cli/fboss2/commands/config/history/CmdConfigHistory.h @@ -0,0 +1,37 @@ +/* + * 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 CmdConfigHistoryTraits : public WriteCommandTraits { + static constexpr utils::ObjectArgTypeId ObjectArgTypeId = + utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_NONE; + using ObjectArgType = std::monostate; + using RetType = std::string; +}; + +class CmdConfigHistory + : public CmdHandler { + public: + using ObjectArgType = CmdConfigHistoryTraits::ObjectArgType; + using RetType = CmdConfigHistoryTraits::RetType; + + RetType queryClient(const HostInfo& hostInfo); + + void printOutput(const RetType& tableOutput); +}; + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.cpp b/fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.cpp new file mode 100644 index 0000000000000..2b3b04e07a0ff --- /dev/null +++ b/fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.cpp @@ -0,0 +1,57 @@ +/* + * 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/rollback/CmdConfigRollback.h" +#include "fboss/cli/fboss2/session/ConfigSession.h" + +namespace facebook::fboss { + +CmdConfigRollbackTraits::RetType CmdConfigRollback::queryClient( + const HostInfo& hostInfo, + const utils::RevisionList& revisions) { + auto& session = ConfigSession::getInstance(); + + // Validate arguments + if (revisions.size() > 1) { + throw std::invalid_argument( + "Too many arguments. Expected 0 or 1 revision specifier."); + } + + if (!revisions.empty() && revisions[0] == "current") { + throw std::invalid_argument( + "Cannot rollback to 'current'. Please specify a revision number like 'r42'."); + } + + try { + int newRevision; + if (revisions.empty()) { + // No revision specified - rollback to previous revision + newRevision = session.rollback(hostInfo); + } else { + // Specific revision specified + newRevision = session.rollback(hostInfo, revisions[0]); + } + if (newRevision) { + return "Successfully rolled back to r" + std::to_string(newRevision) + + " and config reloaded."; + } else { + return "Failed to create a new revision after rollback."; + } + } catch (const std::exception& ex) { + throw std::runtime_error( + "Failed to rollback config: " + std::string(ex.what())); + } +} + +void CmdConfigRollback::printOutput(const RetType& logMsg) { + std::cout << logMsg << std::endl; +} + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.h b/fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.h new file mode 100644 index 0000000000000..e03baab404dff --- /dev/null +++ b/fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.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 CmdConfigRollbackTraits : public WriteCommandTraits { + static constexpr utils::ObjectArgTypeId ObjectArgTypeId = + utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_REVISION_LIST; + using ObjectArgType = utils::RevisionList; + using RetType = std::string; +}; + +class CmdConfigRollback + : public CmdHandler { + public: + using ObjectArgType = CmdConfigRollbackTraits::ObjectArgType; + using RetType = CmdConfigRollbackTraits::RetType; + + RetType queryClient( + const HostInfo& hostInfo, + const utils::RevisionList& revisions); + + void printOutput(const RetType& logMsg); +}; + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.cpp b/fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.cpp new file mode 100644 index 0000000000000..efb0fb447f8aa --- /dev/null +++ b/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 +#include + +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{ + "/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 diff --git a/fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h b/fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h new file mode 100644 index 0000000000000..10655c6485cbd --- /dev/null +++ b/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 { + 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 diff --git a/fboss/cli/fboss2/session/ConfigSession.cpp b/fboss/cli/fboss2/session/ConfigSession.cpp index ecf11bb06a86b..a9a4d959fb124 100644 --- a/fboss/cli/fboss2/session/ConfigSession.cpp +++ b/fboss/cli/fboss2/session/ConfigSession.cpp @@ -167,6 +167,25 @@ void ensureDirectoryExists(const std::string& dirPath) { } } +/* + * Get the current revision number by reading the symlink target. + * Returns -1 if unable to determine the current revision. + */ +int getCurrentRevisionNumber(const std::string& systemConfigPath) { + std::error_code ec; + + if (!fs::is_symlink(systemConfigPath, ec)) { + return -1; + } + + std::string target = fs::read_symlink(systemConfigPath, ec); + if (ec) { + return -1; + } + + return ConfigSession::extractRevisionNumber(target); +} + } // anonymous namespace ConfigSession::ConfigSession() { @@ -197,10 +216,24 @@ ConfigSession::ConfigSession( initializeSession(); } -ConfigSession& ConfigSession::getInstance() { - static ConfigSession instance; +namespace { +std::unique_ptr& getInstancePtr() { + static std::unique_ptr instance; return instance; } +} // namespace + +ConfigSession& ConfigSession::getInstance() { + auto& instance = getInstancePtr(); + if (!instance) { + instance = std::make_unique(); + } + return *instance; +} + +void ConfigSession::setInstance(std::unique_ptr newInstance) { + getInstancePtr() = std::move(newInstance); +} std::string ConfigSession::getSessionConfigPath() const { return sessionConfigPath_; @@ -210,6 +243,10 @@ std::string ConfigSession::getSystemConfigPath() const { return systemConfigPath_; } +std::string ConfigSession::getCliConfigDir() const { + return cliConfigDir_; +} + bool ConfigSession::sessionExists() const { return fs::exists(sessionConfigPath_); } @@ -438,4 +475,100 @@ int ConfigSession::commit(const HostInfo& hostInfo) { return revision; } +int ConfigSession::rollback(const HostInfo& hostInfo) { + // Get the current revision number + int currentRevision = getCurrentRevisionNumber(systemConfigPath_); + if (currentRevision <= 0) { + throw std::runtime_error( + "Cannot rollback: cannot determine the current revision from " + + systemConfigPath_); + } else if (currentRevision == 1) { + throw std::runtime_error( + "Cannot rollback: already at the first revision (r1)"); + } + + // Rollback to the previous revision + std::string targetRevision = "r" + std::to_string(currentRevision - 1); + return rollback(hostInfo, targetRevision); +} + +int ConfigSession::rollback( + const HostInfo& hostInfo, + const std::string& revision) { + ensureDirectoryExists(cliConfigDir_); + + // Build the path to the target revision + std::string targetConfigPath = cliConfigDir_ + "/agent-" + revision + ".conf"; + + // Check if the target revision exists + if (!fs::exists(targetConfigPath)) { + throw std::runtime_error( + "Revision " + revision + " does not exist at " + targetConfigPath); + } + + std::error_code ec; + + // Verify that the system config is a symlink + if (!fs::is_symlink(systemConfigPath_)) { + throw std::runtime_error( + systemConfigPath_ + " is not a symlink. Expected it to be a symlink."); + } + + // Read the old symlink target in case we need to undo the rollback + std::string oldSymlinkTarget = fs::read_symlink(systemConfigPath_, ec); + if (ec) { + throw std::runtime_error( + "Failed to read symlink " + systemConfigPath_ + ": " + ec.message()); + } + + // First, create a new revision with the same content as the target revision + auto [newRevisionPath, newRevision] = + createNextRevisionFile(fmt::format("{}/agent", cliConfigDir_)); + + // Copy the target config to the new revision file + fs::copy_file( + targetConfigPath, + newRevisionPath, + fs::copy_options::overwrite_existing, + ec); + if (ec) { + // Clean up the revision file we created + fs::remove(newRevisionPath); + throw std::runtime_error( + fmt::format( + "Failed to create new revision for rollback: {}", ec.message())); + } + + // Atomically update the symlink to point to the new revision + atomicSymlinkUpdate(systemConfigPath_, newRevisionPath); + + // Reload the config - if this fails, atomically undo the rollback + try { + auto client = + utils::createClient>( + hostInfo); + client->sync_reloadConfig(); + } catch (const std::exception& ex) { + // Rollback: atomically restore the old symlink + try { + atomicSymlinkUpdate(systemConfigPath_, oldSymlinkTarget); + } catch (const std::exception& rollbackEx) { + // If rollback also fails, include both errors in the message + throw std::runtime_error( + fmt::format( + "Failed to reload config: {}. Additionally, failed to rollback the symlink: {}", + ex.what(), + rollbackEx.what())); + } + throw std::runtime_error( + fmt::format( + "Failed to reload config, symlink was rolled back automatically: {}", + ex.what())); + } + + // Successfully rolled back + LOG(INFO) << "Rollback committed as revision r" << newRevision; + return newRevision; +} + } // namespace facebook::fboss diff --git a/fboss/cli/fboss2/session/ConfigSession.h b/fboss/cli/fboss2/session/ConfigSession.h index b07e9962c1702..7838262eaae35 100644 --- a/fboss/cli/fboss2/session/ConfigSession.h +++ b/fboss/cli/fboss2/session/ConfigSession.h @@ -93,12 +93,20 @@ 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 // successful. int commit(const HostInfo& hostInfo); + // Rollback to a specific revision or to the previous revision + // Returns the revision that was rolled back to + int rollback(const HostInfo& hostInfo); + int rollback(const HostInfo& hostInfo, const std::string& revision); + // Check if a session exists bool sessionExists() const; @@ -124,6 +132,9 @@ class ConfigSession { const std::string& systemConfigPath, const std::string& cliConfigDir); + // Set the singleton instance (for testing only) + static void setInstance(std::unique_ptr instance); + private: std::string sessionConfigPath_; std::string systemConfigPath_; diff --git a/fboss/cli/fboss2/test/BUCK b/fboss/cli/fboss2/test/BUCK index f15b5acd5fe8b..b13fe353aa698 100644 --- a/fboss/cli/fboss2/test/BUCK +++ b/fboss/cli/fboss2/test/BUCK @@ -56,7 +56,9 @@ cpp_unittest( name = "cmd_test", srcs = [ "CmdConfigAppliedInfoTest.cpp", + "CmdConfigHistoryTest.cpp", "CmdConfigReloadTest.cpp", + "CmdConfigSessionDiffTest.cpp", "CmdConfigSessionTest.cpp", "CmdGetPcapTest.cpp", "CmdSetPortStateTest.cpp", diff --git a/fboss/cli/fboss2/test/CmdConfigHistoryTest.cpp b/fboss/cli/fboss2/test/CmdConfigHistoryTest.cpp new file mode 100644 index 0000000000000..5db56df97a947 --- /dev/null +++ b/fboss/cli/fboss2/test/CmdConfigHistoryTest.cpp @@ -0,0 +1,263 @@ +// (c) Facebook, Inc. and its affiliates. Confidential and proprietary. + +#include +#include +#include +#include + +#include "fboss/cli/fboss2/commands/config/history/CmdConfigHistory.h" +#include "fboss/cli/fboss2/session/ConfigSession.h" +#include "fboss/cli/fboss2/test/CmdHandlerTestBase.h" +#include "fboss/cli/fboss2/test/TestableConfigSession.h" +#include "fboss/cli/fboss2/utils/PortMap.h" + +#include +#include + +namespace fs = std::filesystem; + +using namespace ::testing; + +namespace facebook::fboss { + +class CmdConfigHistoryTestFixture : public CmdHandlerTestBase { + public: + fs::path testHomeDir_; + fs::path testEtcDir_; + fs::path systemConfigPath_; + fs::path sessionConfigPath_; + fs::path cliConfigDir_; + + void SetUp() override { + CmdHandlerTestBase::SetUp(); + + // Create unique test directories for each test to avoid conflicts when + // running tests in parallel + auto tempBase = fs::temp_directory_path(); + auto uniquePath = boost::filesystem::unique_path( + "fboss2_config_history_test_%%%%-%%%%-%%%%-%%%%"); + testHomeDir_ = tempBase / (uniquePath.string() + "_home"); + testEtcDir_ = tempBase / (uniquePath.string() + "_etc"); + + // Clean up any previous test artifacts (shouldn't exist with unique names) + std::error_code ec; + fs::remove_all(testHomeDir_, ec); + fs::remove_all(testEtcDir_, ec); + + // Create fresh test directories + fs::create_directories(testHomeDir_); + fs::create_directories(testEtcDir_); + + // Set up paths + systemConfigPath_ = testEtcDir_ / "agent.conf"; + sessionConfigPath_ = testHomeDir_ / ".fboss2" / "agent.conf"; + cliConfigDir_ = testEtcDir_ / "coop" / "cli"; + + // Create CLI config directory + fs::create_directories(cliConfigDir_); + + // Create a default system config + createTestConfig( + systemConfigPath_, + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 2, + "speed": 100000 + } + ] + } +})"); + } + + void TearDown() override { + // Reset the singleton to ensure tests don't interfere with each other + TestableConfigSession::setInstance(nullptr); + // Clean up test directories (use error_code to avoid exceptions) + std::error_code ec; + fs::remove_all(testHomeDir_, ec); + fs::remove_all(testEtcDir_, ec); + CmdHandlerTestBase::TearDown(); + } + + void createTestConfig(const fs::path& path, const std::string& content) { + fs::create_directories(path.parent_path()); + std::ofstream file(path); + file << content; + file.close(); + } + + std::string readFile(const fs::path& path) { + std::ifstream file(path); + std::stringstream buffer; + buffer << file.rdbuf(); + return buffer.str(); + } + + void initializeTestSession() { + // Ensure system config exists before initializing session + if (!fs::exists(systemConfigPath_)) { + createTestConfig( + systemConfigPath_, + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 2, + "speed": 100000 + } + ] + } +})"); + } + + // Ensure the parent directory of session config exists + fs::create_directories(sessionConfigPath_.parent_path()); + + // Initialize ConfigSession singleton with test paths + TestableConfigSession::setInstance( + std::make_unique( + sessionConfigPath_.string(), + systemConfigPath_.string(), + cliConfigDir_.string())); + } +}; + +TEST_F(CmdConfigHistoryTestFixture, historyListsRevisions) { + // Create revision files with valid config content + createTestConfig( + cliConfigDir_ / "agent-r1.conf", + R"({"sw": {"ports": [{"logicalID": 1, "name": "eth1/1/1", "state": 2, "speed": 100000}]}})"); + createTestConfig( + cliConfigDir_ / "agent-r2.conf", + R"({"sw": {"ports": [{"logicalID": 1, "name": "eth1/1/1", "state": 2, "speed": 100000}]}})"); + createTestConfig( + cliConfigDir_ / "agent-r3.conf", + R"({"sw": {"ports": [{"logicalID": 1, "name": "eth1/1/1", "state": 2, "speed": 100000}]}})"); + + // Initialize ConfigSession singleton with test paths + initializeTestSession(); + + // Create and execute the command + auto cmd = CmdConfigHistory(); + auto result = cmd.queryClient(localhost()); + + // Verify the output contains all three revisions + EXPECT_NE(result.find("r1"), std::string::npos); + EXPECT_NE(result.find("r2"), std::string::npos); + EXPECT_NE(result.find("r3"), std::string::npos); + + // Verify table headers are present + EXPECT_NE(result.find("Revision"), std::string::npos); + EXPECT_NE(result.find("Owner"), std::string::npos); + EXPECT_NE(result.find("Commit Time"), std::string::npos); +} + +TEST_F(CmdConfigHistoryTestFixture, historyIgnoresNonMatchingFiles) { + // Create valid revision files + createTestConfig( + cliConfigDir_ / "agent-r1.conf", + R"({"sw": {"ports": [{"logicalID": 1, "name": "eth1/1/1", "state": 2, "speed": 100000}]}})"); + createTestConfig( + cliConfigDir_ / "agent-r2.conf", + R"({"sw": {"ports": [{"logicalID": 1, "name": "eth1/1/1", "state": 2, "speed": 100000}]}})"); + + // Create files that should be ignored + createTestConfig(cliConfigDir_ / "agent.conf.bak", R"({"backup": true})"); + createTestConfig(cliConfigDir_ / "other-r1.conf", R"({"other": true})"); + createTestConfig(cliConfigDir_ / "agent-r1.txt", R"({"wrong_ext": true})"); + createTestConfig(cliConfigDir_ / "agent-rX.conf", R"({"invalid": true})"); + + // Initialize ConfigSession singleton with test paths + initializeTestSession(); + + // Create and execute the command + auto cmd = CmdConfigHistory(); + auto result = cmd.queryClient(localhost()); + + // Verify only valid revisions are listed + EXPECT_NE(result.find("r1"), std::string::npos); + EXPECT_NE(result.find("r2"), std::string::npos); + + // Verify invalid files are not listed + EXPECT_EQ(result.find("agent.conf.bak"), std::string::npos); + EXPECT_EQ(result.find("other-r1.conf"), std::string::npos); + EXPECT_EQ(result.find("agent-r1.txt"), std::string::npos); + EXPECT_EQ(result.find("rX"), std::string::npos); +} + +TEST_F(CmdConfigHistoryTestFixture, historyEmptyDirectory) { + // Directory exists but has no revision files + EXPECT_TRUE(fs::exists(cliConfigDir_)); + + // Initialize ConfigSession singleton with test paths + initializeTestSession(); + + // Create and execute the command + auto cmd = CmdConfigHistory(); + auto result = cmd.queryClient(localhost()); + + // Verify the output indicates no revisions found + EXPECT_NE(result.find("No config revisions found"), std::string::npos); + EXPECT_NE(result.find(cliConfigDir_.string()), std::string::npos); +} + +TEST_F(CmdConfigHistoryTestFixture, historyNonSequentialRevisions) { + // Create non-sequential revision files (e.g., after deletions) + createTestConfig( + cliConfigDir_ / "agent-r1.conf", + R"({"sw": {"ports": [{"logicalID": 1, "name": "eth1/1/1", "state": 2, "speed": 100000}]}})"); + createTestConfig( + cliConfigDir_ / "agent-r5.conf", + R"({"sw": {"ports": [{"logicalID": 1, "name": "eth1/1/1", "state": 2, "speed": 100000}]}})"); + createTestConfig( + cliConfigDir_ / "agent-r10.conf", + R"({"sw": {"ports": [{"logicalID": 1, "name": "eth1/1/1", "state": 2, "speed": 100000}]}})"); + + // Initialize ConfigSession singleton with test paths + initializeTestSession(); + + // Create and execute the command + auto cmd = CmdConfigHistory(); + auto result = cmd.queryClient(localhost()); + + // Verify all revisions are listed in order + EXPECT_NE(result.find("r1"), std::string::npos); + EXPECT_NE(result.find("r5"), std::string::npos); + EXPECT_NE(result.find("r10"), std::string::npos); + + // Verify they appear in ascending order (r1 before r5 before r10) + auto pos_r1 = result.find("r1"); + auto pos_r5 = result.find("r5"); + auto pos_r10 = result.find("r10"); + EXPECT_LT(pos_r1, pos_r5); + EXPECT_LT(pos_r5, pos_r10); +} + +TEST_F(CmdConfigHistoryTestFixture, printOutput) { + auto cmd = CmdConfigHistory(); + std::string tableOutput = + "Revision Owner Commit Time\nr1 user1 2024-01-01 12:00:00.000"; + + // Redirect cout to capture output + std::stringstream buffer; + std::streambuf* old = std::cout.rdbuf(buffer.rdbuf()); + + cmd.printOutput(tableOutput); + + // Restore cout + std::cout.rdbuf(old); + + std::string output = buffer.str(); + + EXPECT_NE(output.find("Revision"), std::string::npos); + EXPECT_NE(output.find("r1"), std::string::npos); + EXPECT_NE(output.find("user1"), std::string::npos); +} + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/test/CmdConfigSessionDiffTest.cpp b/fboss/cli/fboss2/test/CmdConfigSessionDiffTest.cpp new file mode 100644 index 0000000000000..8488e18815854 --- /dev/null +++ b/fboss/cli/fboss2/test/CmdConfigSessionDiffTest.cpp @@ -0,0 +1,376 @@ +// (c) Facebook, Inc. and its affiliates. Confidential and proprietary. + +#include +#include +#include +#include + +#include "fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.h" +#include "fboss/cli/fboss2/session/ConfigSession.h" +#include "fboss/cli/fboss2/test/CmdHandlerTestBase.h" +#include "fboss/cli/fboss2/test/TestableConfigSession.h" +#include "fboss/cli/fboss2/utils/CmdUtils.h" +#include "fboss/cli/fboss2/utils/PortMap.h" + +#include +#include + +namespace fs = std::filesystem; + +using namespace ::testing; + +namespace facebook::fboss { + +class CmdConfigSessionDiffTestFixture : public CmdHandlerTestBase { + public: + fs::path testHomeDir_; + fs::path testEtcDir_; + fs::path systemConfigPath_; + fs::path sessionConfigPath_; + fs::path cliConfigDir_; + + void SetUp() override { + CmdHandlerTestBase::SetUp(); + + // Create unique test directories for each test to avoid conflicts when + // running tests in parallel + auto tempBase = fs::temp_directory_path(); + auto uniquePath = boost::filesystem::unique_path( + "fboss2_config_session_diff_test_%%%%-%%%%-%%%%-%%%%"); + testHomeDir_ = tempBase / (uniquePath.string() + "_home"); + testEtcDir_ = tempBase / (uniquePath.string() + "_etc"); + + // Clean up any previous test artifacts (shouldn't exist with unique names) + std::error_code ec; + fs::remove_all(testHomeDir_, ec); + fs::remove_all(testEtcDir_, ec); + + // Create fresh test directories + fs::create_directories(testHomeDir_); + fs::create_directories(testEtcDir_); + + // Set up paths + systemConfigPath_ = testEtcDir_ / "agent.conf"; + sessionConfigPath_ = testHomeDir_ / ".fboss2" / "agent.conf"; + cliConfigDir_ = testEtcDir_ / "coop" / "cli"; + + // Create CLI config directory + fs::create_directories(cliConfigDir_); + + // Create a default system config + createTestConfig( + systemConfigPath_, + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 2, + "speed": 100000 + } + ] + } +})"); + } + + void TearDown() override { + // Reset the singleton to ensure tests don't interfere with each other + TestableConfigSession::setInstance(nullptr); + // Clean up test directories (use error_code to avoid exceptions) + std::error_code ec; + fs::remove_all(testHomeDir_, ec); + fs::remove_all(testEtcDir_, ec); + CmdHandlerTestBase::TearDown(); + } + + void createTestConfig(const fs::path& path, const std::string& content) { + fs::create_directories(path.parent_path()); + std::ofstream file(path); + file << content; + file.close(); + } + + std::string readFile(const fs::path& path) { + std::ifstream file(path); + std::stringstream buffer; + buffer << file.rdbuf(); + return buffer.str(); + } + + void initializeTestSession() { + // Ensure system config exists before initializing session + // (ConfigSession constructor calls initializeSession which will copy it) + if (!fs::exists(systemConfigPath_)) { + createTestConfig( + systemConfigPath_, + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 2, + "speed": 100000 + } + ] + } +})"); + } + + // Ensure the parent directory of session config exists + // (initializeSession will try to copy system config to session config) + fs::create_directories(sessionConfigPath_.parent_path()); + + // Initialize ConfigSession singleton with test paths + // The constructor will automatically call initializeSession() + TestableConfigSession::setInstance( + std::make_unique( + sessionConfigPath_.string(), + systemConfigPath_.string(), + cliConfigDir_.string())); + } +}; + +TEST_F(CmdConfigSessionDiffTestFixture, diffNoSession) { + // Initialize the session (which creates the session config file) + initializeTestSession(); + + // Then delete the session config file to simulate "no session" case + fs::remove(sessionConfigPath_); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList emptyRevisions(std::vector{}); + + auto result = cmd.queryClient(localhost(), emptyRevisions); + + EXPECT_EQ(result, "No config session exists. Make a config change first."); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffIdenticalConfigs) { + initializeTestSession(); + + // initializeTestSession() already creates the session config by copying + // the system config, so we don't need to do it again + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList emptyRevisions(std::vector{}); + + auto result = cmd.queryClient(localhost(), emptyRevisions); + + EXPECT_NE( + result.find( + "No differences between current live config and session config"), + std::string::npos); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffDifferentConfigs) { + initializeTestSession(); + + // Create session directory and modify the config + fs::create_directories(sessionConfigPath_.parent_path()); + createTestConfig( + sessionConfigPath_, + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "description": "Modified port", + "state": 2, + "speed": 100000 + } + ] + } +})"); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList emptyRevisions(std::vector{}); + + auto result = cmd.queryClient(localhost(), emptyRevisions); + + // Should show a diff with the added "description" field + EXPECT_NE(result.find("description"), std::string::npos); + EXPECT_NE(result.find("Modified port"), std::string::npos); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffSessionVsRevision) { + initializeTestSession(); + + // Create a revision file + createTestConfig( + cliConfigDir_ / "agent-r1.conf", + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 1 + } + ] + } +})"); + + // Create session with different content + fs::create_directories(sessionConfigPath_.parent_path()); + createTestConfig( + sessionConfigPath_, + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 2 + } + ] + } +})"); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList revisions(std::vector{"r1"}); + + auto result = cmd.queryClient(localhost(), revisions); + + // Should show a diff between r1 and session (state changed from 1 to 2) + EXPECT_NE(result.find("- \"state\": 1"), std::string::npos); + EXPECT_NE(result.find("+ \"state\": 2"), std::string::npos); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffTwoRevisions) { + initializeTestSession(); + + // Create two different revision files + createTestConfig( + cliConfigDir_ / "agent-r1.conf", + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 2 + } + ] + } +})"); + + createTestConfig( + cliConfigDir_ / "agent-r2.conf", + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "description": "Added description", + "state": 2 + } + ] + } +})"); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList revisions(std::vector{"r1", "r2"}); + + auto result = cmd.queryClient(localhost(), revisions); + + // Should show the added "description" field + EXPECT_NE(result.find("description"), std::string::npos); + EXPECT_NE(result.find("Added description"), std::string::npos); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffWithCurrentKeyword) { + initializeTestSession(); + + // Create a revision file + createTestConfig( + cliConfigDir_ / "agent-r1.conf", + R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 1 + } + ] + } +})"); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList revisions(std::vector{"r1", "current"}); + + auto result = cmd.queryClient(localhost(), revisions); + + // Should show a diff between r1 and current system config (state changed from + // 1 to 2, speed added) + EXPECT_NE(result.find("- \"state\": 1"), std::string::npos); + EXPECT_NE(result.find("+ \"state\": 2"), std::string::npos); + EXPECT_NE(result.find("+ \"speed\": 100000"), std::string::npos); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffNonexistentRevision) { + initializeTestSession(); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList revisions(std::vector{"r999"}); + + // Should throw an exception for nonexistent revision + EXPECT_THROW(cmd.queryClient(localhost(), revisions), std::invalid_argument); +} + +TEST_F(CmdConfigSessionDiffTestFixture, diffTooManyArguments) { + initializeTestSession(); + + auto cmd = CmdConfigSessionDiff(); + utils::RevisionList revisions(std::vector{"r1", "r2", "r3"}); + + // Should throw an exception for too many arguments + EXPECT_THROW(cmd.queryClient(localhost(), revisions), std::invalid_argument); +} + +TEST_F(CmdConfigSessionDiffTestFixture, printOutput) { + auto cmd = CmdConfigSessionDiff(); + std::string diffOutput = + "--- a/config\n+++ b/config\n@@ -1 +1 @@\n-old\n+new"; + + // Redirect cout to capture output + std::stringstream buffer; + std::streambuf* old = std::cout.rdbuf(buffer.rdbuf()); + + cmd.printOutput(diffOutput); + + // Restore cout + std::cout.rdbuf(old); + + std::string output = buffer.str(); + + EXPECT_NE(output.find("--- a/config"), std::string::npos); + EXPECT_NE(output.find("+++ b/config"), std::string::npos); + EXPECT_NE(output.find("-old"), std::string::npos); + EXPECT_NE(output.find("+new"), std::string::npos); +} + +TEST_F(CmdConfigSessionDiffTestFixture, printOutputNoDifferences) { + auto cmd = CmdConfigSessionDiff(); + std::string noDiffMessage = + "No differences between current live config and session config."; + + // Redirect cout to capture output + std::stringstream buffer; + std::streambuf* old = std::cout.rdbuf(buffer.rdbuf()); + + cmd.printOutput(noDiffMessage); + + // Restore cout + std::cout.rdbuf(old); + + std::string output = buffer.str(); + + EXPECT_NE(output.find("No differences"), std::string::npos); +} + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/test/CmdConfigSessionTest.cpp b/fboss/cli/fboss2/test/CmdConfigSessionTest.cpp index 8b08a3ca1c950..1dfb390be1ef0 100644 --- a/fboss/cli/fboss2/test/CmdConfigSessionTest.cpp +++ b/fboss/cli/fboss2/test/CmdConfigSessionTest.cpp @@ -530,4 +530,111 @@ TEST_F(ConfigSessionTestFixture, revisionNumberExtraction) { 999); } +TEST_F(ConfigSessionTestFixture, rollbackCreatesNewRevision) { + // This test actually calls the rollback() method with a specific revision + fs::path cliConfigDir = testEtcDir_ / "coop" / "cli"; + fs::path symlinkPath = testEtcDir_ / "coop" / "agent.conf"; + fs::path sessionConfigPath = testHomeDir_ / ".fboss2" / "agent.conf"; + + // Remove the regular file created by SetUp + if (fs::exists(symlinkPath)) { + fs::remove(symlinkPath); + } + + // Create revision files (simulating previous commits) + createTestConfig(cliConfigDir / "agent-r1.conf", R"({"revision": 1})"); + createTestConfig(cliConfigDir / "agent-r2.conf", R"({"revision": 2})"); + createTestConfig(cliConfigDir / "agent-r3.conf", R"({"revision": 3})"); + + // Create symlink pointing to r3 (current revision) + fs::create_symlink(cliConfigDir / "agent-r3.conf", symlinkPath); + + // Verify initial state + EXPECT_TRUE(fs::is_symlink(symlinkPath)); + EXPECT_EQ(fs::read_symlink(symlinkPath), cliConfigDir / "agent-r3.conf"); + + // Setup mock agent server + setupMockedAgentServer(); + + // Expect reloadConfig to be called once + EXPECT_CALL(getMockAgent(), reloadConfig()).Times(1); + + // Create a testable ConfigSession with test paths + TestableConfigSession session( + sessionConfigPath.string(), symlinkPath.string(), cliConfigDir.string()); + + // Call the actual rollback method to rollback to r1 + int newRevision = session.rollback(localhost(), "r1"); + + // Verify rollback created a new revision (r4) + EXPECT_EQ(newRevision, 4); + EXPECT_TRUE(fs::is_symlink(symlinkPath)); + EXPECT_EQ(fs::read_symlink(symlinkPath), cliConfigDir / "agent-r4.conf"); + EXPECT_TRUE(fs::exists(cliConfigDir / "agent-r4.conf")); + + // Verify r4 has same content as r1 (the target revision) + EXPECT_EQ( + readFile(cliConfigDir / "agent-r1.conf"), + readFile(cliConfigDir / "agent-r4.conf")); + + // Verify old revisions still exist (rollback doesn't delete history) + EXPECT_TRUE(fs::exists(cliConfigDir / "agent-r1.conf")); + EXPECT_TRUE(fs::exists(cliConfigDir / "agent-r2.conf")); + EXPECT_TRUE(fs::exists(cliConfigDir / "agent-r3.conf")); +} + +TEST_F(ConfigSessionTestFixture, rollbackToPreviousRevision) { + // This test actually calls the rollback() method without a revision argument + // to rollback to the previous revision + fs::path cliConfigDir = testEtcDir_ / "coop" / "cli"; + fs::path symlinkPath = testEtcDir_ / "coop" / "agent.conf"; + fs::path sessionConfigPath = testHomeDir_ / ".fboss2" / "agent.conf"; + + // Remove the regular file created by SetUp + if (fs::exists(symlinkPath)) { + fs::remove(symlinkPath); + } + + // Create revision files (simulating previous commits) + createTestConfig(cliConfigDir / "agent-r1.conf", R"({"revision": 1})"); + createTestConfig(cliConfigDir / "agent-r2.conf", R"({"revision": 2})"); + createTestConfig(cliConfigDir / "agent-r3.conf", R"({"revision": 3})"); + + // Create symlink pointing to r3 (current revision) + fs::create_symlink(cliConfigDir / "agent-r3.conf", symlinkPath); + + // Verify initial state + EXPECT_TRUE(fs::is_symlink(symlinkPath)); + EXPECT_EQ(fs::read_symlink(symlinkPath), cliConfigDir / "agent-r3.conf"); + + // Setup mock agent server + setupMockedAgentServer(); + + // Expect reloadConfig to be called once + EXPECT_CALL(getMockAgent(), reloadConfig()).Times(1); + + // Create a testable ConfigSession with test paths + TestableConfigSession session( + sessionConfigPath.string(), symlinkPath.string(), cliConfigDir.string()); + + // Call the actual rollback method without a revision (should go to previous) + int newRevision = session.rollback(localhost()); + + // Verify rollback to previous revision created r4 with content from r2 + EXPECT_EQ(newRevision, 4); + EXPECT_TRUE(fs::is_symlink(symlinkPath)); + EXPECT_EQ(fs::read_symlink(symlinkPath), cliConfigDir / "agent-r4.conf"); + EXPECT_TRUE(fs::exists(cliConfigDir / "agent-r4.conf")); + + // Verify r4 has same content as r2 (the previous revision) + EXPECT_EQ( + readFile(cliConfigDir / "agent-r2.conf"), + readFile(cliConfigDir / "agent-r4.conf")); + + // Verify old revisions still exist (rollback doesn't delete history) + EXPECT_TRUE(fs::exists(cliConfigDir / "agent-r1.conf")); + EXPECT_TRUE(fs::exists(cliConfigDir / "agent-r2.conf")); + EXPECT_TRUE(fs::exists(cliConfigDir / "agent-r3.conf")); +} + } // namespace facebook::fboss diff --git a/fboss/cli/fboss2/test/TestableConfigSession.h b/fboss/cli/fboss2/test/TestableConfigSession.h index 4838656d3db37..1f2179dcf6c10 100644 --- a/fboss/cli/fboss2/test/TestableConfigSession.h +++ b/fboss/cli/fboss2/test/TestableConfigSession.h @@ -23,6 +23,9 @@ class TestableConfigSession : public ConfigSession { const std::string& systemConfigPath, const std::string& cliConfigDir) : ConfigSession(sessionConfigPath, systemConfigPath, cliConfigDir) {} + + // Expose protected setInstance() for testing + using ConfigSession::setInstance; }; } // namespace facebook::fboss diff --git a/fboss/cli/fboss2/utils/CmdUtils.cpp b/fboss/cli/fboss2/utils/CmdUtils.cpp index fba8013585a84..3a3b43ec16096 100644 --- a/fboss/cli/fboss2/utils/CmdUtils.cpp +++ b/fboss/cli/fboss2/utils/CmdUtils.cpp @@ -473,4 +473,41 @@ getUncachedSwitchReachabilityInfo( return reachabilityMatrix; } +RevisionList::RevisionList(std::vector v) { + // Validate each revision specifier + for (const auto& revision : v) { + if (revision == "current") { + // "current" is always valid + data_.push_back(revision); + continue; + } + + // Must be in the form "rN" where N is a positive integer + if (revision.empty() || revision[0] != 'r') { + throw std::invalid_argument( + "Invalid revision specifier: '" + revision + + "'. Expected 'rN' or 'current'"); + } + + // Extract the number part after 'r' + std::string revNum = revision.substr(1); + if (revNum.empty()) { + throw std::invalid_argument( + "Invalid revision specifier: '" + revision + + "'. Expected 'rN' or 'current'"); + } + + // Validate that it's all digits + for (char c : revNum) { + if (!std::isdigit(c)) { + throw std::invalid_argument( + "Invalid revision number: '" + revision + + "'. Expected 'rN' or 'current'"); + } + } + + data_.push_back(revision); + } +} + } // namespace facebook::fboss::utils diff --git a/fboss/cli/fboss2/utils/CmdUtils.h b/fboss/cli/fboss2/utils/CmdUtils.h index 03ce47b283bf9..fed01fabc6855 100644 --- a/fboss/cli/fboss2/utils/CmdUtils.h +++ b/fboss/cli/fboss2/utils/CmdUtils.h @@ -417,6 +417,15 @@ class MirrorList : public BaseObjectArgType { ObjectArgTypeId::OBJECT_ARG_TYPE_ID_MIRROR_LIST; }; +class RevisionList : public BaseObjectArgType { + public: + /* implicit */ RevisionList() : BaseObjectArgType() {} + /* implicit */ RevisionList(std::vector v); + + const static ObjectArgTypeId id = + ObjectArgTypeId::OBJECT_ARG_TYPE_ID_REVISION_LIST; +}; + // Called after CLI11 is initlized but before parsing, for any final // initialization steps void postAppInit(int argc, char* argv[], CLI::App& app); diff --git a/fboss/cli/fboss2/utils/CmdUtilsCommon.h b/fboss/cli/fboss2/utils/CmdUtilsCommon.h index 5987c6bd72e81..340b77dde69ee 100644 --- a/fboss/cli/fboss2/utils/CmdUtilsCommon.h +++ b/fboss/cli/fboss2/utils/CmdUtilsCommon.h @@ -59,6 +59,7 @@ enum class ObjectArgTypeId : uint8_t { OBJECT_ARG_TYPE_ID_MIRROR_LIST, OBJECT_ARG_TYPE_LINK_DIRECTION, OBJECT_ARG_TYPE_FAN_PWM, + OBJECT_ARG_TYPE_ID_REVISION_LIST, }; template