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 activate must use the same cluster as prepare #13573

Merged
3 commits merged into from Feb 22, 2017

Conversation

Projects
None yet
4 participants
@ghost

ghost commented Feb 21, 2017

When dmcrypt is used, the fsid cannot be retrieved from the data
partition because it is encrypted. Store the fsid in the lockbox to
enable dmcrypt activation using the same logic as regular activation.

The fsid is used to retrive the cluster name that was used during
prepare, reason why activation does not and must not have a --cluster
argument.

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

Signed-off-by: Loic Dachary ldachary@redhat.com

ldachary added some commits Feb 21, 2017

Revert "ceph-disk: Adding cluster name support for dmcrypt"
Merged by mistake, teuthology ceph-disk fails

This reverts commit d98d4db.

Signed-off-by: Loic Dachary <ldachary@redhat.com>
Revert "ceph-disk: change get_dmcrypt_key test to support different c…
…luster name"

Merged by mistake, teuthology ceph-disk fails

This reverts commit 1c74747.

Signed-off-by: Loic Dachary <ldachary@redhat.com>

@ghost ghost added bug fix core labels Feb 21, 2017

@ghost ghost requested a review from leseb Feb 21, 2017

@ghost

This comment has been minimized.

@ghost ghost changed the title from revert dmcrypt ceph-disk to ceph-disk: revert dmcrypt activate patch Feb 21, 2017

ceph-disk: dmcrypt activate must use the same cluster as prepare
When dmcrypt is used, the fsid cannot be retrieved from the data
partition because it is encrypted. Store the fsid in the lockbox to
enable dmcrypt activation using the same logic as regular activation.

The fsid is used to retrive the cluster name that was used during
prepare, reason why activation does not and must not have a --cluster
argument.

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

Signed-off-by: Loic Dachary <ldachary@redhat.com>

@ghost ghost changed the title from ceph-disk: revert dmcrypt activate patch to ceph-disk: dmcrypt activate must use the same cluster as prepare Feb 22, 2017

@ghost

This comment has been minimized.

ghost commented Feb 22, 2017

teuthology-suite -k distro --verbose --suite ceph-disk --ceph wip-17821-revert --machine-type vps 

pass http://pulpito.ceph.com/loic-2017-02-22_05:31:46-ceph-disk-wip-17821-revert-distro-basic-vps

@ghost ghost requested a review from tchaikov Feb 22, 2017

@ghost

This comment has been minimized.

ghost commented Feb 22, 2017

teuthology suite for ceph-disk passes #13573 (comment)

@ghost

This comment has been minimized.

ghost commented Feb 22, 2017

There are no specific tests for this change. But it modifies the code path used by every dmcrypt activation and I believe that's enough coverage, via the teuthology ceph-disk suite, to prove it does the right thing.

@tchaikov

since the problem attacked by #11786 is a false problem, we should revert it.

@ghost ghost merged commit 51f4e3b into ceph:master Feb 22, 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
@leseb

This comment has been minimized.

Contributor

leseb commented Apr 4, 2017

@dachary @vumrao will this be in 10.2.7 by any chance?

@leseb

This comment has been minimized.

Contributor

leseb commented Apr 4, 2017

@ktdreyer

This comment has been minimized.

Member

ktdreyer commented Apr 4, 2017

@leseb the backport is in progress at #13496 . It looks like Loic has requested changes of the reviewer. If this needs to get into v10.2.7, please work with @shinobu-x on that. At this point in the schedule anything for v10.2.7 must have Pulpito links to successful Teuthology runs demonstrating that the change is safe to take in.

This issue was closed.

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