[PATCH pve-network v2 07/16] sdn: dhcp: only assert a backend's availability for zones using it

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

The regenerate told every registered backend whether any zone at all
uses dhcp. So any zone with dhcp configured made every backend assert
its own availability, and a failing assert aborts the whole regenerate.
With one backend that was the same thing. With a second one it breaks
network reloads on nodes that only run the other backend. Gate each
backend on the zones using it on this node, and leave the zones of other
nodes unconfigured here as well.

Signed-off-by: Hannes Laimer <h.laimer@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 28a9f2e..1f3619d 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -59,17 +59,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);
@@ -110,7 +117,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 3e78ba9..2429adf 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -104,6 +104,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');
@@ -1134,6 +1141,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_ipam_cache_misses {
     my $test_name = (split(/::/, (caller(0))[3]))[-1];
     my $zoneid = "TESTZONE";
-- 
2.47.3