all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Stefan Hanreich <s.hanreich@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [RFC pve-network 1/1] sdn: apply changes on all nodes in parallel
Date: Thu,  9 Jul 2026 16:06:24 +0200	[thread overview]
Message-ID: <20260709140625.275618-1-s.hanreich@proxmox.com> (raw)

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





                 reply	other threads:[~2026-07-09 14:06 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20260709140625.275618-1-s.hanreich@proxmox.com \
    --to=s.hanreich@proxmox.com \
    --cc=pve-devel@lists.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