all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "DERUMIER, Alexandre" <alexandre.derumier@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>,
	"s.hanreich@proxmox.com" <s.hanreich@proxmox.com>
Subject: Re: [RFC pve-network 1/1] sdn: apply changes on all nodes in parallel
Date: Mon, 21 Sep 2026 10:56:17 +0000	[thread overview]
Message-ID: <081589622ba199dd6576bb504165b50c8e263e81.camel@groupe-cyllene.com> (raw)
In-Reply-To: <20260709140625.275618-1-s.hanreich@proxmox.com>

Hi, 

could we have an option to enable/disable it ?

I remember to have some flood with evpn && frr peering when reloading 
all nodes at same time in the past. (don't remember exactly on which
change, but it was on frr side)




Le jeudi 09 juillet 2026 à 16:06 +0200, Stefan Hanreich a écrit :
> Utilize the API client to spawn tasks on each node and then monitor
> the progress of the tasks via the API as well. This allows for
> starting the reload tasks in parallel. It is particularly useful for
> large setups, where applying the SDN configuration can take a
> considerable amount of time.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
> 
> Notes:
>     Sending this as an RFC, as this requires careful testing imo.
>     
>     An example where this change could backfire has been encountered
>     recently. A user applied the SDN configuration, which broke the
>     cluster / management network of the node that was carrying out
> the
>     reload network task. Since it was executed sequentially, only one
> node
>     was affected. In this scenario, with this patch applied, the
> whole
>     cluster would have been down.
>     
>     We could potentially provide an option that limits the amount of
> nodes
>     where the reload task runs in parallel and expose that in the UI?
>     
>     Also, currently the task could never fail and errors were shown
> in the
>     respective reload tasks executed on the nodes, I've kept that
> behavior
>     the same, but introduced warnings if there are any issues with
> API
>     calls.
> 
>  src/PVE/API2/Network/SDN.pm | 109 ++++++++++++++++++++++++++--------
> --
>  1 file changed, 78 insertions(+), 31 deletions(-)
> 
> diff --git a/src/PVE/API2/Network/SDN.pm
> b/src/PVE/API2/Network/SDN.pm
> index e3c8d9dd..dd2cf05a 100644
> --- a/src/PVE/API2/Network/SDN.pm
> +++ b/src/PVE/API2/Network/SDN.pm
> @@ -10,6 +10,7 @@ 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);
>  use PVE::RESTHandler;
> +use PVE::RESTEnvironment qw(log_warn);
>  use PVE::RPCEnvironment;
>  use PVE::SafeSyslog;
>  use PVE::Tools qw(run_command extract_param);
> @@ -107,30 +108,6 @@ __PACKAGE__->register_method({
>      },
>  });
>  
> -my $create_reload_network_worker = sub {
> -    my ($nodename, $regenerate_frr) = @_;
> -
> -    my @command = ('pvesh', 'set', "/nodes/$nodename/network");
> -    push(@command, '--regenerate-frr', $regenerate_frr);
> -
> -    # FIXME: how to proxy to final node ?
> -    my $upid;
> -    print "$nodename: reloading network config\n";
> -    run_command(
> -        \@command,
> -        outfunc => sub {
> -            my $line = shift;
> -            if ($line =~ /["']?(UPID:[^\s"']+)["']?$/) {
> -                $upid = $1;
> -            }
> -        },
> -    );
> -    #my $upid = PVE::API2::Network->reload_network_config({ node =>
> $nodename });
> -    my $res = PVE::Tools::upid_decode($upid);
> -
> -    return $res->{pid};
> -};
> -
>  __PACKAGE__->register_method({
>      name => 'lock',
>      protected => 1,
> @@ -282,6 +259,41 @@ __PACKAGE__->register_method({
>      },
>  });
>  
> +sub create_api_client {
> +    my ($request_timeout) = @_;
> +
> +    my $rpcenv = PVE::RPCEnvironment::get();
> +    my $authuser = $rpcenv->get_user();
> +    my $credentials = $rpcenv->get_credentials();
> +
> +    my $api_token = $credentials->{api_token};
> +    my $ticket = $credentials->{ticket};
> +    my $csrf_token = $credentials->{token};
> +
> +    my $node = PVE::INotify::nodename();
> +    my $fingerprint = PVE::Cluster::get_node_fingerprint($node);
> +
> +    my $conn_args = {
> +        protocol => 'https',
> +        host => 'localhost', # always call the api locally, let
> pveproxy handle the proxying
> +        port => 8006,
> +        username => $authuser,
> +        ticket => $ticket,
> +        apitoken => $api_token,
> +        timeout => $request_timeout // 25, # default slightly
> shorter than the proxy->daemon timeout
> +        cached_fingerprints => {
> +            $fingerprint => 1,
> +        },
> +    };
> +
> +    my $api_client = PVE::APIClient::LWP->new($conn_args->%*);
> +    if (defined($csrf_token)) {
> +        $api_client->update_csrftoken($csrf_token);
> +    }
> +
> +    return $api_client;
> +}
> +
>  __PACKAGE__->register_method({
>      name => 'reload',
>      protected => 1,
> @@ -291,6 +303,7 @@ __PACKAGE__->register_method({
>      permissions => {
>          check => ['perm', '/sdn', ['SDN.Allocate']],
>      },
> +    expose_credentials => 1,
>      parameters => {
>          additionalProperties => 0,
>          properties => {
> @@ -336,22 +349,56 @@ __PACKAGE__->register_method({
>          my $regenerate_frr = ($previous_config_has_frr ||
> $new_config_has_frr) ? 1 : 0;
>  
>          my $code = sub {
> -            $rpcenv->{type} = 'priv'; # to start tasks in background
>              PVE::Cluster::check_cfs_quorum();
> +
> +            my $api_client = create_api_client();
>              my $nodelist = PVE::Cluster::get_nodelist();
> +
> +            my $tasks = {};
> +
>              for my $node (@$nodelist) {
> -                my $pid = eval { $create_reload_network_worker-
> >($node, $regenerate_frr) };
> -                warn $@ if $@;
> +                print "$node: reloading network config\n";
> +
> +                my $upid = eval {
> +                    $api_client->put(
> +                        "/nodes/$node/network",
> +                        {
> +                            'regenerate-frr' => $regenerate_frr,
> +                        },
> +                    );
> +                };
> +
> +                if ($@) {
> +                    log_warn("$node: could not reload network
> configuration: $@\n");
> +                    next;
> +                }
> +
> +                $tasks->{$upid} = $node;
>              }
>  
> -            # FIXME: use libpve-apiclient (like in cluster join) to
> create
> -            # tasks and moitor the tasks.
> +            print "waiting for reload tasks to finish\n";
>  
> -            return;
> +            while ($tasks->%*) {
> +                for my $upid (keys $tasks->%*) {
> +                    my $node = $tasks->{$upid};
> +                    my $task = eval { $api_client-
> >get("/nodes/$node/tasks/$upid/status") };
> +
> +                    if ($@) {
> +                        log_warn("could not get status of reload
> task: $@\n");
> +                        delete $tasks->{$upid};
> +                        next;
> +                    }
> +
> +                    next if $task->{status} eq 'running';
> +                    print "$node: reload task finished\n";
> +                    delete $tasks->{$upid};
> +                }
> +
> +                sleep(1);
> +            }
>          };
>  
>          return $rpcenv->fork_worker('reloadnetworkall', undef,
> $authuser, $code);
> -
>      },
>  });
>  

      parent reply	other threads:[~2026-09-21 10:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09 14:06 [RFC pve-network 1/1] sdn: apply changes on all nodes in parallel Stefan Hanreich
2026-07-13  8:58 ` Daniel Herzig
2026-09-18  9:50 ` Gabriel Goller
2026-09-18 11:21   ` Stefan Hanreich
2026-09-18 11:28     ` Gabriel Goller
2026-09-21 10:56 ` DERUMIER, Alexandre [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=081589622ba199dd6576bb504165b50c8e263e81.camel@groupe-cyllene.com \
    --to=alexandre.derumier@groupe-cyllene.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=s.hanreich@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal