diff --git a/src/core/sm/launcher/launcher.cpp b/src/core/sm/launcher/launcher.cpp index a9ed66b9c..d2c4fef42 100644 --- a/src/core/sm/launcher/launcher.cpp +++ b/src/core/sm/launcher/launcher.cpp @@ -792,6 +792,11 @@ void Launcher::StopAllNetworks() { LOG_INF() << "[profiling] Stop all networks begin" << Log::Field("count", mInstances.Size()); + auto errBegin = mNetworkManager->BeginBatch(); + if (!errBegin.IsNone()) { + LOG_ERR() << "Failed to begin network batch" << Log::Field(AOS_ERROR_WRAP(errBegin)); + } + for (auto& instance : mInstances) { if (instance.mInfo.mType != UpdateItemTypeEnum::eService) { continue; @@ -804,6 +809,20 @@ void Launcher::StopAllNetworks() } } + if (auto err = mLaunchPool.Wait(); !err.IsNone()) { + LOG_ERR() << "Thread pool wait failed" << Log::Field(AOS_ERROR_WRAP(err)); + } + + if (errBegin.IsNone()) { + auto failedIDs = MakeUnique, cMaxNumInstances>>(&mAllocator); + + mNetworkManager->FlushBatch(*failedIDs); + + if (!failedIDs->IsEmpty()) { + LOG_WRN() << "Network stop batch partially failed" << Log::Field("count", failedIDs->Size()); + } + } + LOG_INF() << "[profiling] Stop all networks end"; } @@ -900,6 +919,11 @@ void Launcher::StartNetworks(const Array& startInstances) { LOG_INF() << "[profiling] Start networks begin" << Log::Field("count", startInstances.Size()); + auto errBegin = mNetworkManager->BeginBatch(); + if (!errBegin.IsNone()) { + LOG_ERR() << "Failed to begin network batch" << Log::Field(AOS_ERROR_WRAP(errBegin)); + } + for (const auto& instance : startInstances) { auto instanceData = FindInstanceData(instance); if (!instanceData) { @@ -932,6 +956,29 @@ void Launcher::StartNetworks(const Array& startInstances) LOG_ERR() << "Thread pool wait failed" << Log::Field(AOS_ERROR_WRAP(err)); } + if (errBegin.IsNone()) { + auto failedIDs = MakeUnique, cMaxNumInstances>>(&mAllocator); + + mNetworkManager->FlushBatch(*failedIDs); + + for (const auto& failedID : *failedIDs) { + auto instanceData = FindInstanceDataByID(failedID); + if (!instanceData) { + continue; + } + + SetInstanceState(*instanceData, InstanceStateEnum::eFailed, + AOS_ERROR_WRAP(Error(ErrorEnum::eFailed, "network batch apply failed"))); + + if (auto err + = mNetworkManager->StopInstanceNetwork(instanceData->mInstanceID, instanceData->mInfo.mOwnerID); + !err.IsNone() && !err.Is(ErrorEnum::eNotFound)) { + LOG_ERR() << "Failed to stop network" << Log::Field("instance", instanceData->mInfo) + << Log::Field(AOS_ERROR_WRAP(err)); + } + } + } + LOG_INF() << "[profiling] Start networks end"; } @@ -957,6 +1004,11 @@ void Launcher::StopNetworks(const Array& stopInstances) { LOG_INF() << "[profiling] Stop networks begin" << Log::Field("count", stopInstances.Size()); + auto errBegin = mNetworkManager->BeginBatch(); + if (!errBegin.IsNone()) { + LOG_ERR() << "Failed to begin network batch" << Log::Field(AOS_ERROR_WRAP(errBegin)); + } + for (const auto& instance : stopInstances) { auto instanceData = FindInstanceData(instance); if (!instanceData) { @@ -981,6 +1033,16 @@ void Launcher::StopNetworks(const Array& stopInstances) LOG_ERR() << "Thread pool wait failed" << Log::Field(AOS_ERROR_WRAP(err)); } + if (errBegin.IsNone()) { + auto failedIDs = MakeUnique, cMaxNumInstances>>(&mAllocator); + + mNetworkManager->FlushBatch(*failedIDs); + + if (!failedIDs->IsEmpty()) { + LOG_WRN() << "Network stop batch partially failed" << Log::Field("count", failedIDs->Size()); + } + } + LOG_INF() << "[profiling] Stop networks end"; } @@ -1127,6 +1189,16 @@ Launcher::InstanceData* Launcher::FindInstanceData(const InstanceIdent& instance return const_cast(this)->FindInstanceData(instanceIdent); } +Launcher::InstanceData* Launcher::FindInstanceDataByID(const String& instanceID) +{ + auto it = mInstances.FindIf([&instanceID](const auto& instance) { return instance.mInstanceID == instanceID; }); + if (it != mInstances.end()) { + return it; + } + + return nullptr; +} + RuntimeItf* Launcher::FindInstanceRuntime(const String& runtimeID) { auto it = mRuntimes.FindIf([&runtimeID](const auto& it) { return it.mSecond == runtimeID; }); diff --git a/src/core/sm/launcher/launcher.hpp b/src/core/sm/launcher/launcher.hpp index 801caafd3..f6c17a9a1 100644 --- a/src/core/sm/launcher/launcher.hpp +++ b/src/core/sm/launcher/launcher.hpp @@ -225,6 +225,7 @@ class Launcher : public LauncherItf, InstanceData* FindInstanceData(const InstanceIdent& instanceIdent); InstanceData* FindInstanceData(const InstanceIdent& instanceIdent) const; + InstanceData* FindInstanceDataByID(const String& instanceID); RuntimeItf* FindInstanceRuntime(const String& runtimeID); RuntimeItf* FindInstanceRuntime(const String& runtimeID) const; RuntimeItf* FindInstanceRuntime(const InstanceIdent& instanceIdent); diff --git a/src/core/sm/launcher/tests/launcher.cpp b/src/core/sm/launcher/tests/launcher.cpp index b9da0b2ae..2eaf05cd7 100644 --- a/src/core/sm/launcher/tests/launcher.cpp +++ b/src/core/sm/launcher/tests/launcher.cpp @@ -194,6 +194,8 @@ class LauncherTest : public Test { EXPECT_CALL(mNetworkManager, StartInstanceNetwork).WillRepeatedly(Return(ErrorEnum::eNone)); EXPECT_CALL(mNetworkManager, StopInstanceNetwork).WillRepeatedly(Return(ErrorEnum::eNone)); EXPECT_CALL(mNetworkManager, ReleaseInstanceNetwork).WillRepeatedly(Return(ErrorEnum::eNone)); + EXPECT_CALL(mNetworkManager, BeginBatch()).WillRepeatedly(Return(ErrorEnum::eNone)); + EXPECT_CALL(mNetworkManager, FlushBatch(_)).WillRepeatedly(Return(ErrorEnum::eNone)); } StaticArray GetRuntimesArray() @@ -462,6 +464,65 @@ TEST_F(LauncherTest, LauncherStartsStoredInstancesOnModuleStart) ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); } +TEST_F(LauncherTest, StartNetworks_FlushFailure_FailsInstance) +{ + const std::vector cStoredInfos = { + CreateInstanceInfo("item0", 0, "1.0.0", "runtime0"), + CreateInstanceInfo("item1", 1, "1.0.0", "runtime1"), + }; + + mStorage.Init(cStoredInfos); + + auto err = mLauncher.Init(GetRuntimesArray(), mImageManager, mSender, mStorage, mOCISpec, mItemInfoProvider, + mCloudConnection, mNetworkManager, mInstanceIDProvider, mResourceInfoProvider); + ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); + + EXPECT_CALL(mInstanceIDProvider, GetInstanceID) + .WillRepeatedly(Invoke([](const InstanceIdent& instance, String& instanceID) { + instanceID = instance.mItemID; + + return ErrorEnum::eNone; + })); + + EXPECT_CALL(mNetworkManager, FlushBatch(_)) + .WillRepeatedly(DoAll(WithArg<0>([](auto& failedInstanceIDs) { failedInstanceIDs.PushBack("item0"); }), + Return(ErrorEnum::eNone))); + + EXPECT_CALL(mNetworkManager, StopInstanceNetwork(String("item0"), _)) + .Times(AtLeast(1)) + .WillRepeatedly(Return(ErrorEnum::eNone)); + + EXPECT_CALL(mRuntime1, StartInstance).WillOnce(Invoke([](const InstanceInfo& instance, InstanceStatus& status) { + SetInstanceStatus(instance, InstanceStateEnum::eActive, status); + + return ErrorEnum::eNone; + })); + + err = mLauncher.Start(); + ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); + + err = mLauncher.GetInstancesStatuses(mReceivedStatuses); + ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); + + ASSERT_EQ(mReceivedStatuses.Size(), cStoredInfos.size()); + + for (const auto& status : mReceivedStatuses) { + if (status.mItemID == "item0") { + EXPECT_EQ(status.mState, InstanceStateEnum::eFailed); + } else { + EXPECT_EQ(status.mState, InstanceStateEnum::eActive); + } + } + + EXPECT_CALL(mRuntime0, StopInstance(static_cast(cStoredInfos[0]), _)) + .WillOnce(Return(ErrorEnum::eNone)); + EXPECT_CALL(mRuntime1, StopInstance(static_cast(cStoredInfos[1]), _)) + .WillOnce(Return(ErrorEnum::eNone)); + + err = mLauncher.Stop(); + ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err); +} + TEST_F(LauncherTest, StopInstancesWithExpiredOfflineTTL) { const std::vector cStoredInfos = { diff --git a/src/core/sm/networkmanager/itf/firewall.hpp b/src/core/sm/networkmanager/itf/firewall.hpp index 3d6af4d28..37aa47f58 100644 --- a/src/core/sm/networkmanager/itf/firewall.hpp +++ b/src/core/sm/networkmanager/itf/firewall.hpp @@ -158,6 +158,35 @@ class FirewallItf { * @return Error. */ virtual Error RemoveMasquerade(const String& subnet, const String& outIf) = 0; + + /** + * Opens a batch; AddInstance/RemoveInstance calls are staged until flush. + * + * @return Error. + */ + virtual Error BeginBatch() = 0; + + /** + * Flushes the staged batch atomically in a single nft transaction. + * + * @return Error. + */ + virtual Error FlushBatch() = 0; + + /** + * Discards the staged batch and leaves batch mode without applying anything to the kernel + * (unlike FlushBatch which commits, or Revert which undoes an already-applied batch). + * + * @return Error. + */ + virtual Error AbortBatch() = 0; + + /** + * Reverts the flushed batch, deleting everything it applied by handle. + * + * @return Error. + */ + virtual Error Revert() = 0; }; /** @}*/ diff --git a/src/core/sm/networkmanager/itf/networkmanager.hpp b/src/core/sm/networkmanager/itf/networkmanager.hpp index 6c6333194..e753cb14a 100644 --- a/src/core/sm/networkmanager/itf/networkmanager.hpp +++ b/src/core/sm/networkmanager/itf/networkmanager.hpp @@ -128,6 +128,21 @@ class NetworkManagerItf : public SystemTrafficProviderItf, * @return Error. */ virtual Error ReleaseInstanceNetwork(const String& instanceID, const String& networkID) = 0; + + /** + * Opens a batch; start/stop operations are staged and applied on flush. + * + * @return Error. + */ + virtual Error BeginBatch() = 0; + + /** + * Flushes the staged batch atomically across firewall, traffic and storage. + * + * @param[out] failedInstanceIDs instances that were not applied. + * @return Error. + */ + virtual Error FlushBatch(Array>& failedInstanceIDs) = 0; }; /** @}*/ diff --git a/src/core/sm/networkmanager/itf/storage.hpp b/src/core/sm/networkmanager/itf/storage.hpp index 08882dfb0..c49654258 100644 --- a/src/core/sm/networkmanager/itf/storage.hpp +++ b/src/core/sm/networkmanager/itf/storage.hpp @@ -202,6 +202,27 @@ class StorageItf { */ virtual Error RemoveTrafficMonitorData(const String& chain) = 0; + /** + * Begins a storage transaction; subsequent writes are staged until commit. + * + * @return Error. + */ + virtual Error BeginTransaction() = 0; + + /** + * Commits the current storage transaction. + * + * @return Error. + */ + virtual Error CommitTransaction() = 0; + + /** + * Rolls back the current storage transaction, discarding staged writes. + * + * @return Error. + */ + virtual Error RollbackTransaction() = 0; + /** * Destroys storage interface. */ diff --git a/src/core/sm/networkmanager/itf/trafficmonitor.hpp b/src/core/sm/networkmanager/itf/trafficmonitor.hpp index 488e9c1c5..f5566c532 100644 --- a/src/core/sm/networkmanager/itf/trafficmonitor.hpp +++ b/src/core/sm/networkmanager/itf/trafficmonitor.hpp @@ -104,6 +104,35 @@ class TrafficMonitorItf { */ virtual Error GetInstanceTraffic(const String& instanceID, uint64_t& inputTraffic, uint64_t& outputTraffic) const = 0; + + /** + * Opens a batch; StartInstanceMonitoring/StopInstanceMonitoring calls are staged until flush. + * + * @return Error. + */ + virtual Error BeginBatch() = 0; + + /** + * Flushes the staged batch atomically in a single nft transaction. + * + * @return Error. + */ + virtual Error FlushBatch() = 0; + + /** + * Discards the staged batch and leaves batch mode without applying anything to the kernel + * (unlike FlushBatch which commits, or Revert which undoes an already-applied batch). + * + * @return Error. + */ + virtual Error AbortBatch() = 0; + + /** + * Reverts the flushed batch, deleting everything it applied by handle. + * + * @return Error. + */ + virtual Error Revert() = 0; }; /** @}*/ diff --git a/src/core/sm/networkmanager/networkmanager.cpp b/src/core/sm/networkmanager/networkmanager.cpp index f9863b38a..b4242480a 100644 --- a/src/core/sm/networkmanager/networkmanager.cpp +++ b/src/core/sm/networkmanager/networkmanager.cpp @@ -285,6 +285,17 @@ Error NetworkManager::StartInstanceNetwork(const String& instanceID, const Strin err = AddInstanceToNetwork(instanceID, networkID, cachedInfo->mNetworkConfig, cachedInfo->mAllocatedParams); + if (err.IsNone()) { + LockGuard lock {mMutex}; + + if (mBatchMode) { + if (auto errBatch = mBatchEntries.PushBack({instanceID, networkID, BatchOp::eAdd}); !errBatch.IsNone()) { + LOG_ERR() << "Failed to register batch entry" << Log::Field("instanceID", instanceID) + << Log::Field(errBatch); + } + } + } + return err; } @@ -415,6 +426,17 @@ Error NetworkManager::StopInstanceNetwork(const String& instanceID, const String } } + { + LockGuard lock {mMutex}; + + if (mBatchMode) { + if (auto errBatch = mBatchEntries.PushBack({instanceID, networkID, BatchOp::eRemove}); !errBatch.IsNone()) { + LOG_ERR() << "Failed to register batch entry" << Log::Field("instanceID", instanceID) + << Log::Field(errBatch); + } + } + } + if (auto errRemove = RemoveInstanceFromCache(instanceID, networkID); !errRemove.IsNone() && err.IsNone()) { err = errRemove; } @@ -516,6 +538,190 @@ Error NetworkManager::ReleaseInstanceNetwork(const String& instanceID, const Str return ErrorEnum::eNone; } +Error NetworkManager::BeginBatch() +{ + Error err; + + { + LockGuard lock {mMutex}; + + mBatchEntries.Clear(); + mBatchMode = true; + } + + auto cleanupBatchMode = DeferRelease(this, [&err](NetworkManager* self) { + if (!err.IsNone()) { + LockGuard lock {self->mMutex}; + + self->mBatchMode = false; + } + }); + + if (err = mStorage->BeginTransaction(); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + auto cleanupStorage = DeferRelease(this, [&err](NetworkManager* self) { + if (!err.IsNone()) { + self->mStorage->RollbackTransaction(); + } + }); + + if (err = mFirewall->BeginBatch(); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + auto cleanupFirewall = DeferRelease(this, [&err](NetworkManager* self) { + if (!err.IsNone()) { + self->mFirewall->AbortBatch(); + } + }); + + if (err = mNetMonitor->BeginBatch(); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + return ErrorEnum::eNone; +} + +Error NetworkManager::FlushBatch(Array>& failedInstanceIDs) +{ + failedInstanceIDs.Clear(); + + if (auto err = mFirewall->FlushBatch(); !err.IsNone()) { + LOG_ERR() << "Failed to flush firewall batch" << Log::Field(err); + + mNetMonitor->AbortBatch(); + mStorage->RollbackTransaction(); + + ReapplyBatchEntries(failedInstanceIDs); + ClearBatchState(); + + return ErrorEnum::eNone; + } + + if (auto err = mNetMonitor->FlushBatch(); !err.IsNone()) { + LOG_ERR() << "Failed to flush traffic monitor batch" << Log::Field(err); + + mFirewall->Revert(); + mStorage->RollbackTransaction(); + + ReapplyBatchEntries(failedInstanceIDs); + ClearBatchState(); + + return ErrorEnum::eNone; + } + + if (auto err = mStorage->CommitTransaction(); !err.IsNone()) { + LOG_ERR() << "Failed to commit batch transaction" << Log::Field(err); + + mFirewall->Revert(); + mNetMonitor->Revert(); + mStorage->RollbackTransaction(); + + for (const auto& entry : mBatchEntries) { + failedInstanceIDs.PushBack(entry.mInstanceID); + } + } + + ClearBatchState(); + + return ErrorEnum::eNone; +} + +void NetworkManager::ReapplyBatchEntries(Array>& failedInstanceIDs) +{ + for (const auto& entry : mBatchEntries) { + if (auto err = ReapplyInstancePolicy(entry); !err.IsNone()) { + LOG_ERR() << "Failed to reapply instance policy" << Log::Field("instanceID", entry.mInstanceID) + << Log::Field(err); + + failedInstanceIDs.PushBack(entry.mInstanceID); + } + } +} + +void NetworkManager::ClearBatchState() +{ + LockGuard lock {mMutex}; + + mBatchMode = false; + mBatchEntries.Clear(); +} + +Error NetworkManager::ReapplyInstancePolicy(const BatchEntry& entry) +{ + if (entry.mOp == BatchOp::eRemove) { + Error err; + + if (auto errFW = mFirewall->RemoveInstance(entry.mInstanceID); !errFW.IsNone()) { + err = errFW; + } + + if (auto errTR = mNetMonitor->StopInstanceMonitoring(entry.mInstanceID); !errTR.IsNone() && err.IsNone()) { + err = errTR; + } + + return err; + } + + auto info = MakeUnique(&mAllocator); + + { + LockGuard lock {mMutex}; + + auto it = mInstanceNetworkInfos.Find(entry.mInstanceID); + if (it == mInstanceNetworkInfos.end()) { + return AOS_ERROR_WRAP(Error(ErrorEnum::eNotFound, "instance network info not found")); + } + + *info = it->mSecond; + } + + auto firewallParams = MakeUnique(&mAllocator); + + if (auto err = PrepareInstanceFirewallParams(info->mNetworkConfig, info->mAllocatedParams, *firewallParams); + !err.IsNone()) { + return err; + } + + Error err; + + if (err = mFirewall->AddInstance(entry.mInstanceID, *firewallParams); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + auto cleanupFirewall = DeferRelease(this, [&err, &entry](NetworkManager* self) { + if (!err.IsNone()) { + if (auto errRemove = self->mFirewall->RemoveInstance(entry.mInstanceID); !errRemove.IsNone()) { + LOG_ERR() << "Failed to remove firewall instance on rollback" + << Log::Field("instanceID", entry.mInstanceID) << Log::Field(errRemove); + } + } + }); + + if (err = mNetMonitor->StartInstanceMonitoring(entry.mInstanceID, info->mAllocatedParams.mIP, + info->mNetworkConfig.mDownloadLimit, info->mNetworkConfig.mUploadLimit); + !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + auto cleanupMonitoring = DeferRelease(this, [&err, &entry](NetworkManager* self) { + if (!err.IsNone()) { + if (auto errStop = self->mNetMonitor->StopInstanceMonitoring(entry.mInstanceID); !errStop.IsNone()) { + LOG_ERR() << "Failed to stop instance monitoring on rollback" + << Log::Field("instanceID", entry.mInstanceID) << Log::Field(errStop); + } + } + }); + + if (err = mStorage->UpdateInstanceNetworkInfo(*info); !err.IsNone()) { + return AOS_ERROR_WRAP(err); + } + + return ErrorEnum::eNone; +} + Error NetworkManager::PrepareUpdateItemNetworkParams( const InstanceNetworkConfig& params, const String& networkID, UpdateItemNetworkParams& serviceData) const { @@ -720,13 +926,16 @@ Error NetworkManager::AddInstanceToNetwork(const String& instanceID, const Strin return AOS_ERROR_WRAP(err); } - auto cleanupBandwidth = DeferRelease(&attachResult.mHostIfName, [this, &err](const String* ifName) { - if (!err.IsNone()) { - if (auto errClear = mBandwidth->Clear(*ifName); !errClear.IsNone()) { - LOG_ERR() << "Failed to clear bandwidth" << Log::Field("ifName", *ifName) << Log::Field(errClear); - } - } - }); + const bool bandwidthApplied = bandwidthParams->mIngressRate > 0 || bandwidthParams->mEgressRate > 0; + + auto cleanupBandwidth + = DeferRelease(&attachResult.mHostIfName, [this, &err, bandwidthApplied](const String* ifName) { + if (!err.IsNone() && bandwidthApplied) { + if (auto errClear = mBandwidth->Clear(*ifName); !errClear.IsNone()) { + LOG_ERR() << "Failed to clear bandwidth" << Log::Field("ifName", *ifName) << Log::Field(errClear); + } + } + }); DNSServerItf* dnsServer = nullptr; @@ -846,25 +1055,20 @@ Error NetworkManager::EnsureNodeNetworkPhysical(const String& networkID) Error NetworkManager::DeleteInstanceNetworkConfig(const String& instanceID, const String& networkID) { - StaticString bridgeIfName; StaticString hostIfName; - DNSServerItf* dnsServer = nullptr; + DNSServerItf* dnsServer = nullptr; + bool hasBandwidth = false; { LockGuard lock {mMutex}; - if (auto it = mNetworkProviders.Find(networkID); it != mNetworkProviders.end()) { - bridgeIfName = it->mSecond.mBridgeIfName; - } else { - LOG_WRN() << "Network provider not found for cleanup" << Log::Field("networkID", networkID); - } - if (auto it = mDNSServers.Find(networkID); it != mDNSServers.end()) { dnsServer = it->mSecond; } if (auto it = mInstanceNetworkInfos.Find(instanceID); it != mInstanceNetworkInfos.end()) { - hostIfName = it->mSecond.mHostIfName; + hostIfName = it->mSecond.mHostIfName; + hasBandwidth = it->mSecond.mNetworkConfig.mIngressKbit > 0 || it->mSecond.mNetworkConfig.mEgressKbit > 0; } else { LOG_WRN() << "Instance network info not found for cleanup" << Log::Field("instanceID", instanceID); } @@ -882,20 +1086,23 @@ Error NetworkManager::DeleteInstanceNetworkConfig(const String& instanceID, cons << Log::Field("networkID", networkID); } - if (auto errClear = mBandwidth->Clear(hostIfName); !errClear.IsNone() && err.IsNone()) { - err = AOS_ERROR_WRAP(errClear); + if (hasBandwidth) { + if (auto errClear = mBandwidth->Clear(hostIfName); !errClear.IsNone() && err.IsNone()) { + err = AOS_ERROR_WRAP(errClear); + } } if (auto errRemove = mFirewall->RemoveInstance(instanceID); !errRemove.IsNone() && err.IsNone()) { err = AOS_ERROR_WRAP(errRemove); } - if (!bridgeIfName.IsEmpty()) { - if (auto errDetach = mBridgeNetwork->Detach(instanceID, bridgeIfName); - !errDetach.IsNone() && err.IsNone()) { - err = AOS_ERROR_WRAP(errDetach); - } - } + // The host veth is intentionally NOT detached here. DeleteNetworkNamespace + // below drops the instance netns (lazy umount); the kernel then reaps the + // peer veth - and with it the host end, since they die as a pair - + // asynchronously via cleanup_net, off the critical stop path. A synchronous + // delete here would block on a per-device RCU grace period for every + // instance (O(N) rtnl_lock serialization on mass teardown) for no benefit, + // as the namespace teardown already removes the interface. } else { LOG_DBG() << "Instance was never started, skipping itf cleanup" << Log::Field("instanceID", instanceID); } @@ -1123,6 +1330,11 @@ Error NetworkManager::ReconcileInstances() if (alive) { if (err = InitInstance(entry.mInstanceID, entry.mNetworkID); err.IsNone()) { + if (auto dnsErr = AdoptDNSServer(entry.mNetworkID); !dnsErr.IsNone()) { + LOG_WRN() << "Failed to adopt DNS server for running instance" + << Log::Field("networkID", entry.mNetworkID) << Log::Field(dnsErr); + } + continue; } else { LOG_WRN() << "Failed to adopt leftover instance, falling back to cleanup" diff --git a/src/core/sm/networkmanager/networkmanager.hpp b/src/core/sm/networkmanager/networkmanager.hpp index 0c12f6500..eb58deb5e 100644 --- a/src/core/sm/networkmanager/networkmanager.hpp +++ b/src/core/sm/networkmanager/networkmanager.hpp @@ -165,6 +165,21 @@ class NetworkManager : public NetworkManagerItf { */ Error ReleaseInstanceNetwork(const String& instanceID, const String& networkID) override; + /** + * Opens a batch for start/stop operations. + * + * @return Error. + */ + Error BeginBatch() override; + + /** + * Flushes the staged batch. + * + * @param[out] failedInstanceIDs instances that were not applied. + * @return Error. + */ + Error FlushBatch(Array>& failedInstanceIDs) override; + /** * Called when pending firewall rules are resolved for an instance. * @@ -180,18 +195,18 @@ class NetworkManager : public NetworkManagerItf { void OnConnect() override; private: - Error EnsureNodeNetwork(const String& networkID); - Error EnsureNodeNetworkPhysical(const String& networkID); - Error UpdateInstanceFirewall(const String& instanceID, const String& networkID, - const InstanceNetworkConfig& networkConfig, const aos::InstanceNetworkAllocation& networkParams); - - Error AddInstanceToNetwork(const String& instanceID, const String& networkID, - const InstanceNetworkConfig& networkConfig, const aos::InstanceNetworkAllocation& networkParams); - using InstanceHosts = StaticArray, cMaxNumHosts>; using InstanceCache = StaticMap, InstanceHosts, cMaxNumInstances>; using NetworkCache = StaticMap, InstanceCache, cMaxNumOwners>; + enum class BatchOp { eAdd, eRemove }; + + struct BatchEntry { + StaticString mInstanceID; + StaticString mNetworkID; + BatchOp mOp; + }; + // StartInstanceNetwork keeps its cached InstanceNetworkInfo alive across the nested call to // AddInstanceToNetwork, which in turn allocates hosts, bridge/firewall/bandwidth/DNS params and // its own InstanceNetworkInfo before returning. That is the largest concurrent footprint of any @@ -237,6 +252,15 @@ class NetworkManager : public NetworkManagerItf { Error InitInstance(const String& instanceID, const String& networkID); Error ReconcileInstances(); Error RemoveFirewallOrphans(); + Error EnsureNodeNetwork(const String& networkID); + Error EnsureNodeNetworkPhysical(const String& networkID); + Error UpdateInstanceFirewall(const String& instanceID, const String& networkID, + const InstanceNetworkConfig& networkConfig, const aos::InstanceNetworkAllocation& networkParams); + Error AddInstanceToNetwork(const String& instanceID, const String& networkID, + const InstanceNetworkConfig& networkConfig, const aos::InstanceNetworkAllocation& networkParams); + Error ReapplyInstancePolicy(const BatchEntry& entry); + void ReapplyBatchEntries(Array>& failedInstanceIDs); + void ClearBatchState(); Error RemoveDNSOrphans(); Error AdoptDNSServer(const String& networkID); Error PrepareBridgeParams( @@ -296,6 +320,8 @@ class NetworkManager : public NetworkManagerItf { StaticMap, DNSServerItf*, cMaxNumOwners> mDNSServers; StaticMap, InstanceNetworkInfo, cMaxNumInstances * cMaxNumOwners> mInstanceNetworkInfos; StaticArray, cMaxNumOwners> mPhysicalNetworks; + bool mBatchMode {false}; + StaticArray mBatchEntries; StaticAllocator)> mNetworkInfosAllocator; StaticAllocator)> mInstanceNetworkInfosAllocator; diff --git a/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp b/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp index db9f668d4..325d83283 100644 --- a/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp +++ b/src/core/sm/networkmanager/tests/mocks/firewallmock.hpp @@ -23,6 +23,10 @@ class FirewallMock : public FirewallItf { MOCK_METHOD(Error, UpdateInstance, (const String&, const InstanceFirewallParams&), (override)); MOCK_METHOD(Error, AddMasquerade, (const String&, const String&), (override)); MOCK_METHOD(Error, RemoveMasquerade, (const String&, const String&), (override)); + MOCK_METHOD(Error, BeginBatch, (), (override)); + MOCK_METHOD(Error, FlushBatch, (), (override)); + MOCK_METHOD(Error, AbortBatch, (), (override)); + MOCK_METHOD(Error, Revert, (), (override)); }; } // namespace aos::sm::networkmanager diff --git a/src/core/sm/networkmanager/tests/mocks/trafficmonitormock.hpp b/src/core/sm/networkmanager/tests/mocks/trafficmonitormock.hpp index af96da377..0e6256cf8 100644 --- a/src/core/sm/networkmanager/tests/mocks/trafficmonitormock.hpp +++ b/src/core/sm/networkmanager/tests/mocks/trafficmonitormock.hpp @@ -22,6 +22,10 @@ class TrafficMonitorMock : public TrafficMonitorItf { MOCK_METHOD(Error, StopInstanceMonitoring, (const String&), (override)); MOCK_METHOD(Error, GetSystemTraffic, (uint64_t&, uint64_t&), (const, override)); MOCK_METHOD(Error, GetInstanceTraffic, (const String&, uint64_t&, uint64_t&), (const, override)); + MOCK_METHOD(Error, BeginBatch, (), (override)); + MOCK_METHOD(Error, FlushBatch, (), (override)); + MOCK_METHOD(Error, AbortBatch, (), (override)); + MOCK_METHOD(Error, Revert, (), (override)); }; } // namespace aos::sm::networkmanager diff --git a/src/core/sm/networkmanager/tests/networkmanager.cpp b/src/core/sm/networkmanager/tests/networkmanager.cpp index 382f351f5..8432aaa45 100644 --- a/src/core/sm/networkmanager/tests/networkmanager.cpp +++ b/src/core/sm/networkmanager/tests/networkmanager.cpp @@ -292,16 +292,19 @@ class NetworkManagerTest : public Test { 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)); + // The leftover instance has no bandwidth shaping, so no bandwidth clear is expected. + // The host veth is no longer detached synchronously on cleanup; the instance + // netns teardown reaps it asynchronously. 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); + // Adopting a running instance registers the network DNS server so later cleanup can reach it. + EXPECT_CALL(mDNSName, CreateServer(_, _)) + .WillOnce(Return(aos::RetWithError {&mDNSServer, aos::ErrorEnum::eNone})); EXPECT_CALL(mDNSServer, RemoveHost(_)).Times(0); EXPECT_CALL(mBandwidth, Clear(_)).Times(0); EXPECT_CALL(mFirewall, RemoveInstance(_)).Times(0); @@ -323,9 +326,7 @@ class NetworkManagerTest : public Test { void ExpectDeleteInstanceCalls(int times = 1) { EXPECT_CALL(mDNSServer, RemoveHost(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mBandwidth, Clear(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mFirewall, RemoveInstance(_)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); - EXPECT_CALL(mBridgeNetwork, Detach(_, _)).Times(times).WillRepeatedly(Return(aos::ErrorEnum::eNone)); } StrictMock mStorage; @@ -626,6 +627,234 @@ TEST_F(NetworkManagerTest, CreateAndStartInstanceNetwork_VerifyResolvConfFile) } } +TEST_F(NetworkManagerTest, BeginFlushBatch_ForwardToBackends) +{ + EXPECT_CALL(mStorage, BeginTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->BeginBatch(), aos::ErrorEnum::eNone); + + EXPECT_CALL(mFirewall, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mStorage, CommitTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, AbortBatch()).Times(0); + + aos::StaticArray, aos::cMaxNumInstances> failed; + + EXPECT_EQ(mNetManager->FlushBatch(failed), aos::ErrorEnum::eNone); + EXPECT_TRUE(failed.IsEmpty()); +} + +TEST_F(NetworkManagerTest, BeginBatch_PropagatesBackendError) +{ + EXPECT_CALL(mStorage, BeginTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eRuntime)); + EXPECT_CALL(mStorage, RollbackTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + + EXPECT_NE(mNetManager->BeginBatch(), aos::ErrorEnum::eNone); +} + +TEST_F(NetworkManagerTest, FlushBatch_NftFailure_RevertsAndFallsBack) +{ + const aos::String instanceID1 = "test-instance-1"; + const aos::String instanceID2 = "test-instance-2"; + const aos::String networkID = "test-network"; + auto params = CreateTestInstanceNetworkConfig(); + auto allocatedParams = CreateTestAllocatedParams(); + + SetupEnsureNodeNetworkCreateMocks(networkID, allocatedParams.mSubnet, "192.168.1.1", 100ULL); + + EXPECT_CALL(mNetworkProvider, AllocateInstanceNetwork(_, networkID, aos::String("test-node"), _, _)) + .Times(2) + .WillRepeatedly(DoAll(SetArgReferee<4>(allocatedParams), Return(aos::ErrorEnum::eNone))); + EXPECT_CALL(mStorage, AddInstanceNetworkInfo(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->CreateInstanceNetwork(instanceID1, networkID, params), aos::ErrorEnum::eNone); + + params.mHosts.Clear(); + params.mAliases.Clear(); + params.mHosts.PushBack(aos::Host {"10.0.0.3", "host3.example.com"}); + params.mAliases.PushBack("alias3"); + params.mHostname = "test-host-3"; + params.mInstanceIdent.mInstance = 1; + + ASSERT_EQ(mNetManager->CreateInstanceNetwork(instanceID2, networkID, params), aos::ErrorEnum::eNone); + + EXPECT_CALL(mStorage, BeginTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->BeginBatch(), aos::ErrorEnum::eNone); + + SetupEnsureNodeNetworkPhysicalMocks("192.168.1.1", allocatedParams.mSubnet, 100ULL); + + BridgeAttachResult attachResult; + attachResult.mHostIfName = "veth-test"; + attachResult.mContainerIfName = "eth0"; + + EXPECT_CALL(mBridgeNetwork, Attach(_, _, _)) + .Times(2) + .WillRepeatedly(DoAll(SetArgReferee<2>(attachResult), Return(aos::ErrorEnum::eNone))); + EXPECT_CALL(mBandwidth, Apply(_, _)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mDNSServer, AddHost(_, _)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, CreateNetworkNamespace(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, GetNetworkNamespacePath(_)) + .Times(2) + .WillRepeatedly(Return(aos::RetWithError> {{}, aos::ErrorEnum::eNone})); + + EXPECT_CALL(mFirewall, AddInstance(instanceID1, _)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, AddInstance(instanceID2, _)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, RemoveInstance(instanceID2)).WillOnce(Return(aos::ErrorEnum::eNone)); + + EXPECT_CALL(mTrafficMonitor, StartInstanceMonitoring(instanceID1, _, _, _)) + .Times(2) + .WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, StartInstanceMonitoring(instanceID2, _, _, _)) + .WillOnce(Return(aos::ErrorEnum::eNone)) + .WillOnce(Return(aos::ErrorEnum::eRuntime)); + + EXPECT_CALL(mStorage, UpdateInstanceNetworkInfo(_)).Times(3).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID1, networkID), aos::ErrorEnum::eNone); + ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID2, networkID), aos::ErrorEnum::eNone); + + EXPECT_CALL(mFirewall, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eRuntime)); + EXPECT_CALL(mTrafficMonitor, AbortBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, FlushBatch()).Times(0); + EXPECT_CALL(mStorage, RollbackTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + + aos::StaticArray, aos::cMaxNumInstances> failed; + + EXPECT_EQ(mNetManager->FlushBatch(failed), aos::ErrorEnum::eNone); + ASSERT_EQ(failed.Size(), 1U); + EXPECT_EQ(failed[0], instanceID2); +} + +TEST_F(NetworkManagerTest, FlushBatch_NftFailure_AbortsTrafficBatchBeforeReapply) +{ + const aos::String instanceID = "test-instance"; + const aos::String networkID = "test-network"; + auto params = CreateTestInstanceNetworkConfig(); + auto allocatedParams = CreateTestAllocatedParams(); + + SetupEnsureNodeNetworkCreateMocks(networkID, allocatedParams.mSubnet, "192.168.1.1", 100ULL); + + EXPECT_CALL(mNetworkProvider, AllocateInstanceNetwork(_, networkID, aos::String("test-node"), _, _)) + .WillOnce(DoAll(SetArgReferee<4>(allocatedParams), Return(aos::ErrorEnum::eNone))); + EXPECT_CALL(mStorage, AddInstanceNetworkInfo(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->CreateInstanceNetwork(instanceID, networkID, params), aos::ErrorEnum::eNone); + + EXPECT_CALL(mStorage, BeginTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->BeginBatch(), aos::ErrorEnum::eNone); + + SetupEnsureNodeNetworkPhysicalMocks("192.168.1.1", allocatedParams.mSubnet, 100ULL); + + BridgeAttachResult attachResult; + attachResult.mHostIfName = "veth-test"; + attachResult.mContainerIfName = "eth0"; + + EXPECT_CALL(mBridgeNetwork, Attach(_, _, _)) + .WillOnce(DoAll(SetArgReferee<2>(attachResult), Return(aos::ErrorEnum::eNone))); + EXPECT_CALL(mBandwidth, Apply(_, _)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mDNSServer, AddHost(_, _)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, CreateNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, GetNetworkNamespacePath(_)) + .WillOnce(Return(aos::RetWithError> {{}, aos::ErrorEnum::eNone})); + EXPECT_CALL(mFirewall, AddInstance(instanceID, _)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mStorage, UpdateInstanceNetworkInfo(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + + // The batch is dead once the firewall flush fails, so the traffic batch must be dropped before + // the per-instance fallback re-applies monitoring, otherwise the re-apply is staged and lost. + Sequence trafficSeq; + + EXPECT_CALL(mTrafficMonitor, StartInstanceMonitoring(instanceID, _, _, _)) + .InSequence(trafficSeq) + .WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, AbortBatch()).InSequence(trafficSeq).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, StartInstanceMonitoring(instanceID, _, _, _)) + .InSequence(trafficSeq) + .WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID, networkID), aos::ErrorEnum::eNone); + + EXPECT_CALL(mFirewall, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eRuntime)); + EXPECT_CALL(mTrafficMonitor, FlushBatch()).Times(0); + EXPECT_CALL(mStorage, RollbackTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + + aos::StaticArray, aos::cMaxNumInstances> failed; + + EXPECT_EQ(mNetManager->FlushBatch(failed), aos::ErrorEnum::eNone); + EXPECT_TRUE(failed.IsEmpty()); +} + +TEST_F(NetworkManagerTest, FlushBatch_CommitFailure_MarksAllFailed) +{ + const aos::String instanceID1 = "test-instance-1"; + const aos::String instanceID2 = "test-instance-2"; + const aos::String networkID = "test-network"; + auto params = CreateTestInstanceNetworkConfig(); + auto allocatedParams = CreateTestAllocatedParams(); + + SetupEnsureNodeNetworkCreateMocks(networkID, allocatedParams.mSubnet, "192.168.1.1", 100ULL); + + EXPECT_CALL(mNetworkProvider, AllocateInstanceNetwork(_, networkID, aos::String("test-node"), _, _)) + .Times(2) + .WillRepeatedly(DoAll(SetArgReferee<4>(allocatedParams), Return(aos::ErrorEnum::eNone))); + EXPECT_CALL(mStorage, AddInstanceNetworkInfo(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->CreateInstanceNetwork(instanceID1, networkID, params), aos::ErrorEnum::eNone); + + params.mHosts.Clear(); + params.mAliases.Clear(); + params.mHosts.PushBack(aos::Host {"10.0.0.3", "host3.example.com"}); + params.mAliases.PushBack("alias3"); + params.mHostname = "test-host-3"; + params.mInstanceIdent.mInstance = 1; + + ASSERT_EQ(mNetManager->CreateInstanceNetwork(instanceID2, networkID, params), aos::ErrorEnum::eNone); + + EXPECT_CALL(mStorage, BeginTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, BeginBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + + ASSERT_EQ(mNetManager->BeginBatch(), aos::ErrorEnum::eNone); + + SetupEnsureNodeNetworkPhysicalMocks("192.168.1.1", allocatedParams.mSubnet, 100ULL); + + ExpectAddInstanceCalls(2); + ExpectPersistInstanceCalls(2); + EXPECT_CALL(mTrafficMonitor, StartInstanceMonitoring(_, _, _, _)) + .Times(2) + .WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, CreateNetworkNamespace(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, GetNetworkNamespacePath(_)) + .Times(2) + .WillRepeatedly(Return(aos::RetWithError> {{}, aos::ErrorEnum::eNone})); + + ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID1, networkID), aos::ErrorEnum::eNone); + ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID2, networkID), aos::ErrorEnum::eNone); + + EXPECT_CALL(mFirewall, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, FlushBatch()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mStorage, CommitTransaction()).WillOnce(Return(aos::ErrorEnum::eRuntime)); + EXPECT_CALL(mFirewall, Revert()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, Revert()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mStorage, RollbackTransaction()).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mTrafficMonitor, AbortBatch()).Times(0); + + aos::StaticArray, aos::cMaxNumInstances> failed; + + EXPECT_EQ(mNetManager->FlushBatch(failed), aos::ErrorEnum::eNone); + ASSERT_EQ(failed.Size(), 2U); + EXPECT_EQ(failed[0], instanceID1); + EXPECT_EQ(failed[1], instanceID2); +} + TEST_F(NetworkManagerTest, StartInstanceNetwork_FailOnAttachError) { const aos::String instanceID = "test-instance"; @@ -682,6 +911,7 @@ TEST_F(NetworkManagerTest, StartInstanceNetwork_FailOnTrafficMonitorError) EXPECT_CALL(mNetns, GetNetworkNamespacePath(_)) .WillOnce(Return(aos::RetWithError> {{}, aos::ErrorEnum::eNone})); EXPECT_CALL(mNetns, DeleteNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mBridgeNetwork, Detach(_, _)).WillOnce(Return(aos::ErrorEnum::eNone)); ExpectDeleteInstanceCalls(); EXPECT_EQ(mNetManager->StartInstanceNetwork(instanceID, networkID), aos::ErrorEnum::eRuntime); @@ -896,7 +1126,7 @@ TEST_F(NetworkManagerTest, StopReleaseAndRecreateInstance) ASSERT_EQ(mNetManager->StartInstanceNetwork(instanceID, networkID), aos::ErrorEnum::eNone); } -TEST_F(NetworkManagerTest, StopInstanceNetwork_FailOnDetachError) +TEST_F(NetworkManagerTest, StopInstanceNetwork_FailOnFirewallRemoveError) { const aos::String instanceID = "test-instance"; const aos::String networkID = "test-network"; @@ -928,9 +1158,7 @@ TEST_F(NetworkManagerTest, StopInstanceNetwork_FailOnDetachError) EXPECT_CALL(mTrafficMonitor, StopInstanceMonitoring(instanceID)).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mDNSServer, RemoveHost(_)).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::eRuntime)); + EXPECT_CALL(mFirewall, RemoveInstance(_)).WillOnce(Return(aos::ErrorEnum::eRuntime)); EXPECT_CALL(mNetns, DeleteNetworkNamespace(_)).WillOnce(Return(aos::ErrorEnum::eNone)); EXPECT_CALL(mNetIf, DeleteLink(_)).Times(2).WillRepeatedly(Return(aos::ErrorEnum::eNone)); @@ -1448,6 +1676,44 @@ TEST_F(NetworkManagerTest, Start_KeepsLeftoverInstanceWithLiveInterface) mNetManager->StartInstanceNetwork(leftover.mInstanceID, network.mNetworkID).Is(aos::ErrorEnum::eAlreadyExist)); } +TEST_F(NetworkManagerTest, Start_AdoptsDNSServerForRunningInstanceCleanedOnStop) +{ + 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)); + + // Adopting the running instance must register the network DNS server. + EXPECT_CALL(mDNSName, CreateServer(network.mNetworkID, _)) + .WillOnce(Return(aos::RetWithError {&mDNSServer, aos::ErrorEnum::eNone})); + 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); + + // Stopping the adopted instance must reach the DNS server and drop its host entry. + EXPECT_CALL(mTrafficMonitor, StopInstanceMonitoring(leftover.mInstanceID)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mDNSServer, RemoveHost(leftover.mInstanceID)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mFirewall, RemoveInstance(leftover.mInstanceID)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mNetns, DeleteNetworkNamespace(leftover.mInstanceID)).WillOnce(Return(aos::ErrorEnum::eNone)); + EXPECT_CALL(mStorage, UpdateInstanceNetworkInfo(_)).WillOnce(Return(aos::ErrorEnum::eNone)); + + EXPECT_EQ(mNetManager->StopInstanceNetwork(leftover.mInstanceID, network.mNetworkID), aos::ErrorEnum::eNone); +} + TEST_F(NetworkManagerTest, Start_CleansLeftoverInstanceWhenNamespaceMissing) { const auto network = CreateTestNetworkInfo(); diff --git a/src/core/sm/tests/mocks/networkmanagermock.hpp b/src/core/sm/tests/mocks/networkmanagermock.hpp index 352deab33..ee9726132 100644 --- a/src/core/sm/tests/mocks/networkmanagermock.hpp +++ b/src/core/sm/tests/mocks/networkmanagermock.hpp @@ -28,6 +28,8 @@ class NetworkManagerMock : public NetworkManagerItf { MOCK_METHOD(Error, GetHosts, (const String& instanceID, Array& hosts), (const, override)); MOCK_METHOD(Error, StopInstanceNetwork, (const String& instanceID, const String& networkID), (override)); MOCK_METHOD(Error, ReleaseInstanceNetwork, (const String& instanceID, const String& networkID), (override)); + MOCK_METHOD(Error, BeginBatch, (), (override)); + MOCK_METHOD(Error, FlushBatch, (Array> & failedInstanceIDs), (override)); MOCK_METHOD(void, OnPendingFirewallUpdate, (const String& nodeID, const aos::networkmanager::PendingFirewallUpdate& update), (override)); MOCK_METHOD(void, OnConnect, (), (override)); diff --git a/src/core/sm/tests/mocks/storagemock.hpp b/src/core/sm/tests/mocks/storagemock.hpp index 9ee8195ab..eadbcc5a3 100644 --- a/src/core/sm/tests/mocks/storagemock.hpp +++ b/src/core/sm/tests/mocks/storagemock.hpp @@ -25,6 +25,9 @@ class StorageMock : public StorageItf { MOCK_METHOD(Error, SetTrafficMonitorData, (const String&, const Time&, uint64_t), (override)); MOCK_METHOD(Error, GetTrafficMonitorData, (const String&, Time&, uint64_t&), (const, override)); MOCK_METHOD(Error, RemoveTrafficMonitorData, (const String&), (override)); + MOCK_METHOD(Error, BeginTransaction, (), (override)); + MOCK_METHOD(Error, CommitTransaction, (), (override)); + MOCK_METHOD(Error, RollbackTransaction, (), (override)); }; } // namespace aos::sm::networkmanager