Merge lp:~peter-sabaini/charms/trusty/ceph-osd/ceph-osd-multijournal-next into lp:~openstack-charmers-archive/charms/trusty/ceph-osd/next

[Peter Sabaini]

  * Handle multiple journals

  * Select least-used journal disk instead of blind RR

  * Add safeguard for OSD journals

  * Avoid zapping journals repeatedly, add check for journal partition type

  * Check that journal devices don't overlap with data devices

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #14718 ceph-osd-next for peter-sabaini mp281149
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/14718/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #15770 ceph-osd-next for peter-sabaini mp281149
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: pastebin not avail., cmd error
Build: http://10.245.162.77:8080/job/charm_lint_check/15770/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #8384 ceph-osd-next for peter-sabaini mp281149
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/8384/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #16579 ceph-osd-next for peter-sabaini mp281149
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/14407597/
Build: http://10.245.162.77:8080/job/charm_lint_check/16579/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #15488 ceph-osd-next for peter-sabaini mp281149
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/15488/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #8504 ceph-osd-next for peter-sabaini mp281149
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/8504/

[Chris Holcombe (xfactor973)]

If you could just change that write over to being atomic I think this is good to go. I've love to deploy it to aws to test it.

review: Needs Fixing

[Peter Sabaini (peter-sabaini)]

Thanks for the review and the suggestion around the atomic write, will update!

[Chris MacNaughton (chris.macnaughton)]

Hey Peter, this looks great and I look forward to t hat change! If we want to get this landed for the 16.01 feature freeze, we need that change in soon!

Thanks again!

[lp:~peter-sabaini/charms/trusty/ceph-osd/ceph-osd-multijournal-next]

56. By Peter Sabaini on 2016-01-22

Make writing the zapfile safer

[Peter Sabaini (peter-sabaini)]

Heya Chris+Chris, hope that was what you were looking for?

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #17945 ceph-osd-next for peter-sabaini mp281149
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/14598402/
Build: http://10.245.162.77:8080/job/charm_lint_check/17945/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #16770 ceph-osd-next for peter-sabaini mp281149
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/16770/

[Chris Holcombe (xfactor973)]

Yes that's perfect. Thanks!

review: Approve

[Chris Holcombe (xfactor973)]

Peter Sabaini - If you could rebase and take care of that merge conflict we can land this :)

[James Page (james-page)]

Note that the lint on the python code also needs resolving:

hooks/ceph_hooks.py:108:1: E302 expected 2 blank lines, found 1
hooks/ceph_hooks.py:118:1: E302 expected 2 blank lines, found 1
hooks/ceph_hooks.py:126:1: E302 expected 2 blank lines, found 1
hooks/ceph_hooks.py:128:80: E501 line too long (85 > 79 characters)
hooks/ceph_hooks.py:133:1: E302 expected 2 blank lines, found 1
hooks/ceph_hooks.py:152:1: E302 expected 2 blank lines, found 1
hooks/ceph_hooks.py:217:1: E302 expected 2 blank lines, found 1
hooks/ceph_hooks.py:225:1: E302 expected 2 blank lines, found 1
hooks/ceph.py:113:44: E261 at least two spaces before inline comment
hooks/ceph.py:114:44: E261 at least two spaces before inline comment
hooks/ceph.py:115:5: E123 closing bracket does not match indentation of opening bracket's line
hooks/ceph.py:117:1: E302 expected 2 blank lines, found 1
hooks/ceph.py:314:1: E302 expected 2 blank lines, found 1
hooks/ceph.py:316:80: E501 line too long (80 > 79 characters)
hooks/ceph.py:322:1: E302 expected 2 blank lines, found 1
hooks/ceph.py:332:1: E302 expected 2 blank lines, found 1
hooks/ceph.py:337:1: E302 expected 2 blank lines, found 1
hooks/ceph.py:356:80: E501 line too long (85 > 79 characters)

Peter - do you have time to make these changes?

review: Needs Fixing

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #17028 ceph-osd-next for peter-sabaini mp281149
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/17028/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #18277 ceph-osd-next for peter-sabaini mp281149
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/14691030/
Build: http://10.245.162.77:8080/job/charm_lint_check/18277/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #9094 ceph-osd-next for peter-sabaini mp281149
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/14692078/
Build: http://10.245.162.77:8080/job/charm_amulet_test/9094/

[James Page (james-page)]

In the interests of getting this landed ASAP:

https://code.launchpad.net/~james-page/charms/trusty/ceph-osd/multijournal-fixes

Unmerged revisions

=== modified file 'hooks/ceph.py'
--- hooks/ceph.py	2016-01-12 14:16:54 +0000
+++ hooks/ceph.py	2016-01-22 15:57:27 +0000
@@ -22,9 +22,13 @@
 )
 from charmhelpers.core.hookenv import (
     log,
+<<<<<<< TREE
     ERROR,
     WARNING,
     cached,
+=======
+    ERROR, WARNING, DEBUG,
+>>>>>>> MERGE-SOURCE
     status_set,
 )
 from charmhelpers.fetch import (
@@ -170,16 +174,20 @@
     'btrfs'
 ]
 
+CEPH_PARTITIONS = [
+    '4FBD7E29-9D25-41B8-AFD0-062C0CEFF05D', # ceph osd data
+    '45B0969E-9B03-4F30-B4C6-B4B80CEFF106', # ceph osd journal
+    ]
 
 def is_osd_disk(dev):
     try:
         info = subprocess.check_output(['sgdisk', '-i', '1', dev])
         info = info.split("\n")  # IGNORE:E1103
         for line in info:
-            if line.startswith(
-                'Partition GUID code: 4FBD7E29-9D25-41B8-AFD0-062C0CEFF05D'
-            ):
-                return True
+            for ptype in CEPH_PARTITIONS:
+                sig = 'Partition GUID code: {}'.format(ptype)
+                if line.startswith(sig):
+                    return True
     except subprocess.CalledProcessError:
         pass
     return False
@@ -394,6 +402,28 @@
         with open(init_marker, 'w'):
             pass
 
+def maybe_zap_journal(journal_dev):
+    if (is_osd_disk(journal_dev)):
+        log('Looks like {} is already an OSD data or journal, skipping.'.format(
+            journal_dev))
+        return
+    zap_disk(journal_dev)
+    log("Zapped journal device {}".format(journal_dev))
+
+def get_partitions(dev):
+    cmd = ['partx', '--raw', '--noheadings', dev]
+    try:
+        out = subprocess.check_output(cmd).splitlines()
+        log("get partitions: {}".format(out), level=DEBUG)
+        return out
+    except subprocess.CalledProcessError as e:
+        log("Can't get info for {0}: {1}".format(dev, e.output))
+        return []
+
+def find_least_used_journal(journal_devices):
+    usages = map(lambda a: (len(get_partitions(a)), a), journal_devices)
+    least = min(usages, key=lambda t: t[0])
+    return least[1]
 
 def osdize(dev, osd_format, osd_journal, reformat_osd=False,
            ignore_errors=False):
@@ -414,7 +444,7 @@
         return
 
     if (is_osd_disk(dev) and not reformat_osd):
-        log('Looks like {} is already an OSD, skipping.'.format(dev))
+        log('Looks like {} is already an OSD data or journal, skipping.'.format(dev))
         return
 
     if is_device_mounted(dev):
@@ -431,8 +461,9 @@
         if reformat_osd:
             cmd.append('--zap-disk')
         cmd.append(dev)
-        if osd_journal and os.path.exists(osd_journal):
-            cmd.append(osd_journal)
+        if osd_journal:
+            least_used = find_least_used_journal(osd_journal)
+            cmd.append(least_used)
     else:
         # Just provide the device - no other options
         # for older versions of ceph
@@ -441,6 +472,7 @@
             zap_disk(dev)
 
     try:
+        log("osdize cmd: {}".format(cmd))
         subprocess.check_call(cmd)
     except subprocess.CalledProcessError as e:
         if ignore_errors:
 
