[RFC pve-network 1/1] sdn: apply changes on all nodes in parallel

[RFC pve-network 1/1] sdn: apply changes on all nodes in parallel

[Stefan Hanreich]

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