Upgrade the charm to py3 runtime

Change-Id: I98f4e6664080407a045ca5e76db59d46ffa9c38a

actions/actions.py

@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 #
 # Copyright 2016 Canonical Ltd
 #
@@ -19,6 +19,17 @@ import os
 import sys
 import yaml
 
+_path = os.path.dirname(os.path.realpath(__file__))
+_root = os.path.abspath(os.path.join(_path, '..'))
+
+
+def _add_path(path):
+    if path not in sys.path:
+        sys.path.insert(1, path)
+
+
+_add_path(_root)
+
 from charmhelpers.core.host import service_pause, service_resume
 from charmhelpers.core.hookenv import action_fail
 from charmhelpers.core.unitdata import HookData, kv
@@ -101,7 +112,7 @@ def main(argv):
     try:
         action = ACTIONS[action_name]
     except KeyError:
-        return "Action %s undefined" % action_name
+        return "Action {} undefined".format(action_name)
     else:
         try:
             action(args)

actions/charmhelpers

@@ -1 +0,0 @@
-../charmhelpers/

actions/openstack_upgrade.py

@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 #
 # Copyright 2016 Canonical Ltd
 #
@@ -14,15 +14,26 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import os
 import sys
 
-sys.path.append('hooks/')
+_path = os.path.dirname(os.path.realpath(__file__))
+_root = os.path.abspath(os.path.join(_path, '..'))
+
+
+def _add_path(path):
+    if path not in sys.path:
+        sys.path.insert(1, path)
+
+
+_add_path(_root)
+
 
 from charmhelpers.contrib.openstack.utils import (
     do_action_openstack_upgrade,
 )
 
-from swift_storage_hooks import (
+from hooks.swift_storage_hooks import (
     config_changed,
     CONFIGS,
 )

files/nrpe-external-master/check_swift_storage.py

@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 
 # Copyright (C) 2014, 2017 Canonical
 # All Rights Reserved
@@ -6,7 +6,7 @@
 
 import sys
 import json
-import urllib2
+import urllib
 import argparse
 import hashlib
 import datetime
@@ -20,10 +20,11 @@ STATUS_UNKNOWN = 3
 def generate_md5(filename):
     with open(filename, 'rb') as f:
         md5 = hashlib.md5()
+        while True:
             buffer = f.read(2 ** 20)
-        while buffer:
+            if not buffer:
+                break
             md5.update(buffer)
-            buffer = f.read(2 ** 20)
     return md5.hexdigest()
 
 
@@ -34,9 +35,9 @@ def check_md5(base_url):
                  "/etc/swift/container.ring.gz"]
     results = []
     try:
-        data = urllib2.urlopen(url).read()
+        data = urllib.request.urlopen(url).read()
         ringmd5_info = json.loads(data)
-    except urllib2.URLError:
+    except urllib.error.URLError:
         return [(STATUS_UNKNOWN, "Can't open url: {}".format(url))]
     except ValueError:
         return [(STATUS_UNKNOWN, "Can't parse status data")]
@@ -96,9 +97,9 @@ def check_replication(base_url, limits):
     for repl in types:
         url = base_url + "replication/" + repl
         try:
-            data = urllib2.urlopen(url).read()
+            data = urllib.request.urlopen(url).read()
             repl_info = json.loads(data)
-        except urllib2.URLError:
+        except urllib.error.URLError:
             results.append((STATUS_UNKNOWN, "Can't open url: {}".format(url)))
             continue
         except ValueError:

hooks/charmhelpers

@@ -1 +0,0 @@
-../charmhelpers/

hooks/install

@@ -11,7 +11,7 @@ check_and_install() {
     fi
 }
 
-PYTHON="python"
+PYTHON="python3"
 
 for dep in ${DEPS[@]}; do
     check_and_install ${PYTHON} ${dep}

hooks/lib

@@ -1 +0,0 @@
-../lib

hooks/swift_storage_hooks.py

@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 #
 # Copyright 2016 Canonical Ltd
 #
@@ -24,6 +24,19 @@ import socket
 import subprocess
 import tempfile
 
+
+_path = os.path.dirname(os.path.realpath(__file__))
+_root = os.path.abspath(os.path.join(_path, '..'))
+
+
+def _add_path(path):
+    if path not in sys.path:
+        sys.path.insert(1, path)
+
+
+_add_path(_root)
+
+
 from lib.swift_storage_utils import (
     PACKAGES,
     RESTART_MAP,

lib/swift_storage_utils.py

@@ -10,13 +10,13 @@ from subprocess import check_call, call, CalledProcessError, check_output
 
 # Stuff copied from cinder py charm, needs to go somewhere
 # common.
-from misc_utils import (
+from lib.misc_utils import (
     ensure_block_device,
     clean_storage,
     is_paused
 )
 
-from swift_storage_context import (
+from lib.swift_storage_context import (
     SwiftStorageContext,
     SwiftStorageServerContext,
     RsyncContext,
@@ -222,7 +222,7 @@ def _is_storage_ready(partition):
 def get_mount_point(device):
     mnt_point = None
     try:
-        out = check_output(['findmnt', device])
+        out = check_output(['findmnt', device]).decode('ascii')
         mnt_points = []
         for line in out.split('\n'):
             if line and not line.startswith('TARGET'):
@@ -286,7 +286,8 @@ def determine_block_devices():
     storage_ids = storage_list('block-devices')
     bdevs.extend((storage_get('location', s) for s in storage_ids))
 
-    bdevs = list(set(bdevs))
+    # only sorted so the tests pass; doesn't affect functionality
+    bdevs = sorted(set(bdevs))
     # attempt to ensure block devices, but filter out missing devs
     _none = ['None', 'none']
     valid_bdevs = \
@@ -340,7 +341,7 @@ def is_device_in_ring(dev, skip_rel_check=False, ignore_deactivated=True):
                 "devstore)" % (dev), level=INFO)
             return True
 
-        for key, val in devstore.iteritems():
+        for key, val in devstore.items():
             if key != masterkey and val.get('blkid') == blk_uuid:
                 log("Device '%s' appears to be in use by Swift (found in "
                     "local devstore) but has a different "
@@ -354,7 +355,7 @@ def is_device_in_ring(dev, skip_rel_check=False, ignore_deactivated=True):
         if ignore_deactivated:
             deactivated = [k == masterkey and v.get('blkid') == blk_uuid and
                            v.get('status') != 'active'
-                           for k, v in devstore.iteritems()]
+                           for k, v in devstore.items()]
 
     if skip_rel_check:
         log("Device '%s' does not appear to be in use by swift (searched "
@@ -387,7 +388,9 @@ def get_device_blkid(dev):
     :returns: UUID of device if found else None
     """
     try:
-        blk_uuid = subprocess.check_output(['blkid', '-s', 'UUID', dev])
+        blk_uuid = (subprocess
+                    .check_output(['blkid', '-s', 'UUID', dev])
+                    .decode('ascii'))
     except CalledProcessError:
         # If the device has not be used or formatted yet we expect this to
         # fail.
@@ -420,7 +423,7 @@ def remember_devices(devs):
             log("Device '%s' already in devstore (status:%s)" %
                 (dev, devstore[key].get('status')), level=DEBUG)
         else:
-            existing = [(k, v) for k, v in devstore.iteritems()
+            existing = [(k, v) for k, v in devstore.items()
                         if v.get('blkid') == blk_uuid and
                         re.match("^(.+)@(.+)$", k).group(1) == dev]
             if existing:
@@ -435,7 +438,7 @@ def remember_devices(devs):
                 devstore[key] = {'blkid': blk_uuid, 'status': 'active'}
 
     if devstore:
-        kvstore.set(key='devices', value=json.dumps(devstore))
+        kvstore.set(key='devices', value=json.dumps(devstore, sort_keys=True))
 
     kvstore.flush()
     kvstore.close()

tox.ini

@@ -2,8 +2,9 @@
 # This file is managed centrally by release-tools and should not be modified
 # within individual charm repos.
 [tox]
-envlist = pep8,py27
+envlist = pep8,py27,py35,py36
 skipsdist = True
+skip_missing_interpreters = True
 
 [testenv]
 setenv = VIRTUAL_ENV={envdir}
@@ -18,8 +19,7 @@ passenv = HOME TERM AMULET_* CS_API_*
 
 [testenv:py27]
 basepython = python2.7
-deps = -r{toxinidir}/requirements.txt
+commands = /bin/true
-       -r{toxinidir}/test-requirements.txt
 
 [testenv:py35]
 basepython = python3.5
@@ -32,7 +32,7 @@ deps = -r{toxinidir}/requirements.txt
        -r{toxinidir}/test-requirements.txt
 
 [testenv:pep8]
-basepython = python2.7
+basepython = python3
 deps = -r{toxinidir}/requirements.txt
        -r{toxinidir}/test-requirements.txt
 commands = flake8 {posargs} hooks unit_tests tests actions lib

unit_tests/__init__.py

@@ -11,3 +11,20 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+
+import os
+import sys
+
+_path = os.path.dirname(os.path.realpath(__file__))
+_actions = os.path.abspath(os.path.join(_path, '../actions'))
+_hooks = os.path.abspath(os.path.join(_path, '../hooks'))
+_lib = os.path.abspath(os.path.join(_path, '../lib'))
+
+
+def _add_path(path):
+    if path not in sys.path:
+        sys.path.insert(1, path)
+
+_add_path(_actions)
+_add_path(_hooks)
+_add_path(_lib)

unit_tests/test_actions.py

@@ -21,7 +21,7 @@ import unittest
 import mock
 import yaml
 
-from test_utils import CharmTestCase
+from unit_tests.test_utils import CharmTestCase
 
 from mock import patch, MagicMock
 
@@ -30,12 +30,12 @@ from mock import patch, MagicMock
 sys.modules['apt'] = MagicMock()
 sys.modules['apt_pkg'] = MagicMock()
 
-with patch('actions.hooks.charmhelpers.contrib.hardening.harden.harden') as \
+with patch('charmhelpers.contrib.hardening.harden.harden') as \
         mock_dec:
     mock_dec.side_effect = (lambda *dargs, **dkwargs: lambda f:
                             lambda *args, **kwargs: f(*args, **kwargs))
-    with patch('actions.hooks.lib.misc_utils.is_paused') as is_paused:
+    with patch('lib.misc_utils.is_paused') as is_paused:
-        with patch('actions.hooks.lib.swift_storage_utils.register_configs'):
+        with patch('lib.swift_storage_utils.register_configs'):
             import actions.actions
 
 
@@ -196,7 +196,8 @@ class GetActionParserTestCase(unittest.TestCase):
         """ArgumentParser is seeded from actions.yaml."""
         actions_yaml = tempfile.NamedTemporaryFile(
             prefix="GetActionParserTestCase", suffix="yaml")
-        actions_yaml.write(yaml.dump({"foo": {"description": "Foo is bar"}}))
+        actions_yaml.write(
+            yaml.dump({"foo": {"description": "Foo is bar"}}).encode('utf-8'))
         actions_yaml.seek(0)
         parser = actions.actions.get_action_parser(actions_yaml.name, "foo",
                                                    get_services=lambda: [])

unit_tests/test_actions_openstack_upgrade.py

@@ -31,9 +31,7 @@ with patch('charmhelpers.contrib.hardening.harden.harden') as mock_dec:
         with patch('lib.swift_storage_utils.register_configs'):
             import actions.openstack_upgrade as openstack_upgrade
 
-from test_utils import (
+from unit_tests.test_utils import CharmTestCase
-    CharmTestCase
-)
 
 TO_PATCH = [
     'config_changed',
@@ -47,10 +45,9 @@ class TestSwiftStorageUpgradeActions(CharmTestCase):
         super(TestSwiftStorageUpgradeActions, self).setUp(openstack_upgrade,
                                                           TO_PATCH)
 
-    @patch('actions.charmhelpers.contrib.openstack.utils.config')
+    @patch('charmhelpers.contrib.openstack.utils.config')
-    @patch('actions.charmhelpers.contrib.openstack.utils.action_set')
+    @patch('charmhelpers.contrib.openstack.utils.action_set')
-    @patch('actions.charmhelpers.contrib.openstack.utils.'
+    @patch('charmhelpers.contrib.openstack.utils.openstack_upgrade_available')
-           'openstack_upgrade_available')
     def test_openstack_upgrade_true(self, upgrade_avail,
                                     action_set, config):
         upgrade_avail.return_value = True
@@ -61,10 +58,9 @@ class TestSwiftStorageUpgradeActions(CharmTestCase):
         self.assertTrue(self.do_openstack_upgrade.called)
         self.assertTrue(self.config_changed.called)
 
-    @patch('actions.charmhelpers.contrib.openstack.utils.config')
+    @patch('charmhelpers.contrib.openstack.utils.config')
-    @patch('actions.charmhelpers.contrib.openstack.utils.action_set')
+    @patch('charmhelpers.contrib.openstack.utils.action_set')
-    @patch('actions.charmhelpers.contrib.openstack.utils.'
+    @patch('charmhelpers.contrib.openstack.utils.openstack_upgrade_available')
-           'openstack_upgrade_available')
     def test_openstack_upgrade_false(self, upgrade_avail,
                                      action_set, config):
         upgrade_avail.return_value = True

unit_tests/test_check_swift_storage.py

@@ -15,7 +15,7 @@
 import datetime
 import sys
 import unittest
-import urllib2
+import urllib
 
 from mock import (
     patch,
@@ -63,7 +63,7 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
         """
         Ensure md5 checksum is generated from a file content
         """
-        with patch("__builtin__.open", mock_open(read_data='data')) as \
+        with patch("builtins.open", mock_open(read_data=b'data')) as \
                 mock_file:
             result = generate_md5('path/to/file')
             mock_file.assert_called_with('path/to/file', 'rb')
@@ -71,24 +71,26 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
             self.assertEqual(result,
                              '8d777f385d3dfec8815d20f7496026dc')
 
-    @patch('urllib2.urlopen')
+    @patch('urllib.request.urlopen')
     def test_check_md5_unknown_urlerror(self, mock_urlopen):
         """
-        Force urllib2.URLError to test try-except
+        Force urllib.request.URLError to test try-except
         """
         base_url = 'http://localhost:6000/recon/'
         url = '{}ringmd5'.format(base_url)
         error = 'connection refused'
-        mock_urlopen.side_effect = urllib2.URLError(Mock(return_value=error))
+        mock_urlopen.side_effect = (urllib
+                                    .error
+                                    .URLError(Mock(return_value=error)))
         result = check_md5(base_url)
         self.assertEqual(result,
                          [(STATUS_UNKNOWN,
                            "Can't open url: {}".format(url))])
 
-    @patch('urllib2.urlopen')
+    @patch('urllib.request.urlopen')
     def test_check_md5_unknown_valueerror1(self, mock_urlopen):
         """
-        Force ValueError on urllib2 to test try-except
+        Force ValueError on urllib.error.URLError to test try-except
         """
         base_url = 'asdfasdf'
         url = '{}ringmd5'.format(base_url)
@@ -99,7 +101,7 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                          [(STATUS_UNKNOWN,
                            "Can't parse status data")])
 
-    @patch('urllib2.urlopen')
+    @patch('urllib.request.urlopen')
     def test_check_md5_unknown_valueerror2(self, mock_urlopen):
         """
         Force ValueError on json to test try-catch
@@ -125,7 +127,7 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                 '"0ea1ec9585ef644ce2b5c5b1dced4128"}'
         pmock_jdata = PropertyMock(return_value=jdata)
         mock_generate_md5.side_effect = IOError()
-        with patch('urllib2.urlopen') as mock_urlopen:
+        with patch('urllib.request.urlopen') as mock_urlopen:
             mock_urlopen.return_value = MagicMock(read=pmock_jdata)
             result = check_md5('.')
             mock_urlopen.assert_called_with('.ringmd5')
@@ -148,7 +150,7 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                 '"0ea1ec9585ef644ce2b5c5b1dced4128"}'
         pmock_jdata = PropertyMock(return_value=jdata)
         mock_generate_md5.return_value = 'xxxx'
-        with patch('urllib2.urlopen') as mock_urlopen:
+        with patch('urllib.request.urlopen') as mock_urlopen:
             mock_urlopen.return_value = MagicMock(read=pmock_jdata)
             result = check_md5('.')
             mock_urlopen.assert_called_with('.ringmd5')
@@ -171,22 +173,24 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                 '"6b4f3a0ef3731f18291ecd053ce0d9b6"}'
         pmock_jdata = PropertyMock(return_value=jdata)
         mock_generate_md5.return_value = '6b4f3a0ef3731f18291ecd053ce0d9b6'
-        with patch('urllib2.urlopen') as mock_urlopen:
+        with patch('urllib.request.urlopen') as mock_urlopen:
             mock_urlopen.return_value = MagicMock(read=pmock_jdata)
             result = check_md5('.')
             mock_urlopen.assert_called_with('.ringmd5')
             self.assertEqual(result,
                              [(STATUS_OK, 'OK')])
 
-    @patch('urllib2.urlopen')
+    @patch('urllib.request.urlopen')
     def test_check_replication_unknown_urlerror(self, mock_urlopen):
         """
-        Force urllib2.URLError to test try-catch
+        Force urllib.error.URLError to test try-catch
         """
         base_url = 'http://localhost:6000/recon/'
         url = '{}replication/{}'
         error = 'connection refused'
-        mock_urlopen.side_effect = urllib2.URLError(Mock(return_value=error))
+        mock_urlopen.side_effect = (urllib
+                                    .error
+                                    .URLError(Mock(return_value=error)))
         result = check_replication(base_url, 60)
         expected_result = [(STATUS_UNKNOWN,
                             "Can't open url: "
@@ -194,10 +198,10 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                            for name in ('account', 'object', 'container')]
         self.assertEqual(result, expected_result)
 
-    @patch('urllib2.urlopen')
+    @patch('urllib.request.urlopen')
     def test_check_replication_unknown_valueerror1(self, mock_urlopen):
         """
-        Force ValueError on urllib2 to test try-catch
+        Force ValueError on urllib.error to test try-catch
         """
         base_url = '.'
         mock_urlopen.side_effect = ValueError(Mock(return_value=''))
@@ -206,7 +210,7 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                          3*[(STATUS_UNKNOWN,
                              "Can't parse status data")])
 
-    @patch('urllib2.urlopen')
+    @patch('urllib.request.urlopen')
     def test_check_replication_unknown_valueerror2(self, mock_urlopen):
         """
         Force ValueError on json to test try-catch
@@ -233,7 +237,7 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                 '"empty": 0}, "replication_time": 0.0076580047607421875}'
         pmock_jdata = PropertyMock(return_value=jdata)
         mock_timestamp.return_value = (None, 0)
-        with patch('urllib2.urlopen') as mock_urlopen:
+        with patch('urllib.request.urlopen') as mock_urlopen:
             mock_urlopen.return_value = MagicMock(read=pmock_jdata)
             result = check_replication(base_url, [4, 10, 4, 10])
             self.assertEqual(result,
@@ -256,7 +260,7 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                 '"empty": 0}, "replication_time": 0.0076580047607421875}'
         pmock_jdata = PropertyMock(return_value=jdata)
         mock_timestamp.return_value = (MagicMock(days=0, seconds=12), 0)
-        with patch('urllib2.urlopen') as mock_urlopen:
+        with patch('urllib.request.urlopen') as mock_urlopen:
             mock_urlopen.return_value = MagicMock(read=pmock_jdata)
             result = check_replication(base_url, [4, 10, 4, 10])
             self.assertEqual(result,
@@ -279,7 +283,7 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                 '"empty": 0}, "replication_time": 0.0076580047607421875}'
         pmock_jdata = PropertyMock(return_value=jdata)
         mock_timestamp.return_value = (MagicMock(days=0, seconds=0), 12)
-        with patch('urllib2.urlopen') as mock_urlopen:
+        with patch('urllib.request.urlopen') as mock_urlopen:
             mock_urlopen.return_value = MagicMock(read=pmock_jdata)
             result = check_replication(base_url, [4, 10, 4, 10])
             self.assertEqual(result,
@@ -299,7 +303,7 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                 '"empty": 0}, "replication_time": 0.0076580047607421875}'
         pmock_jdata = PropertyMock(return_value=jdata)
         mock_timestamp.return_value = (MagicMock(days=0, seconds=0), -1)
-        with patch('urllib2.urlopen') as mock_urlopen:
+        with patch('urllib.request.urlopen') as mock_urlopen:
             mock_urlopen.return_value = MagicMock(read=pmock_jdata)
             result = check_replication(base_url, [4, 10, 4, 10])
             self.assertEqual(result,
@@ -321,7 +325,7 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                 '"empty": 0}, "replication_time": 0.0076580047607421875}'
         pmock_jdata = PropertyMock(return_value=jdata)
         mock_timestamp.return_value = (MagicMock(days=0, seconds=5), 0)
-        with patch('urllib2.urlopen') as mock_urlopen:
+        with patch('urllib.request.urlopen') as mock_urlopen:
             mock_urlopen.return_value = MagicMock(read=pmock_jdata)
             result = check_replication(base_url, [4, 10, 4, 10])
             self.assertEqual(result,
@@ -344,7 +348,7 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                 '"empty": 0}, "replication_time": 0.0076580047607421875}'
         pmock_jdata = PropertyMock(return_value=jdata)
         mock_timestamp.return_value = (MagicMock(days=2, seconds=5), 0)
-        with patch('urllib2.urlopen') as mock_urlopen:
+        with patch('urllib.request.urlopen') as mock_urlopen:
             mock_urlopen.return_value = MagicMock(read=pmock_jdata)
             result = check_replication(base_url, [4, 10, 4, 10])
             self.assertEqual(result,
@@ -367,7 +371,8 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                 '"empty": 0}, "replication_time": 0.0076580047607421875}'
         pmock_jdata = PropertyMock(return_value=jdata)
         mock_timestamp.return_value = (MagicMock(days=0, seconds=0), 5)
-        with patch('urllib2.urlopen') as mock_urlopen:
+        # with patch('urllib2.urlopen') as mock_urlopen:
+        with patch('urllib.request.urlopen') as mock_urlopen:
             mock_urlopen.return_value = MagicMock(read=pmock_jdata)
             result = check_replication(base_url, [4, 10, 4, 10])
             self.assertEqual(result,
@@ -388,7 +393,7 @@ class CheckSwiftStorageTestCase(unittest.TestCase):
                 '"empty": 0}, "replication_time": 0.0076580047607421875}'
         pmock_jdata = PropertyMock(return_value=jdata)
         mock_timestamp.return_value = (MagicMock(days=0, seconds=0), 0)
-        with patch('urllib2.urlopen') as mock_urlopen:
+        with patch('urllib.request.urlopen') as mock_urlopen:
             mock_urlopen.return_value = MagicMock(read=pmock_jdata)
             result = check_replication(base_url, [4, 10, 4, 10])
             self.assertEqual(result, [(STATUS_OK, 'OK')])

unit_tests/test_swift_storage_context.py

@@ -13,7 +13,8 @@
 # limitations under the License.
 
 from mock import MagicMock
-from test_utils import CharmTestCase, patch_open
+
+from unit_tests.test_utils import CharmTestCase, patch_open
 
 import lib.swift_storage_context as swift_context
 

unit_tests/test_swift_storage_relations.py

@@ -13,18 +13,18 @@
 # limitations under the License.
 
 from mock import patch
-import os
 import json
-import uuid
+import os
 import tempfile
+import uuid
 
-from test_utils import CharmTestCase, TestKV, patch_open
+from unit_tests.test_utils import CharmTestCase, TestKV, patch_open
 
-with patch('hooks.charmhelpers.contrib.hardening.harden.harden') as mock_dec:
+with patch('charmhelpers.contrib.hardening.harden.harden') as mock_dec:
     mock_dec.side_effect = (lambda *dargs, **dkwargs: lambda f:
                             lambda *args, **kwargs: f(*args, **kwargs))
-    with patch('hooks.lib.misc_utils.is_paused') as is_paused:
+    with patch('lib.misc_utils.is_paused') as is_paused:
-        with patch('hooks.lib.swift_storage_utils.register_configs') as _:
+        with patch('lib.swift_storage_utils.register_configs') as _:
             import hooks.swift_storage_hooks as hooks
 
 from lib.swift_storage_utils import PACKAGES
@@ -69,7 +69,7 @@ TO_PATCH = [
 ]
 
 
-UFW_DUMMY_RULES = """
+UFW_DUMMY_RULES = b"""
 # Don't delete these required lines, otherwise there will be errors
 *filter
 :ufw-before-input - [0:0]
@@ -88,8 +88,7 @@ UFW_DUMMY_RULES = """
 class SwiftStorageRelationsTests(CharmTestCase):
 
     def setUp(self):
-        super(SwiftStorageRelationsTests, self).setUp(hooks,
+        super(SwiftStorageRelationsTests, self).setUp(hooks, TO_PATCH)
-                                                      TO_PATCH)
         self.config.side_effect = self.test_config.get
         self.relation_get.side_effect = self.test_relation.get
         self.get_relation_ip.return_value = '10.10.10.2'
@@ -178,14 +177,14 @@ class SwiftStorageRelationsTests(CharmTestCase):
         self.assertTrue(self.update_nrpe_config.called)
         self.assertTrue(mock_ensure_devs_tracked.called)
 
-    @patch('hooks.lib.swift_storage_utils.get_device_blkid',
+    @patch('lib.swift_storage_utils.get_device_blkid',
            lambda dev: str(uuid.uuid4()))
     @patch.object(hooks.os, 'environ')
-    @patch('hooks.lib.swift_storage_utils.os.path.isdir', lambda *args: True)
+    @patch('lib.swift_storage_utils.os.path.isdir', lambda *args: True)
     @patch.object(hooks, 'relation_set')
-    @patch('hooks.lib.swift_storage_utils.local_unit')
+    @patch('lib.swift_storage_utils.local_unit')
-    @patch('hooks.lib.swift_storage_utils.relation_ids', lambda *args: [])
+    @patch('lib.swift_storage_utils.relation_ids', lambda *args: [])
-    @patch('hooks.lib.swift_storage_utils.KVStore')
+    @patch('lib.swift_storage_utils.KVStore')
     @patch.object(uuid, 'uuid4', lambda: 'a-test-uuid')
     def _test_storage_joined_single_device(self, mock_kvstore, mock_local_unit,
                                            mock_rel_set, mock_environ,
@@ -199,6 +198,9 @@ class SwiftStorageRelationsTests(CharmTestCase):
         kvstore.get.return_value = None
         self.test_kv.set('prepared-devices', ['/dev/vdb'])
 
+        # py3 is very picky, and log is only patched in
+        # hooks.swift_storage_hooks
+        with patch('lib.swift_storage_utils.log'):
             hooks.swift_storage_relation_joined()
 
         self.get_relation_ip.assert_called_once_with('swift-storage')
@@ -230,8 +232,8 @@ class SwiftStorageRelationsTests(CharmTestCase):
         devices = {"vdb@%s" % (test_uuid):
                    {"status": "active",
                     "blkid": 'a-test-uuid'}}
-        kvstore.set.assert_called_with(key='devices',
+        kvstore.set.assert_called_with(
-                                       value=json.dumps(devices))
+            key='devices', value=json.dumps(devices, sort_keys=True))
 
     def test_storage_joined_single_device_juju_1(self):
         '''Ensure use of JUJU_ENV_UUID for Juju < 2'''
@@ -241,13 +243,13 @@ class SwiftStorageRelationsTests(CharmTestCase):
         '''Ensure use of JUJU_MODEL_UUID for Juju >= 2'''
         self._test_storage_joined_single_device(env_key='JUJU_MODEL_UUID')
 
-    @patch('hooks.lib.swift_storage_utils.get_device_blkid',
+    @patch('lib.swift_storage_utils.get_device_blkid',
            lambda dev: '%s-blkid-uuid' % os.path.basename(dev))
     @patch.object(hooks.os, 'environ')
-    @patch('hooks.lib.swift_storage_utils.os.path.isdir', lambda *args: True)
+    @patch('lib.swift_storage_utils.os.path.isdir', lambda *args: True)
-    @patch('hooks.lib.swift_storage_utils.local_unit')
+    @patch('lib.swift_storage_utils.local_unit')
-    @patch('hooks.lib.swift_storage_utils.relation_ids', lambda *args: [])
+    @patch('lib.swift_storage_utils.relation_ids', lambda *args: [])
-    @patch('hooks.lib.swift_storage_utils.KVStore')
+    @patch('lib.swift_storage_utils.KVStore')
     @patch.object(uuid, 'uuid4', lambda: 'a-test-uuid')
     def test_storage_joined_multi_device(self, mock_kvstore, mock_local_unit,
                                          mock_environ):
@@ -272,6 +274,9 @@ class SwiftStorageRelationsTests(CharmTestCase):
 
         kvstore.get.side_effect = fake_kv_get
 
+        # py3 is very picky, and log is only patched in
+        # hooks.swift_storage_hooks
+        with patch('lib.swift_storage_utils.log'):
             hooks.swift_storage_relation_joined()
         devices = {"vdb@%s" % (test_uuid): {"status": "active",
                                             "blkid": 'vdb-blkid-uuid'},
@@ -280,17 +285,17 @@ class SwiftStorageRelationsTests(CharmTestCase):
                    "vdc@%s" % (test_uuid): {"status": "active",
                                             "blkid": 'vdc-blkid-uuid'}}
         kvstore.set.assert_called_with(
-            key='devices', value=json.dumps(devices)
+            key='devices', value=json.dumps(devices, sort_keys=True)
         )
         self.get_relation_ip.assert_called_once_with('swift-storage')
 
-    @patch('hooks.lib.swift_storage_utils.get_device_blkid',
+    @patch('lib.swift_storage_utils.get_device_blkid',
            lambda dev: '%s-blkid-uuid' % os.path.basename(dev))
     @patch.object(hooks.os, 'environ')
-    @patch('hooks.lib.swift_storage_utils.os.path.isdir', lambda *args: True)
+    @patch('lib.swift_storage_utils.os.path.isdir', lambda *args: True)
-    @patch('hooks.lib.swift_storage_utils.local_unit')
+    @patch('lib.swift_storage_utils.local_unit')
-    @patch('hooks.lib.swift_storage_utils.relation_ids', lambda *args: [])
+    @patch('lib.swift_storage_utils.relation_ids', lambda *args: [])
-    @patch('hooks.lib.swift_storage_utils.KVStore')
+    @patch('lib.swift_storage_utils.KVStore')
     def test_storage_joined_dev_exists_unknown_juju_env_uuid(self,
                                                              mock_kvstore,
                                                              mock_local_unit,
@@ -317,7 +322,11 @@ class SwiftStorageRelationsTests(CharmTestCase):
 
         kvstore.get.side_effect = fake_kv_get
 
+        # py3 is very picky, and log is only patched in
+        # hooks.swift_storage_hooks
+        with patch('lib.swift_storage_utils.log'):
             hooks.swift_storage_relation_joined()
+
         devices = {"vdb@%s" % (test_uuid): {"status": "active",
                                             "blkid": 'vdb-blkid-uuid'},
                    "vdd@%s" % (test_uuid): {"status": "active",
@@ -325,7 +334,7 @@ class SwiftStorageRelationsTests(CharmTestCase):
                    "vdc@%s" % (test_uuid): {"status": "active",
                                             "blkid": 'vdc-blkid-uuid'}}
         kvstore.set.assert_called_with(
-            key='devices', value=json.dumps(devices)
+            key='devices', value=json.dumps(devices, sort_keys=True)
         )
         self.get_relation_ip.assert_called_once_with('swift-storage')
 
@@ -345,8 +354,8 @@ class SwiftStorageRelationsTests(CharmTestCase):
             'http://swift-proxy.com/rings/'
         )
 
-    @patch('sys.argv')
+    @patch('sys.argv', new=['dodah'])
-    def test_main_hook_missing(self, _argv):
+    def test_main_hook_missing(self):
         hooks.main()
         self.assertTrue(self.log.called)
 

unit_tests/test_swift_storage_utils.py

@@ -12,12 +12,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from collections import namedtuple
+from mock import call, patch, MagicMock
 import shutil
 import tempfile
-from collections import namedtuple
 
-from mock import call, patch, MagicMock
+from unit_tests.test_utils import CharmTestCase, TestKV, patch_open
-from test_utils import CharmTestCase, TestKV, patch_open
 
 import lib.swift_storage_utils as swift_utils
 
@@ -175,7 +175,7 @@ class SwiftStorageUtilsTests(CharmTestCase):
         self.test_config.set('block-device', bdevs)
         result = swift_utils.determine_block_devices()
         ex = ['/dev/vdb', '/dev/vdc', '/tmp/swift.img']
-        ex = list(set(ex))
+        ex = sorted(set(ex))
         self.assertEqual(ex, result)
 
     @patch.object(swift_utils, 'ensure_block_device')
@@ -185,7 +185,7 @@ class SwiftStorageUtilsTests(CharmTestCase):
         self.test_config.set('block-device', bdevs)
         result = swift_utils.determine_block_devices()
         ex = ['/dev/vdb', '/dev/vdc', '/tmp/swift.img']
-        ex = list(set(ex))
+        ex = sorted(set(ex))
         self.assertEqual(ex, result)
 
     @patch.object(swift_utils, 'ensure_block_device')
@@ -206,14 +206,16 @@ class SwiftStorageUtilsTests(CharmTestCase):
         def _findmnt(cmd):
             dev = cmd[1].split('/')[-1]
             mnt_point = '/srv/node/' + dev
-            return FINDMNT_FOUND_TEMPLATE.format(mnt_point, dev)
+            return (FINDMNT_FOUND_TEMPLATE
+                    .format(mnt_point, dev).encode('ascii'))
         _check_output.side_effect = _findmnt
         _ensure.side_effect = self._fake_ensure
         self.test_config.set('block-device', 'guess')
         _find.return_value = ['/dev/vdb', '/dev/sdb']
         result = swift_utils.determine_block_devices()
         self.assertTrue(_find.called)
-        self.assertEqual(result, ['/dev/vdb', '/dev/sdb'])
+        # always returns sorted results
+        self.assertEqual(result, ['/dev/sdb', '/dev/vdb'])
 
     @patch.object(swift_utils, 'check_output')
     @patch.object(swift_utils, 'find_block_devices')
@@ -225,7 +227,8 @@ class SwiftStorageUtilsTests(CharmTestCase):
         def _findmnt(cmd):
             dev = cmd[1].split('/')[-1]
             mnt_point = '/'
-            return FINDMNT_FOUND_TEMPLATE.format(mnt_point, dev)
+            return (FINDMNT_FOUND_TEMPLATE
+                    .format(mnt_point, dev).encode('ascii'))
         _check_output.side_effect = _findmnt
         _ensure.side_effect = self._fake_ensure
         self.test_config.set('block-device', 'guess')
@@ -569,7 +572,7 @@ class SwiftStorageUtilsTests(CharmTestCase):
     def test_get_device_blkid(self, mock_check_output):
         dev = '/dev/vdb'
         cmd = ['blkid', '-s', 'UUID', dev]
-        ret = '/dev/vdb: UUID="808bc298-0609-4619-aaef-ed7a5ab0ebb7" \n'
+        ret = b'/dev/vdb: UUID="808bc298-0609-4619-aaef-ed7a5ab0ebb7" \n'
         mock_check_output.return_value = ret
         uuid = swift_utils.get_device_blkid(dev)
         self.assertEqual(uuid, "808bc298-0609-4619-aaef-ed7a5ab0ebb7")

unit_tests/test_utils.py

@@ -12,9 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import io
 import logging
-import unittest
 import os
+import unittest
 import yaml
 
 from contextlib import contextmanager
@@ -37,7 +38,7 @@ def load_config():
 
     if not config:
         logging.error('Could not find config.yaml in any parent directory '
-                      'of %s. ' % file)
+                      'of %s. ' % __file__)
         raise Exception
 
     return yaml.safe_load(open(config).read())['options']
@@ -50,7 +51,7 @@ def get_default_config():
     '''
     default_config = {}
     config = load_config()
-    for k, v in config.iteritems():
+    for k, v in config.items():
         if 'default' in v:
             default_config[k] = v['default']
         else:
@@ -141,12 +142,12 @@ def patch_open():
 
     Yields the mock for "open" and "file", respectively.'''
     mock_open = MagicMock(spec=open)
-    mock_file = MagicMock(spec=file)
+    mock_file = MagicMock(spec=io.FileIO)
 
     @contextmanager
     def stub_open(*args, **kwargs):
         mock_open(*args, **kwargs)
         yield mock_file
 
-    with patch('__builtin__.open', stub_open):
+    with patch('builtins.open', stub_open):
         yield mock_open, mock_file