New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ceph-mgr: Implement new pecan-based rest api #14457

Merged
merged 14 commits into from May 31, 2017

Conversation

Projects
None yet
@b-ranto
Contributor

b-ranto commented Apr 11, 2017

The new rest API uses Flask with flask-restful extension and simplifies
the code significantly. It should be mostly equivalent in functionality
to the django-based rest API.

The new API uses Ceph auth system (requires caps mon allow *) and is
self-documenting via /doc endpoint.

Signed-off-by: Boris Ranto branto@redhat.com

@b-ranto b-ranto requested a review from jcsp Apr 11, 2017

@b-ranto b-ranto changed the title from ceph-mgr: Implement new Flask-based rest api to [DNM] ceph-mgr: Implement new Flask-based rest api Apr 11, 2017

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 11, 2017

Initial comments (sorry if this is all a bit critical, such is the nature of code review...)

When someone authenticates by CephX key, you're sending a remote command to the mon for every single HTTP request -- that's adding latency to the requests and adding load to the mons. I imagine you're hoping that users will be well behaved and use the "token" mechanism most of the time -- I think that is quite optimistic. IMHO if a user is going to require cephx credentials to initially create/destroy tokens, then that initial config might as well just be a CLI thing, and avoid the risk of users just using cephx keys for everything.

Encouraging people to send CephX tokens to an unauthenticated host is also just plain bad security. While the connection is SSL, it is shipping with a self-signed certificate, so there is no authentication.

I can't see where the logic for modifying pg_num/pgp_num went (the part where we increase one first and then gradually increase the other). One of the main reasons I wrote that for Calamari back in the day was an example of how API operations weren't always trivial things, and sometimes they had multiple steps -- the structure that enabled that should persist.

There doesn't seem to be any update to the ubuntu/debian packaging, are the dependencies present in Xenial?

I hope this doesn't seem too picky, but please can we avoid picking deliberately obscure names. This module should probably just be called rest, or if we're going to have the old one co-existing, it should be called rest2 or rest-flask or something like that.

Exposing documentation on /doc is a neat, but exposing it on docs.ceph.com is much more important, is there a path to doing that?

If we're going to the trouble of having a brand new module, let's have some documentation to go with it, at least demonstrating how to authenticate.

return " ".join(out)
def HUMANIFY_REQUEST(request):

This comment has been minimized.

@jcsp

jcsp Apr 11, 2017

Contributor

Any reason this isn't a method on CommandsRequest?

This comment has been minimized.

@b-ranto

b-ranto Apr 11, 2017

Contributor

It was on a TODO list, looks like I should have written it down. :)

# Transform command to a human readable form
def HUMANIFY_COMMAND(command):

This comment has been minimized.

@jcsp

jcsp Apr 11, 2017

Contributor

Function names should be lower_case

This comment has been minimized.

@b-ranto

b-ranto Apr 11, 2017

Contributor

This was from back then when I used 'from common import *' and I used upper case to identify the bits from the common file. I will change that.

@staticmethod
def run(commands, uuid=str(uuid4())):

This comment has been minimized.

@jcsp

jcsp Apr 11, 2017

Contributor

This currently gets self.uuid passed in always, so no default needed, and instead of a static method that takes a uuid argument, this could be a class method that just uses self.uuid.

This comment has been minimized.

@b-ranto

b-ranto Apr 11, 2017

Contributor

True, it is a relict from one of the older ideas I had which I deprecated since then.

try:
results = self.run(commands_arrays[0], self.uuid)
except:
instance.log.error(str(traceback.format_exc()))

This comment has been minimized.

@jcsp

jcsp Apr 11, 2017

Contributor

This should mark itself failed at this stage, presumably.

This comment has been minimized.

@b-ranto

b-ranto Apr 11, 2017

Contributor

Looks like I forgot to remove this debugging try: except: statement which should no longer be necessary (I have previously had some issues with logging from different places so I had to catch this early but it is no longer necessary).

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Apr 11, 2017

When someone authenticates by CephX key, you're sending a remote command to the mon for every single HTTP request -- that's adding latency to the requests and adding load to the mons. I imagine you're hoping that users will be well behaved and use the "token" mechanism most of the time -- I think that is quite optimistic. IMHO if a user is going to require cephx credentials to initially create/destroy tokens, then that initial config might as well just be a CLI thing, and avoid the risk of users just using cephx keys for everything.

Yep, I was hoping the users will use tokens when necessary. To create a persistent token all you need to do is to visit the /auth node while being authenticated by a CephX key. That will generate a persistent token (well, until you delete it with delete method or refresh it by visiting the /auth endpoint again).

You can access this easily via browser, since it sends back 401 with www-authenticate, the browser will prompt for the username/password. From python, all you need to do is add auth=('user', 'password') to the request method, i.e.
requests.get(<url>, auth=('client.admin', <key>)
or if you are using a token
requests.get(<url>, auth=(<token>, '')

btw: As far as speed goes, I have tested this in my VM with 1500 authenticated requests and it took 13.5 seconds when sending remote command for auth each time compared to about 10 seconds when using tokens.

Encouraging people to send CephX tokens to an unauthenticated host is also just plain bad security. While the connection is SSL, it is shipping with a self-signed certificate, so there is no authentication.

I would not exactly call the host unauthenticated. It is running on a monitor and that is a host that is under the control of a Ceph admin. Also, paranoid admins can use their own keys for this and they can do the authentication properly. The packaging changes do not overwrite the existing keys on purpose. That being said, I admit I did not spend that much time on this part and I would be happy to do this better. I would e.g. like it if all the nodes did have the same cert/key, have some mechanism to verify the self-signed keys, etc.

btw: One of the reasons I wanted to use the CephX keys was that I was hoping we could automate the initial need for creation of ceph-mgr keys and we could use the key provided by a user to authenticate ceph-mgr at a later point and only then, it would start serving requests. Although I did not look into how easy/difficult that would be, yet.

I can't see where the logic for modifying pg_num/pgp_num went (the part where we increase one first and then gradually increase the other). One of the main reasons I wrote that for Calamari back in the day was an example of how API operations weren't always trivial things, and sometimes they had multiple steps -- the structure that enabled that should persist.

The submit_request method accepts list of lists. The pg_num is changed in the first iteration and only if that succeeds the command will change the pgp_num. I may have missed something there though. i.e. the request looks like

[[change pg_num, change stg, change stg_else], [change pgp_num]]

The request mechanism will run the commands in the first sub-list in parallel and once those are done it will run the commands in the second sublist (well, the one command). If you are saying gradually then maybe there should be more sub-lists?

There doesn't seem to be any update to the ubuntu/debian packaging, are the dependencies present in Xenial?

Sigh, I always forget about these. The ssl bits will definitely be there. I have just checked for the python-flask-restful extension on pkgs.org and it seems to be available in ubuntu xenial as well as debian sid.

I hope this doesn't seem too picky, but please can we avoid picking deliberately obscure names. This module should probably just be called rest, or if we're going to have the old one co-existing, it should be called rest2 or rest-flask or something like that.

Sure, the primary reason I went for an obscure name was that 'rest' was already taken and I did not want to do a PR that would patch the old rest directory as that would look like a total mess on github. It might be ok if I did that in two commits I suppose (one that removed the old rest api and one that added the flask-based one).

btw: My working name was neverest, I can switch to that if you like it better?

Exposing documentation on /doc is a neat, but exposing it on docs.ceph.com is much more important, is there a path to doing that?

If we're going to the trouble of having a brand new module, let's have some documentation to go with it, at least demonstrating how to authenticate.

I do plan to create a (html-formatted) snapshot of the /doc endpoint once the API stabilizes and document it on docs.ceph.com. No automated path for that, yet. :)

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 12, 2017

btw: As far as speed goes, I have tested this in my VM with 1500 authenticated requests and it took 13.5 seconds when sending remote command for auth each time compared to about 10 seconds when using tokens.

I'm not so concerned about the API performance as I am about it placing extra load on the mons (and also spamming the audit log). If someone is running e.g. a UI that's doing several requests every few seconds, it's unreasonable to be sending a command to mon for every one of those.

I really wouldn't rely on users to be well-behaved enough to switch auth methods to the more efficient tokens. If you just avoid giving them the cephx option to begin with, we don't have to worry.

Users/tools would have to go the CLI or their keyring file to get their cephx token to begin with, so it's probably not at all painful to just have them use the CLI to create their token for API access.

I would not exactly call the host unauthenticated. It is running on a monitor and that is a host that is under the control of a Ceph admin.

From the REST client's point of view, it is just talking to an IP address and hoping the thing responding is really the monitor. This is a "red address bar" situation, where we have a SSL connection but we are only hoping that the thing sending response packets is really the host we thought we were talking to. The host definitely is unauthenticated in the security sense of the word, there's no ambiguity about that.

Also, paranoid admins can use their own keys for this and they can do the authentication properly.

At the risk of sounding a bit preachy, we have a duty to ship things as secure as we can by default, not rely on admins to secure them after the fact.

Self-signed certificates are an unfortunate reality in some environments, but we can mitigate this by avoiding sending over-privileged secrets (like cephx keys) over that channel. I think there's also an argument that we should really wave this issue in the admin's face by requiring them to either load their own SSL key, or at least requiring them to manually type a command that tells them they are creating a self-signed certificate.

The request mechanism will run the commands in the first sub-list in parallel and once those are done it will run the commands in the second sublist (well, the one command). If you are saying gradually then maybe there should be more sub-lists?

Look at PgCreatingRequest. It needs to batch the creation of PGs and wait for them to be done before creating some more.

That reminds me of a more general point -- the commands in the current rest code (OsdMapModifyingRequest etc) will make sure the up-to-date cluster maps are loaded before considering the job complete. That's so that if someone does a POST and then a GET, the results reflect the change they just made. It could be reasonable to just not try and do that any more, but it is a change you are making that you should be aware of.

btw: My working name was neverest, I can switch to that if you like it better?

That's better but my preference would be for something that starts with "rest". See how ceph-devel feels about just replacing the old one outright. You need to make some effort to find out about anyone who is using the old one, to have the discussion about whether to kill it.

I do plan to create a (html-formatted) snapshot of the /doc endpoint once the API stabilizes and document it on docs.ceph.com. No automated path for that, yet. :)

Ah, good. Hopefully that will include some text explaining things, and an example piece of client code etc, not just a list of fields.

@liewegas liewegas added the mgr label Apr 12, 2017

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Apr 13, 2017

Latest changelog:

  • renamed to restful
  • the SSL key is generated at runtime (first node that starts generates it), not in packaging
  • the SSL key/cert is stored as a ceph config-key and shared between the nodes
  • increased the permissions for mon profile mgr (we need to be able to write config-keys and read(+x) auth)
  • implemented a shutdown method+endpoint (for internal use only, requires a master key that is not shared anywhere and is unique to each running instance) to do a clean shutdown
  • minor fixes
  • deployed the latest chacra build with ansible, passed my testing
  • the auth system now accepts 'mon profile mgr' keys
@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Apr 13, 2017

security: We do not authenticate the servers in the sense of SSL but afaik, we don't do that with Ceph tools either (*). The SSL certificates just make it more obvious because they are designed to be signed by a CA but they do have a use even when they are just self-signed -- you can easily detect a change in host (certificate changed), the communication is encrypted,...

It is only the first encounter with the server when you are vulnerable to the MITM attack just like you are with Ceph itself (imagine someone spoofing your DNS records and acting as a middle man resending hte data to the real Ceph node, the person would have your data, know your key, ...).

In any case, the new way of handling the certificates allows you to verify the certificate via the usual channels (ceph config-key get mgr.restful.cert) and compare it to the key reported by the REST api server running on port 8002. This gives you the same level of server authentication as the cli commands have. This could also probably be further automated -- we could e.g. provide a CLI utility that checks that the two certificates are the same.

There is also the point that if you use a non-standard auth system, the admins are very likely to not take the API that seriously but that is really rather bad -- the API is rather powerful (you can even delete pools with it...). If you use the CephX key for auth, it makes this much more obvious and it should make users much more cautious about it which is a good thing.

(*) We would need a third party -- some kind of CA -- for that. I really can't imagine how a server authentication without some kind of CA would work and afaik, we do not have a CA in Ceph.

speed: My point was that 20-30% performance penalty if you are running the API as a user (i.e. one request per few seconds or minutes) does not sound that bad. On the other hand, if you are doing several requests per second with a web UI, it is really rather easy to generate a token and use the token-based authentication.

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 13, 2017

security: We do not authenticate the servers in the sense of SSL but afaik, we don't do that with Ceph tools either (*)

Without having double checked this, I believe that CephX authentication doesn't transmit the key. It's shared secret crypto: the server proves to the client that it already knows the client's key, and the client proves to the server that it has the key, all without actually sending the key.

The central issue with what you're doing here is that you're transmitting that key like a password, but it's not a password. It's a key for use in shared secret crypto.

speed: My point was that 20-30% performance penalty if you are running the API as a user (i.e. one request per few seconds or minutes) does not sound that bad.

I don't care about the penalty for the rest api consumer, I care about it spamming the monitors. I don't want every monitor log we see to be a long stream of "auth get" commands, and I don't want monitors to have to service these extra requests unnecessarily.

On the other hand, if you are doing several requests per second with a web UI, it is really rather easy to generate a token and use the token-based authentication.

Certainly it's easy to generate a token, but that doesn't mean users will do it. There's no need to offer them the choice -- just let the tokens be configured via the CLI, and then only allow HTTP access with tokens.

def _shutdown(self):
# We need request module to get the shutdown function
requests.post(
'https://localhost:8002/shutdown',

This comment has been minimized.

@jcsp

jcsp Apr 13, 2017

Contributor

Does the werkzeug server really not have a better way to shut down than making a process POST to itself?

This comment has been minimized.

@b-ranto

b-ranto Apr 13, 2017

Contributor

Werkzeug does but Flask (that runs the werkzeug server) does not provide it directly so you need the request to get the environment variable that points to the shutdown function. Some more details can be found e.g. here:

http://stackoverflow.com/questions/15562446/how-to-stop-flask-application-without-using-ctrl-c

@@ -173,7 +173,8 @@ void MonCapGrant::expand_profile_mon(const EntityName& name) const
profile_grants.push_back(MonCapGrant("mon", MON_CAP_R));
profile_grants.push_back(MonCapGrant("mds", MON_CAP_R));
profile_grants.push_back(MonCapGrant("osd", MON_CAP_R | MON_CAP_W));
profile_grants.push_back(MonCapGrant("config-key", MON_CAP_R));
profile_grants.push_back(MonCapGrant("auth", MON_CAP_R | MON_CAP_X));
profile_grants.push_back(MonCapGrant("config-key", MON_CAP_R | MON_CAP_W));

This comment has been minimized.

@jcsp

jcsp Apr 13, 2017

Contributor

Hmm, so it looks like @liewegas rolled back the permissions of the mgr quite a bit, in practice I suspect we're going to end up adding back read/write access to most of these in order to enable mgr modules to do management stuff.

This comment has been minimized.

@b-ranto

b-ranto Apr 13, 2017

Contributor

Actually, when I first tested this, even the "osd" had only MON_CAP_R so I could not e.g. create a pool. I think @tchaikov changed that in the last couple of days.

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 13, 2017

@tchaikov @dmick you guys may also have some comments on this?

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Apr 13, 2017

@jcsp will review this change (and yours) in one day or two.

@tchaikov tchaikov requested review from dillaman and tchaikov and removed request for dillaman Apr 13, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented Apr 13, 2017

@dmick

This comment has been minimized.

Member

dmick commented Apr 14, 2017

I've been trying to find any rationale or discussion, and failing. Is there some background for "why are we doing this"?

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Apr 14, 2017

@dmick Dependencies, the django based REST api won't run unless you manually (with pip I believe) install all the dependencies. This one only requires stuff that is already in EPEL. It was easier for me to do a complete rewrite based on the django based REST api than trying to strip the django dependencies from the current API. This runs out of the box, no manual steps are necessary and we can package it easily.

@jcsp

This comment has been minimized.

Contributor

jcsp commented Apr 15, 2017

(Aside: I am still surprised that a total rewrite was believed to be less effort than generating a Django RPM)

Speaking of rationales, I notice that this still hasn't been brought up on ceph-users, ceph-devel, or at a CDM call. If the hope here is that this API will be used by anybody, then it would probably make a lot of sense to try and find some would-be users and/or see if anyone was depending on the old rest module and/or the old calamari api.

In fairness to @b-ranto, he initially just pointed me at his branch, and I asked for a PR so that we had a place to leave comments/discussion. So now that we've got a PR, where do we want to go from here? Is this something that somebody wants to include in luminous? Are there tests on the way[1]?

  1. the rest module did not have tests in the tree, but it did have the old calamari tests that were ready to be ported over mostly as-is
@tchaikov

IMHO, once we have a user for this RESTful API, we should include it as a plugin for ceph-mgr. ideally, it should be packaged in an separated package which depends on ceph-mgr and python-flask.

# Check for invalid pool args
invalid = common.invalid_pool_args(args)
if invalid:
return {'message': 'Invalid arguments found: "%s"' % str(invalid)}, 500

This comment has been minimized.

@tchaikov

tchaikov Apr 24, 2017

Contributor

the error code should be 400 for bad requests, right?

This comment has been minimized.

@b-ranto

b-ranto Apr 25, 2017

Contributor

I was thinking about that but for the sake of consistency I used 500 almost everywhere (apart from auth where I used 401 to prompt for login/password). I suppose the important thing here is it is not 2xx.

@b-ranto b-ranto changed the title from [DNM] ceph-mgr: Implement new Flask-based rest api to [DNM] ceph-mgr: Implement new pecan-based rest api Apr 25, 2017

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Apr 25, 2017

Latest changelog:

  • switched to pecan due to the demand by other developers in the team and availability (no new non-rhel dependencies)
  • switched the authentication to the one in the old rest api but it does not work because the handle_command interface is broken, probably since commit 2357507 (it always says command not supported although ceph tell can find the commands)
@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Apr 25, 2017

@jcsp Packaging Django is highly non-trivial and a long-term project but it is not just the dependencies, there are several people in our team that are familiar with pecan and it is generally easier to maintain thanks to it being much more lightweight (the same holds for flask).

The primary user of the API should be storage console/tendrl which wants to switch from calamari to ceph-mgr and it is already familiar with pecan-based rest APIs (e.g. ceph-installer).

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented Apr 27, 2017

we have a duty to ship things as secure as we can by default

Agreed, and this includes keeping up with Django CVEs :) It's not just about generating a Django RPM (or set of RPMs), but staying on top of all the vulnerabilities that stack would entail.

b-ranto added some commits Apr 12, 2017

ceph-mgr: Implement new pecan-based rest api
The new rest API uses pecan for the restful functionality and simplifies
the code significantly. It should be mostly equivalent in functionality
to the django-based rest API. The api is self-documenting via /doc
endpoint.

Signed-off-by: Boris Ranto <branto@redhat.com>
restful: Use keys instead of tokens+cephx
Signed-off-by: Boris Ranto <branto@redhat.com>
restful: Prepare for api file split
The patch moves decorators into a separate file so they could be shared
amongst various files.

It also fixes key generation in werkzeug 0.10+ and fixes get input
method on various endpoints.

Signed-off-by: Boris Ranto <branto@redhat.com>
restful: Split api.py file into a module
The patch makes the REST api being served by a module, not a single
monolithic file.

Signed-off-by: Boris Ranto <branto@redhat.com>
restful: Use fixed cert/pkey files
This allows users to create their own certificates that are signed by a
CA. If the keys are not present then a self-signed certificate will be
created and distributed amongst all the mgr nodes.

It also allows us to get rid of the pyOpenSSL dependency.

Signed-off-by: Boris Ranto <branto@redhat.com>
restful: send_command changed type
The send_command function now requires 6 arguments, switching back to
mon commands as that is what we used before.

Signed-off-by: Boris Ranto <branto@redhat.com>
restful: Do not store the config file in /etc/ceph
This also contains a documentation fix for authentication in / endpoint.

Signed-off-by: Boris Ranto <branto@redhat.com>
restful: Generate cert/key in post scripts
This is the simplest way to generate the keys and probably the least
likely to cause trouble in the future.

Signed-off-by: Boris Ranto <branto@redhat.com>
restful: Use port 8003
This allows us to run both calamari and the rest api at the same time.

Signed-off-by: Boris Ranto <branto@redhat.com>
restful: Use pecan hook instead of catch decorator
This removes the catch decorator which was useful primarily for testing
purposes and instead switches to a pecan error hook which will print the
traceback to the ceph-mgr log.

Signed-off-by: Boris Ranto <branto@redhat.com>
restful: List full requests in /request
Signed-off-by: Boris Ranto <branto@redhat.com>
restful: Output sub-commands more clearly
Signed-off-by: Boris Ranto <branto@redhat.com>
restful: Add pagination support
This commit adds a pagination support via paginate decorator. The
decorator will paginate the output if it is told to. The first page is
0, the last page is -1. The decorator was applied to /request endpoint
so you can get requests in hunderds if you supply the endpoint with
?page=N. If no ?page=N is supplied, the decorator will output the full
list.

Signed-off-by: Boris Ranto <branto@redhat.com>
@b-ranto

This comment has been minimized.

Contributor

b-ranto commented May 22, 2017

@liewegas I have rebased and fixed the conflicts. I use this simple script to test the code:

https://pastebin.com/AnYgJPXZ

It mostly just checks the code integrity by going through almost all the endpoints and checking that they return http 200. It could use a bit more automation e.g. to pass the admin key, etc.

@liewegas

This comment has been minimized.

Member

liewegas commented May 22, 2017

Cool, can you add that script to qa/workunits/rest/test_mgr_rest_api.py (or something like that), and make it take the endpoint via the command line? Thanks!

restful: Add a qa workunit
This commit adds a simple test that will send the requests to most of
the API endpoints and check the status code of the requests.

Signed-off-by: Boris Ranto <branto@redhat.com>
@b-ranto

This comment has been minimized.

Contributor

b-ranto commented May 22, 2017

I guess you meant the admin key should be a cli argument. I have added the test script.

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented May 31, 2017

Can we merge this?

@liewegas

This comment has been minimized.

Member

liewegas commented May 31, 2017

Oops! Yeah, I thought we already had.

@liewegas liewegas merged commit 2984e5a into master May 31, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@liewegas liewegas deleted the wip-rest-flask branch May 31, 2017

@jacob-go

This comment has been minimized.

jacob-go commented Aug 17, 2017

Hello everyone,

I hope I'm not rude for commenting after a while, but I'm trying to work with this module right now and have some problems.

Actually, my only problem is with the /request endpoint. I don't understand the structure I need to use to get this endpoint responding. You talked about "a proper python structure" and "prefix", but unfortunately it's not clear for everybody.

The command I'm issuing right now looks something like:
curl -k https://<SERVER_IP>:8003/request -X POST -H "Authorization: Basic <BASE64_ENCODED_STRING>" -d '{"ceph": "status"}'. So as you can see the structure I'm using is just a plain JSON, validated by lint. It's not working also with {"ceph": status}.

The logs say thus:

2017-08-17 16:10:07.009483 7fba5bd89700 0 mgr[restful] Traceback (most recent call last):
File "/usr/lib/python2.7/dist-packages/pecan/core.py", line 678, in call
self.invoke_controller(controller, args, kwargs, state)
File "/usr/lib/python2.7/dist-packages/pecan/core.py", line 572, in invoke_controller
result = controller(*args, **kwargs)
File "/usr/lib/ceph/mgr/restful/decorators.py", line 33, in decorated
return f(*args, **kwargs)
File "/usr/lib/ceph/mgr/restful/api/request.py", line 87, in post
return module.instance.submit_request([[request.json]], **kwargs)
File "/usr/lib/ceph/mgr/restful/module.py", line 585, in submit_request
request = CommandsRequest(_request)
File "/usr/lib/ceph/mgr/restful/module.py", line 66, in init
results = self.run(commands_arrays[0])
File "/usr/lib/ceph/mgr/restful/module.py", line 84, in run
result.command = common.humanify_command(commands[index])
File "/usr/lib/ceph/mgr/restful/common.py", line 38, in humanify_command
out = [command['prefix']]
KeyError: 'prefix'
2017-08-17 16:10:07.009483 7fba5bd89700 0 mgr[restful] Traceback (most recent call last):
File "/usr/lib/python2.7/dist-packages/pecan/core.py", line 678, in call
self.invoke_controller(controller, args, kwargs, state)
File "/usr/lib/python2.7/dist-packages/pecan/core.py", line 572, in invoke_controller
result = controller(*args, **kwargs)
File "/usr/lib/ceph/mgr/restful/decorators.py", line 33, in decorated
return f(*args, **kwargs)
File "/usr/lib/ceph/mgr/restful/api/request.py", line 87, in post
return module.instance.submit_request([[request.json]], **kwargs)
File "/usr/lib/ceph/mgr/restful/module.py", line 585, in submit_request
request = CommandsRequest(_request)
File "/usr/lib/ceph/mgr/restful/module.py", line 66, in init
results = self.run(commands_arrays[0])
File "/usr/lib/ceph/mgr/restful/module.py", line 84, in run
result.command = common.humanify_command(commands[index])
File "/usr/lib/ceph/mgr/restful/common.py", line 38, in humanify_command
out = [command['prefix']]
KeyError: 'prefix'

I must admit I don't understand the prefix thing. And maybe I'm wrong in the understanding of the endpoint itself as a whole - shouldn't I use it to pass ANY ceph command? It'll be great if somebody would explain. Maybe the developer himself @b-ranto?

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Aug 17, 2017

Hi,

that method is a simple pass through method for arbitrary commands as defined in src/mon/MonCommands.h. Let's consider the following command

COMMAND("osd ls " \
        "name=epoch,type=CephInt,range=0,req=false", \
        "show all OSD ids", "osd", "r", "cli,rest")

The prefix is osd ls (or osd ls , not exactly sure which one is the correct one at the moment, even both of them might work). The optional argument's name is epoch and it is of type CephInt (python's int should work just fine).

I.e. You should send something like {'prefix': 'osd ls', 'epoch': 0} to the endpoint for it to work.

The endpoint is hard to use by design. It is not recommended to post to the endpoint unless you know exactly what you are doing. There is no parsing of output, no error handling, nothing. It will just pass the python structure and return the output.

@jacob-go

This comment has been minimized.

jacob-go commented Aug 17, 2017

Thank you SO much @b-ranto !
I don't know if it's your original intention, but that explanation really helped me, so I could quickly post the command and query the result.

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Aug 18, 2017

Yeah, feel free to play with it or use it, just really don't expect anything special out of it. We had some issues in the past with a similar pass through method that was easier to use where people expected it to do way more than it actually did.

@jacob-go

This comment has been minimized.

jacob-go commented Aug 18, 2017

@b-ranto No, be certain I'm not after anything special, just run basic commands via REST. :)

else:
self.keys[command['key_name']] = str(uuid4())
self.set_config_json('keys', self.keys)

This comment has been minimized.

@Liuchang0812

Liuchang0812 Aug 18, 2017

Contributor

hi, a quick question: self.set_config_json probably run failed, like bad authorization. should we process this case here?

This comment has been minimized.

@b-ranto

b-ranto Aug 18, 2017

Contributor

This code has changed a lot since this PR has been merged. Feel free to create an upstream issue or PR if you think there is something wrong with the current code.

However, in general, set_config_json is unlikely to fail. If it does it may point to some deeper issues with ceph-mgr code.

This comment has been minimized.

@Liuchang0812

Liuchang0812 Aug 18, 2017

Contributor

thanks @b-ranto roger that.

@jacob-go

This comment has been minimized.

jacob-go commented Aug 20, 2017

@b-ranto I guess the answer is 'no', but I have to ask just in case you know better - Is it possible to invoke the Ceph admin socket (under /var/run/ceph/) via the /request endpoint? Execute something like ceph --admin-daemon $file perfcounters_dump or ceph daemon osd.0 perf dump? I guess it's problematic since it's outside the scope of MonCommands, and maybe a bit dangerous in general.

@jcsp

This comment has been minimized.

Contributor

jcsp commented Aug 21, 2017

If this discussion could move into a ceph-devel mailing list thread for broader visibility (and so that I don't keep getting notifications from this long-closed PR?), I would appreciate it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment