Convert charm to Python 3

Change-Id: Ib7cc06b3b42f26f725a9ea79f09189cc72952d29

.zuul.yaml

@@ -1,4 +1,3 @@
 - project:
     templates:
-      - python-charm-jobs
+      - python35-charm-jobs
-      - openstack-python35-jobs-nonvoting

actions/actions.py

@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python3
 #
 # Copyright 2016 Canonical Ltd
 #
@@ -14,11 +14,26 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import sys
 import os
 import subprocess
+import sys
 import traceback
+
 sys.path.append('hooks/')
+
+_path = os.path.dirname(os.path.realpath(__file__))
+_hooks = os.path.abspath(os.path.join(_path, '../hooks'))
+_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(_hooks)
+_add_path(_root)
+
+
 from charmhelpers.core.hookenv import (
     action_fail,
     action_get,
@@ -50,7 +65,7 @@ def status(args):
     cmd = ['crm', 'status', '--inactive']
 
     try:
-        result = subprocess.check_output(cmd)
+        result = subprocess.check_output(cmd).decode('utf-8')
         action_set({'result': result})
     except subprocess.CalledProcessError as e:
         log("ERROR: Failed call to crm resource status. "

charm-helpers-hooks.yaml

@@ -1,5 +1,5 @@
 repo: https://github.com/juju/charm-helpers
-destination: hooks/charmhelpers
+destination: charmhelpers
 include:
     - core
     - cli

hooks/hooks.py

@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python3
 #
 # Copyright 2016 Canonical Ltd
 #
@@ -20,6 +20,18 @@ import shutil
 import socket
 import sys
 
+_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)
+
+
 import pcmk
 
 from charmhelpers.core.hookenv import (
@@ -200,7 +212,7 @@ def migrate_maas_dns():
                         write_maas_dns_address(resource, res_ipaddr)
 
 
-@hooks.hook()
+@hooks.hook('upgrade-charm.real')
 def upgrade_charm():
     install()
     migrate_maas_dns()
@@ -269,10 +281,10 @@ def ha_relation_changed():
         return
 
     if True in [ra.startswith('ocf:openstack')
-                for ra in resources.itervalues()]:
+                for ra in resources.values()]:
         apt_install('openstack-resource-agents')
     if True in [ra.startswith('ocf:ceph')
-                for ra in resources.itervalues()]:
+                for ra in resources.values()]:
         apt_install('ceph-resource-agents')
 
     if True in [ra.startswith('ocf:maas')
@@ -328,7 +340,7 @@ def ha_relation_changed():
                 pcmk.commit('crm -w -F configure delete %s' % res_name)
 
         log('Configuring Resources: %s' % (resources), level=DEBUG)
-        for res_name, res_type in resources.iteritems():
+        for res_name, res_type in resources.items():
             # disable the service we are going to put in HA
             if res_type.split(':')[0] == "lsb":
                 disable_lsb_services(res_type.split(':')[1])
@@ -367,7 +379,7 @@ def ha_relation_changed():
                     raise Exception(msg)
 
         log('Configuring Groups: %s' % (groups), level=DEBUG)
-        for grp_name, grp_params in groups.iteritems():
+        for grp_name, grp_params in groups.items():
             if not pcmk.crm_opt_exists(grp_name):
                 cmd = ('crm -w -F configure group %s %s' %
                        (grp_name, grp_params))
@@ -375,14 +387,14 @@ def ha_relation_changed():
                 log('%s' % cmd, level=DEBUG)
 
         log('Configuring Master/Slave (ms): %s' % (ms), level=DEBUG)
-        for ms_name, ms_params in ms.iteritems():
+        for ms_name, ms_params in ms.items():
             if not pcmk.crm_opt_exists(ms_name):
                 cmd = 'crm -w -F configure ms %s %s' % (ms_name, ms_params)
                 pcmk.commit(cmd)
                 log('%s' % cmd, level=DEBUG)
 
         log('Configuring Orders: %s' % (orders), level=DEBUG)
-        for ord_name, ord_params in orders.iteritems():
+        for ord_name, ord_params in orders.items():
             if not pcmk.crm_opt_exists(ord_name):
                 cmd = 'crm -w -F configure order %s %s' % (ord_name,
                                                            ord_params)
@@ -390,7 +402,7 @@ def ha_relation_changed():
                 log('%s' % cmd, level=DEBUG)
 
         log('Configuring Clones: %s' % clones, level=DEBUG)
-        for cln_name, cln_params in clones.iteritems():
+        for cln_name, cln_params in clones.items():
             if not pcmk.crm_opt_exists(cln_name):
                 cmd = 'crm -w -F configure clone %s %s' % (cln_name,
                                                            cln_params)
@@ -402,7 +414,7 @@ def ha_relation_changed():
         # need to exist otherwise constraint creation will fail.
 
         log('Configuring Colocations: %s' % colocations, level=DEBUG)
-        for col_name, col_params in colocations.iteritems():
+        for col_name, col_params in colocations.items():
             if not pcmk.crm_opt_exists(col_name):
                 cmd = 'crm -w -F configure colocation %s %s' % (col_name,
                                                                 col_params)
@@ -410,14 +422,14 @@ def ha_relation_changed():
                 log('%s' % cmd, level=DEBUG)
 
         log('Configuring Locations: %s' % locations, level=DEBUG)
-        for loc_name, loc_params in locations.iteritems():
+        for loc_name, loc_params in locations.items():
             if not pcmk.crm_opt_exists(loc_name):
                 cmd = 'crm -w -F configure location %s %s' % (loc_name,
                                                               loc_params)
                 pcmk.commit(cmd)
                 log('%s' % cmd, level=DEBUG)
 
-        for res_name, res_type in resources.iteritems():
+        for res_name, res_type in resources.items():
             if len(init_services) != 0 and res_name in init_services:
                 # Checks that the resources are running and started.
                 # Ensure that clones are excluded as the resource is

hooks/install

@@ -1,20 +1,4 @@
 #!/bin/bash -e
-# Wrapper to deal with newer Ubuntu versions that don't have py2 installed
-# by default.
-
-declare -a DEPS=('apt' 'netaddr' 'netifaces' 'pip' 'yaml' 'dnspython')
-
-check_and_install() {
-    pkg="${1}-${2}"
-    if ! dpkg -s ${pkg} 2>&1 > /dev/null; then
-        apt-get -y install ${pkg}
-    fi
-}
-
-PYTHON="python"
-
-for dep in ${DEPS[@]}; do
-    check_and_install ${PYTHON} ${dep}
-done
 
+./hooks/install_deps
 exec ./hooks/install.real

hooks/install_deps

@@ -0,0 +1,18 @@
+#!/bin/bash -e
+# Wrapper to deal with newer Ubuntu versions that don't have py2 installed
+# by default.
+
+declare -a DEPS=('apt' 'netaddr' 'netifaces' 'pip' 'yaml' 'dnspython')
+
+check_and_install() {
+    pkg="${1}-${2}"
+    if ! dpkg -s ${pkg} 2>&1 > /dev/null; then
+        apt-get -y install ${pkg}
+    fi
+}
+
+PYTHON="python3"
+
+for dep in ${DEPS[@]}; do
+    check_and_install ${PYTHON} ${dep}
+done

hooks/maas.py

@@ -72,7 +72,7 @@ class MAASHelper(object):
 
         try:
             cmd = ['maas-cli', MAAS_PROFILE_NAME, 'nodes', 'list']
-            out = subprocess.check_output(cmd)
+            out = subprocess.check_output(cmd).decode('utf-8')
         except subprocess.CalledProcessError:
             log('Could not get node inventory from MAAS.', ERROR)
             return False

hooks/pcmk.py

@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import commands
 import hashlib
 import re
 import subprocess
@@ -22,7 +21,7 @@ import time
 import xml.etree.ElementTree as etree
 
 from distutils.version import StrictVersion
-from StringIO import StringIO
+from io import StringIO
 from charmhelpers.core import unitdata
 from charmhelpers.core.hookenv import (
     log,
@@ -47,7 +46,7 @@ def wait_for_pcmk(retries=12, sleep=10):
     for i in range(retries):
         if crm_up:
             return True
-        output = commands.getstatusoutput("crm node list")[1]
+        output = subprocess.getstatusoutput("crm node list")[1]
         crm_up = hostname in output
         time.sleep(sleep)
     if not crm_up:
@@ -61,7 +60,7 @@ def commit(cmd):
 
 
 def is_resource_present(resource):
-    status = commands.getstatusoutput("crm resource status %s" % resource)[0]
+    status = subprocess.getstatusoutput("crm resource status %s" % resource)[0]
     if status != 0:
         return False
 
@@ -87,7 +86,7 @@ def online(node=None):
 
 
 def crm_opt_exists(opt_name):
-    output = commands.getstatusoutput("crm configure show")[1]
+    output = subprocess.getstatusoutput("crm configure show")[1]
     if opt_name in output:
         return True
 
@@ -95,7 +94,8 @@ def crm_opt_exists(opt_name):
 
 
 def crm_res_running(opt_name):
-    (_, output) = commands.getstatusoutput("crm resource status %s" % opt_name)
+    (_, output) = subprocess.getstatusoutput(
+        "crm resource status %s" % opt_name)
     if output.startswith("resource %s is running" % opt_name):
         return True
 
@@ -104,7 +104,7 @@ def crm_res_running(opt_name):
 
 def list_nodes():
     cmd = ['crm', 'node', 'list']
-    out = subprocess.check_output(cmd)
+    out = subprocess.check_output(cmd).decode('utf-8')
     nodes = []
     for line in str(out).split('\n'):
         if line != '':
@@ -185,17 +185,18 @@ def get_property(name):
     # crmsh >= 2.3 renamed show-property to get-property, 2.3.x is
     # available since zesty
     if crm_version() >= StrictVersion('2.3.0'):
-        output = subprocess.check_output(['crm', 'configure',
+        output = subprocess.check_output(
-                                          'get-property', name],
+            ['crm', 'configure', 'get-property', name],
             universal_newlines=True)
     elif crm_version() < StrictVersion('2.2.0'):
         # before 2.2.0 there is no method to get a property
         output = subprocess.check_output(['crm', 'configure', 'show', 'xml'],
                                          universal_newlines=True)
+
         return get_property_from_xml(name, output)
     else:
-        output = subprocess.check_output(['crm', 'configure',
+        output = subprocess .check_output(
-                                          'show-property', name],
+            ['crm', 'configure', 'show-property', name],
             universal_newlines=True)
 
     return output
@@ -207,7 +208,7 @@ def set_property(name, value):
     :param name: property name
     :param value: new value
     """
-    subprocess.check_output(['crm', 'configure',
+    subprocess.check_call(['crm', 'configure',
                            'property', '%s=%s' % (name, value)],
                           universal_newlines=True)
 
@@ -218,11 +219,8 @@ def crm_version():
     """
     ver = subprocess.check_output(['crm', '--version'],
                                   universal_newlines=True)
-
     r = re.compile(r'.*(\d\.\d\.\d).*')
-
     matched = r.match(ver)
-
     if not matched:
         raise ValueError('error parsin crm version: %s' % ver)
     else:
@@ -245,12 +243,12 @@ def crm_update_resource(res_name, res_type, res_params=None, force=False):
         return 0
 
     with tempfile.NamedTemporaryFile() as f:
-        f.write('primitive {} {}'.format(res_name, res_type))
+        f.write('primitive {} {}'.format(res_name, res_type).encode('ascii'))
 
         if res_params:
-            f.write(' \\\n\t{}'.format(res_params))
+            f.write(' \\\n\t{}'.format(res_params).encode('ascii'))
         else:
-            f.write('\n')
+            f.write('\n'.encode('ascii'))
 
         f.flush()
         f.seek(0)
@@ -281,6 +279,7 @@ def resource_checksum(res_name, res_type, res_params=None):
     """
 
     m = hashlib.md5()
-    m.update(res_type)
+    m.update(res_type.encode('utf-8'))
-    m.update(res_params)
+    if res_params is not None:
+        m.update(res_params.encode('utf-8'))
     return m.hexdigest()

hooks/upgrade-charm

@@ -1 +0,0 @@
-hooks.py

hooks/upgrade-charm

@@ -0,0 +1,7 @@
+#!/bin/bash
+# Wrapper to ensure that old python bytecode isn't hanging around
+# after we upgrade the charm with newer libraries
+rm -rf **/*.pyc
+
+./hooks/install_deps
+exec ./hooks/upgrade-charm.real

hooks/upgrade-charm.real

@@ -0,0 +1 @@
+hooks.py

hooks/utils.py

@@ -124,7 +124,7 @@ class MAASConfigIncomplete(Exception):
 
 def disable_upstart_services(*services):
     for service in services:
-        with open("/etc/init/{}.override".format(service), "w") as override:
+        with open("/etc/init/{}.override".format(service), "wt") as override:
             override.write("manual")
 
 
@@ -214,7 +214,7 @@ def get_corosync_id(unit_name):
 
 def nulls(data):
     """Returns keys of values that are null (but not bool)"""
-    return [k for k in data.iterkeys()
+    return [k for k in data.keys()
             if not isinstance(data[k], bool) and not data[k]]
 
 
@@ -295,7 +295,7 @@ def get_corosync_conf():
 
             return conf
 
-    missing = [k for k, v in conf.iteritems() if v is None]
+    missing = [k for k, v in conf.items() if v is None]
     log('Missing required configuration: %s' % missing)
     return None
 
@@ -404,12 +404,12 @@ def get_ipv6_addr():
     for rid in relation_ids('ha'):
         for unit in related_units(rid):
             resources = parse_data(rid, unit, 'resources')
-            for res in resources.itervalues():
+            for res in resources.values():
                 if 'ocf:heartbeat:IPv6addr' in res:
                     res_params = parse_data(rid, unit, 'resource_params')
                     res_p = res_params.get(res)
                     if res_p:
-                        for k, v in res_p.itervalues():
+                        for k, v in res_p.values():
                             if utils.is_ipv6(v):
                                 log("Excluding '%s' from address list" % v,
                                     level=DEBUG)
@@ -766,7 +766,9 @@ def is_in_standby_mode(node_name):
     @param node_name: The name of the node to check
     @returns boolean - True if node_name is in standby mode
     """
-    out = subprocess.check_output(['crm', 'node', 'status', node_name])
+    out = (subprocess
+           .check_output(['crm', 'node', 'status', node_name])
+           .decode('utf-8'))
     root = ET.fromstring(out)
 
     standby_mode = False
@@ -807,7 +809,7 @@ def node_has_resources(node_name):
     @param node_name: The name of the node to check
     @returns boolean - True if node_name has resources
     """
-    out = subprocess.check_output(['crm_mon', '-X'])
+    out = subprocess.check_output(['crm_mon', '-X']).decode('utf-8')
     root = ET.fromstring(out)
     has_resources = False
     for resource in root.iter('resource'):
@@ -936,7 +938,7 @@ def ocf_file_exists(res_name, resources,
     @return: boolean - True if the ocf resource exists
     """
     res_type = None
-    for key, val in resources.iteritems():
+    for key, val in resources.items():
         if res_name == key:
             if len(val.split(':')) > 2:
                 res_type = val.split(':')[1]
@@ -955,7 +957,7 @@ def kill_legacy_ocf_daemon_process(res_name):
     ocf_name = res_name.replace('res_', '').replace('_', '-')
     reg_expr = '([0-9]+)\s+[^0-9]+{}'.format(ocf_name)
     cmd = ['ps', '-eo', 'pid,cmd']
-    ps = subprocess.check_output(cmd)
+    ps = subprocess.check_output(cmd).decode('utf-8')
     res = re.search(reg_expr, ps, re.MULTILINE)
     if res:
         pid = res.group(1)

ocf/maas/maas_dns.py

@@ -1,4 +1,4 @@
-#!/usr/bin/python3
+#!/usr/bin/env python3
 #
 # Copyright 2016 Canonical Ltd
 #

templates/corosync.conf

@@ -69,7 +69,7 @@ quorum {
 
 {% if transport == "udpu" %}
 nodelist {
-{% for nodeid, ip in ha_nodes.iteritems() %}
+{% for nodeid, ip in ha_nodes.items() %}
 	node {
 		ring0_addr: {{ ip }}
 		nodeid: {{ nodeid }}

tox.ini

@@ -2,7 +2,7 @@
 # This file is managed centrally by release-tools and should not be modified
 # within individual charm repos.
 [tox]
-envlist = pep8,py27
+envlist = pep8,py3{5,6}
 skipsdist = True
 
 [testenv]
@@ -16,11 +16,6 @@ commands = stestr run {posargs}
 whitelist_externals = juju
 passenv = HOME TERM AMULET_* CS_API_*
 
-[testenv:py27]
-basepython = python2.7
-deps = -r{toxinidir}/requirements.txt
-       -r{toxinidir}/test-requirements.txt
-
 [testenv:py35]
 basepython = python3.5
 deps = -r{toxinidir}/requirements.txt

unit_tests/__init__.py

@@ -12,5 +12,21 @@
 # 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__))
+_actions = os.path.abspath(os.path.join(_path, '../actions'))
+_hooks = os.path.abspath(os.path.join(_path, '../hooks'))
+_charmhelpers = os.path.abspath(os.path.join(_path, '../charmhelpers'))
+_unit_tests = os.path.abspath(os.path.join(_path, '../unit_tests'))
+
+
+def _add_path(path):
+    if path not in sys.path:
+        sys.path.insert(1, path)
+
+_add_path(_actions)
+_add_path(_hooks)
+_add_path(_charmhelpers)
+_add_path(_unit_tests)

unit_tests/test_hacluster_utils.py

@@ -26,7 +26,7 @@ import pcmk
 
 
 def write_file(path, content, *args, **kwargs):
-    with open(path, 'w') as f:
+    with open(path, 'wt') as f:
         f.write(content)
         f.flush()
 
@@ -74,7 +74,7 @@ class UtilsTestCaseWriteTmp(unittest.TestCase):
 
         self.assertTrue(utils.emit_corosync_conf())
 
-        with open(utils.COROSYNC_CONF) as fd:
+        with open(utils.COROSYNC_CONF, 'rt') as fd:
             content = fd.read()
             if enabled:
                 pattern = 'debug: on\n'
@@ -216,7 +216,7 @@ class UtilsTestCase(unittest.TestCase):
     @mock.patch.object(subprocess, 'call')
     def test_kill_legacy_ocf_daemon_process(self, call_mock,
                                             check_output_mock):
-        ps_output = '''
+        ps_output = b'''
           PID CMD
           6863 sshd: ubuntu@pts/7
           11109 /usr/bin/python /usr/bin/ceilometer-agent-central --config

unit_tests/test_pcmk.py

@@ -82,25 +82,25 @@ class TestPcmk(unittest.TestCase):
     def tearDown(self):
         os.remove(self.tmpfile.name)
 
-    @mock.patch('commands.getstatusoutput')
+    @mock.patch('subprocess.getstatusoutput')
     def test_crm_res_running_true(self, getstatusoutput):
         getstatusoutput.return_value = (0, ("resource res_nova_consoleauth is "
                                             "running on: juju-xxx-machine-6"))
         self.assertTrue(pcmk.crm_res_running('res_nova_consoleauth'))
 
-    @mock.patch('commands.getstatusoutput')
+    @mock.patch('subprocess.getstatusoutput')
     def test_crm_res_running_stopped(self, getstatusoutput):
         getstatusoutput.return_value = (0, ("resource res_nova_consoleauth is "
                                             "NOT running"))
         self.assertFalse(pcmk.crm_res_running('res_nova_consoleauth'))
 
-    @mock.patch('commands.getstatusoutput')
+    @mock.patch('subprocess.getstatusoutput')
     def test_crm_res_running_undefined(self, getstatusoutput):
         getstatusoutput.return_value = (1, "foobar")
         self.assertFalse(pcmk.crm_res_running('res_nova_consoleauth'))
 
     @mock.patch('socket.gethostname')
-    @mock.patch('commands.getstatusoutput')
+    @mock.patch('subprocess.getstatusoutput')
     def test_wait_for_pcmk(self, getstatusoutput, gethostname):
         # Pacemaker is down
         gethostname.return_value = 'hanode-1'
@@ -124,8 +124,8 @@ class TestPcmk(unittest.TestCase):
 
         # trusty
         mock_check_output.mock_reset()
-        mock_check_output.return_value = ("1.2.5 (Build f2f315daf6a5fd7ddea8e5"
+        mock_check_output.return_value = (
-                                          "64cd289aa04218427d)\n")
+            "1.2.5 (Build f2f315daf6a5fd7ddea8e564cd289aa04218427d)\n")
         ret = pcmk.crm_version()
         self.assertEqual(StrictVersion('1.2.5'), ret)
         mock_check_output.assert_called_with(['crm', '--version'],
@@ -170,7 +170,7 @@ class TestPcmk(unittest.TestCase):
                                               'show', 'xml'],
                                              universal_newlines=True)
 
-    @mock.patch('subprocess.check_output')
+    @mock.patch('subprocess.check_call')
     def test_set_property(self, mock_check_output):
         pcmk.set_property('maintenance-mode', 'false')
         mock_check_output.assert_called_with(['crm', 'configure', 'property',
@@ -189,7 +189,7 @@ class TestPcmk(unittest.TestCase):
 
         mock_call.assert_any_call(['crm', 'configure', 'load',
                                    'update', self.tmpfile.name])
-        with open(self.tmpfile.name, 'r') as f:
+        with open(self.tmpfile.name, 'rt') as f:
             self.assertEqual(f.read(),
                              ('primitive res_test IPaddr2 \\\n'
                               '\tparams ip=1.2.3.4 cidr_netmask=255.255.0.0'))

unit_tests/test_utils.py

@@ -51,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:
@@ -148,5 +148,5 @@ def patch_open():
         mock_open(*args, **kwargs)
         yield mock_file
 
-    with patch('__builtin__.open', stub_open):
+    with patch('builtins.open', stub_open):
         yield mock_open, mock_file