Merge lp:~adam-collard/charms/trusty/swift-proxy/lib-in-python-package [a=adam.collard] [r=tribaal]

Refactor hooks/lib into a lib/ folder to prepare for actions implementation.

.coveragerc

@@ -4,3 +4,4 @@ exclude_lines =
     if __name__ == .__main__.:
 include=
     hooks/swift_*
+    lib/swift_*

Makefile

@@ -1,9 +1,7 @@
-#!/usr/bin/make
 PYTHON := /usr/bin/env python
 
 lint:
-	@flake8 --exclude hooks/charmhelpers --ignore=E125 hooks
-	@flake8 --exclude hooks/charmhelpers --ignore=E125 unit_tests tests
+	@flake8 --exclude hooks unit_tests tests lib
 	@charm proof
 
 unit_test:
@@ -29,3 +27,5 @@ sync: bin/charm_helpers_sync.py
 publish: lint unit_test
 	bzr push lp:charms/swift-proxy
 	bzr push lp:charms/trusty/swift-proxy
+
+.PHONY: lint unit_test test sync publish

charm-helpers-hooks.yaml

@@ -1,5 +1,5 @@
 branch: lp:charm-helpers
-destination: hooks/charmhelpers
+destination: charmhelpers
 include:
     - core
     - cli

charmhelpers/contrib/openstack/utils.py

@@ -114,6 +114,7 @@ SWIFT_CODENAMES = OrderedDict([
     ('2.2.1', 'kilo'),
     ('2.2.2', 'kilo'),
     ('2.3.0', 'liberty'),
+    ('2.4.0', 'liberty'),
 ])
 
 # >= Liberty version->codename mapping
@@ -142,6 +143,9 @@ PACKAGE_CODENAMES = {
     'glance-common': OrderedDict([
         ('11.0.0', 'liberty'),
     ]),
+    'openstack-dashboard': OrderedDict([
+        ('8.0.0', 'liberty'),
+    ]),
 }
 
 DEFAULT_LOOPBACK_SIZE = '5G'

charmhelpers/contrib/storage/linux/ceph.py

@@ -56,6 +56,8 @@ from charmhelpers.fetch import (
     apt_install,
 )
 
+from charmhelpers.core.kernel import modprobe
+
 KEYRING = '/etc/ceph/ceph.client.{}.keyring'
 KEYFILE = '/etc/ceph/ceph.client.{}.key'
 
@@ -288,17 +290,6 @@ def place_data_on_block_device(blk_device, data_src_dst):
     os.chown(data_src_dst, uid, gid)
 
 
-# TODO: re-use
-def modprobe(module):
-    """Load a kernel module and configure for auto-load on reboot."""
-    log('Loading kernel module', level=INFO)
-    cmd = ['modprobe', module]
-    check_call(cmd)
-    with open('/etc/modules', 'r+') as modules:
-        if module not in modules.read():
-            modules.write(module)
-
-
 def copy_files(src, dst, symlinks=False, ignore=None):
     """Copy files from src to dst."""
     for item in os.listdir(src):

charmhelpers/core/kernel.py

@@ -0,0 +1,68 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+# Copyright 2014-2015 Canonical Limited.
+#
+# This file is part of charm-helpers.
+#
+# charm-helpers is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License version 3 as
+# published by the Free Software Foundation.
+#
+# charm-helpers is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with charm-helpers.  If not, see <http://www.gnu.org/licenses/>.
+
+__author__ = "Jorge Niedbalski <jorge.niedbalski@canonical.com>"
+
+from charmhelpers.core.hookenv import (
+    log,
+    INFO
+)
+
+from subprocess import check_call, check_output
+import re
+
+
+def modprobe(module, persist=True):
+    """Load a kernel module and configure for auto-load on reboot."""
+    cmd = ['modprobe', module]
+
+    log('Loading kernel module %s' % module, level=INFO)
+
+    check_call(cmd)
+    if persist:
+        with open('/etc/modules', 'r+') as modules:
+            if module not in modules.read():
+                modules.write(module)
+
+
+def rmmod(module, force=False):
+    """Remove a module from the linux kernel"""
+    cmd = ['rmmod']
+    if force:
+        cmd.append('-f')
+    cmd.append(module)
+    log('Removing kernel module %s' % module, level=INFO)
+    return check_call(cmd)
+
+
+def lsmod():
+    """Shows what kernel modules are currently loaded"""
+    return check_output(['lsmod'],
+                        universal_newlines=True)
+
+
+def is_module_loaded(module):
+    """Checks if a kernel module is already loaded"""
+    matches = re.findall('^%s[ ]+' % module, lsmod(), re.M)
+    return len(matches) > 0
+
+
+def update_initramfs(version='all'):
+    """Updates an initramfs image"""
+    return check_call(["update-initramfs", "-k", version, "-u"])

