From 1db5d0ae59ea5e1673173a1c5ed649328156b2d1 Mon Sep 17 00:00:00 2001 From: Rodrigo Barbieri Date: Thu, 19 Jan 2023 10:04:28 -0300 Subject: [PATCH] Fix application_name = None when single cluster relation (#243) When a cluster relation only has one member, the self.application_name property used when reading application databag returns None during a window of opportunity that is repeated every time a new hook runs. Subsequent handlers eventually set this property but the first handler that runs causes issues (in vault charm for example, by failing to get the current certs and then assuming they don't exist, therefore recreating certs every time update-status runs). --- charms/reactive/endpoints.py | 8 +- tests/test_endpoints.py | 364 +++++++++++++++++++++++++++++++++++ 2 files changed, 370 insertions(+), 2 deletions(-) diff --git a/charms/reactive/endpoints.py b/charms/reactive/endpoints.py index f11fde3..ae32cc8 100644 --- a/charms/reactive/endpoints.py +++ b/charms/reactive/endpoints.py @@ -377,8 +377,12 @@ def application_name(self): relation.units[0].unit_name.split('/')[0] """ - if self._application_name is None and self.units: - self._application_name = self.units[0].unit_name.split('/')[0] + if self._application_name is None: + if self.units: + self._application_name = self.units[0].unit_name.split('/')[0] + else: + # NOTE(ganso): This is a single unit peer relation corner case + return hookenv.application_name() return self._application_name @property diff --git a/tests/test_endpoints.py b/tests/test_endpoints.py index edd33e2..3280ff3 100644 --- a/tests/test_endpoints.py +++ b/tests/test_endpoints.py @@ -566,3 +566,367 @@ def test_handlers(self): 'changed.foo: test-endpoint', 'changed.foo: test-endpoint2', ]) + + +class TestEndpointOneUnitPeer(unittest.TestCase): + def setUp(self): + tests_dir = Path(__file__).parent + + tf = tempfile.NamedTemporaryFile(delete=False) + tf.close() + self.test_db = Path(tf.name) + unitdata._KV = self.kv = unitdata.Storage(str(self.test_db)) + + self.log_p = mock.patch('charmhelpers.core.hookenv.log') + self.log_p.start() + + self.charm_dir = str(tests_dir / 'data') + self.charm_dir_p = mock.patch('charmhelpers.core.hookenv.charm_dir') + mcharm_dir = self.charm_dir_p.start() + mcharm_dir.side_effect = lambda: self.charm_dir + + self.hook_name = 'upgrade-charm' + self.hook_name_p = mock.patch('charmhelpers.core.hookenv.hook_name') + mhook_name = self.hook_name_p.start() + mhook_name.side_effect = lambda: self.hook_name + + self.local_unit_p = mock.patch('charmhelpers.core.hookenv.local_unit', + mock.MagicMock(return_value='local/0')) + self.local_unit_p.start() + + self.app_name_p = mock.patch('charmhelpers.core.hookenv.application_name', + mock.MagicMock(return_value='local')) + self.app_name_p.start() + + self.remote_unit = None + self.remote_unit_p = mock.patch('charmhelpers.core.hookenv.remote_unit') + mremote_unit = self.remote_unit_p.start() + mremote_unit.side_effect = lambda: self.remote_unit + + self.relation_id = None + self.relation_id_p = mock.patch('charmhelpers.core.hookenv.relation_id') + mrelation_id = self.relation_id_p.start() + mrelation_id.side_effect = lambda: self.relation_id + + self.relations = { + 'test-endpoint': [ + { + 'local': {'app-key': 'value'}, + 'local/0': {'key': 'value'}, + }, + ], + } + + def _rel(rid): + rn, ri = rid.split(':') + return self.relations[rn][int(ri)] + + def _rel_get(attribute=None, unit=None, rid=None, app=None): + data = _rel(rid)[unit or app] + if attribute is not None: + return data[attribute] + else: + return data + + self.rel_ids_p = mock.patch('charmhelpers.core.hookenv.relation_ids') + rel_ids_m = self.rel_ids_p.start() + rel_ids_m.side_effect = lambda endpoint: [ + '{}:{}'.format(endpoint, i) for i in range( + len(self.relations.get(endpoint, [])))] + self.rel_units_p = mock.patch('charmhelpers.core.hookenv.related_units') + rel_units_m = self.rel_units_p.start() + rel_units_m.side_effect = lambda rid: [ + key for key in _rel(rid).keys() + if ('/' in key and # exclude apps + not _rel(rid)[key].get('departed'))] + self.rel_get_p = mock.patch('charmhelpers.core.hookenv.relation_get') + rel_get_m = self.rel_get_p.start() + rel_get_m.side_effect = _rel_get + + self.rel_set_p = mock.patch('charmhelpers.core.hookenv.relation_set') + self.relation_set = self.rel_set_p.start() + + self.data_changed_p = mock.patch('charms.reactive.endpoints.data_changed') + self.data_changed = self.data_changed_p.start() + + self.atexit_p = mock.patch('charmhelpers.core.hookenv.atexit') + self.atexit = self.atexit_p.start() + + self.sysm_p = mock.patch.dict(sys.modules) + self.sysm_p.start() + + discover() + + def tearDown(self): + self.log_p.stop() + self.charm_dir_p.stop() + self.hook_name_p.stop() + self.local_unit_p.stop() + self.remote_unit_p.stop() + self.rel_ids_p.stop() + self.rel_units_p.stop() + self.rel_get_p.stop() + self.rel_set_p.stop() + self.data_changed_p.stop() + self.atexit_p.stop() + self.test_db.unlink() + self.sysm_p.stop() + Endpoint._endpoints.clear() + Handler._HANDLERS.clear() + + def test_from_name(self): + Endpoint._endpoints['foo'] = endpoint = Endpoint('foo') + + self.assertIs(Endpoint.from_name('foo'), endpoint) + self.assertIsNone(Endpoint.from_name('bar')) + + def test_from_flag(self): + Endpoint._endpoints['foo'] = endpoint = Endpoint('foo') + + self.assertIsNone(Endpoint.from_flag('foo')) + self.assertIsNone(Endpoint.from_flag('bar.qux.zod')) + + # should return None for unset flag + self.assertIsNone(Endpoint.from_flag('endpoint.foo.qux')) + + # once flag is set, should return the endpoint + set_flag('endpoint.foo.qux') + self.assertIs(Endpoint.from_flag('endpoint.foo.qux'), endpoint) + + set_flag('foo.qux') + self.assertIs(Endpoint.from_flag('foo.qux'), endpoint) + + def test_startup(self): + assert not is_flag_set('endpoint.test-endpoint.joined') + assert not is_flag_set('endpoint.test-endpoint.changed') + + def _register_triggers(self): + joined_flag = self.expand_name('endpoint.{endpoint_name}.joined') + alias_joined_flag = self.expand_name('alias.{endpoint_name}.joined') + register_trigger(when=joined_flag, set_flag=alias_joined_flag) + register_trigger(when_not=joined_flag, clear_flag=alias_joined_flag) + + self.data_changed.return_value = True + with mock.patch.object(Endpoint, 'register_triggers', + _register_triggers): + Endpoint._startup() + assert Endpoint.from_name('test-endpoint') is not None + assert Endpoint.from_name('test-endpoint').endpoint_name == 'test-endpoint' + assert Endpoint.from_name('test-endpoint').is_joined + assert Endpoint.from_name('test-endpoint').joined # deprecated + assert is_flag_set('endpoint.test-endpoint.joined') + assert is_flag_set('endpoint.test-endpoint.changed') + assert is_flag_set('alias.test-endpoint.joined') + self.assertEqual(self.atexit.call_args_list, [ + mock.call(Endpoint.from_name('test-endpoint').relations[0]._flush_data), + ]) + + # already joined, not relation hook + clear_flag('endpoint.test-endpoint.changed') + Endpoint._startup() + assert not is_flag_set('endpoint.test-endpoint.changed') + + # relation hook + self.hook_name = 'test-endpoint-relation-joined' + clear_flag('endpoint.test-endpoint.changed') + Endpoint._startup() + assert is_flag_set('endpoint.test-endpoint.changed') + + # not already joined + self.hook_name = 'upgrade-charm' + clear_flag('endpoint.test-endpoint.joined') + clear_flag('endpoint.test-endpoint.changed') + Endpoint._startup() + assert is_flag_set('endpoint.test-endpoint.changed') + + # data not changed + self.data_changed.return_value = False + clear_flag('endpoint.test-endpoint.joined') + clear_flag('endpoint.test-endpoint.changed') + Endpoint._startup() + assert not is_flag_set('endpoint.test-endpoint.changed') + + def test_collections(self): + Endpoint._startup() + tep = Endpoint.from_name('test-endpoint') + + self.assertEqual(len(tep.relations), 1) + self.assertEqual(len(tep.relations[0].joined_units), 1) + self.assertEqual(tep.relations['test-endpoint:0'].relation_id, 'test-endpoint:0') + self.assertEqual(len(tep.all_joined_units), 1) + self.assertEqual([u.unit_name for r in tep.relations for u in r.joined_units], + ['local/0']) + self.assertEqual([u.unit_name for u in tep.all_joined_units], + ['local/0']) + self.assertEqual(tep.relations[0].joined_units['local/0'].unit_name, 'local/0') + self.assertEqual(tep.relations[0].relation_id, 'test-endpoint:0') + self.assertEqual(tep.relations[0].endpoint_name, 'test-endpoint') + self.assertEqual(tep.relations[0].application_name, 'local') + self.assertEqual(tep.relations[0].joined_units[0].unit_name, 'local/0') + self.assertEqual(tep.all_joined_units.keys(), ['local/0']) + self.assertEqual(tep.relations[0].joined_units.keys(), ['local/0']) + self.assertEqual(tep.relations.keys(), ['test-endpoint:0']) + self.assertIs(tep.all_units, tep.all_joined_units) # deprecated + self.assertIs(tep.relations[0].units, tep.relations[0].joined_units) # deprecated + + def test_receive(self): + Endpoint._startup() + tep = Endpoint.from_name('test-endpoint') + + self.assertEqual(tep.all_joined_units.received_raw, {'key': 'value'}) + self.assertEqual(tep.all_joined_units.received, {'key': 'value'}) + self.assertEqual(tep.relations[0].joined_units.received_raw, {'key': 'value'}) + self.assertEqual(tep.relations[0].joined_units.received, {'key': 'value'}) + self.assertEqual(tep.relations[0].joined_units[0].received_raw, {'key': 'value'}) + self.assertEqual(tep.relations[0].joined_units[0].received, {'key': 'value'}) + + assert not tep.all_joined_units.received_raw.writeable + assert not tep.all_joined_units.received.writeable + + with self.assertRaises(ValueError): + tep.all_joined_units.received_raw['foo'] = 'nope' + + with self.assertRaises(ValueError): + tep.relations[0].joined_units.received_raw['foo'] = 'nope' + + with self.assertRaises(ValueError): + tep.relations[0].joined_units[0].received_raw['foo'] = 'nope' + + with self.assertRaises(ValueError): + tep.all_joined_units.received['foo'] = 'nope' + + with self.assertRaises(ValueError): + tep.relations[0].joined_units.received['foo'] = 'nope' + + with self.assertRaises(ValueError): + tep.relations[0].joined_units[0].received['foo'] = 'nope' + + def test_receive_app(self): + Endpoint._startup() + tep = Endpoint.from_name('test-endpoint') + + self.assertEqual(tep.relations[0].received_app_raw, {'app-key': 'value'}) + self.assertEqual(tep.relations[0].received_app, {'app-key': 'value'}) + + assert not tep.relations[0].received_app_raw.writeable + assert not tep.relations[0].received_app.writeable + + with self.assertRaises(ValueError): + tep.relations[0].received_app_raw['simple'] = 'nope' + + with self.assertRaises(ValueError): + tep.relations[0].received_app['simple'] = 'nope' + + def test_to_publish(self): + Endpoint._startup() + tep = Endpoint.from_name('test-endpoint') + rel = tep.relations[0] + + self.assertEqual(rel.to_publish_raw, {'key': 'value'}) + rel._flush_data() + assert not self.relation_set.called + + rel.to_publish_raw['key'] = 'new-value' + rel._flush_data() + self.relation_set.assert_called_once_with('test-endpoint:0', {'key': 'new-value'}) + + self.relation_set.reset_mock() + rel.to_publish['key'] = {'new': 'complex'} + rel._flush_data() + self.relation_set.assert_called_once_with('test-endpoint:0', {'key': '{"new": "complex"}'}) + + rel.to_publish_raw.update({'key': 'new-new'}) + self.assertEqual(rel.to_publish_raw, {'key': 'new-new'}) + + rel.to_publish.update({'key': {'new': 'new'}}) + self.assertEqual(rel.to_publish_raw, {'key': '{"new": "new"}'}) + + assert 'foo' not in rel.to_publish + assert rel.to_publish.get('foo', 'one') == 'one' + assert 'foo' not in rel.to_publish + assert rel.to_publish.setdefault('foo', 'two') == 'two' + assert 'foo' in rel.to_publish + assert rel.to_publish['foo'] == 'two' + del rel.to_publish['foo'] + assert 'foo' not in rel.to_publish + with self.assertRaises(KeyError): + del rel.to_publish['foo'] + assert 'foo' not in rel.to_publish + assert rel.to_publish['foo'] is None + + def test_to_publish_app(self): + Endpoint._startup() + tep = Endpoint.from_name('test-endpoint') + rel = tep.relations[0] + + self.assertEqual(rel.to_publish_app_raw, {'app-key': 'value'}) + rel._flush_data() + assert not self.relation_set.called + + rel.to_publish_app_raw['app-key'] = 'new-value' + rel._flush_data() + self.relation_set.assert_called_once_with('test-endpoint:0', {'app-key': 'new-value'}, app=True) + + self.relation_set.reset_mock() + rel.to_publish_app['app-key'] = {'new': 'complex'} + rel._flush_data() + self.relation_set.assert_called_once_with('test-endpoint:0', {'app-key': '{"new": "complex"}'}, app=True) + + rel.to_publish_app.update({'app-key': 'new-new'}) + self.assertEqual(rel.to_publish_app, {'app-key': 'new-new'}) + + rel.to_publish_app.update({'app-key': {'new': 'new'}}) + self.assertEqual(rel.to_publish_app, {'app-key': {"new": "new"}}) + + assert 'foo' not in rel.to_publish + assert rel.to_publish.get('foo', 'one') == 'one' + assert 'foo' not in rel.to_publish + assert rel.to_publish.setdefault('foo', 'two') == 'two' + assert 'foo' in rel.to_publish + assert rel.to_publish['foo'] == 'two' + del rel.to_publish['foo'] + assert 'foo' not in rel.to_publish + with self.assertRaises(KeyError): + del rel.to_publish['foo'] + assert 'foo' not in rel.to_publish + assert rel.to_publish['foo'] is None + + def test_handlers(self): + Handler._HANDLERS = {k: h for k, h in Handler._HANDLERS.items() + if hasattr(h, '_action') and + h._action.__qualname__.startswith('TestAltRequires.')} + assert Handler._HANDLERS + preds = [h._predicates[0].args[0][0] for h in Handler.get_handlers()] + for pred in preds: + self.assertRegex(pred, r'^endpoint.test-endpoint.') + + self.data_changed.return_value = False + Endpoint._startup() + tep = Endpoint.from_name('test-endpoint') + + self.assertCountEqual(tep.invocations, []) + dispatch() + self.assertCountEqual(tep.invocations, [ + 'joined: test-endpoint', + ]) + + tep.invocations.clear() + clear_flag('endpoint.test-endpoint.joined') + clear_flag('endpoint.test-endpoint.changed') + self.data_changed.return_value = True + Endpoint._startup() + dispatch() + self.assertCountEqual(tep.invocations, [ + 'joined: test-endpoint', + 'changed: test-endpoint', + ]) + + tep.invocations.clear() + clear_flag('endpoint.test-endpoint.joined') + clear_flag('endpoint.test-endpoint.changed') + Endpoint._startup() + dispatch() + self.assertCountEqual(tep.invocations, [ + 'joined: test-endpoint', + 'changed: test-endpoint', + ])