* [pve-devel] [PATCH v3 pve-guest-common 1/5] vzdump: config: add 'notification-mode' param for backup jobs
2023-11-21 12:23 [pve-devel] [PATCH v3 guest-common/manager 0/5] vzdump: add 'notification-mode' parameter Lukas Wagner
@ 2023-11-21 12:23 ` Lukas Wagner
2023-11-21 12:23 ` [pve-devel] [PATCH v3 pve-manager 2/5] vzdump: support 'notification-mode' parameter Lukas Wagner
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Lukas Wagner @ 2023-11-21 12:23 UTC (permalink / raw)
To: pve-devel
'legacy-sendmail': Use mailto/mailnotification parameters and send
emails directly.
'notification-system': Always notify via notification system
'auto': Notify via mail if mailto is set, otherwise use notification
system.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/PVE/VZDump/Common.pm | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index b93ad86..6ee8d3c 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -188,6 +188,20 @@ my $confdesc = {
enum => [ 'always', 'failure' ],
default => 'always',
},
+ 'notification-mode' => {
+ type => 'string',
+ description => "Determine which notification system to use." .
+ " If set to 'legacy-sendmail', vzdump will consider the" .
+ " mailto/mailnotification parameters and send emails to the" .
+ " specified address(es) via the 'sendmail' command." .
+ " If set to 'notification-system', a notification will be sent via PVE's" .
+ " notification system and mailto/mailnotification will be ignored" .
+ " If set to 'auto' (default setting), an email will be sent if " .
+ " mailto is set, and the notification system will be used if not.",
+ optional => 1,
+ enum => [ 'auto', 'legacy-sendmail', 'notification-system'],
+ default => 'auto',
+ },
'notification-policy' => {
type => 'string',
description => "Deprecated: Do not use",
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH v3 pve-manager 2/5] vzdump: support 'notification-mode' parameter
2023-11-21 12:23 [pve-devel] [PATCH v3 guest-common/manager 0/5] vzdump: add 'notification-mode' parameter Lukas Wagner
2023-11-21 12:23 ` [pve-devel] [PATCH v3 pve-guest-common 1/5] vzdump: config: add 'notification-mode' param for backup jobs Lukas Wagner
@ 2023-11-21 12:23 ` Lukas Wagner
2023-11-21 12:23 ` [pve-devel] [PATCH v3 pve-manager 3/5] ui: backup jobs: add 'notification-mode' selector for backup jobs Lukas Wagner
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Lukas Wagner @ 2023-11-21 12:23 UTC (permalink / raw)
To: pve-devel
This parameter lets us choose between the 'legacy' notification
system (sendmail to some email addresses) and the 'new' notification
system (pub-sub based system with targets and matchers).
'auto' (default) will use the 'legacy' system if a mail address is
provided and the 'new' system if not.
This is allows users to opt-in/opt-out from the new notification
system, which might be a bit chatty by default.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
PVE/VZDump.pm | 95 +++++++++++++++++++++++++++++++--------------------
1 file changed, 58 insertions(+), 37 deletions(-)
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index b0574d41..4185ed62 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -452,12 +452,8 @@ sub send_notification {
my $opts = $self->{opts};
my $mailto = $opts->{mailto};
my $cmdline = $self->{cmdline};
- # Old-style notification policy. This parameter will influce
- # if an ad-hoc notification target/matcher will be created.
- my $policy = $opts->{"notification-policy"} //
- $opts->{mailnotification} //
- 'always';
-
+ my $policy = $opts->{mailnotification} // 'always';
+ my $mode = $opts->{"notification-mode"} // 'auto';
sanitize_task_list($tasklist);
my $error_count = count_failed_tasks($tasklist);
@@ -499,44 +495,69 @@ sub send_notification {
};
my $fields = {
+ # TODO: There is no straight-forward way yet to get the
+ # backup job id here... (I think pvescheduler would need
+ # to pass that to the vzdump call?)
type => "vzdump",
hostname => $hostname,
};
- my $notification_config = PVE::Notify::read_config();
-
- my $legacy_sendmail = $policy eq "always" || ($policy eq "failure" && $failed);
-
- if ($mailto && scalar(@$mailto) && $legacy_sendmail) {
- # <, >, @ are not allowed in endpoint names, but that is only
- # verified once the config is serialized. That means that
- # we can rely on that fact that no other endpoint with this name exists.
- my $endpoint_name = "<mail-to-" . join(",", @$mailto) . ">";
- $notification_config->add_sendmail_endpoint(
- $endpoint_name,
- $mailto,
- undef,
- undef,
- "vzdump backup tool");
-
- my $endpoints = [$endpoint_name];
+ my $severity = $failed ? "error" : "info";
+ my $email_configured = $mailto && scalar(@$mailto);
+
+ if (($mode eq 'auto' && $email_configured) || $mode eq 'legacy-sendmail') {
+ if ($email_configured && ($policy eq "always" || ($policy eq "failure" && $failed))) {
+ # Start out with an empty config. Might still contain
+ # built-ins, so we need to disable/remove them.
+ my $notification_config = Proxmox::RS::Notify->parse_config('', '');
+
+ # Remove built-in matchers, since we only want to send an
+ # email to the specified recipients and nobody else.
+ for my $matcher (@{$notification_config->get_matchers()}) {
+ $notification_config->delete_matcher($matcher->{name});
+ }
- $notification_config->add_matcher(
- "<matcher-$endpoint_name>",
- $endpoints,
+ # <, >, @ are not allowed in endpoint names, but that is only
+ # verified once the config is serialized. That means that
+ # we can rely on that fact that no other endpoint with this name exists.
+ my $endpoint_name = "<" . join(",", @$mailto) . ">";
+ $notification_config->add_sendmail_endpoint(
+ $endpoint_name,
+ $mailto,
+ undef,
+ undef,
+ "vzdump backup tool"
+ );
+
+ my $endpoints = [$endpoint_name];
+
+ # Add a matcher that matches all notifications, set our
+ # newly created target as a target.
+ $notification_config->add_matcher(
+ "<matcher-$endpoint_name>",
+ $endpoints,
+ );
+
+ PVE::Notify::notify(
+ $severity,
+ $subject_template,
+ $body_template,
+ $notification_props,
+ $fields,
+ $notification_config
+ );
+ }
+ } else {
+ # We use the 'new' system, or we are set to 'auto' and
+ # no email addresses were configured.
+ PVE::Notify::notify(
+ $severity,
+ $subject_template,
+ $body_template,
+ $notification_props,
+ $fields,
);
}
-
- my $severity = $failed ? "error" : "info";
-
- PVE::Notify::notify(
- $severity,
- $subject_template,
- $body_template,
- $notification_props,
- $fields,
- $notification_config
- );
};
sub new {
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH v3 pve-manager 3/5] ui: backup jobs: add 'notification-mode' selector for backup jobs
2023-11-21 12:23 [pve-devel] [PATCH v3 guest-common/manager 0/5] vzdump: add 'notification-mode' parameter Lukas Wagner
2023-11-21 12:23 ` [pve-devel] [PATCH v3 pve-guest-common 1/5] vzdump: config: add 'notification-mode' param for backup jobs Lukas Wagner
2023-11-21 12:23 ` [pve-devel] [PATCH v3 pve-manager 2/5] vzdump: support 'notification-mode' parameter Lukas Wagner
@ 2023-11-21 12:23 ` Lukas Wagner
2023-11-21 12:51 ` Philipp Hufnagl
2023-11-21 12:23 ` [pve-devel] [PATCH v3 pve-manager 4/5] ui: backup: add 'notification-mode' param for one-shot " Lukas Wagner
2023-11-21 12:23 ` [pve-devel] [PATCH v3 pve-manager 5/5] ui: backup job: change field text for 'mailnotification' field Lukas Wagner
4 siblings, 1 reply; 8+ messages in thread
From: Lukas Wagner @ 2023-11-21 12:23 UTC (permalink / raw)
To: pve-devel
This selector allows one to selected between the 'old' (send email
directly via sendmail) or the 'new' notification system.
The default is 'auto', which sends and email if one is configured,
and uses the notification system if no email address is set.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
www/manager6/dc/Backup.js | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 9aae4090..8f7bab5a 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -206,12 +206,15 @@ Ext.define('PVE.dc.BackupEdit', {
viewModel: {
data: {
selMode: 'include',
+ notificationMode: '__default__',
},
formulas: {
poolMode: (get) => get('selMode') === 'pool',
disableVMSelection: (get) => get('selMode') !== 'include' && get('selMode') !== 'exclude',
- mailNotificationSelected: (get) => get('notificationMode') === 'mailto',
+ showMailtoFields: (get) => {
+ return ['auto', 'legacy-sendmail', '__default__'].includes(get('notificationMode'));
+ },
},
},
@@ -301,6 +304,28 @@ Ext.define('PVE.dc.BackupEdit', {
},
],
column2: [
+ {
+ xtype: 'proxmoxKVComboBox',
+ comboItems: [
+ [
+ '__default__',
+ Ext.String.format(
+ gettext('{0} (Auto)'), Proxmox.Utils.defaultText
+ )
+ ],
+ ['auto', gettext('Auto')],
+ ['legacy-sendmail', gettext('Email (legacy)')],
+ ['notification-system', gettext('Notification system')],
+ ],
+ fieldLabel: gettext('Notification mode'),
+ name: 'notification-mode',
+ cbind: {
+ deleteEmpty: '{!isCreate}',
+ },
+ bind: {
+ value: '{notificationMode}',
+ },
+ },
{
xtype: 'pveEmailNotificationSelector',
fieldLabel: gettext('Notify'),
@@ -309,11 +334,17 @@ Ext.define('PVE.dc.BackupEdit', {
value: (get) => get('isCreate') ? 'always' : '',
deleteEmpty: '{!isCreate}',
},
+ bind: {
+ disabled: '{!showMailtoFields}',
+ }
},
{
xtype: 'textfield',
fieldLabel: gettext('Send email to'),
name: 'mailto',
+ bind: {
+ disabled: '{!showMailtoFields}',
+ }
},
{
xtype: 'pveBackupCompressionSelector',
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [pve-devel] [PATCH v3 pve-manager 3/5] ui: backup jobs: add 'notification-mode' selector for backup jobs
2023-11-21 12:23 ` [pve-devel] [PATCH v3 pve-manager 3/5] ui: backup jobs: add 'notification-mode' selector for backup jobs Lukas Wagner
@ 2023-11-21 12:51 ` Philipp Hufnagl
0 siblings, 0 replies; 8+ messages in thread
From: Philipp Hufnagl @ 2023-11-21 12:51 UTC (permalink / raw)
To: pve-devel
On 11/21/23 13:23, Lukas Wagner wrote:
> This selector allows one to selected between the 'old' (send email
> directly via sendmail) or the 'new' notification system.
>
> The default is 'auto', which sends and email if one is configured,
> and uses the notification system if no email address is set.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> www/manager6/dc/Backup.js | 33 ++++++++++++++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
> index 9aae4090..8f7bab5a 100644
> --- a/www/manager6/dc/Backup.js
> +++ b/www/manager6/dc/Backup.js
> @@ -206,12 +206,15 @@ Ext.define('PVE.dc.BackupEdit', {
> viewModel: {
> data: {
> selMode: 'include',
> + notificationMode: '__default__',
> },
>
> formulas: {
> poolMode: (get) => get('selMode') === 'pool',
> disableVMSelection: (get) => get('selMode') !== 'include' && get('selMode') !== 'exclude',
> - mailNotificationSelected: (get) => get('notificationMode') === 'mailto',
> + showMailtoFields: (get) => {
> + return ['auto', 'legacy-sendmail', '__default__'].includes(get('notificationMode'));
> + },
WARN: line 215 col 33: arrow-body-style - Unexpected block statement
surrounding arrow body; move the returned value immediately after the
`=>`. (*)
I think it wants you to inline this like
showMailtoFields: (get) => ['auto', 'legacy-sendmail',
'__default__'].includes(get('notificationMode')),
> },
> },
>
> @@ -301,6 +304,28 @@ Ext.define('PVE.dc.BackupEdit', {
> },
> ],
> column2: [
> + {
> + xtype: 'proxmoxKVComboBox',
> + comboItems: [
> + [
> + '__default__',
> + Ext.String.format(
> + gettext('{0} (Auto)'), Proxmox.Utils.defaultText
nit: comma
> + )
nit: comma
> + ],
> + ['auto', gettext('Auto')],
> + ['legacy-sendmail', gettext('Email (legacy)')],
> + ['notification-system', gettext('Notification system')],
> + ],
> + fieldLabel: gettext('Notification mode'),
> + name: 'notification-mode',
> + cbind: {
> + deleteEmpty: '{!isCreate}',
> + },
> + bind: {
> + value: '{notificationMode}',
> + },
> + },
> {
> xtype: 'pveEmailNotificationSelector',
> fieldLabel: gettext('Notify'),
> @@ -309,11 +334,17 @@ Ext.define('PVE.dc.BackupEdit', {
> value: (get) => get('isCreate') ? 'always' : '',
> deleteEmpty: '{!isCreate}',
> },
> + bind: {
> + disabled: '{!showMailtoFields}',
> + }
nit: comma
> },
> {
> xtype: 'textfield',
> fieldLabel: gettext('Send email to'),
> name: 'mailto',
> + bind: {
> + disabled: '{!showMailtoFields}',
> + }
nit: comma
> },
> {
> xtype: 'pveBackupCompressionSelector',
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH v3 pve-manager 4/5] ui: backup: add 'notification-mode' param for one-shot backup jobs.
2023-11-21 12:23 [pve-devel] [PATCH v3 guest-common/manager 0/5] vzdump: add 'notification-mode' parameter Lukas Wagner
` (2 preceding siblings ...)
2023-11-21 12:23 ` [pve-devel] [PATCH v3 pve-manager 3/5] ui: backup jobs: add 'notification-mode' selector for backup jobs Lukas Wagner
@ 2023-11-21 12:23 ` Lukas Wagner
[not found] ` <6e27a91f-2a4b-4072-87ee-f097dd175a9e@proxmox.com>
2023-11-21 12:23 ` [pve-devel] [PATCH v3 pve-manager 5/5] ui: backup job: change field text for 'mailnotification' field Lukas Wagner
4 siblings, 1 reply; 8+ messages in thread
From: Lukas Wagner @ 2023-11-21 12:23 UTC (permalink / raw)
To: pve-devel
This selector allows one to selected between the 'old' (send email
directly via sendmail) or the 'new' notification system.
The default is 'auto', which sends and email if one is configured,
and uses the notification system if no email address is set.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
www/manager6/window/Backup.js | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/www/manager6/window/Backup.js b/www/manager6/window/Backup.js
index 8d8c9ff0..1733ae7c 100644
--- a/www/manager6/window/Backup.js
+++ b/www/manager6/window/Backup.js
@@ -36,6 +36,23 @@ Ext.define('PVE.window.Backup', {
emptyText: Proxmox.Utils.noneText,
});
+ let notificationModeSelector = Ext.create({
+ xtype: 'proxmoxKVComboBox',
+ comboItems: [
+ ['auto', gettext('Auto')],
+ ['legacy-sendmail', gettext('Email (legacy)')],
+ ['notification-system', gettext('Notification system')],
+ ],
+ fieldLabel: gettext('Notification mode'),
+ name: 'notification-mode',
+ value: 'auto',
+ listeners: {
+ change: function(field, value) {
+ mailtoField.setDisabled(value === 'notification-system');
+ }
+ }
+ });
+
const keepNames = [
['keep-last', gettext('Keep Last')],
['keep-hourly', gettext('Keep Hourly')],
@@ -110,6 +127,9 @@ Ext.define('PVE.window.Backup', {
if (!initialDefaults && data.mailto !== undefined) {
mailtoField.setValue(data.mailto);
}
+ if (!initialDefaults && data['notification-mode'] !== undefined) {
+ notificationModeSelector.setValue(data['notification-mode']);
+ }
if (!initialDefaults && data.mode !== undefined) {
modeSelector.setValue(data.mode);
}
@@ -176,6 +196,7 @@ Ext.define('PVE.window.Backup', {
],
column2: [
compressionSelector,
+ notificationModeSelector,
mailtoField,
removeCheckbox,
],
@@ -256,6 +277,10 @@ Ext.define('PVE.window.Backup', {
params.mailto = values.mailto;
}
+ if (values['notification-mode']) {
+ params['notification-mode'] = values['notification-mode'];
+ }
+
if (values.compress) {
params.compress = values.compress;
}
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [pve-devel] [PATCH v3 pve-manager 5/5] ui: backup job: change field text for 'mailnotification' field
2023-11-21 12:23 [pve-devel] [PATCH v3 guest-common/manager 0/5] vzdump: add 'notification-mode' parameter Lukas Wagner
` (3 preceding siblings ...)
2023-11-21 12:23 ` [pve-devel] [PATCH v3 pve-manager 4/5] ui: backup: add 'notification-mode' param for one-shot " Lukas Wagner
@ 2023-11-21 12:23 ` Lukas Wagner
4 siblings, 0 replies; 8+ messages in thread
From: Lukas Wagner @ 2023-11-21 12:23 UTC (permalink / raw)
To: pve-devel
... to highlight that this setting only affects the 'legacy-sendmail'
mail notifications.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Notes:
New in v3.
www/manager6/dc/Backup.js | 2 +-
www/manager6/form/NotificationPolicySelector.js | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 8f7bab5a..87cdea35 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -328,7 +328,7 @@ Ext.define('PVE.dc.BackupEdit', {
},
{
xtype: 'pveEmailNotificationSelector',
- fieldLabel: gettext('Notify'),
+ fieldLabel: gettext('Send email'),
name: 'mailnotification',
cbind: {
value: (get) => get('isCreate') ? 'always' : '',
diff --git a/www/manager6/form/NotificationPolicySelector.js b/www/manager6/form/NotificationPolicySelector.js
index f318ea18..d2a51386 100644
--- a/www/manager6/form/NotificationPolicySelector.js
+++ b/www/manager6/form/NotificationPolicySelector.js
@@ -2,7 +2,7 @@ Ext.define('PVE.form.EmailNotificationSelector', {
extend: 'Proxmox.form.KVComboBox',
alias: ['widget.pveEmailNotificationSelector'],
comboItems: [
- ['always', gettext('Notify always')],
+ ['always', gettext('Always')],
['failure', gettext('On failure only')],
],
});
--
2.39.2
^ permalink raw reply [flat|nested] 8+ messages in thread