* [RFC pve-network 1/1] sdn: apply changes on all nodes in parallel
@ 2026-07-09 14:06 Stefan Hanreich
2026-07-13 8:58 ` Daniel Herzig
0 siblings, 1 reply; 2+ messages in thread
From: Stefan Hanreich @ 2026-07-09 14:06 UTC (permalink / raw)
To: pve-devel
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);
-
},
});
--
2.47.3
^ permalink raw reply related [flat|nested] 2+ 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
0 siblings, 0 replies; 2+ 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] 2+ messages in thread
end of thread, other threads:[~2026-07-13 8:58 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox