Merge lp:~xfactor973/charm-helpers/ceph-functions into lp:charm-helpers

[Chris Holcombe]

This change adds many additional functions for working with Ceph. Most of the changes are centered around erasure coding support. I added unit tests to bring the testing coverage to 93%. This patch also introduces support for adding/removing cache tiers from existing ceph pools.

I also added a new pgs calculation which is based on ceph's latest documentation recommending that we round to the nearest power of 2. Since rounding to the nearest power is extremely slow in python ( many seconds) I have opted to store the answers in a list called powers_of_two. I'm not creative with names.

You will also notice a function called validator. This function was born because of this documentation: http://docs.ceph.com/docs/master/rados/operations/pools/#set-pool-values You'll notice that Ceph allows you to set many different pool values and they all accept a different type and range of input. To help people I have included helpful error messages to indicate 1) where the problem is and 2) How they might go about fixing it.

[Chris Holcombe (xfactor973)]

Results from make test2: https://gist.github.com/cholcombe973/a6603ed6e8820ad323d2

[James Page (james-page)]

$ make test
Checking for Python syntax...
tests/contrib/storage/test_linux_ceph.py:259:5: F811 redefinition of unused 'test_replicated_pool_create_failed' from line 253
Makefile:73: recipe for target 'lint' faile

[James Page (james-page)]

python 3 tests as also erroring- discussed with Chris in irc....

review: Needs Fixing

[lp:~xfactor973/charm-helpers/ceph-functions]

479. By Chris Holcombe on 2015-11-17

Add basestring from the past

[James Page (james-page)]

You might want to make use of six to deal with the py2/py3 compatibility:

  https://pythonhosted.org/six/#six.string_types

======================================================================
ERROR: Failure: ImportError (No module named past.builtins)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jamespage/src/charm-helpers/charm-helpers/.venv/local/lib/python2.7/site-packages/nose/loader.py", line 418, in loadTestsFromName
    addr.filename, addr.module)
  File "/home/jamespage/src/charm-helpers/charm-helpers/.venv/local/lib/python2.7/site-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqname)
  File "/home/jamespage/src/charm-helpers/charm-helpers/.venv/local/lib/python2.7/site-packages/nose/importer.py", line 94, in importFromDir
    mod = load_module(part_fqname, fh, filename, desc)
  File "/home/jamespage/src/charm-helpers/charm-helpers/tests/contrib/storage/test_linux_ceph.py", line 4, in <module>
    from past.builtins import basestring
ImportError: No module named past.builtins

[lp:~xfactor973/charm-helpers/ceph-functions]

480. By Chris Holcombe on 2015-11-18

Switch to more compatible six for python 2 and 3 basestring support

481. By Chris Holcombe on 2015-11-30

Merge upstream

482. By Chris Holcombe on 2015-11-30

Remove duplicated fns

[James Page (james-page)]

Hi Chris

I'm still seeing test failures under python3:

======================================================================
ERROR: tests.contrib.storage.test_linux_ceph.CephUtilsTests.test_erasure_pool_create_failed
----------------------------------------------------------------------
testtools.testresult.real._StringException: Traceback (most recent call last):
  File "/home/jamespage/src/charm-helpers/charm-helpers/tests/contrib/storage/test_linux_ceph.py", line 270, in test_erasure_pool_create_failed
    p.create()
  File "/home/jamespage/src/charm-helpers/charm-helpers/charmhelpers/contrib/storage/linux/ceph.py", line 228, in create
    erasure_profile = get_erasure_profile(service=self.service, name=self.erasure_code_profile)
  File "/home/jamespage/src/charm-helpers/charm-helpers/charmhelpers/contrib/storage/linux/ceph.py", line 265, in get_erasure_profile
    return json.loads(out)
  File "/usr/lib/python3.4/json/__init__.py", line 312, in loads
    s.__class__.__name__))
TypeError: the JSON object must be str, not 'MagicMock'

======================================================================
ERROR: tests.contrib.storage.test_linux_ceph.CephUtilsTests.test_get_cache_mode
----------------------------------------------------------------------
testtools.testresult.real._StringException: Traceback (most recent call last):
  File "/home/jamespage/src/charm-helpers/charm-helpers/tests/contrib/storage/test_linux_ceph.py", line 389, in test_get_cache_mode
    cache_mode = ceph_utils.get_cache_mode(service='admin', pool_name='rbd')
  File "/home/jamespage/src/charm-helpers/charm-helpers/charmhelpers/contrib/storage/linux/ceph.py", line 434, in get_cache_mode
    osd_json = json.loads(out)
  File "/usr/lib/python3.4/json/__init__.py", line 312, in loads
    s.__class__.__name__))
TypeError: the JSON object must be str, not 'bytes'

======================================================================
ERROR: tests.contrib.storage.test_linux_ceph.CephUtilsTests.test_pool_set_fails
----------------------------------------------------------------------
testtools.testresult.real._StringException: Traceback (most recent call last):
  File "/home/jamespage/src/charm-helpers/charm-helpers/tests/contrib/storage/test_linux_ceph.py", line 304, in test_pool_set_fails
    ceph_utils.pool_set(service='admin', pool_name='data', key='test', value=2)
  File "/home/jamespage/src/charm-helpers/charm-helpers/charmhelpers/contrib/storage/linux/ceph.py", line 281, in pool_set
    check_call(cmd)
  File "/home/jamespage/src/charm-helpers/charm-helpers/.venv3/lib/python3.4/site-packages/mock.py", line 955, in __call__
    return _mock_self._mock_call(*args, **kwargs)
  File "/home/jamespage/src/charm-helpers/charm-helpers/.venv3/lib/python3.4/site-packages/mock.py", line 1010, in _mock_call
    raise effect
TypeError: __init__() missing 2 required positional arguments: 'returncode' and 'cmd'

======================================================================
ERROR: tests.contrib.storage.test_linux_ceph.CephUtilsTests.test_replicated_pool_create_failed
----------------------------------------------------------------------
testtools.testresult.real._Str...

review: Needs Fixing

[James Page (james-page)]

Hi Chris

I've tried every which way to get the unit tests to pass under python3, but even in a clean trusty container, I see the same failures.

unittest.expectedFailure is to catchall for your intent and was masking a number of mock setup failures under python2, which got raised as errors under python3.

I've reworked to use assertRaises and fixed another misc issue:

  https://code.launchpad.net/~james-page/charm-helpers/assertRaises/+merge/279990

With this branch merged, all tests pass.

review: Needs Fixing

[Chris Holcombe (xfactor973)]

It looks good to me! Thanks for doing this

On 12/09/2015 02:15 AM, James Page wrote:
> Review: Needs Fixing
>
> Hi Chris
>
> I've tried every which way to get the unit tests to pass under python3, but even in a clean trusty container, I see the same failures.
>
> unittest.expectedFailure is to catchall for your intent and was masking a number of mock setup failures under python2, which got raised as errors under python3.
>
> I've reworked to use assertRaises and fixed another misc issue:
>
> https://code.launchpad.net/~james-page/charm-helpers/assertRaises/+merge/279990
>
> With this branch merged, all tests pass.

=== modified file '.bzrignore'
--- .bzrignore	2015-02-10 22:24:39 +0000
+++ .bzrignore	2015-11-30 18:12:02 +0000
@@ -8,6 +8,7 @@
 .env/
 coverage.xml
 docs/_build
+.idea
 .project
 .pydevproject
 .settings

=== modified file 'charmhelpers/contrib/storage/linux/ceph.py'
--- charmhelpers/contrib/storage/linux/ceph.py	2015-11-19 18:44:50 +0000
+++ charmhelpers/contrib/storage/linux/ceph.py	2015-11-30 18:12:02 +0000
@@ -23,10 +23,11 @@
 #  James Page <[e-mail]>
 #  Adam Gandelman <[e-mail]>
 #
+import bisect
+import six
 
 import os
 import shutil
-import six
 import json
 import time
 import uuid
@@ -73,6 +74,394 @@
 err to syslog = {use_syslog}
 clog to syslog = {use_syslog}
 """
+# For 50 < osds < 240,000 OSDs (Roughly 1 Exabyte at 6T OSDs)
+powers_of_two = [8192, 16384, 32768, 65536, 131072, 262144, 524288, 1048576, 2097152, 4194304, 8388608]
+
+
+def validator(value, valid_type, valid_range=None):
+    """
+    Used to validate these: http://docs.ceph.com/docs/master/rados/operations/pools/#set-pool-values
+    Example input:
+        validator(value=1,
+                  valid_type=int,
+                  valid_range=[0, 2])
+    This says I'm testing value=1.  It must be an int inclusive in [0,2]
+
+    :param value: The value to validate
+    :param valid_type: The type that value should be.
+    :param valid_range: A range of values that value can assume.
+    :return:
+    """
+    assert isinstance(value, valid_type), "{} is not a {}".format(
+        value,
+        valid_type)
+    if valid_range is not None:
+        assert isinstance(valid_range, list), \
+            "valid_range must be a list, was given {}".format(valid_range)
+        # If we're dealing with strings
+        if valid_type is six.string_types:
+            assert value in valid_range, \
+                "{} is not in the list {}".format(value, valid_range)
+        # Integer, float should have a min and max
+        else:
+            if len(valid_range) != 2:
+                raise ValueError(
+                    "Invalid valid_range list of {} for {}.  "
+                    "List must be [min,max]".format(valid_range, value))
+            assert value >= valid_range[0], \
+                "{} is less than minimum allowed value of {}".format(
+                    value, valid_range[0])
+            assert value <= valid_range[1], \
+                "{} is greater than maximum allowed value of {}".format(
+                    value, valid_range[1])
+
+
+class PoolCreationError(Exception):
+    """
+    A custom error to inform the caller that a pool creation failed.  Provides an error message
+    """
+    def __init__(self, message):
+        super(PoolCreationError, self).__init__(message)
+
+
+class Pool(object):
+    """
+    An object oriented approach to Ceph pool creation. This base class is inherited by ReplicatedPool and ErasurePool.
+    Do not call create() on this base class as it will not do anything.  Instantiate a child class and call create().
+    """
+    def __init__(self, service, name):
+        self.service = service
+        self.name = name
+
+    # Create the pool if it doesn't exist already
+    # To be implemented by subclasses
+    def create(self):
+        pass
+
+    def add_cache_tier(self, cache_pool, mode):
+        """
+        Adds a new cache tier to an existing pool.
+        :param cache_pool: six.string_types.  The cache tier pool name to add.
+        :param mode: six.string_types. The caching mode to use for this pool.  valid range = ["readonly", "writeback"]
+        :return: None
+        """
+        # Check the input types and values
+        validator(value=cache_pool, valid_type=six.string_types)
+        validator(value=mode, valid_type=six.string_types, valid_range=["readonly", "writeback"])
+
+        check_call(['ceph', '--id', self.service, 'osd', 'tier', 'add', self.name, cache_pool])
+        check_call(['ceph', '--id', self.service, 'osd', 'tier', 'cache-mode', cache_pool, mode])
+        check_call(['ceph', '--id', self.service, 'osd', 'tier', 'set-overlay', self.name, cache_pool])
+        check_call(['ceph', '--id', self.service, 'osd', 'pool', 'set', cache_pool, 'hit_set_type', 'bloom'])
+
+    def remove_cache_tier(self, cache_pool):
+        """
+        Removes a cache tier from Ceph.  Flushes all dirty objects from writeback pools and waits for that to complete.
+        :param cache_pool: six.string_types.  The cache tier pool name to remove.
+        :return: None
+        """
+        # read-only is easy, writeback is much harder
+        mode = get_cache_mode(cache_pool)
+        if mode == 'readonly':
+            check_call(['ceph', '--id', self.service, 'osd', 'tier', 'cache-mode', cache_pool, 'none'])
+            check_call(['ceph', '--id', self.service, 'osd', 'tier', 'remove', self.name, cache_pool])
+
+        elif mode == 'writeback':
+            check_call(['ceph', '--id', self.service, 'osd', 'tier', 'cache-mode', cache_pool, 'forward'])
+            # Flush the cache and wait for it to return
+            check_call(['ceph', '--id', self.service, '-p', cache_pool, 'cache-flush-evict-all'])
+            check_call(['ceph', '--id', self.service, 'osd', 'tier', 'remove-overlay', self.name])
+            check_call(['ceph', '--id', self.service, 'osd', 'tier', 'remove', self.name, cache_pool])
+
+    def get_pgs(self, pool_size):
+        """
+        :param pool_size: int. pool_size is either the number of replicas for replicated pools or the K+M sum for
+            erasure coded pools
+        :return: int.  The number of pgs to use.
+        """
+        validator(value=pool_size, valid_type=int)
+        osds = get_osds(self.service)
+        if not osds:
+            # NOTE(james-page): Default to 200 for older ceph versions
+            # which don't support OSD query from cli
+            return 200
+
+        # Calculate based on Ceph best practices
+        if osds < 5:
+            return 128
+        elif 5 < osds < 10:
+            return 512
+        elif 10 < osds < 50:
+            return 4096
+        else:
+            estimate = (osds * 100) / pool_size
+            # Return the next nearest power of 2
+            index = bisect.bisect_right(powers_of_two, estimate)
+            return powers_of_two[index]
+
+
+class ReplicatedPool(Pool):
+    def __init__(self, service, name, replicas=2):
+        super(ReplicatedPool, self).__init__(service=service, name=name)
+        self.replicas = replicas
+
+    def create(self):
+        if not pool_exists(self.service, self.name):
+            # Create it
+            pgs = self.get_pgs(self.replicas)
+            cmd = ['ceph', '--id', self.service, 'osd', 'pool', 'create', self.name, str(pgs)]
+            try:
+                check_call(cmd)
+            except CalledProcessError:
+                raise
+
+
+# Default jerasure erasure coded pool
+class ErasurePool(Pool):
+    def __init__(self, service, name, erasure_code_profile="default"):
+        super(ErasurePool, self).__init__(service=service, name=name)
+        self.erasure_code_profile = erasure_code_profile
+
+    def create(self):
+        if not pool_exists(self.service, self.name):
+            # Try to find the erasure profile information so we can properly size the pgs
+            erasure_profile = get_erasure_profile(service=self.service, name=self.erasure_code_profile)
+
+            # Check for errors
+            if erasure_profile is None:
+                log(message='Failed to discover erasure_profile named={}'.format(self.erasure_code_profile),
+                    level=ERROR)
+                raise PoolCreationError(message='unable to find erasure profile {}'.format(self.erasure_code_profile))
+            if 'k' not in erasure_profile or 'm' not in erasure_profile:
+                # Error
+                log(message='Unable to find k (data chunks) or m (coding chunks) in {}'.format(erasure_profile),
+                    level=ERROR)
+                raise PoolCreationError(
+                    message='unable to find k (data chunks) or m (coding chunks) in {}'.format(erasure_profile))
+
+            pgs = self.get_pgs(int(erasure_profile['k']) + int(erasure_profile['m']))
+            # Create it
+            cmd = ['ceph', '--id', self.service, 'osd', 'pool', 'create', self.name, str(pgs),
+                   'erasure', self.erasure_code_profile]
+            try:
+                check_call(cmd)
+            except CalledProcessError:
+                raise
+
+    """Get an existing erasure code profile if it already exists.
+       Returns json formatted output"""
+
+
+def get_erasure_profile(service, name):
+    """
+    :param service: six.string_types. The Ceph user name to run the command under
+    :param name:
+    :return:
+    """
+    try:
+        out = check_output(['ceph', '--id', service,
+                            'osd', 'erasure-code-profile', 'get',
+                            name, '--format=json'])
+        return json.loads(out)
+    except (CalledProcessError, OSError, ValueError):
+        return None
+
+
+def pool_set(service, pool_name, key, value):
+    """
+    Sets a value for a RADOS pool in ceph.
+    :param service: six.string_types. The Ceph user name to run the command under
+    :param pool_name: six.string_types
+    :param key: six.string_types
+    :param value:
+    :return: None.  Can raise CalledProcessError
+    """
+    cmd = ['ceph', '--id', service, 'osd', 'pool', 'set', pool_name, key, value]
+    try:
+        check_call(cmd)
+    except CalledProcessError:
+        raise
+
+
+def snapshot_pool(service, pool_name, snapshot_name):
+    """
+    Snapshots a RADOS pool in ceph.
+    :param service: six.string_types. The Ceph user name to run the command under
+    :param pool_name: six.string_types
+    :param snapshot_name: six.string_types
+    :return: None.  Can raise CalledProcessError
+    """
+    cmd = ['ceph', '--id', service, 'osd', 'pool', 'mksnap', pool_name, snapshot_name]
+    try:
+        check_call(cmd)
+    except CalledProcessError:
+        raise
+
+
+def remove_pool_snapshot(service, pool_name, snapshot_name):
+    """
+    Remove a snapshot from a RADOS pool in ceph.
+    :param service: six.string_types. The Ceph user name to run the command under
+    :param pool_name: six.string_types
+    :param snapshot_name: six.string_types
+    :return: None.  Can raise CalledProcessError
+    """
+    cmd = ['ceph', '--id', service, 'osd', 'pool', 'rmsnap', pool_name, snapshot_name]
+    try:
+        check_call(cmd)
+    except CalledProcessError:
+        raise
+
+
+# max_bytes should be an int or long
+def set_pool_quota(service, pool_name, max_bytes):
+    """
+    :param service: six.string_types. The Ceph user name to run the command under
+    :param pool_name: six.string_types
+    :param max_bytes: int or long
+    :return: None.  Can raise CalledProcessError
+    """
+    # Set a byte quota on a RADOS pool in ceph.
+    cmd = ['ceph', '--id', service, 'osd', 'pool', 'set-quota', pool_name, 'max_bytes', max_bytes]
+    try:
+        check_call(cmd)
+    except CalledProcessError:
+        raise
+
+
+def remove_pool_quota(service, pool_name):
+    """
+    Set a byte quota on a RADOS pool in ceph.
+    :param service: six.string_types. The Ceph user name to run the command under
+    :param pool_name: six.string_types
+    :return: None.  Can raise CalledProcessError
+    """
+    cmd = ['ceph', '--id', service, 'osd', 'pool', 'set-quota', pool_name, 'max_bytes', '0']
+    try:
+        check_call(cmd)
+    except CalledProcessError:
+        raise
+
+
+def create_erasure_profile(service, profile_name, erasure_plugin_name='jerasure', failure_domain='host',
+                           data_chunks=2, coding_chunks=1,
+                           locality=None, durability_estimator=None):
+    """
+    Create a new erasure code profile if one does not already exist for it.  Updates
+    the profile if it exists. Please see http://docs.ceph.com/docs/master/rados/operations/erasure-code-profile/
+    for more details
+    :param service: six.string_types. The Ceph user name to run the command under
+    :param profile_name: six.string_types
+    :param erasure_plugin_name: six.string_types
+    :param failure_domain: six.string_types.  One of ['chassis', 'datacenter', 'host', 'osd', 'pdu', 'pod', 'rack', 'region',
+        'room', 'root', 'row'])
+    :param data_chunks: int
+    :param coding_chunks: int
+    :param locality: int
+    :param durability_estimator: int
+    :return: None.  Can raise CalledProcessError
+    """
+    # Ensure this failure_domain is allowed by Ceph
+    validator(failure_domain, six.string_types,
+              ['chassis', 'datacenter', 'host', 'osd', 'pdu', 'pod', 'rack', 'region', 'room', 'root', 'row'])
+
+    cmd = ['ceph', '--id', service, 'osd', 'erasure-code-profile', 'set', profile_name,
+           'plugin=' + erasure_plugin_name, 'k=' + str(data_chunks), 'm=' + str(coding_chunks),
+           'ruleset_failure_domain=' + failure_domain]
+    if locality is not None and durability_estimator is not None:
+        raise ValueError("create_erasure_profile should be called with k, m and one of l or c but not both.")
+
+    # Add plugin specific information
+    if locality is not None:
+        # For local erasure codes
+        cmd.append('l=' + str(locality))
+    if durability_estimator is not None:
+        # For Shec erasure codes
+        cmd.append('c=' + str(durability_estimator))
+
+    if erasure_profile_exists(service, profile_name):
+        cmd.append('--force')
+
+    try:
+        check_call(cmd)
+    except CalledProcessError:
+        raise
+
+
+def rename_pool(service, old_name, new_name):
+    """
+    Rename a Ceph pool from old_name to new_name
+    :param service: six.string_types. The Ceph user name to run the command under
+    :param old_name: six.string_types
+    :param new_name: six.string_types
+    :return: None
+    """
+    validator(value=old_name, valid_type=six.string_types)
+    validator(value=new_name, valid_type=six.string_types)
+
+    cmd = ['ceph', '--id', service, 'osd', 'pool', 'rename', old_name, new_name]
+    check_call(cmd)
+
+
+def erasure_profile_exists(service, name):
+    """
+    Check to see if an Erasure code profile already exists.
+    :param service: six.string_types. The Ceph user name to run the command under
+    :param name: six.string_types
+    :return: int or None
+    """
+    validator(value=name, valid_type=six.string_types)
+    try:
+        check_call(['ceph', '--id', service,
+                    'osd', 'erasure-code-profile', 'get',
+                    name])
+        return True
+    except CalledProcessError:
+        return False
+
+
+def get_cache_mode(service, pool_name):
+    """
+    Find the current caching mode of the pool_name given.
+    :param service: six.string_types. The Ceph user name to run the command under
+    :param pool_name: six.string_types
+    :return: int or None
+    """
+    validator(value=service, valid_type=six.string_types)
+    validator(value=pool_name, valid_type=six.string_types)
+    out = check_output(['ceph', '--id', service, 'osd', 'dump', '--format=json'])
+    try:
+        osd_json = json.loads(out)
+        for pool in osd_json['pools']:
+            if pool['pool_name'] == pool_name:
+                return pool['cache_mode']
+        return None
+    except ValueError:
+        raise
+
+
+def pool_exists(service, name):
+    """Check to see if a RADOS pool already exists."""
+    try:
+        out = check_output(['rados', '--id', service,
+                            'lspools']).decode('UTF-8')
+    except CalledProcessError:
+        return False
+
+    return name in out
+
+
+def get_osds(service):
+    """Return a list of all Ceph Object Storage Daemons currently in the
+    cluster.
+    """
+    version = ceph_version()
+    if version and version >= '0.56':
+        return json.loads(check_output(['ceph', '--id', service,
+                                        'osd', 'ls',
+                                        '--format=json']).decode('UTF-8'))
+
+    return None
 
 
 def install():
@@ -102,30 +491,6 @@
     check_call(cmd)
 
 
-def pool_exists(service, name):
-    """Check to see if a RADOS pool already exists."""
-    try:
-        out = check_output(['rados', '--id', service,
-                            'lspools']).decode('UTF-8')
-    except CalledProcessError:
-        return False
-
-    return name in out
-
-
-def get_osds(service):
-    """Return a list of all Ceph Object Storage Daemons currently in the
-    cluster.
-    """
-    version = ceph_version()
-    if version and version >= '0.56':
-        return json.loads(check_output(['ceph', '--id', service,
-                                        'osd', 'ls',
-                                        '--format=json']).decode('UTF-8'))
-
-    return None
-
-
 def update_pool(client, pool, settings):
     cmd = ['ceph', '--id', client, 'osd', 'pool', 'set', pool]
     for k, v in six.iteritems(settings):
@@ -414,6 +779,7 @@
 
     The API is versioned and defaults to version 1.
     """
