[pve-devel] [PATCH v2 pve-network 0/9] SDN: Testing VNets as a blackbox.

[pve-devel] [PATCH v2 pve-network 0/9] SDN: Testing VNets as a blackbox.

[Stefan Lendl]

This add several tests for SDN VNets.
State setup as well as testing results is done only via the API to test on the
API boundaries and not against the internal state. Internal state and config
files are mocked to avoid requiring access to system files or pmxcfs.

The first 7 commits extract various functions to allow mocking them in the
tests. The tests are then added as the test_vnets_blackbox test.
The last commit removes the old vnets tests which are not working anyway.

Tests validate the events of a nic joining a Vnet or a nic staring on a vnet
with different subnet configurations. Further descriptions in the commit.

*Several of the tests fail due to known bugs and limitations.*
These bugs where discussed off-list with @s.hanreich and due to be fixed with
the next set of SDN patches.
Applying the tests first would allow fixing them with already existing
tests.

Differences v1 -> v2:
* Add tests that expect a failure when no IP can be allocated
* Removed commented out debug stuff

Stefan Lendl (9):
  sdn: extract function that reads datacenter config
  dnsmasq: extract function to systemctl command.
  dnsmasq: extract function that generates the ethers file path
  dnsmasq: extract function that updates dnsmasq lease via dbus
  controllers: extract function that reads network intreaces config
  evpn: extract function that reads frr config.
  api: extract function that creates the sdn directory.
  tests: test VNets functionality as a blackbox
  tests: remove old Vnets tests

 src/PVE/API2/Network/SDN/Zones.pm             |   6 +-
 src/PVE/Network/SDN/Controllers.pm            |  16 +-
 src/PVE/Network/SDN/Controllers/EvpnPlugin.pm |  10 +-
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm           |  47 +-
 src/PVE/Network/SDN/Zones/EvpnPlugin.pm       |   3 +-
 src/PVE/Network/SDN/Zones/Plugin.pm           |   5 +
 src/PVE/Network/SDN/Zones/SimplePlugin.pm     |   2 +-
 src/test/Makefile                             |   5 +-
 src/test/run_test_vnets.pl                    | 343 -------
 src/test/run_test_vnets_blackbox.pl           | 859 ++++++++++++++++++
 10 files changed, 925 insertions(+), 371 deletions(-)
 delete mode 100755 src/test/run_test_vnets.pl
 create mode 100755 src/test/run_test_vnets_blackbox.pl

-- 
2.44.0

[pve-devel] [PATCH v2 pve-network 1/9] sdn: extract function that reads datacenter config

[Stefan Lendl]

The datacenter_config() functions in SDN::Zones::Plugin is a simple
wrapper that reads datacenter.cfg via cfs.
This allows mocking datacenter.cfg in tests.

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
Tested-by: Max Carrara <m.carrara@proxmox.com>
---
 src/PVE/Network/SDN/Zones/EvpnPlugin.pm   | 3 ++-
 src/PVE/Network/SDN/Zones/Plugin.pm       | 5 +++++
 src/PVE/Network/SDN/Zones/SimplePlugin.pm | 2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
index 655a9f0..4843756 100644
--- a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
+++ b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
@@ -3,6 +3,7 @@ package PVE::Network::SDN::Zones::EvpnPlugin;
 use strict;
 use warnings;
 use PVE::Network::SDN::Zones::VxlanPlugin;
+use PVE::Network::SDN::Zones::Plugin;
 use PVE::Exception qw(raise raise_param_exc);
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::Tools qw($IPV4RE);
@@ -294,7 +295,7 @@ sub on_update_hook {
     }
 
     if (!defined($zone_cfg->{ids}->{$zoneid}->{'mac'})) {
-	my $dc = PVE::Cluster::cfs_read_file('datacenter.cfg');
+        my $dc = PVE::Network::SDN::Zones::Plugin->datacenter_config();
 	$zone_cfg->{ids}->{$zoneid}->{'mac'} = PVE::Tools::random_ether_addr($dc->{mac_prefix});
     }
 }
diff --git a/src/PVE/Network/SDN/Zones/Plugin.pm b/src/PVE/Network/SDN/Zones/Plugin.pm
index b55b967..247d0b2 100644
--- a/src/PVE/Network/SDN/Zones/Plugin.pm
+++ b/src/PVE/Network/SDN/Zones/Plugin.pm
@@ -356,4 +356,9 @@ sub get_bridge_ifaces {
 
     return @bridge_ifaces;
 }
+
+sub datacenter_config {
+    return PVE::Cluster::cfs_read_file('datacenter.cfg');
+}
+
 1;
diff --git a/src/PVE/Network/SDN/Zones/SimplePlugin.pm b/src/PVE/Network/SDN/Zones/SimplePlugin.pm
index c996bf3..65e9ad4 100644
--- a/src/PVE/Network/SDN/Zones/SimplePlugin.pm
+++ b/src/PVE/Network/SDN/Zones/SimplePlugin.pm
@@ -139,7 +139,7 @@ sub vnet_update_hook {
     raise_param_exc({ tag => "vlan tag is not allowed on simple zone"}) if defined($tag);
 
     if (!defined($vnet->{mac})) {
-        my $dc = PVE::Cluster::cfs_read_file('datacenter.cfg');
+        my $dc = PVE::Network::SDN::Zones::Plugin::datacenter_config();
         $vnet->{mac} = PVE::Tools::random_ether_addr($dc->{mac_prefix});
     }
 }
-- 
2.44.0

[pve-devel] [PATCH v2 pve-network 2/9] dnsmasq: extract function to systemctl command.

[Stefan Lendl]

systemctl_service() is a wrapper around PVE::Tools::run_command to allow
mocking the systemctl interactions in tests.

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
Tested-by: Max Carrara <m.carrara@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 2844943..f9f1c39 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -101,7 +101,7 @@ sub add_ip_mapping {
     }
 
     my $service_name = "dnsmasq\@$dhcpid";
-    PVE::Tools::run_command(['systemctl', 'reload', $service_name]) if $reload;
+    systemctl_service('reload', $service_name) if $reload;
 
     #update lease as ip could still be associated to an old removed mac
     my $bus = Net::DBus->system();
@@ -163,6 +163,12 @@ sub configure_vnet {
     );
 }
 
+sub systemctl_service {
+    my ($action, $service) = @_;
+
+    PVE::Tools::run_command(['systemctl', $action, $service]);
+}
+
 sub before_configure {
     my ($class, $dhcpid) = @_;
 
@@ -250,9 +256,9 @@ sub after_configure {
 
     my $service_name = "dnsmasq\@$dhcpid";
 
-    PVE::Tools::run_command(['systemctl', 'reload', 'dbus']);
-    PVE::Tools::run_command(['systemctl', 'enable', $service_name]);
-    PVE::Tools::run_command(['systemctl', 'restart', $service_name]);
+    systemctl_service('reload', 'dbus');
+    systemctl_service('enable', $service_name);
+    systemctl_service('restart', $service_name);
 }
 
 sub before_regenerate {
@@ -260,8 +266,8 @@ sub before_regenerate {
 
     return if !assert_dnsmasq_installed($noerr);
 
-    PVE::Tools::run_command(['systemctl', 'stop', "dnsmasq@*"]);
-    PVE::Tools::run_command(['systemctl', 'disable', 'dnsmasq@']);
+    systemctl_service('stop', "dnsmasq@*");
+    systemctl_service('disable', 'dnsmasq@');
 }
 
 sub after_regenerate {
-- 
2.44.0

[pve-devel] [PATCH v2 pve-network 3/9] dnsmasq: extract function that generates the ethers file path

[Stefan Lendl]

Extracted to a function so it can be mocked in tests.

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
Tested-by: Max Carrara <m.carrara@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index f9f1c39..5a227ba 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -33,10 +33,15 @@ my sub assert_dnsmasq_installed {
     return 1;
 }
 
+sub ethers_file {
+    my ($dhcpid) = @_;
+    return "$DNSMASQ_CONFIG_ROOT/$dhcpid/ethers";
+}
+
 sub add_ip_mapping {
     my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
 
-    my $ethers_file = "$DNSMASQ_CONFIG_ROOT/$dhcpid/ethers";
+    my $ethers_file = ethers_file($dhcpid);
     my $ethers_tmp_file = "$ethers_file.tmp";
 
     my $reload = undef;
-- 
2.44.0

[pve-devel] [PATCH v2 pve-network 4/9] dnsmasq: extract function that updates dnsmasq lease via dbus

[Stefan Lendl]

Extract the dbus based interactions with dnsmasq so that it can be
mocked in tests.

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
Tested-by: Max Carrara <m.carrara@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 5a227ba..c14f5d7 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -38,6 +38,17 @@ sub ethers_file {
     return "$DNSMASQ_CONFIG_ROOT/$dhcpid/ethers";
 }
 
+sub update_lease {
+    my ($dhcpid, $ip4, $mac) = @_;
+    #update lease as ip could still be associated to an old removed mac
+    my $bus = Net::DBus->system();
+    my $dnsmasq = $bus->get_service("uk.org.thekelleys.dnsmasq.$dhcpid");
+    my $manager = $dnsmasq->get_object("/uk/org/thekelleys/dnsmasq","uk.org.thekelleys.dnsmasq.$dhcpid");
+
+    my @hostname = unpack("C*", "*");
+    $manager->AddDhcpLease($ip4, $mac, \@hostname, undef, 0, 0, 0) if $ip4;
+}
+
 sub add_ip_mapping {
     my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
 
@@ -107,16 +118,7 @@ sub add_ip_mapping {
 
     my $service_name = "dnsmasq\@$dhcpid";
     systemctl_service('reload', $service_name) if $reload;
-
-    #update lease as ip could still be associated to an old removed mac
-    my $bus = Net::DBus->system();
-    my $dnsmasq = $bus->get_service("uk.org.thekelleys.dnsmasq.$dhcpid");
-    my $manager = $dnsmasq->get_object("/uk/org/thekelleys/dnsmasq","uk.org.thekelleys.dnsmasq.$dhcpid");
-
-    my @hostname = unpack("C*", "*");
-    $manager->AddDhcpLease($ip4, $mac, \@hostname, undef, 0, 0, 0) if $ip4;
-#    $manager->AddDhcpLease($ip6, $mac, \@hostname, undef, 0, 0, 0) if $ip6;
-
+    update_lease($dhcpid, $ip4, $mac);
 }
 
 sub configure_subnet {
-- 
2.44.0

[pve-devel] [PATCH v2 pve-network 5/9] controllers: extract function that reads network intreaces config

[Stefan Lendl]

read_etc_network_interfaces reads /etc/network/interfaces.
This allows mocking access to local fs.

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
Tested-by: Max Carrara <m.carrara@proxmox.com>
---
 src/PVE/Network/SDN/Controllers.pm | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/PVE/Network/SDN/Controllers.pm b/src/PVE/Network/SDN/Controllers.pm
index 167d3ea..fd7ad54 100644
--- a/src/PVE/Network/SDN/Controllers.pm
+++ b/src/PVE/Network/SDN/Controllers.pm
@@ -70,6 +70,16 @@ sub complete_sdn_controller {
     return  $cmdname eq 'add' ? [] : [ PVE::Network::SDN::sdn_controllers_ids($cfg) ];
 }
 
+sub read_etc_network_interfaces {
+    # read main config for physical interfaces
+    my $current_config_file = "/etc/network/interfaces";
+    my $fh = IO::File->new($current_config_file) or die "failed to open $current_config_file - $!\n";
+    my $interfaces_config = PVE::INotify::read_etc_network_interfaces($current_config_file, $fh);
+    $fh->close();
+
+    return $interfaces_config;
+}
+
 sub generate_controller_config {
 
     my $cfg = PVE::Network::SDN::running_config();
@@ -79,11 +89,7 @@ sub generate_controller_config {
 
     return if !$vnet_cfg && !$zone_cfg && !$controller_cfg;
 
-    # read main config for physical interfaces
-    my $current_config_file = "/etc/network/interfaces";
-    my $fh = IO::File->new($current_config_file) or die "failed to open $current_config_file - $!\n";
-    my $interfaces_config = PVE::INotify::read_etc_network_interfaces($current_config_file, $fh);
-    $fh->close();
+    my $interfaces_config = read_etc_network_interfaces();
 
     # check uplinks
     my $uplinks = {};
-- 
2.44.0

[pve-devel] [PATCH v2 pve-network 6/9] evpn: extract function that reads frr config.

[Stefan Lendl]

read_local_frr_config reads /etc/frr/frr.conf.local.
This allows mocking local fs access in tests.

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
Tested-by: Max Carrara <m.carrara@proxmox.com>
---
 src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
index c2fdf88..836a689 100644
--- a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
+++ b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
@@ -487,6 +487,12 @@ sub generate_frr_list {
     }
 }
 
+sub read_local_frr_config {
+    if (-e "/etc/frr/frr.conf.local") {
+	return file_get_contents("/etc/frr/frr.conf.local");
+    }
+};
+
 sub generate_controller_rawconfig {
     my ($class, $plugin_config, $config) = @_;
 
@@ -500,8 +506,8 @@ sub generate_controller_rawconfig {
     push @{$final_config}, "service integrated-vtysh-config";
     push @{$final_config}, "!";
 
-    if (-e "/etc/frr/frr.conf.local") {
-	my $local_conf = file_get_contents("/etc/frr/frr.conf.local");
+    my $local_conf = read_local_frr_config();
+    if ($local_conf) {
 	parse_merge_frr_local_config($config, $local_conf);
     }
 
-- 
2.44.0

[pve-devel] [PATCH v2 pve-network 7/9] api: extract function that creates the sdn directory.

[Stefan Lendl]

create_etc_interfaces_sdn_dir creates the /etc/pve/sdn directory.
This allows mocking in tests to prevent system fs access in tests

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
Tested-by: Max Carrara <m.carrara@proxmox.com>
---
 src/PVE/API2/Network/SDN/Zones.pm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/Network/SDN/Zones.pm b/src/PVE/API2/Network/SDN/Zones.pm
index b09c9ad..35e2f7f 100644
--- a/src/PVE/API2/Network/SDN/Zones.pm
+++ b/src/PVE/API2/Network/SDN/Zones.pm
@@ -186,6 +186,10 @@ __PACKAGE__->register_method ({
 	return &$api_sdn_zones_config($cfg, $param->{zone});
     }});
 
+sub create_etc_interfaces_sdn_dir {
+    mkdir("/etc/pve/sdn");
+}
+
 __PACKAGE__->register_method ({
     name => 'create',
     protected => 1,
@@ -207,7 +211,7 @@ __PACKAGE__->register_method ({
 	my $opts = $plugin->check_config($id, $param, 1, 1);
 
 	PVE::Cluster::check_cfs_quorum();
-	mkdir("/etc/pve/sdn");
+	create_etc_interfaces_sdn_dir();
 
 	PVE::Network::SDN::lock_sdn_config(sub {
 	    my $zone_cfg = PVE::Network::SDN::Zones::config();
-- 
2.44.0

[pve-devel] [PATCH v2 pve-network 8/9] tests: test VNets functionality as a blackbox

[Stefan Lendl]

Add several tests for Vnets in test_vnets_blackbox. State setup as well
as testing results is done only via the API to test on the API
boundaries not not against the internal state. Internal state is mocked
to avoid requiring access to system files or pmxcfs.

Mocking is done by reading and writing to a hash that holds the entire
state of SDN. The state is reset after every test run.

Testing is done via helper functions: nic_join and nic_start.
When a nic joins a Vnet, currently it always - and only - calls
add_next_free_cidr(). The same is true if a nic starts on Vnet, which
only calles add_dhcp_mapping.

These test functions homogenize the parameter list in contrast to the
current calls to the current functions.  The intention for the functions
is that they can be moved to Vnets.pm to be called from QemuServer and
LXC!

The tests are composed of a test function which can be parameterized. To
call the test function, the run_test function takes the function pointer
and passes the rest of the arguments to the test functions. It also
takes care of resetting the test state.
This allows fine-grained parameterization per-test directly in the code
instead of separated files that require the entire state to be passed
in.

The tests setup the SDN by creating a simple zone and a simple vnet. The
nic_join and nic_start function is called with different subnet
configuration wiht and without a dhcp-range configured and with or
without an already present IP in the IPAM.

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
Tested-by: Max Carrara <m.carrara@proxmox.com>
---
 src/test/Makefile                   |   5 +-
 src/test/run_test_vnets_blackbox.pl | 859 ++++++++++++++++++++++++++++
 2 files changed, 863 insertions(+), 1 deletion(-)
 create mode 100755 src/test/run_test_vnets_blackbox.pl

diff --git a/src/test/Makefile b/src/test/Makefile
index eb59d5f..5a937a4 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -1,6 +1,6 @@
 all: test
 
-test: test_zones test_ipams test_dns test_subnets
+test: test_zones test_ipams test_dns test_subnets test_vnets_blackbox
 
 test_zones: run_test_zones.pl
 	./run_test_zones.pl
@@ -14,4 +14,7 @@ test_dns: run_test_dns.pl
 test_subnets: run_test_subnets.pl
 	./run_test_subnets.pl
 
+test_vnets_blackbox: run_test_vnets_blackbox.pl
+	./run_test_vnets_blackbox.pl
+
 clean:
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
new file mode 100755
index 0000000..18cb94f
--- /dev/null
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -0,0 +1,859 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use lib qw(..);
+use File::Slurp;
+use List::Util qw(first all);
+use NetAddr::IP qw(:lower);
+
+use Test::More;
+use Test::MockModule;
+
+use PVE::Tools qw(extract_param file_set_contents);
+
+use PVE::Network::SDN;
+use PVE::Network::SDN::Zones;
+use PVE::Network::SDN::Zones::Plugin;
+use PVE::Network::SDN::Controllers;
+use PVE::Network::SDN::Dns;
+use PVE::Network::SDN::Vnets;
+
+use PVE::RESTEnvironment;
+
+use PVE::API2::Network::SDN::Zones;
+use PVE::API2::Network::SDN::Subnets;
+use PVE::API2::Network::SDN::Vnets;
+use PVE::API2::Network::SDN::Ipams;
+
+my $TMP_ETHERS_FILE = "/tmp/ethers";
+
+my $test_state = undef;
+sub clear_test_state {
+    $test_state = {
+	locks => {},
+	datacenter_config => {},
+	subnets_config => {},
+	controller_config => {},
+	dns_config => {},
+	zones_config => {},
+	vnets_config => {},
+	macdb => {},
+	ipamdb => {},
+	ipam_config => {
+	    'ids' => {
+		'pve' => {
+		    'type' => 'pve'
+		},
+	    }
+	},
+    };
+    PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "\n");
+}
+clear_test_state();
+
+my $mocked_cfs_lock_file = sub {
+    my ($filename, $timeout, $code, @param) = @_;
+
+    die "$filename already locked\n" if ($test_state->{locks}->{$filename});
+
+    $test_state->{locks}->{$filename} = 1;
+
+    my $res = eval { $code->(@param); };
+
+    delete $test_state->{locks}->{$filename};
+
+    return $res;
+};
+
+sub read_sdn_config {
+    my ($file) = @_;
+    # Read structure back in again
+    open my $in, '<', $file or die $!;
+    my $sdn_config;
+    {
+	local $/;    # slurp mode
+	$sdn_config = eval <$in>;
+    }
+    close $in;
+    return $sdn_config;
+}
+
+my $mocked_pve_sdn;
+$mocked_pve_sdn = Test::MockModule->new('PVE::Network::SDN');
+$mocked_pve_sdn->mock(
+    cfs_lock_file => $mocked_cfs_lock_file,
+);
+
+my $mocked_pve_tools = Test::MockModule->new('PVE::Tools');
+$mocked_pve_tools->mock(
+    lock_file => $mocked_cfs_lock_file,
+);
+
+
+my $mocked_sdn_zones;
+$mocked_sdn_zones = Test::MockModule->new('PVE::Network::SDN::Zones');
+$mocked_sdn_zones->mock(
+    config => sub {
+	return $test_state->{zones_config};
+    },
+    write_config => sub {
+	my ($cfg) = @_;
+	$test_state->{zones_config} = $cfg;
+    },
+);
+
+my $mocked_sdn_zones_super_plugin;
+$mocked_sdn_zones_super_plugin = Test::MockModule->new('PVE::Network::SDN::Zones::Plugin');
+$mocked_sdn_zones_super_plugin->mock(
+    datacenter_config => sub {
+	return $test_state->{datacenter_config};
+    },
+);
+
+my $mocked_sdn_vnets;
+$mocked_sdn_vnets = Test::MockModule->new('PVE::Network::SDN::Vnets');
+$mocked_sdn_vnets->mock(
+    config => sub {
+	return $test_state->{vnets_config};
+    },
+    write_config => sub {
+	my ($cfg) = @_;
+	$test_state->{vnets_config} = $cfg;
+    },
+    cfs_lock_file => $mocked_cfs_lock_file,
+);
+
+my $mocked_sdn_subnets;
+$mocked_sdn_subnets = Test::MockModule->new('PVE::Network::SDN::Subnets');
+$mocked_sdn_subnets->mock(
+    config => sub {
+	return $test_state->{subnets_config};
+    },
+    write_config => sub {
+	my ($cfg) = @_;
+	$test_state->{subnets_config} = $cfg;
+    },
+    cfs_lock_file => $mocked_cfs_lock_file,
+);
+
+my $mocked_sdn_controller;
+$mocked_sdn_controller = Test::MockModule->new('PVE::Network::SDN::Controllers');
+$mocked_sdn_controller->mock(
+    config => sub {
+	return $test_state->{controller_config};
+    },
+    write_config => sub {
+	my ($cfg) = @_;
+	$test_state->{controller_config} = $cfg;
+    },
+    cfs_lock_file => $mocked_cfs_lock_file,
+);
+
+
+my $mocked_sdn_dns;
+$mocked_sdn_dns = Test::MockModule->new('PVE::Network::SDN::Dns');
+$mocked_sdn_dns->mock(
+    config => sub {
+	return $test_state->{dns_config};
+    },
+    write_config => sub {
+	my ($cfg) = @_;
+	$test_state->{dns_config} = $cfg;
+    },
+    cfs_lock_file => $mocked_cfs_lock_file,
+);
+
+
+my $mocked_sdn_ipams;
+$mocked_sdn_ipams = Test::MockModule->new('PVE::Network::SDN::Ipams');
+$mocked_sdn_ipams->mock(
+    config => sub {
+	return $test_state->{ipam_config};
+    },
+    write_config => sub {
+	my ($cfg) = @_;
+	$test_state->{ipam_config} = $cfg;
+    },
+    read_macdb => sub {
+	return $test_state->{macdb};
+    },
+    write_macdb => sub {
+	my ($cfg) = @_;
+	$test_state->{macdb} = $cfg;
+    },
+    cfs_lock_file => $mocked_cfs_lock_file,
+);
+
+my $ipam_plugin = PVE::Network::SDN::Ipams::Plugin->lookup("pve"); # NOTE this is hard-coded to pve
+my $mocked_ipam_plugin = Test::MockModule->new($ipam_plugin);
+$mocked_ipam_plugin->mock(
+    read_db => sub {
+	return $test_state->{ipamdb};
+    },
+    write_db => sub {
+	my ($cfg) = @_;
+	$test_state->{ipamdb} = $cfg;
+    },
+    cfs_lock_file => $mocked_cfs_lock_file,
+);
+
+my $mocked_sdn_dhcp_dnsmasq = Test::MockModule->new('PVE::Network::SDN::Dhcp::Dnsmasq');
+$mocked_sdn_dhcp_dnsmasq->mock(
+    ethers_file => sub {
+	return "/tmp/ethers";
+    },
+    update_lease => sub {
+	my ($dhcpid, $ip4, $mac) = @_;
+    },
+    assert_dnsmasq_installed => sub {
+	return 1;
+    },
+    before_configure => sub {
+	my ($class, $dhcpid) = @_;
+    },
+    systemctl_service => sub {
+	my ($action, $service) = @_;
+    },
+);
+
+my $mocked_api_zones = Test::MockModule->new('PVE::API2::Network::SDN::Zones');
+$mocked_api_zones->mock(
+    'create_etc_interfaces_sdn_dir', sub {},
+);
+
+my $rpcenv = PVE::RESTEnvironment->init('priv');
+$rpcenv->init_request();
+$rpcenv->set_language("en_US.UTF-8");
+$rpcenv->set_user('root@pam');
+
+my $mocked_rpc_env_obj = Test::MockModule->new('PVE::RESTEnvironment');
+$mocked_rpc_env_obj->mock(
+    check_any => sub {
+	my ($self, $user, $path, $privs, $noerr) = @_;
+	return 1;
+    },
+);
+
+# ------- TEST FUNCTIONS --------------
+
+sub nic_join {
+    my ($vnetid, $mac, $hostname, $vmid) = @_;
+    return PVE::Network::SDN::Vnets::add_next_free_cidr($vnetid, $hostname, $mac, "$vmid", undef, 1);
+}
+
+sub nic_leave {
+    my ($vnetid, $mac, $hostname) = @_;
+    return PVE::Network::SDN::Vnets::del_ips_from_mac($vnetid, $mac, $hostname);
+}
+
+sub nic_start {
+    my ($vnetid, $mac, $vmid, $hostname) = @_;
+    return PVE::Network::SDN::Vnets::add_dhcp_mapping($vnetid, $mac, $vmid, $hostname);
+}
+
+
+# ---- API HELPER FUNCTIONS FOR THE TESTS -----
+
+my $t_invalid;
+sub get_zone {
+    my ($id) = @_;
+    return eval { PVE::API2::Network::SDN::Zones->read({zone => $id}); };
+}
+# verify get_zone actually fails if invalid
+$t_invalid = get_zone("invalid");
+die("getting an invalid zone must fail") if (!$@);
+fail("getting an invalid zone must fail") if (defined $t_invalid);
+
+sub create_zone {
+    my ($params) = @_;
+    my $zoneid = $params->{zone};
+    # die if failed!
+    eval { PVE::API2::Network::SDN::Zones->create($params); };
+    die("creating zone failed: $@") if ($@);
+
+    my $zone = get_zone($zoneid);
+    die ("test setup: zone ($zoneid) not defined") if (!defined $zone);
+    return $zone;
+}
+
+sub get_vnet {
+    my ($id) = @_;
+    return eval { PVE::API2::Network::SDN::Vnets->read({vnet => $id}); };
+}
+# verify get_vnet
+$t_invalid = get_vnet("invalid");
+die("getting an invalid vnet must fail") if (!$@);
+fail("getting an invalid vnet must fail") if (defined $t_invalid);
+
+sub create_vnet {
+    my ($params) = @_;
+    my $vnetid = $params->{vnet};
+    PVE::API2::Network::SDN::Vnets->create($params);
+
+    my $vnet = get_vnet($vnetid);
+    die ("test setup: vnet ($vnetid) not defined") if (!defined $vnet);
+    return $vnet;
+}
+
+sub get_subnet {
+    my ($id) = @_;
+    return eval { PVE::API2::Network::SDN::Subnets->read({subnet => $id}); };
+}
+# verify get_subnet
+$t_invalid = get_subnet("invalid");
+die("getting an invalid subnet must fail") if (!$@);
+fail("getting an invalid subnet must fail") if (defined $t_invalid);
+
+sub create_subnet {
+    my ($params) = @_;
+    PVE::API2::Network::SDN::Subnets->create($params);
+}
+
+sub get_ipam_entries {
+    return PVE::API2::Network::SDN::Ipams->ipamindex({ipam => "pve"});
+}
+
+sub create_ip {
+    my ($param) = @_;
+    return PVE::API2::Network::SDN::Ips->ipcreate($param);
+}
+
+sub run_test {
+    my $test = shift;
+    clear_test_state();
+    $test->(@_);
+}
+
+sub get_ips_from_mac {
+    my ($mac) = @_;
+    my $ipam_entries = get_ipam_entries();
+    return grep { $_->{mac} eq $mac if defined $_->{mac} } $ipam_entries->@* if $ipam_entries;
+}
+
+sub get_ip4 {
+    my $ip4 = first { Net::IP::ip_is_ipv4($_->{ip}) } @_;
+    return $ip4->{ip} if defined $ip4;
+}
+
+sub get_ip6 {
+    my $ip6 = first { Net::IP::ip_is_ipv6($_->{ip}) } @_;
+    return $ip6->{ip} if defined $ip6;
+}
+
+
+# -------------- ACTUAL TESTS  -----------------------
+
+sub test_create_vnet_with_gateway {
+    my $test_name = (split(/::/,(caller(0))[3]))[-1];
+    my $zoneid = "TESTZONE";
+    my $vnetid = "testvnet";
+
+    my $zone = create_zone({
+	type => "simple",
+	dhcp => "dnsmasq",
+	ipam => "pve",
+	zone => $zoneid,
+    });
+
+    my $vnet = create_vnet({
+	type => "vnet",
+	zone => $zoneid,
+	vnet => $vnetid,
+    });
+
+    create_subnet({
+	type => "subnet",
+	vnet => $vnetid,
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    });
+
+    my ($p) = first { $_->{gateway} == 1 } get_ipam_entries()->@*;
+    ok ($p, "$test_name: Gateway IP was created in IPAM");
+}
+run_test(\&test_create_vnet_with_gateway);
+
+
+sub test_without_subnet {
+    my $test_name = (split(/::/,(caller(0))[3]))[-1];
+
+    my $zoneid = "TESTZONE";
+    my $vnetid = "testvnet";
+
+    my $zone = create_zone({
+	type => "simple",
+	dhcp => "dnsmasq",
+	ipam => "pve",
+	zone => $zoneid,
+    });
+
+    my $vnet = create_vnet({
+	type => "vnet",
+	zone => $zoneid,
+	vnet => $vnetid,
+    });
+
+    my $hostname = "testhostname";
+    my $mac = "da:65:8f:18:9b:6f";
+    my $vmid = "999";
+
+    eval {
+	nic_join($vnetid, $mac, $hostname, $vmid);
+    };
+
+    if ($@) {
+	fail("$test_name: $@");
+	return;
+    }
+
+    my @ips = grep { $_->{mac} eq $mac } get_ipam_entries()->@*;
+    my $num_ips = scalar @ips;
+    is ($num_ips, 0, "$test_name: No IP allocated in IPAM");
+}
+run_test(\&test_without_subnet);
+
+
+sub test_nic_join {
+    my ($test_name, $subnets) = @_;
+
+    die "$test_name: we're expecting an array of subnets" if !$subnets;
+    my $num_subnets = scalar $subnets->@*;
+    die "$test_name: we're expecting an array of subnets. $num_subnets elements found" if ($num_subnets < 1);
+    my $num_dhcp_ranges = scalar grep { $_->{'dhcp-range'} } $subnets->@*;
+
+    my $zoneid = "TESTZONE";
+    my $vnetid = "testvnet";
+
+    my $zone = create_zone({
+	type => "simple",
+	dhcp => "dnsmasq",
+	ipam => "pve",
+	zone => $zoneid,
+    });
+
+    my $vnet = create_vnet({
+	type => "vnet",
+	zone => $zoneid,
+	vnet => $vnetid,
+    });
+
+    foreach my $subnet ($subnets->@*) {
+	$subnet->{type} = "subnet";
+	$subnet->{vnet} = $vnetid;
+	create_subnet($subnet);
+    };
+
+    my $hostname = "testhostname";
+    my $mac = "da:65:8f:18:9b:6f";
+    my $vmid = "999";
+
+    eval {
+	nic_join($vnetid, $mac, $hostname, $vmid);
+    };
+
+    if ($@) {
+	fail("$test_name: $@");
+	return;
+    }
+
+    my @ips = get_ips_from_mac($mac);
+    my $num_ips = scalar @ips;
+    is ($num_ips, $num_dhcp_ranges, "$test_name: Expecting $num_dhcp_ranges IPs, found $num_ips");
+    ok ((all { ($_->{vnet} eq $vnetid && $_->{zone} eq $zoneid) } @ips),
+	"$test_name: all IPs in correct vnet and zone"
+    );
+}
+
+run_test(
+    \&test_nic_join,
+    "nic_join IPv4 with dhcp",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    },
+]);
+
+run_test(
+    \&test_nic_join,
+    "nic_join IPv6 with dhcp",
+    [{
+	subnet => "8888::/64",
+	gateway => "8888::1",
+	'dhcp-range' => ["start-address=8888::100,end-address=8888::200"],
+    },
+]);
+
+run_test(
+    \&test_nic_join,
+    "nic_join IPv4+6 with dhcp",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    }, {
+	subnet => "8888::/64",
+	gateway => "8888::1",
+	'dhcp-range' => ["start-address=8888::100,end-address=8888::200"],
+    },
+]);
+
+run_test(
+    \&test_nic_join,
+    "nic_join IPv4 no DHCP, IPv6 with DHCP",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+    }, {
+	subnet => "8888::/64",
+	gateway => "8888::1",
+	'dhcp-range' => ["start-address=8888::100,end-address=8888::200"],
+    },
+]);
+
+run_test(
+    \&test_nic_join,
+    "nic_join IPv4 with DHCP, IPv6 no DHCP",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    }, {
+	subnet => "8888::/64",
+	gateway => "8888::1",
+    },
+]);
+
+sub test_nic_join_no_or_full_dhcp_range {
+    my ($test_name, $subnets, $expected_ip4, $expected_ip6) = @_;
+
+    die "$test_name: we're expecting an array of subnets" if !$subnets;
+    my $num_subnets = scalar $subnets->@*;
+    die "$test_name: we're expecting an array of subnets. $num_subnets elements found" if ($num_subnets < 1);
+
+    my $zoneid = "TESTZONE";
+    my $vnetid = "testvnet";
+
+    my $zone = create_zone({
+	type => "simple",
+	dhcp => "dnsmasq",
+	ipam => "pve",
+	zone => $zoneid,
+    });
+
+    my $vnet = create_vnet({
+	type => "vnet",
+	zone => $zoneid,
+	vnet => $vnetid,
+    });
+
+    foreach my $subnet ($subnets->@*) {
+	$subnet->{type} = "subnet";
+	$subnet->{vnet} = $vnetid;
+	create_subnet($subnet);
+    };
+
+    my $hostname = "testhostname";
+    my $mac = "da:65:8f:18:9b:6f";
+    my $vmid = "999";
+
+    eval {
+	nic_join($vnetid, $mac, $hostname, $vmid);
+    };
+
+    if (! $@) {
+	fail ("$test_name: nic_join() is expected to fail because we cannot allocate all IPs");
+    }
+
+    my @ips = grep { $_->{mac} eq $mac } get_ipam_entries()->@*;
+    my $num_ips = scalar @ips;
+    is ($num_ips, 0, "$test_name: No IP allocated in IPAM");
+}
+
+run_test(
+    \&test_nic_join_no_or_full_dhcp_range,
+    "nic_join IPv4 no dhcp",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+    },
+]);
+
+run_test(
+    \&test_nic_join_no_or_full_dhcp_range,
+    "nic_join IPv6 no dhcp",
+    [{
+	subnet => "8888::/64",
+	gateway => "8888::1",
+    },
+]);
+
+run_test(
+    \&test_nic_join_no_or_full_dhcp_range,
+    "nic_join IPv4+6 no dhcp",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+    }, {
+	subnet => "8888::/64",
+	gateway => "8888::1",
+    },
+]);
+
+run_test(
+    \&test_nic_join_no_or_full_dhcp_range,
+    "nic_join IPv4 with DHCP, dhcp-range full",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.100", # the gateway uses the only available IP in the dhcp-range
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.100"],
+    }
+]);
+
+run_test(
+    \&test_nic_join_no_or_full_dhcp_range,
+    "nic_join IPv6 with DHCP, dhcp-range full",
+    [{
+	subnet => "8888::/64",
+	gateway => "8888::100", # the gateway uses the only available IP in the dhcp-range
+	'dhcp-range' => ["start-address=8888::100,end-address=8888::100"],
+    },
+]);
+
+run_test(
+    \&test_nic_join_no_or_full_dhcp_range,
+    "nic_join IPv4+6 with DHCP, dhcp-range full for both",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.100",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.100"],
+    }, {
+	subnet => "8888::/64",
+	gateway => "8888::100",
+	'dhcp-range' => ["start-address=8888::100,end-address=8888::100"],
+    }
+]);
+
+run_test(
+    \&test_nic_join_no_or_full_dhcp_range,
+    "nic_join IPv4+6 with DHCP, dhcp-range full for IPv4",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.100", # the gateway uses the only available IP in the dhcp-range
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.100"],
+    }, {
+	subnet => "8888::/64",
+	gateway => "8888::1",
+	'dhcp-range' => ["start-address=8888::100,end-address=8888::100"],
+    }],
+);
+
+run_test(
+    \&test_nic_join_no_or_full_dhcp_range,
+    "nic_join IPv4+6 with DHCP, dhcp-range full for IPv6",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.100"],
+    }, {
+	subnet => "8888::/64",
+	gateway => "8888::100",
+	'dhcp-range' => ["start-address=8888::100,end-address=8888::100"],
+    }],
+);
+
+
+# -------------- nic_start
+sub test_nic_start {
+    my ($test_name, $subnets, $current_ip4, $current_ip6) = @_;
+
+    die "$test_name: we're expecting an array of subnets" if !$subnets;
+    my $num_subnets = scalar $subnets->@*;
+    die "$test_name: we're expecting an array of subnets. $num_subnets elements found" if ($num_subnets < 1);
+
+    my $num_expected_ips = scalar grep { $_->{'dhcp-range'} } $subnets->@*;
+    if (defined $current_ip4 && defined $current_ip6) {
+	# if we already have an IP for a subnet without a dhcp-range (special case for a single test)
+	$num_expected_ips = 2;
+    }
+
+    my $zoneid = "TESTZONE";
+    my $vnetid = "testvnet";
+
+    my $zone = create_zone({
+	type => "simple",
+	dhcp => "dnsmasq",
+	ipam => "pve",
+	zone => $zoneid,
+    });
+
+    my $vnet = create_vnet({
+	type => "vnet",
+	zone => $zoneid,
+	vnet => $vnetid,
+    });
+
+    foreach my $subnet ($subnets->@*) {
+	$subnet->{type} = "subnet";
+	$subnet->{vnet} = $vnetid;
+	create_subnet($subnet);
+    };
+
+    my $hostname = "testhostname";
+    my $mac = "da:65:8f:18:9b:6f";
+    my $vmid = "999";
+
+    if ($current_ip4) {
+	create_ip({
+	    zone => $zoneid,
+	    vnet => $vnetid,
+	    mac => $mac,
+	    ip => $current_ip4,
+	});
+    }
+
+    if ($current_ip6) {
+	create_ip({
+	    zone => $zoneid,
+	    vnet => $vnetid,
+	    mac => $mac,
+	    ip => $current_ip6,
+	});
+    }
+    my @current_ips = get_ips_from_mac($mac);
+    is ( get_ip4(@current_ips), $current_ip4, "$test_name: setup current IPv4: $current_ip4" ) if defined $current_ip4;
+    is ( get_ip6(@current_ips), $current_ip6, "$test_name: setup current IPv6: $current_ip6" ) if defined $current_ip6;
+
+    eval {
+	nic_start($vnetid, $mac, $hostname, $vmid);
+    };
+
+    if ($@) {
+	fail("$test_name: $@");
+	return;
+    }
+
+    my @ips = get_ips_from_mac($mac);
+    my $num_ips = scalar @ips;
+    is ($num_ips, $num_expected_ips, "$test_name: Expecting $num_expected_ips IPs, found $num_ips");
+    ok ((all { ($_->{vnet} eq $vnetid && $_->{zone} eq $zoneid) } @ips),
+	"$test_name: all IPs in correct vnet and zone"
+    );
+
+    is ( get_ip4(@ips), $current_ip4, "$test_name: still current IPv4: $current_ip4" ) if $current_ip4;
+    is ( get_ip6(@ips), $current_ip6, "$test_name: still current IPv6: $current_ip6" ) if $current_ip6;
+}
+
+run_test(
+    \&test_nic_start,
+    "nic_start no IP, IPv4 subnet with dhcp",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    },
+]);
+
+run_test(
+    \&test_nic_start,
+    "nic_start already IP, IPv4 subnet with dhcp",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    }],
+    "10.0.0.99"
+);
+
+run_test(
+    \&test_nic_start,
+    "nic_start already IP, IPv6 subnet with dhcp",
+    [{
+	subnet => "8888::/64",
+	gateway => "8888::1",
+	'dhcp-range' => ["start-address=8888::100,end-address=8888::200"],
+    }],
+    undef,
+    "8888::99"
+);
+
+run_test(
+    \&test_nic_start,
+    "nic_start IP, IPv4+6 subnet with dhcp",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    }, {
+	subnet => "8888::/64",
+	gateway => "8888::1",
+	'dhcp-range' => ["start-address=8888::100,end-address=8888::200"],
+    },
+]);
+
+run_test(
+    \&test_nic_start,
+    "nic_start already IPv4, IPv4+6 subnet with dhcp",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    }, {
+	subnet => "8888::/64",
+	gateway => "8888::1",
+	'dhcp-range' => ["start-address=8888::100,end-address=8888::200"],
+    }],
+    "10.0.0.99"
+);
+
+run_test(
+    \&test_nic_start,
+    "nic_start already IPv6, IPv4+6 subnet with dhcp",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    }, {
+	subnet => "8888::/64",
+	gateway => "8888::1",
+	'dhcp-range' => ["start-address=8888::100,end-address=8888::200"],
+    }],
+    undef,
+    "8888::99"
+);
+
+run_test(
+    \&test_nic_start,
+    "nic_start already IPv4+6, IPv4+6 subnets with dhcp",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    }, {
+	subnet => "8888::/64",
+	gateway => "8888::1",
+	'dhcp-range' => ["start-address=8888::100,end-address=8888::200"],
+    }],
+    "10.0.0.99",
+    "8888::99"
+);
+
+run_test(
+    \&test_nic_start,
+    "nic_start already IPv4+6, only IPv4 subnet with dhcp",
+    [{
+	subnet => "10.0.0.0/24",
+	gateway => "10.0.0.1",
+	'dhcp-range' => ["start-address=10.0.0.100,end-address=10.0.0.200"],
+    }, {
+	subnet => "8888::/64",
+	gateway => "8888::1",
+    }],
+    "10.0.0.99",
+    "8888::99"
+);
+
+done_testing();
-- 
2.44.0

