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 GUI module #14946

Merged
merged 12 commits into from May 19, 2017

Conversation

Projects
None yet
6 participants
@dmick
Member

dmick commented May 4, 2017

Cleanup of work from @jcsp to add a ceph-mgr Python module to provide a web service that displays a simple GUI status, with one feature added to allow configuration of the listen addr and port for the server.

Configuration is done with ceph config-key put; the key names are

mgr.dashboard.server_addr
mgr.dashboard.server_port

@dmick dmick requested a review from jcsp May 4, 2017

@liewegas

This comment has been minimized.

Member

liewegas commented May 4, 2017

Since you can have multiple mgr's on different hosts, I think we need to qualify these with the mgr id. Something like

mgr/x/dashboard/server_addr
mgr/x/dashboard/server_port
mgr/y/dashboard/server_addr
mgr/y/dashboard/server_port

(The dm-crypt stuff in config-key is currently using / instead of . as delimiter; we may as well stick with that?)

@liewegas liewegas changed the title from Manager dashboard GUI module to mgr: dashboard GUI module May 4, 2017

@dmick

This comment has been minimized.

Member

dmick commented May 4, 2017

ok, will hack

@dmick dmick changed the title from mgr: dashboard GUI module to DNM: mgr: dashboard GUI module May 4, 2017

@dmick dmick changed the title from DNM: mgr: dashboard GUI module to mgr: dashboard GUI module May 5, 2017

@dmick

This comment has been minimized.

Member

dmick commented May 6, 2017

Working on Yet Another Rebase

@dmick

This comment has been minimized.

Member

dmick commented May 8, 2017

Removing some commits that had made it into master in the meantime made this merge more cleanly; this branch appears to pass dumb functional tests again.

@dmick dmick changed the title from mgr: dashboard GUI module to DNM: mgr: dashboard GUI module May 8, 2017

@dmick

This comment has been minimized.

Member

dmick commented May 8, 2017

uhh....there are apparently a few more files than I wanted here. Edit: ah, no, all the JS and CSS. My bad.

@dmick dmick changed the title from DNM: mgr: dashboard GUI module to mgr: dashboard GUI module May 8, 2017

@jcsp

This comment has been minimized.

Contributor

jcsp commented May 10, 2017

@dmick you could probably cull a lot of the AdminLTE guff, I think there are various optional bits (themes/plugins) in there where we only need the files for the ones we use.

@liewegas liewegas added the needs-qa label May 15, 2017

@dmick

This comment has been minimized.

Member

dmick commented May 16, 2017

rebased again

@liewegas

This comment has been minimized.

Member

liewegas commented May 17, 2017

weird thing on this run: http://pulpito.ceph.com/sage-2017-05-17_15:24:19-rados-wip-sage-testing---basic-smithi/1189010

smithi055 and smithi067 are both trusty, but the cherrypy error only came up on one of them. not a package dependency for deb yet?

@liewegas

This comment has been minimized.

Member

liewegas commented May 17, 2017

@tserong

This comment has been minimized.

Contributor

tserong commented May 18, 2017

specfile change LGTM FWIW

@liewegas

This comment has been minimized.

Member

liewegas commented May 18, 2017

Ok, I suggest we merge this as is and refine on top of master.

@tchaikov

yeah, we can remove the unused AdminLTE bits later on. and move this into a separated package probably.

