* [pve-devel] [PATCH pve-cluster 1/1] notify: add common_template_data
2025-03-27 14:23 [pve-devel] [PATCH cluster/ha-manager/manager 0/6] preparation for #6143: notification template cleanup Lukas Wagner
@ 2025-03-27 14:23 ` Lukas Wagner
2025-03-27 15:31 ` Thomas Lamprecht
2025-03-27 14:23 ` [pve-devel] [PATCH pve-manager 1/4] notification templates: vzdump: generate HTML table in template Lukas Wagner
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Lukas Wagner @ 2025-03-27 14:23 UTC (permalink / raw)
To: pve-devel
This commit add the `common_template_data` sub to PVE::Notify,
providing a convenient way to get a hash with properties that
should be accessible from all templates, namely hostname, fqdn
and cluster-name.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/PVE/Notify.pm | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/src/PVE/Notify.pm b/src/PVE/Notify.pm
index c514111..9f7a74c 100644
--- a/src/PVE/Notify.pm
+++ b/src/PVE/Notify.pm
@@ -4,6 +4,7 @@ use strict;
use warnings;
use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_lock_file cfs_write_file);
+use PVE::INotify;
use Proxmox::RS::Notify;
cfs_register_file(
@@ -130,4 +131,23 @@ sub check_may_use_target {
}
}
+sub common_template_data {
+ my $hostname = PVE::INotify::nodename();
+ my $fqdn = `hostname -f` || $hostname;
+ chomp $fqdn;
+
+ my $data = {
+ hostname => $hostname,
+ fqdn => $fqdn,
+ };
+
+ my $cluster_info = PVE::Cluster::get_clinfo();
+
+ if (my $d = $cluster_info->{cluster}) {
+ $data->{"cluster-name"} = $d->{name};
+ }
+
+ return $data;
+}
+
1;
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [pve-devel] [PATCH pve-cluster 1/1] notify: add common_template_data
2025-03-27 14:23 ` [pve-devel] [PATCH pve-cluster 1/1] notify: add common_template_data Lukas Wagner
@ 2025-03-27 15:31 ` Thomas Lamprecht
2025-03-28 8:28 ` Lukas Wagner
0 siblings, 1 reply; 12+ messages in thread
From: Thomas Lamprecht @ 2025-03-27 15:31 UTC (permalink / raw)
To: Proxmox VE development discussion, Lukas Wagner
Am 27.03.25 um 15:23 schrieb Lukas Wagner:
> This commit add the `common_template_data` sub to PVE::Notify,
> providing a convenient way to get a hash with properties that
> should be accessible from all templates, namely hostname, fqdn
> and cluster-name.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> src/PVE/Notify.pm | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/src/PVE/Notify.pm b/src/PVE/Notify.pm
> index c514111..9f7a74c 100644
> --- a/src/PVE/Notify.pm
> +++ b/src/PVE/Notify.pm
> @@ -4,6 +4,7 @@ use strict;
> use warnings;
>
> use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_lock_file cfs_write_file);
> +use PVE::INotify;
> use Proxmox::RS::Notify;
>
> cfs_register_file(
> @@ -130,4 +131,23 @@ sub check_may_use_target {
> }
> }
>
> +sub common_template_data {
> + my $hostname = PVE::INotify::nodename();
> + my $fqdn = `hostname -f` || $hostname;
> + chomp $fqdn;
A bit odd seeing the highly efficient hostname cached from inotify used
with getting the FQDN in a rather expensive (fork+exec) way.
Not so keen on the use of backticks, albeit it would not be a huge
problem.
I'd favor using PVE::Tools::get_fqdn which should work as well as calling
`hostname -f` but also be quite a bit cheaper due to avoiding fork+exec.
It'd be probably even fine to cache in a module-local variable, as this
is not something that changes often, but also probably not a must for now.
> +
> + my $data = {
> + hostname => $hostname,
> + fqdn => $fqdn,
> + };
> +
> + my $cluster_info = PVE::Cluster::get_clinfo();
> +
> + if (my $d = $cluster_info->{cluster}) {
> + $data->{"cluster-name"} = $d->{name};
> + }
> +
> + return $data;
> +}
> +
> 1;
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [pve-devel] [PATCH pve-cluster 1/1] notify: add common_template_data
2025-03-27 15:31 ` Thomas Lamprecht
@ 2025-03-28 8:28 ` Lukas Wagner
2025-03-28 9:38 ` Thomas Lamprecht
0 siblings, 1 reply; 12+ messages in thread
From: Lukas Wagner @ 2025-03-28 8:28 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox VE development discussion
On 2025-03-27 16:31, Thomas Lamprecht wrote:
> Am 27.03.25 um 15:23 schrieb Lukas Wagner:
>> This commit add the `common_template_data` sub to PVE::Notify,
>> providing a convenient way to get a hash with properties that
>> should be accessible from all templates, namely hostname, fqdn
>> and cluster-name.
>>
>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>> ---
>> src/PVE/Notify.pm | 20 ++++++++++++++++++++
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/src/PVE/Notify.pm b/src/PVE/Notify.pm
>> index c514111..9f7a74c 100644
>> --- a/src/PVE/Notify.pm
>> +++ b/src/PVE/Notify.pm
>> @@ -4,6 +4,7 @@ use strict;
>> use warnings;
>>
>> use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_lock_file cfs_write_file);
>> +use PVE::INotify;
>> use Proxmox::RS::Notify;
>>
>> cfs_register_file(
>> @@ -130,4 +131,23 @@ sub check_may_use_target {
>> }
>> }
>>
>> +sub common_template_data {
>> + my $hostname = PVE::INotify::nodename();
>> + my $fqdn = `hostname -f` || $hostname;
>> + chomp $fqdn;
>
> A bit odd seeing the highly efficient hostname cached from inotify used
> with getting the FQDN in a rather expensive (fork+exec) way.
> Not so keen on the use of backticks, albeit it would not be a huge
> problem.
>
> I'd favor using PVE::Tools::get_fqdn which should work as well as calling
> `hostname -f` but also be quite a bit cheaper due to avoiding fork+exec.
> It'd be probably even fine to cache in a module-local variable, as this
> is not something that changes often, but also probably not a must for now.
>
Yeah, fair. The `hostname -f` was pre-existing in the place where we send APT update notifications,
I just moved it from there. I didn't know that there was a helper for it, good to know!
I'll check it out and send a v2.
We of course can cache the FQDN, but realistically speaking, this is only called once per
notification being sent, thus any real-world performance impact is absolutely tiny.
Thanks for the feedback!
--
- Lukas
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [pve-devel] [PATCH pve-cluster 1/1] notify: add common_template_data
2025-03-28 8:28 ` Lukas Wagner
@ 2025-03-28 9:38 ` Thomas Lamprecht
2025-03-28 10:04 ` Lukas Wagner
0 siblings, 1 reply; 12+ messages in thread
From: Thomas Lamprecht @ 2025-03-28 9:38 UTC (permalink / raw)
To: Proxmox VE development discussion, Lukas Wagner
Am 28.03.25 um 09:28 schrieb Lukas Wagner:
> We of course can cache the FQDN, but realistically speaking, this is only called once per
> notification being sent, thus any real-world performance impact is absolutely tiny.
Not so sure about that in general, e.g. sending out notifications could
correlate with an overloaded system, and for really overloaded systems
things that are normally cheap suddenly ain't – e.g., on low memory
situations even a doing a plain fork+exec of a tiny binary can hang for
a long time then, socket operations like our helper does are definitively
less problematic (I think, as I did not evaluate it [0] and that's something
one can less easily experience directly compared to the former, where even
starting a new basic dash shell on such overloaded system can need minutes).
And as the notification system now also handles things like HA events it's
definitively part of the more critical systems which _can_ justify some extra
scrutiny. That said, switching to the get_fqdn method makes this indeed quite
cheap to get [0], so I'm fine with not doing any caching here for now, but
let's not underestimate the impact of such things too much, especially for
anything in critical chains that can be important in critical (load) situation
(as general strategy for all, as I'm really not thinking about you here, and
it certainly is a balance).
[0]: FWIW, I just did a quick evaluation of querying the fqdn 100 000 times
with the socket variant and the hostname one, this was done on a very healthy
system though, I'd expect that the fork+exec one degrades a lot worse with
higher cpu/memory pressure. Test and result:
# perl -wE 'use PVE::Tools; use Time::HiRes qw(gettimeofday tv_interval); my $t0 = [gettimeofday]; for(my $i = 0; $i <= 100_000; $i++) { my $fqdn = PVE::Tools::get_fqdn("nina"); } my $elapsed = tv_interval ( $t0, [gettimeofday]); say "elapsed (s): ". $elapsed;'
elapsed (s): 0.436712
Same with 1 million runs gets me 4.368217 s, so seems to scale quite linearly.
# perl -wE 'use Time::HiRes qw(gettimeofday tv_interval); my $t0 = [gettimeofday]; for(my $i = 0; $i <= 100_000; $i++) { my $fqdn = `hostname -f`; } my $elapsed = tv_interval ( $t0, [gettimeofday]); say "elapsed: ". $elapsed;'
elapsed (s): 82.484177
Same with 1 million runs gets me 577.653117 s, so not fully linearly, but
in any way about 188x and 132x times slower, respectively.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [pve-devel] [PATCH pve-cluster 1/1] notify: add common_template_data
2025-03-28 9:38 ` Thomas Lamprecht
@ 2025-03-28 10:04 ` Lukas Wagner
0 siblings, 0 replies; 12+ messages in thread
From: Lukas Wagner @ 2025-03-28 10:04 UTC (permalink / raw)
To: Proxmox VE development discussion, Thomas Lamprecht
On 2025-03-28 10:38, Thomas Lamprecht wrote:
> Am 28.03.25 um 09:28 schrieb Lukas Wagner:
>> We of course can cache the FQDN, but realistically speaking, this is only called once per
>> notification being sent, thus any real-world performance impact is absolutely tiny.
>
> Not so sure about that in general, e.g. sending out notifications could
> correlate with an overloaded system, and for really overloaded systems
> things that are normally cheap suddenly ain't – e.g., on low memory
> situations even a doing a plain fork+exec of a tiny binary can hang for
> a long time then, socket operations like our helper does are definitively
> less problematic (I think, as I did not evaluate it [0] and that's something
> one can less easily experience directly compared to the former, where even
> starting a new basic dash shell on such overloaded system can need minutes).
Yes, my 'impact is tiny' was mostly based on already using PVE::Tools::get_fqdn. For the
old fork+exec version you are absolutely right, this could take very long on a
system at its limits.
>
> And as the notification system now also handles things like HA events it's
> definitively part of the more critical systems which _can_ justify some extra
> scrutiny. That said, switching to the get_fqdn method makes this indeed quite
> cheap to get [0], so I'm fine with not doing any caching here for now, but
> let's not underestimate the impact of such things too much, especially for
> anything in critical chains that can be important in critical (load) situation
> (as general strategy for all, as I'm really not thinking about you here, and
> it certainly is a balance).
That makes a lot of sense. Thanks for the detailed explanation, highly appreciated.
Actually I will add caching right away, it a pretty trivial change anyways.
>
> [0]: FWIW, I just did a quick evaluation of querying the fqdn 100 000 times
> with the socket variant and the hostname one, this was done on a very healthy
> system though, I'd expect that the fork+exec one degrades a lot worse with
> higher cpu/memory pressure. Test and result:
>
> # perl -wE 'use PVE::Tools; use Time::HiRes qw(gettimeofday tv_interval); my $t0 = [gettimeofday]; for(my $i = 0; $i <= 100_000; $i++) { my $fqdn = PVE::Tools::get_fqdn("nina"); } my $elapsed = tv_interval ( $t0, [gettimeofday]); say "elapsed (s): ". $elapsed;'
> elapsed (s): 0.436712
>
> Same with 1 million runs gets me 4.368217 s, so seems to scale quite linearly.
>
> # perl -wE 'use Time::HiRes qw(gettimeofday tv_interval); my $t0 = [gettimeofday]; for(my $i = 0; $i <= 100_000; $i++) { my $fqdn = `hostname -f`; } my $elapsed = tv_interval ( $t0, [gettimeofday]); say "elapsed: ". $elapsed;'
> elapsed (s): 82.484177
>
> Same with 1 million runs gets me 577.653117 s, so not fully linearly, but
> in any way about 188x and 132x times slower, respectively.
Good to know, thanks for backing that up with some concrete data! :)
--
- Lukas
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [pve-devel] [PATCH pve-manager 1/4] notification templates: vzdump: generate HTML table in template
2025-03-27 14:23 [pve-devel] [PATCH cluster/ha-manager/manager 0/6] preparation for #6143: notification template cleanup Lukas Wagner
2025-03-27 14:23 ` [pve-devel] [PATCH pve-cluster 1/1] notify: add common_template_data Lukas Wagner
@ 2025-03-27 14:23 ` Lukas Wagner
2025-03-27 14:23 ` [pve-devel] [PATCH pve-manager 2/4] notifications: apt: clean up notification template Lukas Wagner
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Lukas Wagner @ 2025-03-27 14:23 UTC (permalink / raw)
To: pve-devel
Instead of relying on the 'magic' table helper, we generate the guest
list in the vzdump HTML notification template using native Handlebars
syntax ({{#each}}). The template becomes a bit more ugly, mostly due to
HTML email's requirements to use inline CSS styling, but in the context
of providing user-customizable notification templates this is much
nicer. It gives the user a starting point for their modificiations
(custom styling, changing table formatting/order/etc.), which was not
possible before.
The plaintext template stays as is for now, mostly due to the fact that
the table helper automatically aligns table columns - something that
is not easily possible with native Handlebars syntax.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
PVE/VZDump.pm | 21 +++++++++------------
templates/default/vzdump-body.html.hbs | 24 ++++++++++++++++++++++--
templates/default/vzdump-body.txt.hbs | 2 +-
templates/default/vzdump-subject.txt.hbs | 2 +-
4 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index fd89945e..e0fbc555 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -528,16 +528,13 @@ sub send_notification {
"See Task History for details!\n";
};
- my $notification_props = {
- # Hostname, might include domain part
- "hostname" => get_hostname(),
- "error-message" => $err,
- "guest-table" => build_guest_table($tasklist),
- "logs" => $text_log_part,
- "status-text" => $status_text,
- "total-time" => $total_time,
- "total-size" => $total_size,
- };
+ my $template_data = PVE::Notify::common_template_data();
+ $template_data->{error} = $err;
+ $template_data->{"guest-table"} = build_guest_table($tasklist);
+ $template_data->{logs} = $text_log_part;
+ $template_data->{"status-text"} = $status_text;
+ $template_data->{"total-size"} = $total_size;
+ $template_data->{"total-time"} = $total_time;
my $fields = {
type => "vzdump",
@@ -586,7 +583,7 @@ sub send_notification {
PVE::Notify::notify(
$severity,
"vzdump",
- $notification_props,
+ $template_data,
$fields,
$notification_config
);
@@ -597,7 +594,7 @@ sub send_notification {
PVE::Notify::notify(
$severity,
"vzdump",
- $notification_props,
+ $template_data,
$fields,
);
}
diff --git a/templates/default/vzdump-body.html.hbs b/templates/default/vzdump-body.html.hbs
index b6730cbb..7f2245d7 100644
--- a/templates/default/vzdump-body.html.hbs
+++ b/templates/default/vzdump-body.html.hbs
@@ -1,8 +1,28 @@
<html>
<body>
- {{error-message}}
+ {{error}}
<h1 style="font-size: 1.2em">Details</h1>
- {{table guest-table}}
+ <table style="border: 1px solid;border-collapse=collapse;">
+ <tr>
+ <th style="border: 1px solid;border-collapse=collapse;">VMID</th>
+ <th style="border: 1px solid;border-collapse=collapse;">Name</th>
+ <th style="border: 1px solid;border-collapse=collapse;">Status</th>
+ <th style="border: 1px solid;border-collapse=collapse;">Time</th>
+ <th style="border: 1px solid;border-collapse=collapse;">Size</th>
+ <th style="border: 1px solid;border-collapse=collapse;">Filename</th>
+ </tr>
+ {{#each guest-table.data}}
+ <tr>
+ <td style="border: 1px solid;border-collapse=collapse;">{{this.vmid}}</th>
+ <td style="border: 1px solid;border-collapse=collapse;">{{this.name}}</th>
+ <td style="border: 1px solid;border-collapse=collapse;">{{this.status}}</th>
+ <td style="border: 1px solid;border-collapse=collapse;">{{duration this.time}}</th>
+ <td style="border: 1px solid;border-collapse=collapse;">{{human-bytes this.size}}</th>
+ <td style="border: 1px solid;border-collapse=collapse;">{{this.filename}}</th>
+ </tr>
+ {{/each}}
+ </table>
+ <br/>
Total running time: {{duration total-time}}<br/>
Total size: {{human-bytes total-size}}<br/>
<h1 style="font-size: 1.2em">Logs</h1>
diff --git a/templates/default/vzdump-body.txt.hbs b/templates/default/vzdump-body.txt.hbs
index 90f5b0a4..4bd0fab8 100644
--- a/templates/default/vzdump-body.txt.hbs
+++ b/templates/default/vzdump-body.txt.hbs
@@ -1,4 +1,4 @@
-{{error-message}}
+{{error}}
Details
=======
{{table guest-table}}
diff --git a/templates/default/vzdump-subject.txt.hbs b/templates/default/vzdump-subject.txt.hbs
index 98a3d9aa..107b5fc7 100644
--- a/templates/default/vzdump-subject.txt.hbs
+++ b/templates/default/vzdump-subject.txt.hbs
@@ -1 +1 @@
-vzdump backup status ({{hostname}}): {{status-text}}
+vzdump backup status ({{fqdn}}): {{status-text}}
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* [pve-devel] [PATCH pve-manager 2/4] notifications: apt: clean up notification template
2025-03-27 14:23 [pve-devel] [PATCH cluster/ha-manager/manager 0/6] preparation for #6143: notification template cleanup Lukas Wagner
2025-03-27 14:23 ` [pve-devel] [PATCH pve-cluster 1/1] notify: add common_template_data Lukas Wagner
2025-03-27 14:23 ` [pve-devel] [PATCH pve-manager 1/4] notification templates: vzdump: generate HTML table in template Lukas Wagner
@ 2025-03-27 14:23 ` Lukas Wagner
2025-03-27 14:23 ` [pve-devel] [PATCH pve-manager 3/4] notification: replication: add common properties to template data Lukas Wagner
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Lukas Wagner @ 2025-03-27 14:23 UTC (permalink / raw)
To: pve-devel
Clean up the notification templates to prepare for user-customizable
templates
- Change some of the template variable names to improve clarity
- Generate the table for available updates in the template itself,
not via the 'table' helper. This makes it possible for users
to change the style/structure of the table.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
PVE/API2/APT.pm | 27 ++++++++-----------
.../default/package-updates-body.html.hbs | 15 ++++++++++-
.../default/package-updates-body.txt.hbs | 2 +-
.../default/package-updates-subject.txt.hbs | 2 +-
4 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 47c50961..99c2e152 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -306,43 +306,38 @@ __PACKAGE__->register_method({
schema => {
columns => [
{
- label => "Package",
- id => "package",
+ label => "Package Name",
+ id => "package-name",
},
{
- label => "Old Version",
- id => "old-version",
+ label => "Installed Version",
+ id => "installed-version",
},
{
- label => "New Version",
- id => "new-version",
+ label => "Available Version",
+ id => "available-version",
}
]
},
data => []
};
- my $hostname = `hostname -f` || PVE::INotify::nodename();
- chomp $hostname;
-
my $count = 0;
foreach my $p (sort {$a->{Package} cmp $b->{Package} } @$pkglist) {
next if $p->{NotifyStatus} && $p->{NotifyStatus} eq $p->{Version};
$count++;
push @{$updates_table->{data}}, {
- "package" => $p->{Package},
- "old-version" => $p->{OldVersion},
- "new-version" => $p->{Version}
+ "package-name" => $p->{Package},
+ "installed-version" => $p->{OldVersion},
+ "available-version" => $p->{Version}
};
}
return if !$count;
- my $template_data = {
- updates => $updates_table,
- hostname => $hostname,
- };
+ my $template_data = PVE::Notify::common_template_data();
+ $template_data->{"available-updates"} = $updates_table;
# Additional metadata fields that can be used in notification
# matchers.
diff --git a/templates/default/package-updates-body.html.hbs b/templates/default/package-updates-body.html.hbs
index d058e114..ad29a941 100644
--- a/templates/default/package-updates-body.html.hbs
+++ b/templates/default/package-updates-body.html.hbs
@@ -1,6 +1,19 @@
<html>
<body>
The following updates are available:
- {{table updates}}
+ <table style="border: 1px solid;border-collapse=collapse;margin-top: 5px;">
+ <tr>
+ <th style="border: 1px solid;border-collapse=collapse;">Package Name</th>
+ <th style="border: 1px solid;border-collapse=collapse;">Installed Version</th>
+ <th style="border: 1px solid;border-collapse=collapse;">Available Version</th>
+ </tr>
+ {{#each available-updates.data}}
+ <tr>
+ <td style="border: 1px solid;border-collapse=collapse;">{{this.package-name}}</th>
+ <td style="border: 1px solid;border-collapse=collapse;">{{this.installed-version}}</th>
+ <td style="border: 1px solid;border-collapse=collapse;">{{this.available-version}}</th>
+ </tr>
+ {{/each}}
+ </table>
</body>
</html>
diff --git a/templates/default/package-updates-body.txt.hbs b/templates/default/package-updates-body.txt.hbs
index 14bdbf9e..74969433 100644
--- a/templates/default/package-updates-body.txt.hbs
+++ b/templates/default/package-updates-body.txt.hbs
@@ -1,3 +1,3 @@
The following updates are available:
-{{table updates}}
+{{table available-updates}}
diff --git a/templates/default/package-updates-subject.txt.hbs b/templates/default/package-updates-subject.txt.hbs
index 556a67b8..b73fdefd 100644
--- a/templates/default/package-updates-subject.txt.hbs
+++ b/templates/default/package-updates-subject.txt.hbs
@@ -1 +1 @@
-New software packages available ({{hostname}})
+New software packages available ({{fqdn}})
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* [pve-devel] [PATCH pve-manager 3/4] notification: replication: add common properties to template data
2025-03-27 14:23 [pve-devel] [PATCH cluster/ha-manager/manager 0/6] preparation for #6143: notification template cleanup Lukas Wagner
` (2 preceding siblings ...)
2025-03-27 14:23 ` [pve-devel] [PATCH pve-manager 2/4] notifications: apt: clean up notification template Lukas Wagner
@ 2025-03-27 14:23 ` Lukas Wagner
2025-03-27 14:23 ` [pve-devel] [PATCH pve-manager 4/4] notifications: test: style fixup Lukas Wagner
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Lukas Wagner @ 2025-03-27 14:23 UTC (permalink / raw)
To: pve-devel
The new PVE::Notify::common_template_data helper gives us a hash of
properties which should be available in all notifications (hostname,
fqdn, cluster-name at this moment). This commit makes sure that
replication notifications have these available.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
PVE/API2/Replication.pm | 16 +++++++---------
templates/default/replication-body.txt.hbs | 2 +-
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index e4a7180f..3a2be73a 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -112,15 +112,13 @@ my sub _handle_job_err {
# The replication job is run every 15 mins if no schedule is set.
my $schedule = $job->{schedule} // '*/15';
- my $template_data = {
- "failure-count" => $fail_count,
- "last-sync" => $jobstate->{last_sync},
- "next-sync" => $next_sync,
- "job-id" => $job->{id},
- "job-target" => $job->{target},
- "job-schedule" => $schedule,
- "error" => $err,
- };
+ my $template_data = PVE::Notify::common_template_data();
+ $template_data->{"failure-count"} = $fail_count;
+ $template_data->{"last-sync"} = $jobstate->{last_sync};
+ $template_data->{"job-id"} = $job->{id};
+ $template_data->{"job-target"} = $job->{target};
+ $template_data->{"job-schedule"} = $schedule;
+ $template_data->{"error"} = $err;
my $metadata_fields = {
type => "replication",
diff --git a/templates/default/replication-body.txt.hbs b/templates/default/replication-body.txt.hbs
index a9273fef..b1894ce9 100644
--- a/templates/default/replication-body.txt.hbs
+++ b/templates/default/replication-body.txt.hbs
@@ -9,4 +9,4 @@ Note: The system will now reduce the frequency of error reports, as the job
appears to be stuck.
{{/if}}
Error:
-{{ error }}
+{{error}}
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* [pve-devel] [PATCH pve-manager 4/4] notifications: test: style fixup
2025-03-27 14:23 [pve-devel] [PATCH cluster/ha-manager/manager 0/6] preparation for #6143: notification template cleanup Lukas Wagner
` (3 preceding siblings ...)
2025-03-27 14:23 ` [pve-devel] [PATCH pve-manager 3/4] notification: replication: add common properties to template data Lukas Wagner
@ 2025-03-27 14:23 ` Lukas Wagner
2025-03-27 14:23 ` [pve-devel] [PATCH pve-ha-manager 1/1] notifications: overhaul fence notification Lukas Wagner
2025-03-28 10:21 ` [pve-devel] superseded: [PATCH cluster/ha-manager/manager 0/6] preparation for #6143: notification template cleanup Lukas Wagner
6 siblings, 0 replies; 12+ messages in thread
From: Lukas Wagner @ 2025-03-27 14:23 UTC (permalink / raw)
To: pve-devel
Other commits introduced a consistent style for handlebars expressions,
this commit applies the same style to test notification templates.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
templates/default/test-body.html.hbs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/templates/default/test-body.html.hbs b/templates/default/test-body.html.hbs
index 26a43dde..24454433 100644
--- a/templates/default/test-body.html.hbs
+++ b/templates/default/test-body.html.hbs
@@ -1 +1 @@
-This is a test of the notification target '{{ target }}'.
+This is a test of the notification target '{{target}}'.
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* [pve-devel] [PATCH pve-ha-manager 1/1] notifications: overhaul fence notification
2025-03-27 14:23 [pve-devel] [PATCH cluster/ha-manager/manager 0/6] preparation for #6143: notification template cleanup Lukas Wagner
` (4 preceding siblings ...)
2025-03-27 14:23 ` [pve-devel] [PATCH pve-manager 4/4] notifications: test: style fixup Lukas Wagner
@ 2025-03-27 14:23 ` Lukas Wagner
2025-03-28 10:21 ` [pve-devel] superseded: [PATCH cluster/ha-manager/manager 0/6] preparation for #6143: notification template cleanup Lukas Wagner
6 siblings, 0 replies; 12+ messages in thread
From: Lukas Wagner @ 2025-03-27 14:23 UTC (permalink / raw)
To: pve-devel
- try to make template variable names more clear (in preparation
for #6143)
- add common tempate variables (fqdn, hostname, cluster-name)
- Instead of dumping the status-data variable as a JSON blob we
add template variables for the most useful information and
render it in a structured manner
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/PVE/HA/NodeStatus.pm | 43 +++++++++++++++----
src/PVE/HA/Sim/Env.pm | 8 ++--
src/templates/default/fencing-body.html.hbs | 43 ++++++++++++++++---
src/templates/default/fencing-body.txt.hbs | 40 +++++++++++++----
src/templates/default/fencing-subject.txt.hbs | 6 ++-
5 files changed, 111 insertions(+), 29 deletions(-)
diff --git a/src/PVE/HA/NodeStatus.pm b/src/PVE/HA/NodeStatus.pm
index 9e6d898..3a42746 100644
--- a/src/PVE/HA/NodeStatus.pm
+++ b/src/PVE/HA/NodeStatus.pm
@@ -3,6 +3,8 @@ package PVE::HA::NodeStatus;
use strict;
use warnings;
+use PVE::Notify;
+
use JSON;
my $fence_delay = 60;
@@ -195,15 +197,38 @@ my $send_fence_state_email = sub {
my $haenv = $self->{haenv};
my $status = $haenv->read_manager_status();
- my $template_data = {
- "status-data" => {
- manager_status => $status,
- node_status => $self->{status}
- },
- "node" => $node,
- "subject-prefix" => $subject_prefix,
- "subject" => $subject,
- };
+ my $template_data = PVE::Notify::common_template_data();
+ # Those two are needed for the expected output for test cases,
+ # see src/PVE/HA/Sim/Env.pm
+ $template_data->{"fence-status"} = $subject;
+ $template_data->{"fence-prefix"} = $subject_prefix;
+
+ $template_data->{"is-success"} = 1 ? $subject_prefix eq "SUCCEED" : 0;
+
+ $template_data->{"failed-node"} = $node;
+ $template_data->{"master-node"} = $status->{master_node};
+ # There is a handlebars helper 'timestamp', we should not
+ # name a variable the same way.
+ $template_data->{"fence-timestamp"} = $status->{timestamp};
+
+ $template_data->{"nodes"} = [];
+ for my $key (sort keys $status->{node_status}->%*) {
+ push $template_data->{"nodes"}->@*, {
+ node => $key,
+ status => $status->{node_status}->{$key}
+ };
+ }
+
+ $template_data->{"resources"} = [];
+ for my $key (sort keys $status->{service_status}->%*) {
+ my $resource_status = $status->{service_status}->{$key};
+ push $template_data->{"resources"}->@*, {
+ resource => $key,
+ state => $resource_status->{state},
+ node => $resource_status->{node},
+ running => $resource_status->{running},
+ };
+ }
my $metadata_fields = {
type => 'fencing',
diff --git a/src/PVE/HA/Sim/Env.pm b/src/PVE/HA/Sim/Env.pm
index b2ab231..f250d43 100644
--- a/src/PVE/HA/Sim/Env.pm
+++ b/src/PVE/HA/Sim/Env.pm
@@ -299,12 +299,12 @@ sub log {
sub send_notification {
my ($self, $template_name, $properties) = @_;
- # The template for the subject is "{{subject-prefix}}: {{subject}}"
+ # The template for the subject is "{{fence-status}}: {{fence-message}}"
# We have to perform poor-man's template rendering to pass the test cases.
- my $subject = "{{subject-prefix}}: {{subject}}";
- $subject = $subject =~ s/\{\{subject-prefix}}/$properties->{"subject-prefix"}/r;
- $subject = $subject =~ s/\{\{subject}}/$properties->{"subject"}/r;
+ my $subject = "{{fence-prefix}}: {{fence-status}}";
+ $subject = $subject =~ s/\{\{fence-prefix}}/$properties->{"fence-prefix"}/r;
+ $subject = $subject =~ s/\{\{fence-status}}/$properties->{"fence-status"}/r;
# only log subject, do not spam the logs
$self->log('email', $subject);
diff --git a/src/templates/default/fencing-body.html.hbs b/src/templates/default/fencing-body.html.hbs
index 1420348..901a0e1 100644
--- a/src/templates/default/fencing-body.html.hbs
+++ b/src/templates/default/fencing-body.html.hbs
@@ -1,14 +1,43 @@
<html>
<body>
- The node '{{node}}' failed and needs manual intervention.<br/><br/>
+ The node '{{failed-node}}' in cluster '{{cluster-name}}' failed and
+ needs manual intervention.<br/><br/>
- The PVE HA manager tries to fence it and recover the configured HA resources to
- a healthy node if possible.<br/><br/>
+ {{#if is-success~}}
+ The PVE HA manager successfully fenced '{{failed-node}}'.<br/><br/>
+ {{else}}
+ The PVE HA manager will now fence '{{failed-node}}'.<br/><br/>
+ {{/if}}
- Current fence status: {{subject-prefix}}<br/>
- {{subject}}<br/>
+ <b>Status:</b> {{fence-status}}<br/>
+ <b>Timestamp:</b> {{timestamp fence-timestamp}}<br/>
- <h2 style="font-size: 1em">Overall Cluster status:</h2>
- {{object status-data}}
+ <h2 style="font-size: 1em">Cluster Node Status:</h2>
+ <ul>
+ {{#each nodes}}
+ <li>
+ {{this.node}}: {{this.status}} {{#if (eq this.node ../master-node)}}[master]{{/if}}
+ </li>
+ {{/each}}
+ </ul>
+
+ <h2 style="font-size: 1em">HA Resources:</h2>
+ The following HA resources were running on the failed node and will be
+ recovered to a healthy node if possible:
+ <ul>
+ {{#each resources}}
+ {{#if (eq this.node ../failed-node)}}
+ <li>{{this.resource}} [{{this.node}}]: {{this.state}}</li>
+ {{/if}}
+ {{/each}}
+ </ul>
+ The other HA resources in this cluster are:
+ <ul>
+ {{#each resources}}
+ {{#if (ne this.node ../failed-node)}}
+ <li>{{this.resource}} [{{this.node}}]: {{this.state}}</li>
+ {{/if}}
+ {{/each}}
+ </ul>
</body>
</html>
diff --git a/src/templates/default/fencing-body.txt.hbs b/src/templates/default/fencing-body.txt.hbs
index e46a1fd..6f54122 100644
--- a/src/templates/default/fencing-body.txt.hbs
+++ b/src/templates/default/fencing-body.txt.hbs
@@ -1,11 +1,35 @@
-The node '{{node}}' failed and needs manual intervention.
+The node '{{failed-node}}' in cluster '{{cluster-name}}' failed
+and needs manual intervention.
-The PVE HA manager tries to fence it and recover the configured HA resources to
-a healthy node if possible.
+{{#if is-success~}}
+The PVE HA manager successfully fenced '{{failed-node}}'.
+{{else~}}
+The PVE HA manager will now fence '{{failed-node}}'.
+{{/if}}
+Status: {{fence-status}}
+Timestamp: {{timestamp fence-timestamp}}
-Current fence status: {{subject-prefix}}
-{{subject}}
+Cluster Node Status:
+--------------------
+{{#each nodes~}}
+ - {{this.node}}: {{this.status}} {{#if (eq this.node ../master-node)}}[master]{{/if}}
+{{/each}}
+
+HA Resources:
+-------------
+The following HA resources were running on the failed node and will be
+recovered to a healthy node if possible:
+
+{{#each resources~}}
+{{#if (eq this.node ../failed-node)~}}
+ - {{this.resource}} [{{this.node}}]: {{this.state}}
+{{/if~}}
+{{/each}}
+The other HA resources in this cluster are:
+
+{{#each resources~}}
+{{#if (ne this.node ../failed-node)~}}
+ - {{this.resource}} [{{this.node}}]: {{this.state}}
+{{/if~}}
+{{/each~}}
-Overall Cluster status:
------------------------
-{{object status-data}}
diff --git a/src/templates/default/fencing-subject.txt.hbs b/src/templates/default/fencing-subject.txt.hbs
index 43651f9..1c140e3 100644
--- a/src/templates/default/fencing-subject.txt.hbs
+++ b/src/templates/default/fencing-subject.txt.hbs
@@ -1 +1,5 @@
-{{subject-prefix}}: {{subject}}
+{{#if is-success~}}
+Successfully fenced node '{{failed-node}}'
+{{else}}
+Trying to fence node '{{failed-node}}'
+{{/if}}
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* [pve-devel] superseded: [PATCH cluster/ha-manager/manager 0/6] preparation for #6143: notification template cleanup
2025-03-27 14:23 [pve-devel] [PATCH cluster/ha-manager/manager 0/6] preparation for #6143: notification template cleanup Lukas Wagner
` (5 preceding siblings ...)
2025-03-27 14:23 ` [pve-devel] [PATCH pve-ha-manager 1/1] notifications: overhaul fence notification Lukas Wagner
@ 2025-03-28 10:21 ` Lukas Wagner
6 siblings, 0 replies; 12+ messages in thread
From: Lukas Wagner @ 2025-03-28 10:21 UTC (permalink / raw)
To: pve-devel
v2 available!
Superseded-by: https://lore.proxmox.com/pve-devel/20250328101915.73951-1-l.wagner@proxmox.com/T/#t
--
- Lukas
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 12+ messages in thread