[PATCH ha-manager v2] resources: Expand max_restart option

[PATCH ha-manager v2] resources: Expand max_restart option

[Maximiliano Sandoval]

When read next to `max_relocate` it is not clear which happens first
after a service fails to start.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>

---

Differences from v1:
 - Incorporate feedback. Namely, "the service will be attempted to be relocated"
   was a bit too convoluted.

 src/PVE/HA/Resources.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/HA/Resources.pm b/src/PVE/HA/Resources.pm
index 68d9d16..404f6dc 100644
--- a/src/PVE/HA/Resources.pm
+++ b/src/PVE/HA/Resources.pm
@@ -73,7 +73,8 @@ EODESC
         },
         max_restart => {
             description => "Maximal number of tries to restart the service on"
-                . " a node after its start failed.",
+                . " a node after its start failed. When reached, the HA manager will try to"
+                . " relocate the service to an eligible node.",
             type => 'integer',
             optional => 1,
             default => 1,
-- 
2.47.3

Re: [PATCH ha-manager v2] resources: Expand max_restart option

[Michael Köppl]

I prefer this one over v1 because I think it's more comprehensible. lgtm

Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>

On Tue Mar 3, 2026 at 2:22 PM CET, Maximiliano Sandoval wrote:
> When read next to `max_relocate` it is not clear which happens first
> after a service fails to start.
>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>
> ---
>
> Differences from v1:
>  - Incorporate feedback. Namely, "the service will be attempted to be relocated"
>    was a bit too convoluted.
>
>  src/PVE/HA/Resources.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/HA/Resources.pm b/src/PVE/HA/Resources.pm
> index 68d9d16..404f6dc 100644
> --- a/src/PVE/HA/Resources.pm
> +++ b/src/PVE/HA/Resources.pm
> @@ -73,7 +73,8 @@ EODESC
>          },
>          max_restart => {
>              description => "Maximal number of tries to restart the service on"
> -                . " a node after its start failed.",
> +                . " a node after its start failed. When reached, the HA manager will try to"
> +                . " relocate the service to an eligible node.",
>              type => 'integer',
>              optional => 1,
>              default => 1,

Re: [PATCH ha-manager v2] resources: Expand max_restart option

[Thomas Lamprecht]

Am 03.03.26 um 14:22 schrieb Maximiliano Sandoval:
> When read next to `max_relocate` it is not clear which happens first
> after a service fails to start.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> 
> ---
> 
> Differences from v1:
>  - Incorporate feedback. Namely, "the service will be attempted to be relocated"
>    was a bit too convoluted.
> 
>  src/PVE/HA/Resources.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/HA/Resources.pm b/src/PVE/HA/Resources.pm
> index 68d9d16..404f6dc 100644
> --- a/src/PVE/HA/Resources.pm
> +++ b/src/PVE/HA/Resources.pm
> @@ -73,7 +73,8 @@ EODESC
>          },
>          max_restart => {
>              description => "Maximal number of tries to restart the service on"
> -                . " a node after its start failed.",
> +                . " a node after its start failed. When reached, the HA manager will try to"
> +                . " relocate the service to an eligible node.",


IIRC we decided to try using the term "resources" for the VM/CTs managed by HA
and "services" more for the daemons for new docs/description (and Someday™ clean
up the existing usage). Can be easily fixed up on applying, just wanted to ensure
I did not misremember (CCing also Dano for that)

>              type => 'integer',
>              optional => 1,
>              default => 1,

Re: [PATCH ha-manager v2] resources: Expand max_restart option

[Daniel Kral]

On Tue Mar 3, 2026 at 9:09 PM CET, Thomas Lamprecht wrote:
> Am 03.03.26 um 14:22 schrieb Maximiliano Sandoval:
>> When read next to `max_relocate` it is not clear which happens first
>> after a service fails to start.
>> 
>> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>> 
>> ---
>> 
>> Differences from v1:
>>  - Incorporate feedback. Namely, "the service will be attempted to be relocated"
>>    was a bit too convoluted.
>> 
>>  src/PVE/HA/Resources.pm | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/PVE/HA/Resources.pm b/src/PVE/HA/Resources.pm
>> index 68d9d16..404f6dc 100644
>> --- a/src/PVE/HA/Resources.pm
>> +++ b/src/PVE/HA/Resources.pm
>> @@ -73,7 +73,8 @@ EODESC
>>          },
>>          max_restart => {
>>              description => "Maximal number of tries to restart the service on"
>> -                . " a node after its start failed.",
>> +                . " a node after its start failed. When reached, the HA manager will try to"
>> +                . " relocate the service to an eligible node.",
>
>
> IIRC we decided to try using the term "resources" for the VM/CTs managed by HA
> and "services" more for the daemons for new docs/description (and Someday™ clean
> up the existing usage). Can be easily fixed up on applying, just wanted to ensure
> I did not misremember (CCing also Dano for that)

+1, in descriptions I'd also tend to go for "HA resource" to make sure
the term differentiates itself from "cluster resource" or other kinds of
resources, where it makes sense and doesn't bloat the message length too
much.

s/service/HA resource/ should be done for max_relocate here as well,
maybe in a patch before that.

>
>>              type => 'integer',
>>              optional => 1,
>>              default => 1,

superseded: [PATCH ha-manager v2] resources: Expand max_restart option

[Maximiliano Sandoval]

Maximiliano Sandoval <m.sandoval@proxmox.com> writes:

> When read next to `max_relocate` it is not clear which happens first
> after a service fails to start.
>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>
> ---
>
> Differences from v1:
>  - Incorporate feedback. Namely, "the service will be attempted to be relocated"
>    was a bit too convoluted.
>
>  src/PVE/HA/Resources.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/HA/Resources.pm b/src/PVE/HA/Resources.pm
> index 68d9d16..404f6dc 100644
> --- a/src/PVE/HA/Resources.pm
> +++ b/src/PVE/HA/Resources.pm
> @@ -73,7 +73,8 @@ EODESC
>          },
>          max_restart => {
>              description => "Maximal number of tries to restart the service on"
> -                . " a node after its start failed.",
> +                . " a node after its start failed. When reached, the HA manager will try to"
> +                . " relocate the service to an eligible node.",
>              type => 'integer',
>              optional => 1,
>              default => 1,

Superseded-by: https://lore.proxmox.com/all/20260304083559.75293-1-m.sandoval@proxmox.com/

-- 
Maximiliano