@@ -196,10 +196,10 @@ ceph_config_get(PyObject *self, PyObject *args)
std::string value;
bool found = global_handle->get_config(handle, what, &value);
if (found) {
derr << "Found" << dendl;
derr << "ceph_config_get " << what << " found: " << value.c_str() << dendl;

This comment has been minimized.

@tchaikov

tchaikov May 18, 2017

Contributor

could you use __func__ instead here?

return PyString_FromString(value.c_str());
} else {
derr << "Not found" << dendl;
derr << "ceph_config_get " << what << " not found " << dendl;

This comment has been minimized.

@tchaikov

tchaikov May 18, 2017

Contributor

ditto.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 18, 2017

@dmick please feel free to merge this when you feel appropriate.

John Spray added some commits Nov 13, 2016

pybind/mgr: web dashboard
A read only display of some of the cluster's state, plus
some cephfs/rbd/osd pages that demonstrate how to go
read state remotely from daemons.

Signed-off-by: John Spray <john.spray@redhat.com>
vstart: enable gui plugin
Signed-off-by: John Spray <john.spray@redhat.com>
osd: wire up "histogram dump" to `tell`
...so that mgr modules can get at it.

Signed-off-by: John Spray <john.spray@redhat.com>

dmick added some commits Apr 11, 2017

ServeThread: add 'running' flag, don't notify non-running threads
Signed-off-by: Dan Mick <dan.mick@redhat.com>
ceph_syn.cc, client/SyntheticClient.cc: Client -> StandaloneClient
Signed-off-by: Dan Mick <dan.mick@redhat.com>
client/SyntheticClient.cc: filer is now a std::unique_ptr
Signed-off-by: Dan Mick <dan.mick@redhat.com>
mgr/PyModules.cc,PyState.cc: improve config access debug logging
Signed-off-by: Dan Mick <dan.mick@redhat.com>
pybind/mgr/dashboard/module.py: allow custom server addr, port
Signed-off-by: Dan Mick <dan.mick@redhat.com>
mgr/PyModules: move constructor definition to .cc
Otherwise, the incomplete ServeThread type causes problems
for others who include PyModules.h

Signed-off-by: Dan Mick <dan.mick@redhat.com>
mgr: change config_prefix to be 'mgr/<id>/' and separate handle/key b…
…y '/'

dm_crypt config-key usage has established '/' as the separator; keep
that as a convention for now

Signed-off-by: Dan Mick <dan.mick@redhat.com>
ceph.spec.in, debian/control: build/runtime dependency for cherrypy
Signed-off-by: Dan Mick <dan.mick@redhat.com>
@dmick

This comment has been minimized.

Member

dmick commented May 19, 2017

Dependencies added, very simple smoke test added to make check

@liewegas

This comment has been minimized.

Member

liewegas commented May 19, 2017

/home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mgr/mgr-dashboard-smoke.sh:45: run:  sleep 1
/home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mgr/mgr-dashboard-smoke.sh:39: run:  [[ 4 < 30 ]]
/home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mgr/mgr-dashboard-smoke.sh:47: run:  curl -s http://127.0.0.1:7001/toplevel_data
/home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mgr/mgr-dashboard-smoke.sh:48: run:  jq .health.overall_status
/home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mgr/mgr-dashboard-smoke.sh:48: run:  grep HEALTH_
parse error: Invalid numeric literal at line 1, column 10
/home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mgr/mgr-dashboard-smoke.sh:48: run:  return 1
/home/jenkins-build/build/workspace/ceph-pull-requests/qa/workunits/ceph-helpers.sh:1692: main:  display_logs td/mgr-dashboard-smoke
@dmick

This comment has been minimized.

Member

dmick commented May 19, 2017

Yep; internal failure in dashboard. Trying to reproduce now.

test: add mgr/ and smoke test for mgr dashboard
Signed-off-by: Dan Mick <dan.mick@redhat.com>
@dmick

This comment has been minimized.

Member

dmick commented May 19, 2017

It was intermittent; sometimes after the mgr said 'available', dashboard still wasn't quite ready. Added a second retry loop; running with ctest --output-on-failure --repeat-until-fail 100 -R mgr-dash looks solid now

@liewegas

This comment has been minimized.

Member

liewegas commented May 19, 2017

excellent, thanks!

@liewegas liewegas merged commit de6c0bd into ceph:master May 19, 2017

2 of 3 checks passed

default Build started sha1 is merged.
Details
Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
@dmick

This comment has been minimized.

Member

dmick commented May 19, 2017

Sorry @tchaikov, missed your comments about func; I wasn't ignoring you on purpose

@dmick dmick deleted the dmick:wip-mgr-dashboard branch May 19, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 20, 2017

@dmick no worries, we can always improve it once we are at it.

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