Skip to content

Refactor network manager#528

Merged
al1img merged 10 commits intoaosedge:feature_release_9.1from
MykolaSuperman:refactor_network_manager
Mar 31, 2026
Merged

Refactor network manager#528
al1img merged 10 commits intoaosedge:feature_release_9.1from
MykolaSuperman:refactor_network_manager

Conversation

@MykolaSuperman
Copy link
Copy Markdown

No description provided.

@MykolaSuperman MykolaSuperman force-pushed the refactor_network_manager branch from 529ffc6 to f022560 Compare March 6, 2026 10:21
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 6, 2026

Codecov Report

❌ Patch coverage is 91.39559% with 82 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.25%. Comparing base (79d1152) to head (9711d96).
⚠️ Report is 37 commits behind head on feature_release_9.1.

Files with missing lines Patch % Lines
src/core/sm/networkmanager/networkmanager.cpp 78.14% 80 Missing ⚠️
...rc/core/sm/networkmanager/tests/networkmanager.cpp 99.57% 2 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                   @@
##           feature_release_9.1     #528      +/-   ##
=======================================================
- Coverage                85.25%   85.25%   -0.01%     
=======================================================
  Files                      311      313       +2     
  Lines                    27938    28209     +271     
  Branches                  3762     3775      +13     
=======================================================
+ Hits                     23819    24049     +230     
- Misses                    4119     4160      +41     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@MykolaSuperman MykolaSuperman force-pushed the refactor_network_manager branch 7 times, most recently from b90964f to 1aede72 Compare March 9, 2026 11:03
@@ -0,0 +1,75 @@
/*
* Copyright (C) 2025 EPAM Systems, Inc.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

2026

/**
* Network provider interface (SM -> CM network service).
*/
class NetworkProviderItf {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Add unit test for this interface and network.md file that describes the purpose if this interfae.

* SPDX-License-Identifier: Apache-2.0
*/

#ifndef AOS_CORE_COMMON_NETWORK_ITF_NETWORKPROVIDER_HPP_
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As it is part of networkmanager then it should be in networkmanager folder and networkmanager namespace.


void Balancer::Init(InstanceManager& instanceManager, imagemanager::ItemInfoProviderItf& itemInfoProvider,
oci::OCISpecItf& ociSpec, NodeManager& nodeManager, MonitoringProviderItf& monitorProvider,
InstanceRunnerItf& runner, NetworkManager& networkManager)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do launcher unit tests work after these changes?

StaticString<cSubnetLen> mSubnet;
StaticString<cIPLen> mIP;
uint64_t mVlanID {};
struct NetworkServiceData {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We don't have services any more. Rename to UpdateItemNetworkParameters.

Shortcut parameters to params:

NetworkParams
InstanceNetworkParams
UpdateItemNetworkParams

Can InstanceNetworkParams have the following fields:

struct InstanceNetworkParams {
    NetworkParams mNetworkParams;
    StaticArray<StaticString<cIPLen>, cMaxNumDNSServers> mDNSServers;
    StaticArray<FirewallRule, cMaxNumFirewallRules>      mFirewallRules;
   ...

?

* @param networks network parameters.
* @return Error.
* @param period traffic period.
* @return Error
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

period at the end

LOG_DBG() << "Update networks";
LOG_DBG() << "Get system traffic";

return AOS_ERROR_WRAP(mNetMonitor->GetSystemTraffic(inputTraffic, outputTraffic));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do not put operators in macro. It reduces readability.

{
LOG_DBG() << "Get instance traffic" << Log::Field("instanceID", instanceID);

return AOS_ERROR_WRAP(mNetMonitor->GetInstanceTraffic(instanceID, inputTraffic, outputTraffic));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ditto

*/
Error SetTrafficPeriod(TrafficPeriod period) override;

Error PrepareInstanceNetwork(const String& instanceID, const String& networkID,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comment public API

Error PrepareInstanceNetwork(const String& instanceID, const String& networkID,
const InstanceNetworkParameters& instanceNetworkParameters, const NetworkServiceData& serviceData) override;

Error CleanupInstanceNetwork(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ditto

@MykolaSuperman MykolaSuperman force-pushed the refactor_network_manager branch 5 times, most recently from c3523c3 to 3d896e4 Compare March 18, 2026 14:27
/**
* Instance network runtime parameters for Start (not stored in DB).
*/
struct InstanceNetworkRuntimeParameters {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

InstanceNetworkRuntimeParams

@MykolaSuperman MykolaSuperman force-pushed the refactor_network_manager branch 5 times, most recently from 5c3686d to af7d8f3 Compare March 25, 2026 14:55
@al1img al1img force-pushed the feature_release_9.1 branch from 79d1152 to febcb95 Compare March 30, 2026 07:51
# SPDX-License-Identifier: Apache-2.0
#

if(WITH_TEST)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Add headers target name etc. See other headers only modules in common.

@MykolaSuperman MykolaSuperman force-pushed the refactor_network_manager branch 3 times, most recently from 26723d8 to 692b8de Compare March 31, 2026 09:39
Copy link
Copy Markdown
Contributor

@mykola-kobets-epam mykola-kobets-epam left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Mykola Kobets <mykola_kobets@epam.com>


### NetworkProviderItf

Network provider interface used by SM to communicate with CM for network resource management.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Describe where it is used in CM why it places inb common.

bool operator!=(const FirewallRule& rule) const { return !operator==(rule); }
};

/**
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Put into separate header.

@MykolaSuperman MykolaSuperman force-pushed the refactor_network_manager branch 2 times, most recently from ca3c9c1 to 8445ff3 Compare March 31, 2026 12:28
@MykolaSuperman MykolaSuperman force-pushed the refactor_network_manager branch from 8445ff3 to 2262ff6 Compare March 31, 2026 14:13
Copy link
Copy Markdown
Collaborator

@al1img al1img left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Mykola Solianko added 10 commits March 31, 2026 17:48
- NetworkParameters -> NetworkParams
- InstanceNetworkParameters -> InstanceNetworkAllocation
- UpdateNetworkParameters -> UpdateItemNetworkParams (NetworkServiceData)

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>
Add network provider interface for SM to communicate with CM
for network allocation and release operations.

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>
- Remove CM networkmanager module (moved to SM-driven model)
- Update CM launcher to remove network management
- Update smcontroller to use NetworkProviderItf

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>
Add types.hpp with:
- InstanceNetworkConfig: network config for Create (stored in DB)
- InstanceNetworkRuntimeParameters: runtime paths for Start

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>
Replace PrepareInstanceNetwork/CleanupInstanceNetwork with 4 methods:
- CreateInstanceNetwork: CM allocation + DB (idempotent)
- StartInstanceNetwork: physical setup from cached params
- StopInstanceNetwork: physical cleanup, clears bridge/VLAN if last
- ReleaseInstanceNetwork: DB + CM release, requires Stop first

Key changes:
- InstanceNetworkInfo extended with InstanceNetworkConfig and
  InstanceNetworkAllocation for offline Start after reboot
- Init no longer removes instance network info from DB
- Init no longer creates bridge/VLAN (deferred to Start)
- EnsureNodeNetwork split into logical (CM) and physical parts
- Thread-safe access to mInstanceNetworkInfos cache
- networkID validation in Start/Stop/Release

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>
- Update all NM tests to use Create+Start / Stop+Release
- Add tests: networkID mismatch, Release without Stop, idempotent Create
- Update NetworkManagerMock with 4 methods
- Add NetworkProviderMock
- Update launcher tests for new API

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>
- Add PendingFirewallUpdate struct to network.hpp (instanceIdent + firewall rules)
- Add PendingUpdateHandlerItf to networkprovider.hpp for deferred firewall
  update notifications between CM and SM
- Add PendingUpdateHandlerMock for testing

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>
- Add UpdateFirewall to CNIItf interface for firewall-only CNI updates
  (DEL old + ADD new rules while preserving bridge/DNS/bandwidth cache)
- Update CNIMock with UpdateFirewall

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>
- NetworkManagerItf inherits PendingUpdateHandlerItf
- Add OnPendingFirewallUpdate: receives resolved pending rules from CM,
  updates storage and RAM cache atomically (storage first), applies
  firewall-only CNI update for running instances
- Add UpdateInstanceFirewall: retrieves cached CNI config, rebuilds
  firewall plugin config, calls CNI::UpdateFirewall
- Validate nodeID matches before processing update
- Update NetworkManagerMock with OnPendingFirewallUpdate

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>
- OnPendingFirewallUpdate_UpdatesFirewallRules: notification updates
  rules in storage for non-running instance
- OnPendingFirewallUpdate_RunningInstance_CallsCNIUpdate: notification
  triggers CNI UpdateFirewall for running instance

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 MykolaSuperman force-pushed the refactor_network_manager branch from 2262ff6 to 9711d96 Compare March 31, 2026 14:49
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@al1img al1img merged commit 61e5def into aosedge:feature_release_9.1 Mar 31, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants