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: Fix objects named default are inaccessible #20976

Merged

Conversation

sebastian-philipp
Copy link
Contributor

@sebastian-philipp sebastian-philipp commented Mar 20, 2018

These two calls to default() are identical: vpath and params are both empty:

$ curl 'http://localhost/api/cp_path/'

and

$ curl 'http://localhost/api/cp_path/default'

But we need to distinguish them. To fix this, we need to add the missing default

Also, I had to slightly refactor PerfCounters in order to have a reliable _cp_path_ property.

Signed-off-by: Sebastian Wagner sebastian.wagner@suse.com

# $ curl 'http://localhost/api/cp_path/'
# and
# $ curl 'http://localhost/api/cp_path/default'
# But we need to dinstinguish them. To fix this, we need
Copy link
Contributor

@LenzGr LenzGr Mar 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/dinstinguish/distinguish/ (please also fix this in the changeset comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@sebastian-philipp sebastian-philipp force-pushed the dashboard-restcontroller-default-get branch from 78bacee to dce9616 Compare March 20, 2018 15:36
@sebastian-philipp
Copy link
Contributor Author

jenkins retest this please

@rjfd
Copy link
Contributor

rjfd commented Mar 21, 2018

@sebastian-philipp the test_perf_counters.py API test failed with this PR:

2018-03-21 15:21:21,328.328 INFO:tasks.mgr.dashboard.helper:request GET to http://rdias-suse-laptop.rdias.home.pt:7790/api/perf_counters/mgr/y
2018-03-21 15:21:21,333.333 INFO:__main__:test_perf_counters_mgr_get (tasks.mgr.dashboard.test_perf_counters.PerfCountersControllerTest) ... FAIL
2018-03-21 15:21:21,333.333 INFO:__main__:run args=['./bin/ceph', 'log', 'Ended test tasks.mgr.dashboard.test_perf_counters.PerfCountersControllerTest.test_perf_counters_mgr_get']
2018-03-21 15:21:21,334.334 INFO:__main__:Running ['./bin/ceph', 'log', 'Ended test tasks.mgr.dashboard.test_perf_counters.PerfCountersControllerTest.test_perf_counters_mgr_get']
2018-03-21 15:21:21,606.606 INFO:__main__:Stopped test: test_perf_counters_mgr_get (tasks.mgr.dashboard.test_perf_counters.PerfCountersControllerTest) in 2.015035s
2018-03-21 15:21:21,607.607 INFO:__main__:
2018-03-21 15:21:21,607.607 INFO:__main__:======================================================================
2018-03-21 15:21:21,607.607 INFO:__main__:FAIL: test_perf_counters_mgr_get (tasks.mgr.dashboard.test_perf_counters.PerfCountersControllerTest)
2018-03-21 15:21:21,607.607 INFO:__main__:----------------------------------------------------------------------
2018-03-21 15:21:21,607.607 INFO:__main__:Traceback (most recent call last):
2018-03-21 15:21:21,607.607 INFO:__main__:  File "/home/rdias/Work/ceph/qa/tasks/mgr/dashboard/helper.py", line 23, in decorate
2018-03-21 15:21:21,607.607 INFO:__main__:    return func(self, *args, **kwargs)
2018-03-21 15:21:21,607.607 INFO:__main__:  File "/home/rdias/Work/ceph/qa/tasks/mgr/dashboard/test_perf_counters.py", line 44, in test_perf_counters_mgr_get
2018-03-21 15:21:21,607.607 INFO:__main__:    self.assertStatus(200)
2018-03-21 15:21:21,607.607 INFO:__main__:  File "/home/rdias/Work/ceph/qa/tasks/mgr/dashboard/helper.py", line 119, in assertStatus
2018-03-21 15:21:21,607.607 INFO:__main__:    self.assertEqual(self._resp.status_code, status)
2018-03-21 15:21:21,608.608 INFO:__main__:AssertionError: 500 != 200
2018-03-21 15:21:21,608.608 INFO:__main__:
2018-03-21 15:21:21,608.608 INFO:__main__:----------------------------------------------------------------------

From the mgr log I got this traceback:

2018-03-21 15:21:21.327 7f0e30b73700  0 mgr[dashboard] [::ffff:192.168.1.102:38496] [GET] [500] [0.001s] [admin] [862B] /perf_counters/mgr/y
2018-03-21 15:21:21.327 7f0e30b73700  0 mgr[dashboard] ['{"status": "500 Internal Server Error", "version": "10.2.1", "detail": "The server encountered an unexpected condition which prevented it from fulfilling the request.", "traceback": "Traceback (most recent call last):\\n  File \\"/usr/lib/python2.7/site-packages/cherrypy/_cprequest.py\\", line 670, in respond\\n    response.body = self.handler()\\n  File \\"/usr/lib/python2.7/site-packages/cherrypy/lib/encoding.py\\", line 221, in __call__\\n    self.body = self.oldhandler(*args, **kwargs)\\n  File \\"/usr/lib/python2.7/site-packages/cherrypy/_cpdispatch.py\\", line 60, in __call__\\n    return self.callable(*self.args, **self.kwargs)\\n  File \\"/home/rdias/Work/ceph/src/pybind/mgr/dashboard/tools.py\\", line 330, in default\\n    \'{}/api/{}/default\'.format(mgr.url_prefix, self._cp_path_)) or \\\\\\nAttributeError: \'PerfCounter\' object has no attribute \'_cp_path_\'\\n"}']

@sebastian-philipp sebastian-philipp force-pushed the dashboard-restcontroller-default-get branch 2 times, most recently from de1a90a to eb80aa9 Compare March 22, 2018 13:32
@sebastian-philipp
Copy link
Contributor Author

@rjfd fixed the internal server error

@rjfd
Copy link
Contributor

rjfd commented Mar 26, 2018

@sebastian-philipp please rebase this PR to resolve the conflicts

@sebastian-philipp sebastian-philipp force-pushed the dashboard-restcontroller-default-get branch 2 times, most recently from 1afd194 to 6b0b118 Compare March 26, 2018 12:58
@sebastian-philipp
Copy link
Contributor Author

sebastian-philipp commented Mar 26, 2018

@sebastian-philipp please rebase this PR to resolve the conflicts

done

@rjfd
Copy link
Contributor

rjfd commented Mar 27, 2018

Copy link
Contributor

@rjfd rjfd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you put the changes made to the perf_counters controller in a separate commit?

@sebastian-philipp sebastian-philipp force-pushed the dashboard-restcontroller-default-get branch from 6b0b118 to d8af9ba Compare March 27, 2018 13:09
@sebastian-philipp
Copy link
Contributor Author

Could you put the changes made to the perf_counters controller in a separate commit?

@rjfd: done

@rjfd
Copy link
Contributor

rjfd commented Mar 27, 2018

@sebastian-philipp please rebase the PR on top of master

The next commit will require that all controllers
have a `@ApiController` decorator.

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
These two calls to `default()` are identical: `vpath` and
params` are both empty:

$ curl 'http://localhost/api/cp_path/'

and

$ curl 'http://localhost/api/cp_path/default'

But we need to dinstinguish them. To fix this, we need
to add the missing `default`

We now require that all exposed controllers have a @APIController
decorator.

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
@sebastian-philipp sebastian-philipp force-pushed the dashboard-restcontroller-default-get branch from d8af9ba to 452992b Compare March 27, 2018 13:21
@sebastian-philipp
Copy link
Contributor Author

Rebased, cause of a conflict in src/pybind/mgr/dashboard/tools.py

@rjfd rjfd merged commit dd03c5e into ceph:master Mar 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants