canonical
Race condition: RuntimeError: unable to define an event with event_kind that overlaps with an existing type
I'm writing a test for my charm, and found a race condition while initializing the Harness - the SAME code and the SAME tests are providing the different results sometimes.
GLOB sdist-make: /home/ubuntu/charm-k8s-prometheus/setup.py
py37 inst-nodeps: /home/ubuntu/charm-k8s-prometheus/.tox/dist/charm-k8s-prometheus-0.1.0.zip
py37 installed: attrs==19.3.0,charm-k8s-prometheus @ file:///home/ubuntu/charm-k8s-prometheus/.tox/dist/charm-k8s-prometheus-0.1.0.zip,coverage==5.1,flake8==3.8.2,importlib-metadata==1.6.0,mccabe==0.6.1,more-itertools==8.3.0,packaging==20.4,pkg-resources==0.0.0,pluggy==0.13.1,py==1.8.1,pyaml==20.4.0,pycodestyle==2.6.0,pyflakes==2.2.0,pyparsing==2.4.7,pytest==5.4.2,pytest-cov==2.9.0,pytest-randomly==3.4.0,PyYAML==5.3.1,six==1.15.0,wcwidth==0.1.9,zipp==3.1.0
py37 runtests: PYTHONHASHSEED='1871937448'
py37 runtests: commands[0] | flake8 --config=.flake8
py37 runtests: commands[1] | pytest -c pytest.ini
================================================================== test session starts ===================================================================
platform linux -- Python 3.7.7, pytest-5.4.2, py-1.8.1, pluggy-0.13.1
Using --randomly-seed=1590779897
rootdir: /home/ubuntu/charm-k8s-prometheus, inifile: pytest.ini, testpaths: test
plugins: cov-2.9.0, randomly-3.4.0
collected 27 items
test/charm_test.py ... [ 11%]
test/adapters/framework_test.py ........ [ 40%]
test/domain_test.py ......... [ 74%]
test/adapters/k8s_test.py ....... [100%]
----------- coverage: platform linux, python 3.7.7-final-0 -----------
Name Stmts Miss Branch BrPart Cover
-------------------------------------------------------------
src/adapters/framework.py 60 6 6 0 91%
src/adapters/k8s.py 43 1 10 2 94%
src/charm.py 68 10 10 3 83%
src/domain.py 88 5 36 8 90%
src/exceptions.py 5 1 0 0 80%
src/interface_http.py 25 7 2 0 67%
-------------------------------------------------------------
TOTAL 289 30 64 13 87%
Coverage HTML written to dir coverage-report
=================================================================== 27 passed in 1.17s ===================================================================
________________________________________________________________________ summary _________________________________________________________________________
py37: commands succeeded
congratulations :)
ubuntu@node10:~/charm-k8s-prometheus$ tox -epy37
GLOB sdist-make: /home/ubuntu/charm-k8s-prometheus/setup.py
py37 inst-nodeps: /home/ubuntu/charm-k8s-prometheus/.tox/dist/charm-k8s-prometheus-0.1.0.zip
py37 installed: attrs==19.3.0,charm-k8s-prometheus @ file:///home/ubuntu/charm-k8s-prometheus/.tox/dist/charm-k8s-prometheus-0.1.0.zip,coverage==5.1,flake8==3.8.2,importlib-metadata==1.6.0,mccabe==0.6.1,more-itertools==8.3.0,packaging==20.4,pkg-resources==0.0.0,pluggy==0.13.1,py==1.8.1,pyaml==20.4.0,pycodestyle==2.6.0,pyflakes==2.2.0,pyparsing==2.4.7,pytest==5.4.2,pytest-cov==2.9.0,pytest-randomly==3.4.0,PyYAML==5.3.1,six==1.15.0,wcwidth==0.1.9,zipp==3.1.0
py37 runtests: PYTHONHASHSEED='803018218'
py37 runtests: commands[0] | flake8 --config=.flake8
py37 runtests: commands[1] | pytest -c pytest.ini
================================================================== test session starts ===================================================================
platform linux -- Python 3.7.7, pytest-5.4.2, py-1.8.1, pluggy-0.13.1
Using --randomly-seed=1590779901
rootdir: /home/ubuntu/charm-k8s-prometheus, inifile: pytest.ini, testpaths: test
plugins: cov-2.9.0, randomly-3.4.0
collected 27 items
test/adapters/framework_test.py ........ [ 29%]
test/domain_test.py ......... [ 62%]
test/adapters/k8s_test.py ....... [ 88%]
test/charm_test.py ..F [100%]
======================================================================== FAILURES ========================================================================
_____________________________________________ OnConfigChangedHandlerTest.test__it_blocks_until_pod_is_ready ______________________________________________
self = <charm_test.OnConfigChangedHandlerTest testMethod=test__it_blocks_until_pod_is_ready>
mock_pod_spec = <function set_juju_pod_spec at 0x7fb334d35f80>, mock_juju_pod_spec = <function build_juju_pod_spec at 0x7fb334d4c710>
mock_time = <NonCallableMagicMock name='time' spec_set='module' id='140407662251216'>
mock_k8s_mod = <NonCallableMagicMock name='k8s' spec_set='module' id='140407662251984'>
mock_build_juju_unit_status_func = <function build_juju_unit_status at 0x7fb334d5e050>
@patch('charm.build_juju_unit_status', spec_set=True, autospec=True)
@patch('charm.k8s', spec_set=True, autospec=True)
@patch('charm.time', spec_set=True, autospec=True)
@patch('charm.build_juju_pod_spec', spec_set=True, autospec=True)
@patch('charm.set_juju_pod_spec', spec_set=True, autospec=True)
def test__it_blocks_until_pod_is_ready(
self,
mock_pod_spec,
mock_juju_pod_spec,
mock_time,
mock_k8s_mod,
mock_build_juju_unit_status_func):
# Setup
mock_fw_adapter_cls = \
create_autospec(framework.FrameworkAdapter, spec_set=True)
mock_fw_adapter = mock_fw_adapter_cls.return_value
mock_juju_unit_states = [
MaintenanceStatus(str(uuid4())),
MaintenanceStatus(str(uuid4())),
ActiveStatus(str(uuid4())),
]
mock_build_juju_unit_status_func.side_effect = mock_juju_unit_states
mock_event_cls = create_autospec(EventBase, spec_set=True)
mock_event = mock_event_cls.return_value
harness = Harness(charm.Charm)
> harness.begin()
test/charm_test.py:93:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
lib/ops/testing.py:121: in begin
self._charm = TestCharm(self._framework, self._framework.meta.name)
src/charm.py:39: in __init__
super().__init__(*args)
lib/ops/charm.py:353: in __init__
self.on.define_event(relation_name + '_relation_created', RelationCreatedEvent)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
cls = <class 'ops.testing.Harness.begin.<locals>.TestEvents'>, event_kind = 'http_api_relation_created'
event_type = <class 'ops.charm.RelationCreatedEvent'>
@classmethod
def define_event(cls, event_kind, event_type):
"""Define an event on this type at runtime.
cls: a type to define an event on.
event_kind: an attribute name that will be used to access the
event. Must be a valid python identifier, not be a keyword
or an existing attribute.
event_type: a type of the event to define.
"""
prefix = 'unable to define an event with event_kind that '
if not event_kind.isidentifier():
raise RuntimeError(prefix + 'is not a valid python identifier: ' + event_kind)
elif keyword.iskeyword(event_kind):
raise RuntimeError(prefix + 'is a python keyword: ' + event_kind)
try:
getattr(cls, event_kind)
raise RuntimeError(
> prefix + 'overlaps with an existing type {} attribute: {}'.format(cls, event_kind))
E RuntimeError: unable to define an event with event_kind that overlaps with an existing type <class 'ops.testing.Harness.begin.<locals>.TestEvents'> attribute: http_api_relation_created
lib/ops/framework.py:322: RuntimeError
----------- coverage: platform linux, python 3.7.7-final-0 -----------
Name Stmts Miss Branch BrPart Cover
-------------------------------------------------------------
src/adapters/framework.py 60 6 6 0 91%
src/adapters/k8s.py 43 1 10 2 94%
src/charm.py 68 28 10 2 56%
src/domain.py 88 5 36 8 90%
src/exceptions.py 5 1 0 0 80%
src/interface_http.py 25 7 2 0 67%
-------------------------------------------------------------
TOTAL 289 48 64 12 81%
Coverage HTML written to dir coverage-report
================================================================ short test summary info =================================================================
FAILED test/charm_test.py::OnConfigChangedHandlerTest::test__it_blocks_until_pod_is_ready - RuntimeError: unable to define an event with event_kind tha...
============================================================== 1 failed, 26 passed in 1.15s ==============================================================
ERROR: InvocationError: '/home/ubuntu/charm-k8s-prometheus/.tox/py37/bin/pytest -c pytest.ini'
________________________________________________________________________ summary _________________________________________________________________________
ERROR: py37: commands failed
ubuntu@node10:~/charm-k8s-prometheus$
The framework version is the latest one available:
ubuntu@node10:~/charm-k8s-prometheus/mod/operator$ git rev-parse HEAD
81e5f36571bab0efe315254f60d5a5fcf2693c8e
Reproduces also with
harness = Harness(charm.Charm)
harness.begin()
harness.charm._stored.set_default(is_started=False)
harness.charm.on.config_changed.emit()
in the test code.
I'm not sure what 'charm.Charm' you're using here. Given the context, I was thinking it might be: https://github.com/exceptorr/charm-k8s-prometheus/blob/master/src/charm.py#L35
But that class doesn't have a _stored attribute. And doing it with my own code:
def test_with_stored_state(self):
class Charm(CharmBase):
_stored = StoredState()
harness = Harness(Charm)
harness.begin()
harness.charm._stored.set_default(is_started=False)
harness.charm.on.config_changed.emit()
Doesn't show any errors. I do have an idea that I'll explore.
My guess didn't work. I was wondering if you somehow ended up registering a StoredState for Charm twice (once for the actual Charm, and once for the wrapper around Charm that Harness creates.)
Given the sheer quantity of mocks in what I can read of your original test, it isn't clear to me that there isn't something in there that is creating a Framework and then registering things in it or not (even as an accidental side effect).
Note that it is during 'begin()' where it is failing, which doesn't have to do with 'config_changed.emit()' nor with StoredState, but just with instantiating Charm.
The actual error is because Charm.init is because it is trying to register the 'http_api_relation_created' event for the same class twice. Which sounds like you're sharing a Framework, even though Harness intentionally is creating its own Framework to use. (And it is being backed by sqlite ':memory:' which means it can't be sharing the backend with any other instance of Framework.)
I'm happy to dig further if you can create a reproducer that is independent of your charm code, or possibly even just point me to your actual charm that is failing.
@jameinel here's a reproducer: https://github.com/exceptorr/charm-k8s-prometheus/tree/prepare-for-config-dynamic-reload - just uncomment the test back https://github.com/exceptorr/charm-k8s-prometheus/commit/ef04be0b44154d09ed3989cd96a77f9334913230#diff-62dd0694ddd90672dc34aa34f68aa827R61 and that's it!
I was thinking it might be: https://github.com/exceptorr/charm-k8s-prometheus/blob/master/src/charm.py#L35 But that class doesn't have a _stored attribute
Yeah because you were looking at the master branch, while this code is in another branch: https://github.com/exceptorr/charm-k8s-prometheus/commit/ef04be0b44154d09ed3989cd96a77f9334913230#diff-38f277fc95ad7520237f91e48ec77e8dR38
So it is a case of some other part of the test suite leaving things in an unclean state. If I run just that test:
.tox/py36/bin/pytest -c pytest.ini
test/charm_test.py::OnConfigChangedHandlerTest
Then it fails with:
harness = Harness(charm.Charm)
harness.begin()
harness.charm._stored.set_default(is_started=False)
> harness.charm.on.config_changed.emit()
test/charm_test.py:101:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
lib/ops/framework.py:207: in emit
framework._emit(event)
lib/ops/framework.py:714: in _emit
self._reemit(event_path)
lib/ops/framework.py:757: in _reemit
custom_handler(event)
src/charm.py:74: in on_config_changed
on_config_changed_handler(event, self.fw_adapter, self._stored)
src/charm.py:96: in on_config_changed_handler
juju_model = fw_adapter.get_model_name()
src/adapters/framework.py:88: in get_model_name
return os.environ["JUJU_MODEL_NAME"]
(eg, it gets past harness.begin() but fails once it gets to your framework adapter that requires the env var JUJU_MODEL_NAME to be set, but you aren't setting it in the test.)
The issue is more that your other tests are instantiating Charm directly, into a Framework object. Which causes the 'on' attribute of your Charm class to be populated with attributes for relation_changed events (such as http_relation_changed). If you just had 2 tests that both instantiated Charm directly (with different Framework objects) you'd have exactly the same problem.
Harness already has code to work around it (rather than instantiating Charm directly it instantiates a child of Charm and charm's on class.). However, because your tests have already instantiated Charm directly somewhere else, it has already gotten its attributes populated.
With this patch:
@@ -56,58 +57,65 @@ class CharmTest(unittest.TestCase):
def test__init__works_without_a_hitch(self,
mock_framework_adapter_cls):
# Exercise
- charm.Charm(self.create_framework(), None)
+ class EventChild(CharmEvents):
+ pass
+ class Child(charm.Charm):
+ on = EventChild()
+ Child(self.create_framework(), None)
Running all tests fails in the same way (with JUJU_MODEL_NAME).
At Runtime, there is only ever 1 framework, so the instantiation code is ok as is. It is unfortunate that we have to do the workarounds for tests, which is what Harness already implements, but it is the other test that is leaving things in a state that the Harness can't work around.
We have discussed changing so that registered events are on the instance rather than the class, but it is non-trivial due to how bound objects, etc work in Python.
On Mon, Jun 1, 2020 at 10:57 AM Vladimir Grevtsev [email protected] wrote:
I was thinking it might be:
https://github.com/exceptorr/charm-k8s-prometheus/blob/master/src/charm.py#L35 But that class doesn't have a _stored attribute
Yeah because you were looking at the master branch, while this code is in another branch: exceptorr/charm-k8s-prometheus@ef04be0 #diff-38f277fc95ad7520237f91e48ec77e8dR38 https://github.com/exceptorr/charm-k8s-prometheus/commit/ef04be0b44154d09ed3989cd96a77f9334913230#diff-38f277fc95ad7520237f91e48ec77e8dR38
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/canonical/operator/issues/307#issuecomment-636650087, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQ7J6SDXBKOXY25HPH53RUNGODANCNFSM4NOJI4XQ .
The core issue here is that we need to make define_event work on the instance, not the class
Arguably we should do the dynamism using an instance, but we're probably not going to change that at this point. Harness.begin "solves" this by dynamically inheriting from the charm class.
