Skip to content
This repository has been archived by the owner on Apr 2, 2024. It is now read-only.

Commit

Permalink
Fix application_name = None when single cluster relation (#243)
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
rodrigogansobarbieri committed Jan 19, 2023
1 parent 9aadb59 commit 1db5d0a
Show file tree
Hide file tree
Showing 2 changed files with 370 additions and 2 deletions.
8 changes: 6 additions & 2 deletions charms/reactive/endpoints.py
Expand Up @@ -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
Expand Down
364 changes: 364 additions & 0 deletions tests/test_endpoints.py
Expand Up @@ -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',
])

0 comments on commit 1db5d0a

Please sign in to comment.