Add functional & unit tests

.gitignore

@@ -1,8 +1,8 @@
-build/
-.local/
-.testrepository/
-.tox/
-func-results.json
-test-charm/
+build
+.tox
+layers
 interfaces
+trusty
+.testrepository
 __pycache__
+.stestr

.stestr.conf

@@ -0,0 +1,3 @@
+[DEFAULT]
+test_path=./unit_tests
+top_dir=./

src/config.yaml

@@ -4,3 +4,8 @@ options:
     default: False
     description: |
         Whether to request a stoniith resoiurce is created for this nodee
+  enable-resources:
+    type: boolean
+    default: True
+    description: |
+        Whether to host resources from the main cluster

src/metadata.yaml

@@ -9,8 +9,9 @@ tags:
 series:
   - bionic
 requires:
-  pacemaker-remote:
-    interface: pacemaker-remote
   juju-info:
     interface: juju-info
     scope: container
+provides:
+  pacemaker-remote:
+    interface: pacemaker-remote

src/reactive/pacemaker_remote_handlers.py

@@ -22,13 +22,14 @@ import charmhelpers.core.host as ch_host
 
 import charmhelpers.contrib.network.ip
 
-COROSYNC_DIR = '/etc/corosync/'
+COROSYNC_DIR = '/etc/corosync'
 SERVICES = ['pacemaker_remote', 'pcsd']
 PACKAGES = ['pacemaker-remote', 'pcs', 'resource-agents', 'corosync']
 PACEMAKER_KEY = '/etc/pacemaker/authkey'
 
 
 def install_packages():
+    """Install apckages neede dby charm"""
     hookenv.status_set('maintenance', 'Installing packages')
     fetch.apt_install(PACKAGES, fatal=True)
     hookenv.status_set('maintenance',
@@ -36,6 +37,7 @@ def install_packages():
 
 
 def wipe_corosync_state():
+    """Remove state left by corosync package"""
     try:
         shutil.rmtree('{}/uidgid.d'.format(COROSYNC_DIR))
     except FileNotFoundError:
@@ -47,12 +49,14 @@ def wipe_corosync_state():
 
 
 def restart_services():
+    """Restart pacemker_remote and associated services"""
     for svc in SERVICES:
         ch_host.service_restart(svc)
 
 
 @reactive.when_not('pacemaker-remote.installed')
 def install():
+    """Perform initial setup of pacmekaer-remote"""
     install_packages()
     wipe_corosync_state()
     reactive.set_flag('pacemaker-remote.installed')
@@ -60,16 +64,24 @@ def install():
 
 @reactive.when('endpoint.pacemaker-remote.joined')
 def publish_stonith_info():
-    if hookenv.config(''):
-        remote = reactive.endpoint_from_flag(
-            'endpoint.pacemaker-remote.joined')
-        remote.publish_info(
-            stonith_hostname=charmhelpers.contrib.network.ip.get_hostname(
-                hookenv.unit_get('private-address')))
+    """Provide remote hacluster with info for including remote in cluster"""
+    remote_hostname = charmhelpers.contrib.network.ip.get_hostname(
+        hookenv.unit_get('private-address'))
+    if hookenv.config('enable-stonith'):
+        stonith_hostname = remote_hostname
+    else:
+        stonith_hostname = None
+    remote = reactive.endpoint_from_flag(
+        'endpoint.pacemaker-remote.joined')
+    remote.publish_info(
+        remote_hostname=remote_hostname,
+        enable_resources=hookenv.config('enable-resources'),
+        stonith_hostname=stonith_hostname)
 
 
 @reactive.when('endpoint.pacemaker-remote.changed.pacemaker-key')
 def write_pacemaker_key():
+    """Finish setup of pacemaker-remote"""
     remote = reactive.endpoint_from_flag('endpoint.pacemaker-remote.changed')
     key = remote.get_pacemaker_key()
     if key:
@@ -80,3 +92,4 @@ def write_pacemaker_key():
             group='haclient',
             perms=0o444)
         restart_services()
+        hookenv.status_set('active', 'Unit is ready')

src/tests/bundles/basic.yaml

@@ -0,0 +1,27 @@
+series: bionic
+relations:
+- - compute:juju-info
+  - pacemaker-remote:juju-info
+- - api:juju-info
+  - hacluster:juju-info
+- - hacluster:pacemaker-remote
+  - pacemaker-remote:pacemaker-remote
+applications:
+  api:
+    charm: cs:bionic/ubuntu
+    num_units: 3
+  hacluster:
+    charm: cs:~gnuoy/hacluster-16
+    options:
+      corosync_transport: unicast
+      cluster_count: 3
+  compute:
+    charm: cs:bionic/ubuntu
+    num_units: 1
+  pacemaker-remote:
+    series: bionic
+    charm: pacemaker-remote
+    options:
+      enable-stonith: False
+      enable-resources: False
+

src/tests/tests.yaml

@@ -0,0 +1,10 @@
+charm_name: pacemaker-remote
+tests:
+  - tests.tests_pacemaker_remote.PacemakerRemoteTest
+configure:
+  - zaza.charm_tests.noop.setup.basic_setup
+gate_bundles:
+  - basic
+smoke_bundles:
+  - basic
+

src/tests/tests_pacemaker_remote.py

@@ -0,0 +1,41 @@
+#!/usr/bin/env python3
+
+# Copyright 2019 Canonical Ltd.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+
+"""Encapsulate pacemaker-remote testing."""
+
+import logging
+import unittest
+import xml.etree.ElementTree as ET
+
+import zaza.model
+import zaza.charm_tests.test_utils as test_utils
+
+
+class PacemakerRemoteTest(unittest.TestCase):
+
+    """Encapsulate pacemaker-remote tests."""
+
+    def test_check_nodes_online(self):
+        status_cmd = 'crm status --as-xml'
+        status_xml = zaza.model.run_on_leader('api', status_cmd)['Stdout']
+        root = ET.fromstring(status_xml)
+        for child in root:
+            if child.tag == 'nodes':
+                for node in child:
+                    logging.info(
+                        'Node {name} of type {type} is '
+                        '{online}'.format(**node.attrib))
+                    self.assertEqual(node.attrib['online'], "true")

tox.ini

@@ -3,20 +3,7 @@
 # within individual charm repos.
 [tox]
 skipsdist = True
-envlist = pep8,py34,py35,py27
-skip_missing_interpreters = True
-
-[bundleenv]
-setenv = VIRTUAL_ENV={envdir}
-         PYTHONHASHSEED=0
-         TERM=linux
-         LAYER_PATH={toxinidir}/layers
-         INTERFACE_PATH={toxinidir}/interfaces
-         JUJU_REPOSITORY={toxinidir}/build
-install_command =
-  pip install {opts} {packages}
-deps =
-    -r{toxinidir}/requirements.txt
+envlist = pep8,py3
 
 [testenv]
 setenv = VIRTUAL_ENV={envdir}
@@ -25,7 +12,7 @@ setenv = VIRTUAL_ENV={envdir}
          LAYER_PATH={toxinidir}/layers
          INTERFACE_PATH={toxinidir}/interfaces
          JUJU_REPOSITORY={toxinidir}/build
-passenv = http_proxy https_proxy CHARM_TEMPLATE_LOCAL_BRANCH CHARM_INTERFACES_DIR
+passenv = http_proxy https_proxy CHARM_INTERFACES_DIR
 install_command =
   pip install {opts} {packages}
 deps =
@@ -36,34 +23,54 @@ basepython = python2.7
 commands =
     charm-build --log-level DEBUG -o {toxinidir}/build src {posargs}
 
-[testenv:py34]
-basepython = python3.4
-deps = -r{toxinidir}/test-requirements.txt
-commands = ostestr {posargs}
+[testenv:py27]
+basepython = python2.7
+# Reactive source charms are Python3-only, but a py27 unit test target
+# is required by OpenStack Governance.  Remove this shim as soon as
+# permitted.  https://governance.openstack.org/tc/reference/cti/python_cti.html
+whitelist_externals = true
+commands = true
 
-[testenv:py35]
-basepython = python3.5
+[testenv:py3]
+basepython = python3
 deps = -r{toxinidir}/test-requirements.txt
-commands = ostestr {posargs}
+commands = stestr run {posargs}
 
 [testenv:pep8]
-basepython = python2.7
+basepython = python3
 deps = -r{toxinidir}/test-requirements.txt
 commands = flake8 {posargs} src unit_tests
 
-[testenv:test_create]
-# This tox target is used for template generation testing and can be removed
-# from a generated source charm or built charm
-basepython = python2.7
-deps = -r{toxinidir}/test-generate-requirements.txt
+[testenv:cover]
+# Technique based heavily upon
+# https://github.com/openstack/nova/blob/master/tox.ini
+basepython = python3
+deps = -r{toxinidir}/requirements.txt
+       -r{toxinidir}/test-requirements.txt
 setenv =
-    CHARM_TEMPLATE_ALT_REPO = {toxinidir}
+    {[testenv]setenv}
+    PYTHON=coverage run
 commands =
-    charm-create -t openstack-api -a congress test-charm
-    /bin/cp test-artifacts/congress.conf.sample {toxinidir}/test-charm/congress/src/templates/congress.conf
-#    charm-build --log-level DEBUG -o {toxinidir}/test-charm/congress/build {toxinidir}/test-charm/congress/src {posargs}
+    coverage erase
+    stestr run {posargs}
+    coverage combine
+    coverage html -d cover
+    coverage xml -o cover/coverage.xml
+    coverage report
+
+[coverage:run]
+branch = True
+concurrency = multiprocessing
+parallel = True
+source =
+    .
+omit =
+    .tox/*
+    */charmhelpers/*
+    unit_tests/*
 
 [testenv:venv]
+basepython = python3
 commands = {posargs}
 
 [flake8]

unit_tests/__init__.py

@@ -0,0 +1,22 @@
+# Copyright 2016 Canonical Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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 sys
+
+sys.path.append('src')
+sys.path.append('src/lib')
+
+# Mock out charmhelpers so that we can test without it.
+import charms_openstack.test_mocks  # noqa
+charms_openstack.test_mocks.mock_charmhelpers()

unit_tests/test_pacemaker_remote_handlers.py

@@ -0,0 +1,211 @@
+# Copyright 2019 Canonical Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#  http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# 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.
+
+
+from __future__ import absolute_import
+from __future__ import print_function
+
+import unittest
+
+import mock
+
+import reactive.pacemaker_remote_handlers as handlers
+
+
+_when_args = {}
+_when_not_args = {}
+
+
+def mock_hook_factory(d):
+
+    def mock_hook(*args, **kwargs):
+
+        def inner(f):
+            # remember what we were passed.  Note that we can't actually
+            # determine the class we're attached to, as the decorator only gets
+            # the function.
+            try:
+                d[f.__name__].append(dict(args=args, kwargs=kwargs))
+            except KeyError:
+                d[f.__name__] = [dict(args=args, kwargs=kwargs)]
+            return f
+        return inner
+    return mock_hook
+
+
+class TestPAcemakerRemoteHandlers(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        cls._patched_when = mock.patch('charms.reactive.when',
+                                       mock_hook_factory(_when_args))
+        cls._patched_when_started = cls._patched_when.start()
+        cls._patched_when_not = mock.patch('charms.reactive.when_not',
+                                           mock_hook_factory(_when_not_args))
+        cls._patched_when_not_started = cls._patched_when_not.start()
+        # force requires to rerun the mock_hook decorator:
+        # try except is Python2/Python3 compatibility as Python3 has moved
+        # reload to importlib.
+        try:
+            reload(handlers)
+        except NameError:
+            import importlib
+            importlib.reload(handlers)
+
+    @classmethod
+    def tearDownClass(cls):
+        cls._patched_when.stop()
+        cls._patched_when_started = None
+        cls._patched_when = None
+        cls._patched_when_not.stop()
+        cls._patched_when_not_started = None
+        cls._patched_when_not = None
+        # and fix any breakage we did to the module
+        try:
+            reload(handlers)
+        except NameError:
+            import importlib
+            importlib.reload(handlers)
+
+    def setUp(self):
+        self._patches = {}
+        self._patches_start = {}
+
+    def tearDown(self):
+        for k, v in self._patches.items():
+            v.stop()
+            setattr(self, k, None)
+        self._patches = None
+        self._patches_start = None
+
+    def patch(self, obj, attr, return_value=None):
+        mocked = mock.patch.object(obj, attr)
+        self._patches[attr] = mocked
+        started = mocked.start()
+        started.return_value = return_value
+        self._patches_start[attr] = started
+        setattr(self, attr, started)
+
+    def test_install_packages(self):
+        self.patch(handlers.hookenv, 'status_set')
+        self.patch(handlers.fetch, 'apt_install')
+        handlers.install_packages()
+        self.apt_install.assert_called_once_with(
+            ['pacemaker-remote', 'pcs', 'resource-agents', 'corosync'],
+            fatal=True)
+        status_calls = [
+            mock.call('maintenance', 'Installing packages'),
+            mock.call(
+                'maintenance', 'Installation complete - awaiting next status')]
+        self.status_set.assert_has_calls(status_calls)
+
+    def test_wipe_corosync_state(self):
+        self.patch(handlers.shutil, 'rmtree')
+        self.patch(handlers.os, 'remove')
+        handlers.wipe_corosync_state()
+        self.rmtree.assert_called_once_with('/etc/corosync/uidgid.d')
+        self.remove.assert_called_once_with('/etc/corosync/corosync.conf')
+
+    def test_wipe_corosync_state_noop(self):
+        self.patch(handlers.shutil, 'rmtree')
+        self.patch(handlers.os, 'remove')
+        self.rmtree.side_effect = FileNotFoundError
+        self.remove.side_effect = FileNotFoundError
+        handlers.wipe_corosync_state()
+
+    def test_restart_services(self):
+        self.patch(handlers.ch_host, 'service_restart')
+        handlers.restart_services()
+        restart_calls = [
+            mock.call('pacemaker_remote'),
+            mock.call('pcsd')]
+        self.service_restart.assert_has_calls(restart_calls)
+
+    def test_install(self):
+        self.patch(handlers.reactive, 'set_flag')
+        self.patch(handlers, 'install_packages')
+        self.patch(handlers, 'wipe_corosync_state')
+        handlers.install()
+        self.set_flag.assert_called_once_with('pacemaker-remote.installed')
+        self.install_packages.assert_called_once_with()
+        self.wipe_corosync_state.assert_called_once_with()
+
+    def test_publish_stonith_info(self):
+        self.patch(handlers.charmhelpers.contrib.network.ip, 'get_hostname')
+        self.patch(handlers.hookenv, 'status_set')
+        self.patch(handlers.hookenv, 'config')
+        self.patch(handlers.hookenv, 'unit_get')
+        self.patch(handlers.reactive, 'endpoint_from_flag')
+        self.unit_get.return_value = '10.0.0.10'
+        self.get_hostname.return_value = 'myhost.maas'
+        cfg = {
+            'enable-stonith': True,
+            'enable-resources': True}
+        self.config.side_effect = lambda x: cfg.get(x)
+        endpoint_mock = mock.MagicMock()
+        self.endpoint_from_flag.return_value = endpoint_mock
+        handlers.publish_stonith_info()
+        endpoint_mock.publish_info.assert_called_once_with(
+            enable_resources=True,
+            remote_hostname='myhost.maas',
+            stonith_hostname='myhost.maas')
+
+    def test_publish_stonith_info_all_off(self):
+        self.patch(handlers.charmhelpers.contrib.network.ip, 'get_hostname')
+        self.patch(handlers.hookenv, 'status_set')
+        self.patch(handlers.hookenv, 'config')
+        self.patch(handlers.hookenv, 'unit_get')
+        self.patch(handlers.reactive, 'endpoint_from_flag')
+        self.unit_get.return_value = '10.0.0.10'
+        self.get_hostname.return_value = 'myhost.maas'
+        cfg = {
+            'enable-stonith': False,
+            'enable-resources': False}
+        self.config.side_effect = lambda x: cfg.get(x)
+        endpoint_mock = mock.MagicMock()
+        self.endpoint_from_flag.return_value = endpoint_mock
+        handlers.publish_stonith_info()
+        endpoint_mock.publish_info.assert_called_once_with(
+            enable_resources=False,
+            remote_hostname='myhost.maas',
+            stonith_hostname=None)
+
+    def test_write_pacemaker_key(self):
+        self.patch(handlers.reactive, 'endpoint_from_flag')
+        endpoint_mock = mock.MagicMock()
+        endpoint_mock.get_pacemaker_key.return_value = 'corokey'
+        self.endpoint_from_flag.return_value = endpoint_mock
+        self.patch(handlers.hookenv, 'status_set')
+        self.patch(handlers, 'restart_services')
+        self.patch(handlers.ch_host, 'write_file')
+        handlers.write_pacemaker_key()
+        self.restart_services.assert_called_once_with()
+        self.write_file.assert_called_once_with(
+            '/etc/pacemaker/authkey',
+            'corokey',
+            group='haclient',
+            owner='hacluster',
+            perms=292)
+        self.status_set.assert_called_once_with('active', 'Unit is ready')
+
+    def test_write_pacemaker_key_nokey(self):
+        self.patch(handlers.reactive, 'endpoint_from_flag')
+        endpoint_mock = mock.MagicMock()
+        endpoint_mock.get_pacemaker_key.return_value = None
+        self.endpoint_from_flag.return_value = endpoint_mock
+        self.patch(handlers, 'restart_services')
+        self.patch(handlers.ch_host, 'write_file')
+        handlers.write_pacemaker_key()
+        self.assertFalse(self.restart_services.called)
+        self.assertFalse(self.write_file.called)