Switch charm runtime to python3

Update charm to execute hooks and actions under Python 3; this
includes dealing with upgrades (by switching the upgrade-charm
hook to be a bash script which installs the required charm runtime
dependencies).

This commit also drops code from ceph.py which was used across
other ceph charms in the past; only the functions required for this
charm have been retained.

Change-Id: I5e222d907bfa34ffacad16c51abd1278d7d82f56

.zuul.yaml

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

actions/actions.py

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

hooks/ceph.py

@@ -13,13 +13,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import json
 import os
 import subprocess
-import time
-
-
-from socket import gethostname as get_unit_hostname
 
 from utils import get_pkg_version
 
@@ -30,151 +25,6 @@ from charmhelpers.contrib.storage.linux.ceph import (
     CephBrokerRq,
 )
 
-LEADER = 'leader'
-PEON = 'peon'
-QUORUM = [LEADER, PEON]
-
-
-def is_quorum():
-    asok = "/var/run/ceph/ceph-mon.{}.asok".format(get_unit_hostname())
-    cmd = [
-        "ceph",
-        "--admin-daemon",
-        asok,
-        "mon_status"
-    ]
-    if os.path.exists(asok):
-        try:
-            result = json.loads(subprocess.check_output(cmd))
-        except subprocess.CalledProcessError:
-            return False
-        except ValueError:
-            # Non JSON response from mon_status
-            return False
-        if result['state'] in QUORUM:
-            return True
-        else:
-            return False
-    else:
-        return False
-
-
-def is_leader():
-    asok = "/var/run/ceph/ceph-mon.{}.asok".format(get_unit_hostname())
-    cmd = [
-        "ceph",
-        "--admin-daemon",
-        asok,
-        "mon_status"
-    ]
-    if os.path.exists(asok):
-        try:
-            result = json.loads(subprocess.check_output(cmd))
-        except subprocess.CalledProcessError:
-            return False
-        except ValueError:
-            # Non JSON response from mon_status
-            return False
-        if result['state'] == LEADER:
-            return True
-        else:
-            return False
-    else:
-        return False
-
-
-def wait_for_quorum():
-    while not is_quorum():
-        time.sleep(3)
-
-
-def add_bootstrap_hint(peer):
-    asok = "/var/run/ceph/ceph-mon.{}.asok".format(get_unit_hostname())
-    cmd = [
-        "ceph",
-        "--admin-daemon",
-        asok,
-        "add_bootstrap_peer_hint",
-        peer
-    ]
-    if os.path.exists(asok):
-        # Ignore any errors for this call
-        subprocess.call(cmd)
-
-
-DISK_FORMATS = [
-    'xfs',
-    'ext4',
-    'btrfs'
-]
-
-
-def is_osd_disk(dev):
-    try:
-        info = subprocess.check_output(['sgdisk', '-i', '1', dev])
-        info = info.split("\n")  # IGNORE:E1103
-        for line in info:
-            if line.startswith(
-                    'Partition GUID code: 4FBD7E29-9D25-41B8-AFD0-062C0CEFF05D'
-            ):
-                return True
-    except subprocess.CalledProcessError:
-        pass
-    return False
-
-
-def rescan_osd_devices():
-    cmd = [
-        'udevadm', 'trigger',
-        '--subsystem-match=block', '--action=add'
-    ]
-
-    subprocess.call(cmd)
-
-
-def zap_disk(dev):
-    cmd = ['sgdisk', '--zap-all', dev]
-    subprocess.check_call(cmd)
-
-
-_bootstrap_keyring = "/var/lib/ceph/bootstrap-osd/ceph.keyring"
-
-
-def is_bootstrapped():
-    return os.path.exists(_bootstrap_keyring)
-
-
-def wait_for_bootstrap():
-    while (not is_bootstrapped()):
-        time.sleep(3)
-
-
-def import_osd_bootstrap_key(key):
-    if not os.path.exists(_bootstrap_keyring):
-        cmd = [
-            'ceph-authtool',
-            _bootstrap_keyring,
-            '--create-keyring',
-            '--name=client.bootstrap-osd',
-            '--add-key={}'.format(key)
-        ]
-        subprocess.check_call(cmd)
-
-# OSD caps taken from ceph-create-keys
-_osd_bootstrap_caps = {
-    'mon': [
-        'allow command osd create ...',
-        'allow command osd crush set ...',
-        r'allow command auth add * osd allow\ * mon allow\ rwx',
-        'allow command mon getmap'
-    ]
-}
-
-
-def get_osd_bootstrap_key():
-    return get_named_key('bootstrap-osd', _osd_bootstrap_caps)
-
-
 _radosgw_keyring = "/etc/ceph/keyring.rados.gateway"
 
 
