Skip to content
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

mgr/dashboard: RGW page #19512

Merged
merged 7 commits into from Jan 2, 2018

Conversation

Projects
None yet
5 participants
@liuchang0812
Copy link
Contributor

commented Dec 14, 2017

Signed-off-by: Chang Liu liuchang0812@gmail.com

mgr/dashboard: create RGW page
Signed-off-by: Chang Liu <liuchang0812@gmail.com>
@liuchang0812

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2017

List all RGW daemons:

image

Metadata and Status of one RGW daemon:

image

@mattbenjamin mattbenjamin added the rgw label Dec 14, 2017

@mattbenjamin mattbenjamin self-assigned this Dec 14, 2017

@mattbenjamin mattbenjamin added the mgr label Dec 14, 2017

@mattbenjamin mattbenjamin requested a review from jcsp Dec 14, 2017

@mattbenjamin

This comment has been minimized.

Copy link
Contributor

commented Dec 14, 2017

@liuchang0812 looks reasonable offhand, thanks very much; note that this area may evolve quickly

@mattbenjamin mattbenjamin requested a review from yehudasa Dec 14, 2017

@liuchang0812

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2017

@mattbenjamin thank you. as we discussed last CDM, we are going to create a RGW page. actually this is a early version, and a lot of work need to do. I wonder that we could show more information :

  1. RGW-NFS daemons
  2. RGW request statistics: qps/average latency
  3. RGW sync status: data sync status/metadata sync status/sync error
  4. user/bucket informations: i afraid that we need to pythonize RGWRados class
@liuchang0812

This comment has been minimized.

Copy link
Contributor Author

commented Dec 15, 2017

I find that we have already implemented pybind/rgw.

@mattbenjamin

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2017

@liuchang0812 for things like user and bucket info--and day2 mgmt operations, the consensus within the RGW team was to delegate to a running instance of the RGW restful admin server, rather than to embed an RGW instance in ceph-mgr via librgw; There are already community bindings to the currently exported methods here: #19164
@liuchang0812 would you be able to come to some regular meetings of the upstream RGW team to discuss ideas and share progress?

@mattbenjamin mattbenjamin requested a review from cbodley Dec 15, 2017

@cbodley

This comment has been minimized.

Copy link
Contributor

commented Dec 15, 2017

  1. user/bucket informations: i afraid that we need to pythonize RGWRados class

this is indeed terrifying :) ideally we would just send s3 requests to a running gateway for this

similarly, we have admin apis that can serve sync status for 3.

@liuchang0812

This comment has been minimized.

Copy link
Contributor Author

commented Dec 18, 2017

@Liuchang0812 would you be able to come to some regular meetings of the upstream RGW team to discuss ideas and share progress?

Of course. I'm very glad to discuss it with you all. My colleague @Leeshine is also working on it. We will prepare a simple slide. Would you mind giving me more information about the meeting? what time is the meeting and how could we join it?

@jcsp
Copy link
Contributor

left a comment

This is great, I just have minor suggested changes


<li class="{%if path_info.startswith('/rgw')%}active{%endif%}">
<a href="{{ url_prefix }}/rgw">
<i class="fa fa-files-o" aria-hidden="true"></i>

This comment has been minimized.

Copy link
@jcsp

jcsp Dec 19, 2017

Contributor

