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

[pve-devel] [RFC cluster 1/2] 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 anything without 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).

This parameter will allow to increase the chances for successful
execution of the hook.

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

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/CLI/pvecm.pm | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/PVE/CLI/pvecm.pm b/src/PVE/CLI/pvecm.pm
index 564dc99..94f1e83 100755
--- a/src/PVE/CLI/pvecm.pm
+++ b/src/PVE/CLI/pvecm.pm
@@ -6,7 +6,7 @@ use warnings;
 use Cwd qw(getcwd);
 use File::Path;
 use File::Basename;
-use PVE::Tools qw(run_command);
+use PVE::Tools qw(extract_param run_command);
 use PVE::Cluster;
 use PVE::INotify;
 use PVE::JSONSchema qw(get_standard_option);
@@ -566,12 +566,33 @@ __PACKAGE__->register_method ({
 		type => 'boolean',
 		optional => 1,
 	    },
+	    'quorum-wait-seconds' => {
+		description => "Wait for quorum for this many seconds.",
+		type => 'integer',
+		minimum => 0,
+		optional => 1,
+	    },
 	},
     },
     returns => { type => 'null' },
     code => sub {
 	my ($param) = @_;
 
+	my $quorum_wait = extract_param($param, 'quorum-wait-seconds');
+
+	if ($quorum_wait && !PVE::Cluster::check_cfs_quorum(1)) {
+	    print "waiting for quorum...";
+	    STDOUT->flush();
+	    for (my $i = 0; $i < $quorum_wait; $i++) {
+		if (PVE::Cluster::check_cfs_quorum(1)) {
+		    print "OK";
+		    last;
+		}
+		sleep(1);
+	    }
+	    print "\n";
+	}
+
 	# 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 cluster 2/2] fix typo

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 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

[pve-devel] [RFC manager] pveproxy service: specify wait time for pvecm updatecerts command

[Fiona Ebner]

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 anything without 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).

To increase the chances for successful execution of the hook during
startup, wait for up to 5 seconds for quorum.

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

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

Dependency bump for libpve-cluster-api-perl needed.

 services/pveproxy.service | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/services/pveproxy.service b/services/pveproxy.service
index 51038785..7575faee 100644
--- a/services/pveproxy.service
+++ b/services/pveproxy.service
@@ -11,7 +11,7 @@ After=pvedaemon.service
 After=ssh.service
 
 [Service]
-ExecStartPre=-/usr/bin/pvecm updatecerts --silent
+ExecStartPre=-/usr/bin/pvecm updatecerts --silent --quorum-wait-seconds 5
 ExecStart=/usr/bin/pveproxy start
 ExecStop=/usr/bin/pveproxy stop
 ExecReload=/usr/bin/pveproxy restart
-- 
2.39.2

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

[Thomas Lamprecht]

Am 29/06/2023 um 15:59 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 anything without 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).
> 
> This parameter will allow to increase the chances for successful
> execution of the hook.
> 
> [0]: https://forum.proxmox.com/threads/129644/
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/CLI/pvecm.pm | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 


Hmm, I would just do something like (untested and needs importing Time::HiRes):


@@ -576,6 +578,11 @@ __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 {
+           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);
+               $i++;
+           }
            PVE::Cluster::Setup::updatecerts_and_ssh($param->@{qw(force silent)});
            PVE::Cluster::prepare_observed_file_basedirs();
        });


after all any user or tooling calling this want's it to happen, so waiting until
the timeout seems sensible enough as hard coded default to me..

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

[Fiona Ebner]

Am 29.06.23 um 16:26 schrieb Thomas Lamprecht:
> Am 29/06/2023 um 15:59 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 anything without 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).
>>
>> This parameter will allow to increase the chances for successful
>> execution of the hook.
>>
>> [0]: https://forum.proxmox.com/threads/129644/
>>
>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>> ---
>>  src/PVE/CLI/pvecm.pm | 23 ++++++++++++++++++++++-
>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>
> 
> 
> Hmm, I would just do something like (untested and needs importing Time::HiRes):
> 
> 
> @@ -576,6 +578,11 @@ __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 {
> +           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);
> +               $i++;
> +           }
>             PVE::Cluster::Setup::updatecerts_and_ssh($param->@{qw(force silent)});
>             PVE::Cluster::prepare_observed_file_basedirs();
>         });
> 
> 
> after all any user or tooling calling this want's it to happen, so waiting until
> the timeout seems sensible enough as hard coded default to me..

The issue here is that it would delay the pveproxy.service start a full
30 seconds when a node can't get quorum (e.g. after all nodes in a
cluster were down). Is that tolerable?

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

[Thomas Lamprecht]

Am 29/06/2023 um 16:36 schrieb Fiona Ebner:
> Am 29.06.23 um 16:26 schrieb Thomas Lamprecht:
>> Am 29/06/2023 um 15:59 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 anything without 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).
>>>
>>> This parameter will allow to increase the chances for successful
>>> execution of the hook.
>>>
>>> [0]: https://forum.proxmox.com/threads/129644/
>>>
>>> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
>>> ---
>>>  src/PVE/CLI/pvecm.pm | 23 ++++++++++++++++++++++-
>>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>>
>>
>>
>> Hmm, I would just do something like (untested and needs importing Time::HiRes):
>>
>>
>> @@ -576,6 +578,11 @@ __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 {
>> +           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);
>> +               $i++;
>> +           }
>>             PVE::Cluster::Setup::updatecerts_and_ssh($param->@{qw(force silent)});
>>             PVE::Cluster::prepare_observed_file_basedirs();
>>         });
>>
>>
>> after all any user or tooling calling this want's it to happen, so waiting until
>> the timeout seems sensible enough as hard coded default to me..
> 
> The issue here is that it would delay the pveproxy.service start a full
> 30 seconds when a node can't get quorum (e.g. after all nodes in a
> cluster were down). Is that tolerable?

Yes, but I'm not sure if waiting just 5 seconds is much better if basic files like
certs or parent directories are then missing, and it would only affect cold cluster
boots, which are a bit rare; and I don't think 30s (or well, 25s more) are that much
for those relatively rare cases, especially in a server environments; no guest could
have been started before that anyway (we wait indefinitely in node -> startall),
so it's not like we would delay the actual operations a PVE set up provides.

And a small benefit of trying this first is that it doesn't adds any public switch
to our CLI, so we'd be quite flexible in changing it, if needed.