Tested this on my machine, works as advertised. The general idea of regenerating the FRR config anytime we reload SDN also makes sense to me, since it allows users to have a FRR config and potentially create controllers / fabrics at a later time and not have to worry about their frr.conf getting overwritten accidentally.
Reviewed-by: Stefan Hanreich <[email protected]> Tested-by: Stefan Hanreich <[email protected]> On 9/19/25 3:25 PM, Gabriel Goller wrote: > Currently we check if there is a fabric or a controller existing to > decide if we write the frr.conf file. Also add a check if the > frr.conf.local exists. This way it's possible to have a custom config in > the frr.conf.local file without any SDN objects configured and still > be able to apply the config. > By not checking the content we can also clear the frr config by clearing > the frr.conf.local file. > > Reported-by: Hannes Dürr <[email protected]> > Signed-off-by: Gabriel Goller <[email protected]> > --- > src/PVE/Network/SDN.pm | 3 ++- > src/PVE/Network/SDN/Frr.pm | 20 ++++++++++++++++++-- > 2 files changed, 20 insertions(+), 3 deletions(-) > > diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm > index f2ecd4ab7318..e867d1ab886e 100644 > --- a/src/PVE/Network/SDN.pm > +++ b/src/PVE/Network/SDN.pm > @@ -123,8 +123,9 @@ sub running_config_has_frr { > # both can be empty if the SDN configuration was never applied > my $controllers = $running_config->{controllers}->{ids} // {}; > my $fabrics = $running_config->{fabrics}->{ids} // {}; > + my $local_frr_config = PVE::Network::SDN::Frr::local_frr_config_exists(); > > - return %$controllers || %$fabrics; > + return %$controllers || %$fabrics || $local_frr_config; > } > > sub pending_config { > diff --git a/src/PVE/Network/SDN/Frr.pm b/src/PVE/Network/SDN/Frr.pm > index b607b32c248d..e77eba182177 100644 > --- a/src/PVE/Network/SDN/Frr.pm > +++ b/src/PVE/Network/SDN/Frr.pm > @@ -32,6 +32,22 @@ string that is a FRR configuration line. > use PVE::RESTEnvironment qw(log_warn); > use PVE::Tools qw(file_get_contents file_set_contents run_command); > > +my $FRR_CONF_LOCAL_FILE = "/etc/frr/frr.conf.local"; > + > +=head3 local_frr_config_exists > + > +Returns true if the `/etc/frr/frr.conf.local` file exists, otherwise false. > + > +=cut > + > +sub local_frr_config_exists { > + if (-e $FRR_CONF_LOCAL_FILE) { > + return 1; > + } else { > + return 0; > + } > +} > + > =head3 read_local_frr_config > > Returns the contents of `/etc/frr/frr.conf.local` as a string if it exists, > otherwise undef. > @@ -39,8 +55,8 @@ Returns the contents of `/etc/frr/frr.conf.local` as a > string if it exists, othe > =cut > > sub read_local_frr_config { > - if (-e "/etc/frr/frr.conf.local") { > - return file_get_contents("/etc/frr/frr.conf.local"); > + if (-e $FRR_CONF_LOCAL_FILE) { small nit: potentially use the helper here, if we define one anyway? + return file_get_contents($FRR_CONF_LOCAL_FILE); > } > } > _______________________________________________ pve-devel mailing list [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
