Adopt network to running instance - #635
Merged
MykolaSuperman merged 6 commits intoAug 3, 2026
Merged
Conversation
mlohvynenko
approved these changes
Jul 31, 2026
mlohvynenko
left a comment
mlohvynenko
left a comment
Member
There was a problem hiding this comment.
Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
mykola-kobets-epam
approved these changes
Jul 31, 2026
mykola-kobets-epam
left a comment
mykola-kobets-epam
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>
al1img
reviewed
Jul 31, 2026
| /** | ||
| * Bridge link kind. | ||
| */ | ||
| static constexpr auto cLinkKindBridge = "bridge"; |
Collaborator
There was a problem hiding this comment.
Probably better to make it as Aos Enum?
| Error AddInstanceToCache(const String& instanceID, const String& networkID); | ||
| RetWithError<bool> IsInstanceInterfaceAlive( | ||
| const String& instanceID, const String& hostIfName, const String& bridgeIfName) const; | ||
| Error AdoptInstance(const String& instanceID, const String& networkID); |
| } | ||
| } | ||
|
|
||
| if (auto errDNS = AdoptDNSServer(entry.mNetworkID); !errDNS.IsNone()) { |
Collaborator
There was a problem hiding this comment.
Use err =
and in the following condition
|
|
||
| auto cleanupMonitoring = DeferRelease(&instanceID, [this, &err](const String* id) { | ||
| if (!err.IsNone()) { | ||
| if (auto errStop = mNetMonitor->StopInstanceMonitoring(*id); !errStop.IsNone()) { |
Author
There was a problem hiding this comment.
no, assigning to err here would disarm the other defers: they all guard on !err.IsNone() and are destroyed after this one, so a successful StopInstanceMonitoring would reset err and silently skip the remaining rollbacks.
MykolaSuperman
force-pushed
the
adopt_network_to_running_instance
branch
from
August 2, 2026 08:21
9ffe3e6 to
cb3a210
Compare
al1img
approved these changes
Aug 3, 2026
al1img
left a comment
al1img
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
added 6 commits
August 3, 2026 11:18
InterfaceManagerItf exposed only mutating operations, so there was no way to tell an interface that must be created from one that is already on the system and can be adopted as is. Same for network namespaces. Add InterfaceManagerItf::GetLink returning link kind, master, vlan ID and admin state (eNotFound when the link is absent) and NamespaceManagerItf::IsNetworkNamespaceExist. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
After a crash SM restarts with an empty mPhysicalNetworks, so the first StartInstanceNetwork ran CreateNetwork over links that are still up. Recreating them is not a no-op: rtnl_link_add is issued without NLM_F_EXCL, so the kernel treats it as a modify request and CreateVlan pushes a freshly generated MAC onto the live vlan, breaking traffic of the instances still running on it. Probe each link with the new InterfaceManagerItf::GetLink and create only what is missing. Rollback deletes a link only when this call created it, so a failure midway no longer tears down an adopted one. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
CleanupLeftoverInstances tore down every instance recorded in storage on Start: it detached the host veth and deleted the network namespace even when the instance was alive and correctly wired, so an SM crash cut the network of containers that kept running. Replace it with ReconcileInstances, which checks the system before acting. An instance whose host veth is up, is a veth and is enslaved to its own bridge, and whose network namespace still exists, is adopted: nothing on the system is touched, only the runtime cache is restored. Everything else keeps the previous teardown path. Adopting into the runtime cache also makes the launcher restart flow work: StartInstanceNetwork now short-circuits with eAlreadyExist, which the launcher already tolerates. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
DeleteInstanceNetworkConfig removes the DNS record through the handle in mDNSServers, which is runtime state and is empty after a restart. Without a handle a dead leftover instance kept its addnhosts record. Adopt the handle before the cleanup again, now that DNSServer::Init loads the existing records instead of truncating them and so no longer drops the records of instances adopted on the same network. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
FirewallItf gains RemoveOrphans so that the firewall no longer has to wipe its whole table on start to get rid of what a crashed SM left behind. Call it from Start with the instances and networks known from storage, so artifacts of anything gone are removed while the rules protecting the instances that kept running stay in place. Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
Signed-off-by: Mykola Solianko <mykola_solianko@epam.com> Reviewed-by: Mykola Kobets <mykola_kobets@epam.com> Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com> Reviewed-by: Mykhailo Lohvynenko <mykhailo_lohvynenko@epam.com>
MykolaSuperman
force-pushed
the
adopt_network_to_running_instance
branch
from
August 3, 2026 08:19
cb3a210 to
beaa5b2
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.