[pve-devel] [PATCH v3 manager 0/2] rework ceph cfg api

[pve-devel] [PATCH v3 manager 0/2] rework ceph cfg api

[Aaron Lauterer]

to avoid multiple configXXX endpoints for different things.

A new API endpoint is used: 'cfg' and the current 'config' and
'configdb' are moved there (first 2 patches). The result is
* cfg/
  * raw (formerly config)
  * db (formerly configdb)

The other patches from the previous series (adding a new key/value
endpoint and fixing #2515) are omitted as we can work on them later too.


changes since v2: some cleanup of unused imports

Aaron Lauterer (2):
  api: ceph: add ceph/cfg path, deprecate ceph/config and ceph/configdb
  ui: ceph config: use new ceph/cfg/ API endpoints

 PVE/API2/Ceph.pm            |  15 ++++-
 PVE/API2/Ceph/Cfg.pm        | 115 ++++++++++++++++++++++++++++++++++++
 PVE/API2/Ceph/Makefile      |   1 +
 www/manager6/ceph/Config.js |   4 +-
 4 files changed, 130 insertions(+), 5 deletions(-)
 create mode 100644 PVE/API2/Ceph/Cfg.pm

-- 
2.30.2

[pve-devel] [PATCH v3 manager 1/2] api: ceph: add ceph/cfg path, deprecate ceph/config and ceph/configdb

[Aaron Lauterer]

Consolidating the different config paths lets us add more as needed
without polluting our API with too many 'configxxx' endpoints.

The config and configdb paths are renamed under the ceph/cfg path:
* config -> raw (returns the ceph.conf file as is)
* configdb -> db (returns the ceph config db contents)

The old paths are still available and need to be dropped at some point.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
changes since v2:
* removed unused import of PVE::Cluster
* removed unused imports from PVE::Tools, only 'file_get_contents' is
  needed

 PVE/API2/Ceph.pm       |  15 ++++--
 PVE/API2/Ceph/Cfg.pm   | 115 +++++++++++++++++++++++++++++++++++++++++
 PVE/API2/Ceph/Makefile |   1 +
 3 files changed, 128 insertions(+), 3 deletions(-)
 create mode 100644 PVE/API2/Ceph/Cfg.pm

diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
index 786a1870..3e3dd399 100644
--- a/PVE/API2/Ceph.pm
+++ b/PVE/API2/Ceph.pm
@@ -18,6 +18,7 @@ use PVE::RPCEnvironment;
 use PVE::Storage;
 use PVE::Tools qw(run_command file_get_contents file_set_contents extract_param);
 
+use PVE::API2::Ceph::Cfg;
 use PVE::API2::Ceph::OSD;
 use PVE::API2::Ceph::FS;
 use PVE::API2::Ceph::MDS;
@@ -30,6 +31,11 @@ use base qw(PVE::RESTHandler);
 
 my $pve_osd_default_journal_size = 1024*5;
 
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Ceph::Cfg",
+    path => 'cfg',
+});
+
 __PACKAGE__->register_method ({
     subclass => "PVE::API2::Ceph::OSD",
     path => 'osd',
@@ -88,6 +94,7 @@ __PACKAGE__->register_method ({
 
 	my $result = [
 	    { name => 'cmd-safety' },
+	    { name => 'cfg' },
 	    { name => 'config' },
 	    { name => 'configdb' },
 	    { name => 'crush' },
@@ -109,6 +116,8 @@ __PACKAGE__->register_method ({
 	return $result;
     }});
 
+
+# TODO: deprecrated, remove with PVE 8
 __PACKAGE__->register_method ({
     name => 'config',
     path => 'config',
@@ -117,7 +126,7 @@ __PACKAGE__->register_method ({
     permissions => {
 	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
     },
-    description => "Get the Ceph configuration file.",
+    description => "Get the Ceph configuration file. Deprecated, please use `/nodes/{node}/ceph/cfg/raw.",
     parameters => {
 	additionalProperties => 0,
 	properties => {
@@ -135,6 +144,7 @@ __PACKAGE__->register_method ({
 
     }});
 
+# TODO: deprecrated, remove with PVE 8
 __PACKAGE__->register_method ({
     name => 'configdb',
     path => 'configdb',
@@ -144,7 +154,7 @@ __PACKAGE__->register_method ({
     permissions => {
 	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
     },
-    description => "Get the Ceph configuration database.",
+    description => "Get the Ceph configuration database. Deprecated, please use `/nodes/{node}/ceph/cfg/db.",
     parameters => {
 	additionalProperties => 0,
 	properties => {
@@ -179,7 +189,6 @@ __PACKAGE__->register_method ({
 	return $res;
     }});
 
-
 __PACKAGE__->register_method ({
     name => 'init',
     path => 'init',
diff --git a/PVE/API2/Ceph/Cfg.pm b/PVE/API2/Ceph/Cfg.pm
new file mode 100644
index 00000000..f3c2589d
--- /dev/null
+++ b/PVE/API2/Ceph/Cfg.pm
@@ -0,0 +1,115 @@
+package PVE::API2::Ceph::Cfg;
+
+use strict;
+use warnings;
+
+use PVE::Ceph::Tools;
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::RADOS;
+use PVE::Tools qw(file_get_contents);
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    description => "Directory index.",
+    permissions => { user => 'all' },
+    permissions => {
+	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
+    },
+    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) = @_;
+
+	my $result = [
+	    { name => 'raw' },
+	    { name => 'db' },
+	];
+
+	return $result;
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'raw',
+    path => 'raw',
+    method => 'GET',
+    proxyto => 'node',
+    permissions => {
+	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
+    },
+    description => "Get the Ceph configuration file.",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	},
+    },
+    returns => { type => 'string' },
+    code => sub {
+	my ($param) = @_;
+
+	PVE::Ceph::Tools::check_ceph_inited();
+
+	my $path = PVE::Ceph::Tools::get_config('pve_ceph_cfgpath');
+	return file_get_contents($path);
+
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'db',
+    path => 'db',
+    method => 'GET',
+    proxyto => 'node',
+    protected => 1,
+    permissions => {
+	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
+    },
+    description => "Get the Ceph configuration database.",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => {
+		section => { type => "string", },
+		name => { type => "string", },
+		value => { type => "string", },
+		level => { type => "string", },
+		'can_update_at_runtime' => { type => "boolean", },
+		mask => { type => "string" },
+	    },
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+
+	PVE::Ceph::Tools::check_ceph_inited();
+
+	my $rados = PVE::RADOS->new();
+	my $res = $rados->mon_command( { prefix => 'config dump', format => 'json' });
+	foreach my $entry (@$res) {
+	    $entry->{can_update_at_runtime} = $entry->{can_update_at_runtime}? 1 : 0; # JSON::true/false -> 1/0
+	}
+
+	return $res;
+    }});
diff --git a/PVE/API2/Ceph/Makefile b/PVE/API2/Ceph/Makefile
index 45daafda..be7b6926 100644
--- a/PVE/API2/Ceph/Makefile
+++ b/PVE/API2/Ceph/Makefile
@@ -1,6 +1,7 @@
 include ../../../defines.mk
 
 PERLSOURCE= 			\
+	Cfg.pm 			\
 	MGR.pm			\
 	MON.pm			\
 	OSD.pm			\
-- 
2.30.2

Re: [pve-devel] [PATCH v3 manager 1/2] api: ceph: add ceph/cfg path, deprecate ceph/config and ceph/configdb

[Dominik Csapak]

one super small nit inline, rest LGTM

On 3/20/23 11:50, Aaron Lauterer wrote:
> Consolidating the different config paths lets us add more as needed
> without polluting our API with too many 'configxxx' endpoints.
> 
> The config and configdb paths are renamed under the ceph/cfg path:
> * config -> raw (returns the ceph.conf file as is)
> * configdb -> db (returns the ceph config db contents)
> 
> The old paths are still available and need to be dropped at some point.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> changes since v2:
> * removed unused import of PVE::Cluster
> * removed unused imports from PVE::Tools, only 'file_get_contents' is
>    needed
> 
>   PVE/API2/Ceph.pm       |  15 ++++--
>   PVE/API2/Ceph/Cfg.pm   | 115 +++++++++++++++++++++++++++++++++++++++++
>   PVE/API2/Ceph/Makefile |   1 +
>   3 files changed, 128 insertions(+), 3 deletions(-)
>   create mode 100644 PVE/API2/Ceph/Cfg.pm
> 
> diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
> index 786a1870..3e3dd399 100644
> --- a/PVE/API2/Ceph.pm
> +++ b/PVE/API2/Ceph.pm
> @@ -18,6 +18,7 @@ use PVE::RPCEnvironment;
>   use PVE::Storage;
>   use PVE::Tools qw(run_command file_get_contents file_set_contents extract_param);
>   
> +use PVE::API2::Ceph::Cfg;
>   use PVE::API2::Ceph::OSD;
>   use PVE::API2::Ceph::FS;
>   use PVE::API2::Ceph::MDS;
> @@ -30,6 +31,11 @@ use base qw(PVE::RESTHandler);
>   
>   my $pve_osd_default_journal_size = 1024*5;
>   
> +__PACKAGE__->register_method ({
> +    subclass => "PVE::API2::Ceph::Cfg",
> +    path => 'cfg',
> +});
> +
>   __PACKAGE__->register_method ({
>       subclass => "PVE::API2::Ceph::OSD",
>       path => 'osd',
> @@ -88,6 +94,7 @@ __PACKAGE__->register_method ({
>   
>   	my $result = [
>   	    { name => 'cmd-safety' },
> +	    { name => 'cfg' },
>   	    { name => 'config' },
>   	    { name => 'configdb' },
>   	    { name => 'crush' },
> @@ -109,6 +116,8 @@ __PACKAGE__->register_method ({
>   	return $result;
>       }});
>   
> +
> +# TODO: deprecrated, remove with PVE 8
>   __PACKAGE__->register_method ({
>       name => 'config',
>       path => 'config',
> @@ -117,7 +126,7 @@ __PACKAGE__->register_method ({
>       permissions => {
>   	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
>       },
> -    description => "Get the Ceph configuration file.",
> +    description => "Get the Ceph configuration file. Deprecated, please use `/nodes/{node}/ceph/cfg/raw.",
>       parameters => {
>   	additionalProperties => 0,
>   	properties => {
> @@ -135,6 +144,7 @@ __PACKAGE__->register_method ({
>   
>       }});
>   
> +# TODO: deprecrated, remove with PVE 8
>   __PACKAGE__->register_method ({
>       name => 'configdb',
>       path => 'configdb',
> @@ -144,7 +154,7 @@ __PACKAGE__->register_method ({
>       permissions => {
>   	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
>       },
> -    description => "Get the Ceph configuration database.",
> +    description => "Get the Ceph configuration database. Deprecated, please use `/nodes/{node}/ceph/cfg/db.",
>       parameters => {
>   	additionalProperties => 0,
>   	properties => {
> @@ -179,7 +189,6 @@ __PACKAGE__->register_method ({
>   	return $res;
>       }});
>   
> -
>   __PACKAGE__->register_method ({
>       name => 'init',
>       path => 'init',
> diff --git a/PVE/API2/Ceph/Cfg.pm b/PVE/API2/Ceph/Cfg.pm
> new file mode 100644
> index 00000000..f3c2589d
> --- /dev/null
> +++ b/PVE/API2/Ceph/Cfg.pm
> @@ -0,0 +1,115 @@
> +package PVE::API2::Ceph::Cfg;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Ceph::Tools;
> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::RADOS;
> +use PVE::Tools qw(file_get_contents);
> +
> +use base qw(PVE::RESTHandler);
> +
> +__PACKAGE__->register_method ({
> +    name => 'index',
> +    path => '',
> +    method => 'GET',
> +    description => "Directory index.",
> +    permissions => { user => 'all' },
> +    permissions => {
> +	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
> +    },

nit: double 'permissions' property, also from where you copied it^^

could be fixed up too

also, we don't do it on other index calls either, but maybe we want
to start adding a "proxyto => 'node'" for index api calls too, so that
the api user sees the correct index for the node he retrieves?

does not really matter for this now, as we'd want to that either
for all or none anyway

> (+    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) = @_;
> +
> +	my $result = [
> +	    { name => 'raw' },
> +	    { name => 'db' },
> +	];
> +
> +	return $result;
> +    }});
> +
> +__PACKAGE__->register_method ({
> +    name => 'raw',
> +    path => 'raw',
> +    method => 'GET',
> +    proxyto => 'node',
> +    permissions => {
> +	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
> +    },
> +    description => "Get the Ceph configuration file.",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	},
> +    },
> +    returns => { type => 'string' },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	PVE::Ceph::Tools::check_ceph_inited();
> +
> +	my $path = PVE::Ceph::Tools::get_config('pve_ceph_cfgpath');
> +	return file_get_contents($path);
> +
> +    }});
> +
> +__PACKAGE__->register_method ({
> +    name => 'db',
> +    path => 'db',
> +    method => 'GET',
> +    proxyto => 'node',
> +    protected => 1,
> +    permissions => {
> +	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
> +    },
> +    description => "Get the Ceph configuration database.",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	},
> +    },
> +    returns => {
> +	type => 'array',
> +	items => {
> +	    type => 'object',
> +	    properties => {
> +		section => { type => "string", },
> +		name => { type => "string", },
> +		value => { type => "string", },
> +		level => { type => "string", },
> +		'can_update_at_runtime' => { type => "boolean", },
> +		mask => { type => "string" },
> +	    },
> +	},
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	PVE::Ceph::Tools::check_ceph_inited();
> +
> +	my $rados = PVE::RADOS->new();
> +	my $res = $rados->mon_command( { prefix => 'config dump', format => 'json' });
> +	foreach my $entry (@$res) {
> +	    $entry->{can_update_at_runtime} = $entry->{can_update_at_runtime}? 1 : 0; # JSON::true/false -> 1/0
> +	}
> +
> +	return $res;
> +    }});
> diff --git a/PVE/API2/Ceph/Makefile b/PVE/API2/Ceph/Makefile
> index 45daafda..be7b6926 100644
> --- a/PVE/API2/Ceph/Makefile
> +++ b/PVE/API2/Ceph/Makefile
> @@ -1,6 +1,7 @@
>   include ../../../defines.mk
>   
>   PERLSOURCE= 			\
> +	Cfg.pm 			\
>   	MGR.pm			\
>   	MON.pm			\
>   	OSD.pm			\

[pve-devel] [PATCH v3 manager 2/2] ui: ceph config: use new ceph/cfg/ API endpoints

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
nothing changes since v1

 www/manager6/ceph/Config.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/www/manager6/ceph/Config.js b/www/manager6/ceph/Config.js
index 7f07f15f..d4da20a8 100644
--- a/www/manager6/ceph/Config.js
+++ b/www/manager6/ceph/Config.js
@@ -53,7 +53,7 @@ Ext.define('PVE.node.CephConfigDb', {
 	    throw "no node name specified";
 	}
 
-	me.store.proxy.url = '/api2/json/nodes/' + nodename + '/ceph/configdb';
+	me.store.proxy.url = '/api2/json/nodes/' + nodename + '/ceph/cfg/db';
 
 	me.callParent();
 
@@ -102,7 +102,7 @@ Ext.define('PVE.node.CephConfig', {
 	}
 
 	Ext.apply(me, {
-	    url: '/nodes/' + nodename + '/ceph/config',
+	    url: '/nodes/' + nodename + '/ceph/cfg/raw',
 	    listeners: {
 		activate: function() {
 		    me.load();
-- 
2.30.2

Re: [pve-devel] [PATCH v3 manager 0/2] rework ceph cfg api

[Dominik Csapak]

aside from the one nit (which can be fixed up)

Reviewed-By: Dominik Csapak <d.csapak@proxmox.com>
Tested-By: Dominik Csapak <d.csapak@proxmox.com>

[pve-devel] applied-series: [PATCH v3 manager 0/2] rework ceph cfg api

[Thomas Lamprecht]

Am 20/03/2023 um 14:36 schrieb Dominik Csapak:
> aside from the one nit (which can be fixed up)

fixed up in a follow up

> 
> Reviewed-By: Dominik Csapak <d.csapak@proxmox.com>
> Tested-By: Dominik Csapak <d.csapak@proxmox.com>
> 

with above: applied series, thanks!