From 64d1f50a092d3206fb96ad369dae2817fbfbb47d Mon Sep 17 00:00:00 2001
From: Abdul Karim <Abdul.Karim@morganstanley.com>
Date: Tue, 5 Dec 2023 12:40:29 +0000
Subject: [PATCH] ncm-network: provide a better way to cleanup inactive
 connections

New approach to cleaning up inactive connections so have clean output after the run.

This also fixes an issue where active connnections named 'Wired connection ethX' are deleted
before network changes are applied. This is means in some edge cases
if first ncm-network run fails then you have no networking.
Therefore no need to delete any auto connections created by NetworkManager, instead
we just cleanup connections which are inactive at the end.

Wired connection ethX is created by NM automatically, to stop this one can install NetworkManage-config-server package,
which stops NM creating such auto connections automaticailly.

provide option to turned this off.
---
 .../pan/components/network/core-schema.pan    |  2 ++
 ncm-network/src/main/perl/nmstate.pm          | 34 +++++++++++--------
 ncm-network/src/test/perl/nmstate_simple.t    |  2 +-
 3 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/ncm-network/src/main/pan/components/network/core-schema.pan b/ncm-network/src/main/pan/components/network/core-schema.pan
index 47e4773f6e..5ce4940bb2 100644
--- a/ncm-network/src/main/pan/components/network/core-schema.pan
+++ b/ncm-network/src/main/pan/components/network/core-schema.pan
@@ -443,6 +443,8 @@ type structure_network = {
     "allow_nm" ? boolean
     @{let NetworkManager manage the dns (only for nmstate)}
     "nm_manage_dns" : boolean = false
+    @{let ncm-network cleanup inactive connections (only for nmstate)}
+    "nm_clean_inactive_conn" : boolean = true
     "primary_ip" ? string
     "routers" ? structure_router{}
     "ipv6" ? structure_ipv6
diff --git a/ncm-network/src/main/perl/nmstate.pm b/ncm-network/src/main/perl/nmstate.pm
index 029877b509..7b402bbdc4 100644
--- a/ncm-network/src/main/perl/nmstate.pm
+++ b/ncm-network/src/main/perl/nmstate.pm
@@ -480,21 +480,19 @@ sub is_active_interface
     return $found;
 }
 
-# check for existing connections, will clear the default connections created by 'NM with Wired connecton x'
+# check for existing connections, any connections which are not active.
 # good to have.
-sub clear_default_nm_connections
+sub clear_inactive_nm_connections
 {
     my ($self) = @_;
-    # NM creates auto connections with Wired connection x
-    # Delete all connections with name 'Wired connection', everything ncm-network creates will have connection name set to interface name.
-    my $output = $self->runrun([$NMCLI_CMD, "-t", "-f", "name", "conn"]);
-    my @existing_conn = split('\n', $output);
-    my %current_conn;
-    foreach  my $conn_name  (@existing_conn) {
-        $conn_name =~ s/\s+$//;
-        if ($conn_name =~ /Wired connection/){
-            $self->verbose("Clearing default connections created automatically by NetworkManager [ $conn_name ]");
-            $output = $self->runrun([$NMCLI_CMD,"conn", "delete", $conn_name]);
+    # clean any inactive connections
+    my $output = $self->runrun([$NMCLI_CMD, "-t", "-f", "uuid,device,name,state,active", "conn"]);
+    my @all_conn = split('\n', $output);
+    foreach  my $conn  (@all_conn) {
+        my ($uuid,$device,$name,$state,$active) = split(':', $conn);
+        if ($active eq 'no') {
+           $self->verbose("Clearing inactive connection for [ uuid=$uuid, name=$name, state=$state, active=$active ]");
+           $output = $self->runrun([$NMCLI_CMD,"conn", "delete", $uuid]);
             $self->verbose($output);
         }
     }
@@ -510,9 +508,7 @@ sub nmstate_apply
 
     if (@ifaces) {
         $self->info("Applying changes using $NMSTATECTL ", join(', ', @ifaces));
-        my @cmds;
-        # clear any connections created by NM with 'Wired connection x' to start fresh.
-        $self->clear_default_nm_connections();
+        my @cmds;     
         foreach my $iface (@ifaces) {
             # apply config using nmstatectl
             my $ymlfile = $self->iface_filename($iface);
@@ -738,6 +734,14 @@ sub Configure
         }
     };
 
+    # cleanup dangling inactive connections after ncm network changes are applied.
+    # defaults to cleanup
+    my $clean_inactive_conn = $net->{nm_clean_inactive_conn} || 1;
+    if ($clean_inactive_conn and $stopstart) {
+        # cleanup any inactive connections
+        $self->clear_inactive_nm_connections;
+    }
+
     # test network
     my $ccm_tree = $config->getTree("/software/components/ccm");
     my $profile = $ccm_tree && $ccm_tree->{profile};
diff --git a/ncm-network/src/test/perl/nmstate_simple.t b/ncm-network/src/test/perl/nmstate_simple.t
index a6dc97d926..813fd4a0db 100644
--- a/ncm-network/src/test/perl/nmstate_simple.t
+++ b/ncm-network/src/test/perl/nmstate_simple.t
@@ -104,7 +104,7 @@ ok(command_history_ok([
   '/usr/bin/nmcli connection',
   'service NetworkManager reload',
   'systemctl disable nmstate',
-  '/usr/bin/nmcli -t -f name conn',
+  '/usr/bin/nmcli -t -f uuid,device,name,state,active conn',
   '/usr/bin/nmstatectl apply /etc/nmstate/eth0.yml',
   '/usr/bin/nmstatectl apply /etc/nmstate/resolv.yml',
   'service NetworkManager reload',