@@ -189,54 +39,6 @@ def import_radosgw_key(key):
         ]
         subprocess.check_call(cmd)
 
-# OSD caps taken from ceph-create-keys
-_radosgw_caps = {
-    'mon': ['allow r'],
-    'osd': ['allow rwx']
-}
-
-
-def get_radosgw_key():
-    return get_named_key('radosgw.gateway', _radosgw_caps)
-
-
-_default_caps = {
-    'mon': ['allow r'],
-    'osd': ['allow rwx']
-}
-
-
-def get_named_key(name, caps=None):
-    caps = caps or _default_caps
-    cmd = [
-        'ceph',
-        '--name', 'mon.',
-        '--keyring',
-        '/var/lib/ceph/mon/ceph-{}/keyring'.format(
-            get_unit_hostname()
-        ),
-        'auth', 'get-or-create', 'client.{}'.format(name),
-    ]
-    # Add capabilities
-    for subsystem, subcaps in caps.iteritems():
-        cmd.extend([
-            subsystem,
-            '; '.join(subcaps),
-        ])
-    output = subprocess.check_output(cmd).strip()  # IGNORE:E1103
-    # get-or-create appears to have different output depending
-    # on whether its 'get' or 'create'
-    # 'create' just returns the key, 'get' is more verbose and
-    # needs parsing
-    key = None
-    if len(output.splitlines()) == 1:
-        key = output
-    else:
-        for element in output.splitlines():
-            if 'key' in element:
-                key = element.split(' = ')[1].strip()  # IGNORE:E1103
-    return key
-
 
 def get_create_rgw_pools_rq(prefix=None):
     """Pre-create RGW pools so that they have the correct settings.

hooks/hooks.py

@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python3
 #
 # Copyright 2016 Canonical Ltd
 #
@@ -136,8 +136,7 @@ def install():
         os.makedirs('/etc/ceph')
 
 
-@hooks.hook('upgrade-charm',
-            'config-changed')
+@hooks.hook('config-changed')
 @restart_on_change({'/etc/ceph/ceph.conf': ['radosgw'],
                     '/etc/haproxy/haproxy.cfg': ['haproxy']})
 @harden()

hooks/install

@@ -2,19 +2,6 @@
 # Wrapper to deal with newer Ubuntu versions that don't have py2 installed
 # by default.
 
-declare -a DEPS=('apt' 'netaddr' 'netifaces' 'pip' 'yaml' 'jinja2' '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,17 @@
+#!/bin/bash
+# Install required dependencies for charm runtime
+
+declare -a DEPS=('apt' 'netaddr' 'netifaces' 'yaml' 'jinja2' '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/upgrade-charm

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

hooks/upgrade-charm

@@ -0,0 +1,4 @@
+#!/bin/bash
+# Re-install dependencies to deal with py2->py3 switch for charm
+
+./hooks/install_deps

hooks/utils.py

@@ -172,7 +172,7 @@ def register_configs(release='icehouse'):
         CONFIGS[CEPH_CONF]['contexts'].append(
             ceph_radosgw_context.IdentityServiceContext()
         )
-    for cfg, rscs in CONFIGS.iteritems():
+    for cfg, rscs in CONFIGS.items():
         configs.register(cfg, rscs['contexts'])
     return configs
 

tests/basic_deployment.py

@@ -427,7 +427,7 @@ class CephRadosGwBasicDeployment(OpenStackAmuletDeployment):
             expected['client.radosgw.gateway']['rgw keystone admin token'] = (
                 'ubuntutesting')
 
-        for section, pairs in expected.iteritems():
+        for section, pairs in expected.items():
             ret = u.validate_config_data(unit, conf, section, pairs)
             if ret:
                 message = "ceph config error: {}".format(ret)
@@ -444,7 +444,7 @@ class CephRadosGwBasicDeployment(OpenStackAmuletDeployment):
                 'volume_driver': 'cinder.volume.drivers.rbd.RBDDriver'
             }
         }
-        for section, pairs in expected.iteritems():
+        for section, pairs in expected.items():
             ret = u.validate_config_data(unit, conf, section, pairs)
             if ret:
                 message = "cinder (rbd) config error: {}".format(ret)
@@ -474,7 +474,7 @@ class CephRadosGwBasicDeployment(OpenStackAmuletDeployment):
             section = 'DEFAULT'
 
         expected = {section: config}
-        for section, pairs in expected.iteritems():
+        for section, pairs in expected.items():
             ret = u.validate_config_data(unit, conf, section, pairs)
             if ret:
                 message = "glance (rbd) config error: {}".format(ret)
@@ -491,7 +491,7 @@ class CephRadosGwBasicDeployment(OpenStackAmuletDeployment):
                 'rbd_secret_uuid': u.not_null
             }
         }
-        for section, pairs in expected.iteritems():
+        for section, pairs in expected.items():
             ret = u.validate_config_data(unit, conf, section, pairs)
             if ret:
                 message = "nova (rbd) config error: {}".format(ret)

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,py35,py36
 skipsdist = True
 
 [testenv]
@@ -12,7 +12,7 @@ setenv = VIRTUAL_ENV={envdir}
          AMULET_SETUP_TIMEOUT=5400
 install_command =
   pip install {opts} {packages}
-commands = stestr run --slowest {posargs} 
+commands = stestr run --slowest {posargs}
 whitelist_externals = juju
 passenv = HOME TERM AMULET_* CS_API_*
 

unit_tests/__init__.py

@@ -14,5 +14,7 @@
 
 import sys
 
-sys.path.append('actions/')
-sys.path.append('hooks/')
+sys.path.append('actions')
+sys.path.append('hooks')
+sys.path.append('lib')
+sys.path.append('unit_tests')

unit_tests/test_ceph.py

@@ -30,10 +30,8 @@ from test_utils import CharmTestCase  # noqa
 
 TO_PATCH = [
     'config',
-    'get_unit_hostname',
     'os',
     'subprocess',
-    'time',
 ]
 
 
@@ -42,143 +40,6 @@ class CephRadosGWCephTests(CharmTestCase):
         super(CephRadosGWCephTests, self).setUp(ceph, TO_PATCH)
         self.config.side_effect = self.test_config.get
 
-    def test_is_quorum_leader(self):
-        self.os.path.exists.return_value = True
-        self.get_unit_hostname.return_value = 'myhost'
-        self.subprocess.check_output.return_value = '{"state": "leader"}'
-        self.assertEqual(ceph.is_quorum(), True)
-
-    def test_is_quorum_notleader(self):
-        self.os.path.exists.return_value = True
-        self.get_unit_hostname.return_value = 'myhost'
-        self.subprocess.check_output.return_value = '{"state": "notleader"}'
-        self.assertEqual(ceph.is_quorum(), False)
-
-    def test_is_quorum_valerror(self):
-        self.os.path.exists.return_value = True
-        self.get_unit_hostname.return_value = 'myhost'
-        self.subprocess.check_output.return_value = "'state': 'bob'}"
-        self.assertEqual(ceph.is_quorum(), False)
-
-    def test_is_quorum_no_asok(self):
-        self.os.path.exists.return_value = False
-        self.assertEqual(ceph.is_quorum(), False)
-
-    def test_is_leader(self):
-        self.get_unit_hostname.return_value = 'myhost'
-        self.os.path.exists.return_value = True
-        self.subprocess.check_output.return_value = '{"state": "leader"}'
-        self.assertEqual(ceph.is_leader(), True)
-
-    def test_is_leader_notleader(self):
-        self.get_unit_hostname.return_value = 'myhost'
-        self.os.path.exists.return_value = True
-        self.subprocess.check_output.return_value = '{"state": "notleader"}'
-        self.assertEqual(ceph.is_leader(), False)
-
-    def test_is_leader_valerror(self):
-        self.get_unit_hostname.return_value = 'myhost'
-        self.os.path.exists.return_value = True
-        self.subprocess.check_output.return_value = "'state': 'bob'}"
-        self.assertEqual(ceph.is_leader(), False)
-
-    def test_is_leader_noasok(self):
-        self.get_unit_hostname.return_value = 'myhost'
-        self.os.path.exists.return_value = False
-        self.assertEqual(ceph.is_leader(), False)
-
-    def test_wait_for_quorum_yes(self):
-        results = [True, False]
-
-        def quorum():
-            return results.pop()
-
-        _is_quorum = self.patch('is_quorum')
-        _is_quorum.side_effect = quorum
-        ceph.wait_for_quorum()
-        self.time.sleep.assert_called_with(3)
-
-    def test_wait_for_quorum_no(self):
-        _is_quorum = self.patch('is_quorum')
-        _is_quorum.return_value = True
-        ceph.wait_for_quorum()
-        self.assertFalse(self.time.sleep.called)
-
-    def test_wait_for_bootstrap(self):
-        results = [True, False]
-
-        def bootstrapped():
-            return results.pop()
-
-        _is_bootstrapped = self.patch('is_bootstrapped')
-        _is_bootstrapped.side_effect = bootstrapped
-        ceph.wait_for_bootstrap()
-        self.time.sleep.assert_called_with(3)
-
-    def test_add_bootstrap_hint(self):
-        self.get_unit_hostname.return_value = 'myhost'
-        cmd = [
-            "ceph",
-            "--admin-daemon",
-            '/var/run/ceph/ceph-mon.myhost.asok',
-            "add_bootstrap_peer_hint",
-            'mypeer'
-        ]
-        self.os.path.exists.return_value = True
-        ceph.add_bootstrap_hint('mypeer')
-        self.subprocess.call.assert_called_with(cmd)
-
-    def test_add_bootstrap_hint_noasok(self):
-        self.get_unit_hostname.return_value = 'myhost'
-        self.os.path.exists.return_value = False
-        ceph.add_bootstrap_hint('mypeer')
-        self.assertFalse(self.subprocess.call.called)
-
-    def test_is_osd_disk(self):
-        # XXX Insert real sgdisk output
-        self.subprocess.check_output.return_value = \
-            'Partition GUID code: 4FBD7E29-9D25-41B8-AFD0-062C0CEFF05D'
-        self.assertEqual(ceph.is_osd_disk('/dev/fmd0'), True)
-
-    def test_is_osd_disk_no(self):
-        # XXX Insert real sgdisk output
-        self.subprocess.check_output.return_value = \
-            'Partition GUID code: 5FBD7E29-9D25-41B8-AFD0-062C0CEFF05D'
-        self.assertEqual(ceph.is_osd_disk('/dev/fmd0'), False)
-
-    def test_rescan_osd_devices(self):
-        cmd = [
-            'udevadm', 'trigger',
-            '--subsystem-match=block', '--action=add'
-        ]
-        ceph.rescan_osd_devices()
-        self.subprocess.call.assert_called_with(cmd)
-
-    def test_zap_disk(self):
-        cmd = [
-            'sgdisk', '--zap-all', '/dev/fmd0',
-        ]
-        ceph.zap_disk('/dev/fmd0')
-        self.subprocess.check_call.assert_called_with(cmd)
-
-    def test_import_osd_bootstrap_key(self):
-        self.os.path.exists.return_value = False
-        cmd = [
-            'ceph-authtool',
-            '/var/lib/ceph/bootstrap-osd/ceph.keyring',
-            '--create-keyring',
-            '--name=client.bootstrap-osd',
-            '--add-key=mykey',
-        ]
-        ceph.import_osd_bootstrap_key('mykey')
-        self.subprocess.check_call.assert_called_with(cmd)
-
-    def test_is_bootstrapped(self):
-        self.os.path.exists.return_value = True
-        self.assertEqual(ceph.is_bootstrapped(), True)
-        self.os.path.exists.return_value = False
-        self.assertEqual(ceph.is_bootstrapped(), False)
-
     def test_import_radosgw_key(self):
         self.os.path.exists.return_value = False
         ceph.import_radosgw_key('mykey')
@@ -191,40 +52,6 @@ class CephRadosGWCephTests(CharmTestCase):
         ]
         self.subprocess.check_call.assert_called_with(cmd)
 
-    def test_get_named_key_create(self):
-        self.get_unit_hostname.return_value = "myhost"
-        self.subprocess.check_output.return_value = """
-
-[client.dummy]
-    key = AQAPiu1RCMb4CxAAmP7rrufwZPRqy8bpQa2OeQ==
-"""
-        self.assertEqual(ceph.get_named_key('dummy'),
-                         'AQAPiu1RCMb4CxAAmP7rrufwZPRqy8bpQa2OeQ==')
-        cmd = [
-            'ceph',
-            '--name', 'mon.',
-            '--keyring',
-            '/var/lib/ceph/mon/ceph-myhost/keyring',
-            'auth', 'get-or-create', 'client.dummy',
-            'mon', 'allow r', 'osd', 'allow rwx'
-        ]
-        self.subprocess.check_output.assert_called_with(cmd)
-
-    def test_get_named_key_get(self):
-        self.get_unit_hostname.return_value = "myhost"
-        key = "AQAPiu1RCMb4CxAAmP7rrufwZPRqy8bpQa2OeQ=="
-        self.subprocess.check_output.return_value = key
-        self.assertEqual(ceph.get_named_key('dummy'), key)
-        cmd = [
-            'ceph',
-            '--name', 'mon.',
-            '--keyring',
-            '/var/lib/ceph/mon/ceph-myhost/keyring',
-            'auth', 'get-or-create', 'client.dummy',
-            'mon', 'allow r', 'osd', 'allow rwx'
-        ]
-        self.subprocess.check_output.assert_called_with(cmd)
-
     @patch('charmhelpers.contrib.storage.linux.ceph.CephBrokerRq'
            '.add_op_create_pool')
     def test_create_rgw_pools_rq_with_prefix(self, mock_broker):

unit_tests/test_ceph_radosgw_context.py

@@ -38,6 +38,7 @@ class HAProxyContextTests(CharmTestCase):
         super(HAProxyContextTests, self).setUp(context, TO_PATCH)
         self.relation_get.side_effect = self.test_relation.get
         self.config.side_effect = self.test_config.get
+        self.cmp_pkgrevno.return_value = 1
 
     @patch('charmhelpers.contrib.openstack.context.get_relation_ip')
     @patch('charmhelpers.contrib.openstack.context.mkdir')
@@ -71,6 +72,7 @@ class IdentityServiceContextTest(CharmTestCase):
         self.relation_get.side_effect = self.test_relation.get
         self.config.side_effect = self.test_config.get
         self.maxDiff = None
+        self.cmp_pkgrevno.return_value = 1
 
     @patch.object(charmhelpers.contrib.openstack.context, 'format_ipv6_addr')
     @patch.object(charmhelpers.contrib.openstack.context, 'context_complete')
@@ -295,6 +297,7 @@ class MonContextTest(CharmTestCase):
         super(MonContextTest, self).setUp(context, TO_PATCH)
         self.config.side_effect = self.test_config.get
         self.unit_public_ip.return_value = '10.255.255.255'
+        self.cmp_pkgrevno.return_value = 1
 
     @patch.object(ceph, 'config', lambda *args:
                   '{"client.radosgw.gateway": {"rgw init timeout": 60}}')

unit_tests/test_hooks.py

@@ -67,6 +67,7 @@ class CephRadosGWTests(CharmTestCase):
         self.test_config.set('source', 'distro')
         self.test_config.set('key', 'secretkey')
         self.test_config.set('use-syslog', False)
+        self.cmp_pkgrevno.return_value = 0
 
     def test_install_packages(self):
         ceph_hooks.install_packages()

unit_tests/test_utils.py

@@ -49,7 +49,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: