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.laute...@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
