diff --git a/juju/application.py b/juju/application.py index 3def4693e..84bf23ff0 100644 --- a/juju/application.py +++ b/juju/application.py @@ -584,10 +584,10 @@ async def get_status(self): full_status = await client_facade.FullStatus(patterns=None) - import pprint - with open("/tmp/full.status.jsonl", "a") as f: - print(file=f) - print(pprint.pformat(full_status.serialize()), file=f) + # import pprint + # with open("/tmp/full.status.jsonl", "a") as f: + # print(file=f) + # print(pprint.pformat(full_status.serialize()), file=f) _app = full_status.applications.get(self.name, None) if not _app: diff --git a/juju/model.py b/juju/model.py index b551d7f3d..a3d4088cd 100644 --- a/juju/model.py +++ b/juju/model.py @@ -3061,7 +3061,6 @@ async def _check_idle( now = datetime.now() expected_idle_since = now - timedelta(seconds=idle_period) full_status = await self.get_status() - # __import__("pdb").set_trace() for app_name in apps: if not (app := full_status.applications.get(app_name)): @@ -3207,7 +3206,6 @@ async def _legacy_check_idle( last_log_time: List[Optional[datetime]], # = [None] start_time: datetime, ): - # __import__("pdb").set_trace() _timeout = timedelta(seconds=timeout) if timeout is not None else None _idle_period = timedelta(seconds=idle_period) log_interval = timedelta(seconds=30) @@ -3250,16 +3248,16 @@ async def _legacy_check_idle( for unit in app.units: if raise_on_error and unit.machine is not None and unit.machine.status == "error": errors.setdefault("Machine", []).append(unit.machine.id) - return False + continue if raise_on_error and unit.agent_status == "error": errors.setdefault("Agent", []).append(unit.name) - return False + continue if raise_on_error and unit.workload_status == "error": errors.setdefault("Unit", []).append(unit.name) - return False + continue if raise_on_blocked and unit.workload_status == "blocked": blocks.setdefault("Unit", []).append(unit.name) - return False + continue # TODO (cderici): we need two versions of wait_for_idle, one for waiting on # individual units, another one for waiting for an application. # The convoluted logic below is the result of trying to do both at the same diff --git a/juju/status.py b/juju/status.py index 8a846bfaf..9fc37a9c6 100644 --- a/juju/status.py +++ b/juju/status.py @@ -10,14 +10,14 @@ """ derive_status is used to determine the application status from a set of unit status values. -:param statues: list of known unit workload statues +:param statuses: list of known unit workload statuses """ -def derive_status(statues): +def derive_status(statuses): current = 'unknown' - for status in statues: + for status in statuses: if status in severity_map and severity_map[status] > severity_map[current]: current = status return current diff --git a/tests/integration/test_model.py b/tests/integration/test_model.py index 7350681d9..aab2279f2 100644 --- a/tests/integration/test_model.py +++ b/tests/integration/test_model.py @@ -51,7 +51,6 @@ async def test_deploy_local_bundle_dir(): for (k, v) in model.applications.items(): writer.write(k) assert app1 and app2 - # import pdb;pdb.set_trace() await model.wait_for_idle(['juju-qa-test', 'nrpe'], wait_for_at_least_units=1) diff --git a/tests/unit/test_wait_for_idle.py b/tests/unit/test_wait_for_idle.py index f9f621628..083cffecb 100644 --- a/tests/unit/test_wait_for_idle.py +++ b/tests/unit/test_wait_for_idle.py @@ -17,7 +17,7 @@ FullStatus, UnitStatus, ) -from juju.errors import JujuAppError +from juju.errors import JujuAppError, JujuUnitError from juju.machine import Machine from juju.model import Model from juju.unit import Unit @@ -60,6 +60,24 @@ async def test_raise_on_error(full_status_response: dict, kwargs: Dict[str, Any] assert "big problem" in str(idle) +async def test_raise_on_error_compounding(full_status_response: dict, kwargs: Dict[str, Any]): + app = full_status_response["response"]["applications"]["hexanator"] + app["status"]["status"] = "unset" + app["units"]["hexanator/0"]["workload-status"]["status"] = "error" + app["units"]["hexanator/0"]["workload-status"]["info"] = "unit problem" + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_error"] = True + idle, legacy = await model_fake(full_status_response).check_both(**kwargs) + assert isinstance(idle, JujuUnitError) + # FIXME looks like from the wait_for_idle point of view: + # app in unset means nothing because + # both app compound status is registered as error + # and unit direct status is registered as error + # and unit status raises first + assert isinstance(legacy, JujuUnitError) + assert "unit problem" in str(idle) + + async def test_raise_on_blocked(full_status_response: dict, kwargs: Dict[str, Any]): full_status_response["response"]["applications"]["hexanator"]["status"]["status"] = "blocked" full_status_response["response"]["applications"]["hexanator"]["status"]["info"] = "big problem" @@ -178,10 +196,8 @@ def model_fake(resp: dict) -> ModelFake: assert fsapp assert fsapp.status # DetailedStatus assert isinstance(fsapp.status.status, str) - app._status = fsapp.status.status - assert isinstance(fsapp.status.info, str) - app._status_message = fsapp.status.info + app.set_status(fsapp.status.status, fsapp.status.info) for uname in fsapp.units: app._units.append(unit := UnitFake(uname, m)) @@ -243,25 +259,25 @@ async def rpc(self, msg, encoder=None): class ApplicationFake(Application): - _status: str = "" - _status_message: str = "" + _safe_data: dict _units: List[Unit] - @property - def status(self) -> str: - return self._status - - @property - def status_message(self) -> str: - return self._status_message + def set_status(self, status="fixme", info="some info"): + self._safe_data["status"]["current"] = status + self._safe_data["status"]["message"] = info @property def units(self) -> List[Unit]: return self._units + @property + def safe_data(self) -> dict: + return self._safe_data + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._units = [] + self._safe_data = {"status": {"current": "fixme", "message": "fixme"}} class UnitFake(Unit):