On 20.11.20 10:50, Dominik Csapak wrote:
> modeled after our typical api endpoints for sectionschema configs
>
> Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
> ---
> PVE/API2/Cluster.pm | 7 +
> PVE/API2/Makefile | 1 +
> PVE/API2/MetricServerConfig.pm | 238 +++++++++++++++++++++++++++++++++
> 3 files changed, 246 insertions(+)
> create mode 100644 PVE/API2/MetricServerConfig.pm
>
> diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
> index e768cbc6..4eb387b1 100644
> --- a/PVE/API2/Cluster.pm
> +++ b/PVE/API2/Cluster.pm
> @@ -29,6 +29,7 @@ use PVE::API2::ClusterConfig;
> use PVE::API2::Firewall::Cluster;
> use PVE::API2::HAConfig;
> use PVE::API2::ReplicationConfig;
> +use PVE::API2::MetricServerConfig;
>
> my $have_sdn;
> eval {
> @@ -43,6 +44,11 @@ __PACKAGE__->register_method ({
> path => 'replication',
> });
>
> +__PACKAGE__->register_method ({
> + subclass => "PVE::API2::MetricServerConfig",
> + path => 'metricserver',
> +});
> +
> __PACKAGE__->register_method ({
> subclass => "PVE::API2::ClusterConfig",
> path => 'config',
> @@ -132,6 +138,7 @@ __PACKAGE__->register_method ({
> { name => 'config' },
> { name => 'acme' },
> { name => 'ceph' },
> + { name => 'metricserver' },
I'd like to use a folder here,
'metric/server'
> ];
>
> if ($have_sdn) {
> diff --git a/PVE/API2/Makefile b/PVE/API2/Makefile
> index bc5ccc36..fdadbc40 100644
> --- a/PVE/API2/Makefile
> +++ b/PVE/API2/Makefile
> @@ -24,6 +24,7 @@ PERLSOURCE = \
> NodeConfig.pm \
> Scan.pm \
> Hardware.pm \
> + MetricServerConfig.pm \
> Services.pm
>
> all:
> diff --git a/PVE/API2/MetricServerConfig.pm b/PVE/API2/MetricServerConfig.pm
please move this inside the Cluster folder, no need to further accumulate most
stuff at the top API2 directory.
> new file mode 100644
> index 00000000..6d4df628
> --- /dev/null
> +++ b/PVE/API2/MetricServerConfig.pm
> @@ -0,0 +1,238 @@
> +package PVE::API2::MetricServerConfig;
> +
> +use warnings;
> +use strict;
> +
> +use PVE::Tools qw(extract_param);
> +use PVE::Exception qw(raise_perm_exc raise_param_exc);
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::RPCEnvironment;
> +use PVE::ExtMetric;
> +
> +use PVE::RESTHandler;
> +
> +use base qw(PVE::RESTHandler);
> +
> +__PACKAGE__->register_method ({
> + name => 'index',
> + path => '',
> + method => 'GET',
> + description => "List configured metric servers.",
> + permissions => {
> + check => ['perm', '/', ['Sys.Audit']],
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {},
> + },
> + returns => {
> + type => 'array',
> + items => {
> + type => "object",
> + properties => {
> + id => {
> + description => "The ID of the entry.",
> + type => 'string'
> + },
> + disable => {
> + description => "Flag to disable the plugin.",
> + type => 'boolean',
> + },
> + type => {
> + description => "Plugin type.",
> + type => 'string',
> + },
> + server => {
> + description => "Server dns name or IP address",
> + type => 'string',
> + },
> + port => {
> + description => "Server network port",
> + type => 'integer',
> + },
nit (can be done as follow up) could we reuse the createSchema here?
A returnSchema for SectionConfig could make sense in general.
> + },
> + },
> + links => [ { rel => 'child', href => "{id}" } ],
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + my $res = [];
> + my $status_cfg = PVE::Cluster::cfs_read_file('status.cfg');
> +
> + for my $id (keys %{$status_cfg->{ids}}) {
please sort the keys above, so the returned array stays stable.
> + my $plugin_config = $status_cfg->{ids}->{$id};
> + push @$res, {
> + id => $id,
> + disable => $plugin_config->{disable} // 0,
> + type => $plugin_config->{type},
> + server => $plugin_config->{server},
> + port => $plugin_config->{port},
> + };
> + }
> +
> + return $res;
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'read',
> + path => '{id}',
> + method => 'GET',
> + description => "Read replication job configuration.",
But this ain't no "replication" job? Please re-check for copy-is-my-hobby
leftovers ;-)
> + permissions => {
> + check => ['perm', '/', ['Sys.Audit']],
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + id => {
> + type => 'string',
> + format => 'pve-configid',
> + },
> + },
> + },
> + returns => { type => 'object' },
this could possibly use the createSchema - with most set as optional? (or a
future return schema)
> + code => sub {
> + my ($param) = @_;
> +
> + my $status_cfg = PVE::Cluster::cfs_read_file('status.cfg');
I do not remember anymore, is this OK without locking?
> + my $id = $param->{id};
> +
> + if (!defined($status_cfg->{ids}->{$id})) {
> + die "status server entry '$id' does not exist\n";
> + }
> +
> + return $status_cfg->{ids}->{$id};
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'create',
> + path => '',
> + protected => 1,
> + method => 'POST',
> + description => "Create a new external metric server config",
> + permissions => {
> + check => ['perm', '/', ['Sys.Modify']],
> + },
> + parameters => PVE::Status::Plugin->createSchema(),
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $type = extract_param($param, 'type');
> + my $plugin = PVE::Status::Plugin->lookup($type);
> + my $id = extract_param($param, 'id');
> +
> + my $code = sub {
> + my $cfg = PVE::Cluster::cfs_read_file('status.cfg');
> +
> + die "Metric server '$id' already exists\n"
> + if $cfg->{ids}->{$id};
> +
> + my $opts = $plugin->check_config($id, $param, 1, 1);
> + $cfg->{ids}->{$id} = $opts;
> +
> + PVE::Cluster::cfs_write_file('status.cfg', $cfg);
> + };
> +
> + PVE::Cluster::cfs_lock_file('status.cfg', undef, $code);
I'd rather like if that over general $code variable was dropped and passed
directly:
PVE::Cluster::cfs_lock_file('status.cfg', undef, sub {
...
});
> + die $@ if $@;
> +
> + return undef;
nit, undef is not needed, `return;` is enough (and a little bit less noise)
> + }});
> +
> +
> +__PACKAGE__->register_method ({
> + name => 'update',
> + protected => 1,
> + path => '{id}',
> + method => 'PUT',
> + description => "Update metric server configuration.",
> + permissions => {
> + check => ['perm', '/', ['Sys.Modify']],
> + },
> + parameters => PVE::Status::Plugin->updateSchema(),
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $id = extract_param($param, 'id');
> + my $digest = extract_param($param, 'digest');
> + my $delete = extract_param($param, 'delete');
> +
> + my $code = sub {
> + my $cfg = PVE::Cluster::cfs_read_file('status.cfg');
> +
> + PVE::SectionConfig::assert_if_modified($cfg, $digest);
> +
> + my $data = $cfg->{ids}->{$id};
> + die "no such server '$id'\n" if !$data;
> +
> + my $plugin = PVE::Status::Plugin->lookup($data->{type});
> + my $opts = $plugin->check_config($id, $param, 0, 1);
> +
> + foreach my $k (%$opts) {
this is wrong, you need to use explicitly (keys %$opts) or you loop over both,
keys and values!
nit: would be good to keep it consistent when adding a brand new module,
for vs. foreach here.
> + $data->{$k} = $opts->{$k};
> + }
> +
> + if ($delete) {
> + my $options = $plugin->private()->{options}->{$data->{type}};
> + foreach my $k (PVE::Tools::split_list($delete)) {
> + my $d = $options->{$k} ||
> + die "no such option '$k'\n";
> + die "unable to delete required option '$k'\n"
> + if !$d->{optional};
> + die "unable to delete fixed option '$k'\n"
> + if $d->{fixed};
those seem all short enough, so we could avoid the newlines but add an extra one
before below delete operation - for readability.
> + delete $data->{$k};
> + }
> + }
> +
> + PVE::Cluster::cfs_write_file('status.cfg', $cfg);
> + };
> +
> + PVE::Cluster::cfs_lock_file('status.cfg', undef, $code);
same as above regarding $code
> + die $@ if $@;
> +
> + return undef;
> + }});
> +
> +__PACKAGE__->register_method ({
> + name => 'delete',
> + protected => 1,
> + path => '{id}',
> + method => 'DELETE',
> + description => "Remove Metric server.",
> + permissions => {
> + check => ['perm', '/', ['Sys.Modify']],
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + id => {
> + type => 'string',
> + format => 'pve-configid',
> + },
> + }
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $rpcenv = PVE::RPCEnvironment::get();
above is unused
> +
> + my $code = sub {
> + my $cfg = PVE::Cluster::cfs_read_file('status.cfg');
> +
> + my $id = $param->{id};
> + delete $cfg->{ids}->{$id};
> + PVE::Cluster::cfs_write_file('status.cfg', $cfg);
> + };
> +
> + PVE::Cluster::cfs_lock_file('status.cfg', undef, $code);
same as above
> + die $@ if $@;
> +
> + return undef;
> + }});
> +
> +1;
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
