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

debian/rpm: move radosgw-admin to ceph-common #14940

Merged
merged 1 commit into from May 12, 2017

Conversation

Projects
None yet
6 participants
@alimaredia
Contributor

alimaredia commented May 3, 2017

This commit moves the radosgw-admin command from
the radosgw package to ceph-common.

This allows for a better user expierence since the
radosgw-admin command can be run on any node.

Resolves: rhbz#1430079

Signed-off-by: Ali Maredia amaredia@redhat.com

@alimaredia

This comment has been minimized.

Contributor

alimaredia commented May 3, 2017

@ktdreyer I've still got to test this in teuthology and in ceph-ansible. Did I miss any files?

@alimaredia alimaredia changed the title from debian/rpm: move radosgw-admin to ceph-common to [DNM] debian/rpm: move radosgw-admin to ceph-common May 3, 2017

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented May 3, 2017

Yes, ceph-ansible testing as well as Teuthology testing is important.

Would you please change "Resolves: rhbz#1430079" to the upstream ticket http://tracker.ceph.com/issues/19577 instead? (See examples in git log)

debian/rpm: move radosgw-admin to ceph-common
This commit moves the radosgw-admin command from
the radosgw package to ceph-common.

This allows for a better user expierence since the
radosgw-admin command can be run on any node.

Fixes: http://tracker.ceph.com/issues/19577

Signed-off-by: Ali Maredia <amaredia@redhat.com>
@l-mb

This comment has been minimized.

l-mb commented May 4, 2017

Does this affect the required dependencies for ceph-common?

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 4, 2017

@l-mb it does, but it has been taken care of by ceph.spec.in and debian/control.

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented May 4, 2017

@tchaikov I don't think it affects it?

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 4, 2017

@ktdreyer, yes and no.

radowsgw-admin is linked against libfcgi, libssl, and libblkid. the subvar of ${shlibs:Depends} in debian/control will be updated automatically after including it in ceph-common. so ceph-common depended on them libs indirectly by depending on python-rados. now it depends on them directly.

as to the rpm packaging, the indirect dependency still works, that's why i said "it has been taken care of".

@ktdreyer ktdreyer requested a review from b-ranto May 4, 2017

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented May 4, 2017

Oh, I didn't know it linked libfcgi. http://tracker.ceph.com/issues/16784 is not yet fixed. (Adding Boris as a reviewer here so he sees this.)

@alimaredia

This comment has been minimized.

Contributor

alimaredia commented May 8, 2017

@b-ranto

b-ranto approved these changes May 9, 2017

It is a bit of an inconvenience to require fcgi in ceph-common but I guess, we can always strip it out where necessary.

@alimaredia

This comment has been minimized.

Contributor

alimaredia commented May 9, 2017

@tchaikov @b-ranto If I understand what you're saying correctly, nothing needs to be done in the PR regarding fcgi an this PR is not blocked on the future removal of fcgi from upstream?

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 10, 2017

@alimaredia exactly!

@alimaredia alimaredia changed the title from [DNM] debian/rpm: move radosgw-admin to ceph-common to debian/rpm: move radosgw-admin to ceph-common May 10, 2017

@alimaredia

This comment has been minimized.

Contributor

alimaredia commented May 10, 2017

Outside of the subset of the rgw teuthology suite I did, I'm not sure what other kind of testing I need to do. If no one else has any suggestions or further testing, this PR is ready to merge for me.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented May 11, 2017

retest this please.

@tchaikov

This comment has been minimized.

@tchaikov

This comment has been minimized.

@tchaikov tchaikov merged commit 05e2528 into master May 12, 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

@alimaredia alimaredia deleted the wip-radosgw-admin-package-move branch May 17, 2017

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