[pve-devel] [PATCH pve-manager 01/18] pvesr: add pve storage replication tool

Dietmar Maurer dietmar at proxmox.com
Mon May 29 11:57:48 CEST 2017


comments inline
> 
> On Tue, May 23, 2017 at 09:08:40AM +0200, Dietmar Maurer wrote:
> > Just added code to configure jobs. Replication itself is not
> > implemented.
> > 
> > Signed-off-by: Dietmar Maurer <dietmar at proxmox.com>
> > ---
> >  PVE/API2/Cluster.pm           |   7 ++
> >  PVE/API2/Makefile             |   2 +
> >  PVE/API2/Nodes.pm             |   7 ++
> >  PVE/API2/Replication.pm       |  93 ++++++++++++++++
> >  PVE/API2/ReplicationConfig.pm | 217 +++++++++++++++++++++++++++++++++++++
> >  PVE/CLI/Makefile              |   2 +-
> >  PVE/CLI/pvesr.pm              | 151 ++++++++++++++++++++++++++
> >  PVE/Makefile                  |   1 +
> >  PVE/Replication.pm            | 242
> > ++++++++++++++++++++++++++++++++++++++++++
> >  bin/Makefile                  |   2 +-
> >  bin/pvesr                     |   8 ++
> >  11 files changed, 730 insertions(+), 2 deletions(-)
> >  create mode 100644 PVE/API2/Replication.pm
> >  create mode 100644 PVE/API2/ReplicationConfig.pm
> >  create mode 100644 PVE/CLI/pvesr.pm
> >  create mode 100644 PVE/Replication.pm
> >  create mode 100644 bin/pvesr
> > 
> > diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
> > index 6ce86de4..0e94e9ff 100644
> > --- a/PVE/API2/Cluster.pm
> > +++ b/PVE/API2/Cluster.pm
> > @@ -22,10 +22,16 @@ use PVE::RPCEnvironment;
> >  use PVE::JSONSchema qw(get_standard_option);
> >  use PVE::Firewall;
> >  use PVE::API2::Firewall::Cluster;
> > +use PVE::API2::ReplicationConfig;
> >  
> >  use base qw(PVE::RESTHandler);
> >  
> >  __PACKAGE__->register_method ({
> > +    subclass => "PVE::API2::ReplicationConfig",
> > +    path => 'replication',
> > +});
> > +
> > +__PACKAGE__->register_method ({
> >      subclass => "PVE::API2::ClusterConfig",
> >      path => 'config',
> >  });
> > @@ -82,6 +88,7 @@ __PACKAGE__->register_method ({
> >  	    { name => 'log' },
> >  	    { name => 'options' },
> >  	    { name => 'resources' },
> > +	    { name => 'replication' },
> >  	    { name => 'tasks' },
> >  	    { name => 'backup' },
> >  	    { name => 'ha' },
> > diff --git a/PVE/API2/Makefile b/PVE/API2/Makefile
> > index 4dfcbb56..86d75d36 100644
> > --- a/PVE/API2/Makefile
> > +++ b/PVE/API2/Makefile
> > @@ -1,6 +1,8 @@
> >  include ../../defines.mk
> >  
> >  PERLSOURCE = 			\
> > +	Replication.pm		\
> > +	ReplicationConfig.pm	\
> >  	Ceph.pm			\
> >  	APT.pm			\
> >  	Subscription.pm		\
> > diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
> > index a45ca6db..885cf116 100644
> > --- a/PVE/API2/Nodes.pm
> > +++ b/PVE/API2/Nodes.pm
> > @@ -40,6 +40,7 @@ use PVE::API2::VZDump;
> >  use PVE::API2::APT;
> >  use PVE::API2::Ceph;
> >  use PVE::API2::Firewall::Host;
> > +use PVE::API2::Replication;
> >  use Digest::MD5;
> >  use Digest::SHA;
> >  use PVE::API2::Disks;
> > @@ -113,6 +114,11 @@ __PACKAGE__->register_method ({
> >  });
> >  
> >  __PACKAGE__->register_method ({
> > +    subclass => "PVE::API2::Replication",
> > +    path => 'replication',
> > +});
> > +
> > +__PACKAGE__->register_method ({
> >      name => 'index', 
> >      path => '', 
> >      method => 'GET',
> > @@ -147,6 +153,7 @@ __PACKAGE__->register_method ({
> >  	    { name => 'tasks' },
> >  	    { name => 'rrd' }, # fixme: remove?
> >  	    { name => 'rrddata' },# fixme: remove?
> > +	    { name => 'replication' },
> >  	    { name => 'vncshell' },
> >  	    { name => 'spiceshell' },
> >  	    { name => 'time' },
> > diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
> > new file mode 100644
> > index 00000000..3c631e44
> > --- /dev/null
> > +++ b/PVE/API2/Replication.pm
> > @@ -0,0 +1,93 @@
> > +package PVE::API2::Replication;
> > +
> > +use warnings;
> > +use strict;
> > +
> > +use PVE::JSONSchema qw(get_standard_option);
> > +use PVE::RPCEnvironment;
> > +use PVE::ReplicationConfig;
> > +use PVE::Replication;
> > +
> > +use PVE::RESTHandler;
> > +
> > +use base qw(PVE::RESTHandler);
> > +
> > +__PACKAGE__->register_method ({
> > +    name => 'index',
> > +    path => '',
> > +    method => 'GET',
> > +    permissions => { user => 'all' },
> > +    description => "Directory index.",
> > +    parameters => {
> > +	additionalProperties => 0,
> > +	properties => {
> > +	    node => get_standard_option('pve-node'),
> > +	},
> > +    },
> > +    returns => {
> > +	type => 'array',
> > +	items => {
> > +	    type => "object",
> > +	    properties => {},
> > +	},
> > +	links => [ { rel => 'child', href => "{name}" } ],
> > +    },
> > +    code => sub {
> > +	my ($param) = @_;
> > +
> > +	return [
> > +	    { name => 'status' },
> > +	];
> > +    }});
> > +
> > +
> > +__PACKAGE__->register_method ({
> > +    name => 'status',
> > +    path => 'status',
> > +    method => 'GET',
> > +    description => "List replication job status.",
> > +    permissions => {
> > +	description => "Only list jobs where you have VM.Audit permissons on
> > /vms/<vmid>.",
> 
> this is not really a description of the permissions ;) also a typo in
> "permissions". maybe:
> 
> Requires the VM.Audit permission on /vms/<vmid>.

fixed in v2

> also, maybe it makes sense to expose the status of a single guest as
> well?

yes, waiting for feedback from GUI development.

> 
> > +	user => 'all',
> > +    },
> > +    protected => 1,
> > +    proxyto => 'node',
> > +    parameters => {
> > +	additionalProperties => 0,
> > +	properties => {
> > +	    node => get_standard_option('pve-node'),
> > +	},
> > +    },
> > +    returns => {
> > +	type => 'array',
> > +	items => {
> > +	    type => "object",
> > +	    properties => {},
> > +	},
> > +	links => [ { rel => 'child', href => "{vmid}" } ],
> > +    },
> > +    code => sub {
> > +	my ($param) = @_;
> > +
> > +	my $rpcenv = PVE::RPCEnvironment::get();
> > +	my $authuser = $rpcenv->get_user();
> > +
> > +	my $jobs = PVE::Replication::job_status();
> > +
> > +	my $res = [];
> > +	foreach my $id (sort keys %$jobs) {
> > +	    my $d = $jobs->{$id};
> > +	    my $state = delete $d->{state};
> > +	    my $vmid = $d->{guest};
> > +	    next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit' ]);
> > +	    $d->{id} = $id;
> > +	    foreach my $k (qw(last_sync fail_count error duration)) {
> > +		$d->{$k} = $state->{$k} if defined($state->{$k});
> > +	    }
> > +	    push @$res, $d;
> > +	}
> > +
> > +	return $res;
> > +    }});
> > +
> > +1;
> > diff --git a/PVE/API2/ReplicationConfig.pm b/PVE/API2/ReplicationConfig.pm
> > new file mode 100644
> > index 00000000..147b5891
> > --- /dev/null
> > +++ b/PVE/API2/ReplicationConfig.pm
> > @@ -0,0 +1,217 @@
> > +package PVE::API2::ReplicationConfig;
> 
> the whole file is riddled with whitespace and indentation errors. these
> should probably be fixed before applying.

fixed in v2
 
> > +
> > +use warnings;
> > +use strict;
> > +
> > +use PVE::Tools qw(extract_param);
> > +use PVE::Exception qw(raise_perm_exc);
> > +use PVE::JSONSchema qw(get_standard_option);
> > +use PVE::RPCEnvironment;
> > +use PVE::ReplicationConfig;
> > +
> > +use PVE::RESTHandler;
> > +
> > +use base qw(PVE::RESTHandler);
> > +
> > +__PACKAGE__->register_method ({
> > +    name => 'index',
> > +    path => '',
> > +    method => 'GET',
> > +    description => "List replication jobs.",
> > +    permissions => {
> > +	description => "Only list jobs where you have VM.Audit permissons on
> > /vms/<vmid>.",
> 
> same as above for the status

fixed in v2

> > +	user => 'all',
> > +    },
> > +    parameters => {
> > +	additionalProperties => 0,
> > +	properties => {},
> > +    },
> > +    returns => {
> > +	type => 'array',
> > +	items => {
> > +	    type => "object",
> > +	    properties => {},
> > +	},
> > +	links => [ { rel => 'child', href => "{vmid}" } ],
> > +    },
> > +    code => sub {
> > +	my ($param) = @_;
> > +
> > +	my $rpcenv = PVE::RPCEnvironment::get();
> > +	my $authuser = $rpcenv->get_user();
> > +
> > +	my $cfg = PVE::ReplicationConfig->new();
> > +
> > +	my $res = [];
> > +	foreach my $id (sort keys %{$cfg->{ids}}) {
> > +	    my $d = $cfg->{ids}->{$id};
> > +	    my $vmid = $d->{guest};
> > +	    next if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit' ]);
> > +	    $d->{id} = $id;
> > +	    push @$res, $d;
> > +	}
> > +
> > +	return $res;
> > +    }});
> > +
> > +__PACKAGE__->register_method ({
> > +    name => 'read', 
> > +    path => '{id}',
> > +    method => 'GET',
> > +    description => "Read replication job configuration.",
> > +    permissions => {
> > +	description => "Only list jobs where you have VM.Audit permissons on
> > /vms/<vmid>.",
> 
> same as for status

fixed in v2

> > +	user => 'all',
> > +    },
> > +    parameters => {
> > +    	additionalProperties => 0,
> > +	properties => {
> > +	    id => get_standard_option('pve-replication-id'),
> > +	},
> > +    },
> > +    returns => { type => 'object' },
> > +    code => sub {
> > +	my ($param) = @_;
> > +
> > +	my $rpcenv = PVE::RPCEnvironment::get();
> > +	my $authuser = $rpcenv->get_user();
> > +
> > +	my $cfg = PVE::ReplicationConfig->new();
> > +
> > +	my $data = $cfg->{ids}->{$param->{id}};
> > +
> > +	die "no such replication job '$param->{id}'\n" if !defined($data);
> > +
> > +	my $vmid = $data->{guest};
> > +
> > +	raise_perm_exc() if !$rpcenv->check($authuser, "/vms/$vmid", [ 'VM.Audit'
> > ]);
> > +	
> > +	$data->{id} = $param->{id};
> > +
> > +	return $data;
> > +    }});
> > +
> > +__PACKAGE__->register_method ({
> > +    name => 'create',
> > +    path => '', 
> > +    protected => 1,
> > +    method => 'POST',
> > +    description => "Create a new replication job",
> > +    permissions => { 
> > +	check => ['perm', '/storage', ['Datastore.Allocate']],
> 
> why? this does not seem right to me..

why not?

> > +    },
> > +    parameters => PVE::ReplicationConfig->createSchema(),
> > +    returns => { type => 'null' },
> > +    code => sub {
> > +	my ($param) = @_;
> > +
> > +	my $type = extract_param($param, 'type');
> > +	my $plugin = PVE::ReplicationConfig->lookup($type);
> > +	my $id = extract_param($param, 'id');
> > +
> > +	my $code = sub {
> > +	    my $cfg = PVE::ReplicationConfig->new();
> > +
> > +	    #die "replication job for guest '$param->{guest}' to target
> > '$param->{target}' already exists\n" 
> 
> leftover? should already be handled by write_config

removed in v2

> > +	    die "replication job '$id' already exists\n" 
> > +		if $cfg->{ids}->{$id};
> > +	
> > +	    my $opts = $plugin->check_config($id, $param, 1, 1);
> > +
> > +	    $cfg->{ids}->{$id} = $opts;
> > +	    
> > +	    $cfg->write();
> > +	};
> > +	
> > +	PVE::ReplicationConfig::lock($code);
> > +	
> > +	return undef;
> > +    }});
> > +
> > +
> > +__PACKAGE__->register_method ({
> > +    name => 'update',
> > +    protected => 1,
> > +    path => '{id}',
> > +    method => 'PUT',
> > +    description => "Update replication job configuration.",
> > +    permissions => { 
> > +	check => ['perm', '/storage', ['Datastore.Allocate']],
> 
> again - this does not seem right to me?
> 
> > +    },
> > +    parameters => PVE::ReplicationConfig->updateSchema(),
> > +    returns => { type => 'null' },
> > +    code => sub {
> > +	my ($param) = @_;
> > +	
> > +	my $id = extract_param($param, 'id');
> > +
> > +	my $code = sub {
> > +	    my $cfg = PVE::ReplicationConfig->new();
> > +
> > +	    my $data = $cfg->{ids}->{$id};
> > +	    die "no such job '$id'\n" if !$data;
> > +	    
> > +	    my $plugin = PVE::ReplicationConfig->lookup($data->{type});
> > +	    my $opts = $plugin->check_config($id, $param, 0, 1);
> > +
> > +	    foreach my $k (%$opts) {
> > +		$data->{$k} = $opts->{$k};
> > +	    }
> > +
> > +	    $cfg->write();
> > +	};
> > +
> > +	PVE::ReplicationConfig::lock($code);
> > +
> > +	return undef;
> > +    }});
> > +
> > +__PACKAGE__->register_method ({
> > +    name => 'delete',
> > +    protected => 1,
> > +    path => '{id}',
> > +    method => 'DELETE',
> > +    description => "Delete replication job",
> > +    permissions => { 
> > +	check => ['perm', '/storage', ['Datastore.Allocate']],
> 
> and once more ;)
> 
> > +    },
> > +    parameters => {
> > +    	additionalProperties => 0,
> > +	properties => { 
> > +	    id => get_standard_option('pve-replication-id'),
> > +	    keep => {
> > +		description => "Keep replicated data at target (do not remove).",
> > +		type => 'boolean',
> > +		optional => 1,
> > +		default => 0,
> > +	    },
> > +	}
> > +    },
> > +    returns => { type => 'null' },
> > +    code => sub {
> > +	my ($param) = @_;
> > +
> > +	my $id = extract_param($param, 'id');
> > +
> > +	my $code = sub {
> > +	    my $cfg = PVE::ReplicationConfig->new();
> > +
> > +	    my $data = $cfg->{ids}->{$id};
> > +	    die "no such job '$id'\n" if !$data;
> > +
> > +	    if (!$param->{keep}) {
> > +		# fixme: cleanup data at target
> > +
> > +	    }
> > +	    # fixme: cleanup snapshots
> > +	    
> > +	    delete $cfg->{ids}->{$id};
> > +
> > +	    $cfg->write();
> > +	};
> > +
> > +	PVE::ReplicationConfig::lock($code);
> > +
> > +	return undef;
> > +    }});
> > +1;
> > diff --git a/PVE/CLI/Makefile b/PVE/CLI/Makefile
> > index b005a8f1..3a27503d 100644
> > --- a/PVE/CLI/Makefile
> > +++ b/PVE/CLI/Makefile
> > @@ -1,6 +1,6 @@
> >  include ../../defines.mk
> >  
> > -SOURCES=vzdump.pm pvesubscription.pm pveceph.pm pveam.pm
> > +SOURCES=vzdump.pm pvesubscription.pm pveceph.pm pveam.pm pvesr.pm
> >  
> >  all:
> >  
> > diff --git a/PVE/CLI/pvesr.pm b/PVE/CLI/pvesr.pm
> > new file mode 100644
> > index 00000000..1dc24635
> > --- /dev/null
> > +++ b/PVE/CLI/pvesr.pm
> > @@ -0,0 +1,151 @@
> > +package PVE::CLI::pvesr;
> > +
> > +use strict;
> > +use warnings;
> > +use POSIX qw(strftime);
> > +use Time::Local;
> 
> not used?

removed in v2

> 
> > +use JSON;
> > +
> > +use PVE::JSONSchema qw(get_standard_option);
> > +use PVE::INotify;
> > +use PVE::RPCEnvironment;
> > +use PVE::Tools qw(extract_param);
> > +use PVE::SafeSyslog;
> > +use PVE::CLIHandler;
> > +
> > +use PVE::Replication;
> > +use PVE::API2::ReplicationConfig;
> > +use PVE::API2::Replication;
> > +
> > +use base qw(PVE::CLIHandler);
> > +
> > +my $nodename = PVE::INotify::nodename();
> > +
> > +sub setup_environment {
> > +    PVE::RPCEnvironment->setup_default_cli_env();
> > +}
> > +
> > +__PACKAGE__->register_method ({
> > +    name => 'run',
> > +    path => 'run',
> > +    method => 'POST',
> > +    description => "This method is called by the systemd-timer and executes
> > all (or a specific) sync jobs.",
> > +    parameters => {
> > +	additionalProperties => 0,
> > +	properties => {
> > +	    id => get_standard_option('pve-replication-id', { optional => 1 }),
> > +	},
> > +    },
> > +    returns => { type => 'null' },
> > +    code => sub {
> > +	my ($param) = @_;
> > +
> > +	if (my $id = extract_param($param, 'id')) {
> > +
> > +	    PVE::Replication::run_single_job($id);
> > +
> > +	} else {
> > +
> > +	    PVE::Replication::run_jobs();
> > +	}
> > +
> > +	return undef;
> > +    }});
> > +
> > +__PACKAGE__->register_method ({
> > +    name => 'enable',
> > +    path => 'enable',
> > +    method => 'POST',
> > +    description => "Enable a replication jobs.",
> 
> s/jobs/job/

fixed in v2
 
> > +    parameters => {
> > +	additionalProperties => 0,
> > +	properties => {
> > +	    id => get_standard_option('pve-replication-id'),
> > +	},
> > +    },
> > +    returns => { type => 'null' },
> > +    code => sub {
> > +	my ($param) = @_;
> > +
> > +	$param->{disable} = 0;
> > +
> > +	return PVE::API2::ReplicationConfig->update($param);
> > +    }});
> > +
> > +__PACKAGE__->register_method ({
> > +    name => 'disable',
> > +    path => 'disable',
> > +    method => 'POST',
> > +    description => "Disable a replication jobs.",
> 
> s/jobs/job/

fixed in v2
 
> > +    parameters => {
> > +	additionalProperties => 0,
> > +	properties => {
> > +	    id => get_standard_option('pve-replication-id'),
> > +	},
> > +    },
> > +    returns => { type => 'null' },
> > +    code => sub {
> > +	my ($param) = @_;
> > +
> > +	$param->{disable} = 1;
> > +
> > +	return PVE::API2::ReplicationConfig->update($param);
> > +    }});
> > +
> > +my $print_job_list = sub {
> > +    my ($list) = @_;
> > +
> > +    my $format = "%-20s %10s %-20s %10s %5s %8s\n";
> > +
> > +    printf($format, "JobID", "GuestID", "Target", "Interval", "Rate",
> > "Enabled");
> > +
> > +    foreach my $job (sort { $a->{guest} <=> $b->{guest} } @$list) {
> > +	my $plugin = PVE::ReplicationConfig->lookup($job->{type});
> > +	my $tid = $plugin->get_unique_target_id($job);
> > +
> > +	printf($format, $job->{id}, $job->{guest}, $tid,
> > +	       defined($job->{interval}) ? $job->{interval} : '-',
> > +	       defined($job->{rate}) ? $job->{rate} : '-',
> > +	       $job->{disable} ? 'no' : 'yes'
> > +	    );
> > +    }
> > +};
> > +
> > +my $print_job_status = sub {
> > +    my ($list) = @_;
> > +
> > +    my $format = "%-20s %10s %-20s %20s %10s %10s %s\n";
> > +
> > +    printf($format, "JobID", "GuestID", "Target", "LastSync", "Duration",
> > "FailCount", "State");
> > +
> > +    foreach my $job (sort { $a->{guest} <=> $b->{guest} } @$list) {
> > +	my $plugin = PVE::ReplicationConfig->lookup($job->{type});
> > +	my $tid = $plugin->get_unique_target_id($job);
> > +
> > +	my $timestr = $job->{last_sync} ?
> > +	    strftime("%Y-%m-%d_%H:%M:%S", localtime($job->{last_sync})) : '-';
> > +
> > +	printf($format, $job->{id}, $job->{guest}, $tid,
> > +	       $timestr, $job->{duration} // '-',
> > +	       $job->{fail_count}, , $job->{error} // 'OK');
> > +    }
> > +};
> > +
> > +our $cmddef = {
> > +    status => [ 'PVE::API2::Replication', 'status', [], { node => $nodename
> > }, $print_job_status ],
> > +
> > +    jobs => [ 'PVE::API2::ReplicationConfig', 'index' , [], {},
> > $print_job_list ],
> > +    read => [ 'PVE::API2::ReplicationConfig', 'read' , ['id'], {},
> > +	     sub { my $res = shift; print to_json($res, { pretty => 1, canonical
> > => 1}); }],
> 
> shouldn't this be encode_json? (according to "perldoc JSON", when
> talkign to the outside world, to ensure proper handling of UTF-8)

encode_json() does not support canonical. But I added utf8 => 1 in v2
 




More information about the pve-devel mailing list