Am 07.07.20 um 08:46 schrieb Fabian Grünbichler:
s/VM/guest in most descriptions - this is not in qemu-server ;)
On June 4, 2020 11:08 am, Fabian Ebner wrote:
Signed-off-by: Fabian Ebner <[email protected]>
---
Not sure if this is the best place for the new API endpoints.
I decided to opt for two distinct calls rather than just using a
--dry-run option and use a worker for actually pruning, because
removing many backups over the network might take a while.
see comments below
PVE/API2/Storage/Makefile | 2 +-
PVE/API2/Storage/PruneBackups.pm | 153 +++++++++++++++++++++++++++++++
PVE/API2/Storage/Status.pm | 7 ++
PVE/CLI/pvesm.pm | 27 ++++++
4 files changed, 188 insertions(+), 1 deletion(-)
create mode 100644 PVE/API2/Storage/PruneBackups.pm
diff --git a/PVE/API2/Storage/Makefile b/PVE/API2/Storage/Makefile
index a33525b..3f667e8 100644
--- a/PVE/API2/Storage/Makefile
+++ b/PVE/API2/Storage/Makefile
@@ -1,5 +1,5 @@
-SOURCES= Content.pm Status.pm Config.pm
+SOURCES= Content.pm Status.pm Config.pm PruneBackups.pm
.PHONY: install
install:
diff --git a/PVE/API2/Storage/PruneBackups.pm b/PVE/API2/Storage/PruneBackups.pm
new file mode 100644
index 0000000..7968730
--- /dev/null
+++ b/PVE/API2/Storage/PruneBackups.pm
@@ -0,0 +1,153 @@
+package PVE::API2::Storage::PruneBackups;
+
+use strict;
+use warnings;
+
+use PVE::Cluster;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::RESTHandler;
+use PVE::RPCEnvironment;
+use PVE::Storage;
+use PVE::Tools qw(extract_param);
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+ name => 'index',
IMHO this is not an index, but just a regular 'GET' API path.
Ok
+ path => '',
+ method => 'GET',
+ description => "Get prune information for backups.",
+ permissions => {
+ check => ['perm', '/storage/{storage}', ['Datastore.Audit',
'Datastore.AllocateSpace'], any => 1],
+ },
+ protected => 1,
+ proxyto => 'node',
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ node => get_standard_option('pve-node'),
+ storage => get_standard_option('pve-storage-id', {
+ completion => \&PVE::Storage::complete_storage_enabled,
+ }),
+ 'prune-backups' => get_standard_option('prune-backups', {
+ description => "Use these retention options instead of those from
the storage configuration.",
+ optional => 1,
+ }),
+ vmid => get_standard_option('pve-vmid', {
+ description => "Only consider backups for this VM.",
of this guest
would it make sense to allow specification of guest-type as well, so
that it's possible to indirectly specify the 'backup-group' ?
We don't really support having backups of two kinds of guest with the
same ID currently, do we? If this is really needed at some point, we can
still extend this, but for now it should be enough if the such backups
get grouped correctly as to not interfere with one another.
+ optional => 1,
+ completion => \&PVE::Cluster::complete_vmid,
+ }),
+ },
+ },
+ returns => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ volid => {
+ description => "Backup volume ID.",
+ type => 'string',
+ },
+ type => {
+ description => "One of 'qemu', 'lxc', 'openvz' or
'unknown'.",
+ type => 'string',
+ },
+ 'ctime' => {
+ description => "Creation time of the backup (seconds since the
UNIX epoch).",
+ type => 'integer',
+ },
+ 'mark' => {
+ description => "Whether the backup would be kept or removed. For
backups that don't " .
+ "use the standard naming scheme, it's
'protected'.",
+ type => 'string',
+ },
+ 'vmid' => {
+ description => "The VM the backup belongs to.",
+ type => 'integer',
+ optional => 1,
+ },
+ },
+ },
+ },
+ code => sub {
+ my ($param) = @_;
+
+ my $cfg = PVE::Storage::config();
+
+ my $vmid = extract_param($param, 'vmid');
+ my $storeid = extract_param($param, 'storage');
+ my $prune_options = extract_param($param, 'prune-backups');
+
+ my $opts_override;
+ if (defined($prune_options)) {
+ $opts_override =
PVE::JSONSchema::parse_property_string('prune-backups', $prune_options);
+ }
+
+ my @res = PVE::Storage::prune_backups($cfg, $storeid, $opts_override,
$vmid, 1);
+ return \@res;
one more reason to make the return value a reference ;)
+ }});
+
+__PACKAGE__->register_method ({
+ name => 'delete',
+ path => '',
+ method => 'DELETE',
+ description => "Prune backups. Only those using the standard naming scheme are
considered.",
+ permissions => {
+ description => "You need the 'Datastore.Allocate' privilege on the storage
" .
+ "(or if a VM ID is specified, 'Datastore.AllocateSpace' and
'VM.Backup' for the VM).",
+ user => 'all',
+ },
+ protected => 1,
+ proxyto => 'node',
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ node => get_standard_option('pve-node'),
+ storage => get_standard_option('pve-storage-id', {
+ completion => \&PVE::Storage::complete_storage,
+ }),
+ 'prune-backups' => get_standard_option('prune-backups', {
+ description => "Use these retention options instead of those from
the storage configuration.",
+ }),
+ vmid => get_standard_option('pve-vmid', {
+ description => "Only prune backups for this VM.",
+ completion => \&PVE::Cluster::complete_vmid,
+ optional => 1,
+ }),
+ },
+ },
+ returns => { type => 'string' },
+ code => sub {
+ my ($param) = @_;
+
+ my $rpcenv = PVE::RPCEnvironment::get();
+ my $authuser = $rpcenv->get_user();
+
+ my $cfg = PVE::Storage::config();
+
+ my $vmid = extract_param($param, 'vmid');
+ my $storeid = extract_param($param, 'storage');
+ my $prune_options = extract_param($param, 'prune-backups');
+
+ my $opts_override;
+ if (defined($prune_options)) {
+ $opts_override =
PVE::JSONSchema::parse_property_string('prune-backups', $prune_options);
+ }
+
+ if (defined($vmid)) {
+ $rpcenv->check($authuser, "/storage/$storeid",
['Datastore.AllocateSpace']);
+ $rpcenv->check($authuser, "/vms/$vmid", ['VM.Backup']);
+ } else {
+ $rpcenv->check($authuser, "/storage/$storeid",
['Datastore.Allocate']);
+ }
+
+ my $id = (defined($vmid) ? "$vmid@" : '') . $storeid;
+ my $worker = sub {
+ PVE::Storage::prune_backups($cfg, $storeid, $opts_override, $vmid,
0);
it would be nice to print progress in $PLUGIN::prune_backups (e.g. when
iterating over the entries before deletion, otherwise neither this API
call nor the CLI wrapper give any indication which backup archives got
pruned?
For Plugin.pm one can print something while iterating, but for
PBSPlugin.pm that's not really possible is it? So maybe just output
something like "executing proxmox-backup-client prune ..." there?
+ };
+
+ return $rpcenv->fork_worker('prunebackups', $id, $authuser, $worker);
+ }});
+
+1;
diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
index d9d9b36..d12643f 100644
--- a/PVE/API2/Storage/Status.pm
+++ b/PVE/API2/Storage/Status.pm
@@ -11,6 +11,7 @@ use PVE::Cluster;
use PVE::RRD;
use PVE::Storage;
use PVE::API2::Storage::Content;
+use PVE::API2::Storage::PruneBackups;
use PVE::RESTHandler;
use PVE::RPCEnvironment;
use PVE::JSONSchema qw(get_standard_option);
@@ -18,6 +19,11 @@ use PVE::Exception qw(raise_param_exc);
use base qw(PVE::RESTHandler);
+__PACKAGE__->register_method ({
+ subclass => "PVE::API2::Storage::PruneBackups",
+ path => '{storage}/prunebackups',
+});
+
__PACKAGE__->register_method ({
subclass => "PVE::API2::Storage::Content",
# set fragment delimiter (no subdirs) - we need that, because volume
@@ -214,6 +220,7 @@ __PACKAGE__->register_method ({
{ subdir => 'upload' },
{ subdir => 'rrd' },
{ subdir => 'rrddata' },
+ { subdir => 'prunebackups' },
];
return $res;
diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
index 30bdcf6..478842f 100755
--- a/PVE/CLI/pvesm.pm
+++ b/PVE/CLI/pvesm.pm
@@ -12,8 +12,10 @@ use PVE::Cluster;
use PVE::INotify;
use PVE::RPCEnvironment;
use PVE::Storage;
+use PVE::Tools;
use PVE::API2::Storage::Config;
use PVE::API2::Storage::Content;
+use PVE::API2::Storage::PruneBackups;
use PVE::API2::Storage::Status;
use PVE::JSONSchema qw(get_standard_option);
use PVE::PTY;
@@ -818,6 +820,31 @@ our $cmddef = {
print "APIVER $res->{apiver}\n";
print "APIAGE $res->{apiage}\n";
}],
+ 'prune-backups-list' => [ "PVE::API2::Storage::PruneBackups", 'index',
['storage'], { node => $nodename }, sub {
I know this is easier since it's a one-to-one mapping to the API
endpoints, but IMHO this would really make more sense to have as an
option to 'pvesm prune-backups' from a usability POV..
Ok
+ my $res = shift;
+
+ my @sorted = sort {
+ my $vmcmp = PVE::Tools::safe_compare($a->{vmid}, $b->{vmid}, sub { $_[0]
<=> $_[1] });
+ return $vmcmp if $vmcmp ne 0;
+ return $a->{ctime} <=> $b->{ctime};
should sort by type first in case IDs are re-used between containers and
VMs
Ok
+ } @{$res};
+
+ my $maxlen = 0;
+ foreach my $backup (@sorted) {
+ my $volid = $backup->{volid};
+ $maxlen = length($volid) if length($volid) > $maxlen;
+ }
+ $maxlen+=1;
+
+ printf("%-${maxlen}s %15s %10s\n", 'Backup', 'Backup-ID', 'Prune-Mark');
+ foreach my $backup (@sorted) {
+ my $type = $backup->{type};
+ my $vmid = $backup->{vmid};
+ my $backup_id = defined($vmid) ? "$type/$vmid" : "$type";
+ printf("%-${maxlen}s %15s %10s\n", $backup->{volid}, $backup_id,
$backup->{mark});
+ }
+ }],
+ 'prune-backups' => [ "PVE::API2::Storage::PruneBackups", 'delete',
['storage'], { node => $nodename }, ],
};
for the CLI it would probably be nice to have an interactive mode?
pvesm prune-backups <storage> [vmid] [--interactive]
...
list of backups and their prune status
do you want to prune these backups? [y|N]
...
Should this be the default behavior for the CLI command? Then we can
have something like --yes or --force to skip the question.
in general it would be nice also for API users if we had a way to
- get list of to-be-pruned backups (client <-> API)
- ask user for confirmation (client <-> user)
- prune exactly those confirmed archives (client <-> API)
otherwise the dry-run mode is a bit dangerous as it offers a false sense
of security. the last step maybe even only if all the ones marked as
keep still exist? I don't know, just some food for thought..
1;
--
2.20.1
_______________________________________________
pve-devel mailing list
[email protected]
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
[email protected]
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
[email protected]
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