[pve-devel] [PATCH v2 pve-network 9/9] tests: remove old Vnets tests

[Stefan Lendl]

The did not work and were primarily testing against internal state.

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/test/run_test_vnets.pl | 343 -------------------------------------
 1 file changed, 343 deletions(-)
 delete mode 100755 src/test/run_test_vnets.pl

diff --git a/src/test/run_test_vnets.pl b/src/test/run_test_vnets.pl
deleted file mode 100755
index 65fbfb7..0000000
--- a/src/test/run_test_vnets.pl
+++ /dev/null
@@ -1,343 +0,0 @@
-#!/usr/bin/perl
-
-use strict;
-use warnings;
-
-use lib qw(..);
-use File::Slurp;
-use NetAddr::IP qw(:lower);
-
-use Test::More;
-use Test::MockModule;
-
-use PVE::Network::SDN;
-use PVE::Network::SDN::Zones;
-use PVE::Network::SDN::Controllers;
-use PVE::INotify;
-use JSON;
-
-use Data::Dumper qw(Dumper);
-$Data::Dumper::Sortkeys = 1;
-
-sub read_sdn_config {
-    my ($file) = @_;
-
-    # Read structure back in again
-    open my $in, '<', $file or die $!;
-    my $sdn_config;
-    {
-	local $/; # slurp mode
-	$sdn_config = eval <$in>;
-    }
-    close $in;
-    return $sdn_config;
-}
-
-my @plugins = read_dir('./vnets/', prefix => 1);
-
-foreach my $path (@plugins) {
-
-    my (undef, $testid) = split(/\//, $path);
-
-    print "test: $testid\n";
-    my $sdn_config = read_sdn_config("$path/sdn_config");
-
-    my $pve_sdn_zones;
-    $pve_sdn_zones = Test::MockModule->new('PVE::Network::SDN::Zones');
-    $pve_sdn_zones->mock(
-	config => sub {
-	    return $sdn_config->{zones};
-	},
-    );
-
-    my $pve_sdn_vnets;
-    $pve_sdn_vnets = Test::MockModule->new('PVE::Network::SDN::Vnets');
-    $pve_sdn_vnets->mock(
-	config => sub {
-	    return $sdn_config->{vnets};
-	},
-    );
-
-    my $pve_sdn_subnets;
-    $pve_sdn_subnets = Test::MockModule->new('PVE::Network::SDN::Subnets');
-    $pve_sdn_subnets->mock(
-	config => sub {
-	    return $sdn_config->{subnets};
-	},
-	verify_dns_zone => sub {
-	    return;
-	},
-	add_dns_record => sub {
-	    return;
-	},
-    );
-
-    my $js = JSON->new;
-    $js->canonical(1);
-
-    #test params;
-    #test params;
-    my $subnets = $sdn_config->{subnets}->{ids};
-
-    my $subnetid = (sort keys %{$subnets})[0];
-    my $subnet =
-	PVE::Network::SDN::Subnets::sdn_subnets_config($sdn_config->{subnets}, $subnetid, 1);
-    my $subnet_cidr = $subnet->{cidr};
-    my $iplist = NetAddr::IP->new($subnet_cidr);
-    my $mask = $iplist->masklen();
-    my $ipversion = undef;
-
-    if (Net::IP::ip_is_ipv4($iplist->canon())) {
-	$iplist++; #skip network address for ipv4
-	$ipversion = 4;
-    } else {
-	$ipversion = 6;
-    }
-
-    my $cidr1 = $iplist->canon() . "/$mask";
-    $iplist++;
-    my $cidr2 = $iplist->canon() . "/$mask";
-    my $cidr_outofrange = '8.8.8.8/8';
-
-    my $subnetid2 = (sort keys %{$subnets})[1];
-    my $subnet2 =
-	PVE::Network::SDN::Subnets::sdn_subnets_config($sdn_config->{subnets}, $subnetid2, 1);
-    my $subnet2_cidr = $subnet2->{cidr};
-    my $iplist2 = NetAddr::IP->new($subnet2_cidr);
-    $iplist2++;
-    my $cidr3 = $iplist2->canon() . "/$mask";
-    $iplist2++;
-    my $cidr4 = $iplist2->canon() . "/$mask";
-
-    my $hostname = "myhostname";
-    my $mac = "da:65:8f:18:9b:6f";
-    my $description = "mydescription";
-    my $ipamdb = read_sdn_config("$path/ipam.db");
-
-    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 $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;
-	},
-    );
-
-    my $vnetid = "myvnet";
-
-    ## add_ip
-    my $test = "add_cidr $cidr1";
-    my $name = "$testid $test";
-    my $result = undef;
-    my $expected = '';
-
-    eval { PVE::Network::SDN::Vnets::add_cidr($vnetid, $cidr1, $hostname, $mac, $description); };
-
-    if ($@) {
-	fail("$name : $@");
-    } else {
-	is(undef, undef, $name);
-    }
-
-    ## add_ip
-    $test = "add_already_exist_cidr $cidr1";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = '';
-
-    eval { PVE::Network::SDN::Vnets::add_cidr($vnetid, $cidr1, $hostname, $mac, $description); };
-
-    if ($@) {
-	is(undef, undef, $name);
-    } elsif ($ipam) {
-	fail("$name : $@");
-    } else {
-	is(undef, undef, $name);
-    }
-
-    ## add_ip
-    $test = "add_cidr $cidr2";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = '';
-
-    eval { PVE::Network::SDN::Vnets::add_cidr($vnetid, $cidr2, $hostname, $mac, $description); };
-
-    if ($@) {
-	fail("$name : $@");
-    } else {
-	is(undef, undef, $name);
-    }
-
-    ## add_ip
-    $test = "add_ip_out_of_range_subnets $cidr_outofrange";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = '';
-
-    eval {
-	PVE::Network::SDN::Vnets::add_cidr($vnetid, $cidr_outofrange, $hostname, $mac,
-	    $description);
-    };
-
-    if ($@) {
-	is(undef, undef, $name);
-    } else {
-	fail("$name : $@");
-    }
-
-    ## add_ip
-    $test = "add_cidr $cidr4";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = '';
-
-    eval { PVE::Network::SDN::Vnets::add_cidr($vnetid, $cidr4, $hostname, $mac, $description); };
-
-    if ($@) {
-	fail("$name : $@");
-    } else {
-	is(undef, undef, $name);
-    }
-
-    $test = "find_next_free_cidr_in_second_subnet ($cidr3)";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = $ipam ? $cidr3 : undef;
-
-    eval {
-	$result =
-	    PVE::Network::SDN::Vnets::add_next_free_cidr($vnetid, $hostname, $mac, $description);
-    };
-
-    if ($@) {
-	fail("$name : $@");
-    } else {
-	is($result, $expected, $name);
-    }
-
-    $test = "del_cidr $cidr1";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = undef;
-
-    eval { $result = PVE::Network::SDN::Vnets::del_cidr($vnetid, $cidr1, $hostname); };
-
-    if ($@) {
-	fail("$name : $@");
-    } else {
-	is(undef, undef, $name);
-    }
-
-    $test = "del_cidr $cidr3";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = undef;
-
-    eval { $result = PVE::Network::SDN::Vnets::del_cidr($vnetid, $cidr3, $hostname); };
-
-    if ($@) {
-	fail("$name : $@");
-    } else {
-	is(undef, undef, $name);
-    }
-
-    $test = "del_cidr not exist $cidr1";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = undef;
-
-    eval { $result = PVE::Network::SDN::Vnets::del_cidr($vnetid, $cidr1, $hostname); };
-
-    if ($@) {
-	is(undef, undef, $name);
-    } elsif ($ipam) {
-	fail("$name : $@");
-    } else {
-	is(undef, undef, $name);
-    }
-
-    $test = "del_cidr outofrange $cidr_outofrange";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = undef;
-
-    eval { $result = PVE::Network::SDN::Vnets::del_cidr($vnetid, $cidr_outofrange, $hostname); };
-
-    if ($@) {
-	is(undef, undef, $name);
-    } else {
-	fail("$name : $@");
-    }
-
-    $test = "find_next_free_cidr_in_first_subnet ($cidr1)";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = $ipam ? $cidr1 : undef;
-
-    eval {
-	$result =
-	    PVE::Network::SDN::Vnets::add_next_free_cidr($vnetid, $hostname, $mac, $description);
-    };
-
-    if ($@) {
-	fail("$name : $@");
-    } else {
-	is($result, $expected, $name);
-    }
-
-    $test = "update_cidr $cidr1";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = undef;
-
-    eval {
-	$result = PVE::Network::SDN::Vnets::update_cidr($vnetid, $cidr1, $hostname, $hostname, $mac,
-	    $description);
-    };
-
-    if ($@) {
-	fail("$name : $@");
-    } else {
-	is(undef, undef, $name);
-    }
-
-    $test = "update_cidr deleted $cidr3";
-    $name = "$testid $test";
-    $result = undef;
-    $expected = undef;
-
-    eval {
-	$result = PVE::Network::SDN::Vnets::update_cidr($vnetid, $cidr1, $hostname, $hostname, $mac,
-	    $description);
-    };
-
-    if ($@) {
-	fail("$name : $@");
-    } else {
-	is(undef, undef, $name);
-    }
-
-}
-
-done_testing();
-
-- 
2.44.0

