From 21cf26b29cfb335fc155f572ce17c18ea0ee0b88 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Mon, 22 Jul 2019 12:41:45 +0200 Subject: [PATCH] Update charm to use bus to get charm instance Add ``py3`` target to tox.ini for developer friendliness Update unit tests to use ``charms.openstack`` unit test helers. Change-Id: I4752d8e776491f934cd5c1232166933a9ba17746 Partial-Bug: #1837379 --- src/lib/charm/openstack/dragent.py | 82 ++-------- src/reactive/dragent_handlers.py | 35 ++-- tox.ini | 12 +- unit_tests/test_dragent_handlers.py | 151 +++++------------- .../test_lib_charm_openstack_dragent.py | 16 +- 5 files changed, 93 insertions(+), 203 deletions(-) diff --git a/src/lib/charm/openstack/dragent.py b/src/lib/charm/openstack/dragent.py index b36d73d..6b09b3d 100644 --- a/src/lib/charm/openstack/dragent.py +++ b/src/lib/charm/openstack/dragent.py @@ -51,71 +51,6 @@ PROVIDER_BINDING = 'provider' charms_openstack.charm.use_defaults('charm.default-select-release') -def render_configs(interfaces_list): - """Using a list of interfaces, render the configs and, if they have - changes, restart the services on the unit. - - :returns: None - """ - - try: - [i for i in interfaces_list] - except TypeError: - interfaces_list = [interfaces_list] - # TODO: Add bgp-speaker as an optional interface. - # Currently, charms.openstack does not understand endpoints and will need - # to be updated for this functionality. - DRAgentCharm.singleton.render_with_interfaces(interfaces_list) - - -def assess_status(): - """Just call the DRAgentCharm.singleton.assess_status() command to update - status on the unit. - - :returns: None - """ - - DRAgentCharm.singleton.assess_status() - - -def configure_ssl(): - """Setup SSL communications calling DRAgentCharm.singleton.configure_ssl() - - :returns: None - """ - DRAgentCharm.singleton.configure_ssl() - - -def upgrade_if_available(interfaces_list): - """Just call the DRAgentCharm.singleton.upgrade_if_available() command to - update OpenStack package if upgrade is available - - @returns: None - """ - - DRAgentCharm.singleton.upgrade_if_available(interfaces_list) - - -def bgp_speaker_bindings(): - """Return BGP speaker bindings for the bgp interface - - :returns: list of bindings - """ - - return [SPEAKER_BINDING] - - -def get_os_codename(): - """Return OpenStack Codename for installed application - - :returns: OpenStack Codename - :rtype: str - """ - return DRAgentCharm.singleton.get_os_codename_package( - DRAgentCharm.singleton.release_pkg, - DRAgentCharm.singleton.package_codenames) - - @os_adapters.config_property def provider_ip(cls): """Return the provider binding network IP @@ -203,6 +138,23 @@ class DRAgentCharm(charms_openstack.charm.OpenStackCharm): pass + def bgp_speaker_bindings(self): + """Return BGP speaker bindings for the bgp interface + + :returns: list of bindings + """ + return [SPEAKER_BINDING] + + def get_os_codename(self): + """Return OpenStack Codename for installed application + + :returns: OpenStack Codename + :rtype: str + """ + return self.get_os_codename_package( + self.release_pkg, + self.package_codenames) + class RockyDRAgentCharm(DRAgentCharm): diff --git a/src/reactive/dragent_handlers.py b/src/reactive/dragent_handlers.py index 22cf6cd..1abac45 100644 --- a/src/reactive/dragent_handlers.py +++ b/src/reactive/dragent_handlers.py @@ -17,12 +17,11 @@ from __future__ import absolute_import import charms.reactive as reactive +import charms_openstack.bus import charms_openstack.charm as charm -# This charm's library contains all of the handler code associated with -# dragent -- we need to import it to get the definitions for the charm. -import charm.openstack.dragent as dragent +charms_openstack.bus.discover() # Use the charms.openstack defaults for common states and hooks charm.use_defaults( @@ -36,14 +35,15 @@ charm.use_defaults( def publish_bgp_info(endpoint): """Publish BGP information about this unit to interface-bgp peers """ - if dragent.get_os_codename() in ['ocata', 'pike']: - use_16bit_asn = True - else: - use_16bit_asn = False - endpoint.publish_info(passive=True, - bindings=dragent.bgp_speaker_bindings(), - use_16bit_asn=use_16bit_asn) - dragent.assess_status() + with charm.provide_charm_instance() as instance: + if instance.get_os_codename() in ['ocata', 'pike']: + use_16bit_asn = True + else: + use_16bit_asn = False + endpoint.publish_info(passive=True, + bindings=instance.bgp_speaker_bindings(), + use_16bit_asn=use_16bit_asn) + instance.assess_status() @reactive.when('amqp.connected') @@ -53,12 +53,14 @@ def setup_amqp_req(amqp): """ amqp.request_access(username='neutron', vhost='openstack') - dragent.assess_status() + with charm.provide_charm_instance() as instance: + instance.assess_status() @reactive.when('amqp.available.ssl') def configure_ssl(amqp): - dragent.configure_ssl() + with charm.provide_charm_instance() as instance: + instance.configure_ssl() @reactive.when('amqp.available') @@ -66,6 +68,7 @@ def render_configs(*args): """Render the configuration for dynamic routing when all the interfaces are available. """ - dragent.upgrade_if_available(args) - dragent.render_configs(args) - dragent.assess_status() + with charm.provide_charm_instance() as instance: + instance.upgrade_if_available(args) + instance.render_with_interfaces(args) + instance.assess_status() diff --git a/tox.ini b/tox.ini index 18a1ef1..4ab5793 100644 --- a/tox.ini +++ b/tox.ini @@ -3,7 +3,7 @@ # within individual charm repos. [tox] skipsdist = True -envlist = pep8,py35 +envlist = pep8,py3 skip_missing_interpreters = True [testenv] @@ -33,6 +33,11 @@ deps = whitelist_externals = true commands = true +[testenv:py3] +basepython = python3 +deps = -r{toxinidir}/test-requirements.txt +commands = stestr run {posargs} + [testenv:py35] basepython = python3.5 deps = -r{toxinidir}/test-requirements.txt @@ -43,6 +48,11 @@ basepython = python3.6 deps = -r{toxinidir}/test-requirements.txt commands = stestr run {posargs} +[testenv:py37] +basepython = python3.7 +deps = -r{toxinidir}/test-requirements.txt +commands = stestr run {posargs} + [testenv:pep8] basepython = python3 deps = -r{toxinidir}/test-requirements.txt diff --git a/unit_tests/test_dragent_handlers.py b/unit_tests/test_dragent_handlers.py index 2cc1ee9..e3654cb 100644 --- a/unit_tests/test_dragent_handlers.py +++ b/unit_tests/test_dragent_handlers.py @@ -15,150 +15,81 @@ from __future__ import absolute_import from __future__ import print_function -import unittest - import mock +import charm.openstack.dragent as dragent import reactive.dragent_handlers as handlers - -_when_args = {} -_when_not_args = {} +import charms_openstack.test_utils as test_utils -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 TestDRAgentHandlers(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, side_effect=None): - mocked = mock.patch.object(obj, attr) - self._patches[attr] = mocked - started = mocked.start() - started.return_value = return_value - started.side_effect = side_effect - self._patches_start[attr] = started - setattr(self, attr, started) - +class TestDRAgentHooks(test_utils.TestRegisteredHooks): 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. - when_patterns = { - 'publish_bgp_info': ('endpoint.bgp-speaker.changed',), - 'setup_amqp_req': ('amqp.connected', ), - 'render_configs': ('amqp.available', ), - 'configure_ssl': ('amqp.available.ssl', ), + defaults = [ + 'charm.installed', + 'config.changed', + 'update-status', + 'upgrade-charm', + ] + hook_set = { + 'when': { + 'publish_bgp_info': ('endpoint.bgp-speaker.changed',), + 'setup_amqp_req': ('amqp.connected', ), + 'render_configs': ('amqp.available', ), + 'configure_ssl': ('amqp.available.ssl', ), + }, } - when_not_patterns = {} - # 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(), - "{} not found".format(f)) - # check that the lists are equal - li = [] - for a in args: - li += a['args'][:] - self.assertEqual(sorted(li), sorted(p[f]), - "{}: incorrect state registration".format(f)) + self.registered_hooks_test_helper(handlers, hook_set, defaults) + + +class TestDRAgentHandlers(test_utils.PatchHelper): + + def setUp(self): + super().setUp() + self.patch_release(dragent.DRAgentCharm.release) + self.dragent_charm = mock.MagicMock() + self.patch_object(handlers.charm, 'provide_charm_instance', + new=mock.MagicMock()) + self.provide_charm_instance().__enter__.return_value = \ + self.dragent_charm + self.provide_charm_instance().__exit__.return_value = None def test_publish_bgp_info(self): _bindings = ['bgp-speaker'] - self.patch(handlers.dragent, 'assess_status') - self.patch(handlers.dragent, 'get_os_codename') bgp = mock.MagicMock() - self.get_os_codename.return_value = 'queens' + self.dragent_charm.bgp_speaker_bindings.return_value = _bindings handlers.publish_bgp_info(bgp) - self.get_os_codename.assert_called() + self.dragent_charm.get_os_codename.assert_called() bgp.publish_info.assert_called_once_with(passive=True, bindings=_bindings, use_16bit_asn=False) def test_publish_bgp_info_pike(self): _bindings = ['bgp-speaker'] - self.patch(handlers.dragent, 'assess_status') - self.patch(handlers.dragent, 'get_os_codename') bgp = mock.MagicMock() - self.get_os_codename.return_value = 'pike' + self.dragent_charm.get_os_codename.return_value = 'pike' + self.dragent_charm.bgp_speaker_bindings.return_value = _bindings handlers.publish_bgp_info(bgp) - self.get_os_codename.assert_called() + self.dragent_charm.get_os_codename.assert_called() bgp.publish_info.assert_called_once_with(passive=True, bindings=_bindings, use_16bit_asn=True) def test_setup_amqp_req(self): - self.patch(handlers.dragent, 'assess_status') amqp = mock.MagicMock() handlers.setup_amqp_req(amqp) amqp.request_access.assert_called_once_with( username='neutron', vhost='openstack') def test_render_configs(self): - self.patch(handlers.dragent, 'render_configs') - self.patch(handlers.dragent, 'assess_status') - self.patch(handlers.dragent, 'upgrade_if_available') + self.patch_object(handlers.reactive, 'set_flag') amqp = mock.MagicMock() handlers.render_configs(amqp) - self.upgrade_if_available.assert_called_once_with((amqp,)) - self.render_configs.assert_called_once_with((amqp,)) - self.assess_status.assert_called_once() + self.dragent_charm.upgrade_if_available.assert_called_once_with( + (amqp,)) + self.dragent_charm.render_with_interfaces.assert_called_once_with( + (amqp,)) + self.dragent_charm.assess_status.assert_called_once() diff --git a/unit_tests/test_lib_charm_openstack_dragent.py b/unit_tests/test_lib_charm_openstack_dragent.py index 8be7003..7aa5913 100644 --- a/unit_tests/test_lib_charm_openstack_dragent.py +++ b/unit_tests/test_lib_charm_openstack_dragent.py @@ -31,17 +31,6 @@ class Helper(test_utils.PatchHelper): class TestOpenStackDRAgent(Helper): - def test_render_configs(self): - self.patch_object(dragent.DRAgentCharm.singleton, - "render_with_interfaces") - dragent.render_configs("interfaces-list") - self.render_with_interfaces.assert_called_once_with( - "interfaces-list") - - def test_bgp_speaker_bindings(self): - self.assertEqual(dragent.bgp_speaker_bindings(), - [self.SPEAKER_BINDING]) - def get_os_codename(self): self.patch_object(dragent.DRAgentCharm.singleton, "get_os_codename_package") @@ -78,3 +67,8 @@ class TestDRAgentCharm(Helper): dra.install() self.configure_source.assert_called_once_with() self.install.assert_called_once_with() + + def test_bgp_speaker_bindings(self): + dra = dragent.DRAgentCharm() + self.assertEqual(dra.bgp_speaker_bindings(), + [dragent.SPEAKER_BINDING])