* Re: [RFC pve-network 1/1] sdn: apply changes on all nodes in parallel
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-21 10:56 ` DERUMIER, Alexandre
2 siblings, 0 replies; 6+ messages in thread
From: Daniel Herzig @ 2026-07-13 8:58 UTC (permalink / raw)
To: Stefan Hanreich; +Cc: pve-devel
Thanks for working on this!
I've tested the patch in 2 scenarios on a three-node cluster:
(1) Worst case (by using VLAN tag 1 on a VNet
backed by a VLAN aware management bridge with default settings)
(2) Good case (doing the same, but using VLAN tag 3 on the same bridge)
Apart from the (expected) instant breaking of the network in case (1)
the patch leads to a more consistent state as previously. The earlier
sequential approach lead to inconsistent configuration states across the
clusternodes (depending on which node was reached first, or -- to which
node access to the management-IP was lost first). So I'd see the
parallel approach as an enhancement in the worst-case setup.
Case (2) just worked nicely, and makes application quite somewhat faster.
I haven't gone through further-reaching implications yet, but my first
impression of the patch is quite good by empirical testing of the above
situations.
Stefan Hanreich <s.hanreich@proxmox.com> writes:
> 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);
> -
> },
> });
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC pve-network 1/1] sdn: apply changes on all nodes in parallel
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-21 10:56 ` DERUMIER, Alexandre
2 siblings, 1 reply; 6+ messages in thread
From: Gabriel Goller @ 2026-09-18 9:50 UTC (permalink / raw)
To: Stefan Hanreich; +Cc: pve-devel
A few small nits inline, no blockers though.
Otherwise LGTM
Consider:
Reviewed-by: Gabriel Goller <g.goller@proxmox.com>
> 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']],
Hmm this endpoint has SDN.Allocate and the PUT /network endpoint we are calling
(in pve-manager) needs Sys.Modify. This shouldn't be an issue, but maybe we
should require Sys.Modify here as well to short-circuit everything, instead of
making a request to every node.
> },
> + 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;
> + }
Hmm should we wait more than 1 second here maybe? What if the reload brings the
network down shortly... Maybe add a counter and require a task to be gone for
3-4 seconds?
> +
> + next if $task->{status} eq 'running';
Should we add a exitstatus check here and print a warning like "reload failed"
or something?
> + print "$node: reload task finished\n";
> + delete $tasks->{$upid};
> + }
> +
> + sleep(1);
Do the pve tasks have a maximum duration where they get killed if they take
longer? Otherwise a stuck task will make this loop forever.
Although this is a task as well isn't it -- it can just be force-stopped in
the ui.
> + }
> };
>
> return $rpcenv->fork_worker('reloadnetworkall', undef, $authuser, $code);
> -
> },
> });
>
> --
> 2.47.3
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC pve-network 1/1] sdn: apply changes on all nodes in parallel
2026-09-18 9:50 ` Gabriel Goller
@ 2026-09-18 11:21 ` Stefan Hanreich
2026-09-18 11:28 ` Gabriel Goller
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Hanreich @ 2026-09-18 11:21 UTC (permalink / raw)
To: Gabriel Goller; +Cc: pve-devel
On 9/18/26 11:50 AM, Gabriel Goller wrote:
> A few small nits inline, no blockers though.
> Otherwise LGTM
>
> Consider:
> Reviewed-by: Gabriel Goller <g.goller@proxmox.com>
>
>> 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']],
>
> Hmm this endpoint has SDN.Allocate and the PUT /network endpoint we are calling
> (in pve-manager) needs Sys.Modify. This shouldn't be an issue, but maybe we
> should require Sys.Modify here as well to short-circuit everything, instead of
> making a request to every node.
Hmm, pre-existing, but what about setups where a user has Sys.Modify only on some nodes? That's
currently possible but probably quite bad, since it allows applying the SDN configuration to
some nodes only. So, I agree - but a breaking change?
>> },
>> + 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;
>> + }
>
> Hmm should we wait more than 1 second here maybe? What if the reload brings the
> network down shortly... Maybe add a counter and require a task to be gone for
> 3-4 seconds?
yes, potentially even a re-try logic before giving up completely...
>> +
>> + next if $task->{status} eq 'running';
>
> Should we add a exitstatus check here and print a warning like "reload failed"
> or something?
>
>> + print "$node: reload task finished\n";
>> + delete $tasks->{$upid};
>> + }
>> +
>> + sleep(1);
>
> Do the pve tasks have a maximum duration where they get killed if they take
> longer? Otherwise a stuck task will make this loop forever.
>
> Although this is a task as well isn't it -- it can just be force-stopped in
> the ui.
Should be fine imo, due to the possibility of cancelling the task itself.
>> + }
>> };
>>
>> return $rpcenv->fork_worker('reloadnetworkall', undef, $authuser, $code);
>> -
>> },
>> });
>>
>> --
>> 2.47.3
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC pve-network 1/1] sdn: apply changes on all nodes in parallel
2026-09-18 11:21 ` Stefan Hanreich
@ 2026-09-18 11:28 ` Gabriel Goller
0 siblings, 0 replies; 6+ messages in thread
From: Gabriel Goller @ 2026-09-18 11:28 UTC (permalink / raw)
To: Stefan Hanreich; +Cc: pve-devel
On 18.09.2026 13:21, Stefan Hanreich wrote:
> >> [snip]
> >> @@ -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']],
> >
> > Hmm this endpoint has SDN.Allocate and the PUT /network endpoint we are calling
> > (in pve-manager) needs Sys.Modify. This shouldn't be an issue, but maybe we
> > should require Sys.Modify here as well to short-circuit everything, instead of
> > making a request to every node.
>
> Hmm, pre-existing, but what about setups where a user has Sys.Modify only on some nodes? That's
> currently possible but probably quite bad, since it allows applying the SDN configuration to
> some nodes only. So, I agree - but a breaking change?
Aah, this is obviously a breaking change -- you're right.
Then this is fine IMO.
> >> },
> >> + expose_credentials => 1,
> >> parameters => {
> >> additionalProperties => 0,
> >> properties => {
> >> [snip]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC pve-network 1/1] sdn: apply changes on all nodes in parallel
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-21 10:56 ` DERUMIER, Alexandre
2 siblings, 0 replies; 6+ messages in thread
From: DERUMIER, Alexandre @ 2026-09-21 10:56 UTC (permalink / raw)
To: pve-devel@lists.proxmox.com, 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);
> -
> },
> });
>
^ permalink raw reply [flat|nested] 6+ messages in thread