Re: [pve-devel] [PATCH v2 pve-network 0/9] SDN: Testing VNets as a blackbox.

[Max Carrara]

On Tue Apr 2, 2024 at 6:07 PM CEST, Stefan Lendl wrote:
> This add several tests for SDN VNets.
> State setup as well as testing results is done only via the API to test on the
> API boundaries and not against the internal state. Internal state and config
> files are mocked to avoid requiring access to system files or pmxcfs.
>
> The first 7 commits extract various functions to allow mocking them in the
> tests. The tests are then added as the test_vnets_blackbox test.
> The last commit removes the old vnets tests which are not working anyway.
>
> Tests validate the events of a nic joining a Vnet or a nic staring on a vnet
> with different subnet configurations. Further descriptions in the commit.
>
> *Several of the tests fail due to known bugs and limitations.*
> These bugs where discussed off-list with @s.hanreich and due to be fixed with
> the next set of SDN patches.
> Applying the tests first would allow fixing them with already existing
> tests.
>
> Differences v1 -> v2:
> * Add tests that expect a failure when no IP can be allocated
> * Removed commented out debug stuff

Just to be sure I've looked over the individual patches again - LGTM!
I've got no complaints at all. Nice work!

I'll leave the remaining things regarding the failing tests to you and
@s.hanreich, as that's your domain ;)

>
> Stefan Lendl (9):
>   sdn: extract function that reads datacenter config
>   dnsmasq: extract function to systemctl command.
>   dnsmasq: extract function that generates the ethers file path
>   dnsmasq: extract function that updates dnsmasq lease via dbus
>   controllers: extract function that reads network intreaces config
>   evpn: extract function that reads frr config.
>   api: extract function that creates the sdn directory.
>   tests: test VNets functionality as a blackbox
>   tests: remove old Vnets tests
>
>  src/PVE/API2/Network/SDN/Zones.pm             |   6 +-
>  src/PVE/Network/SDN/Controllers.pm            |  16 +-
>  src/PVE/Network/SDN/Controllers/EvpnPlugin.pm |  10 +-
>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm           |  47 +-
>  src/PVE/Network/SDN/Zones/EvpnPlugin.pm       |   3 +-
>  src/PVE/Network/SDN/Zones/Plugin.pm           |   5 +
>  src/PVE/Network/SDN/Zones/SimplePlugin.pm     |   2 +-
>  src/test/Makefile                             |   5 +-
>  src/test/run_test_vnets.pl                    | 343 -------
>  src/test/run_test_vnets_blackbox.pl           | 859 ++++++++++++++++++
>  10 files changed, 925 insertions(+), 371 deletions(-)
>  delete mode 100755 src/test/run_test_vnets.pl
>  create mode 100755 src/test/run_test_vnets_blackbox.pl

Re: [pve-devel] [PATCH v2 pve-network 0/9] SDN: Testing VNets as a blackbox.

[Stefan Lendl]

sent v3 which fixes also the broken tests.