[pve-devel] [PATCH v2 cluster 1/4] setup: split generation of local (i.e. non-pmxcfs) files out into helper

[pve-devel] [PATCH v2 cluster 1/4] setup: split generation of local (i.e. non-pmxcfs) files out into helper

[Fiona Ebner]

In preparation to wait for quorum in the updatecerts command. The
generation of files that do not depend on quorum should still be done
beforehand.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v2.

 src/PVE/CLI/pvecm.pm     |  1 +
 src/PVE/Cluster/Setup.pm | 10 ++++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/PVE/CLI/pvecm.pm b/src/PVE/CLI/pvecm.pm
index 564dc99..ebc15bd 100755
--- a/src/PVE/CLI/pvecm.pm
+++ b/src/PVE/CLI/pvecm.pm
@@ -576,6 +576,7 @@ __PACKAGE__->register_method ({
 	# IO (on /etc/pve) which can hang (uninterruptedly D state). That'd be
 	# no-good for ExecStartPre as it fails the whole service in this case
 	PVE::Tools::run_fork_with_timeout(30, sub {
+	    PVE::Cluster::Setup::generate_local_files();
 	    PVE::Cluster::Setup::updatecerts_and_ssh($param->@{qw(force silent)});
 	    PVE::Cluster::prepare_observed_file_basedirs();
 	});
diff --git a/src/PVE/Cluster/Setup.pm b/src/PVE/Cluster/Setup.pm
index 108817e..f6b491c 100644
--- a/src/PVE/Cluster/Setup.pm
+++ b/src/PVE/Cluster/Setup.pm
@@ -786,6 +786,7 @@ sub finish_join {
     }
     print "OK\n" if !$printqmsg;
 
+    generate_local_files();
     updatecerts_and_ssh(1);
 
     print "generated new node certificate, restart pveproxy and pvedaemon services\n";
@@ -794,15 +795,16 @@ sub finish_join {
     print "successfully added node '$nodename' to cluster.\n";
 }
 
+sub generate_local_files {
+    setup_rootsshconfig();
+    gen_pve_vzdump_symlink();
+}
+
 sub updatecerts_and_ssh {
     my ($force_new_cert, $silent) = @_;
 
     my $p = sub { print "$_[0]\n" if !$silent };
 
-    setup_rootsshconfig();
-
-    gen_pve_vzdump_symlink();
-
     if (!PVE::Cluster::check_cfs_quorum(1)) {
 	return undef if $silent;
 	die "no quorum - unable to update files\n";
-- 
2.39.2

[pve-devel] [PATCH v2 cluster 2/4] pvecm: updatecerts: allow specifying time to wait for quorum via CLI argument

[Fiona Ebner]

Useful for the updatecerts call triggered via the ExecStartPre hook
for pveproxy.service.

When starting a node that's part of a cluster, there is a time window
between the start of pve-cluster.service and when quorum is reached
(from the node's perspective). pveproxy.service is ordered after
pve-cluster.service, but that does not prevent the ExecStartPre hook
from being executed before the node is part of the quorate partition.

The pvecm updatecerts command won't do much without quorum. Generating
local (non-pmxcfs) files is still done before waiting on quorum.

In particular, it might happen that the base directories for observed
files will not get created during/after the upgrade from Proxmox VE 7
to 8 (reported in the community forum [0] and reproduced right away in
a virtual test cluster).

Waiting on quorum should highly increase the chances for successful
execution of the ExecStartPre hook.

[0]: https://forum.proxmox.com/threads/129644/

Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
    * Different approach: always wait for quorum until timeout.

 src/PVE/CLI/pvecm.pm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/PVE/CLI/pvecm.pm b/src/PVE/CLI/pvecm.pm
index ebc15bd..a0550c2 100755
--- a/src/PVE/CLI/pvecm.pm
+++ b/src/PVE/CLI/pvecm.pm
@@ -6,6 +6,8 @@ use warnings;
 use Cwd qw(getcwd);
 use File::Path;
 use File::Basename;
+use Time::HiRes qw(usleep);
+
 use PVE::Tools qw(run_command);
 use PVE::Cluster;
 use PVE::INotify;
@@ -577,6 +579,12 @@ __PACKAGE__->register_method ({
 	# no-good for ExecStartPre as it fails the whole service in this case
 	PVE::Tools::run_fork_with_timeout(30, sub {
 	    PVE::Cluster::Setup::generate_local_files();
+
+	    for (my $i = 0; !PVE::Cluster::check_cfs_quorum(1); $i++) {
+		print "waiting for pmxcfs mount to appear and get quorate...\n" if $i % 50 == 0;
+		usleep(100 * 1000);
+	    }
+
 	    PVE::Cluster::Setup::updatecerts_and_ssh($param->@{qw(force silent)});
 	    PVE::Cluster::prepare_observed_file_basedirs();
 	});
-- 
2.39.2

Re: [pve-devel] [PATCH v2 cluster 2/4] pvecm: updatecerts: allow specifying time to wait for quorum via CLI argument

[Fiona Ebner]

Am 30.06.23 um 13:49 schrieb Fiona Ebner:
> Useful for the updatecerts call triggered via the ExecStartPre hook
> for pveproxy.service.
> 
> When starting a node that's part of a cluster, there is a time window
> between the start of pve-cluster.service and when quorum is reached
> (from the node's perspective). pveproxy.service is ordered after
> pve-cluster.service, but that does not prevent the ExecStartPre hook
> from being executed before the node is part of the quorate partition.
> 
> The pvecm updatecerts command won't do much without quorum. Generating
> local (non-pmxcfs) files is still done before waiting on quorum.
> 
> In particular, it might happen that the base directories for observed
> files will not get created during/after the upgrade from Proxmox VE 7
> to 8 (reported in the community forum [0] and reproduced right away in
> a virtual test cluster).
> 
> Waiting on quorum should highly increase the chances for successful
> execution of the ExecStartPre hook.
> 
> [0]: https://forum.proxmox.com/threads/129644/
> 
> Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>

Sorry, noticed that I forgot to update the commit title here, will send
a v3.

[pve-devel] [PATCH v2 cluster 3/4] pvecm: updatecerts: silence warning that potentially has no context

[Fiona Ebner]

If timeout is reached, run_fork_with_timeout will warn "got timeout".
When processing triggers for pve-manager (because of ExecStartPre of
pveproxy.service invoking pvecm updatecerts) that warning can appear
in the apt output without any context (output of the forked sub
doesn't appear there). So make sure to silence it, if the silent param
is set.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v2.

Can't use a normal if here, because of the 'local'.

 src/PVE/CLI/pvecm.pm | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/PVE/CLI/pvecm.pm b/src/PVE/CLI/pvecm.pm
index a0550c2..2cdc2cd 100755
--- a/src/PVE/CLI/pvecm.pm
+++ b/src/PVE/CLI/pvecm.pm
@@ -574,6 +574,17 @@ __PACKAGE__->register_method ({
     code => sub {
 	my ($param) = @_;
 
+	# If timeout is reached, run_fork_with_timeout will warn "got timeout". When processing
+	# triggers for pve-manager (because of ExecStartPre of pveproxy.service invoking this
+	# command here) that warning can appear in the apt output without any context (output of the
+	# forked sub doesn't appear there). So make sure to silence it, if the silent param is set.
+	my $filter_timeout_warning = sub {
+	    my ($msg) = @_;
+	    return if $msg =~ m/^got timeout/;
+	    print STDERR $msg;
+	};
+	local $SIG{__WARN__} = $filter_timeout_warning if $param->{silent};
+
 	# we get called by the pveproxy.service ExecStartPre and as we do
 	# IO (on /etc/pve) which can hang (uninterruptedly D state). That'd be
 	# no-good for ExecStartPre as it fails the whole service in this case
-- 
2.39.2

[pve-devel] [PATCH v2 cluster 4/4] fix typo

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

No changes in v2.

 src/PVE/Cluster.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Cluster.pm b/src/PVE/Cluster.pm
index ff777ba..c310a67 100644
--- a/src/PVE/Cluster.pm
+++ b/src/PVE/Cluster.pm
@@ -93,7 +93,7 @@ sub prepare_observed_file_basedirs {
 	next if $f !~ m!^(.*)/[^/]+$!;
 	my $dir = "$basedir/$1";
 	next if -e $dir; # can also be a link, so just use -e xist check
-	print "creating directory '$dir' for observerd files\n";
+	print "creating directory '$dir' for observed files\n";
 	make_path($dir);
     }
 }
-- 
2.39.2