hooks/swift_hooks.py

@@ -8,7 +8,7 @@ from subprocess import (
     CalledProcessError,
 )
 
-from swift_utils import (
+from lib.swift_utils import (
     SwiftProxyCharmException,
     register_configs,
     restart_map,

lib/swift_context.py

@@ -50,9 +50,15 @@ class HAProxyContext(OSContextGenerator):
 
 class ApacheSSLContext(SSLContext):
     interfaces = ['https']
-    external_ports = [config('bind-port')]
     service_namespace = 'swift'
 
+    # We make this a property so that we avoid import-time
+    # dependencies on config()
+
+    @property
+    def external_ports(self):
+        return [config('bind-port')]
+
 
 class SwiftRingContext(OSContextGenerator):
 

setup.cfg

@@ -2,4 +2,4 @@
 verbosity=1
 with-coverage=1
 cover-erase=1
-cover-package=hooks
+cover-package=lib,swift_hooks

unit_tests/__init__.py

@@ -1,2 +0,0 @@
-import sys
-sys.path.append('hooks')

unit_tests/test_swift_context.py

@@ -6,12 +6,12 @@ import uuid
 
 
 with mock.patch('charmhelpers.core.hookenv.config'):
-    import swift_context
+    import lib.swift_context as swift_context
 
 
 class SwiftContextTestCase(unittest.TestCase):
 
-    @mock.patch('swift_context.config')
+    @mock.patch('lib.swift_context.config')
     def test_get_swift_hash_file(self, mock_config):
         expected = '##FILEHASH##'
         with tempfile.NamedTemporaryFile() as tmpfile:
@@ -24,7 +24,7 @@ class SwiftContextTestCase(unittest.TestCase):
         self.assertFalse(mock_config.called)
         self.assertEqual(expected, hash)
 
-    @mock.patch('swift_context.config')
+    @mock.patch('lib.swift_context.config')
     def test_get_swift_hash_config(self, mock_config):
         expected = '##CFGHASH##'
         mock_config.return_value = expected
@@ -38,8 +38,8 @@ class SwiftContextTestCase(unittest.TestCase):
         self.assertTrue(mock_config.called)
         self.assertEqual(expected, hash)
 
-    @mock.patch('swift_context.service_name')
-    @mock.patch('swift_context.config')
+    @mock.patch('lib.swift_context.service_name')
+    @mock.patch('lib.swift_context.config')
     def test_get_swift_hash_env(self, mock_config, mock_service_name):
         mock_config.return_value = None
         mock_service_name.return_value = "testsvc"
@@ -47,10 +47,10 @@ class SwiftContextTestCase(unittest.TestCase):
         swift_context.SWIFT_HASH_FILE = tmpfile
         with mock.patch('swift_context.os.environ.get') as mock_env_get:
             mock_env_get.return_value = str(uuid.uuid4())
-            hash = swift_context.get_swift_hash()
+            hash_ = swift_context.get_swift_hash()
             mock_env_get.assert_called_with('JUJU_ENV_UUID')
 
         with open(tmpfile, 'r') as fd:
-            self.assertEqual(hash, fd.read())
+            self.assertEqual(hash_, fd.read())
 
         self.assertTrue(mock_config.called)

unit_tests/test_swift_hooks.py

@@ -1,8 +1,9 @@
 from mock import patch
+import sys
 import unittest
 import uuid
 
-
+sys.path.append("hooks")
 with patch('charmhelpers.core.hookenv.log'):
     import swift_hooks
 

unit_tests/test_swift_utils.py

@@ -5,9 +5,8 @@ import tempfile
 import uuid
 import unittest
 
-
 with mock.patch('charmhelpers.core.hookenv.config'):
-    import swift_utils
+    import lib.swift_utils as swift_utils
 
 
 def init_ring_paths(tmpdir):
@@ -21,16 +20,16 @@ def init_ring_paths(tmpdir):
 
 class SwiftUtilsTestCase(unittest.TestCase):
 
-    @mock.patch('swift_utils.get_broker_token')
-    @mock.patch('swift_utils.update_www_rings')
-    @mock.patch('swift_utils.get_builders_checksum')
-    @mock.patch('swift_utils.get_rings_checksum')
-    @mock.patch('swift_utils.balance_rings')
-    @mock.patch('swift_utils.log')
-    @mock.patch('swift_utils.os.path.exists')
-    @mock.patch('swift_utils.is_elected_leader')
-    @mock.patch('swift_utils.get_min_part_hours')
-    @mock.patch('swift_utils.set_min_part_hours')
+    @mock.patch('lib.swift_utils.get_broker_token')
+    @mock.patch('lib.swift_utils.update_www_rings')
+    @mock.patch('lib.swift_utils.get_builders_checksum')
+    @mock.patch('lib.swift_utils.get_rings_checksum')
+    @mock.patch('lib.swift_utils.balance_rings')
+    @mock.patch('lib.swift_utils.log')
+    @mock.patch('lib.swift_utils.os.path.exists')
+    @mock.patch('lib.swift_utils.is_elected_leader')
+    @mock.patch('lib.swift_utils.get_min_part_hours')
+    @mock.patch('lib.swift_utils.set_min_part_hours')
     def test_update_rings(self, mock_set_min_hours,
                           mock_get_min_hours,
                           mock_is_elected_leader, mock_path_exists,
@@ -76,13 +75,13 @@ class SwiftUtilsTestCase(unittest.TestCase):
         self.assertTrue(mock_set_min_hours.called)
         self.assertTrue(mock_balance_rings.called)
 
-    @mock.patch('swift_utils.get_broker_token')
-    @mock.patch('swift_utils.balance_rings')
-    @mock.patch('swift_utils.log')
-    @mock.patch('swift_utils.is_elected_leader')
-    @mock.patch('swift_utils.config')
-    @mock.patch('swift_utils.update_www_rings')
-    @mock.patch('swift_utils.cluster_sync_rings')
+    @mock.patch('lib.swift_utils.get_broker_token')
+    @mock.patch('lib.swift_utils.balance_rings')
+    @mock.patch('lib.swift_utils.log')
+    @mock.patch('lib.swift_utils.is_elected_leader')
+    @mock.patch('lib.swift_utils.config')
+    @mock.patch('lib.swift_utils.update_www_rings')
+    @mock.patch('lib.swift_utils.cluster_sync_rings')
     def test_sync_builders_and_rings_if_changed(self, mock_cluster_sync_rings,
                                                 mock_update_www_rings,
                                                 mock_config,
@@ -114,7 +113,7 @@ class SwiftUtilsTestCase(unittest.TestCase):
         self.assertTrue(mock_update_www_rings.called)
         self.assertTrue(mock_cluster_sync_rings.called)
 
-    @mock.patch('swift_utils.get_www_dir')
+    @mock.patch('lib.swift_utils.get_www_dir')
     def test_mark_www_rings_deleted(self, mock_get_www_dir):
         try:
             tmpdir = tempfile.mkdtemp()
@@ -123,7 +122,7 @@ class SwiftUtilsTestCase(unittest.TestCase):
         finally:
             shutil.rmtree(tmpdir)
 
-    @mock.patch('swift_utils.uuid')
+    @mock.patch('lib.swift_utils.uuid')
     def test_cluster_rpc_stop_proxy_request(self, mock_uuid):
         mock_uuid.uuid4.return_value = 'test-uuid'
         rpc = swift_utils.SwiftProxyClusterRPC()
@@ -147,7 +146,7 @@ class SwiftUtilsTestCase(unittest.TestCase):
                           'stop-proxy-service-ack': None,
                           'sync-only-builders': None}, rq)
 
-    @mock.patch('swift_utils.uuid')
+    @mock.patch('lib.swift_utils.uuid')
     def test_cluster_rpc_stop_proxy_ack(self, mock_uuid):
         mock_uuid.uuid4.return_value = 'token2'
         rpc = swift_utils.SwiftProxyClusterRPC()
@@ -161,7 +160,7 @@ class SwiftUtilsTestCase(unittest.TestCase):
                           'stop-proxy-service-ack': 'token1',
                           'sync-only-builders': None}, rq)
 
-    @mock.patch('swift_utils.uuid')
+    @mock.patch('lib.swift_utils.uuid')
     def test_cluster_rpc_sync_request(self, mock_uuid):
         mock_uuid.uuid4.return_value = 'token2'
         rpc = swift_utils.SwiftProxyClusterRPC()
@@ -175,7 +174,7 @@ class SwiftUtilsTestCase(unittest.TestCase):
                           'stop-proxy-service-ack': None,
                           'sync-only-builders': None}, rq)
 
-    @mock.patch('swift_utils.uuid')
+    @mock.patch('lib.swift_utils.uuid')
     def test_cluster_rpc_notify_leader_changed(self, mock_uuid):
         mock_uuid.uuid4.return_value = 'token1'
         rpc = swift_utils.SwiftProxyClusterRPC()