* [pve-devel] [PATCH manager 0/4] add UI support for editing metric servers @ 2020-11-20 9:50 Dominik Csapak 2020-11-20 9:50 ` [pve-devel] [PATCH manager 1/4] Status/Plugin: fix jsonschema for MTU Dominik Csapak ` (3 more replies) 0 siblings, 4 replies; 9+ messages in thread From: Dominik Csapak @ 2020-11-20 9:50 UTC (permalink / raw) To: pve-devel Adds the typical sectionconfig api entry points, as well as a basic CRUD ui under Datacenter -> Metric Server the next step is to adapt the documentation to no refer to editing the config anymore, adding some more useful information, as well as adding a onlinehelp link to the ui Dominik Csapak (4): Status/Plugin: fix jsonschema for MTU Status/Plugin: add id to schema api2/cluster: add 'metricserver' api endpoints ui: add MetricServerView to Datacenter PVE/API2/Cluster.pm | 7 + PVE/API2/Makefile | 1 + PVE/API2/MetricServerConfig.pm | 238 +++++++++++++++++ PVE/Status/Plugin.pm | 6 +- www/manager6/Makefile | 1 + www/manager6/dc/Config.js | 6 + www/manager6/dc/MetricServerView.js | 397 ++++++++++++++++++++++++++++ 7 files changed, 655 insertions(+), 1 deletion(-) create mode 100644 PVE/API2/MetricServerConfig.pm create mode 100644 www/manager6/dc/MetricServerView.js -- 2.20.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH manager 1/4] Status/Plugin: fix jsonschema for MTU 2020-11-20 9:50 [pve-devel] [PATCH manager 0/4] add UI support for editing metric servers Dominik Csapak @ 2020-11-20 9:50 ` Dominik Csapak 2020-11-20 13:02 ` [pve-devel] applied: " Thomas Lamprecht 2020-11-20 9:50 ` [pve-devel] [PATCH manager 2/4] Status/Plugin: add id to schema Dominik Csapak ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Dominik Csapak @ 2020-11-20 9:50 UTC (permalink / raw) To: pve-devel jsonschema wants 'minimum' not 'min' Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- PVE/Status/Plugin.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PVE/Status/Plugin.pm b/PVE/Status/Plugin.pm index 26ac177e..4476c5dd 100644 --- a/PVE/Status/Plugin.pm +++ b/PVE/Status/Plugin.pm @@ -37,7 +37,7 @@ my $defaultData = { type => 'integer', description => "MTU for metrics transmission over UDP", default => 1500, - min => 512, + minimum => 512, maximum => 64*1024, optional => 1, }, -- 2.20.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] applied: [PATCH manager 1/4] Status/Plugin: fix jsonschema for MTU 2020-11-20 9:50 ` [pve-devel] [PATCH manager 1/4] Status/Plugin: fix jsonschema for MTU Dominik Csapak @ 2020-11-20 13:02 ` Thomas Lamprecht 0 siblings, 0 replies; 9+ messages in thread From: Thomas Lamprecht @ 2020-11-20 13:02 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak On 20.11.20 10:50, Dominik Csapak wrote: > jsonschema wants 'minimum' not 'min' > > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > --- > PVE/Status/Plugin.pm | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > applied, thanks! ^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH manager 2/4] Status/Plugin: add id to schema 2020-11-20 9:50 [pve-devel] [PATCH manager 0/4] add UI support for editing metric servers Dominik Csapak 2020-11-20 9:50 ` [pve-devel] [PATCH manager 1/4] Status/Plugin: fix jsonschema for MTU Dominik Csapak @ 2020-11-20 9:50 ` Dominik Csapak 2020-11-20 13:02 ` [pve-devel] applied: " Thomas Lamprecht 2020-11-20 9:50 ` [pve-devel] [PATCH manager 3/4] api2/cluster: add 'metricserver' api endpoints Dominik Csapak 2020-11-20 9:50 ` [pve-devel] [PATCH manager 4/4] ui: add MetricServerView to Datacenter Dominik Csapak 3 siblings, 1 reply; 9+ messages in thread From: Dominik Csapak @ 2020-11-20 9:50 UTC (permalink / raw) To: pve-devel Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- PVE/Status/Plugin.pm | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/PVE/Status/Plugin.pm b/PVE/Status/Plugin.pm index 4476c5dd..baf670ae 100644 --- a/PVE/Status/Plugin.pm +++ b/PVE/Status/Plugin.pm @@ -16,6 +16,10 @@ cfs_register_file('status.cfg', my $defaultData = { propertyList => { + id => { + description => "The ID of the entry.", + type => 'string', format => 'pve-configid', + }, type => { description => "Plugin type.", type => 'string', format => 'pve-configid', -- 2.20.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] applied: [PATCH manager 2/4] Status/Plugin: add id to schema 2020-11-20 9:50 ` [pve-devel] [PATCH manager 2/4] Status/Plugin: add id to schema Dominik Csapak @ 2020-11-20 13:02 ` Thomas Lamprecht 0 siblings, 0 replies; 9+ messages in thread From: Thomas Lamprecht @ 2020-11-20 13:02 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak On 20.11.20 10:50, Dominik Csapak wrote: > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > --- > PVE/Status/Plugin.pm | 4 ++++ > 1 file changed, 4 insertions(+) > > applied, thanks! ^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH manager 3/4] api2/cluster: add 'metricserver' api endpoints 2020-11-20 9:50 [pve-devel] [PATCH manager 0/4] add UI support for editing metric servers Dominik Csapak 2020-11-20 9:50 ` [pve-devel] [PATCH manager 1/4] Status/Plugin: fix jsonschema for MTU Dominik Csapak 2020-11-20 9:50 ` [pve-devel] [PATCH manager 2/4] Status/Plugin: add id to schema Dominik Csapak @ 2020-11-20 9:50 ` Dominik Csapak 2020-11-20 13:21 ` Thomas Lamprecht 2020-11-20 9:50 ` [pve-devel] [PATCH manager 4/4] ui: add MetricServerView to Datacenter Dominik Csapak 3 siblings, 1 reply; 9+ messages in thread From: Dominik Csapak @ 2020-11-20 9:50 UTC (permalink / raw) To: pve-devel modeled after our typical api endpoints for sectionschema configs Signed-off-by: Dominik Csapak <d.csapak@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' }, ]; 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 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', + }, + }, + }, + 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}}) { + 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.", + permissions => { + check => ['perm', '/', ['Sys.Audit']], + }, + parameters => { + additionalProperties => 0, + properties => { + id => { + type => 'string', + format => 'pve-configid', + }, + }, + }, + returns => { type => 'object' }, + code => sub { + my ($param) = @_; + + my $status_cfg = PVE::Cluster::cfs_read_file('status.cfg'); + 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); + die $@ if $@; + + return undef; + }}); + + +__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) { + $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}; + delete $data->{$k}; + } + } + + PVE::Cluster::cfs_write_file('status.cfg', $cfg); + }; + + PVE::Cluster::cfs_lock_file('status.cfg', undef, $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(); + + 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); + die $@ if $@; + + return undef; + }}); + +1; -- 2.20.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH manager 3/4] api2/cluster: add 'metricserver' api endpoints 2020-11-20 9:50 ` [pve-devel] [PATCH manager 3/4] api2/cluster: add 'metricserver' api endpoints Dominik Csapak @ 2020-11-20 13:21 ` Thomas Lamprecht 0 siblings, 0 replies; 9+ messages in thread From: Thomas Lamprecht @ 2020-11-20 13:21 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak On 20.11.20 10:50, Dominik Csapak wrote: > modeled after our typical api endpoints for sectionschema configs > > Signed-off-by: Dominik Csapak <d.csapak@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; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [pve-devel] [PATCH manager 4/4] ui: add MetricServerView to Datacenter 2020-11-20 9:50 [pve-devel] [PATCH manager 0/4] add UI support for editing metric servers Dominik Csapak ` (2 preceding siblings ...) 2020-11-20 9:50 ` [pve-devel] [PATCH manager 3/4] api2/cluster: add 'metricserver' api endpoints Dominik Csapak @ 2020-11-20 9:50 ` Dominik Csapak 2020-11-20 13:32 ` Thomas Lamprecht 3 siblings, 1 reply; 9+ messages in thread From: Dominik Csapak @ 2020-11-20 9:50 UTC (permalink / raw) To: pve-devel adds ui support for configuring the External Metric servers. for now, influxdb and graphite edit windows do not share code, it would be ideal to do that once we add more plugins Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> --- www/manager6/Makefile | 1 + www/manager6/dc/Config.js | 6 + www/manager6/dc/MetricServerView.js | 397 ++++++++++++++++++++++++++++ 3 files changed, 404 insertions(+) create mode 100644 www/manager6/dc/MetricServerView.js diff --git a/www/manager6/Makefile b/www/manager6/Makefile index 4fa8e1a3..a2514464 100644 --- a/www/manager6/Makefile +++ b/www/manager6/Makefile @@ -146,6 +146,7 @@ JSSRC= \ dc/TokenView.js \ dc/UserEdit.js \ dc/UserView.js \ + dc/MetricServerView.js \ lxc/CmdMenu.js \ lxc/Config.js \ lxc/CreateWizard.js \ diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js index 905c3dc0..2fdba743 100644 --- a/www/manager6/dc/Config.js +++ b/www/manager6/dc/Config.js @@ -231,6 +231,12 @@ Ext.define('PVE.dc.Config', { list_refs_url: '/cluster/firewall/refs', itemId: 'firewall-ipset' }, + { + xtype: 'pveMetricServerView', + title: gettext('Metric Server'), + iconCls: 'fa fa-bar-chart', + itemId: 'metricservers', + }, { xtype: 'pveDcSupport', title: gettext('Support'), diff --git a/www/manager6/dc/MetricServerView.js b/www/manager6/dc/MetricServerView.js new file mode 100644 index 00000000..894cd18a --- /dev/null +++ b/www/manager6/dc/MetricServerView.js @@ -0,0 +1,397 @@ +Ext.define('PVE.dc.MetricServerView', { + extend: 'Ext.grid.Panel', + alias: ['widget.pveMetricServerView'], + + stateful: true, + stateId: 'grid-metricserver', + + controller: { + xclass: 'Ext.app.ViewController', + + render_type: function(value) { + switch(value) { + case 'influxdb': return "InfluxDB"; + case 'graphite': return "Graphite"; + default: break; + } + return Proxmox.Utils.unknownText; + }, + + addInfluxDB: function() { + let me = this; + Ext.create(`PVE.dc.InfluxDBEdit`, { + url: `/api2/extjs/cluster/metricserver`, + listeners: { + destroy: function() { + me.reload(); + } + } + }).show(); + }, + + addGraphite: function() { + let me = this; + Ext.create(`PVE.dc.GraphiteEdit`, { + url: `/api2/extjs/cluster/metricserver`, + listeners: { + destroy: function() { + me.reload(); + } + } + }).show(); + }, + + editServer: function() { + let me = this; + let view = me.getView(); + let selection = view.getSelection(); + if (!selection || selection.length < 1) { return; } + + let rec = selection[0]; + + let xtype = me.render_type(rec.data.type); + Ext.create(`PVE.dc.${xtype}Edit`, { + url: `/api2/extjs/cluster/metricserver/${rec.data.id}`, + serverid: rec.data.id, + listeners: { + destroy: function() { + me.reload(); + } + } + }).show(); + }, + + reload: function() { this.getView().getStore().load(); }, + }, + + store: { + autoLoad: true, + id: 'metricservers', + proxy: { + type: 'proxmox', + url: '/api2/json/cluster/metricserver', + }, + }, + + columns: [ + { + text: gettext('Name'), + flex: 1, + dataIndex: 'id', + }, + { + text: gettext('Type'), + flex: 1, + dataIndex: 'type', + renderer: 'render_type', + }, + { + text: gettext('Enabled'), + dataIndex: 'disable', + width: 100, + renderer: Proxmox.Utils.format_neg_boolean, + }, + { + text: gettext('Server'), + flex: 1, + dataIndex: 'server', + }, + { + text: gettext('Port'), + flex: 1, + dataIndex: 'port', + }, + ], + + tbar: [ + { + text: gettext('Add'), + menu: [ + { + text: 'Graphite', + handler: 'addGraphite', + }, + { + text: 'InfluxDB', + handler: 'addInfluxDB', + }, + ] + }, + { + text: gettext('Edit'), + xtype: 'proxmoxButton', + handler: 'editServer', + disabled: true, + }, + { + xtype: 'proxmoxStdRemoveButton', + baseurl: `/api2/extjs/cluster/metricserver`, + callback: 'reload', + } + ], + + listeners: { + itemdblclick: 'editServer', + }, + + initComponent : function() { + var me = this; + + me.callParent(); + + Proxmox.Utils.monStoreErrors(me, me.getStore()); + } +}); + +Ext.define('PVE.dc.InfluxDBEdit', { + extend: 'Proxmox.window.Edit', + mixins: ['Proxmox.Mixin.CBind'], + + subject: 'InfluxDB', + + cbindData: function() { + let me = this; + me.isCreate = !me.serverid; + me.serverid = me.serverid || ""; + me.method = me.isCreate ? 'POST' : 'PUT'; + if (!me.isCreate) { + me.subject = `InfluxDB: ${me.serverid}`; + } + return {}; + }, + + items: [ + { + xtype: 'inputpanel', + + onGetValues: function(values) { + values.disable = values.enable ? 0 : 1; + delete values.enable; + return values; + }, + + column1: [ + { + xtype: 'hidden', + name: 'type', + value: 'influxdb', + cbind: { + submitValue: '{isCreate}', + }, + }, + { + fieldLabel: gettext('Name'), + xtype: 'pmxDisplayEditField', + name: 'id', + allowBlank: false, + cbind: { + editable: '{isCreate}', + value: '{serverid}', + }, + }, + { + fieldLabel: gettext('Enabled'), + xtype: 'checkbox', + inputValue: 1, + uncheckedValue: 0, + checked: true, + name: 'enable', + }, + ], + + column2: [ + { + fieldLabel: gettext('Server'), + xtype: 'proxmoxtextfield', + name: 'server', + allowBlank: false, + }, + { + fieldLabel: gettext('Port'), + xtype: 'proxmoxintegerfield', + minValue: 1, + maximum: 65536, + name: 'port', + allowBlank: false, + }, + ], + + advancedColumn1: [], // has to exists to render any advanced columns + + advancedColumn2: [ + { + fieldLabel: 'MTU', + xtype: 'proxmoxintegerfield', + name: 'mtu', + minValue: 1, + emptyText: '1500', + submitEmpty: false, + cbind: { + deleteEmpty: '{!isCreate}', + }, + }, + ] + }, + ], + + initComponent: function() { + let me = this; + me.callParent(); + if (!me.serverid) { return; } + + me.load({ + success: function(response, options) { + let values = response.result.data; + values.enable = !values.disable; + me.down('inputpanel').setValues(values); + } + }) + } +}); + +Ext.define('PVE.dc.GraphiteEdit', { + extend: 'Proxmox.window.Edit', + mixins: ['Proxmox.Mixin.CBind'], + + subject: 'Graphite', + + cbindData: function() { + let me = this; + me.isCreate = !me.serverid; + me.serverid = me.serverid || ""; + me.method = me.isCreate ? 'POST' : 'PUT'; + if (!me.isCreate) { + me.subject = `Graphite: ${me.serverid}`; + } + return {}; + }, + + items: [ + { + xtype: 'inputpanel', + + onGetValues: function(values) { + values.disable = values.enable ? 0 : 1; + delete values.enable; + return values; + }, + + column1: [ + { + xtype: 'hidden', + name: 'type', + value: 'graphite', + cbind: { + submitValue: '{isCreate}', + }, + }, + { + fieldLabel: gettext('Name'), + xtype: 'pmxDisplayEditField', + name: 'id', + allowBlank: false, + cbind: { + editable: '{isCreate}', + value: '{serverid}', + }, + }, + { + fieldLabel: gettext('Enabled'), + xtype: 'checkbox', + inputValue: 1, + uncheckedValue: 0, + checked: true, + name: 'enable', + }, + ], + + column2: [ + { + fieldLabel: gettext('Server'), + xtype: 'proxmoxtextfield', + name: 'server', + allowBlank: false, + }, + { + fieldLabel: gettext('Port'), + xtype: 'proxmoxintegerfield', + minimum: 1, + maximum: 65536, + name: 'port', + allowBlank: false, + }, + { + fieldLabel: gettext('Path'), + xtype: 'proxmoxtextfield', + emptyText: 'proxmox', + name: 'path', + cbind: { + deleteEmpty: '{!isCreate}', + }, + } + ], + + advancedColumn1: [ + { + fieldLabel: gettext('Protocol'), + xtype: 'proxmoxKVComboBox', + name: 'proto', + value: '__default__', + cbind: { + deleteEmpty: '{!isCreate}', + }, + comboItems: [ + ['__default__', 'UDP'], + ['tcp', 'TCP'], + ], + listeners: { + change: function(field, value) { + let me = this; + me.up('inputpanel').down('field[name=timeout]').setDisabled(value !== 'tcp'); + me.up('inputpanel').down('field[name=mtu]').setDisabled(value === 'tcp'); + }, + }, + }, + ], + + advancedColumn2: [ + { + fieldLabel: 'MTU', + xtype: 'proxmoxintegerfield', + name: 'mtu', + minimum: 1, + emptyText: '1500', + submitEmpty: false, + cbind: { + deleteEmpty: '{!isCreate}', + }, + }, + { + fieldLabel: gettext('TCP Timeout'), + xtype: 'proxmoxintegerfield', + name: 'timeout', + disabled: true, + cbind: { + deleteEmpty: '{!isCreate}', + }, + minValue: 1, + emptyText: 1, + }, + ], + }, + ], + + initComponent: function() { + let me = this; + me.callParent(); + if (!me.serverid) { return; } + + me.load({ + success: function(response, options) { + let values = response.result.data; + values.enable = !values.disable; + me.down('inputpanel').setValues(values); + } + }) + } +}); -- 2.20.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [pve-devel] [PATCH manager 4/4] ui: add MetricServerView to Datacenter 2020-11-20 9:50 ` [pve-devel] [PATCH manager 4/4] ui: add MetricServerView to Datacenter Dominik Csapak @ 2020-11-20 13:32 ` Thomas Lamprecht 0 siblings, 0 replies; 9+ messages in thread From: Thomas Lamprecht @ 2020-11-20 13:32 UTC (permalink / raw) To: Proxmox VE development discussion, Dominik Csapak On 20.11.20 10:50, Dominik Csapak wrote: > adds ui support for configuring the External Metric servers. > for now, influxdb and graphite edit windows do not share code, > it would be ideal to do that once we add more plugins > > Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> > --- > www/manager6/Makefile | 1 + > www/manager6/dc/Config.js | 6 + > www/manager6/dc/MetricServerView.js | 397 ++++++++++++++++++++++++++++ > 3 files changed, 404 insertions(+) > create mode 100644 www/manager6/dc/MetricServerView.js > > diff --git a/www/manager6/Makefile b/www/manager6/Makefile > index 4fa8e1a3..a2514464 100644 > --- a/www/manager6/Makefile > +++ b/www/manager6/Makefile > @@ -146,6 +146,7 @@ JSSRC= \ > dc/TokenView.js \ > dc/UserEdit.js \ > dc/UserView.js \ > + dc/MetricServerView.js \ > lxc/CmdMenu.js \ > lxc/Config.js \ > lxc/CreateWizard.js \ > diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js > index 905c3dc0..2fdba743 100644 > --- a/www/manager6/dc/Config.js > +++ b/www/manager6/dc/Config.js > @@ -231,6 +231,12 @@ Ext.define('PVE.dc.Config', { > list_refs_url: '/cluster/firewall/refs', > itemId: 'firewall-ipset' > }, > + { > + xtype: 'pveMetricServerView', > + title: gettext('Metric Server'), > + iconCls: 'fa fa-bar-chart', > + itemId: 'metricservers', > + }, > { > xtype: 'pveDcSupport', > title: gettext('Support'), > diff --git a/www/manager6/dc/MetricServerView.js b/www/manager6/dc/MetricServerView.js > new file mode 100644 > index 00000000..894cd18a > --- /dev/null > +++ b/www/manager6/dc/MetricServerView.js > @@ -0,0 +1,397 @@ > +Ext.define('PVE.dc.MetricServerView', { > + extend: 'Ext.grid.Panel', > + alias: ['widget.pveMetricServerView'], > + > + stateful: true, > + stateId: 'grid-metricserver', > + > + controller: { > + xclass: 'Ext.app.ViewController', > + > + render_type: function(value) { > + switch(value) { > + case 'influxdb': return "InfluxDB"; > + case 'graphite': return "Graphite"; > + default: break; > + } > + return Proxmox.Utils.unknownText; nit, that catch-all return could be in the switch default arm - so all function exits are there - it's a short function and no hard feeling, but stood out to me. > + }, > + > + addInfluxDB: function() { > + let me = this; > + Ext.create(`PVE.dc.InfluxDBEdit`, { > + url: `/api2/extjs/cluster/metricserver`, > + listeners: { > + destroy: function() { > + me.reload(); > + } could be a single line arrow function, for such short definitions they are still visible enough, IMO. destroy: () => me.reload(), > + } > + }).show(); you could use autoShow: true instead > + }, > + > + addGraphite: function() { > + let me = this; > + Ext.create(`PVE.dc.GraphiteEdit`, { > + url: `/api2/extjs/cluster/metricserver`, > + listeners: { > + destroy: function() { > + me.reload(); > + } same with reload as arrow function > + } > + }).show(); same as above (autoShow) > + }, > + > + editServer: function() { > + let me = this; > + let view = me.getView(); > + let selection = view.getSelection(); > + if (!selection || selection.length < 1) { return; } please format above if nicely, return and closing } on it's one line. > + > + let rec = selection[0]; > + > + let xtype = me.render_type(rec.data.type); > + Ext.create(`PVE.dc.${xtype}Edit`, { > + url: `/api2/extjs/cluster/metricserver/${rec.data.id}`, > + serverid: rec.data.id, > + listeners: { > + destroy: function() { > + me.reload(); same with reload as arrow function > + } > + } > + }).show(); could be autoShow > + }, > + > + reload: function() { this.getView().getStore().load(); }, please format in new lines > + }, > + > + store: { > + autoLoad: true, > + id: 'metricservers', > + proxy: { > + type: 'proxmox', > + url: '/api2/json/cluster/metricserver', > + }, > + }, > + > + columns: [ > + { > + text: gettext('Name'), > + flex: 1, > + dataIndex: 'id', > + }, > + { > + text: gettext('Type'), > + flex: 1, > + dataIndex: 'type', > + renderer: 'render_type', > + }, > + { > + text: gettext('Enabled'), > + dataIndex: 'disable', > + width: 100, > + renderer: Proxmox.Utils.format_neg_boolean, > + }, > + { > + text: gettext('Server'), > + flex: 1, > + dataIndex: 'server', > + }, > + { > + text: gettext('Port'), > + flex: 1, avoid flex for things we know there are fixed boundaries, while I am a fan of defining flex having all flexed is most of the time also "bad", as in, increases space between information needlessly so that there's much eye movement required for getting it on bigger screens. I discussed this with Dietmar a bit ago for some PBS job grids, and there he was right and making fixed length columns exactly that, fixed :) Sometimes a combination of flex and maxWidth can have really good results too. > + dataIndex: 'port', > + }, > + ], > + > + tbar: [ > + { > + text: gettext('Add'), > + menu: [ > + { > + text: 'Graphite', > + handler: 'addGraphite', > + }, > + { > + text: 'InfluxDB', > + handler: 'addInfluxDB', > + }, > + ] > + }, > + { > + text: gettext('Edit'), > + xtype: 'proxmoxButton', > + handler: 'editServer', > + disabled: true, > + }, > + { > + xtype: 'proxmoxStdRemoveButton', > + baseurl: `/api2/extjs/cluster/metricserver`, > + callback: 'reload', > + } > + ], > + > + listeners: { > + itemdblclick: 'editServer', > + }, > + > + initComponent : function() { > + var me = this; > + > + me.callParent(); > + > + Proxmox.Utils.monStoreErrors(me, me.getStore()); > + } > +}); > + > +Ext.define('PVE.dc.InfluxDBEdit', { > + extend: 'Proxmox.window.Edit', > + mixins: ['Proxmox.Mixin.CBind'], > + > + subject: 'InfluxDB', > + > + cbindData: function() { > + let me = this; > + me.isCreate = !me.serverid; > + me.serverid = me.serverid || ""; > + me.method = me.isCreate ? 'POST' : 'PUT'; > + if (!me.isCreate) { > + me.subject = `InfluxDB: ${me.serverid}`; > + } > + return {}; > + }, > + > + items: [ > + { > + xtype: 'inputpanel', > + > + onGetValues: function(values) { > + values.disable = values.enable ? 0 : 1; > + delete values.enable; > + return values; > + }, > + > + column1: [ > + { > + xtype: 'hidden', > + name: 'type', > + value: 'influxdb', > + cbind: { > + submitValue: '{isCreate}', > + }, > + }, > + { > + fieldLabel: gettext('Name'), > + xtype: 'pmxDisplayEditField', > + name: 'id', > + allowBlank: false, > + cbind: { > + editable: '{isCreate}', > + value: '{serverid}', > + }, > + }, > + { > + fieldLabel: gettext('Enabled'), > + xtype: 'checkbox', > + inputValue: 1, > + uncheckedValue: 0, > + checked: true, > + name: 'enable', > + }, > + ], > + > + column2: [ > + { > + fieldLabel: gettext('Server'), > + xtype: 'proxmoxtextfield', > + name: 'server', > + allowBlank: false, > + }, > + { > + fieldLabel: gettext('Port'), > + xtype: 'proxmoxintegerfield', > + minValue: 1, > + maximum: 65536, > + name: 'port', > + allowBlank: false, > + }, > + ], > + > + advancedColumn1: [], // has to exists to render any advanced columns > + > + advancedColumn2: [ > + { > + fieldLabel: 'MTU', > + xtype: 'proxmoxintegerfield', > + name: 'mtu', > + minValue: 1, > + emptyText: '1500', > + submitEmpty: false, > + cbind: { > + deleteEmpty: '{!isCreate}', > + }, > + }, > + ] > + }, > + ], > + > + initComponent: function() { > + let me = this; > + me.callParent(); > + if (!me.serverid) { return; } > + > + me.load({ > + success: function(response, options) { > + let values = response.result.data; > + values.enable = !values.disable; > + me.down('inputpanel').setValues(values); > + } > + }) > + } > +}); > + > +Ext.define('PVE.dc.GraphiteEdit', { > + extend: 'Proxmox.window.Edit', > + mixins: ['Proxmox.Mixin.CBind'], > + > + subject: 'Graphite', > + > + cbindData: function() { > + let me = this; > + me.isCreate = !me.serverid; > + me.serverid = me.serverid || ""; > + me.method = me.isCreate ? 'POST' : 'PUT'; > + if (!me.isCreate) { > + me.subject = `Graphite: ${me.serverid}`; > + } > + return {}; > + }, > + > + items: [ > + { > + xtype: 'inputpanel', > + > + onGetValues: function(values) { > + values.disable = values.enable ? 0 : 1; > + delete values.enable; > + return values; > + }, > + > + column1: [ > + { > + xtype: 'hidden', > + name: 'type', > + value: 'graphite', > + cbind: { > + submitValue: '{isCreate}', > + }, > + }, > + { > + fieldLabel: gettext('Name'), > + xtype: 'pmxDisplayEditField', > + name: 'id', > + allowBlank: false, > + cbind: { > + editable: '{isCreate}', > + value: '{serverid}', > + }, > + }, > + { > + fieldLabel: gettext('Enabled'), > + xtype: 'checkbox', > + inputValue: 1, > + uncheckedValue: 0, > + checked: true, > + name: 'enable', > + }, > + ], > + > + column2: [ > + { > + fieldLabel: gettext('Server'), > + xtype: 'proxmoxtextfield', > + name: 'server', > + allowBlank: false, > + }, > + { > + fieldLabel: gettext('Port'), > + xtype: 'proxmoxintegerfield', > + minimum: 1, > + maximum: 65536, > + name: 'port', > + allowBlank: false, > + }, > + { > + fieldLabel: gettext('Path'), > + xtype: 'proxmoxtextfield', > + emptyText: 'proxmox', > + name: 'path', > + cbind: { > + deleteEmpty: '{!isCreate}', > + }, > + } > + ], > + > + advancedColumn1: [ > + { > + fieldLabel: gettext('Protocol'), > + xtype: 'proxmoxKVComboBox', > + name: 'proto', > + value: '__default__', > + cbind: { > + deleteEmpty: '{!isCreate}', > + }, > + comboItems: [ > + ['__default__', 'UDP'], > + ['tcp', 'TCP'], > + ], > + listeners: { > + change: function(field, value) { > + let me = this; > + me.up('inputpanel').down('field[name=timeout]').setDisabled(value !== 'tcp'); > + me.up('inputpanel').down('field[name=mtu]').setDisabled(value === 'tcp'); > + }, > + }, > + }, > + ], > + > + advancedColumn2: [ > + { > + fieldLabel: 'MTU', > + xtype: 'proxmoxintegerfield', > + name: 'mtu', > + minimum: 1, > + emptyText: '1500', > + submitEmpty: false, > + cbind: { > + deleteEmpty: '{!isCreate}', > + }, > + }, > + { > + fieldLabel: gettext('TCP Timeout'), > + xtype: 'proxmoxintegerfield', > + name: 'timeout', > + disabled: true, > + cbind: { > + deleteEmpty: '{!isCreate}', > + }, > + minValue: 1, > + emptyText: 1, > + }, > + ], > + }, > + ], > + > + initComponent: function() { > + let me = this; > + me.callParent(); > + if (!me.serverid) { return; } > + > + me.load({ > + success: function(response, options) { > + let values = response.result.data; > + values.enable = !values.disable; > + me.down('inputpanel').setValues(values); > + } > + }) > + } > +}); > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-11-20 13:32 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-20 9:50 [pve-devel] [PATCH manager 0/4] add UI support for editing metric servers Dominik Csapak 2020-11-20 9:50 ` [pve-devel] [PATCH manager 1/4] Status/Plugin: fix jsonschema for MTU Dominik Csapak 2020-11-20 13:02 ` [pve-devel] applied: " Thomas Lamprecht 2020-11-20 9:50 ` [pve-devel] [PATCH manager 2/4] Status/Plugin: add id to schema Dominik Csapak 2020-11-20 13:02 ` [pve-devel] applied: " Thomas Lamprecht 2020-11-20 9:50 ` [pve-devel] [PATCH manager 3/4] api2/cluster: add 'metricserver' api endpoints Dominik Csapak 2020-11-20 13:21 ` Thomas Lamprecht 2020-11-20 9:50 ` [pve-devel] [PATCH manager 4/4] ui: add MetricServerView to Datacenter Dominik Csapak 2020-11-20 13:32 ` Thomas Lamprecht
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox