[pve-devel] [PATCH manager v3 10/20] ceph/destroypool: optionally remove storages

[Thomas Lamprecht]

On 08/31/2017 11:38 AM, Fabian Grünbichler wrote:
> only storages which have the 'pveceph' flag set are removed
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> ---
> changes since v2:
> - adapted for $get_storages changes
> - inlined $remove_storage
> 
> changes since v1:
> - die if any of the storages could not be removed
> - move storage retrieval into lower if (refactoring was dropped in v2)
> 
>   PVE/API2/Ceph.pm | 28 +++++++++++++++++++++++++++-
>   1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
> index a8ea2b39..5620d6b3 100644
> --- a/PVE/API2/Ceph.pm
> +++ b/PVE/API2/Ceph.pm
> @@ -1820,7 +1820,13 @@ __PACKAGE__->register_method ({
>   		type => 'boolean',
>   		optional => 1,
>   		default => 0,
> -	    }
> +	    },
> +	    remove_storages => {
> +		description => "Remove all pveceph-managed storages configured for this pool",
> +		type => 'boolean',
> +		optional => 1,
> +		default => 0,
> +	    },
>   	},
>       },
>       returns => { type => 'null' },
> @@ -1829,7 +1835,13 @@ __PACKAGE__->register_method ({
>   
>   	PVE::CephTools::check_ceph_inited();
>   
> +	my $rpcenv = PVE::RPCEnvironment::get();
> +	my $user = $rpcenv->get_user();
> +	$rpcenv->check($user, '/storage', ['Datastore.Allocate'])
> +	    if $param->{remove_storages};
> +

possible nitpick: in the former patch in the create API call you use an
if block over this whole check, i.e. only declaring $rpcenv and $user
maybe use that here too? a) for consistency and b) the other one is easier
to read, IMO :)

>   	my $pool = $param->{name};
> +	my $storages = $get_storages->($pool);
>   
>   	# if not forced, destroy ceph pool only when no
>   	# vm disks are on it anymore
> @@ -1857,6 +1869,20 @@ __PACKAGE__->register_method ({
>   	    format => 'plain',
>           });
>   
> +	if ($param->{remove_storages}) {
> +	    my $err;
> +	    foreach my $storeid (keys %$storages) {
> +		next if !$storages->{$storeid}->{pveceph};
> +		eval { PVE::API2::Storage::Config->delete({storage => $storeid}) };
> +		if ($@) {
> +		    warn "failed to remove storage '$storeid': $@\n";
> +		    $err = 1;
> +		}
> +	    }
> +	    die "failed to remove (some) storages - check log and remove manually!\n"
> +		if $err;
> +	}
> +
>   	return undef;
>       }});
>   
>