[pve-devel] [PATCH pve-network v5 08/21] sdn: api: add check for rewriting frr configuration

[Gabriel Goller <g.goller@proxmox.com>]

From: Stefan Hanreich <s.hanreich@proxmox.com>

With the old FRR config generation logic, we never wrote an empty FRR
configuration if all controllers got deleted. This meant that deleting
all controllers still left the previous FRR configuration on the
nodes, never disabling BGP / IS-IS. The new logic now writes an empty
configuration if there is no controller / fabric configured, fixing
this behavior. This has a side effect for users with an existing FRR
configuration not managed by SDN, but utilizing other SDN features
(zones, vnets, ...). Their manual FRR configuration would get
overwritten when applying any SDN configuration. This is particularly
an issue with full-mesh Ceph setups, that were set up according to our
Wiki guide [1]. User with such a full-mesh setup could get their FRR
configuration overwritten when using unrelated SDN features.

Since we call the API endpoint in pve-manager for generating and
writing configuration files, we cannot directly prevent the FRR
configuration from being written in the SDN API call. Instead a new
parameter, skip_frr, has been added to the endpoint in pve-manager,
that skips writing the FRR configuration. We skip writing the FRR
configuration if neither the previous SDN configuration, nor the new
SDN configuration contains an entity that requires writing FRR
configuration. This should minimize the impact of the change to the
FRR config generation on existing FRR setups.

[1] https://pve.proxmox.com/mediawiki/index.php?title=Full_Mesh_Network_for_Ceph_Server&oldid=12146

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/Network/SDN.pm | 15 ++++++++++++---
 src/PVE/Network/SDN.pm      | 21 +++++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
index b51480332d2c..0824410f43cb 100644
--- a/src/PVE/API2/Network/SDN.pm
+++ b/src/PVE/API2/Network/SDN.pm
@@ -83,13 +83,18 @@ __PACKAGE__->register_method({
 });
 
 my $create_reload_network_worker = sub {
-    my ($nodename) = @_;
+    my ($nodename, $skip_frr) = @_;
+
+    my @command = ('pvesh', 'set', "/nodes/$nodename/network");
+    if ($skip_frr) {
+        push(@command, '--skip_frr');
+    }
 
     # FIXME: how to proxy to final node ?
     my $upid;
     print "$nodename: reloading network config\n";
     run_command(
-        ['pvesh', 'set', "/nodes/$nodename/network"],
+        \@command,
         outfunc => sub {
             my $line = shift;
             if ($line =~ /["']?(UPID:[^\s"']+)["']?$/) {
@@ -124,14 +129,18 @@ __PACKAGE__->register_method({
         my $rpcenv = PVE::RPCEnvironment::get();
         my $authuser = $rpcenv->get_user();
 
+        my $previous_config_has_frr = PVE::Network::SDN::running_config_has_frr();
         PVE::Network::SDN::commit_config();
 
+        my $new_config_has_frr = PVE::Network::SDN::running_config_has_frr();
+        my $skip_frr = !($previous_config_has_frr || $new_config_has_frr);
+
         my $code = sub {
             $rpcenv->{type} = 'priv'; # to start tasks in background
             PVE::Cluster::check_cfs_quorum();
             my $nodelist = PVE::Cluster::get_nodelist();
             for my $node (@$nodelist) {
-                my $pid = eval { $create_reload_network_worker->($node) };
+                my $pid = eval { $create_reload_network_worker->($node, $skip_frr) };
                 warn $@ if $@;
             }
 
diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index ce4b41e53a6e..b321013708a9 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -84,6 +84,27 @@ sub running_config {
     return cfs_read_file($running_cfg);
 }
 
+=head3 running_config_has_frr(\%running_config)
+
+Determines whether C<\%running_config> contains any entities that generate an
+FRR configuration. This is used by pve-manager to determine whether a rewrite of
+the FRR configuration is required or not.
+
+If C<\%running_config> is not provided, it will query the current running
+configuration and then evaluate it.
+
+=cut
+
+sub running_config_has_frr {
+    my $running_config = PVE::Network::SDN::running_config();
+
+    # both can be empty if the SDN configuration was never applied
+    my $controllers = $running_config->{controllers}->{ids} // {};
+    my $fabrics = $running_config->{fabrics}->{ids} // {};
+
+    return %$controllers || %$fabrics;
+}
+
 sub pending_config {
     my ($running_cfg, $cfg, $type) = @_;
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel