[pve-devel] [PATCH cluster/manager/ha-manager/proxmox{, -perl-rs} 00/18] fix #4156: introduce new notification module

Lukas Wagner l.wagner at proxmox.com
Mon Mar 27 17:18:39 CEST 2023


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 at pam's mail address),
root at 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 at 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






More information about the pve-devel mailing list