[pve-devel] [PATCH v2 manager 0/5] rework ceph cfg api & fix 2515 use size defaults

[pve-devel] [PATCH v2 manager 0/5] rework ceph cfg api & fix 2515 use size defaults

[Aaron Lauterer]

The main goal of this series is to improve the handling of configured
default size & min_size values when creating a new Ceph Pool in the GUI.

But since that means a new necessary API endpoint, we also rework the
Ceph API to make it cleaner.

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)

Then we add a new endpoint 'cfg/value' that allows us to fetch values
for config keys that are set either in the config DB of Ceph or in the
ceph.conf file.

More in each patch.

Depending on how we want to handle the API deprecation, we might just
want to apply the first two patches with 7.4, even if the actual
implementation of the fix itself will have to wait.

changes since v1:
* 2 new patches to rework API so that the new endpoint has its place
* incorporated suggested code changes

Aaron Lauterer (5):
  api: ceph: add ceph/cfg path, deprecate ceph/config and ceph/configdb
  ui: ceph config: use new ceph/cfg/ API endpoints
  api: ceph: add endpoint to fetch config keys
  fix #2515: ui: ceph pool create: use configured defaults for size and
    min_size
  ui: ceph pool edit: rework with controller and formulas

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

-- 
2.30.2

[pve-devel] [PATCH v2 manager 1/5] 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 v1:
* add this commit to rework the API

 PVE/API2/Ceph.pm       |  15 ++++--
 PVE/API2/Ceph/Cfg.pm   | 116 +++++++++++++++++++++++++++++++++++++++++
 PVE/API2/Ceph/Makefile |   1 +
 3 files changed, 129 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..a00ef19c
--- /dev/null
+++ b/PVE/API2/Ceph/Cfg.pm
@@ -0,0 +1,116 @@
+package PVE::API2::Ceph::Cfg;
+
+use strict;
+use warnings;
+
+use PVE::Ceph::Tools;
+use PVE::Cluster qw(cfs_read_file cfs_write_file);
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::RADOS;
+use PVE::Tools qw(run_command file_get_contents file_set_contents extract_param);
+
+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

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

[Aaron Lauterer]

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
changes since v1:
* added this patch

 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

[pve-devel] [PATCH v2 manager 3/5] api: ceph: add endpoint to fetch config keys

[Aaron Lauterer]

This new endpoint allows to get the values of config keys that are
either set in the config db or the ceph.conf file.

Values that are set in the ceph.conf file have priority over values set
in the conifg db via 'ceph config set'.

Expects the --config-keys parameter as a semicolon separated list of
"<section>:<config key>" where the section is a section in the ceph.conf
or config db. For example: global:osd_pool_default_size

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
changes since v1:
* use kebab-case parameter names
* use kebab-case for the ceph config parameters, which also are returned
  that way
* improve how we parse and merge the config db and ceph.conf file. This
  way though, we dont warn if we cannot find a config key.
* renamed regex to make the distinctions clearer
* dropped 'format => string-list' as it didn't work when leaving out
  [;, ] from the regex. But we don't need both.

 PVE/API2/Ceph/Cfg.pm | 81 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/PVE/API2/Ceph/Cfg.pm b/PVE/API2/Ceph/Cfg.pm
index a00ef19c..0caa96d3 100644
--- a/PVE/API2/Ceph/Cfg.pm
+++ b/PVE/API2/Ceph/Cfg.pm
@@ -40,6 +40,7 @@ __PACKAGE__->register_method ({
 	my $result = [
 	    { name => 'raw' },
 	    { name => 'db' },
+	    { name => 'value' },
 	];
 
 	return $result;
@@ -114,3 +115,83 @@ __PACKAGE__->register_method ({
 
 	return $res;
     }});
+
+
+my $SINGLE_CONFIGKEY_RE = qr/[0-9a-z\-_\.]+:[0-9a-zA-Z\-_]+/i;
+my $CONFIGKEYS_RE = qr/^(:?${SINGLE_CONFIGKEY_RE})(:?[;, ]${SINGLE_CONFIGKEY_RE})*$/;
+
+__PACKAGE__->register_method ({
+    name => 'value',
+    path => 'value',
+    method => 'GET',
+    proxyto => 'node',
+    protected => 1,
+    permissions => {
+	check => ['perm', '/', [ 'Sys.Audit' ]],
+    },
+    description => "Get configured values from either the config file or config DB.",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    'config-keys' => {
+		type => "string",
+		typetext => "<section>:<config key>[;<section>:<config key>]",
+		pattern => $CONFIGKEYS_RE,
+		description => "List of <section>:<config key> items.",
+	    }
+	},
+    },
+    returns => {
+	type => 'object',
+	description => "Contains {section}->{key} children with the values",
+    },
+    code => sub {
+	my ($param) = @_;
+
+	PVE::Ceph::Tools::check_ceph_inited();
+
+	# Ceph treats '-' and '_' the same in parameter names, stick with '-'
+	my $normalize = sub {
+	    my $t = shift;
+	    $t =~ s/_/-/g;
+	    return $t;
+	};
+
+	my $requested_keys = {};
+	for my $pair (PVE::Tools::split_list($param->{'config-keys'})) {
+	    my ($section, $key) = split(":", $pair);
+	    $section = $normalize->($section);
+	    $key = $normalize->($key);
+
+	    $requested_keys->{$section}->{$key} = 1;
+	}
+
+	my $config = {};
+
+	my $rados = PVE::RADOS->new();
+	my $configdb = $rados->mon_command( { prefix => 'config dump', format => 'json' });
+	for my $s (@{$configdb}) {
+	    my ($section, $name, $value) = $s ->@{'section', 'name', 'value'};
+	    my $n_section = $normalize->($section);
+	    my $n_name = $normalize->($name);
+
+	    $config->{$n_section}->{$n_name} = $value
+		if defined $requested_keys->{$n_section} && $n_name eq $n_name;
+	}
+
+	# read ceph.conf after config db as it has priority if settings are present in both
+	my $config_file = cfs_read_file('ceph.conf');
+	for my $section (keys %{$config_file}) {
+	    my $n_section = $normalize->($section);
+	    next if !defined $requested_keys->{$n_section};
+
+	    for my $key (keys %{$config_file->{$section}}) {
+		my $n_key = $normalize->($key);
+		$config->{$n_section}->{$n_key} = $config_file->{$section}->{$key}
+		    if $requested_keys->{$n_section}->{$n_key};
+	    }
+	}
+
+	return $config;
+    }});
-- 
2.30.2

[pve-devel] [PATCH v2 manager 4/5] fix #2515: ui: ceph pool create: use configured defaults for size and min_size

[Aaron Lauterer]

Instead of hard coded defaults for the size and min_size parameter,
check if we have defaults configured in the ceph.conf or config db and
use those.

There are clusters where different defaults are needed. For example if
the cluster spans two rooms and needs to survive the loss of one. A
size/min_size of 4/2 are common defaults in such a situation.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---

changes since v1:
* set defaultSize and defaultMinSize to undefined in CephPoolInputPanel
* set them in cbindData in PoolEdit (needed when editing existing pool)
* waitMsgTarget when opening pool create window
* guard returned values for config keys in case they are empty

 www/manager6/ceph/Pool.js | 63 +++++++++++++++++++++++++++++++--------
 1 file changed, 51 insertions(+), 12 deletions(-)

diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js
index f7a4d9ba..e155a731 100644
--- a/www/manager6/ceph/Pool.js
+++ b/www/manager6/ceph/Pool.js
@@ -7,6 +7,11 @@ Ext.define('PVE.CephPoolInputPanel', {
     onlineHelp: 'pve_ceph_pools',
 
     subject: 'Ceph Pool',
+
+    // sets default pool sizes
+    defaultSize: undefined,
+    defaultMinSize: undefined,
+
     column1: [
 	{
 	    xtype: 'pmxDisplayEditField',
@@ -27,7 +32,9 @@ Ext.define('PVE.CephPoolInputPanel', {
 	    name: 'size',
 	    editConfig: {
 		xtype: 'proxmoxintegerfield',
-		value: 3,
+		cbind: {
+		    value: (get) => get('defaultSize') ?? 3,
+		},
 		minValue: 2,
 		maxValue: 7,
 		allowBlank: false,
@@ -40,7 +47,6 @@ Ext.define('PVE.CephPoolInputPanel', {
 		    },
 		},
 	    },
-
 	},
     ],
     column2: [
@@ -78,9 +84,15 @@ Ext.define('PVE.CephPoolInputPanel', {
 	    xtype: 'proxmoxintegerfield',
 	    fieldLabel: gettext('Min. Size'),
 	    name: 'min_size',
-	    value: 2,
 	    cbind: {
-		minValue: (get) => get('isCreate') ? 2 : 1,
+		value: (get) => get('defaultMinSize') ?? 2,
+		minValue: (get) => {
+		    if (Number(get('defaultMinSize')) === 1) {
+			return 1;
+		    } else {
+			return get('isCreate') ? 2 : 1;
+		    }
+		},
 	    },
 	    maxValue: 7,
 	    allowBlank: false,
@@ -195,6 +207,8 @@ Ext.define('PVE.Ceph.PoolEdit', {
     cbindData: {
 	pool_name: '',
 	isCreate: (cfg) => !cfg.pool_name,
+	defaultSize: undefined,
+	defaultMinSize: undefined,
     },
 
     cbind: {
@@ -216,6 +230,8 @@ Ext.define('PVE.Ceph.PoolEdit', {
 	    pool_name: '{pool_name}',
 	    isErasure: '{isErasure}',
 	    isCreate: '{isCreate}',
+	    defaultSize: '{defaultSize}',
+	    defaultMinSize: '{defaultMinSize}',
 	},
     }],
 });
@@ -388,14 +404,37 @@ Ext.define('PVE.node.Ceph.PoolList', {
 		{
 		    text: gettext('Create'),
 		    handler: function() {
-			Ext.create('PVE.Ceph.PoolEdit', {
-			    title: gettext('Create') + ': Ceph Pool',
-			    isCreate: true,
-			    isErasure: false,
-			    nodename: nodename,
-			    autoShow: true,
-			    listeners: {
-				destroy: () => rstore.load(),
+			let keys = [
+			    'global:osd-pool-default-min-size',
+			    'global:osd-pool-default-size',
+			];
+			let params = {
+			    'config-keys': keys.join(';'),
+			};
+
+			Proxmox.Utils.API2Request({
+			    url: '/nodes/localhost/ceph/cfg/value',
+			    method: 'GET',
+			    params,
+			    waitMsgTarget: me.getView(),
+			    failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+			    success: function({ result: { data } }) {
+				let global = data.global;
+				let defaultSize = global?.['osd-pool-default-size'] ?? undefined;
+				let defaultMinSize = global?.['osd-pool-default-min-size'] ?? undefined;
+
+				Ext.create('PVE.Ceph.PoolEdit', {
+				    title: gettext('Create') + ': Ceph Pool',
+				    isCreate: true,
+				    isErasure: false,
+				    defaultSize,
+				    defaultMinSize,
+				    nodename: nodename,
+				    autoShow: true,
+				    listeners: {
+					destroy: () => rstore.load(),
+				    },
+				});
 			    },
 			});
 		    },
-- 
2.30.2

[pve-devel] [PATCH v2 manager 5/5] ui: ceph pool edit: rework with controller and formulas

[Aaron Lauterer]

instead of relying purely on listeners that then manually change other
components, we can use binds, formulas and a basic controller.

This makes it quite a bit easier to let multiple components react to
changes.

A cbind is used for the size component to set the initial start value.
Other options, like using setValue in the controller init, will trigger
the change listener and therefore can affect the min size without any
user interaction.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---

I left the 'show....Warning' as is. They are also used in the
'minSizeLabel' formula and I prefer to have them there in a non-negated
form.

changes since v1:
* moved view between controller and layout
* small code style cleanups

 www/manager6/ceph/Pool.js | 83 ++++++++++++++++++++++++++++-----------
 1 file changed, 59 insertions(+), 24 deletions(-)

diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js
index e155a731..d511b1a4 100644
--- a/www/manager6/ceph/Pool.js
+++ b/www/manager6/ceph/Pool.js
@@ -12,6 +12,48 @@ Ext.define('PVE.CephPoolInputPanel', {
     defaultSize: undefined,
     defaultMinSize: undefined,
 
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	init: function(view) {
+	    let vm = this.getViewModel();
+	    vm.set('size', view.defaultSize ? Number(view.defaultSize) : 3);
+	    vm.set('minSize', view.defaultMinSize ? Number(view.defaultMinSize) : 2);
+	},
+	sizeChange: function(field, val) {
+	    let vm = this.getViewModel();
+	    let minSize = Math.round(val / 2);
+	    if (minSize > 1) {
+		vm.set('minSize', minSize);
+	    }
+	    vm.set('size', val); // bind does not work in a pmxDisplayEditField, update manually
+	},
+    },
+
+    viewModel: {
+	data: {
+	    minSize: null,
+	    size: null,
+	},
+	formulas: {
+	    minSizeLabel: (get) => {
+		if (get('showMinSizeOneWarning') || get('showMinSizeHalfWarning')) {
+		    return `${gettext('Min. Size')} <i class="fa fa-exclamation-triangle warning"></i>`;
+		}
+		return gettext('Min. Size');
+	    },
+	    showMinSizeOneWarning: (get) => get('minSize') === 1,
+	    showMinSizeHalfWarning: (get) => {
+		let minSize = get('minSize');
+		let size = get('size');
+		if (minSize === 1) {
+		    return false;
+		}
+		return minSize < (size / 2) && minSize !== size;
+	    },
+	},
+    },
+
     column1: [
 	{
 	    xtype: 'pmxDisplayEditField',
@@ -39,12 +81,7 @@ Ext.define('PVE.CephPoolInputPanel', {
 		maxValue: 7,
 		allowBlank: false,
 		listeners: {
-		    change: function(field, val) {
-			let size = Math.round(val / 2);
-			if (size > 1) {
-			    field.up('inputpanel').down('field[name=min_size]').setValue(size);
-			}
-		    },
+		    change: 'sizeChange',
 		},
 	    },
 	},
@@ -82,10 +119,12 @@ Ext.define('PVE.CephPoolInputPanel', {
     advancedColumn1: [
 	{
 	    xtype: 'proxmoxintegerfield',
-	    fieldLabel: gettext('Min. Size'),
+	    bind: {
+		fieldLabel: '{minSizeLabel}',
+		value: '{minSize}',
+	    },
 	    name: 'min_size',
 	    cbind: {
-		value: (get) => get('defaultMinSize') ?? 2,
 		minValue: (get) => {
 		    if (Number(get('defaultMinSize')) === 1) {
 			return 1;
@@ -96,28 +135,24 @@ Ext.define('PVE.CephPoolInputPanel', {
 	    },
 	    maxValue: 7,
 	    allowBlank: false,
-	    listeners: {
-		change: function(field, minSize) {
-		    let panel = field.up('inputpanel');
-		    let size = panel.down('field[name=size]').getValue();
-
-		    let showWarning = minSize < (size / 2) && minSize !== size;
-
-		    let fieldLabel = gettext('Min. Size');
-		    if (showWarning) {
-			fieldLabel = gettext('Min. Size') + ' <i class="fa fa-exclamation-triangle warning"></i>';
-		    }
-		    panel.down('field[name=min_size-warning]').setHidden(!showWarning);
-		    field.setFieldLabel(fieldLabel);
-		},
-	    },
 	},
 	{
 	    xtype: 'displayfield',
-	    name: 'min_size-warning',
+	    bind: {
+		hidden: '{!showMinSizeHalfWarning}',
+	    },
+	    hidden: true,
 	    userCls: 'pmx-hint',
 	    value: gettext('min_size < size/2 can lead to data loss, incomplete PGs or unfound objects.'),
+	},
+	{
+	    xtype: 'displayfield',
+	    bind: {
+		hidden: '{!showMinSizeOneWarning}',
+	    },
 	    hidden: true,
+	    userCls: 'pmx-hint',
+	    value: gettext('a min_size of 1 is not recommended and can lead to data loss'),
 	},
 	{
 	    xtype: 'pmxDisplayEditField',
-- 
2.30.2

[pve-devel] [PATCH v2 follow up manager 3/5] api: ceph: add endpoint to fetch config keys

[Aaron Lauterer]

This new endpoint allows to get the values of config keys that are
either set in the config db or the ceph.conf file.

Values that are set in the ceph.conf file have priority over values set
in the conifg db via 'ceph config set'.

Expects the --config-keys parameter as a semicolon separated list of
"<section>:<config key>" where the section is a section in the ceph.conf
or config db. For example: global:osd_pool_default_size

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---

I found a small code style issue that was present since the initial
patch. The diff to the original v2 to this follow up is:

--- a/PVE/API2/Ceph/Cfg.pm
+++ b/PVE/API2/Ceph/Cfg.pm
@@ -172,7 +172,7 @@ __PACKAGE__->register_method ({
        my $rados = PVE::RADOS->new();
        my $configdb = $rados->mon_command( { prefix => 'config dump', format => 'json' });
        for my $s (@{$configdb}) {
-           my ($section, $name, $value) = $s ->@{'section', 'name', 'value'};
+           my ($section, $name, $value) = $s->@{'section', 'name', 'value'};
            my $n_section = $normalize->($section);
            my $n_name = $normalize->($name);

 PVE/API2/Ceph/Cfg.pm | 81 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/PVE/API2/Ceph/Cfg.pm b/PVE/API2/Ceph/Cfg.pm
index a00ef19c..6d05db9c 100644
--- a/PVE/API2/Ceph/Cfg.pm
+++ b/PVE/API2/Ceph/Cfg.pm
@@ -40,6 +40,7 @@ __PACKAGE__->register_method ({
 	my $result = [
 	    { name => 'raw' },
 	    { name => 'db' },
+	    { name => 'value' },
 	];
 
 	return $result;
@@ -114,3 +115,83 @@ __PACKAGE__->register_method ({
 
 	return $res;
     }});
+
+
+my $SINGLE_CONFIGKEY_RE = qr/[0-9a-z\-_\.]+:[0-9a-zA-Z\-_]+/i;
+my $CONFIGKEYS_RE = qr/^(:?${SINGLE_CONFIGKEY_RE})(:?[;, ]${SINGLE_CONFIGKEY_RE})*$/;
+
+__PACKAGE__->register_method ({
+    name => 'value',
+    path => 'value',
+    method => 'GET',
+    proxyto => 'node',
+    protected => 1,
+    permissions => {
+	check => ['perm', '/', [ 'Sys.Audit' ]],
+    },
+    description => "Get configured values from either the config file or config DB.",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    'config-keys' => {
+		type => "string",
+		typetext => "<section>:<config key>[;<section>:<config key>]",
+		pattern => $CONFIGKEYS_RE,
+		description => "List of <section>:<config key> items.",
+	    }
+	},
+    },
+    returns => {
+	type => 'object',
+	description => "Contains {section}->{key} children with the values",
+    },
+    code => sub {
+	my ($param) = @_;
+
+	PVE::Ceph::Tools::check_ceph_inited();
+
+	# Ceph treats '-' and '_' the same in parameter names, stick with '-'
+	my $normalize = sub {
+	    my $t = shift;
+	    $t =~ s/_/-/g;
+	    return $t;
+	};
+
+	my $requested_keys = {};
+	for my $pair (PVE::Tools::split_list($param->{'config-keys'})) {
+	    my ($section, $key) = split(":", $pair);
+	    $section = $normalize->($section);
+	    $key = $normalize->($key);
+
+	    $requested_keys->{$section}->{$key} = 1;
+	}
+
+	my $config = {};
+
+	my $rados = PVE::RADOS->new();
+	my $configdb = $rados->mon_command( { prefix => 'config dump', format => 'json' });
+	for my $s (@{$configdb}) {
+	    my ($section, $name, $value) = $s->@{'section', 'name', 'value'};
+	    my $n_section = $normalize->($section);
+	    my $n_name = $normalize->($name);
+
+	    $config->{$n_section}->{$n_name} = $value
+		if defined $requested_keys->{$n_section} && $n_name eq $n_name;
+	}
+
+	# read ceph.conf after config db as it has priority if settings are present in both
+	my $config_file = cfs_read_file('ceph.conf');
+	for my $section (keys %{$config_file}) {
+	    my $n_section = $normalize->($section);
+	    next if !defined $requested_keys->{$n_section};
+
+	    for my $key (keys %{$config_file->{$section}}) {
+		my $n_key = $normalize->($key);
+		$config->{$n_section}->{$n_key} = $config_file->{$section}->{$key}
+		    if $requested_keys->{$n_section}->{$n_key};
+	    }
+	}
+
+	return $config;
+    }});
-- 
2.30.2

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

[Fabian Grünbichler]

On March 16, 2023 2:48 pm, 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>

other than the nit below, patches 1 & 2:

Acked-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

> ---
> 
> changes since v1:
> * add this commit to rework the API
> 
>  PVE/API2/Ceph.pm       |  15 ++++--
>  PVE/API2/Ceph/Cfg.pm   | 116 +++++++++++++++++++++++++++++++++++++++++
>  PVE/API2/Ceph/Makefile |   1 +
>  3 files changed, 129 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..a00ef19c
> --- /dev/null
> +++ b/PVE/API2/Ceph/Cfg.pm
> @@ -0,0 +1,116 @@
> +package PVE::API2::Ceph::Cfg;
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Ceph::Tools;
> +use PVE::Cluster qw(cfs_read_file cfs_write_file);

nit: not used by this patch, but only by later ones

maybe also a good time to evalute whether ceph.conf should be read via
cfs_read_file or file_get_contents, there seems to be some disagreement in this
series ;)

> +use PVE::JSONSchema qw(get_standard_option);
> +use PVE::RADOS;
> +use PVE::Tools qw(run_command file_get_contents file_set_contents extract_param);
> +
> +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
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>