]> git.puffer.fish Git - matthieu/pve-network.git/commitdiff
partial fix #5496: subnet: ipam: add update_subnet hook
authorStefan Hanreich <s.hanreich@proxmox.com>
Mon, 10 Mar 2025 08:51:03 +0000 (09:51 +0100)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Mon, 7 Apr 2025 15:43:56 +0000 (17:43 +0200)
Because of how the Netbox IPAM plugin works (utilizing IP ranges to
represent DHCP ranges), we need a hook in the IPAM plugin that runs on
updates to the subnet because DHCP ranges can be edited. The update
hook in Netbox checks which DHCP ranges got added and which got
deleted and then performs the respective changes in the Netbox IPAM.
This operates under the assumption that DHCP ranges do not overlap
(which is not supported by Netbox anyway).

Only Netbox needs to do work on update, so we can leave this as noop
in phpIPAM and the PVE IPAM, because they have no notion of IP ranges
or similar entities. phpIPAM doesn't support DHCP ranges at all and
PVE IPAM simply uses DHCP ranges as a constraint when allocating an
IP.

I decided on this approach over just creating IP ranges on demand when
assigning IPs, because this keeps Netbox clean and in sync with the
PVE state. It doesn't leave remnants of IP ranges in the Netbox
database, which can lead to errors when trying to create IP ranges
that overlap with IP ranges that already existed in an SDN subnet.

This method tries to check for any possible errors before editing the
entities. There is still a small window where external changes can
occur that lead to errors. We are touching multiple entities here, so
in case of errors users have to fix their Netbox instance manually.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
Tested-by: Hannes Duerr <h.duerr@proxmox.com>
Link: https://lore.proxmox.com/20250310085103.30549-8-s.hanreich@proxmox.com
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
src/PVE/Network/SDN/Ipams/PVEPlugin.pm
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
src/PVE/Network/SDN/Ipams/Plugin.pm
src/PVE/Network/SDN/SubnetPlugin.pm
src/PVE/Network/SDN/Subnets.pm

index af9be2543b895efbe927845b3ae3cbb15c60bbe4..a691323afe18ba0d509872ade3c351201e45ff87 100644 (file)
@@ -96,6 +96,60 @@ sub add_subnet {
     }
 }
 
+sub update_subnet {
+    my ($class, $plugin_config, $subnetid, $subnet, $old_subnet, $noerr) = @_;
+
+    # old subnet in SubnetPlugin hook has already parsed dhcp-ranges
+    # new subnet doesn't
+    my $old_dhcp_ranges = $old_subnet->{'dhcp-range'};
+    my $new_dhcp_ranges = PVE::Network::SDN::Subnets::get_dhcp_ranges($subnet);
+
+    my $hash_range = sub {
+       my ($dhcp_range) = @_;
+       "$dhcp_range->{'start-address'} - $dhcp_range->{'end-address'}"
+    };
+
+    my $old_lookup = {};
+    for my $dhcp_range (@$old_dhcp_ranges) {
+       my $hash = $hash_range->($dhcp_range);
+       $old_lookup->{$hash} = undef;
+    }
+
+    my $new_lookup = {};
+    for my $dhcp_range (@$new_dhcp_ranges) {
+       my $hash = $hash_range->($dhcp_range);
+       $new_lookup->{$hash} = undef;
+    }
+
+    my $to_delete_ids = ();
+
+    # delete first so we don't get errors with overlapping ranges
+    for my $dhcp_range (@$old_dhcp_ranges) {
+       my $hash = $hash_range->($dhcp_range);
+
+       if (exists($new_lookup->{$hash})) {
+           next;
+       }
+
+       my $internalid = get_iprange_id($plugin_config, $dhcp_range, $noerr);
+
+       # definedness check, because ID could be 0
+       if (!defined($internalid)) {
+           warn "could not find id for ip range $dhcp_range->{'start-address'}:$dhcp_range->{'end-address'}";
+           next;
+       }
+
+       del_dhcp_range($plugin_config, $internalid, $noerr);
+    }
+
+    for my $dhcp_range (@$new_dhcp_ranges) {
+       my $hash = $hash_range->($dhcp_range);
+
+       add_dhcp_range($plugin_config, $dhcp_range, $noerr)
+           if !exists($old_lookup->{$hash});
+    }
+}
+
 sub del_subnet {
     my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
 
index 742f1b12e1f2d3960a2ea8940811a03376aa9ceb..59ad4eae93debeb1cb7a17ad06c9438e6cbbb0cb 100644 (file)
@@ -82,6 +82,11 @@ sub add_subnet {
     die "$@" if $@;
 }
 
+sub update_subnet {
+    my ($class, $plugin_config, $subnetid, $subnet, $old_subnet, $noerr) = @_;
+    # we don't need to do anything on update
+}
+
 sub only_gateway_remains {
     my ($ips) = @_;
 
index df5048d2a14b54ec35d449ce86dd0c5ebd872fa4..8ee430abe1299838f0ce49e825847fe15b741b43 100644 (file)
@@ -67,6 +67,11 @@ sub add_subnet {
     }
 }
 
+sub update_subnet {
+    my ($class, $plugin_config, $subnetid, $subnet, $old_subnet, $noerr) = @_;
+    # we don't need to do anything on update
+}
+
 sub del_subnet {
     my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
 
index ab4cae821c8c5494d4911cec05776ed19ecef297..6190c246b42128f7cd48b78acd5d6e50ac8a42e7 100644 (file)
@@ -75,6 +75,12 @@ sub add_subnet {
     die "please implement inside plugin";
 }
 
+sub update_subnet {
+    my ($class, $plugin_config, $subnetid, $subnet, $old_subnet, $noerr) = @_;
+
+    die "please implement inside plugin";
+}
+
 sub del_subnet {
     my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
 
index 049f7e1feb2b7ef2e58b261bfc50cb6fa77446fc..e73d97a84fc0d5b9e97885fc703d9242ae25c067 100644 (file)
@@ -186,7 +186,11 @@ sub on_update_hook {
     validate_dhcp_ranges($subnet);
 
     if ($ipam) {
-       PVE::Network::SDN::Subnets::add_subnet($zone, $subnetid, $subnet);
+       if ($old_subnet) {
+           PVE::Network::SDN::Subnets::update_subnet($zone, $subnetid, $subnet, $old_subnet);
+       } else {
+           PVE::Network::SDN::Subnets::add_subnet($zone, $subnetid, $subnet);
+       }
 
        #don't register gateway for pointopoint
        return if $pointopoint;
index e2c8c9c7ab72489b44eea345c6fc235402a1f92c..18847c2b88508b37f7bd5c86f6224efb8eaca7ab 100644 (file)
@@ -194,6 +194,18 @@ sub add_subnet {
     $plugin->add_subnet($plugin_config, $subnetid, $subnet);
 }
 
+sub update_subnet {
+    my ($zone, $subnetid, $subnet, $old_subnet) = @_;
+
+    my $ipam = $zone->{ipam};
+    return if !$ipam;
+
+    my $ipam_cfg = PVE::Network::SDN::Ipams::config();
+    my $plugin_config = $ipam_cfg->{ids}->{$ipam};
+    my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
+    $plugin->update_subnet($plugin_config, $subnetid, $subnet, $old_subnet);
+}
+
 sub del_subnet {
     my ($zone, $subnetid, $subnet) = @_;