There is no "right" icon for RGW (as far as I know), but what about cloud-download (http://fontawesome.io/icon/cloud-download/) instead of a file-related icon?

'server': server,
'metadata': metadata,
'status': status,
'url': "/rgw/detail/{0}".format(service['id'])

This comment has been minimized.

Copy link
@jcsp

jcsp Dec 19, 2017

Contributor

I suggest leaving out the /detail/ part, and just use /rgw for the list and /rgw/<id> for the detail

This comment has been minimized.

Copy link
@liuchang0812

liuchang0812 Dec 21, 2017

Author Contributor

sorry, John, how could I get this <id> in RGWEndpoint::index function. I tested with index(self, rgw_id=None) and index(self, rgw_id), cherrypy could not dispatch request correctly as expected. we use /osd/perf/<id> in OSDEndpoint, not /osd/<id>. I wonder whether cherrypy could support this url pattern. I 'm looking cherrypy's document.

This comment has been minimized.

Copy link
@jcsp

jcsp Dec 21, 2017

Contributor

It's a little awkward, but I think in this case you can prefix a @cherrypy.popargs('rgw_id') to the RGWEndpoint class (http://docs.cherrypy.org/en/latest/advanced.html#restful-style-dispatching), and then the rgw_id argument to index will work.

rgw.{rgw_id}
</h1>
</section>

This comment has been minimized.

Copy link
@jcsp

jcsp Dec 19, 2017

Contributor

Start a section class="content" here, to get proper padding/margins on the main part with the tables.

});
});
if (content_data.rgw_status_list.length() == 0) {

This comment has been minimized.

Copy link
@jcsp

jcsp Dec 19, 2017

Contributor

You can use rv-show="rgw_status_list | length" in the markup, to avoid the need to explicitly set a _show variable up here (the length helper is defined in base.html so you can use it anywhere)

var post_load = function() {
content_data.rgw_metadata_list = [];
content_data.rgw_status_list = [];
_.each(content_data.rgw_metadata, function(v, k) {

This comment has been minimized.

Copy link
@jcsp

jcsp Dec 19, 2017

Contributor

It would also be fine to do this on the python side, so that the javascript just gets the data in the form that it prefers (a list).

These items are currently coming out in arbitrary order, which makes it a bit difficult to scan visually -- it would be nice to sort them alphabetically by key (could also do this server side if passing a list instead of a dict)

If you choose to keep it on the javascript side, you'll also need to make sure that post_load is called after each refresh()

var refresh = function() {
$.get("{{ url_prefix }}/rgw/rgw_data/" + content_data.rgw_id + "/", function(data) {
content_data = data;
setTimeout(refresh, 3000);

This comment has been minimized.

Copy link
@jcsp

jcsp Dec 19, 2017

Contributor

Let's make this period the same as the one on line 16 (probably both 5000)


log = logging.getLogger("dashboard")

class RGWDaemons(RemoteViewCache):

This comment has been minimized.

Copy link
@jcsp

jcsp Dec 19, 2017

Contributor

Please leave two blank lines before class definition for PEP8 style


import json
import re
import rados

This comment has been minimized.

Copy link
@jcsp

jcsp Dec 19, 2017

Contributor

the re and rados imports are unused and can be removed

try:
status = json.loads(status['json'])
except:
status = {}

This comment has been minimized.

Copy link
@jcsp

jcsp Dec 19, 2017

Contributor

Let's do a log.warn here to say the service['id'] which had invalid status json

@cbodley

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2017

what time is the meeting and how could we join it?

the rgw team does informal 15-20min standup meetings in https://bluejeans.com/3919548994. they happen on Monday at 9:45am EST, and Tues/Thurs/Fri at 11:45am EST (sorry, we need to publish this stuff on ceph.com somewhere). you're all welcome to join as often as you like!

<li class="{%if path_info.startswith('/rgw')%}active{%endif%}">
<a href="{{ url_prefix }}/rgw">
<i class="fa fa-files-o" aria-hidden="true"></i>
<span>RGW</span></a>

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 20, 2017

Contributor

Isn't the "official" name "Object Store" or "Object Gateway"? At least that's what the documentation refers to nowadays (albeit not consistently).

This comment has been minimized.

Copy link
@mattbenjamin

mattbenjamin Dec 20, 2017

Contributor

The docs do have object gateway. I think that's pretty formal, and RGW is the shortform everywhere. Just offhand, what do you prefer?

This comment has been minimized.

Copy link
@LenzGr

LenzGr Dec 20, 2017

Contributor

We've settled to using "Object Gateway" in our project - we found "RGW" or "RADOS Gateway" to be too non-descriptive in a Web UI.

liuchang0812 added some commits Dec 21, 2017

mgr/dashbaord: uses fa-cloud-download as RGW's icon
Signed-off-by: Chang Liu <liuchang0812@gmail.com>
mgr/dashboard: remove redundant python modules re/rados
Signed-off-by: Chang Liu <liuchang0812@gmail.com>
mgr/dashboard: logging when loads json failed
Signed-off-by: Chang Liu <liuchang0812@gmail.com>
@liuchang0812

This comment has been minimized.

Copy link
Contributor Author

commented Dec 21, 2017

@cbodley @jcsp thank you all for the information and reviewing.

liuchang0812 added some commits Dec 22, 2017

mgr/dashboard: simplify URL routing, /rgw/detail/<id> -> /rgw/<id>
Signed-off-by: Chang Liu <liuchang0812@gmail.com>
rgw/dashboard: move some logic from JS to Python side, some cleanups
Signed-off-by: Chang Liu <liuchang0812@gmail.com>

@liuchang0812 liuchang0812 force-pushed the liuchang0812:wip-dashboard-rgw branch from 9230bc5 to ffdc050 Dec 22, 2017

@liuchang0812

This comment has been minimized.

Copy link
Contributor Author

commented Dec 22, 2017

image

@jcsp aha. cherrypy.popargs is awesome! It seems I have fixed all your comments. RGW page is more beautiful now.

'server': server,
'metadata': metadata,
'status': status,
'url': "/rgw/{0}".format(service['id'])

This comment has been minimized.

Copy link
@liuchang0812

liuchang0812 Dec 22, 2017

Author Contributor

seems that I missed url_prefix here

rgw/dashboard: supports url_prefix in rgw page
Signed-off-by: Chang Liu <liuchang0812@gmail.com>

@liuchang0812 liuchang0812 changed the title [rfc]mgr/dashboard: RGW page mgr/dashboard: RGW page Dec 22, 2017

@jcsp

jcsp approved these changes Jan 2, 2018

@jcsp

This comment has been minimized.

Copy link
Contributor

commented Jan 2, 2018

Tested this locally with a vstart cluster and it looks good!

@jcsp jcsp merged commit 81fc8e0 into ceph:master Jan 2, 2018

5 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details
@jcsp

This comment has been minimized.

Copy link
Contributor

commented Jan 2, 2018

Thank you @liuchang0812

@liuchang0812 liuchang0812 deleted the liuchang0812:wip-dashboard-rgw branch Jan 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.