diff --git a/src/core/sm/networkmanager/itf/firewall.hpp b/src/core/sm/networkmanager/itf/firewall.hpp index 1520b25a5..3d6af4d28 100644 --- a/src/core/sm/networkmanager/itf/firewall.hpp +++ b/src/core/sm/networkmanager/itf/firewall.hpp @@ -53,6 +53,14 @@ struct InstanceFirewallParams { StaticArray mOutput; }; +/** + * Masquerade rule identity. + */ +struct MasqueradeParams { + StaticString mSubnet; + StaticString mOutIfName; +}; + /** * Firewall interface. * @@ -69,7 +77,8 @@ class FirewallItf { /** * Starts the firewall: creates the `inet aos` table, base chains and - * netfilter hooks (forward filter, postrouting nat). + * netfilter hooks (forward filter, postrouting nat) when they are absent. + * Rules that are already there are left alone. * * @return Error. */ @@ -82,6 +91,21 @@ class FirewallItf { */ virtual Error Stop() = 0; + /** + * Removes the instance chains and masquerade rules that no longer belong + * to anything known, keeping the rest in place. + * + * Called on start so that artifacts left by a crashed SM are reaped + * without touching the rules of the instances that kept running. + * + * @param knownInstanceIDs instance ids whose chains must be kept. + * @param knownMasquerades masquerade rules that must be kept. + * @return Error. + */ + virtual Error RemoveOrphans( + const Array>& knownInstanceIDs, const Array& knownMasquerades) + = 0; + /** * Adds a per-instance chain with the given input/output access rules. * diff --git a/src/core/sm/networkmanager/itf/interfacemanager.hpp b/src/core/sm/networkmanager/itf/interfacemanager.hpp index e552eb3f8..2f1ee9418 100644 --- a/src/core/sm/networkmanager/itf/interfacemanager.hpp +++ b/src/core/sm/networkmanager/itf/interfacemanager.hpp @@ -7,7 +7,9 @@ #ifndef AOS_CORE_SM_NETWORKMANAGER_ITF_INTERFACEMANAGER_HPP_ #define AOS_CORE_SM_NETWORKMANAGER_ITF_INTERFACEMANAGER_HPP_ +#include #include +#include namespace aos::sm::networkmanager { @@ -15,6 +17,35 @@ namespace aos::sm::networkmanager { * @{ */ +/** + * Link kind type. + */ +class LinkKindType { +public: + enum class Enum { eUnknown, eBridge, eVlan, eVeth }; + + static const Array GetStrings() + { + static const char* const sLinkKindStrings[] = {"unknown", "bridge", "vlan", "veth"}; + + return Array(sLinkKindStrings, ArraySize(sLinkKindStrings)); + }; +}; + +using LinkKindEnum = LinkKindType::Enum; +using LinkKind = EnumStringer; + +/** + * Network link attributes as seen on the system. + */ +struct LinkInfo { + StaticString mName; + LinkKind mKind; + StaticString mMaster; + uint64_t mVlanID {}; + bool mUp {}; +}; + /** * Network interface manager interface. */ @@ -25,6 +56,15 @@ class InterfaceManagerItf { */ virtual ~InterfaceManagerItf() = default; + /** + * Returns link attributes as they are on the system. + * + * @param ifname interface name. + * @param[out] info link attributes. + * @return Error, eNotFound if the link doesn't exist. + */ + virtual Error GetLink(const String& ifname, LinkInfo& info) const = 0; + /** * Removes interface. * diff --git a/src/core/sm/networkmanager/itf/namespacemanager.hpp b/src/core/sm/networkmanager/itf/namespacemanager.hpp index d14744afa..e5bf90b26 100644 --- a/src/core/sm/networkmanager/itf/namespacemanager.hpp +++ b/src/core/sm/networkmanager/itf/namespacemanager.hpp @@ -33,6 +33,14 @@ class NamespaceManagerItf { */ virtual Error CreateNetworkNamespace(const String& ns) = 0; + /** + * Checks whether network namespace exists on the system. + * + * @param ns network namespace name. + * @return RetWithError. + */ + virtual RetWithError IsNetworkNamespaceExist(const String& ns) const = 0; + /** * Returns network namespace path. * diff --git a/src/core/sm/networkmanager/networkmanager.cpp b/src/core/sm/networkmanager/networkmanager.cpp index 11e293632..f9863b38a 100644 --- a/src/core/sm/networkmanager/networkmanager.cpp +++ b/src/core/sm/networkmanager/networkmanager.cpp @@ -105,11 +105,15 @@ Error NetworkManager::Start() } }); + if (err = RemoveFirewallOrphans(); !err.IsNone()) { + return err; + } + if (err = RemoveDNSOrphans(); !err.IsNone()) { return err; } - if (err = CleanupLeftoverInstances(); !err.IsNone()) { + if (err = ReconcileInstances(); !err.IsNone()) { return err; } @@ -977,13 +981,110 @@ Error NetworkManager::UpdateInstanceNetworkCache( return ErrorEnum::eNone; } -Error NetworkManager::CleanupLeftoverInstances() +RetWithError NetworkManager::IsInstanceInterfaceAlive( + const String& instanceID, const String& hostIfName, const String& bridgeIfName) const +{ + if (bridgeIfName.IsEmpty()) { + return {false, ErrorEnum::eNone}; + } + + LinkInfo link; + + if (auto err = mNetIf->GetLink(hostIfName, link); !err.IsNone()) { + if (err.Is(ErrorEnum::eNotFound)) { + return {false, ErrorEnum::eNone}; + } + + return {false, AOS_ERROR_WRAP(err)}; + } + + if (link.mKind != LinkKindEnum::eVeth || link.mMaster != bridgeIfName) { + return {false, ErrorEnum::eNone}; + } + + bool nsExists = false; + Error err; + + if (Tie(nsExists, err) = mNetns->IsNetworkNamespaceExist(instanceID); !err.IsNone()) { + return {false, AOS_ERROR_WRAP(err)}; + } + + return {nsExists, ErrorEnum::eNone}; +} + +Error NetworkManager::InitInstance(const String& instanceID, const String& networkID) +{ + LOG_DBG() << "Adopt running instance" << Log::Field("instanceID", instanceID) << Log::Field("networkID", networkID); + + if (auto errCache = AddInstanceToCache(instanceID, networkID); !errCache.IsNone()) { + return errCache; + } + + Error err; + + auto cleanupCache = DeferRelease(&instanceID, [this, &networkID, &err](const String* id) { + if (!err.IsNone()) { + if (auto errRemove = RemoveInstanceFromCache(*id, networkID); !errRemove.IsNone()) { + LOG_ERR() << "Failed to remove instance from cache" << Log::Field("instanceID", *id) + << Log::Field("networkID", networkID) << Log::Field(errRemove); + } + } + }); + + auto config = MakeUnique(&mAllocator); + StaticString instanceIP; + + { + LockGuard lock {mMutex}; + + auto it = mInstanceNetworkInfos.Find(instanceID); + if (it == mInstanceNetworkInfos.end()) { + err = AOS_ERROR_WRAP(Error(ErrorEnum::eNotFound, "instance network info not found")); + + return err; + } + + *config = it->mSecond.mNetworkConfig; + instanceIP = it->mSecond.mAllocatedParams.mIP; + } + + auto hosts = MakeUnique(&mAllocator); + + if (err = PrepareHosts(instanceID, networkID, *config, *hosts); !err.IsNone()) { + return err; + } + + if (err + = mNetMonitor->StartInstanceMonitoring(instanceID, instanceIP, config->mDownloadLimit, config->mUploadLimit); + !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + auto cleanupMonitoring = DeferRelease(&instanceID, [this, &err](const String* id) { + if (!err.IsNone()) { + if (auto errStop = mNetMonitor->StopInstanceMonitoring(*id); !errStop.IsNone()) { + LOG_ERR() << "Failed to stop instance monitoring on rollback" << Log::Field("instanceID", *id) + << Log::Field(errStop); + } + } + }); + + if (err = UpdateInstanceNetworkCache(instanceID, networkID, *hosts); !err.IsNone()) { + return err; + } + + return ErrorEnum::eNone; +} + +Error NetworkManager::ReconcileInstances() { - LOG_DBG() << "Cleanup leftover instances"; + LOG_DBG() << "Reconcile instances"; struct Entry { - StaticString mInstanceID; - StaticString mNetworkID; + StaticString mInstanceID; + StaticString mNetworkID; + StaticString mHostIfName; + StaticString mBridgeIfName; }; auto entries = MakeUnique>(&mAllocator); @@ -992,25 +1093,49 @@ Error NetworkManager::CleanupLeftoverInstances() LockGuard lock {mMutex}; for (const auto& item : mInstanceNetworkInfos) { - if (auto err = entries->PushBack({item.mFirst, item.mSecond.mNetworkID}); !err.IsNone()) { + StaticString bridgeIfName; + + if (auto it = mNetworkProviders.Find(item.mSecond.mNetworkID); it != mNetworkProviders.end()) { + bridgeIfName = it->mSecond.mBridgeIfName; + } + + if (auto err + = entries->PushBack({item.mFirst, item.mSecond.mNetworkID, item.mSecond.mHostIfName, bridgeIfName}); + !err.IsNone()) { return AOS_ERROR_WRAP(err); } } } - // Adopt a DNS handle for each network with leftover instances, so the - // RemoveHost call inside DeleteInstanceNetworkConfig has a backend to - // talk to (CreateInstance is idempotent — it adopts a surviving dnsmasq - // for this networkID or respawns a fresh one). for (const auto& entry : *entries) { - if (auto err = AdoptDNSServer(entry.mNetworkID); !err.IsNone()) { + if (entry.mHostIfName.IsEmpty()) { + continue; + } + + bool alive = false; + Error err; + + if (Tie(alive, err) = IsInstanceInterfaceAlive(entry.mInstanceID, entry.mHostIfName, entry.mBridgeIfName); + !err.IsNone()) { + LOG_WRN() << "Failed to check leftover instance interface" << Log::Field("instanceID", entry.mInstanceID) + << Log::Field("hostIfName", entry.mHostIfName) << Log::Field(err); + } + + if (alive) { + if (err = InitInstance(entry.mInstanceID, entry.mNetworkID); err.IsNone()) { + continue; + } else { + LOG_WRN() << "Failed to adopt leftover instance, falling back to cleanup" + << Log::Field("instanceID", entry.mInstanceID) << Log::Field(err); + } + } + + if (err = AdoptDNSServer(entry.mNetworkID); !err.IsNone()) { LOG_WRN() << "Failed to adopt DNS server for leftover cleanup" << Log::Field("networkID", entry.mNetworkID) << Log::Field(err); } - } - for (const auto& entry : *entries) { - if (auto err = DeleteInstanceNetworkConfig(entry.mInstanceID, entry.mNetworkID); !err.IsNone()) { + if (err = DeleteInstanceNetworkConfig(entry.mInstanceID, entry.mNetworkID); !err.IsNone()) { LOG_WRN() << "Failed to delete leftover instance network config" << Log::Field("instanceID", entry.mInstanceID) << Log::Field("networkID", entry.mNetworkID) << Log::Field(err); @@ -1020,6 +1145,34 @@ Error NetworkManager::CleanupLeftoverInstances() return ErrorEnum::eNone; } +Error NetworkManager::RemoveFirewallOrphans() +{ + auto knownInstanceIDs = MakeUnique, cMaxNumInstances>>(&mAllocator); + auto knownMasquerades = MakeUnique>(&mAllocator); + + { + LockGuard lock {mMutex}; + + for (const auto& [instanceID, _] : mInstanceNetworkInfos) { + if (auto err = knownInstanceIDs->PushBack(instanceID); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + } + + for (const auto& [_, network] : mNetworkProviders) { + if (auto err = knownMasquerades->PushBack({network.mSubnet, network.mBridgeIfName}); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + } + } + + if (auto err = mFirewall->RemoveOrphans(*knownInstanceIDs, *knownMasquerades); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + return ErrorEnum::eNone; +} + Error NetworkManager::RemoveDNSOrphans() { auto known = MakeUnique, cMaxNumOwners>>(&mAllocator); @@ -1367,6 +1520,21 @@ Error NetworkManager::PrepareDNSServerParams(const NetworkInfo& network, DNSServ return ErrorEnum::eNone; } +RetWithError NetworkManager::IsLinkExist(const String& ifName) const +{ + LinkInfo link; + + if (auto err = mNetIf->GetLink(ifName, link); !err.IsNone()) { + if (err.Is(ErrorEnum::eNotFound)) { + return {false, ErrorEnum::eNone}; + } + + return {false, AOS_ERROR_WRAP(err)}; + } + + return {true, ErrorEnum::eNone}; +} + Error NetworkManager::CreateNetwork(const NetworkInfo& network) { LOG_DBG() << "Create network" << Log::Field("networkID", network.mNetworkID) @@ -1376,24 +1544,54 @@ Error NetworkManager::CreateNetwork(const NetworkInfo& network) Error err; - if (err = mNetIfFactory->CreateBridge(network.mBridgeIfName, network.mIP, network.mSubnet); !err.IsNone()) { - return AOS_ERROR_WRAP(err); + // A link may already be there when SM crashed without running its teardown. + // Recreating it is not a no-op: the kernel takes RTM_NEWLINK without + // NLM_F_EXCL as a modify request, so CreateVlan would push a freshly + // generated MAC onto the live vlan and break the traffic of the instances + // still running on it. Adopt what exists and create only what is missing. + bool bridgeExists = false; + + if (Tie(bridgeExists, err) = IsLinkExist(network.mBridgeIfName); !err.IsNone()) { + return err; } - auto cleanupBridge = DeferRelease(&network, [this, &err](const NetworkInfo* network) { - if (!err.IsNone()) { + bool bridgeCreated = false; + + if (!bridgeExists) { + if (err = mNetIfFactory->CreateBridge(network.mBridgeIfName, network.mIP, network.mSubnet); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + bridgeCreated = true; + } + + auto cleanupBridge = DeferRelease(&network, [this, &err, bridgeCreated](const NetworkInfo* network) { + if (!err.IsNone() && bridgeCreated) { mNetIf->DeleteLink(network->mBridgeIfName); } }); - // Create the vlan already enslaved to the bridge (master) in one operation, - // avoiding a separate SetMasterLink round-trip. - if (err = mNetIfFactory->CreateVlan(network.mVlanIfName, network.mVlanID, network.mBridgeIfName); !err.IsNone()) { - return AOS_ERROR_WRAP(err); + bool vlanExists = false; + + if (Tie(vlanExists, err) = IsLinkExist(network.mVlanIfName); !err.IsNone()) { + return err; } - auto cleanupVlan = DeferRelease(&network, [this, &err](const NetworkInfo* network) { - if (!err.IsNone()) { + bool vlanCreated = false; + + if (!vlanExists) { + // Create the vlan already enslaved to the bridge (master) in one operation, + // avoiding a separate SetMasterLink round-trip. + if (err = mNetIfFactory->CreateVlan(network.mVlanIfName, network.mVlanID, network.mBridgeIfName); + !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + vlanCreated = true; + } + + auto cleanupVlan = DeferRelease(&network, [this, &err, vlanCreated](const NetworkInfo* network) { + if (!err.IsNone() && vlanCreated) { mNetIf->DeleteLink(network->mVlanIfName); } }); diff --git a/src/core/sm/networkmanager/networkmanager.hpp b/src/core/sm/networkmanager/networkmanager.hpp index 49b6229cc..0c12f6500 100644 --- a/src/core/sm/networkmanager/networkmanager.hpp +++ b/src/core/sm/networkmanager/networkmanager.hpp @@ -202,12 +202,16 @@ class NetworkManager : public NetworkManagerItf { // Start()/OnConnect() run once, outside the concurrent instance-operation hot path, so their // allocations are added rather than multiplied by cMaxNumConcurrentItems: RemoveDNSOrphans' known - // networks list, CleanupLeftoverInstances' leftover instance/network ID pairs plus the - // InstanceNetworkInfo DeleteInstanceNetworkConfig allocates while clearing a host interface, and - // OnConnect's state sync snapshot. + // networks list, RemoveFirewallOrphans' known instance and masquerade lists, ReconcileInstances' + // leftover entries (instance/network IDs plus host/bridge interface names, so four ID-sized strings + // per instance) plus, alive at the same time, either the InstanceNetworkInfo + // DeleteInstanceNetworkConfig allocates while clearing a host interface or the config and hosts + // InitInstance allocates while restoring the runtime cache, and OnConnect's state sync snapshot. static constexpr auto cAllocatorSize = cMaxOperationAllocatorSize * cMaxNumConcurrentItems + sizeof(StaticArray, cMaxNumOwners>) - + sizeof(StaticArray, cMaxNumInstances>) * 2 + sizeof(InstanceNetworkInfo) + + sizeof(StaticArray, cMaxNumInstances>) * 5 + + sizeof(StaticArray) + sizeof(InstanceNetworkInfo) + + sizeof(InstanceNetworkConfig) + sizeof(InstanceHosts) + sizeof(StaticArray); static constexpr auto cNumAllocations = 8 * cMaxNumConcurrentItems; @@ -226,9 +230,13 @@ class NetworkManager : public NetworkManagerItf { static constexpr auto cVlanIfPrefix = "vlan-"; static constexpr auto cResolvConfLineLen = AOS_CONFIG_NETWORKMANAGER_RESOLV_CONF_LINE_LEN; - Error IsInstanceInNetwork(const String& instanceID, const String& networkID) const; - Error AddInstanceToCache(const String& instanceID, const String& networkID); - Error CleanupLeftoverInstances(); + Error IsInstanceInNetwork(const String& instanceID, const String& networkID) const; + Error AddInstanceToCache(const String& instanceID, const String& networkID); + RetWithError IsInstanceInterfaceAlive( + const String& instanceID, const String& hostIfName, const String& bridgeIfName) const; + Error InitInstance(const String& instanceID, const String& networkID); + Error ReconcileInstances(); + Error RemoveFirewallOrphans(); Error RemoveDNSOrphans(); Error AdoptDNSServer(const String& networkID); Error PrepareBridgeParams( @@ -250,9 +258,10 @@ class NetworkManager : public NetworkManagerItf { Error IsHostnameExist(const InstanceCache& instanceCache, const Array>& hosts) const; Error PushHostWithDomain( const String& host, const String& networkID, Array>& hosts) const; - Error CreateNetwork(const NetworkInfo& network); - Error DeleteInstanceNetworkConfig(const String& instanceID, const String& networkID); - Error GenerateIfName(String& ifName, const String& ifPrefix); + RetWithError IsLinkExist(const String& ifName) const; + Error CreateNetwork(const NetworkInfo& network); + Error DeleteInstanceNetworkConfig(const String& instanceID, const String& networkID); + Error GenerateIfName(String& ifName, const String& ifPrefix); template Error GenerateUniqueIfName(String& ifName, const String& ifPrefix, P&& isUnique) diff --git a/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp b/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp index 3f66c52cc..db9f668d4 100644 --- a/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp +++ b/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp @@ -17,6 +17,7 @@ class FirewallMock : public FirewallItf { public: MOCK_METHOD(Error, Start, (), (override)); MOCK_METHOD(Error, Stop, (), (override)); + MOCK_METHOD(Error, RemoveOrphans, (const Array>&, const Array&), (override)); MOCK_METHOD(Error, AddInstance, (const String&, const InstanceFirewallParams&), (override)); MOCK_METHOD(Error, RemoveInstance, (const String&), (override)); MOCK_METHOD(Error, UpdateInstance, (const String&, const InstanceFirewallParams&), (override)); diff --git a/src/core/sm/networkmanager/tests/mocks/interfacemanagermock.hpp b/src/core/sm/networkmanager/tests/mocks/interfacemanagermock.hpp index 9084ea755..36296083c 100644 --- a/src/core/sm/networkmanager/tests/mocks/interfacemanagermock.hpp +++ b/src/core/sm/networkmanager/tests/mocks/interfacemanagermock.hpp @@ -15,6 +15,7 @@ namespace aos::sm::networkmanager { class InterfaceManagerMock : public InterfaceManagerItf { public: + MOCK_METHOD(Error, GetLink, (const String&, LinkInfo&), (const, override)); MOCK_METHOD(Error, DeleteLink, (const String&), (override)); MOCK_METHOD(Error, SetupLink, (const String&, const String&), (override)); MOCK_METHOD(Error, SetMasterLink, (const String&, const String&), (override)); diff --git a/src/core/sm/networkmanager/tests/mocks/namespacemanagermock.hpp b/src/core/sm/networkmanager/tests/mocks/namespacemanagermock.hpp index 30be8bf71..4565a4523 100644 --- a/src/core/sm/networkmanager/tests/mocks/namespacemanagermock.hpp +++ b/src/core/sm/networkmanager/tests/mocks/namespacemanagermock.hpp @@ -16,6 +16,7 @@ namespace aos::sm::networkmanager { class NamespaceManagerMock : public NamespaceManagerItf { public: MOCK_METHOD(Error, CreateNetworkNamespace, (const String&), (override)); + MOCK_METHOD(RetWithError, IsNetworkNamespaceExist, (const String&), (const, override)); MOCK_METHOD(RetWithError>, GetNetworkNamespacePath, (const String&), (const, override)); MOCK_METHOD(Error, DeleteNetworkNamespace, (const String&), (override)); }; diff --git a/src/core/sm/networkmanager/tests/networkmanager.cpp b/src/core/sm/networkmanager/tests/networkmanager.cpp index 55859a460..382f351f5 100644 --- a/src/core/sm/networkmanager/tests/networkmanager.cpp +++ b/src/core/sm/networkmanager/tests/networkmanager.cpp @@ -42,6 +42,7 @@ class NetworkManagerTest : public Test { std::filesystem::create_directories(mWorkingDir.CStr()); EXPECT_CALL(mFirewall, Start()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, RemoveOrphans(_, _)).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mTrafficMonitor, Start()).WillOnce(Return(aos::ErrorEnum::eNone)); // NetworkManager::Start reaps DNS orphans from a previous SM lifetime @@ -56,6 +57,8 @@ class NetworkManagerTest : public Test { EXPECT_CALL(mDNSName, RemoveServer(_)).Times(AnyNumber()).WillRepeatedly(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mDNSServer, RemoveHost(_)).Times(AnyNumber()).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetIf, GetLink(_, _)).Times(AnyNumber()).WillRepeatedly(Return(aos::ErrorEnum::eNotFound)); + // Masquerade is a per-network rule installed/removed by CreateNetwork / // ClearNetwork; leave it lenient so per-test sequences need not assert it. EXPECT_CALL(mFirewall, AddMasquerade(_, _)).Times(AnyNumber()).WillRepeatedly(Return(aos::ErrorEnum::eNone)); @@ -196,6 +199,127 @@ class NetworkManagerTest : public Test { EXPECT_CALL(mStorage, UpdateInstanceNetworkInfo(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); } + NetworkInfo CreateTestNetworkInfo() + { + NetworkInfo network; + network.mNetworkID = "network1"; + network.mIP = "192.168.1.1"; + network.mSubnet = "192.168.1.0/24"; + network.mVlanID = 100ULL; + network.mVlanIfName = "vlan-1234abcd"; + network.mBridgeIfName = "br-ef567890"; + + return network; + } + + void InitWithStoredNetwork(const NetworkInfo& network) + { + mNetworkInfos.PushBack(network); + + EXPECT_CALL(mStorage, GetNetworksInfo(_)) + .WillOnce(DoAll(SetArgReferee<0>(mNetworkInfos), Return(aos::ErrorEnum::eNone))); + EXPECT_CALL(mStorage, GetInstanceNetworksInfo(_)) + .WillOnce(DoAll(SetArgReferee<0>(mInstanceNetworkInfos), Return(aos::ErrorEnum::eNone))); + + mNetManager = std::make_unique(); + + ASSERT_EQ(mNetManager->Init(mStorage, mBridgeNetwork, mFirewall, mBandwidth, mDNSName, mTrafficMonitor, mNetns, + mNetIf, mRandom, mNetIfFactory, mNetworkProvider, "test-node"), + aos::ErrorEnum::eNone); + } + + void ExpectLinkExists(const aos::String& ifName, LinkKind kind, const aos::String& master = "") + { + LinkInfo link; + link.mName = ifName; + link.mKind = kind; + link.mMaster = master; + + EXPECT_CALL(mNetIf, GetLink(ifName, _)) + .WillRepeatedly(DoAll(SetArgReferee<1>(link), Return(aos::ErrorEnum::eNone))); + } + + void RestartWithStoredState(const aos::Array& networks, + const aos::Array& instances) + { + EXPECT_CALL(mTrafficMonitor, Stop()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, Stop()).WillOnce(Return(aos::ErrorEnum::eNone)); + ASSERT_EQ(mNetManager->Stop(), aos::ErrorEnum::eNone); + + EXPECT_CALL(mNetIf, DeleteLink(_)).Times(AnyNumber()).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + mNetManager.reset(); + + EXPECT_CALL(mFirewall, Start()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, RemoveOrphans(_, _)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, Start()).WillOnce(Return(aos::ErrorEnum::eNone)); + + EXPECT_CALL(mStorage, GetNetworksInfo(_)) + .WillOnce(Invoke([&networks](aos::Array& out) { + out = networks; + return aos::ErrorEnum::eNone; + })); + EXPECT_CALL(mStorage, GetInstanceNetworksInfo(_)) + .WillOnce(Invoke([&instances](aos::Array& out) { + out = instances; + return aos::ErrorEnum::eNone; + })); + + mNetManager = std::make_unique(); + + ASSERT_EQ(mNetManager->Init(mStorage, mBridgeNetwork, mFirewall, mBandwidth, mDNSName, mTrafficMonitor, mNetns, + mNetIf, mRandom, mNetIfFactory, mNetworkProvider, "test-node"), + aos::ErrorEnum::eNone); + } + + aos::sm::networkmanager::InstanceNetworkInfo CreateLeftoverInstance( + const aos::sm::networkmanager::NetworkInfo& network) + { + aos::sm::networkmanager::InstanceNetworkInfo leftover; + leftover.mInstanceID = "leftover-instance"; + leftover.mNetworkID = network.mNetworkID; + leftover.mNetworkConfig.mHostname = "leftover-host"; + leftover.mNetworkConfig.mDownloadLimit = 4096; + leftover.mNetworkConfig.mUploadLimit = 2048; + leftover.mAllocatedParams.mIP = "192.168.1.5"; + leftover.mAllocatedParams.mSubnet = network.mSubnet; + leftover.mHostIfName = "veth-leftover"; + + return leftover; + } + + void ExpectLeftoverInstanceCleaned() + { + EXPECT_CALL(mDNSName, CreateServer(_, _)) + .WillOnce(Return(aos::RetWithError {&mDNSServer, aos::ErrorEnum::eNone})); + EXPECT_CALL(mDNSServer, RemoveHost(aos::String("leftover-instance"))).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mBandwidth, Clear(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, RemoveInstance(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mBridgeNetwork, Detach(_, _)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, DeleteNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mStorage, UpdateInstanceNetworkInfo(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + } + + void ExpectLeftoverInstanceUntouched() + { + EXPECT_CALL(mDNSName, CreateServer(_, _)).Times(0); + EXPECT_CALL(mDNSServer, RemoveHost(_)).Times(0); + EXPECT_CALL(mBandwidth, Clear(_)).Times(0); + EXPECT_CALL(mFirewall, RemoveInstance(_)).Times(0); + EXPECT_CALL(mBridgeNetwork, Detach(_, _)).Times(0); + EXPECT_CALL(mNetns, DeleteNetworkNamespace(_)).Times(0); + EXPECT_CALL(mStorage, UpdateInstanceNetworkInfo(_)).Times(0); + } + + void ExpectStartInstanceOnStoredNetwork(const aos::String& instanceID, const aos::String& networkID) + { + EXPECT_CALL(mNetworkProvider, AllocateInstanceNetwork(_, networkID, aos::String("test-node"), _, _)) + .WillOnce(DoAll(SetArgReferee<4>(CreateTestAllocatedParams()), Return(aos::ErrorEnum::eNone))); + EXPECT_CALL(mStorage, AddInstanceNetworkInfo(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->CreateInstanceNetwork(instanceID, networkID, CreateTestInstanceNetworkConfig()), + aos::ErrorEnum::eNone); + } + void ExpectDeleteInstanceCalls(int times = 1) { EXPECT_CALL(mDNSServer, RemoveHost(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); @@ -1029,6 +1153,81 @@ TEST_F(NetworkManagerTest, InitWithExistingNetworks) ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID, "network1"), aos::ErrorEnum::eNone); } +TEST_F(NetworkManagerTest, CreateNetwork_AdoptsExistingBridgeAndVlan) +{ + const auto network = CreateTestNetworkInfo(); + const aos::String instanceID = "test-instance"; + + InitWithStoredNetwork(network); + + ExpectLinkExists(network.mBridgeIfName, LinkKindEnum::eBridge); + ExpectLinkExists(network.mVlanIfName, LinkKindEnum::eVlan); + + EXPECT_CALL(mNetIfFactory, CreateBridge(_, _, _)).Times(0); + EXPECT_CALL(mNetIfFactory, CreateVlan(_, _, _)).Times(0); + EXPECT_CALL(mDNSName, CreateServer(_, _)) + .WillOnce(Return(aos::RetWithError {&mDNSServer, aos::ErrorEnum::eNone})); + + ExpectStartInstanceOnStoredNetwork(instanceID, network.mNetworkID); + + ExpectAddInstanceCalls(); + ExpectPersistInstanceCalls(); + EXPECT_CALL(mNetns, CreateNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, GetNetworkNamespacePath(_)) + .WillOnce(Return(aos::RetWithError> {{}, aos::ErrorEnum::eNone})); + EXPECT_CALL(mTrafficMonitor, StartInstanceMonitoring(_, _, _, _)).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID, network.mNetworkID), aos::ErrorEnum::eNone); +} + +TEST_F(NetworkManagerTest, CreateNetwork_CreatesOnlyMissingVlan) +{ + const auto network = CreateTestNetworkInfo(); + const aos::String instanceID = "test-instance"; + + InitWithStoredNetwork(network); + + ExpectLinkExists(network.mBridgeIfName, LinkKindEnum::eBridge); + + EXPECT_CALL(mNetIfFactory, CreateBridge(_, _, _)).Times(0); + EXPECT_CALL(mNetIfFactory, CreateVlan(network.mVlanIfName, network.mVlanID, network.mBridgeIfName)) + .WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mDNSName, CreateServer(_, _)) + .WillOnce(Return(aos::RetWithError {&mDNSServer, aos::ErrorEnum::eNone})); + + ExpectStartInstanceOnStoredNetwork(instanceID, network.mNetworkID); + + ExpectAddInstanceCalls(); + ExpectPersistInstanceCalls(); + EXPECT_CALL(mNetns, CreateNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, GetNetworkNamespacePath(_)) + .WillOnce(Return(aos::RetWithError> {{}, aos::ErrorEnum::eNone})); + EXPECT_CALL(mTrafficMonitor, StartInstanceMonitoring(_, _, _, _)).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID, network.mNetworkID), aos::ErrorEnum::eNone); +} + +TEST_F(NetworkManagerTest, CreateNetwork_KeepsAdoptedBridgeWhenVlanCreationFails) +{ + const auto network = CreateTestNetworkInfo(); + const aos::String instanceID = "test-instance"; + + InitWithStoredNetwork(network); + + ExpectLinkExists(network.mBridgeIfName, LinkKindEnum::eBridge); + + EXPECT_CALL(mNetIfFactory, CreateBridge(_, _, _)).Times(0); + EXPECT_CALL(mNetIfFactory, CreateVlan(_, _, _)).WillOnce(Return(aos::ErrorEnum::eFailed)); + EXPECT_CALL(mNetIf, DeleteLink(_)).Times(0); + + ExpectStartInstanceOnStoredNetwork(instanceID, network.mNetworkID); + + EXPECT_FALSE(mNetManager->StartInstanceNetwork(instanceID, network.mNetworkID).IsNone()); + + Mock::VerifyAndClearExpectations(&mNetIf); + Mock::VerifyAndClearExpectations(&mNetIfFactory); +} + TEST_F(NetworkManagerTest, CreateInstanceNetwork_VerifyUpdateItemNetworkParams) { const aos::String networkID = "test-network"; @@ -1192,84 +1391,103 @@ TEST_F(NetworkManagerTest, OnConnect_SyncsNetworkStateWithCM) mNetManager->OnConnect(); } -TEST_F(NetworkManagerTest, Start_AdoptsDNSForLeftoverInstancesAndCleansHosts) +TEST_F(NetworkManagerTest, Start_CleansLeftoverInstanceWithMissingInterface) { - // A leftover instance from a previous SM lifetime: its network is still - // in storage. On Start, NetworkManager should reap DNS orphans (with the - // known network in the list), then adopt the DNS handle for the leftover - // network and call RemoveHost while cleaning up the leftover instance. - aos::sm::networkmanager::NetworkInfo existingNetwork; - existingNetwork.mNetworkID = "leftover-net"; - existingNetwork.mIP = "192.168.7.1"; - existingNetwork.mSubnet = "192.168.7.0/24"; - existingNetwork.mVlanID = 700ULL; - existingNetwork.mVlanIfName = "vlan-leftover"; - existingNetwork.mBridgeIfName = "br-leftover"; - - aos::sm::networkmanager::InstanceNetworkInfo leftover; - leftover.mInstanceID = "leftover-instance"; - leftover.mNetworkID = existingNetwork.mNetworkID; - leftover.mAllocatedParams.mIP = "192.168.7.5"; - leftover.mAllocatedParams.mSubnet = existingNetwork.mSubnet; - leftover.mHostIfName = "veth-leftover"; + const auto network = CreateTestNetworkInfo(); + const auto leftover = CreateLeftoverInstance(network); aos::StaticArray networks; aos::StaticArray instances; - networks.PushBack(existingNetwork); + networks.PushBack(network); instances.PushBack(leftover); - EXPECT_CALL(mFirewall, Start()).WillOnce(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mTrafficMonitor, Start()).WillOnce(Return(aos::ErrorEnum::eNone)); + RestartWithStoredState(networks, instances); - EXPECT_CALL(mStorage, GetNetworksInfo(_)) - .WillOnce(Invoke([&](aos::Array& out) { - out = networks; - return aos::ErrorEnum::eNone; - })); - EXPECT_CALL(mStorage, GetInstanceNetworksInfo(_)) - .WillOnce(Invoke([&](aos::Array& out) { - out = instances; - return aos::ErrorEnum::eNone; - })); - - // Stop the fixture instance — we drive Init/Start manually below. - EXPECT_CALL(mTrafficMonitor, Stop()).WillOnce(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mFirewall, Stop()).WillOnce(Return(aos::ErrorEnum::eNone)); - ASSERT_EQ(mNetManager->Stop(), aos::ErrorEnum::eNone); - EXPECT_CALL(mNetIf, DeleteLink(_)).Times(AnyNumber()).WillRepeatedly(Return(aos::ErrorEnum::eNone)); - mNetManager.reset(); - - mNetManager = std::make_unique(); - - ASSERT_EQ(mNetManager->Init(mStorage, mBridgeNetwork, mFirewall, mBandwidth, mDNSName, mTrafficMonitor, mNetns, - mNetIf, mRandom, mNetIfFactory, mNetworkProvider, "test-node"), - aos::ErrorEnum::eNone); - - // RemoveOrphans must receive the known networkID from storage. EXPECT_CALL(mDNSName, RemoveOrphans(_)) .WillOnce(Invoke([&](const aos::Array>& known) { EXPECT_EQ(known.Size(), 1U); if (known.Size() == 1) { - EXPECT_EQ(known[0], existingNetwork.mNetworkID); + EXPECT_EQ(known[0], network.mNetworkID); } return aos::ErrorEnum::eNone; })); - // Pre-adopt: CreateInstance with the leftover network's bridge IP / ifname. - EXPECT_CALL(mDNSName, CreateServer(existingNetwork.mNetworkID, _)) - .WillOnce(Invoke([&](const aos::String&, const DNSServerParams& params) { - EXPECT_EQ(params.mBridgeIP, existingNetwork.mIP); - EXPECT_EQ(params.mBridgeIfName, existingNetwork.mBridgeIfName); - return aos::RetWithError {&mDNSServer, aos::ErrorEnum::eNone}; - })); + ExpectLeftoverInstanceCleaned(); - // Leftover instance cleanup goes through the adopted handle. - EXPECT_CALL(mDNSServer, RemoveHost(aos::String("leftover-instance"))).WillOnce(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mBandwidth, Clear(_)).WillOnce(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mFirewall, RemoveInstance(_)).WillOnce(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mBridgeNetwork, Detach(_, _)).WillOnce(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mNetns, DeleteNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mStorage, UpdateInstanceNetworkInfo(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + ASSERT_EQ(mNetManager->Start(), aos::ErrorEnum::eNone); +} + +TEST_F(NetworkManagerTest, Start_KeepsLeftoverInstanceWithLiveInterface) +{ + const auto network = CreateTestNetworkInfo(); + const auto leftover = CreateLeftoverInstance(network); + + aos::StaticArray networks; + aos::StaticArray instances; + networks.PushBack(network); + instances.PushBack(leftover); + + RestartWithStoredState(networks, instances); + + ExpectLinkExists(leftover.mHostIfName, LinkKindEnum::eVeth, network.mBridgeIfName); + EXPECT_CALL(mNetns, IsNetworkNamespaceExist(leftover.mInstanceID)) + .WillRepeatedly(Return(aos::RetWithError {true, aos::ErrorEnum::eNone})); + + EXPECT_CALL(mDNSName, RemoveOrphans(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + + ExpectLeftoverInstanceUntouched(); + + EXPECT_CALL(mTrafficMonitor, + StartInstanceMonitoring(leftover.mInstanceID, leftover.mAllocatedParams.mIP, + leftover.mNetworkConfig.mDownloadLimit, leftover.mNetworkConfig.mUploadLimit)) + .WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->Start(), aos::ErrorEnum::eNone); + + EXPECT_TRUE( + mNetManager->StartInstanceNetwork(leftover.mInstanceID, network.mNetworkID).Is(aos::ErrorEnum::eAlreadyExist)); +} + +TEST_F(NetworkManagerTest, Start_CleansLeftoverInstanceWhenNamespaceMissing) +{ + const auto network = CreateTestNetworkInfo(); + const auto leftover = CreateLeftoverInstance(network); + + aos::StaticArray networks; + aos::StaticArray instances; + networks.PushBack(network); + instances.PushBack(leftover); + + RestartWithStoredState(networks, instances); + + ExpectLinkExists(leftover.mHostIfName, LinkKindEnum::eVeth, network.mBridgeIfName); + EXPECT_CALL(mNetns, IsNetworkNamespaceExist(leftover.mInstanceID)) + .WillRepeatedly(Return(aos::RetWithError {false, aos::ErrorEnum::eNone})); + + EXPECT_CALL(mDNSName, RemoveOrphans(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + + ExpectLeftoverInstanceCleaned(); + + ASSERT_EQ(mNetManager->Start(), aos::ErrorEnum::eNone); +} + +TEST_F(NetworkManagerTest, Start_CleansLeftoverInstanceAttachedToForeignBridge) +{ + const auto network = CreateTestNetworkInfo(); + const auto leftover = CreateLeftoverInstance(network); + + aos::StaticArray networks; + aos::StaticArray instances; + networks.PushBack(network); + instances.PushBack(leftover); + + RestartWithStoredState(networks, instances); + + ExpectLinkExists(leftover.mHostIfName, LinkKindEnum::eVeth, "br-someoneelse"); + + EXPECT_CALL(mDNSName, RemoveOrphans(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + + ExpectLeftoverInstanceCleaned(); ASSERT_EQ(mNetManager->Start(), aos::ErrorEnum::eNone); }