[pve-devel] [PATCH manager 0/3] Ceph API fixes/updates

[pve-devel] [PATCH manager 0/3] Ceph API fixes/updates

[Aaron Lauterer]

A few updates on the Ceph related API. Mainly:
- fixing outdated indexes
- adding/updating descritions and return value definitions

This series could collide with a few other Ceph patches that are still
outstanding. So I do expect needed rebases here and there.

Aaron Lauterer (3):
  ceph api: fix descriptions
  api: ceph: update index list
  api: ceph: update return value definitions

 PVE/API2/Ceph.pm         | 30 +++++++++++++++++++-----------
 PVE/API2/Ceph/MON.pm     | 14 +++++++++++---
 PVE/API2/Ceph/OSD.pm     | 13 +++++++++++++
 PVE/API2/Cluster/Ceph.pm | 23 ++++++++++++++++++++++-
 4 files changed, 65 insertions(+), 15 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH manager 1/3] ceph api: fix descriptions

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 PVE/API2/Ceph.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
index f3442408..0490f4a2 100644
--- a/PVE/API2/Ceph.pm
+++ b/PVE/API2/Ceph.pm
@@ -114,7 +114,7 @@ __PACKAGE__->register_method ({
     permissions => {
 	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
     },
-    description => "Get Ceph configuration.",
+    description => "Get the Ceph configuration file.",
     parameters => {
 	additionalProperties => 0,
 	properties => {
@@ -141,7 +141,7 @@ __PACKAGE__->register_method ({
     permissions => {
 	check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
     },
-    description => "Get Ceph configuration database.",
+    description => "Get the Ceph configuration database.",
     parameters => {
 	additionalProperties => 0,
 	properties => {
-- 
2.30.2

[pve-devel] applied: [PATCH manager 1/3] ceph api: fix descriptions

[Thomas Lamprecht]

On 21/12/2022 14:30, Aaron Lauterer wrote:
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  PVE/API2/Ceph.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
>

applied, thanks!

[pve-devel] [PATCH manager 2/3] api: ceph: update index list

[Aaron Lauterer]

some items were missing and placing them in alphabetical order will make
it easier in the future

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 PVE/API2/Ceph.pm | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
index 0490f4a2..55220324 100644
--- a/PVE/API2/Ceph.pm
+++ b/PVE/API2/Ceph.pm
@@ -87,20 +87,23 @@ __PACKAGE__->register_method ({
 	my ($param) = @_;
 
 	my $result = [
+	    { name => 'cmd-safety' },
+	    { name => 'config' },
+	    { name => 'configdb' },
+	    { name => 'crush' },
+	    { name => 'fs' },
 	    { name => 'init' },
+	    { name => 'log' },
+	    { name => 'mds' },
+	    { name => 'mgr' },
 	    { name => 'mon' },
 	    { name => 'osd' },
 	    { name => 'pools' },
-	    { name => 'fs' },
-	    { name => 'mds' },
-	    { name => 'stop' },
-	    { name => 'start' },
 	    { name => 'restart' },
-	    { name => 'status' },
-	    { name => 'crush' },
-	    { name => 'config' },
-	    { name => 'log' },
 	    { name => 'rules' },
+	    { name => 'start' },
+	    { name => 'status' },
+	    { name => 'stop' },
 	];
 
 	return $result;
-- 
2.30.2

[pve-devel] applied: [PATCH manager 2/3] api: ceph: update index list

[Thomas Lamprecht]

On 21/12/2022 14:30, Aaron Lauterer wrote:
> some items were missing and placing them in alphabetical order will make
> it easier in the future
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  PVE/API2/Ceph.pm | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
>

applied, amended the commit to note which items missed, thanks!

[pve-devel] [PATCH manager 3/3] api: ceph: update return value definitions

[Aaron Lauterer]

to have at either a more accurate description or some description at
all. For objects returning a lot of data, for example individual Ceph
services, a full description has been omitted as I think that this would
be a bit much.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
 PVE/API2/Ceph.pm         |  7 ++++++-
 PVE/API2/Ceph/MON.pm     | 14 +++++++++++---
 PVE/API2/Ceph/OSD.pm     | 13 +++++++++++++
 PVE/API2/Cluster/Ceph.pm | 23 ++++++++++++++++++++++-
 4 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
index 55220324..cc8720b2 100644
--- a/PVE/API2/Ceph.pm
+++ b/PVE/API2/Ceph.pm
@@ -622,7 +622,12 @@ __PACKAGE__->register_method ({
 	type => 'array',
 	items => {
 	    type => "object",
-	    properties => {},
+	    properties => {
+		name => {
+		    description => "Name of the CRUSH rule.",
+		    type => "string",
+		}
+	    },
 	},
 	links => [ { rel => 'child', href => "{name}" } ],
     },
diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
index 5771bb46..b452045c 100644
--- a/PVE/API2/Ceph/MON.pm
+++ b/PVE/API2/Ceph/MON.pm
@@ -212,9 +212,17 @@ __PACKAGE__->register_method ({
 	items => {
 	    type => "object",
 	    properties => {
-		name => { type => 'string' },
-		addr => { type => 'string', optional => 1 },
-		host => { type => 'string', optional => 1 },
+		name		    => { type => 'string' },
+		addr		    => { type => 'string', optional => 1 },
+		host		    => { type => 'boolean', optional => 1 },
+		direxists	    => { type => 'string', optional => 1 },
+		quorum		    => { type => 'boolean', optional => 1 },
+		host		    => { type => 'string', optional => 1 },
+		rank		    => { type => 'integer', optional => 1 },
+		service		    => { type => 'integer', optional => 1 },
+		state		    => { type => 'string', optional => 1 },
+		ceph_version	    => { type => 'string', optional => 1 },
+		ceph_version_short  => { type => 'string', optional => 1 },
 	    },
 	},
 	links => [ { rel => 'child', href => "{name}" } ],
diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index 93433b3a..ce1281d9 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -88,6 +88,19 @@ __PACKAGE__->register_method ({
     # fixme: return a list instead of extjs tree format ?
     returns => {
 	type => "object",
+	items => {
+	    type => "object",
+	    properties => {
+		flags => { type => "string" },
+		root => {
+		    type => "object",
+		    description => "extjs formatted grid tree",
+		},
+	    },
+	},
+
+
+	}
     },
     code => sub {
 	my ($param) = @_;
diff --git a/PVE/API2/Cluster/Ceph.pm b/PVE/API2/Cluster/Ceph.pm
index 7f825003..49f84b24 100644
--- a/PVE/API2/Cluster/Ceph.pm
+++ b/PVE/API2/Cluster/Ceph.pm
@@ -68,7 +68,20 @@ __PACKAGE__->register_method ({
 	    },
 	},
     },
-    returns => { type => 'object' },
+    returns => {
+	type => 'object'
+	description => "Items for each type of service containing objects for each instance.",
+	items => {
+	    type => "object",
+	    properties => {
+		mds => { type => "object" },
+		mgr => { type => "object" },
+		mon => { type => "object" },
+		node => { type => "object" },
+		osd => { type => "object" },
+	    }
+	},
+    },
     code => sub {
 	my ($param) = @_;
 
@@ -181,6 +194,14 @@ __PACKAGE__->register_method ({
 		    description => "Flag name.",
 		    type => 'string', enum => $possible_flags_list,
 		},
+		description => {
+		    description => "Flag description.",
+		    type => 'string',
+		},
+		value => {
+		    description => "Flag value.",
+		    type => 'boolean',
+		},
 	    },
 	},
 	links => [ { rel => 'child', href => "{name}" } ],
-- 
2.30.2

Re: [pve-devel] [PATCH manager 3/3] api: ceph: update return value definitions

[Thomas Lamprecht]

On 21/12/2022 14:30, Aaron Lauterer wrote:
> to have at either a more accurate description or some description at
> all. For objects returning a lot of data, for example individual Ceph
> services, a full description has been omitted as I think that this would
> be a bit much.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  PVE/API2/Ceph.pm         |  7 ++++++-
>  PVE/API2/Ceph/MON.pm     | 14 +++++++++++---
>  PVE/API2/Ceph/OSD.pm     | 13 +++++++++++++
>  PVE/API2/Cluster/Ceph.pm | 23 ++++++++++++++++++++++-
>  4 files changed, 52 insertions(+), 5 deletions(-)
> 
> diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
> index 55220324..cc8720b2 100644
> --- a/PVE/API2/Ceph.pm
> +++ b/PVE/API2/Ceph.pm
> @@ -622,7 +622,12 @@ __PACKAGE__->register_method ({
>  	type => 'array',
>  	items => {
>  	    type => "object",
> -	    properties => {},
> +	    properties => {
> +		name => {
> +		    description => "Name of the CRUSH rule.",
> +		    type => "string",
> +		}
> +	    },
>  	},
>  	links => [ { rel => 'child', href => "{name}" } ],
>      },
> diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
> index 5771bb46..b452045c 100644
> --- a/PVE/API2/Ceph/MON.pm
> +++ b/PVE/API2/Ceph/MON.pm
> @@ -212,9 +212,17 @@ __PACKAGE__->register_method ({
>  	items => {
>  	    type => "object",
>  	    properties => {
> -		name => { type => 'string' },
> -		addr => { type => 'string', optional => 1 },
> -		host => { type => 'string', optional => 1 },
> +		name		    => { type => 'string' },
> +		addr		    => { type => 'string', optional => 1 },
> +		host		    => { type => 'boolean', optional => 1 },
> +		direxists	    => { type => 'string', optional => 1 },
> +		quorum		    => { type => 'boolean', optional => 1 },
> +		host		    => { type => 'string', optional => 1 },
> +		rank		    => { type => 'integer', optional => 1 },
> +		service		    => { type => 'integer', optional => 1 },
> +		state		    => { type => 'string', optional => 1 },
> +		ceph_version	    => { type => 'string', optional => 1 },
> +		ceph_version_short  => { type => 'string', optional => 1 },

please avoid that code formatting style! It looks very out of place in PVE
source and would require touching unrelated lines if a longer variant gets
added or the longest one gets removed, which is just a nuisance and messes
with git history without any real benefit.

>  	    },
>  	},
>  	links => [ { rel => 'child', href => "{name}" } ],
> diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
> index 93433b3a..ce1281d9 100644
> --- a/PVE/API2/Ceph/OSD.pm
> +++ b/PVE/API2/Ceph/OSD.pm
> @@ -88,6 +88,19 @@ __PACKAGE__->register_method ({
>      # fixme: return a list instead of extjs tree format ?
>      returns => {
>  	type => "object",
> +	items => {
> +	    type => "object",
> +	    properties => {
> +		flags => { type => "string" },
> +		root => {
> +		    type => "object",
> +		    description => "extjs formatted grid tree",

It's ExtJS, but actually not really sure why that is relevant here, the ExtJS
tree store format is a very simple and relatively common serialization format
for a tree.

> +		},
> +	    },
> +	},
> +
> +
> +	}
>      },
>      code => sub {
>  	my ($param) = @_;
> diff --git a/PVE/API2/Cluster/Ceph.pm b/PVE/API2/Cluster/Ceph.pm
> index 7f825003..49f84b24 100644
> --- a/PVE/API2/Cluster/Ceph.pm
> +++ b/PVE/API2/Cluster/Ceph.pm
> @@ -68,7 +68,20 @@ __PACKAGE__->register_method ({
>  	    },
>  	},
>      },
> -    returns => { type => 'object' },
> +    returns => {
> +	type => 'object'
> +	description => "Items for each type of service containing objects for each instance.",
> +	items => {
> +	    type => "object",
> +	    properties => {
> +		mds => { type => "object" },
> +		mgr => { type => "object" },
> +		mon => { type => "object" },
> +		node => { type => "object" },
> +		osd => { type => "object" },

description (e.g., it might not be clear that "node" holds the ceph version on
that node), optionality and maybe (some common/most-useful) sub-properties could
be great to have too.

> +	    }
> +	},
> +    },
>      code => sub {
>  	my ($param) = @_;
>  
> @@ -181,6 +194,14 @@ __PACKAGE__->register_method ({
>  		    description => "Flag name.",
>  		    type => 'string', enum => $possible_flags_list,
>  		},
> +		description => {
> +		    description => "Flag description.",
> +		    type => 'string',
> +		},
> +		value => {
> +		    description => "Flag value.",
> +		    type => 'boolean',
> +		},
>  	    },
>  	},
>  	links => [ { rel => 'child', href => "{name}" } ],