Skip to content

Commit

Permalink
Treat empty actions.yaml as no actions.yaml. (#459)
Browse files Browse the repository at this point in the history
* Treat empty actions.yaml as no actions.yaml.

Without this change the operator framework would fall over hard if a
charm had an empty `actions.yaml`.

This was found and reported by @davigar15 (thanks!).

Fixes: #458

* check meta.actions is empty dict when actions is empty
  • Loading branch information
chipaca authored Nov 30, 2020
1 parent 2393f59 commit ee7194f
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 12 deletions.
2 changes: 2 additions & 0 deletions ops/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,8 @@ def from_yaml(
raw_actions = {}
if actions is not None:
raw_actions = _loadYaml(actions)
if raw_actions is None:
raw_actions = {}
return cls(meta, raw_actions)


Expand Down
22 changes: 12 additions & 10 deletions test/charms/test_main/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,21 @@ def __init__(self, *args):
self.framework.observe(self.on.mon_relation_departed, self._on_mon_relation_departed)
self.framework.observe(self.on.ha_relation_broken, self._on_ha_relation_broken)

self.framework.observe(self.on.start_action, self._on_start_action)
self.framework.observe(self.on.foo_bar_action, self._on_foo_bar_action)
self.framework.observe(self.on.get_model_name_action, self._on_get_model_name_action)
self.framework.observe(self.on.get_status_action, self._on_get_status_action)
actions = self.charm_dir / 'actions.yaml'
if actions.exists() and actions.read_bytes():
self.framework.observe(self.on.start_action, self._on_start_action)
self.framework.observe(self.on.foo_bar_action, self._on_foo_bar_action)
self.framework.observe(self.on.get_model_name_action, self._on_get_model_name_action)
self.framework.observe(self.on.get_status_action, self._on_get_status_action)

self.framework.observe(self.on.log_critical_action, self._on_log_critical_action)
self.framework.observe(self.on.log_error_action, self._on_log_error_action)
self.framework.observe(self.on.log_warning_action, self._on_log_warning_action)
self.framework.observe(self.on.log_info_action, self._on_log_info_action)
self.framework.observe(self.on.log_debug_action, self._on_log_debug_action)

self.framework.observe(self.on.collect_metrics, self._on_collect_metrics)

self.framework.observe(self.on.log_critical_action, self._on_log_critical_action)
self.framework.observe(self.on.log_error_action, self._on_log_error_action)
self.framework.observe(self.on.log_warning_action, self._on_log_warning_action)
self.framework.observe(self.on.log_info_action, self._on_log_info_action)
self.framework.observe(self.on.log_debug_action, self._on_log_debug_action)

if os.getenv('TRY_EXCEPTHOOK', False):
raise RuntimeError("failing as requested")

Expand Down
4 changes: 4 additions & 0 deletions test/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ def _on_start(self, event):
with self.assertRaisesRegex(TypeError, "observer methods must now be explicitly provided"):
framework.observe(charm.on.start, charm)

def test_empty_action(self):
meta = CharmMeta.from_yaml('name: my-charm', '')
self.assertEqual(meta.actions, {})

def test_helper_properties(self):
framework = self.create_framework()

Expand Down
16 changes: 14 additions & 2 deletions test/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,12 @@ def _read_and_clear_state(self):
if self.CHARM_STATE_FILE.stat().st_size:
storage = SQLiteStorage(self.CHARM_STATE_FILE)
with (self.JUJU_CHARM_DIR / 'metadata.yaml').open() as m:
with (self.JUJU_CHARM_DIR / 'actions.yaml').open() as a:
meta = CharmMeta.from_yaml(m, a)
af = (self.JUJU_CHARM_DIR / 'actions.yaml')
if af.exists():
with af.open() as a:
meta = CharmMeta.from_yaml(m, a)
else:
meta = CharmMeta.from_yaml(m)
framework = Framework(storage, self.JUJU_CHARM_DIR, meta, None)

class ThisCharmEvents(CharmEvents):
Expand Down Expand Up @@ -557,6 +561,14 @@ def test_event_not_implemented(self):
self.fail('Event simulation for an unsupported event'
' results in a non-zero exit code returned')

def test_no_actions(self):
(self.JUJU_CHARM_DIR / 'actions.yaml').unlink()
self._simulate_event(EventSpec(InstallEvent, 'install'))

def test_empty_actions(self):
(self.JUJU_CHARM_DIR / 'actions.yaml').write_text('')
self._simulate_event(EventSpec(InstallEvent, 'install'))

def test_collect_metrics(self):
fake_script(self, 'add-metric', 'exit 0')
self._simulate_event(EventSpec(InstallEvent, 'install'))
Expand Down

0 comments on commit ee7194f

Please sign in to comment.