From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 87BA31FF17C
for <inbox@lore.proxmox.com>; Wed, 2 Apr 2025 16:14:59 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 4FBAA1C54E;
Wed, 2 Apr 2025 16:14:47 +0200 (CEST)
Message-ID: <85eeb1e7-2e78-46a4-81ed-c3ab5f850a36@proxmox.com>
Date: Wed, 2 Apr 2025 16:14:43 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: pve-devel@lists.proxmox.com, Aaron Lauterer <a.lauterer@proxmox.com>
References: <20241223160008.218710-1-a.lauterer@proxmox.com>
<20241223160008.218710-2-a.lauterer@proxmox.com>
Content-Language: en-US
From: Friedrich Weber <f.weber@proxmox.com>
In-Reply-To: <20241223160008.218710-2-a.lauterer@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.010 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [pool.pm]
Subject: Re: [pve-devel] [PATCH manager v2 1/6] api: ceph: add rbd namespace
management endpoints
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
Hi, I have some minor comments inline:
On 23/12/2024 17:00, Aaron Lauterer wrote:
> RBD supports namespaces. To make the management easier and possible via
> the web UI, we need to add API endpoints to:
> * list
> * create
> * delete
> namespaces.
>
> We only allow creatng namespaces for pools that have the RBD application
> set.
>
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> changes since v1:
> * integrated recommendations:
> * code style in many places
> * added check on removal if a storage config that uses the namespace
> exists. I can be overriden with the new optional force flag
> * on create we trim the namespace parameter and check if it has any
> length. Is there an option in the API that would do that for us
> already?
> * added TODO note as we might want to rethink the ACL privileges
>
> PVE/API2/Ceph/Pool.pm | 205 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 203 insertions(+), 2 deletions(-)
>
> diff --git a/PVE/API2/Ceph/Pool.pm b/PVE/API2/Ceph/Pool.pm
> index 5ee982f4..94121dea 100644
> --- a/PVE/API2/Ceph/Pool.pm
> +++ b/PVE/API2/Ceph/Pool.pm
> @@ -3,6 +3,8 @@ package PVE::API2::Ceph::Pool;
> use strict;
> use warnings;
>
> +use JSON qw(decode_json);
> +
> use PVE::Ceph::Tools;
> use PVE::Ceph::Services;
> use PVE::JSONSchema qw(get_standard_option parse_property_string);
> @@ -10,7 +12,7 @@ use PVE::RADOS;
> use PVE::RESTHandler;
> use PVE::RPCEnvironment;
> use PVE::Storage;
> -use PVE::Tools qw(extract_param);
> +use PVE::Tools qw(extract_param run_command);
>
> use PVE::API2::Storage::Config;
>
> @@ -32,6 +34,7 @@ my $get_autoscale_status = sub {
> return $data;
> };
>
> +# TODO: think about adding dedicated Ceph ACL privileges as the currently used ones don't match well
>
> __PACKAGE__->register_method ({
> name => 'lspools',
> @@ -302,7 +305,7 @@ my $ceph_pool_common_options = sub {
>
>
> my $add_storage = sub {
> - my ($pool, $storeid, $ec_data_pool) = @_;
> + my ($pool, $storeid, $ec_data_pool, $namespace) = @_;
>
> my $storage_params = {
> type => 'rbd',
> @@ -312,6 +315,8 @@ my $add_storage = sub {
> content => 'rootdir,images',
> };
>
> + $storage_params->{namespace} = $namespace if $namespace;
> +
> $storage_params->{'data-pool'} = $ec_data_pool if $ec_data_pool;
>
> PVE::API2::Storage::Config->create($storage_params);
> @@ -798,4 +803,200 @@ __PACKAGE__->register_method ({
> }});
>
>
> +my $get_rbd_namespaces = sub {
> + my ($pool) = @_;
> +
> + my $raw = '';
> + run_command(
> + ['/usr/bin/rbd', 'namespace', 'list', $pool, '--format', 'json'],
> + outfunc => sub { $raw .= shift },
> + errmsg => "rbd error",
> + errfunc => sub {},
> + );
> + return [] if $raw eq '[]';
> +
> + my ($untainted_raw) = $raw =~ /^(.+)$/; # untaint
> + my $namespaces = eval { decode_json($untainted_raw) };
> + die "failed to parse as JSON - $@\n" if $@;
> +
> + return [
> + map { { name => $_->{name} } } $namespaces->@*
> + ];
> +};
> +
> +__PACKAGE__->register_method ({
> + name => 'listnamespaces',
> + path => '{name}/namespace',
> + method => 'GET',
> + permissions => {
> + check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
> + },
> + description => "Get pool RBD namespaces.",
> + proxyto => 'node',
> + protected => 1,
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + name => {
> + description => 'The name of the pool.',
> + type => 'string',
> + default => 'rbd',
I was a bit confused about this default -- seeing that {name} is part of
the path, is there a situation where we can fallback to 'rbd'?
> + },
> + },
> + },
> + returns => {
> + type => 'array',
> + items => {
> + type => 'object',
> + properties => {
> + name => { type => 'string', title => "Namespace" }
> + },
> + },
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + my $pool = extract_param($param, 'name') // 'rbd';
Do we need this fallback?
> + return $get_rbd_namespaces->($pool);
> + }});
> +
> +
> +__PACKAGE__->register_method ({
> + name => 'createnamespace',
> + path => '{name}/namespace',
> + method => 'POST',
> + permissions => {
> + check => ['perm', '/', [ 'Sys.Modify' ]],
> + },
> + description => "Create new RBD namespace.",
> + proxyto => 'node',
> + protected => 1,
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + name => {
> + description => 'The name of the pool.',
> + type => 'string',
> + default => 'rbd',
same as above
> + },
> + namespace => {
> + description => 'The name of the new namespace',
> + type => 'string',
> + },
> + 'add-storage' => {
> + description => "Configure VM and CT storage using the new namespace.",
> + type => 'boolean',
> + optional => 1,
> + default => "0",
> + },
> + },
> + },
> + returns => { type => 'string' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $pool = extract_param($param, 'name') // 'rbd';
same as above
> + my $namespace = PVE::Tools::trim(extract_param($param, 'namespace'));
> + my $add_storages = extract_param($param, 'add-storage');
> +
> + die "namespace cannot be empty" if length($namespace) < 1; # in case it was just whitespace
missing a \n at the end
> +
> + my $rpcenv = PVE::RPCEnvironment::get();
> + my $user = $rpcenv->get_user();
> + if ($add_storages) {
> + $rpcenv->check($user, '/storage', ['Datastore.Allocate']);
> + die "namespace name contains characters which are illegal for storage naming\n"
> + if !PVE::JSONSchema::parse_storage_id("${pool}-${namespace}");
> + }
> +
> + my $rados = PVE::RADOS->new();
> + my $apps = $rados->mon_command({ prefix => "osd pool application get", pool => "$pool", });
> + die "the pool does not have application 'rbd' enabled" if !defined($apps->{rbd});
missing a \n at the end
> +
> + my $current_namespaces = { map { $_->{name} => 1 } $get_rbd_namespaces->($pool)->@*};
> + die "namespace already exists" if $current_namespaces->{$namespace};
missing a \n at the end
> +
> + my $worker = sub {
> + run_command(
> + ['/usr/bin/rbd', 'namespace', 'create', "${pool}/${namespace}"],
> + errmsg => "rbd create namespace error",
> + errfunc => sub {},
> + outfunc => sub {},
> + );
> + if ($add_storages) {
> + eval { $add_storage->($pool, "${pool}-${namespace}", 0, $namespace) };
> + die "adding PVE storage for ceph rbd namespace failed: pool: ${pool}, namespace: ${namespace}: $@\n" if $@;
> + }
> + };
> +
> + return $rpcenv->fork_worker('cephcreaterbdnamespace', $pool, $user, $worker);
> + }});
> +
> +
> +__PACKAGE__->register_method ({
> + name => 'destroynamespace',
> + path => '{name}/namespace',
> + method => 'DELETE',
> + permissions => {
> + check => ['perm', '/', [ 'Sys.Modify' ]],
> + },
> + description => "Delete RBD namespace.",
> + proxyto => 'node',
> + protected => 1,
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + name => {
> + description => 'The name of the pool.',
> + type => 'string',
> + default => 'rbd',
> + },
> + namespace => {
> + description => 'The name of the namespace',
> + type => 'string',
> + },
> + force => {
> + description => 'Force removal of the namespace',
> + type => 'boolean',
> + optional => 1,
> + },
> + },
> + },
> + returns => { type => 'string' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $pool = extract_param($param, 'name') // 'rbd';
> + my $namespace = extract_param($param, 'namespace');
> + my $force = extract_param($param, 'force');
> +
> + if (!$force) {
> + my $storages = $get_storages->($pool);
> + for my $storage (keys $storages->%*) {
> + if (
> + defined($storages->{$storage}->{namespace})
> + && $storages->{$storage}->{namespace} eq $namespace
> + ) {
> + die "namespace '${namespace}' is configured in storage '${storage}', remove it first";
missing a \n at the end
> + }
> + }
> + }
> +
> + my $rpcenv = PVE::RPCEnvironment::get();
> + my $user = $rpcenv->get_user();
> + my $worker = sub {
> + run_command(
> + ['/usr/bin/rbd', 'namespace', 'remove', "${pool}/${namespace}"],
> + errmsg => "rbd create namespace error",
*remove
> + errfunc => sub {},
> + outfunc => sub {},
> + );
> + };
> +
> + return $rpcenv->fork_worker('cephdestroyrbdnamespace', $pool, $user, $worker);
> + }});
> +
> 1;
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel