[pve-devel] [PATCH pve-network 0/2] subnets: fix add|del subnets withtout ipam

[pve-devel] [PATCH pve-network 0/2] subnets: fix add|del subnets withtout ipam

[Alexandre Derumier]

a forum user have reported a bug when trying to delete a subnet on a zone without ipam

https://forum.proxmox.com/threads/sdn-subnets-l%C3%B6schen-geht-nicht-bug.88776/


Alexandre Derumier (2):
  subnets: tests: add noipam test
  subnets: fix add|del subnets withtout ipam

 PVE/Network/SDN/Subnets.pm      |   2 +
 test/run_test_subnets.pl        | 122 ++++++++++++++++++++------------
 test/subnets/noipam/ipam_config |  18 +++++
 test/subnets/noipam/sdn_config  |  20 ++++++
 4 files changed, 117 insertions(+), 45 deletions(-)
 create mode 100644 test/subnets/noipam/ipam_config
 create mode 100644 test/subnets/noipam/sdn_config

-- 
2.20.1

[pve-devel] [PATCH pve-network 1/2] subnets: tests: add noipam test

[Alexandre Derumier]

---
 test/run_test_subnets.pl        | 122 ++++++++++++++++++++------------
 test/subnets/noipam/ipam_config |  18 +++++
 test/subnets/noipam/sdn_config  |  20 ++++++
 3 files changed, 115 insertions(+), 45 deletions(-)
 create mode 100644 test/subnets/noipam/ipam_config
 create mode 100644 test/subnets/noipam/sdn_config

