Re: [PATCH pve-network v3 08/17] sdn: dhcp: require a backend only if a zone on this node uses it

[Stefan Hanreich <s.hanreich@proxmox.com>]

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

On 10/20/26 10:01 AM, Hannes Laimer wrote:
> 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>
> ---
>  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 dc54bba3..cda4b7ef 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');
> @@ -1169,6 +1176,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";