[pve-devel] [PATCH v3 pve-network 0/5] SDN: Add mocking to run tests in sbuild

[pve-devel] [PATCH v3 pve-network 0/5] SDN: Add mocking to run 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
Added .gitignore for sbuild artifacts

Changes v2 -> v3:
* Changed commit messages according to commit guide lines
* Not importing Dumper in zones test
* Re-Enabled DNS tests
* Added .gitignore which was previously a separate patch

Changes v1 -> v2:
* Disabled DNS tests because they fail


Stefan Lendl (5):
  controllers: extract read_etc_network_interfaces
  evpn: extract read_local_frr_config
  tests: mocking more functions to avoid system access
  tests: run tests in sbuild
  gitignore: build artifacts from sbuild

 .gitignore                                    |  3 ++
 src/Makefile                                  |  2 +-
 src/PVE/Network/SDN/Controllers.pm            | 16 +++++++---
 src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 10 ++++--
 src/test/run_test_zones.pl                    | 31 +++++++++++++++++++
 5 files changed, 54 insertions(+), 8 deletions(-)

-- 
2.44.0

[pve-devel] [PATCH v3 pve-network 1/5] controllers: extract read_etc_network_interfaces

[Stefan Lendl]

to allow mocking local fs access

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 v3 pve-network 2/5] evpn: extract read_local_frr_config

[Stefan Lendl]

to allow mocking local fs access

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 v3 pve-network 3/5] tests: mocking more functions to avoid system access

[Stefan Lendl]

previously extracted functions are now mocked in the zone 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/test/run_test_zones.pl | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/src/test/run_test_zones.pl b/src/test/run_test_zones.pl
index 5354e4d..274a119 100755
--- a/src/test/run_test_zones.pl
+++ b/src/test/run_test_zones.pl
@@ -46,8 +46,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;
@@ -87,6 +99,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.44.0

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

[Stefan Lendl]

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/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.44.0

[pve-devel] [PATCH v3 pve-network 5/5] gitignore: build artifacts from sbuild

[Stefan Lendl]

source package *.dsc
build log *.build
tarball *.tar.xz

Signed-off-by: Stefan Lendl <s.lendl@proxmox.com>
---
 .gitignore | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.gitignore b/.gitignore
index 60332cc..4694865 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,8 @@
 *.deb
+/*.build
 /*.buildinfo
 /*.changes
+/*.dsc
 /.vscode/
 /libpve-network-perl-[0-9]*/
+/libpve-network-perl_[0-9]*.tar.xz
-- 
2.44.0

Re: [pve-devel] [PATCH v3 pve-network 0/5] SDN: Add mocking to run tests in sbuild

[Stefan Hanreich]

Tested building this with sbuild - completed successfully (including tests).

I've also checked out the changes and tested them by creating a new EVPN
zone and setting a local frr config. Everything still seems to be
working as before.

Consider this:

Tested-By: Stefan Hanreich <s.hanreich@proxmox.com>


On 4/2/24 13:07, Stefan Lendl wrote:
> 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
> Added .gitignore for sbuild artifacts
> 
> Changes v2 -> v3:
> * Changed commit messages according to commit guide lines
> * Not importing Dumper in zones test
> * Re-Enabled DNS tests
> * Added .gitignore which was previously a separate patch
> 
> Changes v1 -> v2:
> * Disabled DNS tests because they fail
> 
> 
> Stefan Lendl (5):
>   controllers: extract read_etc_network_interfaces
>   evpn: extract read_local_frr_config
>   tests: mocking more functions to avoid system access
>   tests: run tests in sbuild
>   gitignore: build artifacts from sbuild
> 
>  .gitignore                                    |  3 ++
>  src/Makefile                                  |  2 +-
>  src/PVE/Network/SDN/Controllers.pm            | 16 +++++++---
>  src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 10 ++++--
>  src/test/run_test_zones.pl                    | 31 +++++++++++++++++++
>  5 files changed, 54 insertions(+), 8 deletions(-)
> 

[pve-devel] applied-series: [PATCH v3 pve-network 0/5] SDN: Add mocking to run tests in sbuild

[Thomas Lamprecht]

Am 02/04/2024 um 13:07 schrieb 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
> Added .gitignore for sbuild artifacts
> 
> Changes v2 -> v3:
> * Changed commit messages according to commit guide lines
> * Not importing Dumper in zones test
> * Re-Enabled DNS tests
> * Added .gitignore which was previously a separate patch
> 
> Changes v1 -> v2:
> * Disabled DNS tests because they fail
> 
> 
> Stefan Lendl (5):
>   controllers: extract read_etc_network_interfaces
>   evpn: extract read_local_frr_config
>   tests: mocking more functions to avoid system access
>   tests: run tests in sbuild
>   gitignore: build artifacts from sbuild
> 
>  .gitignore                                    |  3 ++
>  src/Makefile                                  |  2 +-
>  src/PVE/Network/SDN/Controllers.pm            | 16 +++++++---
>  src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 10 ++++--
>  src/test/run_test_zones.pl                    | 31 +++++++++++++++++++
>  5 files changed, 54 insertions(+), 8 deletions(-)
> 


applied series, with Stefan Hanreich's T-b, thanks!