Merge lp:~adam-collard/charms/trusty/swift-proxy/lib-in-python-package into lp:~openstack-charmers-archive/charms/trusty/swift-proxy/next

[Adam Collard]

With the addition of actions to the OpenStack charms, there's a need to share code between actions and hooks. This branch moves library code to a top-level ./lib/ and then symlinks that into ./hooks/lib, likewise for charm-helpers.

This should be a plain refactor with no change in behaviour, but laying the ground for an actions/lib symlink and an actions/charm-helpers symlink.

(see https://code.launchpad.net/~adam-collard/charms/precise/swift-storage/lib-in-python-package/+merge/265105 for the equivalent MP for swift-storage)

[lp:~adam-collard/charms/trusty/swift-proxy/lib-in-python-package]

110. By Adam Collard on 2015-09-07

Add explanatory comment

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

charm_lint_check #9540 swift-proxy-next for adam-collard mp270304
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/9540/

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

charm_unit_test #8781 swift-proxy-next for adam-collard mp270304
    UNIT OK: passed

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

[Chris Glass (tribaal)]

I think this looks good, small comment inline.

[lp:~adam-collard/charms/trusty/swift-proxy/lib-in-python-package]

111. By Adam Collard on 2015-09-07

Add missing @property (tribaal's review)

[Chris Glass (tribaal)]

Looks good!

review: Approve

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

charm_amulet_test #6311 swift-proxy-next for adam-collard mp270304
    AMULET FAIL: amulet-test failed

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

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

=== modified file '.coveragerc'
--- .coveragerc	2013-11-18 11:16:28 +0000
+++ .coveragerc	2015-09-07 14:15:28 +0000
@@ -4,3 +4,4 @@
     if __name__ == .__main__.:
 include=
     hooks/swift_*
+    lib/swift_*

=== modified file 'Makefile'
--- Makefile	2015-04-16 21:32:07 +0000
+++ Makefile	2015-09-07 14:15:28 +0000
@@ -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 @@
 publish: lint unit_test
 	bzr push lp:charms/swift-proxy
 	bzr push lp:charms/trusty/swift-proxy
+
+.PHONY: lint unit_test test sync publish

=== modified file 'charm-helpers-hooks.yaml'
--- charm-helpers-hooks.yaml	2015-07-31 13:11:02 +0000
+++ charm-helpers-hooks.yaml	2015-09-07 14:15:28 +0000
@@ -1,5 +1,5 @@
 branch: lp:charm-helpers
-destination: hooks/charmhelpers
+destination: charmhelpers
 include:
     - core
     - cli

=== renamed directory 'hooks/charmhelpers' => 'charmhelpers'
=== modified file 'charmhelpers/contrib/openstack/utils.py'
--- hooks/charmhelpers/contrib/openstack/utils.py	2015-09-03 09:41:22 +0000
+++ charmhelpers/contrib/openstack/utils.py	2015-09-07 14:15:28 +0000
@@ -114,6 +114,7 @@
     ('2.2.1', 'kilo'),
     ('2.2.2', 'kilo'),
     ('2.3.0', 'liberty'),
+    ('2.4.0', 'liberty'),
 ])
 
 # >= Liberty version->codename mapping
@@ -142,6 +143,9 @@
     'glance-common': OrderedDict([
         ('11.0.0', 'liberty'),
     ]),
+    'openstack-dashboard': OrderedDict([
+        ('8.0.0', 'liberty'),
+    ]),
 }
 
 DEFAULT_LOOPBACK_SIZE = '5G'

=== modified file 'charmhelpers/contrib/storage/linux/ceph.py'
--- hooks/charmhelpers/contrib/storage/linux/ceph.py	2015-07-29 10:47:50 +0000
+++ charmhelpers/contrib/storage/linux/ceph.py	2015-09-07 14:15:28 +0000
@@ -56,6 +56,8 @@
     apt_install,
 )
 
+from charmhelpers.core.kernel import modprobe
+
 KEYRING = '/etc/ceph/ceph.client.{}.keyring'
 KEYFILE = '/etc/ceph/ceph.client.{}.key'
 
@@ -288,17 +290,6 @@
     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):

=== added file 'charmhelpers/core/kernel.py'
--- charmhelpers/core/kernel.py	1970-01-01 00:00:00 +0000
+++ charmhelpers/core/kernel.py	2015-09-07 14:15:28 +0000
@@ -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 <[e-mail]>"
+
+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"])

=== modified file 'hooks/swift_hooks.py'
--- hooks/swift_hooks.py	2015-05-11 08:04:17 +0000
+++ hooks/swift_hooks.py	2015-09-07 14:15:28 +0000
@@ -8,7 +8,7 @@
     CalledProcessError,
 )
 
-from swift_utils import (
+from lib.swift_utils import (
     SwiftProxyCharmException,
     register_configs,
     restart_map,

=== added directory 'lib'
=== added file 'lib/__init__.py'
=== renamed file 'hooks/swift_context.py' => 'lib/swift_context.py'
--- hooks/swift_context.py	2015-04-13 22:57:02 +0000
+++ lib/swift_context.py	2015-09-07 14:15:28 +0000
@@ -50,9 +50,15 @@
 
 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):
 

=== renamed file 'hooks/swift_utils.py' => 'lib/swift_utils.py'
=== modified file 'setup.cfg'
--- setup.cfg	2013-11-18 11:16:28 +0000
+++ setup.cfg	2015-09-07 14:15:28 +0000
@@ -2,4 +2,4 @@
 verbosity=1
 with-coverage=1
 cover-erase=1
-cover-package=hooks
+cover-package=lib,swift_hooks

=== modified file 'unit_tests/__init__.py'
--- unit_tests/__init__.py	2013-11-18 11:16:28 +0000
+++ unit_tests/__init__.py	2015-09-07 14:15:28 +0000
@@ -1,2 +0,0 @@
-import sys
-sys.path.append('hooks')

=== modified file 'unit_tests/test_swift_context.py'
--- unit_tests/test_swift_context.py	2014-12-01 23:54:03 +0000
+++ unit_tests/test_swift_context.py	2015-09-07 14:15:28 +0000
@@ -6,12 +6,12 @@
 
 
 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 @@
         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 @@
         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 @@
         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)

=== modified file 'unit_tests/test_swift_hooks.py'
--- unit_tests/test_swift_hooks.py	2015-06-11 23:22:54 +0000
+++ unit_tests/test_swift_hooks.py	2015-09-07 14:15:28 +0000
@@ -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
 

=== modified file 'unit_tests/test_swift_utils.py'
--- unit_tests/test_swift_utils.py	2014-12-18 11:58:28 +0000
+++ unit_tests/test_swift_utils.py	2015-09-07 14:15:28 +0000
@@ -5,9 +5,8 @@
 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 @@
 
 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 @@
         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 @@
         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 @@
         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 @@
                           '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 @@
                           '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 @@
                           '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()