+
     def __init__(self, api_version=1, request_id=None):
         self.api_version = api_version
         if request_id:

=== modified file 'tests/contrib/storage/test_linux_ceph.py'
--- tests/contrib/storage/test_linux_ceph.py	2015-09-11 12:19:23 +0000
+++ tests/contrib/storage/test_linux_ceph.py	2015-11-30 18:12:02 +0000
@@ -1,5 +1,7 @@
+import unittest
 from mock import patch, call
 
+import six
 from shutil import rmtree
 from tempfile import mkdtemp
 from threading import Timer
@@ -14,7 +16,6 @@
 import os
 import time
 
-
 LS_POOLS = b"""
 images
 volumes
@@ -31,6 +32,55 @@
 bar
 baz
 """
+# Vastly abbreviated output from ceph osd dump --format=json
+OSD_DUMP = b"""
+{
+    "pools": [
+        {
+            "pool": 2,
+            "pool_name": "rbd",
+            "flags": 1,
+            "flags_names": "hashpspool",
+            "type": 1,
+            "size": 3,
+            "min_size": 2,
+            "crush_ruleset": 0,
+            "object_hash": 2,
+            "pg_num": 64,
+            "pg_placement_num": 64,
+            "crash_replay_interval": 0,
+            "last_change": "1",
+            "last_force_op_resend": "0",
+            "auid": 0,
+            "snap_mode": "selfmanaged",
+            "snap_seq": 0,
+            "snap_epoch": 0,
+            "pool_snaps": [],
+            "removed_snaps": "[]",
+            "quota_max_bytes": 0,
+            "quota_max_objects": 0,
+            "tiers": [],
+            "tier_of": -1,
+            "read_tier": -1,
+            "write_tier": -1,
+            "cache_mode": "writeback",
+            "target_max_bytes": 0,
+            "target_max_objects": 0,
+            "cache_target_dirty_ratio_micro": 0,
+            "cache_target_full_ratio_micro": 0,
+            "cache_min_flush_age": 0,
+            "cache_min_evict_age": 0,
+            "erasure_code_profile": "",
+            "hit_set_params": {
+                "type": "none"
+            },
+            "hit_set_period": 0,
+            "hit_set_count": 0,
+            "stripe_width": 0
+        }
+    ]
+}
+"""
 
 CEPH_CLIENT_RELATION = {
     'ceph:8': {
@@ -92,9 +142,256 @@
         self.addCleanup(_m.stop)
         setattr(self, method, mock)
 
+    def test_validator_valid(self):
+        # 1 is an int
+        ceph_utils.validator(value=1,
+                             valid_type=int)
+
+    def test_validator_valid_range(self):
+        # 1 is an int between 0 and 2
+        ceph_utils.validator(value=1,
+                             valid_type=int,
+                             valid_range=[0, 2])
+
+    @unittest.expectedFailure
+    def test_validator_invalid_range(self):
+        # 1 is an int that isn't in the valid list of only 0
+        ceph_utils.validator(value=1,
+                             valid_type=int,
+                             valid_range=[0])
+
+    @unittest.expectedFailure
+    def test_validator_invalid_string_list(self):
+        # foo is a six.string_types that isn't in the valid string list
+        ceph_utils.validator(value="foo",
+                             valid_type=six.string_types,
+                             valid_range=["valid", "list", "of", "strings"])
+
+    def test_pool_add_cache_tier(self):
+        p = ceph_utils.Pool(name='test', service='admin')
+        p.add_cache_tier('cacher', 'readonly')
+        self.check_call.assert_has_calls([
+            call(['ceph', '--id', 'admin', 'osd', 'tier', 'add', 'test', 'cacher']),
+            call(['ceph', '--id', 'admin', 'osd', 'tier', 'cache-mode', 'cacher', 'readonly']),
+            call(['ceph', '--id', 'admin', 'osd', 'tier', 'set-overlay', 'test', 'cacher']),
+            call(['ceph', '--id', 'admin', 'osd', 'pool', 'set', 'cacher', 'hit_set_type', 'bloom']),
+        ])
+
+    @patch.object(ceph_utils, 'get_cache_mode')
+    def test_pool_remove_readonly_cache_tier(self, cache_mode):
+        cache_mode.return_value = 'readonly'
+
+        p = ceph_utils.Pool(name='test', service='admin')
+        p.remove_cache_tier(cache_pool='cacher')
+        self.check_call.assert_has_calls([
+            call(['ceph', '--id', 'admin', 'osd', 'tier', 'cache-mode', 'cacher', 'none']),
+            call(['ceph', '--id', 'admin', 'osd', 'tier', 'remove', 'test', 'cacher']),
+        ])
+
+    @patch.object(ceph_utils, 'get_cache_mode')
+    def test_pool_remove_writeback_cache_tier(self, cache_mode):
+        cache_mode.return_value = 'writeback'
+
+        p = ceph_utils.Pool(name='test', service='admin')
+        p.remove_cache_tier(cache_pool='cacher')
+        self.check_call.assert_has_calls([
+            call(['ceph', '--id', 'admin', 'osd', 'tier', 'cache-mode', 'cacher', 'forward']),
+            call(['ceph', '--id', 'admin', '-p', 'cacher', 'cache-flush-evict-all']),
+            call(['ceph', '--id', 'admin', 'osd', 'tier', 'remove-overlay', 'test']),
+            call(['ceph', '--id', 'admin', 'osd', 'tier', 'remove', 'test', 'cacher']),
+        ])
+
+    @patch.object(ceph_utils, 'get_osds')
+    def test_replicated_pool_create_old_ceph(self, get_osds):
+        get_osds.return_value = None
+        p = ceph_utils.ReplicatedPool(name='test', service='admin', replicas=3)
+        p.create()
+
+        self.check_call.assert_has_calls([
+            call(['ceph', '--id', 'admin', 'osd', 'pool', 'create', 'test', str(200)]),
+        ])
+
+    @patch.object(ceph_utils, 'get_osds')
+    def test_replicated_pool_create_small_osds(self, get_osds):
+        get_osds.return_value = 4
+        p = ceph_utils.ReplicatedPool(name='test', service='admin', replicas=3)
+        p.create()
+
+        self.check_call.assert_has_calls([
+            call(['ceph', '--id', 'admin', 'osd', 'pool', 'create', 'test', str(128)]),
+        ])
+
+    @patch.object(ceph_utils, 'get_osds')
+    def test_replicated_pool_create_medium_osds(self, get_osds):
+        get_osds.return_value = 8
+        p = ceph_utils.ReplicatedPool(name='test', service='admin', replicas=3)
+        p.create()
+
+        self.check_call.assert_has_calls([
+            call(['ceph', '--id', 'admin', 'osd', 'pool', 'create', 'test', str(512)]),
+        ])
+
+    @patch.object(ceph_utils, 'get_osds')
+    def test_replicated_pool_create_large_osds(self, get_osds):
+        get_osds.return_value = 40
+        p = ceph_utils.ReplicatedPool(name='test', service='admin', replicas=3)
+        p.create()
+
+        self.check_call.assert_has_calls([
+            call(['ceph', '--id', 'admin', 'osd', 'pool', 'create', 'test', str(4096)]),
+        ])
+
+    @patch.object(ceph_utils, 'get_osds')
+    def test_replicated_pool_create_xlarge_osds(self, get_osds):
+        get_osds.return_value = 1000
+        p = ceph_utils.ReplicatedPool(name='test', service='admin', replicas=3)
+        p.create()
+
+        self.check_call.assert_has_calls([
+            call(['ceph', '--id', 'admin', 'osd', 'pool', 'create', 'test', str(65536)]),
+        ])
+
+    @unittest.expectedFailure
+    def test_replicated_pool_create_failed(self):
+        self.check_call.side_effect = CalledProcessError
+        p = ceph_utils.ReplicatedPool(name='test', service='admin', replicas=3)
+        p.create()
+
+    @patch.object(ceph_utils, 'pool_exists')
+    def test_replicated_pool_skips_creation(self, pool_exists):
+        pool_exists.return_value = True
+        p = ceph_utils.ReplicatedPool(name='test', service='admin', replicas=3)
+        p.create()
+        self.check_call.assert_has_calls([])
+
+    @unittest.expectedFailure
+    def test_erasure_pool_create_failed(self):
+        p = ceph_utils.ErasurePool(name='test', service='admin', erasure_code_profile='foo')
+        p.create()
+
+    @patch.object(ceph_utils, 'get_erasure_profile')
+    @patch.object(ceph_utils, 'get_osds')
+    def test_erasure_pool_create(self, get_osds, erasure_profile):
+        get_osds.return_value = 60
+        erasure_profile.return_value = {
+            'directory': '/usr/lib/x86_64-linux-gnu/ceph/erasure-code',
+            'k': '2',
+            'technique': 'reed_sol_van',
+            'm': '1',
+            'plugin': 'jerasure'}
+        p = ceph_utils.ErasurePool(name='test', service='admin')
+        p.create()
+        self.check_call.assert_has_calls(
+            call(['ceph', '--id', 'admin', 'osd', 'pool', 'create', 'test', str(8192),
+                  'erasure', 'default'])
+        )
+
+    def test_get_erasure_profile_none(self):
+        self.check_output.side_effect = CalledProcessError(1, 'ceph')
+        return_value = ceph_utils.get_erasure_profile('admin', 'unknown')
+        self.assertEqual(None, return_value)
+
+    def test_pool_set(self):
+        self.check_call.return_value = 0
+        ceph_utils.pool_set(service='admin', pool_name='data', key='test', value=2)
+        self.check_call.assert_has_calls(
+            call(['ceph', '--id', 'admin', 'osd', 'pool', 'set', 'data', 'test', 2])
+        )
+
+    @unittest.expectedFailure
+    def test_pool_set_fails(self):
+        self.check_call.side_effect = CalledProcessError
+        ceph_utils.pool_set(service='admin', pool_name='data', key='test', value=2)
+
+    def test_snapshot_pool(self):
+        self.check_call.return_value = 0
+        ceph_utils.snapshot_pool(service='admin', pool_name='data', snapshot_name='test-snap-1')
+        self.check_call.assert_has_calls(
+            call(['ceph', '--id', 'admin', 'osd', 'pool', 'mksnap', 'data', 'test-snap-1'])
+        )
+
+    @unittest.expectedFailure
+    def test_snapshot_pool_fails(self):
+        self.check_call.side_effect = CalledProcessError
+        ceph_utils.snapshot_pool(service='admin', pool_name='data', snapshot_name='test-snap-1')
+
+    def test_remove_pool_snapshot(self):
+        self.check_call.return_value = 0
+        ceph_utils.remove_pool_snapshot(service='admin', pool_name='data', snapshot_name='test-snap-1')
+        self.check_call.assert_has_calls(
+            call(['ceph', '--id', 'admin', 'osd', 'pool', 'rmsnap', 'data', 'test-snap-1'])
+        )
+
+    def test_set_pool_quota(self):
+        self.check_call.return_value = 0
+        ceph_utils.set_pool_quota(service='admin', pool_name='data', max_bytes=1024)
+        self.check_call.assert_has_calls(
+            call(['ceph', '--id', 'admin', 'osd', 'pool', 'set-quota', 'data', 'max_bytes', 1024])
+        )
+
+    def test_remove_pool_quota(self):
+        self.check_call.return_value = 0
+        ceph_utils.remove_pool_quota(service='admin', pool_name='data')
+        self.check_call.assert_has_calls(
+            call(['ceph', '--id', 'admin', 'osd', 'pool', 'set-quota', 'data', 'max_bytes', '0'])
+        )
+
+    @patch.object(ceph_utils, 'erasure_profile_exists')
+    def test_create_erasure_profile(self, existing_profile):
+        existing_profile.return_value = True
+        ceph_utils.create_erasure_profile(service='admin', profile_name='super-profile', erasure_plugin_name='jerasure',
+                                          failure_domain='rack', data_chunks=10, coding_chunks=3)
+
+        cmd = ['ceph', '--id', 'admin', 'osd', 'erasure-code-profile', 'set', 'super-profile',
+               'plugin=' + 'jerasure', 'k=' + str(10), 'm=' + str(3),
+               'ruleset_failure_domain=' + 'rack', '--force']
+        self.check_call.assert_has_calls(call(cmd))
+
+    @patch.object(ceph_utils, 'erasure_profile_exists')
+    def test_create_erasure_profile_local(self, existing_profile):
+        existing_profile.return_value = False
+        ceph_utils.create_erasure_profile(service='admin', profile_name='super-profile', erasure_plugin_name='local',
+                                          failure_domain='rack', data_chunks=10, coding_chunks=3, locality=1)
+
+        cmd = ['ceph', '--id', 'admin', 'osd', 'erasure-code-profile', 'set', 'super-profile',
+               'plugin=' + 'local', 'k=' + str(10), 'm=' + str(3),
+               'ruleset_failure_domain=' + 'rack', 'l=' + str(1)]
+        self.check_call.assert_has_calls(call(cmd))
+
+    @patch.object(ceph_utils, 'erasure_profile_exists')
+    def test_create_erasure_profile_shec(self, existing_profile):
+        existing_profile.return_value = False
+        ceph_utils.create_erasure_profile(service='admin', profile_name='super-profile', erasure_plugin_name='shec',
+                                          failure_domain='rack', data_chunks=10, coding_chunks=3,
+                                          durability_estimator=1)
+
+        cmd = ['ceph', '--id', 'admin', 'osd', 'erasure-code-profile', 'set', 'super-profile',
+               'plugin=' + 'shec', 'k=' + str(10), 'm=' + str(3),
+               'ruleset_failure_domain=' + 'rack', 'c=' + str(1)]
+        self.check_call.assert_has_calls(call(cmd))
+
+    def test_rename_pool(self):
+        ceph_utils.rename_pool(service='admin', old_name='old-pool', new_name='new-pool')
+        cmd = ['ceph', '--id', 'admin', 'osd', 'pool', 'rename', 'old-pool', 'new-pool']
+        self.check_call.assert_called_with(cmd)
+
+    def test_erasure_profile_exists(self):
+        self.check_call.return_value = 0
+        profile_exists = ceph_utils.erasure_profile_exists(service='admin', name='super-profile')
+        cmd = ['ceph', '--id', 'admin',
+               'osd', 'erasure-code-profile', 'get',
+               'super-profile']
+        self.check_call.assert_called_with(cmd)
+        self.assertEqual(True, profile_exists)
+
+    def test_get_cache_mode(self):
+        self.check_output.return_value = OSD_DUMP
+        cache_mode = ceph_utils.get_cache_mode(service='admin', pool_name='rbd')
+        self.assertEqual("writeback", cache_mode)
+
     @patch('os.path.exists')
     def test_create_keyring(self, _exists):
-        '''It creates a new ceph keyring'''
+        """It creates a new ceph keyring"""
         _exists.return_value = False
         ceph_utils.create_keyring('cinder', 'cephkey')
         _cmd = ['ceph-authtool', '/etc/ceph/ceph.client.cinder.keyring',
@@ -104,7 +401,7 @@
 
     @patch('os.path.exists')
     def test_create_keyring_already_exists(self, _exists):
-        '''It creates a new ceph keyring'''
+        """It creates a new ceph keyring"""
         _exists.return_value = True
         ceph_utils.create_keyring('cinder', 'cephkey')
         self.assertTrue(self.log.called)
@@ -113,7 +410,7 @@
     @patch('os.remove')
     @patch('os.path.exists')
     def test_delete_keyring(self, _exists, _remove):
-        '''It deletes a ceph keyring.'''
+        """It deletes a ceph keyring."""
         _exists.return_value = True
         ceph_utils.delete_keyring('cinder')
         _remove.assert_called_with('/etc/ceph/ceph.client.cinder.keyring')
@@ -122,7 +419,7 @@
     @patch('os.remove')
     @patch('os.path.exists')
     def test_delete_keyring_not_exists(self, _exists, _remove):
-        '''It creates a new ceph keyring.'''
+        """It creates a new ceph keyring."""
         _exists.return_value = False
         ceph_utils.delete_keyring('cinder')
         self.assertTrue(self.log.called)
@@ -130,7 +427,7 @@
 
     @patch('os.path.exists')
     def test_create_keyfile(self, _exists):
-        '''It creates a new ceph keyfile'''
+        """It creates a new ceph keyfile"""
         _exists.return_value = False
         with patch_open() as (_open, _file):
             ceph_utils.create_key_file('cinder', 'cephkey')
@@ -139,7 +436,7 @@
 
     @patch('os.path.exists')
     def test_create_key_file_already_exists(self, _exists):
-        '''It creates a new ceph keyring'''
+        """It creates a new ceph keyring"""
         _exists.return_value = True
         ceph_utils.create_key_file('cinder', 'cephkey')
         self.assertTrue(self.log.called)
@@ -173,7 +470,7 @@
     @patch.object(ceph_utils, 'get_osds')
     @patch.object(ceph_utils, 'pool_exists')
     def test_create_pool(self, _exists, _get_osds):
-        '''It creates rados pool correctly with default replicas '''
+        """It creates rados pool correctly with default replicas """
         _exists.return_value = False
         _get_osds.return_value = [1, 2, 3]
         ceph_utils.create_pool(service='cinder', name='foo')
@@ -187,7 +484,7 @@
     @patch.object(ceph_utils, 'get_osds')
     @patch.object(ceph_utils, 'pool_exists')
     def test_create_pool_2_replicas(self, _exists, _get_osds):
-        '''It creates rados pool correctly with 3 replicas'''
+        """It creates rados pool correctly with 3 replicas"""
         _exists.return_value = False
         _get_osds.return_value = [1, 2, 3]
         ceph_utils.create_pool(service='cinder', name='foo', replicas=2)
@@ -201,7 +498,7 @@
     @patch.object(ceph_utils, 'get_osds')
     @patch.object(ceph_utils, 'pool_exists')
     def test_create_pool_argonaut(self, _exists, _get_osds):
-        '''It creates rados pool correctly with 3 replicas'''
+        """It creates rados pool correctly with 3 replicas"""
         _exists.return_value = False
         _get_osds.return_value = None
         ceph_utils.create_pool(service='cinder', name='foo')
@@ -220,27 +517,27 @@
         self.check_call.assert_not_called()
 
     def test_keyring_path(self):
-        '''It correctly dervies keyring path from service name'''
+        """It correctly dervies keyring path from service name"""
         result = ceph_utils._keyring_path('cinder')
         self.assertEquals('/etc/ceph/ceph.client.cinder.keyring', result)
 
     def test_keyfile_path(self):
-        '''It correctly dervies keyring path from service name'''
+        """It correctly dervies keyring path from service name"""
         result = ceph_utils._keyfile_path('cinder')
         self.assertEquals('/etc/ceph/ceph.client.cinder.key', result)
 
     def test_pool_exists(self):
-        '''It detects an rbd pool exists'''
+        """It detects an rbd pool exists"""
         self.check_output.return_value = LS_POOLS
         self.assertTrue(ceph_utils.pool_exists('cinder', 'volumes'))
 
     def test_pool_does_not_exist(self):
-        '''It detects an rbd pool exists'''
+        """It detects an rbd pool exists"""
         self.check_output.return_value = LS_POOLS
         self.assertFalse(ceph_utils.pool_exists('cinder', 'foo'))
 
     def test_pool_exists_error(self):
-        ''' Ensure subprocess errors and sandboxed with False '''
+        """ Ensure subprocess errors and sandboxed with False """
         self.check_output.side_effect = CalledProcessError(1, 'rados')
         self.assertFalse(ceph_utils.pool_exists('cinder', 'foo'))
 
@@ -259,7 +556,7 @@
         )
 
     def test_rbd_exists_error(self):
-        ''' Ensure subprocess errors and sandboxed with False '''
+        """ Ensure subprocess errors and sandboxed with False """
         self.check_output.side_effect = CalledProcessError(1, 'rbd')
         self.assertFalse(ceph_utils.rbd_exists('cinder', 'foo', 'rbd'))
 
@@ -473,13 +770,13 @@
         self.service_start.assert_called_with(_services[0])
 
     def test_make_filesystem_default_filesystem(self):
-        '''make_filesystem() uses ext4 as the default filesystem.'''
+        """make_filesystem() uses ext4 as the default filesystem."""
         device = '/dev/zero'
         ceph_utils.make_filesystem(device)
         self.check_call.assert_called_with(['mkfs', '-t', 'ext4', device])
 
     def test_make_filesystem_no_device(self):
-        '''make_filesystem() raises an IOError if the device does not exist.'''
+        """make_filesystem() raises an IOError if the device does not exist."""
         device = '/no/such/device'
         e = self.assertRaises(IOError, ceph_utils.make_filesystem, device,
                               timeout=0)
@@ -512,9 +809,11 @@
         The specified device is formatted if it appears before the timeout
         is reached.
         """
+
         def create_my_device(filename):
             with open(filename, "w") as device:
                 device.write("hello\n")
+
         temp_dir = mkdtemp()
         self.addCleanup(rmtree, temp_dir)
         device = "%s/mydevice" % temp_dir
@@ -639,10 +938,10 @@
         self.relation_ids.side_effect = relation.relation_ids
         self.related_units.side_effect = relation.related_units
 
-#    @patch.object(ceph_utils, 'uuid')
-#    @patch.object(ceph_utils, 'local_unit')
-#    def test_get_request_states(self, mlocal_unit, muuid):
-#        muuid.uuid1.return_value = '0bc7dc54'
+    #    @patch.object(ceph_utils, 'uuid')
+    #    @patch.object(ceph_utils, 'local_unit')
+    #    def test_get_request_states(self, mlocal_unit, muuid):
+    #        muuid.uuid1.return_value = '0bc7dc54'
     @patch.object(ceph_utils, 'local_unit')
     def test_get_request_states(self, mlocal_unit):
         mlocal_unit.return_value = 'glance/0'

=== modified file 'tests/contrib/storage/test_linux_storage_loopback.py'
--- tests/contrib/storage/test_linux_storage_loopback.py	2015-11-02 19:48:05 +0000
+++ tests/contrib/storage/test_linux_storage_loopback.py	2015-11-30 18:12:02 +0000
@@ -17,7 +17,7 @@
 class LoopbackStorageUtilsTests(unittest.TestCase):
     @patch(STORAGE_LINUX_LOOPBACK + '.check_output')
     def test_loopback_devices(self, output):
-        '''It translates current loopback mapping to a dict'''
+        """It translates current loopback mapping to a dict"""
         output.return_value = LOOPBACK_DEVICES
         ex = {
             '/dev/loop1': '/tmp/bar.img',
@@ -31,7 +31,7 @@
     @patch(STORAGE_LINUX_LOOPBACK + '.loopback_devices')
     def test_loopback_create_already_exists(self, loopbacks, check_call,
                                             create):
-        '''It finds existing loopback device for requested file'''
+        """It finds existing loopback device for requested file"""
         loopbacks.return_value = {'/dev/loop1': '/tmp/bar.img'}
         res = loopback.ensure_loopback_device('/tmp/bar.img', '5G')
         self.assertEquals(res, '/dev/loop1')
@@ -43,7 +43,7 @@
     @patch('os.path.exists')
     def test_loop_creation_no_truncate(self, path_exists, create_loopback,
                                        loopbacks):
-        '''It does not create a new sparse image for loopback if one exists'''
+        """It does not create a new sparse image for loopback if one exists"""
         loopbacks.return_value = {}
         path_exists.return_value = True
         with patch('subprocess.check_call') as check_call:
@@ -55,7 +55,7 @@
     @patch('os.path.exists')
     def test_ensure_loopback_creation(self, path_exists, create_loopback,
                                       loopbacks):
-        '''It creates a new sparse image for loopback if one does not exists'''
+        """It creates a new sparse image for loopback if one does not exists"""
         loopbacks.return_value = {}
         path_exists.return_value = False
         create_loopback.return_value = '/dev/loop0'
@@ -66,7 +66,7 @@
 
     @patch.object(loopback, 'loopback_devices')
     def test_create_loopback(self, _devs):
-        '''It corectly calls losetup to create a loopback device'''
+        """It correctly calls losetup to create a loopback device"""
         _devs.return_value = {'/dev/loop0': '/tmp/foo'}
         with patch(STORAGE_LINUX_LOOPBACK + '.check_call') as check_call:
             check_call.return_value = ''

=== modified file 'tests/contrib/storage/test_linux_storage_lvm.py'
--- tests/contrib/storage/test_linux_storage_lvm.py	2014-11-25 13:38:01 +0000
+++ tests/contrib/storage/test_linux_storage_lvm.py	2015-11-30 18:12:02 +0000
@@ -39,14 +39,14 @@
 
 class LVMStorageUtilsTests(unittest.TestCase):
     def test_find_volume_group_on_pv(self):
-        '''It determines any volume group assigned to a LVM PV'''
+        """It determines any volume group assigned to a LVM PV"""
         with patch(STORAGE_LINUX_LVM + '.check_output') as check_output:
             check_output.return_value = PVDISPLAY
             vg = lvm.list_lvm_volume_group('/dev/loop0')
             self.assertEquals(vg, 'foo')
 
     def test_find_empty_volume_group_on_pv(self):
-        '''Return empty string when no volume group is assigned to the PV'''
+        """Return empty string when no volume group is assigned to the PV"""
         with patch(STORAGE_LINUX_LVM + '.check_output') as check_output:
             check_output.return_value = EMPTY_VG_IN_PVDISPLAY
             vg = lvm.list_lvm_volume_group('/dev/loop0')
@@ -54,38 +54,38 @@
 
     @patch(STORAGE_LINUX_LVM + '.list_lvm_volume_group')
     def test_deactivate_lvm_volume_groups(self, ls_vg):
-        '''It deactivates active volume groups on LVM PV'''
+        """It deactivates active volume groups on LVM PV"""
         ls_vg.return_value = 'foo-vg'
         with patch(STORAGE_LINUX_LVM + '.check_call') as check_call:
             lvm.deactivate_lvm_volume_group('/dev/loop0')
             check_call.assert_called_with(['vgchange', '-an', 'foo-vg'])
 
     def test_remove_lvm_physical_volume(self):
-        '''It removes LVM physical volume signatures from block device'''
+        """It removes LVM physical volume signatures from block device"""
         with patch(STORAGE_LINUX_LVM + '.Popen') as popen:
             lvm.remove_lvm_physical_volume('/dev/foo')
             popen.assert_called_with(['pvremove', '-ff', '/dev/foo'], stdin=-1)
 
     def test_is_physical_volume(self):
-        '''It properly reports block dev is an LVM PV'''
+        """It properly reports block dev is an LVM PV"""
         with patch(STORAGE_LINUX_LVM + '.check_output') as check_output:
             check_output.return_value = PVDISPLAY
             self.assertTrue(lvm.is_lvm_physical_volume('/dev/loop0'))
 
     def test_is_not_physical_volume(self):
-        '''It properly reports block dev is an LVM PV'''
+        """It properly reports block dev is an LVM PV"""
         with patch(STORAGE_LINUX_LVM + '.check_output') as check_output:
             check_output.side_effect = subprocess.CalledProcessError('cmd', 2)
             self.assertFalse(lvm.is_lvm_physical_volume('/dev/loop0'))
 
     def test_pvcreate(self):
-        '''It correctly calls pvcreate for a given block dev'''
+        """It correctly calls pvcreate for a given block dev"""
         with patch(STORAGE_LINUX_LVM + '.check_call') as check_call:
             lvm.create_lvm_physical_volume('/dev/foo')
             check_call.assert_called_with(['pvcreate', '/dev/foo'])
 
     def test_vgcreate(self):
-        '''It correctly calls vgcreate for given block dev and vol group'''
+        """It correctly calls vgcreate for given block dev and vol group"""
         with patch(STORAGE_LINUX_LVM + '.check_call') as check_call:
             lvm.create_lvm_volume_group('foo-vg', '/dev/foo')
             check_call.assert_called_with(['vgcreate', 'foo-vg', '/dev/foo'])

=== modified file 'tests/contrib/storage/test_linux_storage_utils.py'
--- tests/contrib/storage/test_linux_storage_utils.py	2015-08-07 09:00:30 +0000
+++ tests/contrib/storage/test_linux_storage_utils.py	2015-11-30 18:12:02 +0000
@@ -13,7 +13,7 @@
     @patch(STORAGE_LINUX_UTILS + '.call')
     @patch(STORAGE_LINUX_UTILS + '.check_call')
     def test_zap_disk(self, check_call, call, check_output):
-        '''It calls sgdisk correctly to zap disk'''
+        """It calls sgdisk correctly to zap disk"""
         check_output.return_value = b'200\n'
         storage_utils.zap_disk('/dev/foo')
         call.assert_any_call(['sgdisk', '--zap-all', '--', '/dev/foo'])
@@ -29,7 +29,7 @@
     @patch('os.path.exists')
     @patch('os.stat')
     def test_is_block_device(self, S_ISBLK, exists, stat):
-        '''It detects device node is block device'''
+        """It detects device node is block device"""
         class fake_stat:
             st_mode = True
         S_ISBLK.return_value = fake_stat()
@@ -40,7 +40,7 @@
     @patch('os.path.exists')
     @patch('os.stat')
     def test_is_block_device_does_not_exist(self, S_ISBLK, exists, stat):
-        '''It detects device node is block device'''
+        """It detects device node is block device"""
         class fake_stat:
             st_mode = True
         S_ISBLK.return_value = fake_stat()
@@ -49,7 +49,7 @@
 
     @patch(STORAGE_LINUX_UTILS + '.check_output')
     def test_is_device_mounted(self, check_output):
-        '''It detects mounted devices as mounted.'''
+        """It detects mounted devices as mounted."""
         check_output.return_value = (
             b"/dev/sda1 on / type ext4 (rw,errors=remount-ro)\n")
         result = storage_utils.is_device_mounted('/dev/sda')
@@ -57,7 +57,7 @@
 
     @patch(STORAGE_LINUX_UTILS + '.check_output')
     def test_is_device_mounted_partition(self, check_output):
-        '''It detects mounted partitions as mounted.'''
+        """It detects mounted partitions as mounted."""
         check_output.return_value = (
             b"/dev/sda1 on / type ext4 (rw,errors=remount-ro)\n")
         result = storage_utils.is_device_mounted('/dev/sda1')
@@ -65,8 +65,8 @@
 
     @patch(STORAGE_LINUX_UTILS + '.check_output')
     def test_is_device_mounted_partition_with_device(self, check_output):
-        '''It detects mounted devices as mounted if "mount" shows only a
-        partition as mounted.'''
+        """It detects mounted devices as mounted if "mount" shows only a
+        partition as mounted."""
         check_output.return_value = (
             b"/dev/sda1 on / type ext4 (rw,errors=remount-ro)\n")
         result = storage_utils.is_device_mounted('/dev/sda')
@@ -74,7 +74,7 @@
 
     @patch(STORAGE_LINUX_UTILS + '.check_output')
     def test_is_device_mounted_not_mounted(self, check_output):
-        '''It detects unmounted devices as not mounted.'''
+        """It detects unmounted devices as not mounted."""
         check_output.return_value = (
             b"/dev/foo on / type ext4 (rw,errors=remount-ro)\n")
         result = storage_utils.is_device_mounted('/dev/sda')
@@ -82,7 +82,7 @@
 
     @patch(STORAGE_LINUX_UTILS + '.check_output')
     def test_is_device_mounted_not_mounted_partition(self, check_output):
-        '''It detects unmounted partitions as not mounted.'''
+        """It detects unmounted partitions as not mounted."""
         check_output.return_value = (
             b"/dev/foo on / type ext4 (rw,errors=remount-ro)\n")
         result = storage_utils.is_device_mounted('/dev/sda1')
@@ -90,7 +90,7 @@
 
     @patch(STORAGE_LINUX_UTILS + '.check_output')
     def test_is_device_mounted_full_disks(self, check_output):
-        '''It detects mounted full disks as mounted.'''
+        """It detects mounted full disks as mounted."""
         check_output.return_value = (
             b"/dev/sda on / type ext4 (rw,errors=remount-ro)\n")
         result = storage_utils.is_device_mounted('/dev/sda')
@@ -98,7 +98,7 @@
 
     @patch(STORAGE_LINUX_UTILS + '.check_output')
     def test_is_device_mounted_cciss(self, check_output):
-        '''It detects mounted cciss partitions as mounted.'''
+        """It detects mounted cciss partitions as mounted."""
         check_output.return_value = (
             b"/dev/cciss/c0d0 on / type ext4 (rw,errors=remount-ro)\n")
         result = storage_utils.is_device_mounted('/dev/cciss/c0d0')
@@ -106,7 +106,7 @@
 
     @patch(STORAGE_LINUX_UTILS + '.check_output')
     def test_is_device_mounted_cciss_not_mounted(self, check_output):
-        '''It detects unmounted cciss partitions as not mounted.'''
+        """It detects unmounted cciss partitions as not mounted."""
         check_output.return_value = (
             b"/dev/cciss/c0d1 on / type ext4 (rw,errors=remount-ro)\n")
         result = storage_utils.is_device_mounted('/dev/cciss/c0d0')