[PATCH pve-network v4 10/20] sdn: dhcp: require a backend only if a zone on this node uses it

[Hannes Laimer <h.laimer@proxmox.com>]

A network reload failed on a node without the dnsmasq package as soon
as any zone had DHCP, even a zone confined to other nodes. On a node
with dnsmasq, the reload wrote such a zone's configuration and started
an instance for it, although none of the zone's vnets exist there.

A reload now skips the zones confined to other nodes, so a backend is
required and started only for the zones on the reloading node.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
Reviewed-by: Stefan Hanreich <s.hanreich@proxmox.com>
Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/Network/SDN/Dhcp.pm         | 13 +++++++--
 src/test/run_test_vnets_blackbox.pl | 43 +++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index ab5d6b5b..2a023534 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -63,17 +63,24 @@ sub regenerate_config {
 
     my $plugins = PVE::Network::SDN::Dhcp::Plugin->lookup_types();
 
-    my $any_zone_needs_dhcp = grep { $_->{dhcp} } values $zone_cfg->{ids}->%*;
+    my %plugin_needed = ();
+    for my $zone (values $zone_cfg->{ids}->%*) {
+        next if !$zone->{dhcp};
+        # a zone confined to other nodes runs no backend here
+        next if defined($zone->{nodes}) && !$zone->{nodes}->{$nodename};
+        $plugin_needed{ $zone->{dhcp} } = 1;
+    }
 
     foreach my $plugin_name (@$plugins) {
         my $plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($plugin_name);
-        eval { $plugin->before_regenerate(!$any_zone_needs_dhcp) };
+        eval { $plugin->before_regenerate(!$plugin_needed{$plugin_name}) };
         die "Could not run before_regenerate for DHCP plugin $plugin_name $@\n" if $@;
     }
 
     foreach my $zoneid (sort keys %{ $zone_cfg->{ids} }) {
         my $zone = $zone_cfg->{ids}->{$zoneid};
         next if !$zone->{dhcp};
+        next if defined($zone->{nodes}) && !$zone->{nodes}->{$nodename};
 
         my $dhcp_plugin_name = $zone->{dhcp};
         my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcp_plugin_name);
@@ -114,7 +121,7 @@ sub regenerate_config {
             warn "Could not configure vnet $vnetid: $@\n" if $@;
         }
 
-        eval { $dhcp_plugin->after_configure($zoneid, !$any_zone_needs_dhcp) };
+        eval { $dhcp_plugin->after_configure($zoneid, !$plugin_needed{$dhcp_plugin_name}) };
         warn "Could not run after_configure for DHCP server $zoneid $@\n" if $@;
 
     }
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 7bc18de1..8ad1e255 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -105,6 +105,13 @@ my $mocked_pve_sdn;
 $mocked_pve_sdn = Test::MockModule->new('PVE::Network::SDN');
 $mocked_pve_sdn->mock(
     cfs_lock_file => $mocked_cfs_lock_file,
+    running_config => sub {
+        return {
+            zones => $test_state->{zones_config},
+            vnets => $test_state->{vnets_config},
+            subnets => $test_state->{subnets_config},
+        };
+    },
 );
 
 my $mocked_pve_tools = Test::MockModule->new('PVE::Tools');
@@ -1301,6 +1308,42 @@ sub test_dnsmasq_lease_time {
 
 run_test(\&test_dnsmasq_lease_time);
 
+sub test_dhcp_backend_needed_per_node {
+    my $test_name = (split(/::/, (caller(0))[3]))[-1];
+
+    # a backend is told whether a zone on this node uses it. A zone confined
+    # to other nodes runs nothing here
+    my $asked = [];
+    my $configured = [];
+    $mocked_sdn_dhcp_dnsmasq->mock(
+        before_regenerate => sub { push @$asked, $_[1] ? 'optional' : 'needed'; },
+        after_configure => sub { push @$configured, $_[1]; },
+    );
+    create_zone({
+        type => "simple",
+        dhcp => "dnsmasq",
+        ipam => "pve",
+        zone => "DNSZONE",
+        nodes => 'other',
+    });
+    PVE::Network::SDN::Dhcp::regenerate_config();
+    eq_or_diff(
+        $asked,
+        ['optional'],
+        "$test_name: a dnsmasq zone confined elsewhere needs no dnsmasq here",
+    );
+    eq_or_diff($configured, [], "$test_name: and is not configured here");
+
+    @$asked = ();
+    update_zone("DNSZONE", { delete => 'nodes' });
+    PVE::Network::SDN::Dhcp::regenerate_config();
+    eq_or_diff($asked, ['needed'], "$test_name: a dnsmasq zone on this node needs it");
+    eq_or_diff($configured, ['DNSZONE'], "$test_name: and is configured");
+    $mocked_sdn_dhcp_dnsmasq->unmock($_) for qw(before_regenerate after_configure);
+}
+
+run_test(\&test_dhcp_backend_needed_per_node);
+
 sub test_dnsmasq_dual_stack_and_sweep {
     my $test_name = (split(/::/, (caller(0))[3]))[-1];
     my $zoneid = "TESTZONE";
-- 
2.47.3