From 9ad5cade97e14b672ac859d34040353c3a40c7aa Mon Sep 17 00:00:00 2001 From: James Page Date: Wed, 7 Sep 2022 11:27:47 +0100 Subject: [PATCH] Refactor to use Endpoint base class The RelationBase class has been deprecated for some time and provides no support for interaction with the application data bag. Migrate requires interface to Endpoint base class and refactor as needed. Change-Id: I82fe7df6c7c3658dd334a830442f1dcbd1e7d7e4 --- requires.py | 111 +++++++++----- test-requirements.txt | 1 + tox.ini | 5 + unit_tests/__init__.py | 18 +++ unit_tests/test_requires.py | 289 +++++++++++++++--------------------- 5 files changed, 217 insertions(+), 207 deletions(-) diff --git a/requires.py b/requires.py index 09f9b30..bfec631 100644 --- a/requires.py +++ b/requires.py @@ -14,18 +14,37 @@ import base64 import json -from charms.reactive import RelationBase -from charms.reactive import hook -from charms.reactive import scopes +import charms.reactive as reactive -class KeystoneRequires(RelationBase): - scope = scopes.GLOBAL +# NOTE: fork of relations.AutoAccessors for forwards compat behaviour +class KeystoneAutoAccessors(type): + """ + Metaclass that converts fields referenced by ``auto_accessors`` into + accessor methods with very basic doc strings. + """ + def __new__(cls, name, parents, dct): + for field in dct.get('auto_accessors', []): + meth_name = field.replace('-', '_') + meth = cls._accessor(field) + meth.__name__ = meth_name + meth.__module__ = dct.get('__module__') + meth.__doc__ = 'Get the %s, if available, or None.' % field + dct[meth_name] = meth + return super(KeystoneAutoAccessors, cls).__new__( + cls, name, parents, dct + ) - # These remote data fields will be automatically mapped to accessors - # with a basic documentation string provided. + @staticmethod + def _accessor(field): + def __accessor(self): + return self.all_joined_units.received.get(field) + return __accessor - auto_accessors = ['private-address', 'service_host', 'service_protocol', + +class KeystoneRequires(reactive.Endpoint, metaclass=KeystoneAutoAccessors): + + auto_accessors = ['service_host', 'service_protocol', 'service_port', 'service_tenant', 'service_username', 'service_password', 'service_tenant_id', 'auth_host', 'auth_protocol', 'auth_port', 'admin_token', 'ssl_key', @@ -37,40 +56,53 @@ class KeystoneRequires(RelationBase): 'admin_domain_id', 'admin_user_id', 'admin_project_id', 'service_type'] - @hook('{requires:keystone}-relation-joined') + @reactive.when('endpoint.{endpoint_name}.joined') def joined(self): - self.set_state('{relation_name}.connected') - self.update_state() + reactive.set_flag(self.expand_name('{endpoint_name}.connected')) - def update_state(self): - if self.base_data_complete(): - self.set_state('{relation_name}.available') - self.set_state('{relation_name}.available.auth') - if self.ssl_data_complete(): - self.set_state('{relation_name}.available.ssl') - else: - self.remove_state('{relation_name}.available.ssl') - if self.ssl_data_complete_legacy(): - self.set_state('{relation_name}.available.ssl_legacy') - else: - self.remove_state('{relation_name}.available.ssl_legacy') - else: - self.remove_state('{relation_name}.available') - self.remove_state('{relation_name}.available.ssl') - self.remove_state('{relation_name}.available.ssl_legacy') - self.remove_state('{relation_name}.available.auth') - - @hook('{requires:keystone}-relation-changed') + @reactive.when('endpoint.{endpoint_name}.changed') def changed(self): - self.update_state() + self.update_flags() + reactive.clear_flag( + self.expand_name( + 'endpoint.{endpoint_name}.changed')) - @hook('{requires:keystone}-relation-{broken,departed}') + def update_flags(self): + if self.base_data_complete(): + reactive.set_flag(self.expand_name('{endpoint_name}.available')) + reactive.set_flag( + self.expand_name('{endpoint_name}.available.auth')) + if self.ssl_data_complete(): + reactive.set_flag( + self.expand_name('{endpoint_name}.available.ssl')) + else: + reactive.clear_flag( + self.expand_name('{endpoint_name}.available.ssl')) + if self.ssl_data_complete_legacy(): + reactive.set_flag( + self.expand_name('{endpoint_name}.available.ssl_legacy')) + else: + reactive.clear_flag( + self.expand_name('{endpoint_name}.available.ssl_legacy')) + else: + reactive.clear_flag( + self.expand_name('{endpoint_name}.available')) + reactive.clear_flag( + self.expand_name('{endpoint_name}.available.ssl')) + reactive.clear_flag( + self.expand_name('{endpoint_name}.available.ssl_legacy')) + reactive.clear_flag( + self.expand_name('{endpoint_name}.available.auth')) + + @reactive.when('endpoint.{endpoint_name}.departed') def departed(self): - self.update_state() + self.update_flags() + reactive.clear_flag( + self.expand_name( + 'endpoint.{endpoint_name}.departed')) def base_data_complete(self): data = { - 'private-address': self.private_address(), 'service_host': self.service_host(), 'service_protocol': self.service_protocol(), 'service_port': self.service_port(), @@ -131,8 +163,8 @@ class KeystoneRequires(RelationBase): if add_role_to_admin: relation_info.update( {'add_role_to_admin': ','.join(add_role_to_admin)}) - self.set_local(**relation_info) - self.set_remote(**relation_info) + for relation in self.relations: + relation.to_publish_raw.update(relation_info) def request_keystone_endpoint_information(self): self.register_endpoints('None', 'None', 'None', 'None', 'None') @@ -147,18 +179,19 @@ class KeystoneRequires(RelationBase): relation_info = { "subscribe_ep_change": " ".join(services), } - self.set_remote(**relation_info) + for relation in self.relations: + relation.to_publish_raw.update(relation_info) def get_ssl_key(self, cn=None): relation_key = 'ssl_key_{}'.format(cn) if cn else 'ssl_key' - key = self.get_remote(relation_key) + key = self.all_joined_units.received.get(relation_key) if key: key = base64.b64decode(key).decode('utf-8') return key def get_ssl_cert(self, cn=None): relation_key = 'ssl_cert_{}'.format(cn) if cn else 'ssl_cert' - cert = self.get_remote(relation_key) + cert = self.all_joined_units.received.get(relation_key) if cert: cert = base64.b64decode(cert).decode('utf-8') return cert diff --git a/test-requirements.txt b/test-requirements.txt index c805e5d..993e44a 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -3,3 +3,4 @@ flake8 stestr>=2.2.0 charms.reactive coverage>=3.6 +git+https://github.com/openstack/charms.openstack.git#egg=charms.openstack diff --git a/tox.ini b/tox.ini index 3dec020..4d2b254 100644 --- a/tox.ini +++ b/tox.ini @@ -33,6 +33,11 @@ basepython = python3.8 deps = -r{toxinidir}/test-requirements.txt commands = stestr run {posargs} +[testenv:py310] +basepython = python3.10 +deps = -r{toxinidir}/test-requirements.txt +commands = stestr run {posargs} + [testenv:py3] basepython = python3 deps = -r{toxinidir}/test-requirements.txt diff --git a/unit_tests/__init__.py b/unit_tests/__init__.py index e69de29..02e5ab1 100644 --- a/unit_tests/__init__.py +++ b/unit_tests/__init__.py @@ -0,0 +1,18 @@ +# Copyright 2022 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. + + +# Mock out charmhelpers so that we can test without it. +import charms_openstack.test_mocks # noqa +charms_openstack.test_mocks.mock_charmhelpers() diff --git a/unit_tests/test_requires.py b/unit_tests/test_requires.py index 2dd73bb..8ce85a3 100644 --- a/unit_tests/test_requires.py +++ b/unit_tests/test_requires.py @@ -11,200 +11,151 @@ # limitations under the License. -import unittest from unittest import mock import requires +import charms_openstack.test_utils as test_utils _hook_args = {} -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 TestKeystoneRequires(unittest.TestCase): - - @classmethod - def setUpClass(cls): - cls._patched_hook = mock.patch('charms.reactive.hook', 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) +class TestKeystoneRequires(test_utils.PatchHelper): def setUp(self): - self.kr = requires.KeystoneRequires('some-relation', []) + self.target = requires.KeystoneRequires('some-relation', []) self._patches = {} self._patches_start = {} def tearDown(self): - self.kr = None + self.target = 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.kr, attr) + def patch_target(self, attr, return_value=None): + mocked = mock.patch.object(self.target, 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. - hook_patterns = { - 'joined': ('{requires:keystone}-relation-joined', ), - 'changed': ('{requires:keystone}-relation-changed', ), - 'departed': ('{requires:keystone}-relation-{broken,departed}', ), - } - for k, v in _hook_args.items(): - self.assertEqual(hook_patterns[k], v['args']) - - def test_changed(self): - self.patch_kr('update_state') - self.kr.changed() - self.update_state.assert_called_once_with() - def test_joined(self): - self.patch_kr('update_state') - self.patch_kr('set_state') - self.kr.joined() - self.set_state.assert_called_once_with('{relation_name}.connected') - self.update_state.assert_called_once_with() + self.patch_object(requires.reactive, 'set_flag') + self.target.joined() + self.set_flag.assert_called_once_with('some-relation.connected') def test_departed(self): - self.patch_kr('update_state') - self.kr.departed() - self.update_state.assert_called_once_with() + self.patch_object(requires.reactive, 'clear_flag') + self.patch_target('update_flags') + self.target.departed() + self.clear_flag.assert_has_calls([ + mock.call('endpoint.some-relation.departed') + ]) + self.update_flags.assert_called_once_with() def test_base_data_complete(self): - self.patch_kr('private_address', '1') - self.patch_kr('service_host', '2') - self.patch_kr('service_protocol', '3') - self.patch_kr('service_port', '4') - self.patch_kr('auth_host', '5') - self.patch_kr('auth_protocol', '6') - self.patch_kr('auth_port', '7') - self.patch_kr('service_tenant', '1') - self.patch_kr('service_username', '2') - self.patch_kr('service_password', '3') - self.patch_kr('service_tenant_id', '4') - self.patch_kr('service_type', 'identity') - assert self.kr.base_data_complete() is True + self.patch_target('service_host', '2') + self.patch_target('service_protocol', '3') + self.patch_target('service_port', '4') + self.patch_target('auth_host', '5') + self.patch_target('auth_protocol', '6') + self.patch_target('auth_port', '7') + self.patch_target('service_tenant', '1') + self.patch_target('service_username', '2') + self.patch_target('service_password', '3') + self.patch_target('service_tenant_id', '4') + self.patch_target('service_type', 'identity') + assert self.target.base_data_complete() is True self.service_tenant.return_value = None - assert self.kr.base_data_complete() is False + assert self.target.base_data_complete() is False def test_ssl_data_complete(self): - self.patch_kr('ssl_cert_admin', '1') - self.patch_kr('ssl_cert_internal', '2') - self.patch_kr('ssl_cert_public', '3') - self.patch_kr('ssl_key_admin', '4') - self.patch_kr('ssl_key_internal', '5') - self.patch_kr('ssl_key_public', '6') - self.patch_kr('ca_cert', '7') - assert self.kr.ssl_data_complete() is True + self.patch_target('ssl_cert_admin', '1') + self.patch_target('ssl_cert_internal', '2') + self.patch_target('ssl_cert_public', '3') + self.patch_target('ssl_key_admin', '4') + self.patch_target('ssl_key_internal', '5') + self.patch_target('ssl_key_public', '6') + self.patch_target('ca_cert', '7') + assert self.target.ssl_data_complete() is True self.ca_cert.return_value = None - assert self.kr.ssl_data_complete() is False + assert self.target.ssl_data_complete() is False self.ca_cert.return_value = '7' self.ssl_key_public.return_value = '__null__' - assert self.kr.ssl_data_complete() is False + assert self.target.ssl_data_complete() is False def test_ssl_data_complete_legacy(self): - self.patch_kr('ssl_key', '1') - self.patch_kr('ssl_cert', '2') - self.patch_kr('ca_cert', '3') - assert self.kr.ssl_data_complete_legacy() is True + self.patch_target('ssl_key', '1') + self.patch_target('ssl_cert', '2') + self.patch_target('ca_cert', '3') + assert self.target.ssl_data_complete_legacy() is True self.ca_cert.return_value = None - assert self.kr.ssl_data_complete_legacy() is False + assert self.target.ssl_data_complete_legacy() is False self.ca_cert.return_value = '3' self.ssl_key.return_value = '__null__' - assert self.kr.ssl_data_complete_legacy() is False + assert self.target.ssl_data_complete_legacy() is False - def test_update_state(self): - self.patch_kr('base_data_complete', False) - self.patch_kr('ssl_data_complete', False) - self.patch_kr('ssl_data_complete_legacy', False) - self.patch_kr('set_state') - self.patch_kr('remove_state') + def test_changed(self): + self.patch_target('base_data_complete', False) + self.patch_target('ssl_data_complete', False) + self.patch_target('ssl_data_complete_legacy', False) + self.patch_object(requires.reactive, 'set_flag') + self.patch_object(requires.reactive, 'clear_flag') # test when not all base data is available. - self.kr.update_state() - self.remove_state.assert_any_call('{relation_name}.available') - self.remove_state.assert_any_call('{relation_name}.available.ssl') - self.remove_state.assert_any_call( - '{relation_name}.available.ssl_legacy') - self.remove_state.assert_any_call('{relation_name}.available.auth') - self.set_state.assert_not_called() - self.remove_state.reset_mock() + self.target.changed() + self.clear_flag.assert_any_call('some-relation.available') + self.clear_flag.assert_any_call('some-relation.available.ssl') + self.clear_flag.assert_any_call( + 'some-relation.available.ssl_legacy') + self.clear_flag.assert_any_call('some-relation.available.auth') + self.set_flag.assert_not_called() + self.clear_flag.assert_any_call( + 'endpoint.some-relation.changed') + self.clear_flag.reset_mock() # test when just the base data is available. self.base_data_complete.return_value = True - self.kr.update_state() - self.set_state.assert_any_call('{relation_name}.available') - self.set_state.assert_any_call('{relation_name}.available.auth') - self.remove_state.assert_any_call('{relation_name}.available.ssl') - self.remove_state.assert_any_call( - '{relation_name}.available.ssl_legacy') - self.set_state.reset_mock() - self.remove_state.reset_mock() + self.target.changed() + self.set_flag.assert_any_call('some-relation.available') + self.set_flag.assert_any_call('some-relation.available.auth') + self.clear_flag.assert_any_call('some-relation.available.ssl') + self.clear_flag.assert_any_call( + 'some-relation.available.ssl_legacy') + self.clear_flag.assert_any_call( + 'endpoint.some-relation.changed') + self.set_flag.reset_mock() + self.clear_flag.reset_mock() # test ssl_data_complete self.ssl_data_complete.return_value = True - self.kr.update_state() - self.set_state.assert_any_call('{relation_name}.available') - self.set_state.assert_any_call('{relation_name}.available.ssl') - self.remove_state.assert_any_call( - '{relation_name}.available.ssl_legacy') - self.set_state.reset_mock() - self.remove_state.reset_mock() + self.target.changed() + self.set_flag.assert_any_call('some-relation.available') + self.set_flag.assert_any_call('some-relation.available.auth') + self.set_flag.assert_any_call('some-relation.available.ssl') + self.clear_flag.assert_any_call( + 'some-relation.available.ssl_legacy') + self.clear_flag.assert_any_call( + 'endpoint.some-relation.changed') + self.set_flag.reset_mock() + self.clear_flag.reset_mock() # test ssl_data_complete_legacy self.ssl_data_complete_legacy.return_value = True - self.kr.update_state() - self.set_state.assert_any_call('{relation_name}.available') - self.set_state.assert_any_call('{relation_name}.available.ssl') - self.set_state.assert_any_call( - '{relation_name}.available.ssl_legacy') - self.set_state.reset_mock() - self.remove_state.reset_mock() - self.kr.update_state() - self.set_state.assert_any_call('{relation_name}.available') - self.set_state.assert_any_call('{relation_name}.available.ssl') - self.set_state.assert_any_call( - '{relation_name}.available.ssl_legacy') - self.set_state.assert_any_call('{relation_name}.available.auth') - self.remove_state.assert_not_called() + self.target.changed() + self.set_flag.assert_any_call('some-relation.available') + self.set_flag.assert_any_call('some-relation.available.auth') + self.set_flag.assert_any_call('some-relation.available.ssl') + self.set_flag.assert_any_call( + 'some-relation.available.ssl_legacy') + self.clear_flag.assert_any_call( + 'endpoint.some-relation.changed') def test_register_endpoints(self): - self.patch_kr('set_local') - self.patch_kr('set_remote') - self.kr.register_endpoints('s', 'r', 'p_url', 'i_url', 'a_url') + relation = mock.MagicMock() + self.patch_target('_relations') + self._relations.__iter__.return_value = [relation] + self.target.register_endpoints('s', 'r', 'p_url', 'i_url', 'a_url') result = { 'service': 's', 'public_url': 'p_url', @@ -212,14 +163,15 @@ class TestKeystoneRequires(unittest.TestCase): 'admin_url': 'a_url', 'region': 'r', } - self.set_local.assert_called_once_with(**result) - self.set_remote.assert_called_once_with(**result) + relation.to_publish_raw.update.assert_called_once_with(result) def test_register_endpoints_requested_roles(self): - self.patch_kr('set_local') - self.patch_kr('set_remote') - self.kr.register_endpoints('s', 'r', 'p_url', 'i_url', 'a_url', - requested_roles=['role1', 'role2']) + relation = mock.MagicMock() + self.patch_target('_relations') + self._relations.__iter__.return_value = [relation] + self.target.register_endpoints( + 's', 'r', 'p_url', 'i_url', 'a_url', + requested_roles=['role1', 'role2']) result = { 'service': 's', 'public_url': 'p_url', @@ -228,15 +180,15 @@ class TestKeystoneRequires(unittest.TestCase): 'region': 'r', 'requested_roles': 'role1,role2', } - self.set_local.assert_called_once_with(**result) - self.set_remote.assert_called_once_with(**result) + relation.to_publish_raw.update.assert_called_once_with(result) def test_register_endpoints_add_role_to_admin(self): - self.patch_kr('set_local') - self.patch_kr('set_remote') - self.kr.register_endpoints('s', 'r', 'p_url', 'i_url', 'a_url', - requested_roles=['role1', 'role2'], - add_role_to_admin=['grole1', 'grole2']) + relation = mock.MagicMock() + self.patch_target('_relations') + self._relations.__iter__.return_value = [relation] + self.target.register_endpoints('s', 'r', 'p_url', 'i_url', 'a_url', + requested_roles=['role1', 'role2'], + add_role_to_admin=['grole1', 'grole2']) result = { 'service': 's', 'public_url': 'p_url', @@ -246,12 +198,12 @@ class TestKeystoneRequires(unittest.TestCase): 'requested_roles': 'role1,role2', 'add_role_to_admin': 'grole1,grole2', } - self.set_local.assert_called_once_with(**result) - self.set_remote.assert_called_once_with(**result) + relation.to_publish_raw.update.assert_called_once_with(result) def test_request_keystone_endpoint_information(self): - self.patch_kr('set_local') - self.patch_kr('set_remote') + relation = mock.MagicMock() + self.patch_target('_relations') + self._relations.__iter__.return_value = [relation] result = { 'service': 'None', 'public_url': 'None', @@ -259,25 +211,26 @@ class TestKeystoneRequires(unittest.TestCase): 'admin_url': 'None', 'region': 'None', } - self.kr.request_keystone_endpoint_information() - self.set_local.assert_called_once_with(**result) - self.set_remote.assert_called_once_with(**result) + self.target.request_keystone_endpoint_information() + relation.to_publish_raw.update.assert_called_once_with(result) def test_request_notification(self): - self.patch_kr('set_remote') + relation = mock.MagicMock() + self.patch_target('_relations') + self._relations.__iter__.return_value = [relation] result = { 'subscribe_ep_change': 'nova neutron' } - self.kr.request_notification(['nova', 'neutron']) - self.set_remote.assert_called_once_with(**result) + self.target.request_notification(['nova', 'neutron']) + relation.to_publish_raw.update.assert_called_once_with(result) def test_endpoint_checksums(self): - self.patch_kr('ep_changed') - self.kr.ep_changed.return_value = ( + self.patch_target('ep_changed') + self.target.ep_changed.return_value = ( '{"nova": "abxcxv", "neutron": "124252"}' ) result = { 'nova': 'abxcxv', 'neutron': '124252', } - self.assertEqual(self.kr.endpoint_checksums(), result) + self.assertEqual(self.target.endpoint_checksums(), result)