Use librbd for rbd management in ceph-integration #273
Conversation
|
@r0h4n @shtripat @nthomas-redhat please review |
|
verified |
GowthamShanmugam
force-pushed
the
librbd
branch
2 times, most recently
from
1268128
to
23a0184
Compare
tendrl/ceph_integration/ceph.py
Outdated
| try: | ||
| if operation == "create": | ||
| rbd_inst = rbd.RBD() | ||
| rbd_inst.create(ioctx, attr["name"], int(attr["size"])) |
There was a problem hiding this comment.
Here RBD creation function takes size as byte so from UI they have to pass rbd size in bytes
e.g
for 1gb they have to pass like 1073741824 bytes
There was a problem hiding this comment.
@GowthamShanmugam you should mention the UI folks if there's some assumption like above
cc: @Tendrl/tendrl_frontend
There was a problem hiding this comment.
@GowthamShanmugam you can convert the size to bytes in backend wherever rbd_operation is called.
There was a problem hiding this comment.
ok i will convert this
|
@r0h4n solved |
tendrl/ceph_integration/ceph.py
Outdated
| @@ -10,11 +10,15 @@ | |||
| import sys | |||
| import tempfile | |||
| import time | |||
| import rbd | |||
There was a problem hiding this comment.
python rbd needs to be added to dependencies?
tendrl/ceph_integration/ceph.py
Outdated
| try: | ||
| if operation == "create": | ||
| rbd_inst = rbd.RBD() | ||
| rbd_inst.create(ioctx, attr["name"], int(attr["size"])) |
There was a problem hiding this comment.
@GowthamShanmugam you can convert the size to bytes in backend wherever rbd_operation is called.
tendrl/ceph_integration/ceph.py
Outdated
| command_args | ||
| def rbd_operation(cluster_name, operation, attr, pool_name, result, queue): | ||
| import rados | ||
| cluster = rados.Rados(name=RADOS_NAME, |
There was a problem hiding this comment.
Remove all rados.Rados() calls, use this file for reference
using above way you can simply do "with ClusterHandle(cluster_name) as cluster_handle"
tendrl/ceph_integration/ceph.py
Outdated
| try: | ||
| ioctx = cluster.open_ioctx(pool_name) | ||
| try: | ||
| if operation == "create": |
There was a problem hiding this comment.
Why are you implementing Create, Delete, Update as part of this issue?
cc: @shtripat
There was a problem hiding this comment.
because we have to call a particular function
create have separate rbdlib func
update have separate function
tendrl/ceph_integration/ceph.py
Outdated
| image.close() | ||
| if operation == "usage": | ||
| args = "du --image %s" % attr['name'] | ||
| cmd_args = "rbd --pool %s --cluster %s " % (pool_name, cluster_name) + \ |
There was a problem hiding this comment.
cant get this data from librbd? why are you using the rbd command again
There was a problem hiding this comment.
Ah this is because i have checked all librbd func so no function si giving used detail so i have discussed this with @shtripat also so only used value i am using this
There was a problem hiding this comment.
But do we really need a usage operation here? In sds_sync we are already having logic to figure this.
Lets keep CRUD only to use librbd. Utilization still would be using command in sds_sync.
There was a problem hiding this comment.
Also I would like lib_rbd stuff to be inside a separate source say librbd_utils.py and invoke function from ceph.py
tendrl/ceph_integration/ceph.py
Outdated
| result['out'] = '' | ||
| # Exchanging objects between processes | ||
| queue = multiprocessing.Queue() | ||
| p = multiprocessing.Process( |
There was a problem hiding this comment.
why do you need a multiprocessing?
There was a problem hiding this comment.
i will change that to gevent, multiprocessing timout is works like it wait unit process finish if the process finished before the specified time it automatically stops wait and executes further. that why i used this.
any way i will changed to gevent
tendrl/ceph_integration/ceph.py
Outdated
|
|
||
| finally: | ||
| cluster.shutdown() | ||
| queue.put(result) |
There was a problem hiding this comment.
is this even gonna work? where have you declared the "queue" variable?
There was a problem hiding this comment.
it will work def rbd_operation(cluster_name, operation, attr, pool_name, result, queue):
queue is used to Exchanging objects between processes
from multiprocessing import Queue
Signed-off-by: GowthamShanmugam <[email protected]>
| @@ -0,0 +1,106 @@ | |||
| import multiprocessing | |||
There was a problem hiding this comment.
@r0h4n i have tried with gevent, problem with gevent is when cluster goes to error state then all librbd function call are not responding so i tried to timeout that function using gevent but gevent can't time out that subprocess. Because gevent required gevent.sleep in sub process function but librbd is built in library so we can't use it. i have tried all the option using gevent but it is not work for this case. Multi-process can timeout the the subprocess in this case.
| def rbd_operation(cluster_name, attributes): | ||
| err = None | ||
| p = None | ||
| queue = multiprocessing.Queue() |
There was a problem hiding this comment.
@r0h4n i am using multiprocessing for sub-process so i used multiprocessing queue
|
|
||
| attributes = {} | ||
| attributes['pool_name'] = self._resolve_pool(pool_id)['pool_name'] | ||
| attributes['operation'] = 'delete' |
There was a problem hiding this comment.
attributes['operation'] = 'delete' to call specific function in librbd_util.py
| @@ -164,8 +164,8 @@ namespace.ceph: | |||
| mandatory: | |||
| - Rbd.name | |||
| - Rbd.size | |||
| optional: | |||
| - Rbd.pool_id | |||
There was a problem hiding this comment.
Rbd.pool_id is must because librbd requires pool_name compulsory, so if we pass pool_id then only pool_name is identified and passed.
| import multiprocessing | ||
| import os | ||
| import rados | ||
| import rbd |
There was a problem hiding this comment.
@r0h4n ceph already have rbd as dependency
tendrl-bug-id: #150
Signed-off-by: GowthamShanmugam [email protected]