Another idea, related to the comments already sent in pve-manager: We could potentially return the output in a similar way to how the tests dp it (using Tests::Difference) and then show this verbatim in the UI?
Of course, having a native diff viewer in the UI that can render diffs would be preferable, but that seems a bit unrealistic considering that we would like to roll this out relatively soon and if we're returning raw diff outputs anyway, we could opt for a flavor that's a bit more human-readable? On 2/3/26 5:04 PM, Gabriel Goller wrote: > Allows users to see the diff of frr configuration before applying > SDN changes. Previously this was not possible and the user had to apply > and then see what changed. Ideally this would also include the ifupdown2 > config, but that's a bit tricky since we add config lines in perl-rs as > well. > > Signed-off-by: Gabriel Goller <[email protected]> > --- > src/PVE/API2/Network/SDN.pm | 67 +++++++++++++++++++++++++++++++++++++ > src/PVE/Network/SDN.pm | 9 +++-- > 2 files changed, 74 insertions(+), 2 deletions(-) > > diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm > index b35a588d391d..9208d6f4e8b3 100644 > --- a/src/PVE/API2/Network/SDN.pm > +++ b/src/PVE/API2/Network/SDN.pm > @@ -3,6 +3,9 @@ package PVE::API2::Network::SDN; > use strict; > use warnings; > > +use File::Temp qw(tempfile); There's a wrapper PVE::File::tempfile(_contents) which we should probably use instead? > +use Encode qw(decode); > + > use PVE::Cluster qw(cfs_lock_file cfs_read_file cfs_write_file); > use PVE::Exception qw(raise_param_exc); > use PVE::JSONSchema qw(get_standard_option); > @@ -325,4 +328,68 @@ __PACKAGE__->register_method({ > }, > }); > > +sub get_diff { > + my ($filename_one, $filename_two) = @_; > + > + my $diff = ''; > + > + my $cmd = ['/usr/bin/diff', '-b', '-N', '-u', $filename_one, > $filename_two]; > + PVE::Tools::run_command( > + $cmd, > + noerr => 1, > + outfunc => sub { > + my ($line) = @_; > + $diff .= decode('UTF-8', $line) . "\n"; > + }, > + ); > + > + $diff = undef if !$diff; > + > + return $diff; > +} > + > +__PACKAGE__->register_method({ > + name => 'dry-apply', > + path => 'dry-apply', We refer to the action as dry-run everywhere but the path is actually 'dry-apply'? I think dry-run would fit better. > + method => 'PUT', > + permissions => { > + check => ['perm', '/nodes/{node}', ['Sys.Modify']], > + }, > + description => "Dry-Run the SDN apply", make this more descriptive? > + protected => 1, > + proxyto => 'node', > + parameters => { > + additionalProperties => 0, > + properties => { > + node => get_standard_option('pve-node'), > + }, > + }, > + > + returns => { > + type => 'object', > + properties => { > + "frr-diff" => > + { type => 'string', description => 'The frr config generated > by SDN.' }, description is kinda wrong? It's the difference/changes in the FRR configuration - not the config itself. > + }, > + }, > + code => sub { > + my ($param) = @_; > + > + my $config = PVE::Network::SDN::compile_running_cfg(); > + > + my $fabric_config = PVE::Network::SDN::Fabrics::config(0); > + my $frr_config = PVE::Network::SDN::generate_frr_raw_config($config, > $fabric_config); > + my $new_config_frr = > PVE::Network::SDN::Frr::raw_config_to_string($frr_config); > + > + my ($frr_tmp_fh, $frr_tmp_filename) = tempfile(); see above comment regarding PVE::File::tempfile > + print $frr_tmp_fh $new_config_frr; > + > + my $return_value = {}; > + $return_value->{"frr-diff"} = get_diff('/etc/frr/frr.conf', > $frr_tmp_filename); > + > + close($frr_tmp_fh); > + return $return_value; > + }, > +}); > + > 1; > diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm > index c000bed498ec..18938d73ba70 100644 > --- a/src/PVE/Network/SDN.pm > +++ b/src/PVE/Network/SDN.pm > @@ -187,8 +187,7 @@ sub pending_config { > > } > > -sub commit_config { > - > +sub compile_running_cfg { > my $cfg = cfs_read_file($running_cfg); > my $version = $cfg->{version}; > > @@ -219,6 +218,12 @@ sub commit_config { > fabrics => $fabrics, > }; > > + return $cfg; > +} > + > +sub commit_config { > + my $cfg = compile_running_cfg(); > + > cfs_write_file($running_cfg, $cfg); > } >
