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-disk: dmcrypt cluster must default to ceph #16776

Merged
merged 1 commit into from Aug 4, 2017

Conversation

Projects
None yet
4 participants
@ghost

ghost commented Aug 3, 2017

If ceph_fsid is not found, which is the case for legacy dmcrypted OSD,
the cluster must default to ceph, as it was before support for non
standard cluster names was introduced.

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

Signed-off-by: Loic Dachary loic@dachary.org

@ghost ghost added bug fix core labels Aug 3, 2017

@ghost ghost requested a review from alram Aug 3, 2017

@ghost

This comment has been minimized.

ghost commented Aug 3, 2017

test this please

@ghost

This comment has been minimized.

ghost commented Aug 3, 2017

make check arm64 passes, pushing to a ceph-ci branch to run the suite https://shaman.ceph.com/builds/ceph/wip-20893-ceph-fsid/

@ircolle ircolle requested a review from alfredodeza Aug 3, 2017

@liewegas

this looks right to me!

@liewegas liewegas added this to the luminous milestone Aug 3, 2017

@ghost

This comment has been minimized.

ghost commented Aug 3, 2017

teuthology-suite -k distro --verbose --suite ceph-disk --ceph wip-20893-ceph-fsid --machine-type vps
@liewegas

This comment has been minimized.

Member

liewegas commented Aug 3, 2017

retest this please

if cluster is None:
raise Error('No cluster conf found in ' + SYSCONFDIR +
' with fsid %s' % ceph_fsid)
cluster = 'ceph'

This comment has been minimized.

@alfredodeza

alfredodeza Aug 3, 2017

Contributor

It would be useful here to know when this is happening by adding a log line like:

LOG.warning("no `ceph_fsid` found at %s, falling back to 'ceph' for cluster name", ceph_fsid) 

This comment has been minimized.

@ghost

ghost Aug 3, 2017

I'd rather keep this bug fix to the bare minimum for the sake of backporting.

This comment has been minimized.

@ircolle

ircolle Aug 3, 2017

A log warning is difficult to backport, @dachary?

This comment has been minimized.

@ghost

ghost Aug 3, 2017

@ircolle everything is difficult to backport

This comment has been minimized.

@ghost

ghost Aug 3, 2017

I'll do what alfredo suggests.

@ghost

This comment has been minimized.

ghost commented Aug 3, 2017

repushed with Alfredo suggestion although, for the record, I don't think it's a wise idea at this stage

ceph-disk: dmcrypt cluster must default to ceph
If ceph_fsid is not found, which is the case for legacy dmcrypted OSD,
the cluster must default to ceph, as it was before support for non
standard cluster names was introduced.

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

Signed-off-by: Loic Dachary <loic@dachary.org>
@alfredodeza

This comment has been minimized.

Contributor

alfredodeza commented Aug 4, 2017

jenkins test this please

@alfredodeza alfredodeza merged commit 0d70d36 into ceph:master Aug 4, 2017

4 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment