Skip to content

[Nexthop][fboss2-dev] Replace file-based config versioning with Git #825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
benoit-nexthop wants to merge 1 commit into from
+1,544 −1,067
Closed

[Nexthop][fboss2-dev] Replace file-based config versioning with Git #825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmake/CliFboss2.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,8 @@ add_library(fboss2_config_lib
fboss/cli/fboss2/commands/config/session/CmdConfigSessionDiff.cpp
fboss/cli/fboss2/session/ConfigSession.h
fboss/cli/fboss2/session/ConfigSession.cpp
fboss/cli/fboss2/session/Git.h
fboss/cli/fboss2/session/Git.cpp
fboss/cli/fboss2/utils/InterfaceList.h
fboss/cli/fboss2/utils/InterfaceList.cpp
fboss/cli/fboss2/CmdListConfig.cpp
Expand Down
1 change: 1 addition & 0 deletions cmake/CliFboss2Test.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ add_executable(fboss2_cmd_test
# fboss/cli/fboss2/test/CmdShowTransceiverTest.cpp - excluded (depends on configerator bgp namespace)
fboss/cli/fboss2/test/CmdStartPcapTest.cpp
fboss/cli/fboss2/test/CmdStopPcapTest.cpp
fboss/cli/fboss2/test/GitTest.cpp
fboss/cli/fboss2/test/PortMapTest.cpp
)

