Ceph » devops

Well that was a fast turnaround!

A minor niggle - I note the osd code does:

init = lsb.choose_init(distro, codename)

to get the right init type - would it not make sense to use the same logic for the mons too?

In fact like this seems to work:

diff --git a/ceph_deploy/hosts/common.py b/ceph_deploy/hosts/common.py
index f5e3b8a..de6fdbf 100644
--- a/ceph_deploy/hosts/common.py
+++ b/ceph_deploy/hosts/common.py
@@ -3,6 +3,7 @@ from ceph_deploy.util.wrappers import check_call
 from ceph_deploy.util.context import remote
 from ceph_deploy import conf
 from StringIO import StringIO
+from .. import lsb

 def ceph_version(conn, logger):
@@ -31,10 +32,7 @@ def mon_create(distro, logger, args, monitor_keyring, hostname):
     logger.debug('remote hostname: %s' % hostname)
     path = paths.mon.path(args.cluster, hostname)
     done_path = paths.mon.done(args.cluster, hostname)
-    if distro.name.lower() == 'ubuntu':
-        init = 'upstart'
-    else:
-        init = 'sysvinit'
+    init = lsb.choose_init(distro.name, distro.codename)

     init_path = paths.mon.init(args.cluster, hostname, init)

Mark,

Are you sure you are able to replicate this with the latest release of ceph-deploy?

This commit fixed that: https://github.com/ceph/ceph-deploy/commit/1d2d9881f93df6383d99720c84d6e7397a04bba0

The reason we are no longer using lsb for this is because we are deliberately wanting to use that abstraction in one place: ceph_deploy/hosts/__init__.py (although it does not use lsb.choose_init)

I am OK though accepting the patch to use lsb.choose_init. But I am still curious if you were able to replicate this problem without these changes and the latest release.

With your commit 1d2d9881f93df6383d99720c84d6e7397a04bba0 the problem is solved. Absolutely. I was merely thinking that it made sense to use the logic in lsb rather than sprinkling "if distro == "ubuntu" in the actual mon code. Also makes the on code look like the osd code, that made sense too.

Given the origin of this bug, I was thinking that the fewer places logic to do with choosing which init was duplicated the better.

I like the idea of pushing that sort of logic into ceph_deploy/hosts/__init__.py - so maybe you should use lsb.choose_init there?

Yeah - no pressure for using my patches, I was just wanting to illustrate that using lsb was actually pretty tidy :-)

Mark, you are spot on.

I will create a new ticket so I can fix this and apply your branch, unless you want to submit a pull request?

That way I am not taking credit for your (much better) implementation :)

Lol - I think opening a new ticket is fine. You can mention that you have discussed the proposed change with me.

Hmm - I don't currently have a (remote) branch setup, I'm just hacking away on master :-)

I'll have a think about how to adapt the patch(es) above to the idea or moving the init deciding to init.py, and if I get anything interesting happening there I'll note it on the (new) ticket!

I created a branch and applied your patch with one (minor) change which was to use `distro.init` for the RuntimeError instead of `init` (init was never defined)

Can you take a look and comment (and/or accept the pull request) ?

Pull Request: https://github.com/ceph/ceph-deploy/pull/40

Looks good to me. Will see if I can accept pull request.