[pve-devel] [PATCH pve-manager 12/18] vzdump: send notifications via new notification module
Lukas Wagner
l.wagner at proxmox.com
Mon Mar 27 17:18:51 CEST 2023
... instead of using sendmail directly.
If the 'mailto' property is set set for a backup, an ephemeral
notification endpoint of type 'sendmail' will be added to the
notification backend. Also, a fitting notification filter will be added,
with the 'min-severity' property set to either 'info' or 'error',
depending on whether 'notify always' or 'on failure only' is set for a
backup job. This approach allows us to add the new notification backend
without breaking any existing mail notifications.
The notification backend only supports plain text for message bodies,
thus the HTML table generation has been removed.
Signed-off-by: Lukas Wagner <l.wagner at proxmox.com>
---
PVE/VZDump.pm | 116 +++++++++++++---------------------------------
test/mail_test.pl | 22 +++++----
2 files changed, 44 insertions(+), 94 deletions(-)
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index a04837e7..c0971220 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -19,6 +19,7 @@ use PVE::Exception qw(raise_param_exc);
use PVE::HA::Config;
use PVE::HA::Env::PVE2;
use PVE::JSONSchema qw(get_standard_option);
+use PVE::Notification;
use PVE::RPCEnvironment;
use PVE::Storage;
use PVE::VZDump::Common;
@@ -308,7 +309,7 @@ sub sendmail {
my $mailto = $opts->{mailto};
- return if !($mailto && scalar(@$mailto));
+ # return if !($mailto && scalar(@$mailto));
my $cmdline = $self->{cmdline};
@@ -326,10 +327,10 @@ sub sendmail {
}
}
- my $notify = $opts->{mailnotification} || 'always';
- return if (!$ecount && !$err && ($notify eq 'failure'));
+ my $failed = ($ecount || $err);
+
+ my $stat = $failed ? 'backup failed' : 'backup successful';
- my $stat = ($ecount || $err) ? 'backup failed' : 'backup successful';
if ($err) {
if ($err =~ /\n/) {
$stat .= ": multiple problems";
@@ -391,90 +392,13 @@ sub sendmail {
}
$text_log_part .= $detail_post if defined($detail_post);
- # html part
- my $html = "<html><body>\n";
- $html .= "<p>" . (escape_html($err) =~ s/\n/<br>/gr) . "</p>\n" if $err;
- $html .= "<table border=1 cellpadding=3>\n";
- $html .= "<tr><td>VMID<td>NAME<td>STATUS<td>TIME<td>SIZE<td>FILENAME</tr>\n";
-
- my $ssize = 0;
- foreach my $task (@$tasklist) {
- my $vmid = $task->{vmid};
- my $name = $task->{hostname};
-
- if ($task->{state} eq 'ok') {
- $ssize += $task->{size};
-
- $html .= sprintf (
- "<tr><td>%s<td>%s<td>OK<td>%s<td align=right>%s<td>%s</tr>\n",
- $vmid,
- $name,
- format_time($task->{backuptime}),
- format_size ($task->{size}),
- escape_html ($task->{target}),
- );
- } else {
- $html .= sprintf (
- "<tr><td>%s<td>%s<td><font color=red>FAILED<td>%s<td colspan=2>%s</tr>\n",
- $vmid,
- $name,
- format_time($task->{backuptime}),
- escape_html ($task->{msg}),
- );
- }
- }
-
- $html .= sprintf ("<tr><td align=left colspan=3>TOTAL<td>%s<td>%s<td></tr>",
- format_time ($totaltime), format_size ($ssize));
-
- $html .= "\n</table><br><br>\n";
- my $html_log_part;
- $html_log_part .= "Detailed backup logs:<br /><br />\n";
- $html_log_part .= "<pre>\n";
- $html_log_part .= escape_html($cmdline) . "\n\n";
-
- $html_log_part .= escape_html($detail_pre) . "\n" if defined($detail_pre);
- foreach my $task (@$tasklist) {
- my $vmid = $task->{vmid};
- my $log = $task->{tmplog};
- if (!$log) {
- $html_log_part .= "$vmid: no log available\n\n";
- next;
- }
- if (open (my $TMP, '<', "$log")) {
- while (my $line = <$TMP>) {
- next if $line =~ /^status: \d+/; # not useful in mails
- if ($line =~ m/^\S+\s\d+\s+\d+:\d+:\d+\s+(ERROR|WARN):/) {
- $html_log_part .= encode8bit ("$vmid: <font color=red>".
- escape_html ($line) . "</font>");
- } else {
- $html_log_part .= encode8bit ("$vmid: " . escape_html ($line));
- }
- }
- close ($TMP);
- } else {
- $html_log_part .= "$vmid: Could not open log file\n\n";
- }
- $html_log_part .= "\n";
- }
- $html_log_part .= escape_html($detail_post) if defined($detail_post);
- $html_log_part .= "</pre>";
- my $html_end = "\n</body></html>\n";
- # end html part
-
- if (length($text) + length($text_log_part) +
- length($html) + length($html_log_part) +
- length($html_end) < MAX_MAIL_SIZE)
+ if (length($text) + length($text_log_part) < MAX_MAIL_SIZE)
{
- $html .= $html_log_part;
- $html .= $html_end;
$text .= $text_log_part;
} else {
- my $msg = "Log output was too long to be sent by mail. ".
+ my $msg = "Log output was too long to be sent. ".
"See Task History for details!\n";
$text .= $msg;
- $html .= "<p>$msg</p>";
- $html .= $html_end;
}
my $subject = "vzdump backup status ($hostname) : $stat";
@@ -482,7 +406,31 @@ sub sendmail {
my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
my $mailfrom = $dcconf->{email_from} || "root";
- PVE::Tools::sendmail($mailto, $subject, $text, $html, $mailfrom, "vzdump backup tool");
+
+ my $notify = $opts->{mailnotification} || 'always';
+ my $min_severity = $notify eq "failure" ? "error" : "info";
+
+ my $config = PVE::Notification::config();
+
+ if ($mailto && scalar(@$mailto)) {
+ # add ephemeral sendmail endpoint for the existing vzdump-job,
+ # in order to not break existing mail notifications. The changed
+ # config is not persisted.
+ # TODO: Remove in PVE x.y ?
+ $config->add_filter("legacy-vzdump-sendmail-filter", $min_severity);
+ $config->add_sendmail_endpoint(
+ "legacy-vzdump-sendmail",
+ $mailto,
+ $mailfrom,
+ "vzdump backup tool",
+ "legacy-vzdump-sendmail-filter");
+ }
+
+ if ($failed) {
+ PVE::Notification::error($subject, $text, $config);
+ } else {
+ PVE::Notification::info($subject, $text, $config);
+ }
};
sub new {
diff --git a/test/mail_test.pl b/test/mail_test.pl
index d0114441..c77c1aae 100755
--- a/test/mail_test.pl
+++ b/test/mail_test.pl
@@ -5,7 +5,7 @@ use warnings;
use lib '..';
-use Test::More tests => 5;
+use Test::More tests => 3;
use Test::MockModule;
use PVE::VZDump;
@@ -29,18 +29,22 @@ sub prepare_mail_with_status {
sub prepare_long_mail {
open(TEST_FILE, '>', $TEST_FILE_PATH); # Removes previous content
# 0.5 MB * 2 parts + the overview tables gives more than 1 MB mail
- print TEST_FILE "a" x (1024*1024/2);
+ print TEST_FILE "a" x (1024*1024);
close(TEST_FILE);
}
-my ($result_text, $result_html);
+my $result_text;
-my $mock_tools_module = Test::MockModule->new('PVE::Tools');
-$mock_tools_module->mock('sendmail', sub {
- my (undef, undef, $text, $html, undef, undef) = @_;
+my $mock_notification_module = Test::MockModule->new('PVE::Notification');
+$mock_notification_module->mock('send_notification', sub {
+ my (undef, undef, $text, undef, undef) = @_;
$result_text = $text;
- $result_html = $html;
});
+$mock_notification_module->mock('config', sub {
+ # Return empty config
+ return Proxmox::RS::Notification->new("");
+});
+
my $mock_cluster_module = Test::MockModule->new('PVE::Cluster');
$mock_cluster_module->mock('cfs_read_file', sub {
@@ -62,7 +66,7 @@ my $SELF = {
my $task = { state => 'ok', vmid => '100', };
my $tasklist;
sub prepare_test {
- $result_text = $result_html = undef;
+ $result_text = undef;
$task->{tmplog} = shift;
$tasklist = [ $task ];
}
@@ -77,13 +81,11 @@ sub prepare_test {
prepare_mail_with_status();
PVE::VZDump::sendmail($SELF, $tasklist, 0, undef, undef, undef);
unlike($result_text, $STATUS, "Status are not in text part of mails");
- unlike($result_html, $STATUS, "Status are not in HTML part of mails");
}
{
prepare_test($TEST_FILE_PATH);
prepare_long_mail();
PVE::VZDump::sendmail($SELF, $tasklist, 0, undef, undef, undef);
like($result_text, $LOG_TOO_LONG, "Text part of mails gets shortened");
- like($result_html, $LOG_TOO_LONG, "HTML part of mails gets shortened");
}
unlink $TEST_FILE_PATH;
--
2.30.2
More information about the pve-devel
mailing list