From: Lukas Wagner <l.wagner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH pve-manager 12/18] vzdump: send notifications via new notification module
Date: Mon, 27 Mar 2023 17:18:51 +0200 [thread overview]
Message-ID: <20230327151857.495565-13-l.wagner@proxmox.com> (raw)
In-Reply-To: <20230327151857.495565-1-l.wagner@proxmox.com>
... 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@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
next prev parent reply other threads:[~2023-03-27 15:19 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-27 15:18 [pve-devel] [PATCH cluster/manager/ha-manager/proxmox{, -perl-rs} 00/18] fix #4156: introduce " Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 01/18] add proxmox-notification crate Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 02/18] notification: implement sendmail endpoint Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 03/18] notification: add notification filter mechanism Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 04/18] notification: implement gotify endpoint Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 05/18] notification: allow adding new sendmail endpoints / filters Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 06/18] notification: add debian packaging Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox-perl-rs 07/18] log: set default log level to 'info', add product specfig logging env var1 Lukas Wagner
2023-03-31 9:17 ` Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox-perl-rs 08/18] add basic bindings for the proxmox_notification crate Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-cluster 09/18] cluster files: add notifications.cfg Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 10/18] test: fix names of .PHONY targets Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 11/18] add PVE::Notification module Lukas Wagner
2023-03-27 15:18 ` Lukas Wagner [this message]
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 13/18] vzdump: rename 'sendmail' sub to 'send_notification' Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 14/18] test: rename mail_test.pl to vzdump_notification_test.pl Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 15/18] api: apt: send notification via new notification module Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 16/18] api: replication: send notifications " Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-ha-manager 17/18] manager: " Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-ha-manager 18/18] manager: rename 'sendmail' --> 'send_notification' Lukas Wagner
2023-04-14 6:19 ` [pve-devel] [PATCH cluster/manager/ha-manager/proxmox{, -perl-rs} 00/18] fix #4156: introduce new notification module Thomas Lamprecht
2023-04-14 9:47 ` Lukas Wagner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230327151857.495565-13-l.wagner@proxmox.com \
--to=l.wagner@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox