From fc81200315e93585ec96c220f792aefbde9826d8 Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Fri, 28 Sep 2018 12:38:36 +0100 Subject: [PATCH] Upgrade the charm to py3 runtime Change-Id: I98f4e6664080407a045ca5e76db59d46ffa9c38a --- actions/actions.py | 15 +++- actions/charmhelpers | 1 - actions/openstack_upgrade.py | 17 ++++- .../check_swift_storage.py | 19 ++--- hooks/charmhelpers | 1 - hooks/install | 2 +- hooks/lib | 1 - hooks/swift_storage_hooks.py | 15 +++- lib/swift_storage_utils.py | 21 +++--- tox.ini | 8 +- unit_tests/__init__.py | 17 +++++ unit_tests/test_actions.py | 11 +-- unit_tests/test_actions_openstack_upgrade.py | 18 ++--- unit_tests/test_check_swift_storage.py | 55 +++++++------- unit_tests/test_swift_storage_context.py | 3 +- unit_tests/test_swift_storage_relations.py | 75 +++++++++++-------- unit_tests/test_swift_storage_utils.py | 21 +++--- unit_tests/test_utils.py | 11 +-- 18 files changed, 190 insertions(+), 121 deletions(-) delete mode 120000 actions/charmhelpers delete mode 120000 hooks/charmhelpers delete mode 120000 hooks/lib diff --git a/actions/actions.py b/actions/actions.py index e087ab3..beae29f 100755 --- a/actions/actions.py +++ b/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) diff --git a/actions/charmhelpers b/actions/charmhelpers deleted file mode 120000 index 8f067ed..0000000 --- a/actions/charmhelpers +++ /dev/null @@ -1 +0,0 @@ -../charmhelpers/ \ No newline at end of file diff --git a/actions/openstack_upgrade.py b/actions/openstack_upgrade.py index ee64237..270e26e 100755 --- a/actions/openstack_upgrade.py +++ b/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, ) diff --git a/files/nrpe-external-master/check_swift_storage.py b/files/nrpe-external-master/check_swift_storage.py index 3869cb9..ff1f6e0 100755 --- a/files/nrpe-external-master/check_swift_storage.py +++ b/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() - buffer = f.read(2 ** 20) - while buffer: - md5.update(buffer) + while True: buffer = f.read(2 ** 20) + if not buffer: + break + md5.update(buffer) 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: diff --git a/hooks/charmhelpers b/hooks/charmhelpers deleted file mode 120000 index 8f067ed..0000000 --- a/hooks/charmhelpers +++ /dev/null @@ -1 +0,0 @@ -../charmhelpers/ \ No newline at end of file diff --git a/hooks/install b/hooks/install index 29ff689..50b8cad 100755 --- a/hooks/install +++ b/hooks/install @@ -11,7 +11,7 @@ check_and_install() { fi } -PYTHON="python" +PYTHON="python3" for dep in ${DEPS[@]}; do check_and_install ${PYTHON} ${dep} diff --git a/hooks/lib b/hooks/lib deleted file mode 120000 index dc598c5..0000000 --- a/hooks/lib +++ /dev/null @@ -1 +0,0 @@ -../lib \ No newline at end of file diff --git a/hooks/swift_storage_hooks.py b/hooks/swift_storage_hooks.py index a37a18c..4c04bc7 100755 --- a/hooks/swift_storage_hooks.py +++ b/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, diff --git a/lib/swift_storage_utils.py b/lib/swift_storage_utils.py index 0f1ad2c..98b7dbc 100644 --- a/lib/swift_storage_utils.py +++ b/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() diff --git a/tox.ini b/tox.ini index 930d526..275ee71 100644 --- a/tox.ini +++ b/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 - -r{toxinidir}/test-requirements.txt +commands = /bin/true [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 diff --git a/unit_tests/__init__.py b/unit_tests/__init__.py index 9b088de..e801b03 100644 --- a/unit_tests/__init__.py +++ b/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) diff --git a/unit_tests/test_actions.py b/unit_tests/test_actions.py index 1991e13..84132d1 100644 --- a/unit_tests/test_actions.py +++ b/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('actions.hooks.lib.swift_storage_utils.register_configs'): + with patch('lib.misc_utils.is_paused') as is_paused: + 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: []) diff --git a/unit_tests/test_actions_openstack_upgrade.py b/unit_tests/test_actions_openstack_upgrade.py index 54eef47..950f02a 100644 --- a/unit_tests/test_actions_openstack_upgrade.py +++ b/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 ( - CharmTestCase -) +from unit_tests.test_utils import 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('actions.charmhelpers.contrib.openstack.utils.action_set') - @patch('actions.charmhelpers.contrib.openstack.utils.' - 'openstack_upgrade_available') + @patch('charmhelpers.contrib.openstack.utils.config') + @patch('charmhelpers.contrib.openstack.utils.action_set') + @patch('charmhelpers.contrib.openstack.utils.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('actions.charmhelpers.contrib.openstack.utils.action_set') - @patch('actions.charmhelpers.contrib.openstack.utils.' - 'openstack_upgrade_available') + @patch('charmhelpers.contrib.openstack.utils.config') + @patch('charmhelpers.contrib.openstack.utils.action_set') + @patch('charmhelpers.contrib.openstack.utils.openstack_upgrade_available') def test_openstack_upgrade_false(self, upgrade_avail, action_set, config): upgrade_avail.return_value = True diff --git a/unit_tests/test_check_swift_storage.py b/unit_tests/test_check_swift_storage.py index ccda416..55bf050 100644 --- a/unit_tests/test_check_swift_storage.py +++ b/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')]) diff --git a/unit_tests/test_swift_storage_context.py b/unit_tests/test_swift_storage_context.py index 6c9e76a..228c424 100644 --- a/unit_tests/test_swift_storage_context.py +++ b/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 diff --git a/unit_tests/test_swift_storage_relations.py b/unit_tests/test_swift_storage_relations.py index d7c53a3..ebc591f 100644 --- a/unit_tests/test_swift_storage_relations.py +++ b/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('hooks.lib.swift_storage_utils.register_configs') as _: + with patch('lib.misc_utils.is_paused') as is_paused: + 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, - TO_PATCH) + super(SwiftStorageRelationsTests, self).setUp(hooks, 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('hooks.lib.swift_storage_utils.relation_ids', lambda *args: []) - @patch('hooks.lib.swift_storage_utils.KVStore') + @patch('lib.swift_storage_utils.local_unit') + @patch('lib.swift_storage_utils.relation_ids', lambda *args: []) + @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,7 +198,10 @@ class SwiftStorageRelationsTests(CharmTestCase): kvstore.get.return_value = None self.test_kv.set('prepared-devices', ['/dev/vdb']) - hooks.swift_storage_relation_joined() + # 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', - value=json.dumps(devices)) + kvstore.set.assert_called_with( + 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('hooks.lib.swift_storage_utils.local_unit') - @patch('hooks.lib.swift_storage_utils.relation_ids', lambda *args: []) - @patch('hooks.lib.swift_storage_utils.KVStore') + @patch('lib.swift_storage_utils.os.path.isdir', lambda *args: True) + @patch('lib.swift_storage_utils.local_unit') + @patch('lib.swift_storage_utils.relation_ids', lambda *args: []) + @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,7 +274,10 @@ class SwiftStorageRelationsTests(CharmTestCase): kvstore.get.side_effect = fake_kv_get - hooks.swift_storage_relation_joined() + # 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", @@ -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('hooks.lib.swift_storage_utils.local_unit') - @patch('hooks.lib.swift_storage_utils.relation_ids', lambda *args: []) - @patch('hooks.lib.swift_storage_utils.KVStore') + @patch('lib.swift_storage_utils.os.path.isdir', lambda *args: True) + @patch('lib.swift_storage_utils.local_unit') + @patch('lib.swift_storage_utils.relation_ids', lambda *args: []) + @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 - hooks.swift_storage_relation_joined() + # 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') - def test_main_hook_missing(self, _argv): + @patch('sys.argv', new=['dodah']) + def test_main_hook_missing(self): hooks.main() self.assertTrue(self.log.called) diff --git a/unit_tests/test_swift_storage_utils.py b/unit_tests/test_swift_storage_utils.py index 889fc44..6436b87 100644 --- a/unit_tests/test_swift_storage_utils.py +++ b/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 test_utils import CharmTestCase, TestKV, patch_open +from unit_tests.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") diff --git a/unit_tests/test_utils.py b/unit_tests/test_utils.py index ee81104..a9f44c4 100644 --- a/unit_tests/test_utils.py +++ b/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