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
ceph-volume: Require lvm2, move to osd package #19529
Conversation
This looks OK to me. Unsure if the Debian control files will need it to. |
If it's just ceph-volume that needs the LVM utils, we can put this requirement in the "ceph-osd" package instead of "ceph-base". Good call on the |
ceph-disk used to be in the ceph-osd package, but was moved to ceph-base by #14871 ceph-volume has been in ceph-base since its inception, but I'm not sure there's a good reason for that. . . for example, it means ceph-volume will be included on mon-only nodes, mds-only nodes, etc. It seems strange to put the Would it be worth considering to move both ceph-volume and its lvm2 dependency to ceph-osd? |
I don't think ceph-volume should be in anything other than ceph-osd, and I think it should be moved. |
@b-ranto can you confirm that there's no reason for ceph-volume to be in ceph-base now? (was ceph-disk only needed there in order to support the previous upgrade?) |
Good call. It's a discrepancy between the Debian and RPM packaging (ceph-volume is in I think ceph-volume should be in the ceph-osd package for both distro families. |
i concur with @smithfarm and @ktdreyer . |
891a247
to
2704ac7
Compare
anything else that needs to be done? |
I'll let others take a look as well, building these changes now so we can run some tests: |
This change passed functional ceph-volume tests. Looks OK to me. |
Would you mind making the change to the |
Fixes: http://tracker.ceph.com/issues/22443 Signed-off-by: Theofilos Mouratidis <t.mour@cern.ch>
@liewegas Yep, we only needed ceph-disk binary in ceph-base so that we could use the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This does not work on the Debian/Ubuntu side of things. Unsure what to suggest here.
Jenkins logs https://jenkins.ceph.com/job/ceph-volume-prs-lvm-xenial-bluestore-create/26/console |
centos7-bluestore certainly worked fine |
turns out ceph-mgr was linked against libstdc++6 from the PPA repo dynamically. @alfredodeza you need to rebase your branch against master to include 75c5e5c, and rebuild it. sorry for the inconvenience =( |
This is good to go. |
Fixes: http://tracker.ceph.com/issues/22443
Signed-off-by: Theofilos Mouratidis t.mour@cern.ch