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

kraken: core: ceph-disk: does not support cluster names different than 'ceph' #13497

Merged
1 commit merged into from Apr 9, 2017

Conversation

shinobu-x
Copy link
Contributor

@liewegas liewegas modified the milestone: kraken Feb 17, 2017
@smithfarm smithfarm requested a review from a user February 21, 2017 21:07
@ghost
Copy link

ghost commented Feb 21, 2017

you'll need #13566 as well, as soon as it's merged

@ghost ghost changed the title kraken: ceph-disk does not support cluster names different than 'ceph' DNM: kraken: ceph-disk does not support cluster names different than 'ceph' Feb 23, 2017
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Please cherry-pick 7f66672 instead

@shinobu-x
Copy link
Contributor Author

@dachary Please check

$ git cherry-pick -x 7f66672

error: could not apply 7f66672... ceph-disk: dmcrypt activate must use the same cluster as prepare

$ git status

# On branch wip-17821-kraken
snip
#	both modified:      src/ceph-disk/ceph_disk/main.py
<<<<<<< HEAD
        self.create_key()
=======
        if self.args.cluster_uuid is None:
            self.args.cluster_uuid = get_fsid(cluster=self.args.cluster)
        write_one_line(path, 'ceph_fsid', self.args.cluster_uuid)
        self.create_key(self.args.cluster)
>>>>>>> 7f66672...

$ git diff 16fc6d8 a88b267 src/ceph-disk/ceph_disk/main.py

snip
-        self.create_key()
+        if self.args.cluster_uuid is None:
+            self.args.cluster_uuid = get_fsid(cluster=self.args.cluster)
+        write_one_line(path, 'ceph_fsid', self.args.cluster_uuid)
+        self.create_key(self.args.cluster)

@ghost
Copy link

ghost commented Feb 24, 2017

self.create_key() must not be modified at all by the patch. It is part of the context and irrelevant to this commit.

@shinobu-x
Copy link
Contributor Author

@dachary Yeah that's perfectly my bad =) Sorry for that.

@shinobu-x
Copy link
Contributor Author

Yay finally I'm completely clear about what the title means :) It's taken a long time..., will update.

ceph-disk does not support cluster names different than 'ceph'

@shinobu-x
Copy link
Contributor Author

@dachary Please check. Thank you for your patience :)

$ git diff --diff-filter=U
diff --cc src/ceph-disk/ceph_disk/main.py
index dc35c2f,c9f0041..0000000
--- a/src/ceph-disk/ceph_disk/main.py
+++ b/src/ceph-disk/ceph_disk/main.py
@@@ -2509,7 -2552,10 +2520,14 @@@ class Lockbox(object)
          LOG.debug('Mounting lockbox ' + str(" ".join(args)))
          command_check_call(args)
          write_one_line(path, 'osd-uuid', self.args.osd_uuid)
++<<<<<<< HEAD
+        self.create_key()
++=======
+         if self.args.cluster_uuid is None:
+             self.args.cluster_uuid = get_fsid(cluster=self.args.cluster)
+         write_one_line(path, 'ceph_fsid', self.args.cluster_uuid)
+         self.create_key(self.args.cluster)
++>>>>>>> 7f66672... ceph-disk: dmcrypt activate must use the same cluster as pr
          self.symlink_spaces(path)
          write_one_line(path, 'magic', CEPH_LOCKBOX_ONDISK_MAGIC)
          if self.device is not None:
$ git diff --diff-filter=U
diff --cc src/ceph-disk/ceph_disk/main.py
index dc35c2f,c9f0041..0000000
--- a/src/ceph-disk/ceph_disk/main.py
+++ b/src/ceph-disk/ceph_disk/main.py
@@@ -2509,7 -2552,10 +2520,10 @@@ class Lockbox(object)
          LOG.debug('Mounting lockbox ' + str(" ".join(args)))
          command_check_call(args)
          write_one_line(path, 'osd-uuid', self.args.osd_uuid)
+        self.create_key()
+         if self.args.cluster_uuid is None:
+             self.args.cluster_uuid = get_fsid(cluster=self.args.cluster)
+         write_one_line(path, 'ceph_fsid', self.args.cluster_uuid)

@ghost
Copy link

ghost commented Feb 24, 2017

In your file what is right after

+         write_one_line(path, 'ceph_fsid', self.args.cluster_uuid)

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>
(cherry picked from commit 7f66672)

Conflicts:
	src/ceph-disk/ceph_disk/main.py
@shinobu-x
Copy link
Contributor Author

@dachary Oh that's cherry-pick! Please :)

@@ -2509,6 +2520,9 @@ class Lockbox(object):
         LOG.debug('Mounting lockbox ' + str(" ".join(args)))
         command_check_call(args)
         write_one_line(path, 'osd-uuid', self.args.osd_uuid)
+        if self.args.cluster_uuid is None:
+            self.args.cluster_uuid = get_fsid(cluster=self.args.cluster)
+        write_one_line(path, 'ceph_fsid', self.args.cluster_uuid)
         self.create_key()

@ghost ghost changed the title DNM: kraken: ceph-disk does not support cluster names different than 'ceph' kraken: ceph-disk does not support cluster names different than 'ceph' Feb 24, 2017
@ghost
Copy link

ghost commented Feb 24, 2017

that's better :-)

@shinobu-x
Copy link
Contributor Author

@dachary Thanks, very very appreciated!!

@shinobu-x
Copy link
Contributor Author

jenkins test this please

@smithfarm
Copy link
Contributor

@shinobu-x This commit message is missing a description of how the conflict was resolved.

@smithfarm smithfarm changed the title kraken: ceph-disk does not support cluster names different than 'ceph' kraken: ceph-disk: does not support cluster names different than 'ceph' Apr 7, 2017
@smithfarm
Copy link
Contributor

@dachary This passed a ceph-disk suite at http://tracker.ceph.com/issues/19009#note-5

OK to merge (note: the commit message is missing a description of how the conflict was resolved)?

@ghost ghost merged commit fb29415 into ceph:kraken Apr 9, 2017
@smithfarm smithfarm changed the title kraken: ceph-disk: does not support cluster names different than 'ceph' kraken: core: ceph-disk: does not support cluster names different than 'ceph' Aug 2, 2017
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants