Unit tests and lint fixes

.gitignore

@@ -4,3 +4,5 @@ layers
 interfaces
 builds
 deps
+.testrepository
+__pycache__

.testr.conf

@@ -0,0 +1,8 @@
+[DEFAULT]
+test_command=OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} \
+             OS_STDERR_CAPTURE=${OS_STDERR_CAPTURE:-1} \
+             OS_TEST_TIMEOUT=${OS_TEST_TIMEOUT:-60} \
+             ${PYTHON:-python} -m subunit.run discover -t ./ ./unit_tests $LISTOPT $IDOPTION
+
+test_id_option=--load-list $IDFILE
+test_list_option=--list

requirements.txt

@@ -1,3 +1,5 @@
+# Requirements to build the charm
 charm-tools
-flake8
 ruamel.yaml==0.10.12
+simplejson
+flake8

src/lib/charm/openstack/designate.py

@@ -30,6 +30,7 @@ def db_sync_done():
     """
     return DesignateCharm.singleton.db_sync_done()
 
+
 def db_sync():
     """Use the singleton from the DesignateCharm to run db migration
     """
@@ -47,6 +48,7 @@ def create_initial_servers_and_domains():
     """
     DesignateCharm.singleton.create_initial_servers_and_domains()
 
+
 def domain_init_done():
     """Use the singleton from the DesignateCharm to run render_base_config
     """
@@ -70,27 +72,33 @@ def register_endpoints(keystone):
                                 charm.internal_url,
                                 charm.admin_url)
 
+
 def configure_ha_resources(hacluster):
     """Use the singleton from the DesignateCharm to run render_base_config
     """
     DesignateCharm.singleton.configure_ha_resources(hacluster)
 
+
 def restart_all():
     """Use the singleton from the DesignateCharm to run render_base_config
     """
     DesignateCharm.singleton.restart_all()
 
+
 def configure_ssl(keystone=None):
     """Use the singleton from the DesignateCharm to run render_base_config
     """
     DesignateCharm.singleton.configure_ssl(keystone)
 
+
 def update_peers(cluster):
     DesignateCharm.singleton.update_peers(cluster)
 
+
 def render_rndc_keys():
     DesignateCharm.singleton.render_rndc_keys()
 
+
 def assess_status():
     """Just call the BarbicanCharm.singleton.assess_status() command to update
     status on the unit.
@@ -116,23 +124,24 @@ class DesignateDBAdapter(openstack_adapters.DatabaseRelationAdapter):
 class BindRNDCRelationAdapter(openstack_adapters.OpenStackRelationAdapter):
 
     interface_type = "dns"
+
     def __init__(self, relation):
         super(BindRNDCRelationAdapter, self).__init__(relation)
 
     @property
     def slave_ips(self):
         return self.relation.slave_ips()
-       
+
     @property
-    def pool_config(self): 
+    def pool_config(self):
         pconfig = []
         for slave in self.slave_ips:
-           unit_name = slave['unit'].replace('/', '_').replace('-', '_')
-           pconfig.append({
-               'nameserver': 'nameserver_{}'.format(unit_name),
-               'pool_target': 'nameserver_{}'.format(unit_name),
-               'address': slave['address'],
-           })
+            unit_name = slave['unit'].replace('/', '_').replace('-', '_')
+            pconfig.append({
+                'nameserver': 'nameserver_{}'.format(unit_name),
+                'pool_target': 'nameserver_{}'.format(unit_name),
+                'address': slave['address'],
+            })
         return pconfig
 
     @property
@@ -146,13 +155,13 @@ class BindRNDCRelationAdapter(openstack_adapters.OpenStackRelationAdapter):
     @property
     def slave_addresses(self):
         return ', '.join(['{}:53'.format(s['address'])
-                                         for s in self.pool_config])
+                         for s in self.pool_config])
 
     @property
     def rndc_info(self):
         return self.relation.rndc_info()
 
- 
+
 class DesignateConfigurationAdapter(
       openstack_adapters.APIConfigurationAdapter):
 
@@ -165,14 +174,15 @@ class DesignateConfigurationAdapter(
     def pool_config(self):
         pconfig = []
         for entry in self.dns_slaves.split():
-           address, port, key = entry.split(':')
-           unit_name = address.replace('.', '_')
-           pconfig.append({
-               'nameserver': 'nameserver_{}'.format(unit_name),
-               'pool_target': 'nameserver_{}'.format(unit_name),
-               'address': address,
-               'rndc_key_file': '/etc/designate/rndc_{}.key'.format(unit_name),
-           })
+            address, port, key = entry.split(':')
+            unit_name = address.replace('.', '_')
+            pconfig.append({
+                'nameserver': 'nameserver_{}'.format(unit_name),
+                'pool_target': 'nameserver_{}'.format(unit_name),
+                'address': address,
+                'rndc_key_file': '/etc/designate/rndc_{}.key'.format(
+                    unit_name),
+            })
         return pconfig
 
     @property
@@ -186,8 +196,8 @@ class DesignateConfigurationAdapter(
     @property
     def slave_addresses(self):
         return ', '.join(['{}:53'.format(s['address'])
-                                         for s in self.pool_config])
- 
+                         for s in self.pool_config])
+
     @property
     def nova_domain_id(self):
         """Returns the id of the domain corresponding to the user supplied
