diff --git a/cmake/CliFboss2.cmake b/cmake/CliFboss2.cmake index d4e37bf5d7a26..8f88d9cdc20a4 100644 --- a/cmake/CliFboss2.cmake +++ b/cmake/CliFboss2.cmake @@ -632,6 +632,10 @@ add_library(fboss2_config_lib 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/interface/switchport/CmdConfigInterfaceSwitchport.h + fboss/cli/fboss2/commands/config/interface/switchport/access/CmdConfigInterfaceSwitchportAccess.h + fboss/cli/fboss2/commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.h + fboss/cli/fboss2/commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.cpp fboss/cli/fboss2/commands/config/qos/CmdConfigQos.h fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPool.cpp fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPool.h diff --git a/cmake/CliFboss2Test.cmake b/cmake/CliFboss2Test.cmake index f3f68a8f20401..fbd094efa1048 100644 --- a/cmake/CliFboss2Test.cmake +++ b/cmake/CliFboss2Test.cmake @@ -38,6 +38,7 @@ add_executable(fboss2_cmd_test fboss/cli/fboss2/test/CmdConfigHistoryTest.cpp fboss/cli/fboss2/test/CmdConfigInterfaceDescriptionTest.cpp fboss/cli/fboss2/test/CmdConfigInterfaceMtuTest.cpp + fboss/cli/fboss2/test/CmdConfigInterfaceSwitchportAccessVlanTest.cpp fboss/cli/fboss2/test/CmdConfigQosBufferPoolTest.cpp fboss/cli/fboss2/test/CmdConfigReloadTest.cpp fboss/cli/fboss2/test/CmdConfigSessionDiffTest.cpp diff --git a/fboss/cli/fboss2/BUCK b/fboss/cli/fboss2/BUCK index 8f0d4dde5a06b..690034ade6cd7 100644 --- a/fboss/cli/fboss2/BUCK +++ b/fboss/cli/fboss2/BUCK @@ -853,6 +853,7 @@ cpp_library( "commands/config/history/CmdConfigHistory.cpp", "commands/config/interface/CmdConfigInterfaceDescription.cpp", "commands/config/interface/CmdConfigInterfaceMtu.cpp", + "commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.cpp", "commands/config/qos/buffer_pool/CmdConfigQosBufferPool.cpp", "commands/config/rollback/CmdConfigRollback.cpp", "commands/config/session/CmdConfigSessionCommit.cpp", @@ -867,6 +868,9 @@ cpp_library( "commands/config/interface/CmdConfigInterface.h", "commands/config/interface/CmdConfigInterfaceDescription.h", "commands/config/interface/CmdConfigInterfaceMtu.h", + "commands/config/interface/switchport/CmdConfigInterfaceSwitchport.h", + "commands/config/interface/switchport/access/CmdConfigInterfaceSwitchportAccess.h", + "commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.h", "commands/config/qos/CmdConfigQos.h", "commands/config/qos/buffer_pool/CmdConfigQosBufferPool.h", "commands/config/rollback/CmdConfigRollback.h", diff --git a/fboss/cli/fboss2/CmdHandlerImplConfig.cpp b/fboss/cli/fboss2/CmdHandlerImplConfig.cpp index 290831c6c86be..515f4dbb239e4 100644 --- a/fboss/cli/fboss2/CmdHandlerImplConfig.cpp +++ b/fboss/cli/fboss2/CmdHandlerImplConfig.cpp @@ -19,6 +19,9 @@ #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/interface/switchport/CmdConfigInterfaceSwitchport.h" +#include "fboss/cli/fboss2/commands/config/interface/switchport/access/CmdConfigInterfaceSwitchportAccess.h" +#include "fboss/cli/fboss2/commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.h" #include "fboss/cli/fboss2/commands/config/qos/CmdConfigQos.h" #include "fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPool.h" #include "fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.h" @@ -36,6 +39,15 @@ template void CmdHandler< CmdConfigInterfaceDescriptionTraits>::run(); template void CmdHandler::run(); +template void CmdHandler< + CmdConfigInterfaceSwitchport, + CmdConfigInterfaceSwitchportTraits>::run(); +template void CmdHandler< + CmdConfigInterfaceSwitchportAccess, + CmdConfigInterfaceSwitchportAccessTraits>::run(); +template void CmdHandler< + CmdConfigInterfaceSwitchportAccessVlan, + CmdConfigInterfaceSwitchportAccessVlanTraits>::run(); template void CmdHandler::run(); template void CmdHandler::run(); template void diff --git a/fboss/cli/fboss2/CmdListConfig.cpp b/fboss/cli/fboss2/CmdListConfig.cpp index 78e4ad709d3ee..697111d4daa13 100644 --- a/fboss/cli/fboss2/CmdListConfig.cpp +++ b/fboss/cli/fboss2/CmdListConfig.cpp @@ -17,6 +17,9 @@ #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/interface/switchport/CmdConfigInterfaceSwitchport.h" +#include "fboss/cli/fboss2/commands/config/interface/switchport/access/CmdConfigInterfaceSwitchportAccess.h" +#include "fboss/cli/fboss2/commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.h" #include "fboss/cli/fboss2/commands/config/qos/CmdConfigQos.h" #include "fboss/cli/fboss2/commands/config/qos/buffer_pool/CmdConfigQosBufferPool.h" #include "fboss/cli/fboss2/commands/config/rollback/CmdConfigRollback.h" @@ -56,6 +59,25 @@ const CommandTree& kConfigCommandTree() { "Set interface MTU", commandHandler, argTypeHandler, + }, + { + "switchport", + "Configure switchport settings", + commandHandler, + argTypeHandler, + {{ + "access", + "Configure access mode settings", + commandHandler, + argTypeHandler, + {{ + "vlan", + "Set access VLAN (ingressVlan) for the interface", + commandHandler, + argTypeHandler< + CmdConfigInterfaceSwitchportAccessVlanTraits>, + }}, + }}, }}, }, diff --git a/fboss/cli/fboss2/CmdSubcommands.cpp b/fboss/cli/fboss2/CmdSubcommands.cpp index 24221d3d6b753..f457b94d73513 100644 --- a/fboss/cli/fboss2/CmdSubcommands.cpp +++ b/fboss/cli/fboss2/CmdSubcommands.cpp @@ -239,6 +239,9 @@ CLI::App* CmdSubcommands::addCommand( " [ ...] where is one " "of: shared-bytes, headroom-bytes, reserved-bytes"); break; + case utils::ObjectArgTypeId::OBJECT_ARG_TYPE_VLAN_ID: + subCmd->add_option("vlan_id", args, "VLAN ID (1-4094)"); + 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/interface/switchport/CmdConfigInterfaceSwitchport.h b/fboss/cli/fboss2/commands/config/interface/switchport/CmdConfigInterfaceSwitchport.h new file mode 100644 index 0000000000000..c32a5eb3e0f32 --- /dev/null +++ b/fboss/cli/fboss2/commands/config/interface/switchport/CmdConfigInterfaceSwitchport.h @@ -0,0 +1,42 @@ +/* + * 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/commands/config/interface/CmdConfigInterface.h" +#include "fboss/cli/fboss2/utils/CmdUtils.h" +#include "fboss/cli/fboss2/utils/InterfaceList.h" + +namespace facebook::fboss { + +struct CmdConfigInterfaceSwitchportTraits : public WriteCommandTraits { + using ParentCmd = CmdConfigInterface; + static constexpr utils::ObjectArgTypeId ObjectArgTypeId = + utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_NONE; + using ObjectArgType = std::monostate; + using RetType = std::string; +}; + +class CmdConfigInterfaceSwitchport : public CmdHandler< + CmdConfigInterfaceSwitchport, + CmdConfigInterfaceSwitchportTraits> { + public: + RetType queryClient( + const HostInfo& /* hostInfo */, + const utils::InterfaceList& /* interfaces */) { + throw std::runtime_error( + "Incomplete command, please use one of the subcommands"); + } + + void printOutput(const RetType& /* model */) {} +}; + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/config/interface/switchport/access/CmdConfigInterfaceSwitchportAccess.h b/fboss/cli/fboss2/commands/config/interface/switchport/access/CmdConfigInterfaceSwitchportAccess.h new file mode 100644 index 0000000000000..0eb3ce70b3556 --- /dev/null +++ b/fboss/cli/fboss2/commands/config/interface/switchport/access/CmdConfigInterfaceSwitchportAccess.h @@ -0,0 +1,43 @@ +/* + * 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/commands/config/interface/switchport/CmdConfigInterfaceSwitchport.h" +#include "fboss/cli/fboss2/utils/CmdUtils.h" +#include "fboss/cli/fboss2/utils/InterfaceList.h" + +namespace facebook::fboss { + +struct CmdConfigInterfaceSwitchportAccessTraits : public WriteCommandTraits { + using ParentCmd = CmdConfigInterfaceSwitchport; + static constexpr utils::ObjectArgTypeId ObjectArgTypeId = + utils::ObjectArgTypeId::OBJECT_ARG_TYPE_ID_NONE; + using ObjectArgType = std::monostate; + using RetType = std::string; +}; + +class CmdConfigInterfaceSwitchportAccess + : public CmdHandler< + CmdConfigInterfaceSwitchportAccess, + CmdConfigInterfaceSwitchportAccessTraits> { + public: + RetType queryClient( + const HostInfo& /* hostInfo */, + const utils::InterfaceList& /* interfaces */) { + throw std::runtime_error( + "Incomplete command, please use one of the subcommands"); + } + + void printOutput(const RetType& /* model */) {} +}; + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.cpp b/fboss/cli/fboss2/commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.cpp new file mode 100644 index 0000000000000..ed32c7c9efbe4 --- /dev/null +++ b/fboss/cli/fboss2/commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.cpp @@ -0,0 +1,71 @@ +/* + * 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/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.h" + +#include + +#include +#include "fboss/cli/fboss2/session/ConfigSession.h" + +namespace facebook::fboss { + +CmdConfigInterfaceSwitchportAccessVlanTraits::RetType +CmdConfigInterfaceSwitchportAccessVlan::queryClient( + const HostInfo& hostInfo, + const utils::InterfaceList& interfaces, + const CmdConfigInterfaceSwitchportAccessVlanTraits::ObjectArgType& + vlanIdValue) { + if (interfaces.empty()) { + throw std::invalid_argument("No interface name provided"); + } + + // Extract the VLAN ID (validation already done in VlanIdValue constructor) + int32_t vlanId = vlanIdValue.getVlanId(); + + // Collect the logical port IDs we need to update + std::unordered_set portIds; + + // Update ingressVlan for all resolved ports + for (const utils::Intf& intf : interfaces) { + cfg::Port* port = intf.getPort(); + if (port) { + port->ingressVlan() = vlanId; + portIds.insert(*port->logicalID()); + } + } + + // Also update the vlanPorts entries for these ports + auto& config = ConfigSession::getInstance().getAgentConfig(); + auto& vlanPorts = *config.sw()->vlanPorts(); + for (auto& vlanPort : vlanPorts) { + if (portIds.count(*vlanPort.logicalPort())) { + vlanPort.vlanID() = vlanId; + } + } + + // Save the updated config + // VLAN changes require agent warmboot as reloadConfig() doesn't apply them + ConfigSession::getInstance().saveConfig( + cli::ServiceType::AGENT, cli::ConfigActionLevel::AGENT_WARMBOOT); + + std::string interfaceList = folly::join(", ", interfaces.getNames()); + std::string message = "Successfully set access VLAN for interface(s) " + + interfaceList + " to " + std::to_string(vlanId); + + return message; +} + +void CmdConfigInterfaceSwitchportAccessVlan::printOutput( + const CmdConfigInterfaceSwitchportAccessVlanTraits::RetType& logMsg) { + std::cout << logMsg << std::endl; +} + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.h b/fboss/cli/fboss2/commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.h new file mode 100644 index 0000000000000..e2af310574b50 --- /dev/null +++ b/fboss/cli/fboss2/commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.h @@ -0,0 +1,82 @@ +/* + * 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 +#include +#include "fboss/cli/fboss2/CmdHandler.h" +#include "fboss/cli/fboss2/commands/config/interface/switchport/access/CmdConfigInterfaceSwitchportAccess.h" +#include "fboss/cli/fboss2/utils/CmdUtils.h" +#include "fboss/cli/fboss2/utils/InterfaceList.h" + +namespace facebook::fboss { + +// Custom type for VLAN ID argument with validation +class VlanIdValue : public utils::BaseObjectArgType { + public: + /* implicit */ VlanIdValue(std::vector v) { + if (v.empty()) { + throw std::invalid_argument("VLAN ID is required"); + } + if (v.size() != 1) { + throw std::invalid_argument( + "Expected single VLAN ID, got: " + folly::join(", ", v)); + } + + try { + int32_t vlanId = folly::to(v[0]); + // VLAN IDs are typically 1-4094 (0 and 4095 are reserved) + if (vlanId < 1 || vlanId > 4094) { + throw std::invalid_argument( + "VLAN ID must be between 1 and 4094 inclusive, got: " + + std::to_string(vlanId)); + } + data_.push_back(vlanId); + } catch (const folly::ConversionError&) { + throw std::invalid_argument("Invalid VLAN ID: " + v[0]); + } + } + + int32_t getVlanId() const { + return data_[0]; + } + + const static utils::ObjectArgTypeId id = + utils::ObjectArgTypeId::OBJECT_ARG_TYPE_VLAN_ID; +}; + +struct CmdConfigInterfaceSwitchportAccessVlanTraits + : public WriteCommandTraits { + using ParentCmd = CmdConfigInterfaceSwitchportAccess; + static constexpr utils::ObjectArgTypeId ObjectArgTypeId = + utils::ObjectArgTypeId::OBJECT_ARG_TYPE_VLAN_ID; + using ObjectArgType = VlanIdValue; + using RetType = std::string; +}; + +class CmdConfigInterfaceSwitchportAccessVlan + : public CmdHandler< + CmdConfigInterfaceSwitchportAccessVlan, + CmdConfigInterfaceSwitchportAccessVlanTraits> { + public: + using ObjectArgType = + CmdConfigInterfaceSwitchportAccessVlanTraits::ObjectArgType; + using RetType = CmdConfigInterfaceSwitchportAccessVlanTraits::RetType; + + RetType queryClient( + const HostInfo& hostInfo, + const utils::InterfaceList& interfaces, + const ObjectArgType& vlanId); + + void printOutput(const RetType& logMsg); +}; + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/session/ConfigSession.cpp b/fboss/cli/fboss2/session/ConfigSession.cpp index e3eeb347aeb18..c3eceaace2734 100644 --- a/fboss/cli/fboss2/session/ConfigSession.cpp +++ b/fboss/cli/fboss2/session/ConfigSession.cpp @@ -460,7 +460,8 @@ void ConfigSession::restartService( if (level == cli::ConfigActionLevel::AGENT_COLDBOOT) { // Step 1: Stop the service try { - folly::Subprocess stopProc({"/usr/bin/systemctl", "stop", serviceName}); + folly::Subprocess stopProc( + {"/usr/bin/sudo", "/usr/bin/systemctl", "stop", serviceName}); stopProc.waitChecked(); } catch (const std::exception& ex) { throw std::runtime_error( @@ -488,7 +489,8 @@ void ConfigSession::restartService( // Step 3: Start the service try { - folly::Subprocess startProc({"/usr/bin/systemctl", "start", serviceName}); + folly::Subprocess startProc( + {"/usr/bin/sudo", "/usr/bin/systemctl", "start", serviceName}); startProc.waitChecked(); } catch (const std::exception& ex) { throw std::runtime_error( @@ -498,7 +500,7 @@ void ConfigSession::restartService( // For warmboot, just do a simple restart try { folly::Subprocess restartProc( - {"/usr/bin/systemctl", "restart", serviceName}); + {"/usr/bin/sudo", "/usr/bin/systemctl", "restart", serviceName}); restartProc.waitChecked(); } catch (const std::exception& ex) { throw std::runtime_error( diff --git a/fboss/cli/fboss2/test/BUCK b/fboss/cli/fboss2/test/BUCK index e00e3226cc278..a2c7cf4184584 100644 --- a/fboss/cli/fboss2/test/BUCK +++ b/fboss/cli/fboss2/test/BUCK @@ -67,6 +67,7 @@ cpp_unittest( "CmdConfigHistoryTest.cpp", "CmdConfigInterfaceDescriptionTest.cpp", "CmdConfigInterfaceMtuTest.cpp", + "CmdConfigInterfaceSwitchportAccessVlanTest.cpp", "CmdConfigQosBufferPoolTest.cpp", "CmdConfigReloadTest.cpp", "CmdConfigSessionDiffTest.cpp", diff --git a/fboss/cli/fboss2/test/CmdConfigInterfaceSwitchportAccessVlanTest.cpp b/fboss/cli/fboss2/test/CmdConfigInterfaceSwitchportAccessVlanTest.cpp new file mode 100644 index 0000000000000..1b740ca660f6c --- /dev/null +++ b/fboss/cli/fboss2/test/CmdConfigInterfaceSwitchportAccessVlanTest.cpp @@ -0,0 +1,266 @@ +/* + * 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 +#include +#include +#include +#include +#include + +#include "fboss/cli/fboss2/commands/config/interface/switchport/access/vlan/CmdConfigInterfaceSwitchportAccessVlan.h" +#include "fboss/cli/fboss2/test/CmdHandlerTestBase.h" +#include "fboss/cli/fboss2/test/TestableConfigSession.h" +#include "fboss/cli/fboss2/utils/PortMap.h" + +namespace fs = std::filesystem; + +using namespace ::testing; + +namespace facebook::fboss { + +class CmdConfigInterfaceSwitchportAccessVlanTestFixture + : public CmdHandlerTestBase { + public: + void SetUp() override { + CmdHandlerTestBase::SetUp(); + + // Create unique test directories + auto tempBase = fs::temp_directory_path(); + auto uniquePath = boost::filesystem::unique_path( + "fboss_switchport_test_%%%%-%%%%-%%%%-%%%%"); + testHomeDir_ = tempBase / (uniquePath.string() + "_home"); + testEtcDir_ = tempBase / (uniquePath.string() + "_etc"); + + std::error_code ec; + if (fs::exists(testHomeDir_)) { + fs::remove_all(testHomeDir_, ec); + } + if (fs::exists(testEtcDir_)) { + fs::remove_all(testEtcDir_, ec); + } + + // Create test directories + fs::create_directories(testHomeDir_); + fs::create_directories(testEtcDir_ / "coop"); + fs::create_directories(testEtcDir_ / "coop" / "cli"); + + // Set environment variables + setenv("HOME", testHomeDir_.c_str(), 1); + setenv("USER", "testuser", 1); + + // Create a test system config file with ports and vlanPorts + fs::path initialRevision = testEtcDir_ / "coop" / "cli" / "agent-r1.conf"; + createTestConfig(initialRevision, R"({ + "sw": { + "ports": [ + { + "logicalID": 1, + "name": "eth1/1/1", + "state": 2, + "speed": 100000, + "ingressVlan": 1 + }, + { + "logicalID": 2, + "name": "eth1/2/1", + "state": 2, + "speed": 100000, + "ingressVlan": 1 + } + ], + "vlanPorts": [ + { + "vlanID": 1, + "logicalPort": 1, + "spanningTreeState": 2, + "emitTags": false + }, + { + "vlanID": 1, + "logicalPort": 2, + "spanningTreeState": 2, + "emitTags": false + } + ] + } +})"); + + // Create symlink + systemConfigPath_ = testEtcDir_ / "coop" / "agent.conf"; + fs::create_symlink(initialRevision, systemConfigPath_); + + // Create session config path + sessionConfigPath_ = testHomeDir_ / ".fboss2" / "agent.conf"; + cliConfigDir_ = testEtcDir_ / "coop" / "cli"; + } + + void TearDown() override { + std::error_code ec; + if (fs::exists(testHomeDir_)) { + fs::remove_all(testHomeDir_, ec); + } + if (fs::exists(testEtcDir_)) { + fs::remove_all(testEtcDir_, ec); + } + CmdHandlerTestBase::TearDown(); + } + + protected: + void createTestConfig(const fs::path& path, const std::string& content) { + std::ofstream file(path); + file << content; + file.close(); + } + + fs::path testHomeDir_; + fs::path testEtcDir_; + fs::path systemConfigPath_; + fs::path sessionConfigPath_; + fs::path cliConfigDir_; +}; + +// Tests for VlanIdValue validation + +TEST_F(CmdConfigInterfaceSwitchportAccessVlanTestFixture, vlanIdValidMin) { + VlanIdValue vlanId({"1"}); + EXPECT_EQ(vlanId.getVlanId(), 1); +} + +TEST_F(CmdConfigInterfaceSwitchportAccessVlanTestFixture, vlanIdValidMax) { + VlanIdValue vlanId({"4094"}); + EXPECT_EQ(vlanId.getVlanId(), 4094); +} + +TEST_F(CmdConfigInterfaceSwitchportAccessVlanTestFixture, vlanIdValidMid) { + VlanIdValue vlanId({"100"}); + EXPECT_EQ(vlanId.getVlanId(), 100); +} + +TEST_F(CmdConfigInterfaceSwitchportAccessVlanTestFixture, vlanIdZeroInvalid) { + EXPECT_THROW(VlanIdValue({"0"}), std::invalid_argument); +} + +TEST_F( + CmdConfigInterfaceSwitchportAccessVlanTestFixture, + vlanIdTooHighInvalid) { + EXPECT_THROW(VlanIdValue({"4095"}), std::invalid_argument); +} + +TEST_F( + CmdConfigInterfaceSwitchportAccessVlanTestFixture, + vlanIdNegativeInvalid) { + EXPECT_THROW(VlanIdValue({"-1"}), std::invalid_argument); +} + +TEST_F( + CmdConfigInterfaceSwitchportAccessVlanTestFixture, + vlanIdNonNumericInvalid) { + EXPECT_THROW(VlanIdValue({"abc"}), std::invalid_argument); +} + +TEST_F(CmdConfigInterfaceSwitchportAccessVlanTestFixture, vlanIdEmptyInvalid) { + EXPECT_THROW(VlanIdValue({}), std::invalid_argument); +} + +TEST_F( + CmdConfigInterfaceSwitchportAccessVlanTestFixture, + vlanIdMultipleValuesInvalid) { + EXPECT_THROW(VlanIdValue({"100", "200"}), std::invalid_argument); +} + +// Test error message format +TEST_F( + CmdConfigInterfaceSwitchportAccessVlanTestFixture, + vlanIdOutOfRangeErrorMessage) { + try { + VlanIdValue({"9999"}); + FAIL() << "Expected std::invalid_argument"; + } catch (const std::invalid_argument& e) { + std::string errorMsg = e.what(); + EXPECT_THAT(errorMsg, HasSubstr("VLAN ID must be between 1 and 4094")); + EXPECT_THAT(errorMsg, HasSubstr("9999")); + } +} + +TEST_F( + CmdConfigInterfaceSwitchportAccessVlanTestFixture, + vlanIdNonNumericErrorMessage) { + try { + VlanIdValue({"notanumber"}); + FAIL() << "Expected std::invalid_argument"; + } catch (const std::invalid_argument& e) { + std::string errorMsg = e.what(); + EXPECT_THAT(errorMsg, HasSubstr("Invalid VLAN ID")); + EXPECT_THAT(errorMsg, HasSubstr("notanumber")); + } +} + +// Tests for queryClient + +TEST_F( + CmdConfigInterfaceSwitchportAccessVlanTestFixture, + queryClientSetsIngressVlanMultiplePorts) { + TestableConfigSession session( + sessionConfigPath_.string(), + systemConfigPath_.string(), + cliConfigDir_.string()); + + auto cmd = CmdConfigInterfaceSwitchportAccessVlan(); + VlanIdValue vlanId({"2001"}); + + // Create InterfaceList from port names + utils::InterfaceList interfaces({"eth1/1/1", "eth1/2/1"}); + + auto result = cmd.queryClient(localhost(), interfaces, vlanId); + + EXPECT_THAT(result, HasSubstr("Successfully set access VLAN")); + EXPECT_THAT(result, HasSubstr("eth1/1/1")); + EXPECT_THAT(result, HasSubstr("eth1/2/1")); + EXPECT_THAT(result, HasSubstr("2001")); + + // Verify the ingressVlan was updated for both ports + auto& config = session.getAgentConfig(); + auto& switchConfig = *config.sw(); + auto& ports = *switchConfig.ports(); + for (const auto& port : ports) { + if (*port.name() == "eth1/1/1" || *port.name() == "eth1/2/1") { + EXPECT_EQ(*port.ingressVlan(), 2001); + } + } + + // Verify the vlanPorts entries were also updated + auto& vlanPorts = *switchConfig.vlanPorts(); + for (const auto& vlanPort : vlanPorts) { + if (*vlanPort.logicalPort() == 1 || *vlanPort.logicalPort() == 2) { + EXPECT_EQ(*vlanPort.vlanID(), 2001); + } + } +} + +TEST_F( + CmdConfigInterfaceSwitchportAccessVlanTestFixture, + queryClientThrowsOnEmptyInterfaceList) { + TestableConfigSession session( + sessionConfigPath_.string(), + systemConfigPath_.string(), + cliConfigDir_.string()); + + auto cmd = CmdConfigInterfaceSwitchportAccessVlan(); + VlanIdValue vlanId({"100"}); + + // Empty InterfaceList is valid to construct but queryClient should throw + utils::InterfaceList emptyInterfaces({}); + EXPECT_THROW( + cmd.queryClient(localhost(), emptyInterfaces, vlanId), + std::invalid_argument); +} + +} // namespace facebook::fboss diff --git a/fboss/cli/fboss2/utils/CmdUtilsCommon.h b/fboss/cli/fboss2/utils/CmdUtilsCommon.h index 53adaa39a9132..384deef7a7cdd 100644 --- a/fboss/cli/fboss2/utils/CmdUtilsCommon.h +++ b/fboss/cli/fboss2/utils/CmdUtilsCommon.h @@ -67,6 +67,7 @@ enum class ObjectArgTypeId : uint8_t { OBJECT_ARG_TYPE_ID_INTERFACE_LIST, OBJECT_ARG_TYPE_ID_REVISION_LIST, OBJECT_ARG_TYPE_ID_BUFFER_POOL_NAME, + OBJECT_ARG_TYPE_VLAN_ID, }; template diff --git a/fboss/oss/cli_tests/cli_test_lib.py b/fboss/oss/cli_tests/cli_test_lib.py index be38ebcf1ec2b..670a0731ba951 100644 --- a/fboss/oss/cli_tests/cli_test_lib.py +++ b/fboss/oss/cli_tests/cli_test_lib.py @@ -132,19 +132,26 @@ def run_cmd(cmd: list[str], check: bool = True) -> subprocess.CompletedProcess: return result -def run_cli(args: list[str], check: bool = True) -> dict[str, Any]: +def run_cli(args: list[str], check: bool = True, quiet: bool = False) -> dict[str, Any]: """Run the fboss2-dev CLI with the given arguments. The --fmt json flag is automatically prepended to all commands. Returns the parsed JSON output as a dict. + + Args: + args: CLI arguments to pass after 'fboss2-dev --fmt json' + check: If True, raise RuntimeError on non-zero return code + quiet: If True, suppress logging of command execution """ cli = get_fboss_cli() cmd = [cli, "--fmt", "json"] + args - print(f"[CLI] Running: {' '.join(args)}") + if not quiet: + print(f"[CLI] Running: {' '.join(args)}") start_time = time.time() result = subprocess.run(cmd, capture_output=True, text=True) elapsed = time.time() - start_time - print(f"[CLI] Completed in {elapsed:.2f}s: {' '.join(args)}") + if not quiet: + print(f"[CLI] Completed in {elapsed:.2f}s: {' '.join(args)}") if check and result.returncode != 0: print(f"Command failed with return code {result.returncode}") print(f"stdout: {result.stdout}") @@ -236,6 +243,46 @@ def is_valid_eth_interface(intf: Interface) -> bool: return matches[0] +def wait_for_agent_ready( + timeout_seconds: float = 60.0, + poll_interval_ms: int = 500, +) -> None: + """ + Wait for the FBOSS agent to be ready by polling 'config applied-info'. + + Args: + timeout_seconds: Maximum time to wait in seconds (default: 60) + poll_interval_ms: How often to poll in milliseconds (default: 500) + + Raises: + RuntimeError: If the agent is not ready within the timeout + """ + start_time = time.time() + poll_interval_s = poll_interval_ms / 1000.0 + + while True: + elapsed = time.time() - start_time + if elapsed >= timeout_seconds: + raise RuntimeError(f"Agent not ready after {timeout_seconds}s") + + try: + # Try to run 'config applied-info' - a simple command to check agent is ready + data = run_cli(["config", "applied-info"], check=False, quiet=True) + if data: + print(f"[CLI] Agent is ready (waited {elapsed:.1f}s)") + # Log the applied-info output for debugging + print(f"[CLI] Applied info: {json.dumps(data)}") + return + except Exception: + pass + + # Agent not ready yet, wait and retry + time.sleep(poll_interval_s) + + def commit_config() -> None: - """Commit the current configuration session.""" + """Commit the current configuration session and wait for agent to be ready.""" run_cli(["config", "session", "commit"]) + # After commit, the agent may restart (warmboot/coldboot). + # Wait for it to be ready before returning. + wait_for_agent_ready() diff --git a/fboss/oss/cli_tests/test_config_vlan.py b/fboss/oss/cli_tests/test_config_vlan.py new file mode 100644 index 0000000000000..5de7cdb62e07b --- /dev/null +++ b/fboss/oss/cli_tests/test_config_vlan.py @@ -0,0 +1,102 @@ +#!/usr/bin/env python3 +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. + +""" +End-to-end test for the 'fboss2-dev config interface switchport access vlan ' command. + +This test: +1. Picks an interface from the running system +2. Gets the current access VLAN +3. Sets a new access VLAN +4. Verifies the VLAN was set correctly via 'fboss2-dev show interface' +5. Restores the original VLAN + +Requirements: +- FBOSS agent must be running with a valid configuration +- The test must be run as root (or with appropriate permissions) +""" + +import sys + +from cli_test_lib import ( + commit_config, + find_first_eth_interface, + get_interface_info, + run_cli, +) + + +def get_interface_vlan(interface_name: str) -> int: + """Get the current access VLAN for an interface.""" + info = get_interface_info(interface_name) + if info.vlan is None: + raise RuntimeError(f"Could not find VLAN for interface {interface_name}") + return info.vlan + + +def set_interface_vlan(interface_name: str, vlan_id: int) -> None: + """Set the access VLAN for an interface and commit the change.""" + run_cli( + [ + "config", + "interface", + interface_name, + "switchport", + "access", + "vlan", + str(vlan_id), + ] + ) + commit_config() + + +def main() -> int: + print("=" * 60) + print("CLI E2E Test: config interface switchport access vlan ") + print("=" * 60) + + # Step 1: Get an interface to test with + print("\n[Step 1] Finding an interface to test...") + interface = find_first_eth_interface() + print(f" Using interface: {interface.name} (VLAN: {interface.vlan})") + + # Step 2: Get the current VLAN + print("\n[Step 2] Getting current access VLAN...") + original_vlan = get_interface_vlan(interface.name) + print(f" Current access VLAN: {original_vlan}") + + # Step 3: Set a new VLAN (toggle between current and current+1) + new_vlan = original_vlan + 1 + print(f"\n[Step 3] Setting access VLAN to {new_vlan}...") + set_interface_vlan(interface.name, new_vlan) + print(f" Access VLAN set to {new_vlan}") + + # Step 4: Verify VLAN via 'show interface' + print("\n[Step 4] Verifying access VLAN via 'show interface'...") + actual_vlan = get_interface_vlan(interface.name) + if actual_vlan != new_vlan: + print(f" ERROR: Expected VLAN {new_vlan}, got {actual_vlan}") + return 1 + print(f" Verified: Access VLAN is {actual_vlan}") + + # Step 5: Restore original VLAN + print(f"\n[Step 5] Restoring original access VLAN ({original_vlan})...") + set_interface_vlan(interface.name, original_vlan) + print(f" Restored access VLAN to {original_vlan}") + + # Verify restoration + restored_vlan = get_interface_vlan(interface.name) + if restored_vlan != original_vlan: + print(f" WARNING: Restoration may have failed. Current: {restored_vlan}") + + print("\n" + "=" * 60) + print("TEST PASSED") + print("=" * 60) + return 0 + + +if __name__ == "__main__": + sys.exit(main())