diff --git a/test/run_test_subnets.pl b/test/run_test_subnets.pl
index 6ffa6a3..364baa6 100755
--- a/test/run_test_subnets.pl
+++ b/test/run_test_subnets.pl
@@ -39,6 +39,7 @@ foreach my $path (@plugins) {
 
     my (undef, $testid) = split(/\//, $path);
 
+    print "test: $testid\n";
     my $sdn_config = read_sdn_config ("$path/sdn_config");
 
 
@@ -83,17 +84,31 @@ foreach my $path (@plugins) {
     my $ipamdb = {};
 
     my $zone = $sdn_config->{zones}->{ids}->{"myzone"};
+    my $ipam = $zone->{ipam};
+
+    my $plugin;
+    my $sdn_ipam_plugin;
+    if($ipam) {
+	$plugin = PVE::Network::SDN::Ipams::Plugin->lookup($ipam);
+	$sdn_ipam_plugin = Test::MockModule->new($plugin);
+	$sdn_ipam_plugin->mock(
+	    read_db => sub {
+		return $ipamdb;
+	    },
+	    write_db => sub {
+		my ($cfg) = @_;
+		$ipamdb = $cfg;
+	    }
+	);
+    }
 
-    my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup('pve');
-    my $sdn_ipam_plugin = Test::MockModule->new($plugin);
-    $sdn_ipam_plugin->mock(
-	read_db => sub {
-	    return $ipamdb;
+    my $pve_sdn_ipams;
+    $pve_sdn_ipams = Test::MockModule->new('PVE::Network::SDN::Ipams');
+    $pve_sdn_ipams->mock(
+	config => sub {
+	    my $ipam_config = read_sdn_config ("$path/ipam_config");
+	    return $ipam_config;
 	},
-	write_db => sub {
-	    my ($cfg) = @_;
-	    $ipamdb = $cfg;
-	}
     );
 
     ## add_subnet
@@ -103,14 +118,17 @@ foreach my $path (@plugins) {
     my $expected = '{"zones":{"myzone":{"subnets":{"'.$subnet_cidr.'":{"ips":{}}}}}}';
 
     eval {
-	$plugin->add_subnet(undef, $subnetid, $subnet, 1);
+        PVE::Network::SDN::Subnets::add_subnet($zone, $subnetid, $subnet);
+
     };
 
     if ($@) {
-        fail($name);
-    } else {
+        fail("$name : $@");
+    } elsif($ipam) {
         $result = $js->encode($plugin->read_db());
         is ($result, $expected, $name);
+    } else {
+        is (undef, undef, $name);
     }
 
     ## add_ip
@@ -125,23 +143,27 @@ foreach my $path (@plugins) {
 
     if ($@) {
         fail("$name : $@");
-    } else {
+    } elsif($ipam) {
         $result = $js->encode($plugin->read_db());
         is ($result, $expected, $name);
+    } else {
+        is (undef, undef, $name);
     }
 
-    ## add_already_exist_ip
-    $test = "add_already_exist_ip";
-    $name = "$testid $test";
+    if($ipam) {
+	## add_already_exist_ip
+	$test = "add_already_exist_ip";
+	$name = "$testid $test";
 
-    eval {
-	PVE::Network::SDN::Subnets::add_ip($zone, $subnetid, $subnet, $ip, $hostname, $mac, $description);
-    };
+	eval {
+	    PVE::Network::SDN::Subnets::add_ip($zone, $subnetid, $subnet, $ip, $hostname, $mac, $description);
+	};
 
-    if ($@) {
-        is (undef, undef, $name);
-    } else {
-        fail("$name : $@");
+	if ($@) {
+	    is (undef, undef, $name);
+	} else {
+	    fail("$name : $@");
+	}
     }
 
     ## add_second_ip
@@ -156,12 +178,13 @@ foreach my $path (@plugins) {
 
     if ($@) {
         fail("$name : $@");
-    } else {
+    } elsif($ipam) {
         $result = $js->encode($plugin->read_db());
         is ($result, $expected, $name);
+    } else {
+        is (undef, undef, $name);
     }
 
-
     ## add_next_free
     $test = "add_next_freeip";
     $name = "$testid $test";
@@ -174,7 +197,7 @@ foreach my $path (@plugins) {
 
     if ($@) {
         fail("$name : $@");
-    } else {
+    } elsif($ipam) {
         $result = $js->encode($plugin->read_db());
         is ($result, $expected, $name);
     }
@@ -191,27 +214,30 @@ foreach my $path (@plugins) {
 
     if ($@) {
         fail("$name : $@");
-    } else {
+    } elsif($ipam) {
         $result = $js->encode($plugin->read_db());
         is ($result, $expected, $name);
-    }
-
-    ## del_subnet_not_empty
-    $test = "del_subnet_not_empty";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = undef;
-
-    eval {
-	PVE::Network::SDN::Subnets::del_subnet($zone, $subnetid, $subnet);
-    };
-
-    if ($@) {
-        is ($result, $expected, $name);
     } else {
-        fail("$name : $@");
+        is (undef, undef, $name);
     }
 
+    if($ipam){
+	## del_subnet_not_empty
+	$test = "del_subnet_not_empty";
+	$name = "$testid $test";
+	$result = undef;
+	$expected = undef;
+
+	eval {
+	    PVE::Network::SDN::Subnets::del_subnet($zone, $subnetid, $subnet);
+	};
+
+	if ($@) {
+	    is ($result, $expected, $name);
+	} else {
+	    fail("$name : $@");
+	}
+    }
 
 
     ## add_ip_rollback_failing_dns
@@ -239,8 +265,12 @@ foreach my $path (@plugins) {
     };
 
     if ($@) {
-        $result = $js->encode($plugin->read_db());
-        is ($result, $expected, $name);
+	if($ipam) {
+	    $result = $js->encode($plugin->read_db());
+	    is ($result, $expected, $name);
+	} else {
+	    is (undef, undef, $name);
+	}
     } else {
         fail("$name : $@");
     }
@@ -261,9 +291,11 @@ foreach my $path (@plugins) {
 
     if ($@) {
         fail("$name : $@");
-    } else {
+    } elsif($ipam) {
         $result = $js->encode($plugin->read_db());
         is ($result, $expected, $name);
+    } else {
+        is (undef, undef, $name);
     }
 
 }
diff --git a/test/subnets/noipam/ipam_config b/test/subnets/noipam/ipam_config
new file mode 100644
index 0000000..a33be30
--- /dev/null
+++ b/test/subnets/noipam/ipam_config
@@ -0,0 +1,18 @@
+{
+          'ids' => {
+                     'phpipam' => {
+                                    'url' => 'https://localhost/api/apiadmin',
+                                    'type' => 'phpipam',
+                                    'section' => 1,
+                                    'token' => 'JPHkPSLB4O_XL-GQz4qtEFmNpx-99Htw'
+                                  },
+                     'pve' => {
+                                'type' => 'pve'
+                              },
+                     'netbox' => {
+                                   'token' => '0123456789abcdef0123456789abcdef01234567',
+                                   'type' => 'netbox',
+                                   'url' => 'http://localhost:8000/api'
+                                 }
+                   },
+}
diff --git a/test/subnets/noipam/sdn_config b/test/subnets/noipam/sdn_config
new file mode 100644
index 0000000..55107d6
--- /dev/null
+++ b/test/subnets/noipam/sdn_config
@@ -0,0 +1,20 @@
+{
+  version => 1,
+  vnets   => {
+               ids => {
+                        myvnet => { type => "vnet", zone => "myzone" },
+                      },
+             },
+
+  zones   => {
+               ids => { myzone => { type =>"simple" } },
+             },
+
+  subnets => {
+              ids => { 'myzone-10.0.0.0-24' => {
+                                                        'type' => 'subnet',
+                                                        'vnet' => 'myvnet',
+                                                  }
+                     }
+             }
+}
-- 
2.20.1

[pve-devel] [PATCH pve-network 2/2] subnets: fix add|del subnets withtout ipam

[Alexandre Derumier]

---
 PVE/Network/SDN/Subnets.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/PVE/Network/SDN/Subnets.pm b/PVE/Network/SDN/Subnets.pm
index 0cdadf7..46d9830 100644
--- a/PVE/Network/SDN/Subnets.pm
+++ b/PVE/Network/SDN/Subnets.pm
@@ -165,6 +165,7 @@ sub add_subnet {
     my ($zone, $subnetid, $subnet) = @_;
 
     my $ipam = $zone->{ipam};
+    return if !$ipam;
     my $ipam_cfg = PVE::Network::SDN::Ipams::config();
     my $plugin_config = $ipam_cfg->{ids}->{$ipam};
     my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
@@ -175,6 +176,7 @@ sub del_subnet {
     my ($zone, $subnetid, $subnet) = @_;
 
     my $ipam = $zone->{ipam};
+    return if !$ipam;
     my $ipam_cfg = PVE::Network::SDN::Ipams::config();
     my $plugin_config = $ipam_cfg->{ids}->{$ipam};
     my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
-- 
2.20.1

[pve-devel] applied-series: [PATCH pve-network 0/2] subnets: fix add|del subnets withtout ipam

[Thomas Lamprecht]

On 08.05.21 07:51, Alexandre Derumier wrote:
> a forum user have reported a bug when trying to delete a subnet on a zone without ipam
> 
> https://forum.proxmox.com/threads/sdn-subnets-l%C3%B6schen-geht-nicht-bug.88776/
> 
> 
> Alexandre Derumier (2):
>   subnets: tests: add noipam test
>   subnets: fix add|del subnets withtout ipam
> 
>  PVE/Network/SDN/Subnets.pm      |   2 +
>  test/run_test_subnets.pl        | 122 ++++++++++++++++++++------------
>  test/subnets/noipam/ipam_config |  18 +++++
>  test/subnets/noipam/sdn_config  |  20 ++++++
>  4 files changed, 117 insertions(+), 45 deletions(-)
>  create mode 100644 test/subnets/noipam/ipam_config
>  create mode 100644 test/subnets/noipam/sdn_config
> 



applied, thanks!