Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: model error on testing module if invalid status set by charm #1107

Merged
merged 7 commits into from
Jan 11, 2024
Merged
Next Next commit
feat: model error on testing module if invalid status set by charm
  • Loading branch information
yanksyoon committed Jan 10, 2024
commit f9749111335ab5253d8f01b4cde5812b193791ca
17 changes: 17 additions & 0 deletions ops/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1790,6 +1790,14 @@ class UnknownStatus(StatusBase):
A unit-agent has finished calling install, config-changed and start, but the
charm has not called status-set yet.

This status is for READ ONLY and should not be used to set a unit status. The following
charm code would be an invalid charm code.

```
# Raises ops.model.ModelError : ERROR invalid status "unknown", expected one of
# [maintenance blocked waiting active]
self.unit.status = UnknownStatus()
```
yanksyoon marked this conversation as resolved.
Show resolved Hide resolved
"""
name = 'unknown'

Expand All @@ -1807,6 +1815,15 @@ class ErrorStatus(StatusBase):

The unit-agent has encountered an error (the application or unit requires
human intervention in order to operate correctly).

yanksyoon marked this conversation as resolved.
Show resolved Hide resolved
This status is for READ ONLY and should not be used to set a unit status. The following
charm code would be an invalid charm code.

```
# Raises ops.model.ModelError : ERROR invalid status "error", expected one of
# [maintenance blocked waiting active]
self.unit.status = ErrorStatus()
```
"""
name = 'error'

Expand Down
14 changes: 12 additions & 2 deletions ops/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ def begin_with_initial_hooks(self) -> None:
# the unit status from "Maintenance" to "Unknown". See gh#726
post_setup_sts = self._backend.status_get()
if post_setup_sts.get("status") == "maintenance" and not post_setup_sts.get("message"):
self._backend.status_set("unknown", "", is_app=False)
self._backend.status_set("unknown", "", is_app=False, by_testing_backend=True)
yanksyoon marked this conversation as resolved.
Show resolved Hide resolved
all_ids = list(self._backend._relation_names.items())
random.shuffle(all_ids)
for rel_id, rel_name in all_ids:
Expand Down Expand Up @@ -2222,7 +2222,17 @@ def status_get(self, *, is_app: bool = False):
else:
return self._unit_status

def status_set(self, status: '_StatusName', message: str = '', *, is_app: bool = False):
def status_set(
yanksyoon marked this conversation as resolved.
Show resolved Hide resolved
self,
status: "_StatusName",
message: str = "",
*,
is_app: bool = False,
by_testing_backend: bool = False,
):
if not by_testing_backend and status in {model.ErrorStatus.name, model.UnknownStatus.name}:
yanksyoon marked this conversation as resolved.
Show resolved Hide resolved
raise model.ModelError(f'ERROR invalid status "{status}", expected one of'
' [maintenance blocked waiting active]')
if is_app:
self._app_status = {'status': status, 'message': message}
else:
Expand Down
23 changes: 23 additions & 0 deletions test/test_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2851,6 +2851,29 @@ def _on_collect_unit_status(self, event: ops.CollectStatusEvent):
self.assertEqual(harness.model.app.status, ops.ActiveStatus('active app'))
self.assertEqual(harness.model.unit.status, ops.ActiveStatus('active unit'))

def test_invalid_status_set(self):
yanksyoon marked this conversation as resolved.
Show resolved Hide resolved

class TestCharm(ops.CharmBase):
def __init__(self, framework: ops.Framework):
super().__init__(framework)
self.framework.observe(self.on.collect_app_status, self._on_collect_app_status)
self.framework.observe(self.on.collect_unit_status, self._on_collect_unit_status)
self.app_status_to_add = ops.ErrorStatus('errored app')
self.unit_status_to_add = ops.ErrorStatus('errored unit')

def _on_collect_app_status(self, event: ops.CollectStatusEvent):
event.add_status(self.app_status_to_add)

def _on_collect_unit_status(self, event: ops.CollectStatusEvent):
event.add_status(self.unit_status_to_add)

harness = ops.testing.Harness(TestCharm)
harness.set_leader(True)
harness.begin()

with self.assertRaises(ops.model.ModelError):
harness.evaluate_status()


class TestNetwork(unittest.TestCase):
def setUp(self):
Expand Down
Loading