Call job_{init,cleanup}() and backup_{init,cleanup}() methods so that
backup providers can prepare and clean up for the whole backup job and
for individual guest backups.
It is necessary to adapt some log messages and special case some
things like is already done for PBS, e.g. log file handling.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Reviewed-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Link: https://lore.proxmox.com/20250404133204.239783-30-f.ebner@proxmox.com
Instead of relying on the 'magic' table helper, we generate the guest
list in the vzdump HTML notification template using native Handlebars
syntax ({{#each}}). The template becomes a bit more ugly, mostly due to
HTML email's requirements to use inline CSS styling, but in the context
of providing user-customizable notification templates this is much
nicer. It gives the user a starting point for their modificiations
(custom styling, changing table formatting/order/etc.), which was not
possible before.
The plaintext template stays as is for now, mostly due to the fact that
the table helper automatically aligns table columns - something that
is not easily possible with native Handlebars syntax.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
jobs converted from vzdump.cron have an ID of the format
$digest:$counter
where $digest is the hash of the vzdump.cron file, and $counter is the
position of the job within the crontab.
while the section config schema pretends jobs.cfg's section IDs are
of type pve-configid, that is not enforced anywhere, and the API
endpoints managing such jobs allowed arbitrary strings in the past.
the ':' character is not allowed by `pve-configid`, but it is by the
section config parsers and the Job API.
convert the API schema to use the unification of previous definition
used by the job API, and what the section config parser accepts.
Fixes: f5a97f1f5 (api: jobs: vzdump: pass job 'job-id' parameter)
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
and put it into PVE::VZDump because there is a cycle between
PVE::Jobs::VZDump, PVE::API2::VZDump and PVE::API2::Backups
that prevents any of those containing it for now.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
This allows us to access the backup job id in the send_notification
function, where we can set it as metadata for the notification.
The 'job-id' parameter can only be used by 'root@pam' to prevent
abuse. This has the side effect that manually triggered backup jobs
cannot have the 'job-id' parameter at the moment. To mitigate that,
manually triggered backup jobs could be changed so that they
are not performed by a direct API call by the UI, but by requesting
pvescheduler to execute the job in the near future (similar to how
manually triggered replication jobs work).
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
[ TL: fleece in d/control bump for guest-common now that the version
is known ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
- The man page warns about the usage of `hostname -f`, since a host
may have multiple domains (or none at all)
- The fallback PVE::INotify::nodename() already only returned the
hostname without the domain part
- Fencing notifications didn't include the domain part anyway
This may result in soft-breakage for any users who have already relied
on the domain being present. If there is need for it, it could include
a fqdn metadata field.
The hostname property used for rendering the notification template
is unaffected for now.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
The documentation 'man vzdump' states that the value is in KiB/s. This
is correct, as seen in the plugin implementations, where the value is
multiplied by 1024.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
This commit adapts notification sending for
- package update
- replication
- backups
to use named templates (installed in /usr/share/pve-manager/templates)
instead of passing template strings defined in code to the
notification stack.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Max Carrara <m.carrara@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
Similar to how Datastore.AllocateSpace is required for the backup
storage, it should also be required for the fleecing storage.
Removing a fleecing storage from a job does not require more
permissions than for modifying the job.
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Previously, the result would only be returned implicitly and if not
already parsed. While callers do not strictly need the return value,
future callers might mistakenly rely on it and even work by chance in
some scenarios, because of the implicit return. Make the code more
future proof by explicitly returning the result in all cases.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Currently, fallback for the 'performance' option is done as a whole,
taking away flexibility from the user. It also means that when only
one of the two sub-properties is specified, the other one will default
to the backend (i.e. QEMU or proxmox-backup-client) default rather
than the schema default. For the latter point in particular, it can be
argued to be incorrect. These limitations will only get worse in the
future with more sub-properties.
Switch to a per-property fallback mechanism to improve the situation,
having each go through the usual preference order (CLI/job > node-wide
default > schema default).
Technically, this is a breaking change, but pbs-entries-max is rather
new and potential for breakage seems rather low. Requirements for
breakage:
* job (or CLI) that defines only one of the performance options
* job also covers a guest where the other performance option applies
* the other performance option is defined in the node-wide configuration
* the node-wide setting is worse for the job than the implicit backend
default (because this change will have the node-wide default win over
the implicit backend default).
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
The 'performance' option itself defines no 'default' in the schema, so
what happened is that the defaults used by the backends (i.e. QEMU and
proxmox-backup-client) would be used. Luckily, they correspond to the
default values defined in the schema, i.e. in the 'backup-performance'
format. Make the code future-proof and use the actual defaults defined
in the schema instead of relying on that correspondence.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
The helpers were split out from the original 'sendmail' function when
migrating to the new notification system. They are not needed anywhere
else and can thus be private.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
The old backup job notification mails from before the notification
system overhaul included the total time as well as the total size.
The total size was missing from the new, template-based backup report,
thus we add it back in this commit.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
[FE: style fix - use parentheses for returning list]
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
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>
To ease the migration from old-style mailto/mailnotification paramters
for backup jobs, the code will add a ephemeral sendmail endpoint and
a matcher.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
In some situations, such as backing up VMs with 2 or more disks to
PBS, we get passed the backup archive size as a string instead of
as an integer. This led to errors rendering the notification template
down the line.
This commit explicitly casts the data from the task table to an int.
It would be a good idea to actually hunt down the places that produced
the string instead of an integer, but as a quick fix and as a
safeguard against similar lurking errors this approach is fine, IMO.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Virtual (or anonymous) endpoints/groups are used for sending
one-off notifications to a target that does not exist in the
config.
VZDump uses this to send out notification mails to those addresses
configured by the `mailto` parameter.
Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
... instead of using sendmail directly.
If the new 'notification-target' parameter is set,
we send the notification to this endpoint or group.
If 'mailto' is set, we add a temporary endpoint and a
temporary group containg both targets.
This commit also refactors the old 'sendmail' sub heavily:
- Use template-based notification text instead of endless
string concatenations
- Removing the old plaintext/HTML table rendering in favor of
the new template/property-based approach offered by the
`proxmox-notify` crate.
- Rename `sendmail` sub to `send_notification`
- Breaking out some of the code into helper subs, hopefully
reducing the spaghetti factor a bit
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
This reverts 7420d7ff ("zstd: add --rsyncable flag")
That flag causes severe slow downs on fast disks, and we still have
other rsyncable compressors available.
It was originally added based on wrong documentation that made the
performance impact look a lot smaller than it actually is.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
we want to move the 'exclude-path' to an array format (from 'string-alist')
prepare the code that it can be either a string or a list
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Since all tasks succeeded, previously no mail was sent in that case.
Note that the error passed to $self->sendmail() is added to the
subject of the mail if it is a single line or the beginning of the
mail otherwise. Thus changing the mail slightly compared to previously
for the case where the job-start hook fails and the case where the
job-end hook fails with mailnotification=always. But can be considered
an improvement, because the user sees the error right away.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Also generalizes the way vzdump property strings are handled for jobs.
Something similar could be done in VZDump.pm, but there the maxfiles
and prune-backups settings are currently coupled, so a dedicated
parse_performance() is used instead. Can be changed once maxfiles is
dropped.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
to avoid warnings like
parse error in '/etc/vzdump.conf' - 'storage': missing property -
'notes-template' requires this property
when there is no default for the required property configured.
In new(), the defaults are mixed in with the regular CLI/API
parameters, so re-check if the required property is set. If it's not,
the defaults do not apply to the current run, and can be dropped.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
For VMs, $task->{hostname} might be undef and when running on a
stand-alone node, there is no cluster name.
Reported-by: Marco Gabriel <mgabriel@inett.de>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
instead of just checking for a newline, do a full check already.
Also do the check at the beginning of generate_notes() for consistency
and remove the check after expansion to avoid failing late for things
like '{{cl{{node}}er}}' (which can even expand to a valid variable
making the error even more confusing).
Co-developed-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
so that '{{foo}}{{bar}}' is not detected as being an unknown variable
named 'foo}}{{bar', but as 'foo' (and 'bar').
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
which is caused by the quoting operators \Q...\E. The actual intention
was to avoid such surprises.
Fixes: e01438a7 ("partially close#438: vzdump: support setting notes-template")
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Previously, if the '--script' argument was passed with a non-existent
file, it would state that a non-executable script was the reason for
failure. This adds a check to see if the hook script exists, in order
to provide a more accurate error message.
Also adds an 'Error:' prefix the 'script not executable' error.
Signed-off-by: Dylan Whyte <d.whyte@proxmox.com>
While vzdump itself wouldn't mind about unescaped newlines, the
parameter isn't supposed to contain any, and when used as part of the
job config, it has to be a single line too, so make it consistent.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Check the number of protected backups early if the protected flag
is set.
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
else this single call site is subtly different from all the rest, which
could cause problems further down the line if we ever change the prune
logic.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
since they are the ones relevant for pruning and protected backups
have their own separate limit.
Since get_backup_file_list is only used in places where the
unprotected backups are needed, adapt the helper accordingly.
If there is a storage, use PVE::Storage::volume_list to count the
unprotected backups. This avoids a direct invocation of the
proxmox-backup-client for PBS and the limit check can also work for
external storage plugins which might not be dir-based or name the
backups differently.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
which allows users to prepare the backup storage for activation (e.g.
by waking up a remote node).
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Acked-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Keeps the behavior consistent with what happens for storages. It also
is required to not get into conflict with the check in archive_remove,
i.e. pruning here marks a backup as 'remove' and then archive_remove
complains that it's protected.
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
which ended up in the backup task log and can be confusing to users:
> INFO: Backup finished at 2021-10-11 08:40:38
> Result: {
> "data": null
> }
> INFO: Backup job finished successfully
For proxmox-backup-server < 2.0.11-1, there was no output for the
upload-log command.
Reported in the community forum:
https://forum.proxmox.com/threads/backup-finishes-ok-but-data-null-info-on-finish.97740/
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
Not doing so leads to an off-by-one error, and pruning in a dumpdir
really should behave the same way as pruning in a storage.
Reported in the community forum:
https://forum.proxmox.com/threads/problem-mit-prune-backups.95737/
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
remove type check for the specific plugins, instead we can deduce it
from the supported content type in config (this can only be set on
storages that declare to support backups). should also work with
external storage plugins.
Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