Expand Down
2 changes: 2 additions & 0 deletions fboss/cli/fboss2/BUCK
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,7 @@ cpp_library(
"commands/config/session/CmdConfigSessionCommit.cpp",
"commands/config/session/CmdConfigSessionDiff.cpp",
"session/ConfigSession.cpp",
"session/Git.cpp",
"utils/InterfaceList.cpp",
],
headers = [
Expand Down Expand Up @@ -992,6 +993,7 @@ cpp_library(
"commands/config/session/CmdConfigSessionCommit.h",
"commands/config/session/CmdConfigSessionDiff.h",
"session/ConfigSession.h",
"session/Git.h",
"utils/InterfaceList.h",
],
exported_deps = [
Expand Down
131 changes: 19 additions & 112 deletions fboss/cli/fboss2/commands/config/history/CmdConfigHistory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,144 +9,51 @@
*/

#include "fboss/cli/fboss2/commands/config/history/CmdConfigHistory.h"
#include <fcntl.h>
#include <pwd.h>
#include <sys/stat.h>
#include <cstdint>
#include <ctime>
#include <filesystem>
#include <iomanip>
#include <sstream>
#include <vector>
#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];
// Format Unix timestamp (seconds) as a human-readable string
std::string formatTime(int64_t timeSec) {
char buffer[32];
tm timeinfo{};
localtime_r(&timeSec, &timeinfo);
std::time_t time = timeSec;
localtime_r(&time, &timeinfo);
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<RevisionInfo> collectRevisions(const std::string& cliConfigDir) {
std::vector<RevisionInfo> 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<int64_t>(stx.stx_btime.tv_sec) * 1000000000 +
stx.stx_btime.tv_nsec;
} else {
info.commitTimeNsec =
static_cast<int64_t>(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;
return buffer;
}

} // namespace

CmdConfigHistoryTraits::RetType CmdConfigHistory::queryClient(
const HostInfo& /* hostInfo */) {
auto& session = ConfigSession::getInstance();
const std::string cliConfigDir = session.getCliConfigDir();
auto& git = session.getGit();

auto revisions = collectRevisions(cliConfigDir);
// Get the commit history from Git for the CLI config file
auto commits = git.log(session.getCliConfigPath());

if (revisions.empty()) {
return "No config revisions found in " + cliConfigDir;
if (commits.empty()) {
return "No config revisions found in Git history";
}

// Build the table
utils::Table table;
table.setHeader({"Revision", "Owner", "Commit Time"});
table.setHeader({"Commit", "Author", "Commit Time", "Message"});

for (const auto& rev : revisions) {
for (const auto& commit : commits) {
// Use short SHA1 (first 8 characters)
std::string shortSha = commit.sha1.substr(0, 8);
table.addRow(
{"r" + std::to_string(rev.revisionNumber),
rev.owner,
formatTime(rev.commitTimeNsec)});
{shortSha,
commit.authorName,
formatTime(commit.timestamp),
commit.subject});
}

// Convert table to string
Expand Down
20 changes: 10 additions & 10 deletions fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,28 @@ CmdConfigRollbackTraits::RetType CmdConfigRollback::queryClient(
// Validate arguments
if (revisions.size() > 1) {
throw std::invalid_argument(
"Too many arguments. Expected 0 or 1 revision specifier.");
"Too many arguments. Expected 0 or 1 commit SHA.");
}

if (!revisions.empty() && revisions[0] == "current") {
throw std::invalid_argument(
"Cannot rollback to 'current'. Please specify a revision number like 'r42'.");
"Cannot rollback to 'current'. Please specify a commit SHA.");
}

try {
int newRevision;
std::string newCommitSha;
if (revisions.empty()) {
// No revision specified - rollback to previous revision
newRevision = session.rollback(hostInfo);
newCommitSha = session.rollback(hostInfo);
} else {
// Specific revision specified
newRevision = session.rollback(hostInfo, revisions[0]);
// Specific commit SHA specified
newCommitSha = session.rollback(hostInfo, revisions[0]);
}
if (newRevision) {
return "Successfully rolled back to r" + std::to_string(newRevision) +
" and config reloaded.";
if (!newCommitSha.empty()) {
return "Successfully rolled back. New commit: " +
newCommitSha.substr(0, 8) + ". Config reloaded.";
} else {
return "Failed to create a new revision after rollback.";
return "Failed to create a new commit after rollback.";
}
} catch (const std::exception& ex) {
throw std::runtime_error(
Expand Down
17 changes: 9 additions & 8 deletions fboss/cli/fboss2/commands/config/session/CmdConfigSessionCommit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,23 +48,24 @@ CmdConfigSessionCommitTraits::RetType CmdConfigSessionCommit::queryClient(

// Build message based on what actions were taken
std::string message;
std::string shortSha = result.commitSha.substr(0, 7);
Copy link
Contributor

@joseph5wu joseph5wu Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to see a common utility function for getShortSha(const std::string& commitSha).
Or a more organized way to getFullSha() or getShortSha() from the result.
This will avoid mis-using the wrong sha string.
Could be addressed in future diff

Copy link
Contributor Author

@benoit-nexthop benoit-nexthop Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, you can always use the long or the short sha1, as long as it's a unique prefix (just like when using git directly). But other than that, yes, we can add a little helper function to clip the string.

if (restartedServices.empty() && reloadedServices.empty()) {
message = fmt::format(
"Config session committed successfully as r{}.", result.revision);
message =
fmt::format("Config session committed successfully as {}.", shortSha);
} else if (restartedServices.empty()) {
message = fmt::format(
"Config session committed successfully as r{} and config reloaded for {}.",
result.revision,
"Config session committed successfully as {} and config reloaded for {}.",
shortSha,
folly::join(", ", reloadedServices));
} else if (reloadedServices.empty()) {
message = fmt::format(
"Config session committed successfully as r{} and {} restarted.",
result.revision,
"Config session committed successfully as {} and {} restarted.",
shortSha,
folly::join(", ", restartedServices));
} else {
message = fmt::format(
"Config session committed successfully as r{}, {} restarted, and config reloaded for {}.",
result.revision,
"Config session committed successfully as {}, {} restarted, and config reloaded for {}.",
shortSha,
folly::join(", ", restartedServices),
folly::join(", ", reloadedServices));
}
Expand Down
Loading
Loading