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

tools: cleanup: rip out ceph-rest-api #17530

Merged
merged 5 commits into from
Mar 6, 2018
Merged

Conversation

smithfarm
Copy link
Contributor

@smithfarm smithfarm commented Sep 6, 2017

@liewegas
Copy link
Member

liewegas commented Sep 6, 2017

@dvanders ping

@smithfarm smithfarm requested a review from jcsp September 6, 2017 19:55
@ktdreyer ktdreyer requested a review from dmick September 6, 2017 20:09
Copy link
Member

@ktdreyer ktdreyer left a comment

Choose a reason for hiding this comment

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

Thanks @smithfarm !

@dmick
Copy link
Member

dmick commented Sep 7, 2017

Not to be a naysayer, but: does the mgr's REST API allow submitting any ceph CLI command in a supported way?

Supposedly ceph-rest-api does (and it uses the command descriptions from the daemons, so supposedly added new commands as they were added). I really doubt any large org is depending on this in any meaningful way, but...it's not clear to me that we have an exact replacement.

@jcsp
Copy link
Contributor

jcsp commented Sep 7, 2017

I don't think there are any detailed docs online for the new rest api :-( so I'm just looking at the code, it looks like the Request.post method is meant to be a passthrough.

ceph-rest-api does have a nice dump of available commands for mistyped URLs, which probably makes it a bit friendlier for people who are really looking for a passthrough.

I'm also a bit concerned that there hasn't been any active development activity on the restful module -- if we are going to maintain something minimal then porting ceph_rest_api forward into a mgr module might make more sense.

@dvanders
Copy link
Contributor

dvanders commented Sep 7, 2017

@liewegas no problem here. Thx for the ping.

@liewegas
Copy link
Member

My preference would also be to transplant this into a mgr module, but that takes actual work. :/

@b-ranto
Copy link
Contributor

b-ranto commented Sep 14, 2017

The restful module does provide a pass through method. It is not properly documented because when the module was introduced it was suggested that it should be difficult to use so that people do not expect too much out of it as they did in case of calamari 'command' endpoint.

We can make it more usable so that it could fully obsolete the ceph-rest-api and it should not be too difficult to achieve.

You are right that we should take a dump of the /doc endpoint and add it to the upstream docs. I'll try to do that in the next couple of days. It is better to have this readily available online as well.

@LenzGr
Copy link
Contributor

LenzGr commented Sep 16, 2017

FWIW, inkscope uses the ceph-rest-api to perform its tasks - it may make sense to ping @A-Dechorgnat on his take on removing this.

@liewegas liewegas added the DNM label Dec 29, 2017
@smithfarm
Copy link
Contributor Author

If someone steps forward to convert ceph-rest-api into a mgr module, feel free to close this PR.

If the consensus is to rip it out for mimic, and assuming this PR is still a viable option for doing that, let me know and I'll rebase it.

@jcsp
Copy link
Contributor

jcsp commented Mar 2, 2018

I'm inclined to proceed with ripping it out, as although people have fairly suggested that it would be nice to port it over to mgr, or to improve the restful module's documentation, I don't think anyone is volunteering to do any work.

Anyone who misses ceph-rest-api has a couple of decent options: use the CLI (this was already just a pass-through CLI wrapper), or move to the restful module.

@liewegas ?

@liewegas
Copy link
Member

liewegas commented Mar 3, 2018

Yeah, as long as we have a specific place in restful to point users toward, I'm fine.

@b-ranto
Copy link
Contributor

b-ranto commented Mar 5, 2018

This should help with regards to the restful documentation. It also explains how to use the pass through method.

#20717

@smithfarm
Copy link
Contributor Author

OK, rebased and removing DNM

@smithfarm smithfarm added needs-review and removed DNM labels Mar 5, 2018
@smithfarm
Copy link
Contributor Author

Will proceed with QA testing after I get one more review?

@smithfarm
Copy link
Contributor Author

Changelog:

  • fixed multiple issues in the PendingReleaseNotes commit

Obsoleted by the mgr REST API.

Fixes: http://tracker.ceph.com/issues/21264
Signed-off-by: Nathan Cutler <ncutler@suse.com>
References: http://tracker.ceph.com/issues/21264
Signed-off-by: Nathan Cutler <ncutler@suse.com>
87399be introduced an explicit dependency on
python-jinja2, but mistakenly as an overall build dependency instead of as a
runtime dependency of ceph-mgr as intended.

Fixes: http://tracker.ceph.com/issues/22457
Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
Copy link
Contributor Author

Oops, found a problem in #19598 which caused a massive rados failure. Added a commit to fix it.

@smithfarm
Copy link
Contributor Author

smithfarm commented Mar 5, 2018

https://shaman.ceph.com/builds/ceph/wip-smithfarm-testing-2018-03-05-2213/d4778ca1ef5d0da8b2d9c4b28ae703b2c692fd3f/

fail http://pulpito.ceph.com/smithfarm-2018-03-06_05:22:19-rados-wip-smithfarm-testing-2018-03-05-2213-distro-basic-smithi/

Interpretation: Two of the failed jobs were related to this PR and a commit has been added to fix them. The other failures are unrelated.

@smithfarm
Copy link
Contributor Author

smithfarm commented Mar 5, 2018

Note to self: need to backport the build/ops (DEB) fix to luminous via http://tracker.ceph.com/issues/22457

Fixes: http://tracker.ceph.com/issues/21264
Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm smithfarm removed the needs-qa label Mar 6, 2018
This workunit is only used by the two rest-api test cases that are also being removed.

Fixes: http://tracker.ceph.com/issues/21264
Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm smithfarm merged commit 233fe83 into ceph:master Mar 6, 2018
@smithfarm smithfarm deleted the wip-21264 branch March 6, 2018 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants