diff --git a/juju/model/__init__.py b/juju/model/__init__.py index 0063fbb6f..f40738b1d 100644 --- a/juju/model/__init__.py +++ b/juju/model/__init__.py @@ -3298,7 +3298,8 @@ async def new_wait_for_idle( if any(not isinstance(o, str) for o in apps): raise TypeError(f"apps must be a Iterable[str], got {apps=}") - deadline = None if timeout is None else time.monotonic() + timeout + started = time.monotonic() + deadline = None if timeout is None else started + timeout async def status_on_demand(): yield _idle.check( @@ -3316,6 +3317,7 @@ async def status_on_demand(): wait_for_units=wait_for_units, idle_period=idle_period, ): + logger.info(f"wait_for_idle start{time.monotonic() - started:+.1f} {done=}") if done: break diff --git a/juju/model/_idle.py b/juju/model/_idle.py index 8f6b9a796..6737fe127 100644 --- a/juju/model/_idle.py +++ b/juju/model/_idle.py @@ -44,7 +44,7 @@ async def loop( idle_since: dict[str, float] = {} async for status in foo: - logger.warning("FIXME unit test debug %r", status) + logger.info("wait_for_idle iteration %s", status) now = time.monotonic() if not status: @@ -52,51 +52,50 @@ async def loop( continue expected_idle_since = now - idle_period - rv = True # FIXME there's some confusion about what a "busy" unit is # are we ready when over the last idle_period, every time sampled: # a. >=N units were ready (possibly different each time), or # b. >=N units were ready each time for name in status.units: - if name in status.ready_units: + if name in status.idle_units: idle_since[name] = min(now, idle_since.get(name, float("inf"))) else: idle_since[name] = float("inf") + if busy := {n for n, t in idle_since.items() if t > expected_idle_since}: + logger.info("Waiting for units to be idle enough: %s", busy) + yield False + continue + for app_name in apps: ready_units = [ n for n in status.ready_units if n.startswith(f"{app_name}/") ] if len(ready_units) < wait_for_units: - logger.warn( + logger.info( "Waiting for app %r units %s >= %s", app_name, len(status.ready_units), wait_for_units, ) - rv = False + yield False + continue if ( wait_for_exact_units is not None and len(ready_units) != wait_for_exact_units ): - logger.warn( + logger.info( "Waiting for app %r units %s == %s", app_name, len(ready_units), wait_for_exact_units, ) - rv = False - - # FIXME possible interaction between "wait_for_units" and "idle_period" - # Assume that we've got some units ready and some busy - # What are the semantics for returning True? - if busy := [n for n, t in idle_since.items() if t > expected_idle_since]: - logger.warn("Waiting for %s to be idle enough", busy) - rv = False + yield False + continue - yield rv + yield True def check( @@ -175,7 +174,6 @@ def check( rv = CheckStatus(set(), set(), set()) for app_name in apps: - ready_units = [] app = full_status.applications[app_name] assert isinstance(app, ApplicationStatus) for unit_name, unit in _app_units(full_status, app_name).items(): @@ -183,16 +181,12 @@ def check( assert unit.agent_status assert unit.workload_status - if unit.agent_status.status != "idle": - continue - if status and unit.workload_status.status != status: - continue + if unit.agent_status.status == "idle": + rv.idle_units.add(unit_name) - ready_units.append(unit) - rv.ready_units.add(unit_name) + if not status or unit.workload_status.status == status: + rv.ready_units.add(unit_name) - # FIXME - # rv.idle_units -- depends on agent status only, not workload status return rv diff --git a/juju/version.py b/juju/version.py index e8d250a6e..86177ffd4 100644 --- a/juju/version.py +++ b/juju/version.py @@ -6,4 +6,4 @@ DEFAULT_ARCHITECTURE = "amd64" -CLIENT_VERSION = "3.6.1.0rc2" +CLIENT_VERSION = "3.6.1.0rc3" diff --git a/pyproject.toml b/pyproject.toml index 6947ad82b..d2eec1cc6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "juju" -version = "3.6.1.0rc2" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION +version = "3.6.1.0rc3" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION description = "Python library for Juju" readme = "docs/readme.rst" license = { file = "LICENSE" } diff --git a/tests/unit/test_idle_check.py b/tests/unit/test_idle_check.py index 8ed89e4f0..bba2cee4e 100644 --- a/tests/unit/test_idle_check.py +++ b/tests/unit/test_idle_check.py @@ -5,6 +5,7 @@ import copy import json from typing import Any +from unittest.mock import ANY import pytest @@ -31,7 +32,12 @@ def test_check_status(full_status: FullStatus, kwargs): "mysql-test-app/0", "mysql-test-app/1", }, - set(), + { + "grafana-agent-k8s/0", + "hexanator/0", + "mysql-test-app/0", + "mysql-test-app/1", + }, ) @@ -44,7 +50,7 @@ def test_check_status_missing_app(full_status: FullStatus, kwargs): def test_check_status_is_selective(full_status: FullStatus, kwargs): kwargs["apps"] = ["hexanator"] status = check(full_status, **kwargs) - assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, {"hexanator/0"}) def test_no_apps(full_status: FullStatus, kwargs): @@ -80,25 +86,13 @@ def test_app_error(response: dict[str, Any], kwargs): assert "big problem" in str(e) -def test_exact_count(response: dict[str, Any], kwargs): - units = response["response"]["applications"]["hexanator"]["units"] - units["hexanator/1"] = units["hexanator/0"] - - kwargs["apps"] = ["hexanator"] - - status = check(convert(response), **kwargs) - assert status == CheckStatus( - {"hexanator/0", "hexanator/1"}, {"hexanator/0", "hexanator/1"}, set() - ) - - def test_ready_units(full_status: FullStatus, kwargs): kwargs["apps"] = ["mysql-test-app"] status = check(full_status, **kwargs) assert status == CheckStatus( {"mysql-test-app/0", "mysql-test-app/1"}, {"mysql-test-app/0", "mysql-test-app/1"}, - set(), + {"mysql-test-app/0", "mysql-test-app/1"}, ) @@ -106,7 +100,7 @@ def test_active_units(full_status: FullStatus, kwargs): kwargs["apps"] = ["mysql-test-app"] kwargs["status"] = "active" status = check(full_status, **kwargs) - assert status == CheckStatus({"mysql-test-app/0", "mysql-test-app/1"}, set(), set()) + assert status == CheckStatus({"mysql-test-app/0", "mysql-test-app/1"}, set(), ANY) def test_ready_unit_requires_idle_agent(response: dict[str, Any], kwargs): @@ -118,9 +112,11 @@ def test_ready_unit_requires_idle_agent(response: dict[str, Any], kwargs): kwargs["status"] = "active" status = check(convert(response), **kwargs) - assert status - assert status.units == {"hexanator/0", "hexanator/1"} - assert status.ready_units == {"hexanator/0"} + assert status == CheckStatus( + {"hexanator/0", "hexanator/1"}, + {"hexanator/0", "hexanator/1"}, + {"hexanator/0"}, + ) def test_ready_unit_requires_workload_status(response: dict[str, Any], kwargs): @@ -132,7 +128,7 @@ def test_ready_unit_requires_workload_status(response: dict[str, Any], kwargs): kwargs["status"] = "active" status = check(convert(response), **kwargs) - assert status == CheckStatus({"hexanator/0", "hexanator/1"}, {"hexanator/0"}, set()) + assert status == CheckStatus({"hexanator/0", "hexanator/1"}, {"hexanator/0"}, ANY) def test_agent_error(response: dict[str, Any], kwargs): @@ -182,7 +178,7 @@ def test_machine_ok(response: dict[str, Any], kwargs): kwargs["raise_on_error"] = True status = check(convert(response), **kwargs) - assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, ANY) def test_machine_error(response: dict[str, Any], kwargs): @@ -244,7 +240,7 @@ def test_maintenance(response: dict[str, Any], kwargs): kwargs["status"] = "maintenance" status = check(convert(response), **kwargs) - assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, ANY) @pytest.fixture diff --git a/tests/unit/test_idle_check_subordinate.py b/tests/unit/test_idle_check_subordinate.py index 149e468ba..6d4fc2371 100644 --- a/tests/unit/test_idle_check_subordinate.py +++ b/tests/unit/test_idle_check_subordinate.py @@ -16,7 +16,11 @@ def test_subordinate_apps(response: dict[str, Any], kwargs): status = check(convert(response), **kwargs) - assert status == CheckStatus({"ntp/0", "ubuntu/0"}, {"ntp/0", "ubuntu/0"}, set()) + assert status == CheckStatus( + {"ntp/0", "ubuntu/0"}, + {"ntp/0", "ubuntu/0"}, + {"ntp/0", "ubuntu/0"}, + ) def test_subordinate_is_selective(response, kwargs): @@ -25,7 +29,11 @@ def test_subordinate_is_selective(response, kwargs): ] subordinates["some-other/0"] = subordinates["ntp/0"] status = check(convert(response), **kwargs) - assert status == CheckStatus({"ntp/0", "ubuntu/0"}, {"ntp/0", "ubuntu/0"}, set()) + assert status == CheckStatus( + {"ntp/0", "ubuntu/0"}, + {"ntp/0", "ubuntu/0"}, + {"ntp/0", "ubuntu/0"}, + ) @pytest.fixture