From feefb5061664f30368f5b57116e29600e0f1421e Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Fri, 27 Sep 2019 15:38:18 +0200 Subject: [PATCH] Move shared methods into shared library --- .travis.yml | 14 +++++ src/lib/ovsdb.py | 78 +++++++++++++++++++----- src/ovsdb-cluster/peers.py | 31 ++++++++++ src/ovsdb-cms/provides.py | 1 + src/ovsdb-cms/requires.py | 5 ++ src/ovsdb/provides.py | 1 + src/ovsdb/requires.py | 5 ++ unit_tests/test_lib_ovsdb.py | 52 +--------------- unit_tests/test_ovsdb_cluster_peers.py | 84 ++++++++++++++++++++++++++ 9 files changed, 206 insertions(+), 65 deletions(-) create mode 100644 .travis.yml create mode 100644 unit_tests/test_ovsdb_cluster_peers.py diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 0000000..7534783 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,14 @@ +sudo: true +dist: xenial +language: python +install: pip install tox-travis +matrix: + include: + - name: "Python 3.6" + python: 3.6 + env: ENV=pep8,py3 + - name: "Python 3.7" + python: 3.7 + env: ENV=pep8,py3 +script: + - tox -c tox.ini -e $ENV diff --git a/src/lib/ovsdb.py b/src/lib/ovsdb.py index 156cd94..66c0c15 100644 --- a/src/lib/ovsdb.py +++ b/src/lib/ovsdb.py @@ -27,6 +27,24 @@ import charms.reactive as reactive class OVSDB(reactive.Endpoint): + DB_NB_PORT = 6641 + DB_SB_PORT = 6642 + + def _format_addr(self, addr): + """Validate and format IP address + + :param addr: IPv6 or IPv4 address + :type addr: str + :returns: Address string, optionally encapsulated in brackets ([]) + :rtype: str + :raises: ValueError + """ + ipaddr = ipaddress.ip_address(addr) + if isinstance(ipaddr, ipaddress.IPv6Address): + fmt = '[{}]' + else: + fmt = '{}' + return fmt.format(ipaddr) @property def cluster_local_addr(self): @@ -36,25 +54,59 @@ class OVSDB(reactive.Endpoint): relation_id=relation.relation_id) for interface in ng_data.get('bind-addresses', []): for addr in interface.get('addresses', []): - return addr['address'] + return self._format_addr(addr['address']) @property - def cluster_addrs(self): + def cluster_remote_addrs(self): for relation in self.relations: for unit in relation.units: try: - addr = ipaddress.ip_address( + addr = self._format_addr( unit.received.get('bound-address', '')) + yield addr except ValueError: continue - if isinstance(addr, ipaddress.IPv6Address): - yield '[{}]'.format(addr) - else: - yield '{}'.format(addr) - def expected_peers_available(self): + @property + def db_nb_port(self): + return self.DB_NB_PORT + + @property + def db_sb_port(self): + return self.DB_SB_PORT + + def db_connection_strs(self, addrs, port, proto='ssl'): + """Provide connection strings + + :param port: Port number + :type port: int + :param proto: Protocol + :type proto: str + :returns: List of connection strings + :rtype: Generator[str, None, None] + """ + for addr in addrs: + yield ':'.join((proto, addr, str(port))) + + @property + def db_nb_connection_strs(self): + return self.db_connection_strs(self.cluster_remote_addrs, + self.db_nb_port) + + @property + def db_sb_connection_strs(self): + return self.db_connection_strs(self.cluster_remote_addrs, + self.db_sb_port) + + def expected_units_available(self): + """Whether expected units have joined and published data on a relation + + NOTE: This does not work for the peer relation, see separate method + for that in the peer relation implementation. + """ if len(self.all_joined_units) == len( - list(ch_core.hookenv.expected_peer_units())): + list(ch_core.hookenv.expected_related_units( + self.expand_name('{endpoint_name}')))): for relation in self.relations: for unit in relation.units: if not unit.received.get('bound-address'): @@ -66,14 +118,15 @@ class OVSDB(reactive.Endpoint): return True return False - def publish_cluster_local_addr(self): + def publish_cluster_local_addr(self, addr=None): """Announce the address we bound our OVSDB Servers to. This will be used by our peers and clients to build a connection string to the remote cluster. """ for relation in self.relations: - relation.to_publish['bound-address'] = self.cluster_local_addr + relation.to_publish['bound-address'] = ( + addr or self.cluster_local_addr) def joined(self): ch_core.hookenv.log('{}: {} -> {}' @@ -82,9 +135,6 @@ class OVSDB(reactive.Endpoint): inspect.currentframe().f_code.co_name), level=ch_core.hookenv.INFO) reactive.set_flag(self.expand_name('{endpoint_name}.connected')) - self.publish_cluster_local_addr() - if self.expected_peers_available: - reactive.set_flag(self.expand_name('{endpoint_name}.available')) def broken(self): reactive.clear_flag(self.expand_name('{endpoint_name}.available')) diff --git a/src/ovsdb-cluster/peers.py b/src/ovsdb-cluster/peers.py index 2eef42d..3e056d9 100644 --- a/src/ovsdb-cluster/peers.py +++ b/src/ovsdb-cluster/peers.py @@ -12,6 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. +import charmhelpers.core as ch_core + +import charms.reactive as reactive + # the reactive framework unfortunately does not grok `import as` in conjunction # with decorators on class instance methods, so we have to revert to `from ...` # imports @@ -23,10 +27,37 @@ from .lib import ovsdb as ovsdb class OVSDBClusterPeer(ovsdb.OVSDB): + DB_NB_CLUSTER_PORT = 6643 + DB_SB_CLUSTER_PORT = 6644 + + @property + def db_nb_cluster_port(self): + return self.DB_NB_CLUSTER_PORT + + @property + def db_sb_cluster_port(self): + return self.DB_SB_CLUSTER_PORT + + def expected_peers_available(self): + if len(self.all_joined_units) == len( + list(ch_core.hookenv.expected_peer_units())): + for relation in self.relations: + for unit in relation.units: + if not unit.received.get('bound-address'): + break + else: + continue + break + else: + return True + return False @when('endpoint.{endpoint_name}.joined') def joined(self): super().joined() + self.publish_cluster_local_addr() + if self.expected_peers_available: + reactive.set_flag(self.expand_name('{endpoint_name}.available')) @when('endpoint.{endpoint_name}.broken') def broken(self): diff --git a/src/ovsdb-cms/provides.py b/src/ovsdb-cms/provides.py index b27dc26..ab1742c 100644 --- a/src/ovsdb-cms/provides.py +++ b/src/ovsdb-cms/provides.py @@ -36,6 +36,7 @@ class OVSDBCMSProvides(ovsdb.OVSDB): type(self).__name__, inspect.currentframe().f_code.co_name), level=ch_core.hookenv.INFO) + super().joined() @when('endpoint.{endpoint_name}.broken') def broken(self): diff --git a/src/ovsdb-cms/requires.py b/src/ovsdb-cms/requires.py index 3e13ae8..141af4f 100644 --- a/src/ovsdb-cms/requires.py +++ b/src/ovsdb-cms/requires.py @@ -20,6 +20,8 @@ import inspect import charmhelpers.core as ch_core +import charms.reactive as reactive + from charms.reactive import ( when, ) @@ -36,6 +38,9 @@ class OVSDBCMSRequires(ovsdb.OVSDB): type(self).__name__, inspect.currentframe().f_code.co_name), level=ch_core.hookenv.INFO) + super().joined() + if self.expected_units_available(): + reactive.set_flag(self.expand_name('{endpoint_name}.available')) @when('endpoint.{endpoint_name}.broken') def broken(self): diff --git a/src/ovsdb/provides.py b/src/ovsdb/provides.py index ffeaef9..0536ac7 100644 --- a/src/ovsdb/provides.py +++ b/src/ovsdb/provides.py @@ -36,6 +36,7 @@ class OVSDBProvides(ovsdb.OVSDB): type(self).__name__, inspect.currentframe().f_code.co_name), level=ch_core.hookenv.INFO) + super().joined() @when('endpoint.{endpoint_name}.broken') def broken(self): diff --git a/src/ovsdb/requires.py b/src/ovsdb/requires.py index 11d0718..b391b2a 100644 --- a/src/ovsdb/requires.py +++ b/src/ovsdb/requires.py @@ -20,6 +20,8 @@ import inspect import charmhelpers.core as ch_core +import charms.reactive as reactive + from charms.reactive import ( when, ) @@ -36,6 +38,9 @@ class OVSDBRequires(ovsdb.OVSDB): type(self).__name__, inspect.currentframe().f_code.co_name), level=ch_core.hookenv.INFO) + super().joined() + if self.expected_units_available(): + reactive.set_flag(self.expand_name('{endpoint_name}.available')) @when('endpoint.{endpoint_name}.broken') def broken(self): diff --git a/unit_tests/test_lib_ovsdb.py b/unit_tests/test_lib_ovsdb.py index ea02f56..685ace3 100644 --- a/unit_tests/test_lib_ovsdb.py +++ b/unit_tests/test_lib_ovsdb.py @@ -22,7 +22,7 @@ import charms_openstack.test_utils as test_utils _hook_args = {} -class TestNeutronLoadBalancerRequires(test_utils.PatchHelper): +class TestOVSDBLib(test_utils.PatchHelper): def setUp(self): super().setUp() @@ -83,44 +83,6 @@ class TestNeutronLoadBalancerRequires(test_utils.PatchHelper): self.network_get.assert_called_once_with( 'some-relation', relation_id='some-endpoint:42') - def test_cluster_addrs(self): - - def _assert_generator(iterable, expect): - for value in iterable: - self.assertEquals(value, expect) - - relation = mock.MagicMock() - relation.relation_id = 'some-endpoint:42' - unit = mock.MagicMock() - relation.units.__iter__.return_value = [unit] - self.patch_target('_relations') - self._relations.__iter__.return_value = [relation] - unit.received.get.return_value = '127.0.0.1' - _assert_generator(self.target.cluster_addrs, '127.0.0.1') - unit.received.get.assert_called_once_with('bound-address', '') - unit.received.get.return_value = '::1' - _assert_generator(self.target.cluster_addrs, '[::1]') - - def test_expected_peers_available(self): - # self.patch_target('_all_joined_units') - self.patch_object(ovsdb.ch_core.hookenv, 'expected_peer_units') - self.patch_target('_relations') - self.target._all_joined_units = ['aFakeUnit'] - self.expected_peer_units.return_value = ['aFakeUnit'] - relation = mock.MagicMock() - unit = mock.MagicMock() - relation.units.__iter__.return_value = [unit] - self._relations.__iter__.return_value = [relation] - unit.received.get.return_value = '127.0.0.1' - self.assertTrue(self.target.expected_peers_available()) - unit.received.get.assert_called_once_with('bound-address') - unit.received.get.return_value = '' - self.assertFalse(self.target.expected_peers_available()) - self.expected_peer_units.return_value = ['firstFakeUnit', - 'secondFakeUnit'] - unit.received.get.return_value = '127.0.0.1' - self.assertFalse(self.target.expected_peers_available()) - def test_publish_cluster_local_addr(self): to_publish = self.patch_topublish() self.target.publish_cluster_local_addr() @@ -128,20 +90,8 @@ class TestNeutronLoadBalancerRequires(test_utils.PatchHelper): def test_joined(self): self.patch_object(ovsdb.reactive, 'set_flag') - self.patch_target('publish_cluster_local_addr') - self.patch_target('expected_peers_available') - self.expected_peers_available.__bool__.return_value = False self.target.joined() - self.expected_peers_available.__bool__.assert_called_once_with() self.set_flag.assert_called_once_with('some-relation.connected') - self.publish_cluster_local_addr.assert_called_once_with() - self.set_flag.reset_mock() - self.expected_peers_available.__bool__.return_value = True - self.target.joined() - self.set_flag.assert_has_calls([ - mock.call('some-relation.connected'), - mock.call('some-relation.available'), - ]) def test_broken(self): self.patch_object(ovsdb.reactive, 'clear_flag') diff --git a/unit_tests/test_ovsdb_cluster_peers.py b/unit_tests/test_ovsdb_cluster_peers.py new file mode 100644 index 0000000..8f155d9 --- /dev/null +++ b/unit_tests/test_ovsdb_cluster_peers.py @@ -0,0 +1,84 @@ +# 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. + +# import mock + +# from x import peers + +# import charms_openstack.test_utils as test_utils + + +# _hook_args = {} + + +# class TestOVSDBPeers(test_utils.PatchHelper): + +# def setUp(self): +# super().setUp() +# self.target = ovsdb.OVSDB('some-relation', []) +# self._patches = {} +# self._patches_start = {} + +# def tearDown(self): +# 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_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_expected_peers_available(self): +# # self.patch_target('_all_joined_units') +# self.patch_object(ovsdb.ch_core.hookenv, 'expected_peer_units') +# self.patch_target('_relations') +# self.target._all_joined_units = ['aFakeUnit'] +# self.expected_peer_units.return_value = ['aFakeUnit'] +# relation = mock.MagicMock() +# unit = mock.MagicMock() +# relation.units.__iter__.return_value = [unit] +# self._relations.__iter__.return_value = [relation] +# unit.received.get.return_value = '127.0.0.1' +# self.assertTrue(self.target.expected_peers_available()) +# unit.received.get.assert_called_once_with('bound-address') +# unit.received.get.return_value = '' +# self.assertFalse(self.target.expected_peers_available()) +# self.expected_peer_units.return_value = ['firstFakeUnit', +# 'secondFakeUnit'] +# unit.received.get.return_value = '127.0.0.1' +# self.assertFalse(self.target.expected_peers_available()) + +# def test_joined(self): +# self.patch_object(ovsdb.reactive, 'set_flag') +# self.patch_target('publish_cluster_local_addr') +# self.patch_target('expected_peers_available') +# self.expected_peers_available.__bool__.return_value = False +# self.target.joined() +# self.expected_peers_available.__bool__.assert_called_once_with() +# self.set_flag.assert_called_once_with('some-relation.connected') +# self.publish_cluster_local_addr.assert_called_once_with() +# self.set_flag.reset_mock() +# self.expected_peers_available.__bool__.return_value = True +# self.target.joined() +# self.set_flag.assert_has_calls([ +# mock.call('some-relation.connected'), +# mock.call('some-relation.available'), +# ])