Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 47 additions & 1 deletion cmake/CliFboss2.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -472,13 +472,15 @@ add_library(fboss2_lib
fboss/cli/fboss2/commands/stop/pcap/CmdStopPcap.h
fboss/cli/fboss2/CmdSubcommands.cpp
fboss/cli/fboss2/oss/CmdGlobalOptions.cpp
fboss/cli/fboss2/oss/CmdList.cpp
fboss/cli/fboss2/utils/CmdUtils.cpp
fboss/cli/fboss2/utils/CLIParserUtils.cpp
fboss/cli/fboss2/utils/CmdClientUtils.cpp
fboss/cli/fboss2/utils/CmdUtilsCommon.cpp
fboss/cli/fboss2/utils/PortMap.cpp
fboss/cli/fboss2/utils/Table.cpp
fboss/cli/fboss2/utils/HostInfo.h
fboss/cli/fboss2/utils/InterfaceList.h
fboss/cli/fboss2/utils/InterfaceList.cpp
fboss/cli/fboss2/utils/FilterOp.h
fboss/cli/fboss2/utils/AggregateOp.h
fboss/cli/fboss2/utils/AggregateUtils.h
Expand Down Expand Up @@ -559,6 +561,7 @@ target_link_libraries(fboss2_lib

add_executable(fboss2
fboss/cli/fboss2/Main.cpp
fboss/cli/fboss2/oss/CmdList.cpp
)

target_link_libraries(fboss2
Expand All @@ -567,3 +570,46 @@ target_link_libraries(fboss2
)

install(TARGETS fboss2)

# Config commands library for fboss2-dev
add_library(fboss2_config_lib
fboss/cli/fboss2/commands/config/CmdConfigAppliedInfo.h
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/interface/CmdConfigInterface.h
fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceDescription.h
fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceDescription.cpp
fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceMtu.h
fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceMtu.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
fboss/cli/fboss2/CmdHandlerImplConfig.cpp
)

target_link_libraries(fboss2_config_lib
fboss2_lib
agent_dir_util
)

add_executable(fboss2-dev
fboss/cli/fboss2/Main.cpp
fboss/cli/fboss2/oss/CmdListConfig.cpp
)

target_link_libraries(fboss2-dev
fboss2_config_lib
fboss2_lib
Folly::folly
)

install(TARGETS fboss2-dev)
9 changes: 9 additions & 0 deletions cmake/CliFboss2Test.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@
# cmd_test - Command tests from BUCK file
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/CmdConfigInterfaceDescriptionTest.cpp
fboss/cli/fboss2/test/CmdConfigInterfaceMtuTest.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/test/CmdShowAgentSslTest.cpp
Expand Down Expand Up @@ -35,10 +42,12 @@ 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/PortMapTest.cpp
)

target_link_libraries(fboss2_cmd_test
fboss2_lib
fboss2_config_lib
${GTEST}
${LIBGMOCK_LIBRARIES}
Folly::folly
Expand Down
61 changes: 61 additions & 0 deletions fboss/cli/fboss2/BUCK
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually don't recommend vendors to touch the BUCK file as they won't be able to verify the changes using buck command.

That's why once we finish landing the PRs, we will adjust the buck file in another diff or the same diff if our on-diff signal complains

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is going to save you time 😁

Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ cpp_library(
name = "cmd-common-utils",
srcs = [
"utils/CmdUtilsCommon.cpp",
"utils/InterfaceList.cpp",
],
headers = [
"commands/clear/CmdClearUtils.h",
Expand All @@ -154,6 +155,7 @@ cpp_library(
"utils/CmdUtilsCommon.h",
"utils/FilterUtils.h",
"utils/HostInfo.h",
"utils/InterfaceList.h",
],
exported_deps = [
":cmd-global-options",
Expand Down Expand Up @@ -758,6 +760,65 @@ auto_tab_complete(
src = ":fboss2",
)

# Config commands are in a separate library/binary for now.
cpp_library(
name = "fboss2-config-lib",
srcs = [
"CmdHandlerImplConfig.cpp",
"CmdListConfig.cpp",
"commands/config/CmdConfigAppliedInfo.cpp",
"commands/config/CmdConfigReload.cpp",
"commands/config/history/CmdConfigHistory.cpp",
"commands/config/interface/CmdConfigInterfaceDescription.cpp",
"commands/config/interface/CmdConfigInterfaceMtu.cpp",
"commands/config/rollback/CmdConfigRollback.cpp",
"commands/config/session/CmdConfigSessionCommit.cpp",
"commands/config/session/CmdConfigSessionDiff.cpp",
"session/ConfigSession.cpp",
"utils/PortMap.cpp",
],
headers = [
"commands/config/CmdConfigAppliedInfo.h",
"commands/config/CmdConfigReload.h",
"commands/config/history/CmdConfigHistory.h",
"commands/config/interface/CmdConfigInterface.h",
"commands/config/interface/CmdConfigInterfaceDescription.h",
"commands/config/interface/CmdConfigInterfaceMtu.h",
"commands/config/rollback/CmdConfigRollback.h",
"commands/config/session/CmdConfigSessionCommit.h",
"commands/config/session/CmdConfigSessionDiff.h",
"session/ConfigSession.h",
"utils/PortMap.h",
],
exported_deps = [
":cmd-handler",
":cmd-list-header",
":fboss2-lib",
"//fboss/agent:agent_dir_util",
],
)

cpp_binary(
name = "fboss2-dev",
srcs = [
"Main.cpp",
"oss/CmdListConfig.cpp",
],
deps = [
":cmd-common-utils",
":cmd-global-options",
":cmd-subcommands",
":fboss2-config-lib",
":fboss2-lib", # @manual
"//folly/init:init",
"//folly/logging:init",
"//folly/logging:logging",
],
external_deps = [
"CLI11",
],
)

cpp_binary(
name = "fboss2-routing-protocol",
srcs = [
Expand Down
2 changes: 2 additions & 0 deletions fboss/cli/fboss2/CmdHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ class CmdHandler {
RetType result;
try {
result = queryClientHelper(hostInfo);
} catch (std::invalid_argument const& err) {
errStr = folly::to<std::string>("Invalid argument: ", err.what());
} catch (std::exception const& err) {
errStr = folly::to<std::string>("Thrift call failed: '", err.what(), "'");
}
Expand Down
41 changes: 41 additions & 0 deletions fboss/cli/fboss2/CmdHandlerImplConfig.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* 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/CmdHandler.cpp"

#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/interface/CmdConfigInterface.h"
#include "fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceDescription.h"
#include "fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceMtu.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<CmdConfigAppliedInfo, CmdConfigAppliedInfoTraits>::run();
template void CmdHandler<CmdConfigReload, CmdConfigReloadTraits>::run();
template void CmdHandler<CmdConfigInterface, CmdConfigInterfaceTraits>::run();
template void CmdHandler<
CmdConfigInterfaceDescription,
CmdConfigInterfaceDescriptionTraits>::run();
template void
CmdHandler<CmdConfigInterfaceMtu, CmdConfigInterfaceMtuTraits>::run();
template void CmdHandler<CmdConfigHistory, CmdConfigHistoryTraits>::run();
template void CmdHandler<CmdConfigRollback, CmdConfigRollbackTraits>::run();
template void
CmdHandler<CmdConfigSessionCommit, CmdConfigSessionCommitTraits>::run();
template void
CmdHandler<CmdConfigSessionDiff, CmdConfigSessionDiffTraits>::run();

} // namespace facebook::fboss
94 changes: 94 additions & 0 deletions fboss/cli/fboss2/CmdListConfig.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* 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/CmdList.h"

#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/interface/CmdConfigInterface.h"
#include "fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceDescription.h"
#include "fboss/cli/fboss2/commands/config/interface/CmdConfigInterfaceMtu.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 {

const CommandTree& kConfigCommandTree() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I can see this function will become massive once we keep adding more and more sub-commands.
Maybe we can adjust the returned CommandTree as:

root = {
  // Config related subcommands, like applied-info/ history/ reload/ rollback
  // Session control subcommands
}
// make the sub class to manage the sub CommandTree so we don't have to update here every time whether there's a new sub command
root.add(CmdConfigInterface.getCommandTree()); 
root.add(CmdConfigXXX.getCommandTree());
return root;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would suggest refactoring this in a separate change. We already have another change internally that is going to avoid mixing everything together like that.

static CommandTree root = {
{"config",
"applied-info",
"Show config applied information",
commandHandler<CmdConfigAppliedInfo>,
argTypeHandler<CmdConfigAppliedInfoTraits>},

{"config",
"history",
"Show history of committed config revisions",
commandHandler<CmdConfigHistory>,
argTypeHandler<CmdConfigHistoryTraits>},

{
"config",
"interface",
"Configure interface settings",
commandHandler<CmdConfigInterface>,
argTypeHandler<CmdConfigInterfaceTraits>,
{{
"description",
"Set interface description",
commandHandler<CmdConfigInterfaceDescription>,
argTypeHandler<CmdConfigInterfaceDescriptionTraits>,
},
{
"mtu",
"Set interface MTU",
commandHandler<CmdConfigInterfaceMtu>,
argTypeHandler<CmdConfigInterfaceMtuTraits>,
}},
},

{
"config",
"session",
"Manage config session",
{{
"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",
"reload",
"Reload agent configuration",
commandHandler<CmdConfigReload>,
argTypeHandler<CmdConfigReloadTraits>},

{"config",
"rollback",
"Rollback to a previous config revision",
commandHandler<CmdConfigRollback>,
argTypeHandler<CmdConfigRollbackTraits>},
};
sort(root.begin(), root.end());
return root;
}

} // namespace facebook::fboss
12 changes: 12 additions & 0 deletions fboss/cli/fboss2/CmdSubcommands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ const std::map<std::string, std::string>& kSupportedVerbs() {
{"stop", "Stop event"},
{"get", "Get object"},
{"reload", "Reload object"},
// Only implemented in fboss2-dev for now.
{"config", "Configuration commands"},
};

return supportedVerbs;
Expand Down Expand Up @@ -217,6 +219,16 @@ 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_MTU:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about how to better manage the sub-commands and arg types of the new config.
We don't wanna blow up the base class with all the different sub-commands

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would suggest refactoring this in a separate change. We already have another change that is going to avoid mixing everything together like that.

subCmd->add_option("mtu", args, "MTU value (68-9216)");
break;
case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_INTERFACE_LIST:
subCmd->add_option("interfaces", args, "Interface(s)");
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;
Expand Down
Loading
Loading