On 08/29/2017 01:04 PM, Fabian Grünbichler wrote:
this patch series implements storage.cfg management for pveceph-managed ceph
clusters. the following is implemented:
- add new 'pveceph' flag to RBD storages, using /etc/pve/ceph.conf instead of
a hard-coded monitor list
- pveceph addstorage/lsstorages/removestorage to add/list/remove storage
entries, per pool
after rethinking the higher level changes introduced here, especially the API
ones,
I'm not sure if that is the approach we want to go with.
API:
We duplicate semantic behavior and add another endpoint which can add remove
and list
storages at the /nodes/{node}/ceph/pools{pool}/storages path.
This goes a bit against our single /storage entry point, you can create a
storage with
the latter and remove it with the former – and with this series you must remove
it with
the former if you do not touch the CLI tools – but the behavior is different.
Also, AFAIK, the storage system in PVE was designed to be at a cluster level,
not node
level.
I think the CLI tools and their commands added would be OK, though.
But the API stuff should really go into /storages, IMO, this would be cleaner
API design.
Adding a pool to a storage over this path seems unfitting. And if we ever would
like to have
a separate Storage Management project, splitting this out would be quite harder
with such a
coupling. IMO, /storages should be there for managing the storage configuration
and
GUI:
Initially just the naming of the button was what took me off, but after
re-thinking this
the "Add as Storage" (named "Create Storage" in the current revision of this
series)
does not fit in the Node->Ceph->Pool panel.
You do not manage storages here, you do not see a clear effect after the this
Creation
window was submitted. Normally we only add/create/remove elements in the GUI
which are
on the same Panel visible – i.e. we have a grid with those elements.
Here we have a grid with pools but can configure a storage, where you get no
direct
feedback of said addition. You could argue that it pops up in the resource
tree, but that
may not be visible in the non-trivial setup case (too much VMs/CTs, another
tree view used,
tree collapsed).
My proposal about how the GUI could incorporate easier pveceph pool is to use
the now
existing "Add RBD" dialog from Datacenter->Storage and make the ID window an
editable
combo box, which lists all pools configured by pveceph and allows specifying an
external
one. Respective to the selction a "pveceph" boolean flag (or therelike) can be
passed.
The "Add Storages" from the Pool creation window could stay, but should use a new
"storage"
parameter in the create ceph/pool API path.
That's how we do it for assigning a newly created VM/CT to a PVE pool, as it
the cleaner
approach, IMO.
API wise I would put into the RBD Storage plugin, currently that should be no
problem,
as only the keyring part needs to be handled? If we need to do more here I'd
rather
choose the way of moving our PVE::Ceph perl modules higher up (into own
package, e.g.)
and use them also there?
Just my thoughts to this.
- optionally adding/removing storages when creating/destroying a pool
- GUI support
high-level changes since v1:
- pveceph flag is now doing its magic on the storage plugin side as well, no
more need to update the monhost string when adding/removing a monitor (thanks
for the suggestion Dietmar!)
- createpool/destroypool are now using a worker
- treat more failures as fatal in API paths
- {name} API placeholder for pool names is now {pool}
- includes and extends Dominik's GUI patches to support the new functionality
patches 2-4, 17, 19-21, 26 and 27 are new
patches 22-25 are from Dominik's 'ceph pool storage creation in gui' series
the old patches 2, 3, 5, 8 and 20 were dropped altogether
pve-storage:
Fabian Grünbichler (4):
rbd: add pveceph storage option
rbd: make monhost option optional
refactor cmdline helpers
rbd: implement pveceph flag handling
PVE/Storage/RBDPlugin.pm | 90 ++++++++++++++++++++++++++++--------------------
1 file changed, 53 insertions(+), 37 deletions(-)
base-commit: 5a2eba91dcaec0ba68052579d5e0e4ebcb499ff5
pve-manager:
Dominik Csapak (4):
add a params object to the safedestroy window
add create storages checkbox to ceph pool creation
add remove_storages parameter to the pool destruction
add a 'create storage' button for ceph pools
Fabian Grünbichler (19):
ceph: add /etc/pve/priv/ceph path
ceph: add add_storage helper
ceph: add get_storages helper
ceph: add remove_storage helper
ceph: implement addstorage API path
pveceph: add addstorage CLI command
ceph: implement lsstorages API path
pveceph: add lsstorages CLI command
ceph: implement removestorage API path
pveceph: add removestorage CLI command
ceph/createpool: shorten pool variable name
ceph/createpool: optionally add storages
ceph/destroypool: shorten pool variable name
ceph/destroypool: optionally remove storages
ceph: rename API placeholder 'name' to 'pool'
ceph: make create/destroypool API paths async
add task description for cephcreatepool
add showProgress to SafeDestroy
enable showProgress for create/destroy pool
PVE/API2/Ceph.pm | 367 +++++++++++++++++++++++++++++++------
PVE/CLI/pveceph.pm | 24 ++-
PVE/CephTools.pm | 2 +
www/manager6/Utils.js | 1 +
www/manager6/ceph/Pool.js | 78 +++++++-
www/manager6/window/SafeDestroy.js | 41 ++++-
6 files changed, 455 insertions(+), 58 deletions(-)
base-commit: f9346961588e086a9cc86d00c16f7f3d0a77c878
_______________________________________________
pve-devel mailing list
[email protected]
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
