From: Lukas Wagner <l.wagner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH cluster/manager/ha-manager/proxmox{, -perl-rs} 00/18] fix #4156: introduce new notification module
Date: Mon, 27 Mar 2023 17:18:39 +0200 [thread overview]
Message-ID: <20230327151857.495565-1-l.wagner@proxmox.com> (raw)
The purpose of this patch series is to overhaul the existing mail
notification infrastructure in Proxmox VE.
The series replaces calls to 'sendmail' with calls to a
new, configurable notification module. The module was designed to
support multiple notification endpoints, 'sendmail' using the system's
sendmail command being the first one. As a proof of the extensibility
of the current approach, the 'gotify' [1] plugin was also implemented
in this series.
The series also includes a first draft of how notification filtering
based on severity and other properties could look like (more about
properties will follow later in this cover letter).
One thing that should be avoided is breaking existing mail
notifications when these changes hit the repositories. This is a bit
tricky, since currently, we send mails to 'root' (replication, HA - is
forwarded by the mail forwarder to root@pam's mail address),
root@pam's email address (APT), as well as to freely chooseable email
addresses (vzdump). In this version of the patch series, a seamless
transition is ensured by adding an 'ephemeral' sendmail notification
endpoint right before sending a notification.
A later patch could mark individual mail recipients in vzdump jobs as
being deprecated in favor of the new notification endpoints. The
drawback of this approach is that we might send a notification twice
in case a user has created another sendmail endpoint with the same
recipient. However, this is still better than not sending a
notification at all. However, I'm open for suggestions for other
approaches for maintaining compatibility.
Alternative approaches that came to my mind are:
- Explicitly break existing mail notifications with a major
release, maybe create a default sendmail endpoint where *all*
notifications are sent to root@pam via a sendmail endpoint.
In this variant, the custom mail recipients for vzdump jobs would
be removed
- On update, create endpoints in the new configuration file for all
possible email addresses, along with appropriate filters so that
the behavior for every component currently sending mails remains
unchanged. I don't really like this approach, as it might lead to
a pretty messy 'notifications.cfg' with lots of endpoints/filters, if
the user has lots of different backup jobs configured.
Side effects of the changes:
- vzdump loses the HTML table which lists VMs/CTs. Since different
endpoints support different markup styles for formatting, it
does not really make sense to support this in the notification API, at
least not without 'cross-rendering' (e.g. markdown --> HTML)
Short summary of the introduced changes per repo:
- proxmox:
Add new proxmox-notification crate, including configuration file
parsing, two endpoints (sendmail, gotify) and per-endpoint
filtering
- proxmox-perl-rs:
Add bindings for proxmox-notification, so that we can use it from
perl. Also configure logging in such a way that log messages from
proxmox-notification integrate nicely in task logs.
- proxmox-cluster:
Register new configuration file, 'notifications.cfg'
- pve-manager:
Add some more glue code, so that the notification functions are
callable in a nice way. This is needed since we need to read the
configuration file and feed the config to the rust bindings as a
string.
Replace calls to 'sendmail' in vzdump,
apt, and replication with calls to the new notification module.
- pve-ha-manager:
Also replace calls to 'sendmail' with the new notification module
Follow-up work (in no particular order):
- Add CRUD API routes for managing notification endpoints and
filters - right now, this can only be done by editing
'notifications.cfg'
- Add a GUI and CLI using this API
- Right now, the notification API is very simple. Sending a
notification can be as simple as
PVE::Notification::info("Title", "Body")
In the future, the API might be changed/extended so that supports
"registering" notifications. This allows us to a.) generate a
list of all possible notification sources in the system b.) allows
users to easily create filters for specific notification events.
In my head, using the notification module could look like this
then:
# Global context
my = PVE::Notification::register({
'id' => 'backup-failed',
'severity' => 'error',
'properties' => ['host', 'vmlist', 'logs'],
'title' => '{{ host }}: Backup failed'
'body' => <<'EOF'
A backup has failed for the following VMs: {{ vmlist }}
{{ logs }}
EOF
});
# Later, to send the notification:
PVE::Notification::send(->instantiate({
'host' => 'earth',
'vmlist' => ... ,
'logs' => ... ,
}));
Title and body effectively become templates that can be
instantiated with arbitrary properties. This has the following
benefits:
- This ensures that notifications could be easily translated in
the future
- Allows us to add functionality that allows users to customize
notification text, as wished in [2].
- Properties can be used in filters (e.g. exact match, regex,
etc.)
- Properties can be listed in the list of notifications
mentioned above, making it easier to create filters.
- proxmox-mail-forward could be integrated as well. This would feed
e.g. zfs-zed events into our notification infrastructure. Special
care must be taken to not create recursive notification loops
(e.g. zed sends to root, forwarder uses notification module, a
configured sendmail endpoint sends to root, forwarder uses module
--> loop)
- Maybe add some CLI so that admins can send notifications in
scripts (an API endpoint callable via pvesh might be enough for a
start)
- Add more notification events
- Add other endpoints, e.g. webhook, a generic SMTP, etc.
- Integrate the new module into the other products
[1] https://gotify.net/
[2] https://bugzilla.proxmox.com/show_bug.cgi?id=4526
proxmox:
Lukas Wagner (6):
add proxmox-notification crate
notification: implement sendmail endpoint
notification: add notification filter mechanism
notification: implement gotify endpoint
notification: allow adding new sendmail endpoints / filters
notification: add debian packaging
Cargo.toml | 2 +
proxmox-notification/Cargo.toml | 22 +
proxmox-notification/debian/changelog | 5 +
proxmox-notification/debian/control | 59 +++
proxmox-notification/debian/copyright | 16 +
proxmox-notification/debian/debcargo.toml | 7 +
proxmox-notification/src/config.rs | 71 +++
proxmox-notification/src/endpoints/gotify.rs | 80 ++++
proxmox-notification/src/endpoints/mod.rs | 2 +
.../src/endpoints/sendmail.rs | 78 ++++
proxmox-notification/src/filter.rs | 427 ++++++++++++++++++
proxmox-notification/src/lib.rs | 307 +++++++++++++
proxmox-notification/src/methods.rs | 55 +++
13 files changed, 1131 insertions(+)
create mode 100644 proxmox-notification/Cargo.toml
create mode 100644 proxmox-notification/debian/changelog
create mode 100644 proxmox-notification/debian/control
create mode 100644 proxmox-notification/debian/copyright
create mode 100644 proxmox-notification/debian/debcargo.toml
create mode 100644 proxmox-notification/src/config.rs
create mode 100644 proxmox-notification/src/endpoints/gotify.rs
create mode 100644 proxmox-notification/src/endpoints/mod.rs
create mode 100644 proxmox-notification/src/endpoints/sendmail.rs
create mode 100644 proxmox-notification/src/filter.rs
create mode 100644 proxmox-notification/src/lib.rs
create mode 100644 proxmox-notification/src/methods.rs
proxmox-perl-rs:
Lukas Wagner (2):
log: set default log level to 'info', add product specfig logging env
var1
add basic bindings for the proxmox_notification crate
Cargo.toml | 2 +
common/pkg/Makefile | 1 +
common/src/logger.rs | 12 +++-
pmg-rs/src/lib.rs | 2 +-
pve-rs/Cargo.toml | 1 +
pve-rs/src/lib.rs | 3 +-
pve-rs/src/notification.rs | 128 +++++++++++++++++++++++++++++++++++++
7 files changed, 145 insertions(+), 4 deletions(-)
create mode 100644 pve-rs/src/notification.rs
pve-cluster:
Lukas Wagner (1):
cluster files: add notifications.cfg
data/PVE/Cluster.pm | 1 +
data/src/status.c | 1 +
2 files changed, 2 insertions(+)
pve-manager:
Lukas Wagner (7):
test: fix names of .PHONY targets
add PVE::Notification module
vzdump: send notifications via new notification module
vzdump: rename 'sendmail' sub to 'send_notification'
test: rename mail_test.pl to vzdump_notification_test.pl
api: apt: send notification via new notification module
api: replication: send notifications via new notification module
PVE/API2/APT.pm | 55 +++++---
PVE/API2/Replication.pm | 13 +-
PVE/API2/VZDump.pm | 2 +-
PVE/Makefile | 1 +
PVE/Notification.pm | 60 +++++++++
PVE/VZDump.pm | 124 +++++-------------
test/Makefile | 16 ++-
...il_test.pl => vzdump_notification_test.pl} | 28 ++--
8 files changed, 169 insertions(+), 130 deletions(-)
create mode 100644 PVE/Notification.pm
rename test/{mail_test.pl => vzdump_notification_test.pl} (73%)
pve-ha-manager:
Lukas Wagner (2):
manager: send notifications via new notification module
manager: rename 'sendmail' --> 'send_notification'
src/PVE/HA/Env.pm | 4 ++--
src/PVE/HA/Env/PVE2.pm | 15 +++++++++++++--
src/PVE/HA/NodeStatus.pm | 2 +-
src/PVE/HA/Sim/Env.pm | 2 +-
4 files changed, 17 insertions(+), 6 deletions(-)
Summary over all repositories:
34 files changed, 1464 insertions(+), 140 deletions(-)
Generated by murpp v0.1.0
--
2.30.2
next 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 Lukas Wagner [this message]
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 ` [pve-devel] [PATCH pve-manager 12/18] vzdump: send notifications via new notification module Lukas Wagner
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-1-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