=== modified file 'hooks/ceph_hooks.py'
--- hooks/ceph_hooks.py	2016-01-18 10:17:06 +0000
+++ hooks/ceph_hooks.py	2016-01-22 15:57:27 +0000
@@ -11,11 +11,13 @@
 import os
 import shutil
 import sys
+import tempfile
 
 import ceph
 from charmhelpers.core.hookenv import (
     log,
     ERROR,
+    DEBUG,
     config,
     relation_ids,
     related_units,
@@ -105,6 +107,30 @@
 
 JOURNAL_ZAPPED = '/var/lib/ceph/journal_zapped'
 
+def read_zapped_journals():
+    if os.path.exists(JOURNAL_ZAPPED):
+        with open(JOURNAL_ZAPPED) as zapfile:
+            zapped = set(
+                filter(None,
+                       [l.strip() for l in zapfile.readlines()]))
+            log("read zapped: {}".format(zapped), level=DEBUG)
+            return zapped
+    return set()
+
+def write_zapped_journals(journal_devs):
+    tmpfh, tmpfile = tempfile.mkstemp()
+    with os.fdopen(tmpfh, 'wb') as zapfile:
+        log("write zapped: {}".format(journal_devs),
+            level=DEBUG)
+        zapfile.write('\n'.join(sorted(list(journal_devs))))
+    os.rename(tmpfile, JOURNAL_ZAPPED)
+
+def check_overlap(journaldevs, datadevs):
+    if not journaldevs.isdisjoint(datadevs):
+        msg = "Journal/data devices mustn't overlap; journal: {0}, data: {1}".format(
+            journaldevs, datadevs)
+        log(msg, level=ERROR)
+        raise ValueError(msg)
 
 @hooks.hook('config-changed')
 def config_changed():
@@ -123,20 +149,24 @@
     e_mountpoint = config('ephemeral-unmount')
     if (e_mountpoint and ceph.filesystem_mounted(e_mountpoint)):
         umount(e_mountpoint)
+    prepare_disks_and_activate()
 
-    osd_journal = config('osd-journal')
-    if (osd_journal and not os.path.exists(JOURNAL_ZAPPED) and
-            os.path.exists(osd_journal)):
-        ceph.zap_disk(osd_journal)
-        with open(JOURNAL_ZAPPED, 'w') as zapped:
-            zapped.write('DONE')
+def prepare_disks_and_activate():
+    osd_journal = get_journal_devices()
+    check_overlap(osd_journal, set(get_devices()))
+    log("got journal devs: {}".format(osd_journal), level=DEBUG)
+    already_zapped = read_zapped_journals()
+    non_zapped = osd_journal - already_zapped
+    for journ in non_zapped:
+        ceph.maybe_zap_journal(journ)
+    write_zapped_journals(osd_journal)
 
     if ceph.is_bootstrapped():
         log('ceph bootstrapped, rescanning disks')
         emit_cephconf()
         for dev in get_devices():
             ceph.osdize(dev, config('osd-format'),
-                        config('osd-journal'), config('osd-reformat'),
+                        osd_journal, config('osd-reformat'),
                         config('ignore-device-errors'))
         ceph.start_osds(get_devices())
 
@@ -186,6 +216,13 @@
     else:
         return []
 
+def get_journal_devices():
+    osd_journal = config('osd-journal')
+    if not osd_journal:
+        return set()
+    osd_journal = [l.strip() for l in config('osd-journal').split(' ')]
+    osd_journal = set(filter(os.path.exists, osd_journal))
+    return osd_journal
 
 @hooks.hook('mon-relation-changed',
             'mon-relation-departed')
@@ -195,11 +232,7 @@
         log('mon has provided conf- scanning disks')
         emit_cephconf()
         ceph.import_osd_bootstrap_key(bootstrap_key)
-        for dev in get_devices():
-            ceph.osdize(dev, config('osd-format'),
-                        config('osd-journal'), config('osd-reformat'),
-                        config('ignore-device-errors'))
-        ceph.start_osds(get_devices())
+        prepare_disks_and_activate()
     else:
         log('mon cluster has not yet provided conf')