[pve-devel] [PATCH pve-network 0/4] SDN tests in sbuild

[pve-devel] [PATCH pve-network 0/4] SDN tests in sbuild

[Stefan Lendl]

Extract and mock functions that otherwise access system files which is not
possible in a clean sbuild environment.
Namely /etc/network/interfaces as well as /etc/frr/frr.config.local

Stefan Lendl (4):
  refactor(controllers): extract read_etc_network_interfaces
  refactor(evpn): extract read_local_frr_config
  tests: mocking more functions to avoid system access
  tests: run tests in sbuild

 src/Makefile                                  |  2 +-
 src/PVE/Network/SDN/Controllers.pm            | 16 ++++++---
 src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 10 ++++--
 src/test/run_test_zones.pl                    | 36 ++++++++++++++++++-
 4 files changed, 55 insertions(+), 9 deletions(-)

-- 
2.42.0

[pve-devel] [PATCH pve-network 1/4] refactor(controllers): extract read_etc_network_interfaces

[Stefan Lendl]

Allows mocking local fs access

Signed-off-by: Stefan Lendl <s.lendl@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.42.0

[pve-devel] [PATCH pve-network 2/4] refactor(evpn): extract read_local_frr_config

[Stefan Lendl]

Allows mocking local fs access

Signed-off-by: Stefan Lendl <s.lendl@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 f320139..fc297f9 100644
--- a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
+++ b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
@@ -461,6 +461,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) = @_;
 
@@ -474,8 +480,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.42.0

[pve-devel] [PATCH pve-network 3/4] tests: mocking more functions to avoid access to system files

[Stefan Lendl]

previously extracted functions are now mocked in the zone tests

mocked PVE::Network::SDN::Controllers::read_etc_network_interfaces and also
INotify::read_etc_network_interfaces to return the same $interfaces_config

created a mock for several PVE::Network::SDN::Controllers::<type>Plugin methods
that would require system access. Just to be sure.

Added a note that mocking INotify::read_file is a HACK and not garanteed to work
if anything other than /etc/network/interfaces is read.

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/test/run_test_zones.pl | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/src/test/run_test_zones.pl b/src/test/run_test_zones.pl
index ce8edc2..2d9be88 100755
--- a/src/test/run_test_zones.pl
+++ b/src/test/run_test_zones.pl
@@ -14,6 +14,10 @@ use PVE::Network::SDN::Zones;
 use PVE::Network::SDN::Controllers;
 use PVE::INotify;
 
+use Data::Dumper qw(Dumper);
+$Data::Dumper::Sortkeys = 1;
+$Data::Dumper::Indent = 1;
+
 sub read_sdn_config {
     my ($file) = @_;
 
@@ -29,7 +33,6 @@ sub read_sdn_config {
     return $sdn_config;
 }
 
-
 my @tests = grep { -d } glob './zones/*/*';
 
 foreach my $test (@tests) {
@@ -47,8 +50,20 @@ foreach my $test (@tests) {
 	    return 'localhost';
 	},
 	read_file => sub {
+	    # HACK this assumes we are always calling PVE::INotify::read_file('interfaces');
 	    return $interfaces_config;
 	},
+	read_etc_network_interfaces => sub {
+	    return $interfaces_config;
+	},
+    );
+
+    my $mocked_pve_sdn_controllers;
+    $mocked_pve_sdn_controllers = Test::MockModule->new('PVE::Network::SDN::Controllers');
+    $mocked_pve_sdn_controllers->mock(
+	read_etc_network_interfaces => sub {
+	    return $interfaces_config;
+	}
     );
 
     my $pve_sdn_subnets;
@@ -88,6 +103,25 @@ foreach my $test (@tests) {
 	},
     );
 
+    my ($first_plugin) = %{$sdn_config->{controllers}->{ids}} if defined $sdn_config->{controllers};
+    if ($first_plugin) {
+	my $controller_plugin = PVE::Network::SDN::Controllers::Plugin->lookup(
+	    $sdn_config->{controllers}->{ids}->{$first_plugin}->{type}
+	);
+	my $mocked_controller_plugin = Test::MockModule->new($controller_plugin);
+	$mocked_controller_plugin->mock(
+	    write_controller_config => sub {
+		return;
+	    },
+	    reload_controller => sub {
+		return;
+	    },
+	    read_local_frr_config => sub {
+		return;
+	    },
+	);
+    }
+
     my $name = $test;
     my $expected = read_file("./$test/expected_sdn_interfaces");
 
-- 
2.42.0

[pve-devel] [PATCH pve-network 4/4] tests: run tests in sbuild

[Stefan Lendl]

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 src/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Makefile b/src/Makefile
index c9dee4c..c4056b4 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -10,7 +10,7 @@ clean:
 
 .PHONY: test
 test:
-	[ -e /run/lock/sbuild ] || $(MAKE) -C $@
+	$(MAKE) -C $@
 
 .PHONY: install
 install:
-- 
2.42.0

Re: [pve-devel] [PATCH pve-network 3/4] tests: mocking more functions to avoid access to system files

[Lukas Wagner]

On 12/5/23 14:12, Stefan Lendl wrote:
> diff --git a/src/test/run_test_zones.pl b/src/test/run_test_zones.pl
> index ce8edc2..2d9be88 100755/
> --- a/src/test/run_test_zones.pl
> +++ b/src/test/run_test_zones.pl
> @@ -14,6 +14,10 @@ use PVE::Network::SDN::Zones;
>   use PVE::Network::SDN::Controllers;
>   use PVE::INotify;
>   
> +use Data::Dumper qw(Dumper);
> +$Data::Dumper::Sortkeys = 1;
> +$Data::Dumper::Indent = 1;
> +

Left-over debug code? :)

-- 
- Lukas

Re: [pve-devel] [PATCH pve-network 3/4] tests: mocking more functions to avoid access to system files

[Stefan Lendl]

Lukas Wagner <l.wagner@proxmox.com> writes:

>
> Left-over debug code? :)

Yes, I am not actually using this here.
This is also included in the other tests.