@@ -232,13 +242,13 @@ class DesignateConfigurationAdapter(
             daemon_arg = '--config-file={}'.format(NEUTRON_SINK_FILE)
         return daemon_arg
 
-
     @property
     def rndc_master_ip(self):
         """Returns IP address slave DNS slave should use to query master
         """
         return os_ip.resolve_address(endpoint_type=os_ip.INTERNAL)
 
+
 class DesignateAdapters(openstack_adapters.OpenStackAPIRelationAdapters):
     """
     Adapters class for the Designate charm.
@@ -306,7 +316,6 @@ class DesignateCharm(openstack_charm.HAOpenStackCharm):
             release = ch_utils.os_release('python-keystonemiddleware')
         super(DesignateCharm, self).__init__(release=release, **kwargs)
 
-
     def install(self):
         """Customise the installation, configure the source and then call the
         parent install() method to install the packages
@@ -314,7 +323,6 @@ class DesignateCharm(openstack_charm.HAOpenStackCharm):
         self.configure_source()
         super(DesignateCharm, self).install()
 
-
     def render_base_config(self, interfaces_list):
         """Render initial config to bootstrap Designate service
 
@@ -360,7 +368,7 @@ class DesignateCharm(openstack_charm.HAOpenStackCharm):
             address, port, key = entry.split(':')
             unit_name = address.replace('.', '_')
             self.write_key_file(unit_name, key)
-     
+
     @classmethod
     def get_domain_id(cls, domain):
         """Return the domain ID for a given domain name
@@ -416,4 +424,3 @@ class DesignateCharm(openstack_charm.HAOpenStackCharm):
                 hookenv.config('neutron-domain'),
                 hookenv.config('neutron-domain-email'))
             hookenv.leader_set({'domain-init-done': True})
-

src/reactive/designate_handlers.py

@@ -9,6 +9,7 @@ COMPLETE_INTERFACE_STATES = [
     'amqp.available',
 ]
 
+
 @reactive.when_not('installed')
 def install_packages():
     designate.install()
@@ -33,7 +34,7 @@ def setup_database(database):
 
 @reactive.when('identity-service.connected')
 def setup_endpoint(keystone):
-    designate.register_endpoints(keystone) 
+    designate.register_endpoints(keystone)
     designate.assess_status()
 
 
@@ -43,10 +44,12 @@ def configure_designate(*args):
     designate.render_base_config(args)
     reactive.set_state('base-config.rendered')
 
+
 @reactive.when('identity-service.available')
 def configure_ssl(keystone):
     designate.configure_ssl(keystone)
 
+
 @reactive.when_not('db.synched')
 @reactive.when('base-config.rendered')
 @reactive.when(*COMPLETE_INTERFACE_STATES)
@@ -55,6 +58,7 @@ def run_db_migration(*args):
     if designate.db_sync_done():
         reactive.set_state('db.synched')
 
+
 @reactive.when_not('domains.created')
 @reactive.when('db.synched')
 @reactive.when('base-config.rendered')
@@ -64,16 +68,19 @@ def create_servers_and_domains(*args):
     if designate.domain_init_done():
         reactive.set_state('domains.created')
 
+
 @reactive.when('cluster.available')
 def update_peers(cluster):
     designate.update_peers(cluster)
 
+
 @reactive.when('cluster.available')
 @reactive.when('domains.created')
 @reactive.when(*COMPLETE_INTERFACE_STATES)
 def render_all_configs(*args):
     designate.render_full_config(args)
 
+
 @reactive.when_not('cluster.available')
 @reactive.when('domains.created')
 @reactive.when(*COMPLETE_INTERFACE_STATES)
@@ -81,11 +88,13 @@ def render_all_configs_single_node(*args):
     designate.render_full_config(args)
     designate.render_rndc_keys()
 
+
 @reactive.when('ha.connected')
 def cluster_connected(hacluster):
     designate.configure_ha_resources(hacluster)
     designate.assess_status()
 
+
 @reactive.when('config.changed')
 def config_changed():
     """When the configuration changes, assess the unit's status to update any

test-requirements.txt

@@ -0,0 +1,7 @@
+# Unit test requirements
+flake8>=2.2.4,<=2.4.1
+os-testr>=0.4.1
+charms.reactive
+mock>=1.2
+coverage>=3.6
+git+https://github.com/gnuoy/charms.openstack.git@bug/general#egg=charms.openstack

tox.ini

@@ -1,8 +1,10 @@
 [tox]
 skipsdist = True
-envlist = generate
+envlist = pep8,py34,py35
+skip_missing_interpreters = True
 
 [testenv]
+basepython = python2.7
 setenv = VIRTUAL_ENV={envdir}
          PYTHONHASHSEED=0
          TERM=linux
@@ -13,16 +15,29 @@ passenv = http_proxy https_proxy
 install_command =
   pip install {opts} {packages}
 deps =
-    -r{toxinidir}/requirements.txt
+  -r{toxinidir}/requirements.txt
 
-[testenv:generate]
-basepython = python2.7
+[testenv:build]
 commands =
-    charm build --log-level DEBUG -o {toxinidir}/build charm
+  charm-build --log-level DEBUG -o {toxinidir}/build src
 
 [testenv:venv]
 commands = {posargs}
 
-[testenv:lint]
+[testenv:pep8]
+commands = flake8 {posargs} src/reactive src/lib unit_tests
+
+[testenv:py27]
 basepython = python2.7
-commands = flake8 {posargs} charm/reactive charm/lib
+deps = -r{toxinidir}/test-requirements.txt
+commands = ostestr {posargs}
+
+[testenv:py34]
+basepython = python3.4
+deps = -r{toxinidir}/test-requirements.txt
+commands = ostestr {posargs}
+
+[testenv:py35]
+basepython = python3.5
+deps = -r{toxinidir}/test-requirements.txt
+commands = ostestr {posargs}

unit_tests/__init__.py

@@ -0,0 +1,43 @@
+# 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
+import mock
+
+sys.path.append('src')
+sys.path.append('src/lib')
+
+# Mock out charmhelpers so that we can test without it.
+# also stops sideeffects from occuring.
+charmhelpers = mock.MagicMock()
+sys.modules['charmhelpers'] = charmhelpers
+sys.modules['charmhelpers.core'] = charmhelpers.core
+sys.modules['charmhelpers.core.hookenv'] = charmhelpers.core.hookenv
+sys.modules['charmhelpers.core.host'] = charmhelpers.core.host
+sys.modules['charmhelpers.core.unitdata'] = charmhelpers.core.unitdata
+sys.modules['charmhelpers.core.templating'] = charmhelpers.core.templating
+sys.modules['charmhelpers.contrib'] = charmhelpers.contrib
+sys.modules['charmhelpers.contrib.openstack'] = charmhelpers.contrib.openstack
+sys.modules['charmhelpers.contrib.openstack.utils'] = (
+    charmhelpers.contrib.openstack.utils)
+sys.modules['charmhelpers.contrib.openstack.templating'] = (
+    charmhelpers.contrib.openstack.templating)
+sys.modules['charmhelpers.contrib.network'] = charmhelpers.contrib.network
+sys.modules['charmhelpers.contrib.network.ip'] = (
+    charmhelpers.contrib.network.ip)
+sys.modules['charmhelpers.fetch'] = charmhelpers.fetch
+sys.modules['charmhelpers.cli'] = charmhelpers.cli
+sys.modules['charmhelpers.contrib.hahelpers'] = charmhelpers.contrib.hahelpers
+sys.modules['charmhelpers.contrib.hahelpers.cluster'] = (
+    charmhelpers.contrib.hahelpers.cluster)

unit_tests/test_designate_handlers.py

@@ -0,0 +1,253 @@
+from __future__ import absolute_import
+from __future__ import print_function
+
+import unittest
+
+import mock
+
+import reactive.designate_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 TestDesignateHandlers(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_registered_hooks(self):
+        # test that the hooks actually registered the relation expressions that
+        # are meaningful for this interface: this is to handle regressions.
+        # The keys are the function names that the hook attaches to.
+        all_interfaces = (
+            'dns-backend.available',
+            'shared-db.available',
+            'identity-service.available',
+            'amqp.available')
+        when_patterns = {
+            'setup_amqp_req': [('amqp.connected', )],
+            'setup_database': [('shared-db.connected', )],
+            'setup_endpoint': [('identity-service.connected', )],
+            'configure_ssl': [('identity-service.available', )],
+            'update_peers': [('cluster.available', )],
+            'config_changed': [('config.changed', )],
+            'cluster_connected': [('ha.connected', )],
+            'create_servers_and_domains': [
+                all_interfaces,
+                ('base-config.rendered', ),
+                ('db.synched', ),
+            ],
+            'render_all_configs': [
+                all_interfaces,
+                ('domains.created', ),
+                ('cluster.available', ),
+            ],
+            'render_all_configs_single_node': [
+                all_interfaces,
+                ('domains.created', ),
+            ],
+            'run_db_migration': [
+                all_interfaces,
+                ('base-config.rendered', ),
+            ],
+            'configure_designate': [
+                all_interfaces,
+            ],
+        }
+        when_not_patterns = {
+            'install_packages': [('installed', )],
+            'run_db_migration': [('db.synched', )],
+            'render_all_configs_single_node': [('cluster.available', )],
+            'configure_designate': [('base-config.rendered', )],
+            'create_servers_and_domains': [('domains.created', )],
+        }
+        # check the when hooks are attached to the expected functions
+        for t, p in [(_when_args, when_patterns),
+                     (_when_not_args, when_not_patterns)]:
+            for f, args in t.items():
+                # check that function is in patterns
+                self.assertTrue(f in p.keys())
+                # check that the lists are equal
+                l = [a['args'] for a in args]
+                self.assertEqual(l, p[f])
+
+    def test_install_packages(self):
+        self.patch(handlers.designate, 'install')
+        self.patch(handlers.reactive, 'set_state')
+        handlers.install_packages()
+        self.install.assert_called_once_with()
+        self.set_state.assert_called_once_with('installed')
+
+    def test_setup_amqp_req(self):
+        self.patch(handlers.designate, 'assess_status')
+        amqp = mock.MagicMock()
+        handlers.setup_amqp_req(amqp)
+        amqp.request_access.assert_called_once_with(
+            username='designate', vhost='openstack')
+        self.assess_status.assert_called_once_with()
+
+    def test_database(self):
+        self.patch(handlers.designate, 'assess_status')
+        database = mock.MagicMock()
+        self.patch(handlers.hookenv, 'unit_private_ip', 'private_ip')
+        handlers.setup_database(database)
+        calls = [
+            mock.call(
+                'designate',
+                'designate',
+                'private_ip',
+                prefix='designate'),
+            mock.call(
+                'dpm',
+                'dpm',
+                'private_ip',
+                prefix='dpm'),
+        ]
+        database.configure.has_calls(calls)
+        self.assess_status.assert_called_once_with()
+
+    def test_setup_endpoint(self):
+        self.patch(handlers.designate, 'assess_status')
+        self.patch(handlers.designate, 'register_endpoints')
+        handlers.setup_endpoint('endpoint_object')
+        self.register_endpoints.assert_called_once_with('endpoint_object')
+        self.assess_status.assert_called_once_with()
+
+    def test_configure_designate(self):
+        self.patch(handlers.reactive, 'set_state')
+        self.patch(handlers.designate, 'render_base_config')
+        handlers.configure_designate('arg1', 'arg2')
+        self.render_base_config.assert_called_once_with(('arg1', 'arg2', ))
+        self.set_state.assert_called_once_with('base-config.rendered')
+
+    def test_configure_ssl(self):
+        keystone = mock.MagicMock()
+        self.patch(handlers.designate, 'configure_ssl')
+        handlers.configure_ssl(keystone)
+        self.configure_ssl.assert_called_once_with(keystone)
+
+    def test_run_db_migration(self):
+        self.patch(handlers.reactive, 'set_state')
+        self.patch(handlers.designate, 'db_sync')
+        self.patch(handlers.designate, 'db_sync_done')
+        self.db_sync_done.return_value = False
+        handlers.run_db_migration('arg1', 'arg2')
+        self.db_sync.assert_called_once_with()
+        self.assertFalse(self.set_state.called)
+        self.db_sync.reset_mock()
+        self.db_sync_done.return_value = True
+        handlers.run_db_migration('arg1', 'arg2')
+        self.db_sync.assert_called_once_with()
+        self.set_state.assert_called_once_with('db.synched')
+
+    def test_create_servers_and_domains(self):
+        self.patch(handlers.reactive, 'set_state')
+        self.patch(handlers.designate, 'create_initial_servers_and_domains')
+        self.patch(handlers.designate, 'domain_init_done')
+        self.domain_init_done.return_value = False
+        handlers.create_servers_and_domains('arg1', 'arg2')
+        self.create_initial_servers_and_domains.assert_called_once_with()
+        self.assertFalse(self.set_state.called)
+        self.create_initial_servers_and_domains.reset_mock()
+        self.domain_init_done.return_value = True
+        handlers.create_servers_and_domains('arg1', 'arg2')
+        self.create_initial_servers_and_domains.assert_called_once_with()
+        self.set_state.assert_called_once_with('domains.created')
+
+    def test_update_peers(self):
+        cluster = mock.MagicMock()
+        self.patch(handlers.designate, 'update_peers')
+        handlers.update_peers(cluster)
+        self.update_peers.assert_called_once_with(cluster)
+
+    def test_render_all_configs(self):
+        self.patch(handlers.designate, 'render_full_config')
+        handlers.render_all_configs('arg1', 'arg2')
+        self.render_full_config.assert_called_once_with(('arg1', 'arg2', ))
+
+    def test_render_all_configs_single_node(self):
+        self.patch(handlers.designate, 'render_full_config')
+        self.patch(handlers.designate, 'render_rndc_keys')
+        handlers.render_all_configs_single_node('arg1', 'arg2')
+        self.render_full_config.assert_called_once_with(('arg1', 'arg2', ))
+        self.render_rndc_keys.assert_called_once_with()
+
+    def test_cluster_connected(self):
+        hacluster = mock.MagicMock()
+        self.patch(handlers.designate, 'configure_ha_resources')
+        self.patch(handlers.designate, 'assess_status')
+        handlers.cluster_connected(hacluster)
+        self.configure_ha_resources.assert_called_once_with(hacluster)
+        self.assess_status.assert_called_once_with()
+
+    def test_config_changed(self):
+        self.patch(handlers.designate, 'assess_status')
+        handlers.config_changed()
+        self.assess_status.assert_called_once_with()