Re: [PATCH pve-network v4 9/9] api: add dry-run endpoint for sdn apply to preview changes

[Hannes Laimer <h.laimer@proxmox.com>]

nit inline

On 2026-03-06 13:54, Gabriel Goller wrote:
> Allows users to see the diff of frr and interfaces configuration files
> before applying SDN changes. Previously this was not possible and the
> user had to apply and then see what changed. For the ifupdown2 dry-run
> to work, pull out the running_config generation to the parent functions.
> 
> Also add an option to the compile_running_cfg function so that we can
> skip the /etc/network/interfaces.d/sdn file version bump. This means
> when nothing has been changed and dry-run is pressed, nothing will be
> shown.
> 
> Rename a few constants so that they don't clash with local variables.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  src/PVE/API2/Network/SDN.pm      | 88 ++++++++++++++++++++++++++++++++
>  src/PVE/Network/SDN.pm           | 36 ++++++++-----
>  src/PVE/Network/SDN/Fabrics.pm   | 10 +++-
>  src/PVE/Network/SDN/Zones.pm     |  3 +-
>  src/test/debug/generateconfig.pl |  3 +-
>  src/test/run_test_zones.pl       |  2 +-
>  6 files changed, 124 insertions(+), 18 deletions(-)
> 
> diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
> index b35a588d391d..b98bc9146004 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;
>  

[..]

> +
> +__PACKAGE__->register_method({
> +    name => 'dry-run',
> +    path => 'dry-run',
> +    method => 'GET',
> +    permissions => {
> +        check => ['perm', '/nodes/{node}', ['Sys.Modify']],

very much a nit, and doesn't warrant a v5, but Sys.Audit is probably
more fitting

> +    },
> +    description =>
> +        "Dry-run the SDN apply action and return the difference between the current configuration and the pending configuration",
> +    protected => 1,
> +    proxyto => 'node',
> +    parameters => {
> +        additionalProperties => 0,
> +        properties => {
> +            node => get_standard_option('pve-node'),
> +        },
> +    },
> +
> +    returns => {
> +        type => 'object',
> +        properties => {
> +            "frr-diff" => {
> +                type => 'string',
> +                description =>
> +                    'The difference between the current and pending FRR configuration.',
> +            },
> +            "interfaces-diff" => {
> +                type => 'string',
> +                description =>
> +                    'The difference between the current and pending /etc/network/interfaces.d/sdn configuration.',
> +            },
> +        },
> +    },

[..]

>  
>  PVE::Network::SDN::Zones::write_etc_network_config($network_config);
>  print "/etc/network/interfaces.d/sdn\n";
> diff --git a/src/test/run_test_zones.pl b/src/test/run_test_zones.pl
> index 806225735e6b..8986c5c52c9f 100755
> --- a/src/test/run_test_zones.pl
> +++ b/src/test/run_test_zones.pl
> @@ -145,7 +145,7 @@ foreach my $test (@tests) {
>      my $name = $test;
>      my $expected = read_file("./$test/expected_sdn_interfaces");
>  
> -    my $result = eval { PVE::Network::SDN::generate_raw_etc_network_config() };
> +    my $result = eval { PVE::Network::SDN::generate_raw_etc_network_config($sdn_config) };
>  
>      if (my $err = $@) {
>          diag("got unexpected error - $err");