Add docstrings, unit tests etc

.gitignore

@@ -0,0 +1,5 @@
+.tox
+.testrepository
+.unit-state.db
+.stestr/
+__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

interface.yaml

@@ -1,4 +1,13 @@
 name: nova-compute
-summary: Controller <-> compute interface
+summary: Interface for joining compute resource to a controller.
 version: 1
-maintainer: "<openstack-charmers@lists.ubuntu.com>"
+maintainer: OpenStack Charmers <openstack-dev@lists.openstack.org>
+ignore:
+  - 'unit_tests'
+  - 'Makefile'
+  - '.testr.conf'
+  - 'test-requirements.txt'
+  - 'tox.ini'
+  - '.gitignore'
+  - '.gitreview'
+  - '.unit-state.db'

provides.py

@@ -1,28 +0,0 @@
-# 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 charms.reactive import set_flag, clear_flag
-from charms.reactive import Endpoint
-from charms.reactive import when_any, when_not, when
-
-
-class NovaComputeProvides(Endpoint):
-
-    @when_not('endpoint.{endpoint_name}.joined')
-    def broken(self):
-        clear_flag(self.expand_name('endpoint.{endpoint_name}.new-request'))
-        clear_flag(self.expand_name('{endpoint_name}.connected'))
-
-    @when('endpoint.{endpoint_name}.joined')
-    def joined(self):
-        set_flag(self.expand_name('{endpoint_name}.connected'))
-

requires.py

@@ -10,65 +10,142 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import socket
 from urllib.parse import urlparse
 import uuid
 
-from charmhelpers.core import hookenv
-
 from charms.reactive import set_flag, clear_flag
 from charms.reactive import Endpoint
-from charms.reactive import when_any, when_not, when
+from charms.reactive import when_not, when
 
 
 class NovaComputeRequires(Endpoint):
 
     @when('endpoint.{endpoint_name}.changed')
     def data_changed(self):
-        #set_flag(self.expand_name('{endpoint_name}.available'))
-        pass
+        """Set flag to indicate to charm relation data has changed."""
+        set_flag(self.expand_name('{endpoint_name}.available'))
 
     @when_not('endpoint.{endpoint_name}.joined')
     def broken(self):
+        """Remove flag to indicate to charm relation has gone.."""
         clear_flag(self.expand_name('{endpoint_name}.available'))
 
     @when('endpoint.{endpoint_name}.joined')
     def joined(self):
+        """Set flag to indicate to charm relation has been joined."""
         set_flag(self.expand_name('{endpoint_name}.connected'))
 
+    def set_network_data(self, neutron_url, neutron_plugin=None,
+                         network_manager=None, enable_security_groups=True):
+        """Send compute nodes data relating to network setup.
 
-    def set_network_data(self, neutron_url, neutron_plugin=None, network_manager=None, enable_security_groups=True):
+        :param neutron_url: URL for network API service
+        :type neutron_url: str
+        :param neutron_plugin: Neutron plugin to use
+        :type neutron_plugin: str
+        :param network_manager: Network Manager
+        :type network_manager: str
+        :param enable_security_groups: Whether to enable security groups
+        :type enable_security_group: bool
+        """
         o = urlparse(neutron_url)
         if enable_security_groups:
             security_groups = 'yes'
         else:
             security_groups = 'no'
-        for relation in self.relations:
-            relation.to_publish_raw['quantum_host'] = o.hostname
-            relation.to_publish_raw['quantum_plugin'] = neutron_plugin or 'ovs'
-            relation.to_publish_raw['quantum_port'] = o.port
-            relation.to_publish_raw['quantum_security_groups'] = security_groups
-            relation.to_publish_raw['quantum_url'] = neutron_url
-            relation.to_publish_raw['network_manager'] = network_manager or 'neutron'
+        for r in self.relations:
+            r.to_publish_raw['quantum_host'] = o.hostname
+            r.to_publish_raw['quantum_plugin'] = neutron_plugin or 'ovs'
+            r.to_publish_raw['quantum_port'] = o.port
+            r.to_publish_raw['quantum_security_groups'] = security_groups
+            r.to_publish_raw['quantum_url'] = neutron_url
+            r.to_publish_raw['network_manager'] = network_manager or 'neutron'
 
     def set_console_data(self, serial_console_base_url, enable_serial_console):
-        for relation in self.relations:
-            relation.to_publish_raw['serial_console_base_url'] = serial_console_base_url
-            relation.to_publish_raw['enable_serial_console'] = enable_serial_console
+        """Send compute nodes data relating to serial console access.
 
-    def trigger_remote_restart(self):
-        trigger = uuid.uuid1()
+        :param serial_console_base_url: URL for accessing the serial console.
+        :type serial_console_base_url: str
+        :param enable_serial_console: Whether to enable the serial console
+        :type enable_serial_console: bool
+        """
+        for r in self.relations:
+            r.to_publish_raw[
+                'serial_console_base_url'] = serial_console_base_url
+            r.to_publish_raw['enable_serial_console'] = enable_serial_console
+
+    def trigger_remote_restart(self, restart_key=None):
+        """Trigger a restart of services on the remote application.
+
+        :param restart_key: Key to send to remote service, restarts are
+                            triggered when the key changes.
+        :type restart_key: str
+        """
+        if not restart_key:
+            restart_key = uuid.uuid1()
         for relation in self.relations:
-            relation.to_publish_raw['restart_trigger'] = trigger
+            relation.to_publish_raw['restart_trigger'] = restart_key
 
     def set_region(self, region):
+        """Send compute nodes region information.
+
+        :param region: Region compute nodes will belong to.
+        :type region: str
+        """
         for relation in self.relations:
             relation.to_publish_raw['region'] = region
 
     def set_volume_data(self, volume_service):
+        """Send compute nodes volume information.
+
+        :param volume_service: Name of volume service to use, eg cinder
+        :type volume_service: str
+        """
         for relation in self.relations:
             relation.to_publish_raw['volume_service'] = volume_service
 
     def set_ec2_data(self, ec2_host):
+        """Send compute nodes ec2 information.
+
+        :param ec2_host: Name of ec2_host.
+        :type ec2_host: str
+        """
         for relation in self.relations:
             relation.to_publish_raw['ec2_host'] = ec2_host
+
+    def collect_ssh_keys(self, application_name=None):
+        """Query related units and collect ssh artifacts.
+
+        :param application_name: Only return artifacts from units of this
+                                 applicationa.
+        :type application_name: str
+        :returns: {APP_NAME: {UNIT_NAME: {pupkey1:.., hostkey1:...}}}
+        :rtype: dict
+        """
+        ssh_keys = {}
+        for rel in self.relations:
+            if application_name and application_name != rel.application_name:
+                continue
+            ssh_keys[rel.application_name] = {}
+            for unit in rel.units:
+                nova_ssh_pub_key = unit.received.get('nova_ssh_public_key')
+                ssh_pub_key = unit.received.get('ssh_public_key')
+                if nova_ssh_pub_key and ssh_pub_key:
+                    ssh_keys[rel.application_name][unit.unit_name] = {
+                        'nova_ssh_pub_key': nova_ssh_pub_key,
+                        'hostname': unit.received.get('hostname'),
+                        'private-address': unit.received.get(
+                            'private-address'),
+                        'ssh_pub_key': ssh_pub_key}
+        return ssh_keys
+
+    def send_ssh_keys(self, relation, settings):
+        """Publish the provided ssh settings on the given relation
+
+        :param relation: Relation to publish settings on.
+        :type relation: charms.reactive.endpoints.Relation
+        :param settings: SSH settings to publish.
+        :type settings: dict
+        """
+        for key, value in settings.items():
+            relation.to_publish_raw[key] = value

test-requirements.txt

@@ -0,0 +1,6 @@
+# Lint and unit test requirements
+flake8
+os-testr>=0.4.1
+charms.reactive
+mock>=1.2
+coverage>=3.6

tox.ini

@@ -0,0 +1,33 @@
+[tox]
+skipsdist = True
+envlist = pep8,py35
+skip_missing_interpreters = True
+
+[testenv]
+setenv = VIRTUAL_ENV={envdir}
+         PYTHONHASHSEED=0
+         TERM=linux
+install_command =
+  pip install {opts} {packages}
+
+[testenv:py35]
+basepython = python3
+deps = -r{toxinidir}/test-requirements.txt
+commands = ostestr {posargs}
+
+[testenv:py36]
+basepython = python3.6
+deps = -r{toxinidir}/test-requirements.txt
+commands = ostestr {posargs}
+
+[testenv:pep8]
+basepython = python2.7
+deps = -r{toxinidir}/test-requirements.txt
+commands = flake8 {posargs} . unit_tests
+
+[testenv:venv]
+commands = {posargs}
+
+[flake8]
+# E402 ignore necessary for path append before sys module import in actions
+ignore = E402

unit_tests/test_requires.py

@@ -0,0 +1,342 @@
+# 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 unittest
+import mock
+
+
+with mock.patch('charmhelpers.core.hookenv.metadata') as _meta:
+    _meta.return_Value = 'ss'
+    import requires
+
+
+_hook_args = {}
+
+TO_PATCH = [
+    'clear_flag',
+    'set_flag',
+]
+
+
+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.
+        _hook_args[f.__name__] = dict(args=args, kwargs=kwargs)
+        return f
+    return inner
+
+
+class _unit_mock:
+    def __init__(self, unit_name, received=None):
+        self.unit_name = unit_name
+        self.received = received or {}
+
+
+class _relation_mock:
+    def __init__(self, application_name=None, units=None):
+        self.to_publish_raw = {}
+        self.application_name = application_name
+        self.units = units
+
+
+class TestNovaComputeRequires(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        cls._patched_hook = mock.patch('charms.reactive.when', mock_hook)
+        cls._patched_hook_started = cls._patched_hook.start()
+        # force requires to rerun the mock_hook decorator:
+        # try except is Python2/Python3 compatibility as Python3 has moved
+        # reload to importlib.
+        try:
+            reload(requires)
+        except NameError:
+            import importlib
+            importlib.reload(requires)
+
+    @classmethod
+    def tearDownClass(cls):
+        cls._patched_hook.stop()
+        cls._patched_hook_started = None
+        cls._patched_hook = None
+        # and fix any breakage we did to the module
+        try:
+            reload(requires)
+        except NameError:
+            import importlib
+            importlib.reload(requires)
+
+    def patch(self, method):
+        _m = mock.patch.object(self.obj, method)
+        _mock = _m.start()
+        self.addCleanup(_m.stop)
+        return _mock
+
+    def setUp(self):
+        self.ncr = requires.NovaComputeRequires('some-relation', [])
+        self._patches = {}
+        self._patches_start = {}
+        self.obj = requires
+        for method in TO_PATCH:
+            setattr(self, method, self.patch(method))
+
+    def tearDown(self):
+        self.ncr = None
+        for k, v in self._patches.items():
+            v.stop()
+            setattr(self, k, None)
+        self._patches = None
+        self._patches_start = None
+
+    def patch_kr(self, attr, return_value=None):
+        mocked = mock.patch.object(self.ncr, 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 decorators 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.
+        hook_patterns = {
+            'data_changed': ('endpoint.{endpoint_name}.changed', ),
+            'joined': ('endpoint.{endpoint_name}.joined', ),
+            'broken': ('endpoint.{endpoint_name}.joined', ),
+        }
+        for k, v in _hook_args.items():
+            self.assertEqual(hook_patterns[k], v['args'])
+
+    def test_date_changed(self):
+        self.ncr.data_changed()
+        self.set_flag.assert_called_once_with('some-relation.available')
+
+    def test_broken(self):
+        self.ncr.broken()
+        self.clear_flag.assert_called_once_with('some-relation.available')
+
+    def test_joined(self):
+        self.ncr.joined()
+        self.set_flag.assert_called_once_with('some-relation.connected')
+
+    def test_set_network_data(self):
+        mock_rel1 = _relation_mock()
+        mock_rel2 = _relation_mock()
+        self.ncr._relations = [mock_rel1, mock_rel2]
+        self.ncr.set_network_data(
+            'http://bob:345/dddd/')
+        expect = {
+            'quantum_host': 'bob',
+            'quantum_plugin': 'ovs',
+            'quantum_port': 345,
+            'quantum_security_groups': 'yes',
+            'quantum_url': 'http://bob:345/dddd/',
+            'network_manager': 'neutron'
+        }
+        self.assertEqual(mock_rel1.to_publish_raw, expect)
+        self.assertEqual(mock_rel2.to_publish_raw, expect)
+
+    def test_set_network_data_no_defaults(self):
+        mock_rel1 = _relation_mock()
+        mock_rel2 = _relation_mock()
+        self.ncr._relations = [mock_rel1, mock_rel2]
+        self.ncr.set_network_data(
+            'http://bob:345/dddd/',
+            neutron_plugin='vTokenRing',
+            network_manager='vTRManager',
+            enable_security_groups=False)
+        expect = {
+            'quantum_host': 'bob',
+            'quantum_plugin': 'vTokenRing',
+            'quantum_port': 345,
+            'quantum_security_groups': 'no',
+            'quantum_url': 'http://bob:345/dddd/',
+            'network_manager': 'vTRManager'
+        }
+        self.assertEqual(mock_rel1.to_publish_raw, expect)
+        self.assertEqual(mock_rel2.to_publish_raw, expect)
+
+    def test_set_console_data(self):
+        mock_rel1 = _relation_mock()
+        mock_rel2 = _relation_mock()
+        self.ncr._relations = [mock_rel1, mock_rel2]
+        self.ncr.set_console_data(
+            'http://bob:345/serial/',
+            enable_serial_console=True)
+        expect = {
+            'serial_console_base_url': 'http://bob:345/serial/',
+            'enable_serial_console': True}
+        self.assertEqual(mock_rel1.to_publish_raw, expect)
+        self.assertEqual(mock_rel2.to_publish_raw, expect)
+
+    def test_trigger_remote_restart(self):
+        mock_rel1 = _relation_mock()
+        mock_rel2 = _relation_mock()
+        self.ncr._relations = [mock_rel1, mock_rel2]
+        self.ncr.trigger_remote_restart(restart_key='akey')
+        expect = {'restart_trigger': 'akey'}
+        self.assertEqual(mock_rel1.to_publish_raw, expect)
+        self.assertEqual(mock_rel2.to_publish_raw, expect)
+
+    def test_trigger_remote_restart_gen_key(self):
+        mock_rel1 = _relation_mock()
+        mock_rel2 = _relation_mock()
+        self.ncr._relations = [mock_rel1, mock_rel2]
+        self.ncr.trigger_remote_restart()
+        expect = {'restart_trigger': mock.ANY}
+        self.assertEqual(mock_rel1.to_publish_raw, expect)
+        self.assertEqual(mock_rel2.to_publish_raw, expect)
+
+    def test_set_region(self):
+        mock_rel1 = _relation_mock()
+        mock_rel2 = _relation_mock()
+        self.ncr._relations = [mock_rel1, mock_rel2]
+        self.ncr.set_region('Region12')
+        expect = {'region': 'Region12'}
+        self.assertEqual(mock_rel1.to_publish_raw, expect)
+        self.assertEqual(mock_rel2.to_publish_raw, expect)
+
+    def test_set_volume_data(self):
+        mock_rel1 = _relation_mock()
+        mock_rel2 = _relation_mock()
+        self.ncr._relations = [mock_rel1, mock_rel2]
+        self.ncr.set_volume_data('http://volhost')
+        expect = {'volume_service': 'http://volhost'}
+        self.assertEqual(mock_rel1.to_publish_raw, expect)
+        self.assertEqual(mock_rel2.to_publish_raw, expect)
+
+    def test_set_ec2_data(self):
+        mock_rel1 = _relation_mock()
+        mock_rel2 = _relation_mock()
+        self.ncr._relations = [mock_rel1, mock_rel2]
+        self.ncr.set_ec2_data('http://ec2host')
+        expect = {'ec2_host': 'http://ec2host'}
+        self.assertEqual(mock_rel1.to_publish_raw, expect)
+        self.assertEqual(mock_rel2.to_publish_raw, expect)
+
+    def test_collect_ssh_keys_single_relation(self):
+        unit1_data = {
+            'hostname': 'juju-4665be-20180716142533-9',
+            'private-address': '10.5.0.17',
+            'nova_ssh_public_key': 'unit1 nova pub key',
+            'ssh_public_key': 'unit1 pub key'}
+        mock_unit1 = _unit_mock(unit_name='unit1', received=unit1_data)
+        mock_rel1 = _relation_mock(
+            application_name='nova-compute',
+            units=[mock_unit1])
+        self.ncr._relations = [mock_rel1]
+        expect = {'nova-compute': {'unit1': {
+            'hostname': 'juju-4665be-20180716142533-9',
+            'nova_ssh_pub_key': 'unit1 nova pub key',
+            'private-address': '10.5.0.17',
+            'ssh_pub_key': 'unit1 pub key'}}}
+        self.assertEqual(self.ncr.collect_ssh_keys(), expect)
+
+    def test_collect_ssh_keys_single_relation_set_app_name(self):
+        unit1_data = {
+            'hostname': 'juju-4665be-20180716142533-9',
+            'private-address': '10.5.0.17',
+            'nova_ssh_public_key': 'unit1 nova pub key',
+            'ssh_public_key': 'unit1 pub key'}
+        mock_unit1 = _unit_mock(unit_name='unit1', received=unit1_data)
+        mock_rel1 = _relation_mock(
+            application_name='nova-compute',
+            units=[mock_unit1])
+        self.ncr._relations = [mock_rel1]
+        expect = {'nova-compute': {'unit1': {
+            'hostname': 'juju-4665be-20180716142533-9',
+            'nova_ssh_pub_key': 'unit1 nova pub key',
+            'private-address': '10.5.0.17',
+            'ssh_pub_key': 'unit1 pub key'}}}
+        self.assertEqual(
+            self.ncr.collect_ssh_keys(application_name='nova-compute'),
+            expect)
+
+    def test_collect_ssh_keys_mutli_relation(self):
+        unit1_data = {
+            'hostname': 'juju-4665be-20180716142533-9',
+            'private-address': '10.5.0.17',
+            'nova_ssh_public_key': 'unit1 nova pub key',
+            'ssh_public_key': 'unit1 pub key'}
+        unit2_data = {
+            'hostname': 'juju-4665be-20180716142533-8',
+            'private-address': '10.5.0.16',
+            'nova_ssh_public_key': 'unit2 nova pub key',
+            'ssh_public_key': 'unit2 pub key'}
+        mock_unit1 = _unit_mock(unit_name='unit1', received=unit1_data)
+        mock_unit2 = _unit_mock(unit_name='unit2', received=unit2_data)
+        mock_rel1 = _relation_mock(
+            application_name='nova-compute',
+            units=[mock_unit1])
+        mock_rel2 = _relation_mock(
+            application_name='nova-compute-cell3',
+            units=[mock_unit2])
+        self.ncr._relations = [mock_rel1, mock_rel2]
+        expect = {
+            'nova-compute': {
+                'unit1': {
+                    'hostname': 'juju-4665be-20180716142533-9',
+                    'nova_ssh_pub_key': 'unit1 nova pub key',
+                    'private-address': '10.5.0.17',
+                    'ssh_pub_key': 'unit1 pub key'}},
+            'nova-compute-cell3': {
+                'unit2': {
+                    'hostname': 'juju-4665be-20180716142533-8',
+                    'nova_ssh_pub_key': 'unit2 nova pub key',
+                    'private-address': '10.5.0.16',
+                    'ssh_pub_key': 'unit2 pub key'}}}
+        self.assertEqual(
+            self.ncr.collect_ssh_keys(),
+            expect)
+
+    def test_collect_ssh_keys_mutli_relation_app_name(self):
+        unit1_data = {
+            'hostname': 'juju-4665be-20180716142533-9',
+            'private-address': '10.5.0.17',
+            'nova_ssh_public_key': 'unit1 nova pub key',
+            'ssh_public_key': 'unit1 pub key'}
+        unit2_data = {
+            'hostname': 'juju-4665be-20180716142533-8',
+            'private-address': '10.5.0.16',
+            'nova_ssh_public_key': 'unit2 nova pub key',
+            'ssh_public_key': 'unit2 pub key'}
+        mock_unit1 = _unit_mock(unit_name='unit1', received=unit1_data)
+        mock_unit2 = _unit_mock(unit_name='unit2', received=unit2_data)
+        mock_rel1 = _relation_mock(
+            application_name='nova-compute',
+            units=[mock_unit1])
+        mock_rel2 = _relation_mock(
+            application_name='nova-compute-cell3',
+            units=[mock_unit2])
+        self.ncr._relations = [mock_rel1, mock_rel2]
+        expect = {
+            'nova-compute': {
+                'unit1': {
+                    'hostname': 'juju-4665be-20180716142533-9',
+                    'nova_ssh_pub_key': 'unit1 nova pub key',
+                    'private-address': '10.5.0.17',
+                    'ssh_pub_key': 'unit1 pub key'}}}
+        self.assertEqual(
+            self.ncr.collect_ssh_keys(application_name='nova-compute'),
+            expect)
+
+    def test_send_ssh_keys(self):
+        mock_rel1 = _relation_mock()
+        self.ncr.send_ssh_keys(mock_rel1, {'key1': 'k1', 'key2': 'k2'})
+        self.assertEqual(
+            mock_rel1.to_publish_raw,
+            {'key1': 'k1', 'key2': 'k2'})