From 03e7d5119103122acf21384e1cef40dbe7945145 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Tue, 26 Nov 2024 17:07:01 +0900 Subject: [PATCH 01/19] feat: new Model.wait_for_idle() --- .github/workflows/test.yaml | 9 +- juju/client/facade.py | 68 +++-- juju/{model.py => model/__init__.py} | 221 ++++++++++---- juju/model/_idle.py | 221 ++++++++++++++ juju/unit.py | 4 +- juju/version.py | 2 +- pyproject.toml | 8 +- setup.py | 1 + tests/unit/data/fullstatus.json | 322 ++++++++++++++++++++ tests/unit/data/subordinate-fullstatus.json | 254 +++++++++++++++ tests/unit/test_idle_check.py | 273 +++++++++++++++++ tests/unit/test_idle_check_subordinate.py | 57 ++++ tests/unit/test_idle_loop.py | 61 ++++ tests/unit/test_model.py | 17 +- tests/unit/test_unit.py | 2 +- tox.ini | 1 + 16 files changed, 1423 insertions(+), 98 deletions(-) rename juju/{model.py => model/__init__.py} (95%) create mode 100644 juju/model/_idle.py create mode 100644 tests/unit/data/fullstatus.json create mode 100644 tests/unit/data/subordinate-fullstatus.json create mode 100644 tests/unit/test_idle_check.py create mode 100644 tests/unit/test_idle_check_subordinate.py create mode 100644 tests/unit/test_idle_loop.py diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index dfe55d160..0073eebcc 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -78,6 +78,9 @@ jobs: - "3.4/stable" - "3.5/stable" - "3.6/stable" + new_wait_for_idle: + - "True" + - "False" steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 @@ -116,7 +119,9 @@ jobs: # # set model defaults # juju model-defaults apt-http-proxy=$PROXY apt-https-proxy=$PROXY juju-http-proxy=$PROXY juju-https-proxy=$PROXY snap-http-proxy=$PROXY snap-https-proxy=$PROXY # juju model-defaults - - run: uvx -p ${{ matrix.python }} tox -e integration + - run: uvx -p ${{ matrix.python }} tox -s -e integration + env: + JUJU_NEW_WAIT_FOR_IDLE: ${{ matrix.new_wait_for_idle }} integration-quarantine: name: Quarantined Integration Tests @@ -144,4 +149,4 @@ jobs: with: provider: lxd juju-channel: ${{ matrix.juju }} - - run: uvx -p ${{ matrix.python }} tox -e integration-quarantine + - run: uvx -p ${{ matrix.python }} tox -s -e integration-quarantine diff --git a/juju/client/facade.py b/juju/client/facade.py index f0ee75130..7ab4fddd0 100644 --- a/juju/client/facade.py +++ b/juju/client/facade.py @@ -14,7 +14,7 @@ from collections import defaultdict from glob import glob from pathlib import Path -from typing import Any, Mapping, Sequence +from typing import Any, Mapping, Sequence, TypeVar, overload import packaging.version import typing_inspect @@ -183,7 +183,7 @@ def ref_type(self, obj): return self.get_ref_type(obj["$ref"]) -CLASSES = {} +CLASSES: dict[str, type[Type]] = {} factories = codegen.Capture() @@ -479,37 +479,48 @@ def ReturnMapping(cls): # noqa: N802 def decorator(f): @functools.wraps(f) async def wrapper(*args, **kwargs): - nonlocal cls reply = await f(*args, **kwargs) - if cls is None: - return reply - if "error" in reply: - cls = CLASSES["Error"] - if typing_inspect.is_generic_type(cls) and issubclass( - typing_inspect.get_origin(cls), Sequence - ): - parameters = typing_inspect.get_parameters(cls) - result = [] - item_cls = parameters[0] - for item in reply: - result.append(item_cls.from_json(item)) - """ - if 'error' in item: - cls = CLASSES['Error'] - else: - cls = item_cls - result.append(cls.from_json(item)) - """ - else: - result = cls.from_json(reply["response"]) - - return result + return _convert_response(reply, cls=cls) return wrapper return decorator +@overload +def _convert_response(response: dict[str, Any], *, cls: type[SomeType]) -> SomeType: ... + + +@overload +def _convert_response(response: dict[str, Any], *, cls: None) -> dict[str, Any]: ... + + +def _convert_response(response: dict[str, Any], *, cls: type[Type] | None) -> Any: + if cls is None: + return response + if "error" in response: + cls = CLASSES["Error"] + if typing_inspect.is_generic_type(cls) and issubclass( + typing_inspect.get_origin(cls), Sequence + ): + parameters = typing_inspect.get_parameters(cls) + result = [] + item_cls = parameters[0] + for item in response: + result.append(item_cls.from_json(item)) + """ + if 'error' in item: + cls = CLASSES['Error'] + else: + cls = item_cls + result.append(cls.from_json(item)) + """ + else: + result = cls.from_json(response["response"]) + + return result + + def make_func(cls, name, description, params, result, _async=True): indent = " " args = Args(cls.schema, params) @@ -663,7 +674,7 @@ async def rpc(self, msg: dict[str, _RichJson]) -> _Json: return result @classmethod - def from_json(cls, data): + def from_json(cls, data: Type | str | dict[str, Any] | list[Any]) -> Type | None: def _parse_nested_list_entry(expr, result_dict): if isinstance(expr, str): if ">" in expr or ">=" in expr: @@ -742,6 +753,9 @@ def get(self, key, default=None): return getattr(self, attr, default) +SomeType = TypeVar("SomeType", bound=Type) + + class Schema(dict): def __init__(self, schema): self.name = schema["Name"] diff --git a/juju/model.py b/juju/model/__init__.py similarity index 95% rename from juju/model.py rename to juju/model/__init__.py index 31e939139..0063fbb6f 100644 --- a/juju/model.py +++ b/juju/model/__init__.py @@ -1,10 +1,12 @@ # Copyright 2023 Canonical Ltd. # Licensed under the Apache V2, see LICENCE file for details. +"""Represent Juju Model, as in the workspace into which applications are deployed.""" + from __future__ import annotations import asyncio import base64 -import collections +import collections.abc import hashlib import json import logging @@ -13,6 +15,7 @@ import stat import sys import tempfile +import time import warnings import weakref import zipfile @@ -20,23 +23,26 @@ from datetime import datetime, timedelta from functools import partial from pathlib import Path -from typing import TYPE_CHECKING, Any, Literal, Mapping, overload +from typing import TYPE_CHECKING, Any, Iterable, Literal, Mapping, overload import websockets import yaml from typing_extensions import deprecated -from . import provisioner, tag, utils -from .annotationhelper import _get_annotations, _set_annotations -from .bundle import BundleHandler, get_charm_series, is_local_charm -from .charmhub import CharmHub -from .client import client, connection, connector -from .client.overrides import Caveat, Macaroon -from .constraints import parse as parse_constraints -from .constraints import parse_storage_constraints -from .controller import ConnectedController, Controller -from .delta import get_entity_class, get_entity_delta -from .errors import ( +from .. import provisioner, tag, utils +from ..annotationhelper import _get_annotations, _set_annotations +from ..bundle import BundleHandler, get_charm_series, is_local_charm +from ..charmhub import CharmHub +from ..client import client, connection, connector +from ..client._definitions import ApplicationStatus as ApplicationStatus +from ..client._definitions import MachineStatus as MachineStatus +from ..client._definitions import UnitStatus as UnitStatus +from ..client.overrides import Caveat, Macaroon +from ..constraints import parse as parse_constraints +from ..constraints import parse_storage_constraints +from ..controller import ConnectedController, Controller +from ..delta import get_entity_class, get_entity_delta +from ..errors import ( JujuAgentError, JujuAPIError, JujuAppError, @@ -49,27 +55,37 @@ JujuUnitError, PylibjujuError, ) -from .exceptions import DeadEntityException -from .names import is_valid_application -from .offerendpoints import ParseError as OfferParseError -from .offerendpoints import parse_local_endpoint, parse_offer_url -from .origin import Channel, Source -from .placement import parse as parse_placement -from .secrets import create_secret_data, read_secret_data -from .tag import application as application_tag -from .url import URL, Schema -from .version import DEFAULT_ARCHITECTURE +from ..exceptions import DeadEntityException +from ..names import is_valid_application +from ..offerendpoints import ParseError as OfferParseError +from ..offerendpoints import parse_local_endpoint, parse_offer_url +from ..origin import Channel, Source +from ..placement import parse as parse_placement +from ..secrets import create_secret_data, read_secret_data +from ..tag import application as application_tag +from ..url import URL, Schema +from ..version import DEFAULT_ARCHITECTURE +from . import _idle if TYPE_CHECKING: - from .application import Application - from .client._definitions import FullStatus - from .constraints import StorageConstraintDict - from .machine import Machine - from .relation import Relation - from .remoteapplication import ApplicationOffer, RemoteApplication - from .unit import Unit + from ..application import Application + from ..client._definitions import FullStatus + from ..constraints import StorageConstraintDict + from ..machine import Machine + from ..relation import Relation + from ..remoteapplication import ApplicationOffer, RemoteApplication + from ..unit import Unit + +log = logger = logging.getLogger(__name__) -log = logging.getLogger(__name__) + +def use_new_wait_for_idle() -> bool: + val = os.getenv("JUJU_NEW_WAIT_FOR_IDLE") + if not val: + return False + if val.isdigit(): + return bool(int(val)) + return val.title() != "False" class _Observer: @@ -632,9 +648,9 @@ class Model: def __init__( self, - max_frame_size=None, - bakery_client=None, - jujudata=None, + max_frame_size: int | None = None, + bakery_client: Any = None, + jujudata: Any = None, ): """Instantiate a new Model. @@ -2664,14 +2680,21 @@ async def get_action_status(self, uuid_or_prefix=None, name=None): results[tag.untag("action-", a.action.tag)] = a.status return results - async def get_status(self, filters=None, utc=False) -> FullStatus: + async def get_status(self, filters=None, utc: bool = False) -> FullStatus: """Return the status of the model. :param str filters: Optional list of applications, units, or machines to include, which can use wildcards ('*'). - :param bool utc: Display time as UTC in RFC3339 format + :param bool utc: Deprecated, display time as UTC in RFC3339 format """ + if utc: + warnings.warn( + "Model.get_status() utc= parameter is deprecated", + DeprecationWarning, + stacklevel=2, + ) + client_facade = client.ClientFacade.from_connection(self.connection()) return await client_facade.FullStatus(patterns=filters) @@ -2998,7 +3021,7 @@ async def _get_source_api(self, url): async def wait_for_idle( self, - apps: list[str] | None = None, + apps: Iterable[str] | None = None, raise_on_error: bool = True, raise_on_blocked: bool = False, wait_for_active: bool = False, @@ -3011,7 +3034,7 @@ async def wait_for_idle( ) -> None: """Wait for applications in the model to settle into an idle state. - :param List[str] apps: Optional list of specific app names to wait on. + :param Iterable[str]|None apps: Optional list of specific app names to wait on. If given, all apps must be present in the model and idle, while other apps in the model can still be busy. If not given, all apps currently in the model must be idle. @@ -3037,6 +3060,7 @@ async def wait_for_idle( units of all apps need to be `idle`. This delay is used to ensure that any pending hooks have a chance to start to avoid false positives. The default is 15 seconds. + Exact behaviour is undefined for very small values and 0. :param float check_freq: How frequently, in seconds, to check the model. The default is every half-second. @@ -3053,6 +3077,21 @@ async def wait_for_idle( going into the idle state. (e.g. useful for scaling down). When set, takes precedence over the `wait_for_units` parameter. """ + if use_new_wait_for_idle(): + await self.new_wait_for_idle( + apps=apps, + raise_on_error=raise_on_error, + raise_on_blocked=raise_on_blocked, + wait_for_active=wait_for_active, + timeout=timeout, + idle_period=idle_period, + check_freq=check_freq, + status=status, + wait_for_at_least_units=wait_for_at_least_units, + wait_for_exact_units=wait_for_exact_units, + ) + return + if wait_for_active: warnings.warn( "wait_for_active is deprecated; use status", @@ -3065,16 +3104,18 @@ async def wait_for_idle( wait_for_at_least_units if wait_for_at_least_units is not None else 1 ) - timeout = timedelta(seconds=timeout) if timeout is not None else None - idle_period = timedelta(seconds=idle_period) + timeout_ = timedelta(seconds=timeout) if timeout is not None else None + idle_period_ = timedelta(seconds=idle_period) start_time = datetime.now() - # Type check against the common error of passing a str for apps - if apps is not None and ( - not isinstance(apps, list) or any(not isinstance(o, str) for o in apps) - ): - raise JujuError(f"Expected a List[str] for apps, given {apps}") - apps = apps or self.applications + if isinstance(apps, (str, bytes, bytearray, memoryview)): + raise TypeError(f"apps must be a Iterable[str], got {apps=}") + + apps_ = list(apps or self.applications) + + if any(not isinstance(o, str) for o in apps_): + raise TypeError(f"apps must be a Iterable[str], got {apps_=}") + idle_times: dict[str, datetime] = {} units_ready: set[str] = set() # The units that are in the desired state last_log_time: datetime | None = None @@ -3110,10 +3151,10 @@ def _raise_for_status(entities: dict[str, list[str]], status: Any): # The list 'busy' is what keeps this loop going, # i.e. it'll stop when busy is empty after all the # units are scanned - busy = [] - errors = {} - blocks = {} - for app_name in apps: + busy: list[str] = [] + errors: dict[str, list[str]] = {} + blocks: dict[str, list[str]] = {} + for app_name in apps_: if app_name not in self.applications: busy.append(app_name + " (missing)") continue @@ -3192,7 +3233,7 @@ def _raise_for_status(entities: dict[str, list[str]], status: Any): now = datetime.now() idle_start = idle_times.setdefault(unit.name, now) - if now - idle_start < idle_period: + if now - idle_start < idle_period_: busy.append( f"{unit.name} [{unit.agent_status}] {unit.workload_status}: {unit.workload_status_message}" ) @@ -3205,14 +3246,84 @@ def _raise_for_status(entities: dict[str, list[str]], status: Any): _raise_for_status(blocks, "blocked") if not busy: break - busy = "\n ".join(busy) - if timeout is not None and datetime.now() - start_time > timeout: - raise asyncio.TimeoutError("Timed out waiting for model:\n" + busy) + if timeout_ is not None and datetime.now() - start_time > timeout_: + raise asyncio.TimeoutError( + "\n ".join(["Timed out waiting for model:", *busy]) + ) if last_log_time is None or datetime.now() - last_log_time > log_interval: - log.info("Waiting for model:\n " + busy) + log.info("\n ".join(["Waiting for model:", *busy])) last_log_time = datetime.now() await asyncio.sleep(check_freq) + async def new_wait_for_idle( + self, + apps: Iterable[str] | None = None, + raise_on_error: bool = True, + raise_on_blocked: bool = False, + wait_for_active: bool = False, + timeout: float | None = 10 * 60, + idle_period: float = 15, + check_freq: float = 0.5, + status: str | None = None, + wait_for_at_least_units: int | None = None, + wait_for_exact_units: int | None = None, + ) -> None: + """Wait for applications in the model to settle into an idle state. + + arguments match those of .wait_for_idle exactly. + """ + if not isinstance(wait_for_exact_units, (int, type(None))): + raise ValueError(f"Must be an int or None, got {wait_for_exact_units=}") + + if isinstance(wait_for_exact_units, int) and wait_for_exact_units < 0: + raise ValueError(f"Must be >=0, got {wait_for_exact_units=}") + + if wait_for_active: + warnings.warn( + "wait_for_active is deprecated; use status", + DeprecationWarning, + stacklevel=3, + ) + status = "active" + + wait_for_units = ( + wait_for_at_least_units if wait_for_at_least_units is not None else 1 + ) + + if isinstance(apps, (str, bytes, bytearray, memoryview)): + raise TypeError(f"apps must be a Iterable[str], got {apps=}") + + apps = frozenset(apps or self.applications) + + 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 + + async def status_on_demand(): + yield _idle.check( + await self.get_status(), + apps=apps, + raise_on_error=raise_on_error, + raise_on_blocked=raise_on_blocked, + status=status, + ) + + async for done in _idle.loop( + status_on_demand(), + apps=apps, + wait_for_exact_units=wait_for_exact_units, + wait_for_units=wait_for_units, + idle_period=idle_period, + ): + if done: + break + + if deadline and time.monotonic() > deadline: + raise asyncio.TimeoutError(f"Timed out after {timeout}s") + + await asyncio.sleep(check_freq) + def _create_consume_args(offer, macaroon, controller_info): """Convert a typed object that has been normalised to a overridden typed diff --git a/juju/model/_idle.py b/juju/model/_idle.py new file mode 100644 index 000000000..8f6b9a796 --- /dev/null +++ b/juju/model/_idle.py @@ -0,0 +1,221 @@ +# Copyright 2024 Canonical Ltd. +# Licensed under the Apache V2, see LICENCE file for details. +"""Implementation of Model.wait_for_idle(), analog to `juju wait_for`.""" + +from __future__ import annotations + +import logging +import time +from dataclasses import dataclass +from typing import AsyncIterable + +from ..client._definitions import ( + ApplicationStatus, + FullStatus, + MachineStatus, + UnitStatus, +) +from ..errors import JujuAgentError, JujuAppError, JujuMachineError, JujuUnitError + +logger = logging.getLogger(__name__) + + +@dataclass +class CheckStatus: + """Return type check(), represents single loop iteration.""" + + units: set[str] + """All units visible at this point.""" + ready_units: set[str] + """Units in good status (workload, agent, machine?).""" + idle_units: set[str] + """Units with stable agent status (FIXME details).""" + + +async def loop( + foo: AsyncIterable[CheckStatus | None], + *, + apps: frozenset[str], + wait_for_exact_units: int | None = None, + wait_for_units: int, + idle_period: float, +) -> AsyncIterable[bool]: + """The outer, time-dependents logic of a wait_for_idle loop.""" + idle_since: dict[str, float] = {} + + async for status in foo: + logger.warning("FIXME unit test debug %r", status) + now = time.monotonic() + + if not status: + yield False + 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: + idle_since[name] = min(now, idle_since.get(name, float("inf"))) + else: + idle_since[name] = float("inf") + + 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( + "Waiting for app %r units %s >= %s", + app_name, + len(status.ready_units), + wait_for_units, + ) + rv = False + + if ( + wait_for_exact_units is not None + and len(ready_units) != wait_for_exact_units + ): + logger.warn( + "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 rv + + +def check( + full_status: FullStatus, + *, + apps: frozenset[str], + raise_on_error: bool, + raise_on_blocked: bool, + status: str | None, +) -> CheckStatus | None: + """A single iteration of a wait_for_idle loop.""" + for app_name in apps: + if not full_status.applications.get(app_name): + logger.info("Waiting for app %r", app_name) + return None + + # Order of errors: + # + # Machine error (any unit of any app from apps) + # Agent error (-"-) + # Workload error (-"-) + # App error (any app from apps) + # + # Workload blocked (any unit of any app from apps) + # App blocked (any app from apps) + units: dict[str, UnitStatus] = {} + + for app_name in apps: + units.update(_app_units(full_status, app_name)) + + for unit_name, unit in units.items(): + if unit.machine: + machine = full_status.machines[unit.machine] + assert isinstance(machine, MachineStatus) + assert machine.instance_status + if machine.instance_status.status == "error" and raise_on_error: + raise JujuMachineError( + f"{unit_name!r} machine {unit.machine!r} has errored: {machine.instance_status.info!r}" + ) + + for unit_name, unit in units.items(): + assert unit.agent_status + if unit.agent_status.status == "error" and raise_on_error: + raise JujuAgentError( + f"{unit_name!r} agent has errored: {unit.agent_status.info!r}" + ) + + for unit_name, unit in units.items(): + assert unit.workload_status + if unit.workload_status.status == "error" and raise_on_error: + raise JujuUnitError( + f"{unit_name!r} workload has errored: {unit.workload_status.info!r}" + ) + + for app_name in apps: + app = full_status.applications[app_name] + assert isinstance(app, ApplicationStatus) + assert app.status + if app.status.status == "error" and raise_on_error: + raise JujuAppError(f"{app_name!r} has errored: {app.status.info!r}") + + for unit_name, unit in units.items(): + assert unit.workload_status + if unit.workload_status.status == "blocked" and raise_on_blocked: + raise JujuUnitError( + f"{unit_name!r} workload is blocked: {unit.workload_status.info!r}" + ) + + for app_name in apps: + app = full_status.applications[app_name] + assert isinstance(app, ApplicationStatus) + assert app.status + if app.status.status == "blocked" and raise_on_blocked: + raise JujuAppError(f"{app_name!r} is blocked: {app.status.info!r}") + + 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(): + rv.units.add(unit_name) + assert unit.agent_status + assert unit.workload_status + + if unit.agent_status.status != "idle": + continue + if status and unit.workload_status.status != status: + continue + + ready_units.append(unit) + rv.ready_units.add(unit_name) + + # FIXME + # rv.idle_units -- depends on agent status only, not workload status + return rv + + +def _app_units(full_status: FullStatus, app_name: str) -> dict[str, UnitStatus]: + """Fish out the app's units' status from a FullStatus response.""" + rv: dict[str, UnitStatus] = {} + app = full_status.applications[app_name] + assert isinstance(app, ApplicationStatus) + + if app.subordinate_to: + parent_name = app.subordinate_to[0] + parent = full_status.applications[parent_name] + assert isinstance(parent, ApplicationStatus) + for parent_unit in parent.units.values(): + assert isinstance(parent_unit, UnitStatus) + for name, unit in parent_unit.subordinates.items(): + if not name.startswith(f"{app_name}/"): + continue + assert isinstance(unit, UnitStatus) + rv[name] = unit + else: + for name, unit in app.units.items(): + assert isinstance(unit, UnitStatus) + rv[name] = unit + + return rv diff --git a/juju/unit.py b/juju/unit.py index 4cb8e2591..64e14ac6d 100644 --- a/juju/unit.py +++ b/juju/unit.py @@ -15,7 +15,9 @@ class Unit(model.ModelEntity): - name: str + @property + def name(self) -> str: + return self.entity_id @property def agent_status(self): diff --git a/juju/version.py b/juju/version.py index b3e36f021..e8d250a6e 100644 --- a/juju/version.py +++ b/juju/version.py @@ -6,4 +6,4 @@ DEFAULT_ARCHITECTURE = "amd64" -CLIENT_VERSION = "3.6.0.0" +CLIENT_VERSION = "3.6.1.0rc2" diff --git a/pyproject.toml b/pyproject.toml index b3c2d4277..6947ad82b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "juju" -version = "3.6.0.0" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION +version = "3.6.1.0rc2" # 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" } @@ -42,6 +42,7 @@ dev = [ "pytest", "pytest-asyncio", "Twine", + "freezegun", ] docs = [ "sphinx==5.3.0", @@ -226,8 +227,9 @@ ignore = [ # These are tentative include = ["**/*.py"] pythonVersion = "3.8.6" -typeCheckingMode = "strict" -useLibraryCodeForTypes = true +# FIXME understand the difference +#typeCheckingMode = "strict" +#useLibraryCodeForTypes = true reportGeneralTypeIssues = true [tool.pytest.ini_options] diff --git a/setup.py b/setup.py index 93825375b..57a713de6 100644 --- a/setup.py +++ b/setup.py @@ -40,6 +40,7 @@ "pytest", "pytest-asyncio", "Twine", + "freezegun", ] }, include_package_data=True, diff --git a/tests/unit/data/fullstatus.json b/tests/unit/data/fullstatus.json new file mode 100644 index 000000000..d470a8c17 --- /dev/null +++ b/tests/unit/data/fullstatus.json @@ -0,0 +1,322 @@ +{ + "request-id": 7, + "response": { + "applications": { + "grafana-agent-k8s": { + "base": { + "channel": "22.04/stable", + "name": "ubuntu" + }, + "can-upgrade-to": "", + "charm": "ch:arm64/jammy/grafana-agent-k8s-75", + "charm-channel": "latest/stable", + "charm-profile": "", + "charm-version": "", + "endpoint-bindings": { + "": "alpha", + "certificates": "alpha", + "grafana-cloud-config": "alpha", + "grafana-dashboards-consumer": "alpha", + "grafana-dashboards-provider": "alpha", + "logging-consumer": "alpha", + "logging-provider": "alpha", + "metrics-endpoint": "alpha", + "peers": "alpha", + "receive-ca-cert": "alpha", + "send-remote-write": "alpha", + "tracing": "alpha" + }, + "exposed": false, + "int": 1, + "life": "", + "meter-statuses": null, + "provider-id": "4ecc75be-f038-4452-b1af-640d1b46f1c6", + "public-address": "10.152.183.55", + "relations": { + "peers": [ + "grafana-agent-k8s" + ] + }, + "status": { + "data": {}, + "info": "installing agent", + "kind": "", + "life": "", + "since": "2024-09-30T07:44:15.63582531Z", + "status": "waiting", + "version": "" + }, + "subordinate-to": [], + "units": { + "grafana-agent-k8s/0": { + "address": "10.1.121.164", + "agent-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T07:44:15.469295423Z", + "status": "idle", + "version": "3.5.1" + }, + "charm": "", + "leader": true, + "machine": "", + "opened-ports": [], + "provider-id": "grafana-agent-k8s-0", + "public-address": "", + "subordinates": null, + "workload-status": { + "data": {}, + "info": "Missing incoming (\"requires\") relation: metrics-endpoint|logging-provider|grafana-dashboards-consumer", + "kind": "", + "life": "", + "since": "2024-09-30T07:43:41.649319444Z", + "status": "blocked", + "version": "" + }, + "workload-version": "0.35.2" + } + }, + "workload-version": "0.35.2" + }, + "hexanator": { + "base": { + "channel": "24.04/stable", + "name": "ubuntu" + }, + "can-upgrade-to": "", + "charm": "local:noble/hexanator-1", + "charm-profile": "", + "charm-version": "", + "endpoint-bindings": { + "": "alpha", + "ingress": "alpha", + "rate-limit": "alpha" + }, + "exposed": false, + "int": 1, + "life": "", + "meter-statuses": null, + "provider-id": "b5efccf2-5a15-41a0-af0f-689a8d93a129", + "public-address": "10.152.183.113", + "relations": {}, + "status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T00:12:47.878239549Z", + "status": "active", + "version": "" + }, + "subordinate-to": [], + "units": { + "hexanator/0": { + "address": "10.1.121.184", + "agent-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T00:13:16.731257044Z", + "status": "idle", + "version": "3.5.1" + }, + "charm": "", + "leader": true, + "machine": "", + "opened-ports": [], + "provider-id": "hexanator-0", + "public-address": "", + "subordinates": null, + "workload-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T00:12:47.878239549Z", + "status": "active", + "version": "" + }, + "workload-version": "" + } + }, + "workload-version": "" + }, + "mysql-test-app": { + "base": { + "channel": "22.04/stable", + "name": "ubuntu" + }, + "can-upgrade-to": "", + "charm": "ch:arm64/jammy/mysql-test-app-62", + "charm-channel": "latest/edge", + "charm-profile": "", + "charm-version": "", + "endpoint-bindings": { + "": "alpha", + "application-peers": "alpha", + "database": "alpha", + "mysql": "alpha" + }, + "exposed": false, + "int": 2, + "life": "", + "meter-statuses": null, + "provider-id": "4338786a-a337-4779-820d-679a59ba1665", + "public-address": "10.152.183.118", + "relations": { + "application-peers": [ + "mysql-test-app" + ] + }, + "status": { + "data": {}, + "info": "installing agent", + "kind": "", + "life": "", + "since": "2024-09-30T07:48:25.106109123Z", + "status": "waiting", + "version": "" + }, + "subordinate-to": [], + "units": { + "mysql-test-app/0": { + "address": "10.1.121.142", + "agent-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-10-01T00:15:03.216904329Z", + "status": "idle", + "version": "3.5.1" + }, + "charm": "", + "leader": true, + "machine": "", + "opened-ports": [], + "provider-id": "mysql-test-app-0", + "public-address": "", + "subordinates": null, + "workload-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T07:47:54.212959856Z", + "status": "waiting", + "version": "" + }, + "workload-version": "0.0.2" + }, + "mysql-test-app/1": { + "address": "10.1.121.190", + "agent-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T23:49:39.923901864Z", + "status": "idle", + "version": "3.5.1" + }, + "charm": "", + "machine": "", + "opened-ports": [], + "provider-id": "mysql-test-app-1", + "public-address": "", + "subordinates": null, + "workload-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T07:47:54.211414881Z", + "status": "waiting", + "version": "" + }, + "workload-version": "0.0.2" + } + }, + "workload-version": "0.0.2" + } + }, + "branches": {}, + "controller-timestamp": "2024-10-01T07:25:22.51380313Z", + "machines": {}, + "model": { + "available-version": "", + "cloud-tag": "cloud-microk8s", + "meter-status": { + "color": "", + "message": "" + }, + "model-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-27T08:21:45.368693216Z", + "status": "available", + "version": "" + }, + "name": "testm", + "region": "localhost", + "sla": "unsupported", + "type": "caas", + "version": "3.5.1" + }, + "offers": {}, + "relations": [ + { + "endpoints": [ + { + "application": "grafana-agent-k8s", + "name": "peers", + "role": "peer", + "subordinate": false + } + ], + "id": 0, + "interface": "grafana_agent_replica", + "key": "grafana-agent-k8s:peers", + "scope": "global", + "status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T07:43:31.018463595Z", + "status": "joined", + "version": "" + } + }, + { + "endpoints": [ + { + "application": "mysql-test-app", + "name": "application-peers", + "role": "peer", + "subordinate": false + } + ], + "id": 1, + "interface": "application-peers", + "key": "mysql-test-app:application-peers", + "scope": "global", + "status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T07:47:52.823202648Z", + "status": "joined", + "version": "" + } + } + ], + "remote-applications": {} + } +} diff --git a/tests/unit/data/subordinate-fullstatus.json b/tests/unit/data/subordinate-fullstatus.json new file mode 100644 index 000000000..12c66d703 --- /dev/null +++ b/tests/unit/data/subordinate-fullstatus.json @@ -0,0 +1,254 @@ +{ + "request-id": 2618, + "response": { + "model": { + "name": "test-7442-test-subordinate-units-8b20", + "type": "iaas", + "cloud-tag": "cloud-localhost", + "region": "localhost", + "version": "3.6.0", + "available-version": "", + "model-status": { + "status": "available", + "info": "", + "data": {}, + "since": "2024-12-04T01:41:47.4040454Z", + "kind": "", + "version": "", + "life": "" + }, + "meter-status": {"color": "", "message": ""}, + "sla": "unsupported" + }, + "machines": { + "0": { + "agent-status": { + "status": "started", + "info": "", + "data": {}, + "since": "2024-12-04T01:43:51.558449988Z", + "kind": "", + "version": "3.6.0", + "life": "" + }, + "instance-status": { + "status": "running", + "info": "Running", + "data": {}, + "since": "2024-12-04T01:42:38.710685177Z", + "kind": "", + "version": "", + "life": "" + }, + "modification-status": { + "status": "applied", + "info": "", + "data": {}, + "since": "2024-12-04T01:42:26.414748546Z", + "kind": "", + "version": "", + "life": "" + }, + "hostname": "juju-eb2c2c-0", + "dns-name": "10.149.76.219", + "ip-addresses": ["10.149.76.219"], + "instance-id": "juju-eb2c2c-0", + "display-name": "", + "base": {"name": "ubuntu", "channel": "22.04/stable"}, + "id": "0", + "network-interfaces": { + "eth0": { + "ip-addresses": ["10.149.76.219"], + "mac-address": "00:16:3e:06:13:5f", + "gateway": "10.149.76.1", + "space": "alpha", + "is-up": true + } + }, + "containers": {}, + "constraints": "arch=amd64", + "hardware": "arch=amd64 cores=0 mem=0M virt-type=container", + "jobs": ["JobHostUnits"], + "has-vote": false, + "wants-vote": false + } + }, + "applications": { + "ntp": { + "charm": "ch:amd64/ntp-50", + "charm-version": "cs-ntp-team-ntp-4-171-g669ff59", + "charm-profile": "", + "charm-channel": "latest/stable", + "charm-rev": 50, + "base": {"name": "ubuntu", "channel": "20.04/stable"}, + "exposed": false, + "life": "", + "relations": {"juju-info": ["ubuntu"], "ntp-peers": ["ntp"]}, + "can-upgrade-to": "", + "subordinate-to": ["ubuntu"], + "units": null, + "meter-statuses": null, + "status": { + "status": "active", + "info": "chrony: Ready, Failed to disable Hyper-V host sync", + "data": {}, + "since": "2024-12-04T01:44:40.346093963Z", + "kind": "", + "version": "", + "life": "" + }, + "workload-version": "4.2", + "endpoint-bindings": { + "": "alpha", + "juju-info": "alpha", + "master": "alpha", + "nrpe-external-master": "alpha", + "ntp-peers": "alpha", + "ntpmaster": "alpha" + }, + "public-address": "" + }, + "ubuntu": { + "charm": "ch:amd64/ubuntu-25", + "charm-version": "", + "charm-profile": "", + "charm-channel": "latest/stable", + "charm-rev": 25, + "base": {"name": "ubuntu", "channel": "22.04/stable"}, + "exposed": false, + "life": "", + "relations": {"juju-info": ["ntp"]}, + "can-upgrade-to": "", + "subordinate-to": [], + "units": { + "ubuntu/0": { + "agent-status": { + "status": "idle", + "info": "", + "data": {}, + "since": "2024-12-04T01:44:44.342778729Z", + "kind": "", + "version": "3.6.0", + "life": "" + }, + "workload-status": { + "status": "active", + "info": "", + "data": {}, + "since": "2024-12-04T01:43:53.391031729Z", + "kind": "", + "version": "", + "life": "" + }, + "workload-version": "22.04", + "machine": "0", + "opened-ports": null, + "public-address": "10.149.76.219", + "charm": "", + "subordinates": { + "ntp/0": { + "agent-status": { + "status": "idle", + "info": "", + "data": {}, + "since": "2024-12-04T01:44:47.418242454Z", + "kind": "", + "version": "3.6.0", + "life": "" + }, + "workload-status": { + "status": "active", + "info": "chrony: Ready, Failed to disable Hyper-V host sync", + "data": {}, + "since": "2024-12-04T01:44:40.346093963Z", + "kind": "", + "version": "", + "life": "" + }, + "workload-version": "4.2", + "machine": "", + "opened-ports": ["123/udp"], + "public-address": "10.149.76.219", + "charm": "", + "subordinates": null, + "leader": true + } + }, + "leader": true + } + }, + "meter-statuses": null, + "status": { + "status": "active", + "info": "", + "data": {}, + "since": "2024-12-04T01:43:53.391031729Z", + "kind": "", + "version": "", + "life": "" + }, + "workload-version": "22.04", + "endpoint-bindings": null, + "public-address": "" + } + }, + "remote-applications": {}, + "offers": {}, + "relations": [ + { + "id": 0, + "key": "ntp:ntp-peers", + "interface": "ntp", + "scope": "global", + "endpoints": [ + { + "application": "ntp", + "name": "ntp-peers", + "role": "peer", + "subordinate": false + } + ], + "status": { + "status": "joined", + "info": "", + "data": {}, + "since": "2024-12-04T01:44:43.940973679Z", + "kind": "", + "version": "", + "life": "" + } + }, + { + "id": 1, + "key": "ntp:juju-info ubuntu:juju-info", + "interface": "juju-info", + "scope": "container", + "endpoints": [ + { + "application": "ubuntu", + "name": "juju-info", + "role": "provider", + "subordinate": false + }, + { + "application": "ntp", + "name": "juju-info", + "role": "requirer", + "subordinate": true + } + ], + "status": { + "status": "joined", + "info": "", + "data": {}, + "since": "2024-12-04T01:43:53.72325443Z", + "kind": "", + "version": "", + "life": "" + } + } + ], + "controller-timestamp": "2024-12-04T02:01:53.569630593Z", + "branches": {} + } +} diff --git a/tests/unit/test_idle_check.py b/tests/unit/test_idle_check.py new file mode 100644 index 000000000..8ed89e4f0 --- /dev/null +++ b/tests/unit/test_idle_check.py @@ -0,0 +1,273 @@ +# Copyright 2024 Canonical Ltd. +# Licensed under the Apache V2, see LICENCE file for details. +from __future__ import annotations + +import copy +import json +from typing import Any + +import pytest + +from juju.client._definitions import ( + FullStatus, +) +from juju.client.facade import _convert_response +from juju.errors import JujuAgentError, JujuAppError, JujuMachineError, JujuUnitError +from juju.model._idle import CheckStatus, check + + +def test_check_status(full_status: FullStatus, kwargs): + status = check(full_status, **kwargs) + assert status == CheckStatus( + { + "grafana-agent-k8s/0", + "hexanator/0", + "mysql-test-app/0", + "mysql-test-app/1", + }, + { + "grafana-agent-k8s/0", + "hexanator/0", + "mysql-test-app/0", + "mysql-test-app/1", + }, + set(), + ) + + +def test_check_status_missing_app(full_status: FullStatus, kwargs): + kwargs["apps"] = ["missing", "hexanator"] + status = check(full_status, **kwargs) + assert status is None + + +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()) + + +def test_no_apps(full_status: FullStatus, kwargs): + kwargs["apps"] = [] + status = check(full_status, **kwargs) + assert status == CheckStatus(set(), set(), set()) + + +def test_missing_app(full_status: FullStatus, kwargs): + kwargs["apps"] = ["missing"] + status = check(full_status, **kwargs) + assert status is None + + +def test_no_units(response: dict[str, Any], kwargs): + response["response"]["applications"]["hexanator"]["units"].clear() + kwargs["apps"] = ["hexanator"] + status = check(convert(response), **kwargs) + assert status == CheckStatus(set(), set(), set()) + + +def test_app_error(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["status"]["status"] = "error" + app["status"]["info"] = "big problem" + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_error"] = True + + with pytest.raises(JujuAppError) as e: + check(convert(response), **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(), + ) + + +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()) + + +def test_ready_unit_requires_idle_agent(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/1"] = copy.deepcopy(app["units"]["hexanator/0"]) + app["units"]["hexanator/1"]["agent-status"]["status"] = "some-other" + + kwargs["apps"] = ["hexanator"] + kwargs["status"] = "active" + + status = check(convert(response), **kwargs) + assert status + assert status.units == {"hexanator/0", "hexanator/1"} + assert status.ready_units == {"hexanator/0"} + + +def test_ready_unit_requires_workload_status(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/1"] = copy.deepcopy(app["units"]["hexanator/0"]) + app["units"]["hexanator/1"]["workload-status"]["status"] = "some-other" + + kwargs["apps"] = ["hexanator"] + kwargs["status"] = "active" + + status = check(convert(response), **kwargs) + assert status == CheckStatus({"hexanator/0", "hexanator/1"}, {"hexanator/0"}, set()) + + +def test_agent_error(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/0"]["agent-status"]["status"] = "error" + app["units"]["hexanator/0"]["agent-status"]["info"] = "agent problem" + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_error"] = True + + with pytest.raises(JujuAgentError) as e: + check(convert(response), **kwargs) + + assert "hexanator/0" in str(e) + assert "agent problem" in str(e) + + +def test_workload_error(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/0"]["workload-status"]["status"] = "error" + app["units"]["hexanator/0"]["workload-status"]["info"] = "workload problem" + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_error"] = True + + with pytest.raises(JujuUnitError) as e: + check(convert(response), **kwargs) + + assert "hexanator/0" in str(e) + assert "workload problem" in str(e) + + +def test_machine_ok(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/0"]["machine"] = "42" + # https://github.com/dimaqq/juju-schema-analysis/blob/main/schemas-juju-3.5.4.txt#L3611-L3674 + response["response"]["machines"] = { + "42": { + "instance-status": { + "status": "running", + "info": "RUNNING", + }, + }, + } + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_error"] = True + + status = check(convert(response), **kwargs) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + + +def test_machine_error(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/0"]["machine"] = "42" + response["response"]["machines"] = { + "42": { + "instance-status": { + "status": "error", + "info": "Battery low. Try a potato?", + }, + }, + } + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_error"] = True + + with pytest.raises(JujuMachineError) as e: + check(convert(response), **kwargs) + + assert "potato" in str(e) + + +def test_app_blocked(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["status"]["status"] = "blocked" + app["status"]["info"] = "big problem" + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_blocked"] = True + + with pytest.raises(JujuAppError) as e: + check(convert(response), **kwargs) + + assert "big problem" in str(e) + + +def test_unit_blocked(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/0"]["workload-status"]["status"] = "blocked" + app["units"]["hexanator/0"]["workload-status"]["info"] = "small problem" + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_blocked"] = True + + with pytest.raises(JujuUnitError) as e: + check(convert(response), **kwargs) + + assert "small problem" in str(e) + + +def test_maintenance(response: dict[str, Any], kwargs): + """Taken from nginx-ingress-integrator-operator integration tests.""" + app = response["response"]["applications"]["hexanator"] + app["status"]["status"] = "maintenance" + app["units"]["hexanator/0"]["workload-status"]["status"] = "maintenance" + + kwargs["apps"] = ["hexanator"] + kwargs["status"] = "maintenance" + + status = check(convert(response), **kwargs) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + + +@pytest.fixture +def kwargs() -> dict[str, Any]: + return dict( + apps=["hexanator", "grafana-agent-k8s", "mysql-test-app"], + raise_on_error=False, + raise_on_blocked=False, + status=None, + ) + + +@pytest.fixture +def response(pytestconfig: pytest.Config) -> dict[str, Any]: + return json.loads( + (pytestconfig.rootpath / "tests/unit/data/fullstatus.json").read_text() + ) + + +def convert(data: dict[str, Any]) -> FullStatus: + return _convert_response(data, cls=FullStatus) + + +@pytest.fixture +def full_status(response) -> FullStatus: + return convert(response) diff --git a/tests/unit/test_idle_check_subordinate.py b/tests/unit/test_idle_check_subordinate.py new file mode 100644 index 000000000..149e468ba --- /dev/null +++ b/tests/unit/test_idle_check_subordinate.py @@ -0,0 +1,57 @@ +# Copyright 2024 Canonical Ltd. +# Licensed under the Apache V2, see LICENCE file for details. +from __future__ import annotations + +import json +from typing import Any + +import pytest + +from juju.client._definitions import ( + FullStatus, +) +from juju.client.facade import _convert_response +from juju.model._idle import CheckStatus, check + + +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()) + + +def test_subordinate_is_selective(response, kwargs): + subordinates = response["response"]["applications"]["ubuntu"]["units"]["ubuntu/0"][ + "subordinates" + ] + subordinates["some-other/0"] = subordinates["ntp/0"] + status = check(convert(response), **kwargs) + assert status == CheckStatus({"ntp/0", "ubuntu/0"}, {"ntp/0", "ubuntu/0"}, set()) + + +@pytest.fixture +def kwargs() -> dict[str, Any]: + return dict( + apps=["ntp", "ubuntu"], + raise_on_error=False, + raise_on_blocked=False, + status=None, + ) + + +@pytest.fixture +def response(pytestconfig: pytest.Config) -> dict[str, Any]: + """Juju rpc response JSON to a FullStatus call.""" + return json.loads( + ( + pytestconfig.rootpath / "tests/unit/data/subordinate-fullstatus.json" + ).read_text() + ) + + +@pytest.fixture +def subordinate_status(response) -> FullStatus: + return convert(response) + + +def convert(data: dict[str, Any]) -> FullStatus: + return _convert_response(data, cls=FullStatus) diff --git a/tests/unit/test_idle_loop.py b/tests/unit/test_idle_loop.py new file mode 100644 index 000000000..34a8c6721 --- /dev/null +++ b/tests/unit/test_idle_loop.py @@ -0,0 +1,61 @@ +# Copyright 2024 Canonical Ltd. +# Licensed under the Apache V2, see LICENCE file for details. +from __future__ import annotations + +import pytest +from freezegun import freeze_time + +from juju.model._idle import CheckStatus, loop + + +# Missing tests +# +# FIXME hexanator idle period 1 +# FIXME workload maintenance, idle period 0 +# FIXME test exact count == 2 +# FIXME test exact count != 2 (1, 3) +# FIXME exact count vs wait_for_units +# FIXME expected idle 1s below +# FIXME idle period 1 +# FIXME sending status=None, meaning some apps are still missing +# +@pytest.mark.xfail(reason="FIXME I misunderstood what 'idle' means") +async def test_at_least_units(): + async def checks(): + yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0"}, set()) + yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0", "u/1"}, set()) + yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0", "u/1", "u/2"}, set()) + + with freeze_time(): + assert [ + v + async for v in loop( + checks(), + apps=frozenset(["u"]), + wait_for_units=2, + idle_period=0, + ) + ] == [False, True, True] + + +async def test_ping_pong(): + good = CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + bad = CheckStatus({"hexanator/0"}, set(), set()) + + async def checks(): + with freeze_time() as clock: + for _ in range(3): + yield good + clock.tick(10) + yield bad + clock.tick(10) + + assert [ + v + async for v in loop( + checks(), + apps=frozenset(["hexanator"]), + wait_for_units=1, + idle_period=15, + ) + ] == [False] * 6 diff --git a/tests/unit/test_model.py b/tests/unit/test_model.py index 091043217..cde663a58 100644 --- a/tests/unit/test_model.py +++ b/tests/unit/test_model.py @@ -11,7 +11,7 @@ from juju.application import Application from juju.client.jujudata import FileJujuData -from juju.errors import JujuConnectionError, JujuError +from juju.errors import JujuConnectionError from juju.model import Model @@ -251,19 +251,19 @@ async def test_no_args(self): # no apps so should return right away await m.wait_for_idle(wait_for_active=True) - async def test_apps_no_lst(self): + async def test_apps_type(self): m = Model() - with self.assertRaises(JujuError): + with self.assertRaises(TypeError): # apps arg has to be a List[str] await m.wait_for_idle(apps="should-be-list") - with self.assertRaises(JujuError): + with self.assertRaises(TypeError): # apps arg has to be a List[str] - await m.wait_for_idle(apps=3) + await m.wait_for_idle(apps=3) # type: ignore - with self.assertRaises(JujuError): + with self.assertRaises(TypeError): # apps arg has to be a List[str] - await m.wait_for_idle(apps=[3]) + await m.wait_for_idle(apps=[3]) # type: ignore async def test_timeout(self): m = Model() @@ -271,7 +271,8 @@ async def test_timeout(self): # no apps so should timeout after timeout period await m.wait_for_idle(apps=["nonexisting_app"]) self.assertEqual( - str(cm.exception), "Timed out waiting for model:\nnonexisting_app (missing)" + str(cm.exception), + "Timed out waiting for model:\n nonexisting_app (missing)", ) @pytest.mark.wait_for_idle diff --git a/tests/unit/test_unit.py b/tests/unit/test_unit.py index 432b5fd1f..4023a170d 100644 --- a/tests/unit/test_unit.py +++ b/tests/unit/test_unit.py @@ -81,7 +81,7 @@ async def test_unit_is_leader(mock_cf): client_facade.FullStatus = mock.AsyncMock(return_value=status) unit = Unit("test", model) - unit.name = test["unit"] + unit.entity_id = test["unit"] rval = await unit.is_leader_from_status() assert rval == test["rval"] diff --git a/tox.ini b/tox.ini index d284673b9..2d6760296 100644 --- a/tox.ini +++ b/tox.ini @@ -17,6 +17,7 @@ passenv = HOME TEST_AGENTS LXD_DIR + JUJU_NEW_WAIT_FOR_IDLE [testenv:docs] deps = From 065d3deb0d6d07d5b49e4bd2f5c47139ad14d7a7 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 12:09:58 +0900 Subject: [PATCH 02/19] chore: remplement best guess for idle timer --- juju/model/__init__.py | 4 ++- juju/model/_idle.py | 42 ++++++++++------------- juju/version.py | 2 +- pyproject.toml | 2 +- tests/unit/test_idle_check.py | 40 ++++++++++----------- tests/unit/test_idle_check_subordinate.py | 12 +++++-- 6 files changed, 51 insertions(+), 51 deletions(-) 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 From 138d8f9058e1023e01a01d587b81949433da61ce Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 12:39:27 +0900 Subject: [PATCH 03/19] chore: better logging in integration tests --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 2d6760296..beae69568 100644 --- a/tox.ini +++ b/tox.ini @@ -60,7 +60,7 @@ commands = # it doesn't get run in CI envdir = {toxworkdir}/py3 commands = - pytest --tb native -s {posargs:-m 'serial'} + pytest --tb native {posargs:-m 'serial'} [testenv:validate] envdir = {toxworkdir}/validate From 1951425e935ab30fe2906d92cfc4ad523e572686 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 13:20:47 +0900 Subject: [PATCH 04/19] chore: studpi bug --- juju/model/__init__.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/juju/model/__init__.py b/juju/model/__init__.py index f40738b1d..951713a78 100644 --- a/juju/model/__init__.py +++ b/juju/model/__init__.py @@ -3302,13 +3302,14 @@ async def new_wait_for_idle( deadline = None if timeout is None else started + timeout async def status_on_demand(): - yield _idle.check( - await self.get_status(), - apps=apps, - raise_on_error=raise_on_error, - raise_on_blocked=raise_on_blocked, - status=status, - ) + while True: + yield _idle.check( + await self.get_status(), + apps=apps, + raise_on_error=raise_on_error, + raise_on_blocked=raise_on_blocked, + status=status, + ) async for done in _idle.loop( status_on_demand(), From c6087a308c4fdda9f64355ea5d7ea863895dce7e Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 15:21:51 +0900 Subject: [PATCH 05/19] fix: async filter should output only one result for each input --- juju/model/_idle.py | 8 +++--- juju/version.py | 2 +- pyproject.toml | 2 +- tests/unit/test_idle_loop.py | 49 ++++++++++++++++++++++++++++++------ tox.ini | 1 + 5 files changed, 49 insertions(+), 13 deletions(-) diff --git a/juju/model/_idle.py b/juju/model/_idle.py index 6737fe127..7ac32c76f 100644 --- a/juju/model/_idle.py +++ b/juju/model/_idle.py @@ -80,7 +80,7 @@ async def loop( wait_for_units, ) yield False - continue + break if ( wait_for_exact_units is not None @@ -93,9 +93,9 @@ async def loop( wait_for_exact_units, ) yield False - continue - - yield True + break + else: + yield True def check( diff --git a/juju/version.py b/juju/version.py index 86177ffd4..c0cd876c6 100644 --- a/juju/version.py +++ b/juju/version.py @@ -6,4 +6,4 @@ DEFAULT_ARCHITECTURE = "amd64" -CLIENT_VERSION = "3.6.1.0rc3" +CLIENT_VERSION = "3.6.1.0rc4" diff --git a/pyproject.toml b/pyproject.toml index d2eec1cc6..6eedab937 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "juju" -version = "3.6.1.0rc3" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION +version = "3.6.1.0rc4" # 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_loop.py b/tests/unit/test_idle_loop.py index 34a8c6721..491231df4 100644 --- a/tests/unit/test_idle_loop.py +++ b/tests/unit/test_idle_loop.py @@ -2,12 +2,10 @@ # Licensed under the Apache V2, see LICENCE file for details. from __future__ import annotations -import pytest from freezegun import freeze_time from juju.model._idle import CheckStatus, loop - # Missing tests # # FIXME hexanator idle period 1 @@ -18,13 +16,15 @@ # FIXME expected idle 1s below # FIXME idle period 1 # FIXME sending status=None, meaning some apps are still missing -# -@pytest.mark.xfail(reason="FIXME I misunderstood what 'idle' means") + + async def test_at_least_units(): async def checks(): - yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0"}, set()) - yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0", "u/1"}, set()) - yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0", "u/1", "u/2"}, set()) + yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0"}, {"u/0", "u/1", "u/2"}) + yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0", "u/1"}, {"u/0", "u/1", "u/2"}) + yield CheckStatus( + {"u/0", "u/1", "u/2"}, {"u/0", "u/1", "u/2"}, {"u/0", "u/1", "u/2"} + ) with freeze_time(): assert [ @@ -38,6 +38,41 @@ async def checks(): ] == [False, True, True] +async def test_for_exact_units(): + good = CheckStatus( + {"u/0", "u/1", "u/2"}, + {"u/1", "u/2"}, + {"u/0", "u/1", "u/2"}, + ) + too_few = CheckStatus( + {"u/0", "u/1", "u/2"}, + {"u/2"}, + {"u/0", "u/1", "u/2"}, + ) + too_many = CheckStatus( + {"u/0", "u/1", "u/2"}, + {"u/1", "u/2", "u/0"}, + {"u/0", "u/1", "u/2"}, + ) + + async def checks(): + yield too_few + yield good + yield too_many + yield good + + assert [ + v + async for v in loop( + checks(), + apps=frozenset(["u"]), + wait_for_units=1, + wait_for_exact_units=2, + idle_period=0, + ) + ] == [False, True, False, True] + + async def test_ping_pong(): good = CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) bad = CheckStatus({"hexanator/0"}, set(), set()) diff --git a/tox.ini b/tox.ini index beae69568..5356d45fa 100644 --- a/tox.ini +++ b/tox.ini @@ -43,6 +43,7 @@ commands = envdir = {toxworkdir}/py3 commands = pytest \ + --log-level=DEBUG \ --tb native \ -m 'not serial' \ {posargs} \ From 3e29b2b8b0aa148dd4e32ca5e157ea46d855da0d Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 16:13:01 +0900 Subject: [PATCH 06/19] chore: add missing tests and refactor for readability --- tests/unit/test_idle_loop.py | 76 +++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 28 deletions(-) diff --git a/tests/unit/test_idle_loop.py b/tests/unit/test_idle_loop.py index 491231df4..eaee5aa0b 100644 --- a/tests/unit/test_idle_loop.py +++ b/tests/unit/test_idle_loop.py @@ -6,16 +6,24 @@ from juju.model._idle import CheckStatus, loop -# Missing tests -# -# FIXME hexanator idle period 1 -# FIXME workload maintenance, idle period 0 -# FIXME test exact count == 2 -# FIXME test exact count != 2 (1, 3) -# FIXME exact count vs wait_for_units -# FIXME expected idle 1s below -# FIXME idle period 1 -# FIXME sending status=None, meaning some apps are still missing + +async def alist(agen): + return [v async for v in agen] + + +async def test_wait_for_apps(): + async def checks(): + yield None + yield None + + assert await alist( + loop( + checks(), + apps=frozenset(["a"]), + wait_for_units=0, + idle_period=0, + ) + ) == [False, False] async def test_at_least_units(): @@ -27,15 +35,14 @@ async def checks(): ) with freeze_time(): - assert [ - v - async for v in loop( + assert await alist( + loop( checks(), apps=frozenset(["u"]), wait_for_units=2, idle_period=0, ) - ] == [False, True, True] + ) == [False, True, True] async def test_for_exact_units(): @@ -61,36 +68,49 @@ async def checks(): yield too_many yield good - assert [ - v - async for v in loop( + assert await alist( + loop( checks(), apps=frozenset(["u"]), wait_for_units=1, wait_for_exact_units=2, idle_period=0, ) - ] == [False, True, False, True] + ) == [False, True, False, True] -async def test_ping_pong(): - good = CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) - bad = CheckStatus({"hexanator/0"}, set(), set()) +async def test_idle_ping_pong(): + good = CheckStatus({"hexanator/0"}, {"hexanator/0"}, {"hexanator/0"}) + bad = CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) async def checks(): with freeze_time() as clock: - for _ in range(3): - yield good + for status in [good, bad, good, bad]: + yield status clock.tick(10) - yield bad + + assert await alist( + loop( + checks(), + apps=frozenset(["hexanator"]), + wait_for_units=1, + idle_period=15, + ) + ) == [False, False, False, False] + + +async def test_idle_period(): + async def checks(): + with freeze_time() as clock: + for _ in range(4): + yield CheckStatus({"hexanator/0"}, {"hexanator/0"}, {"hexanator/0"}) clock.tick(10) - assert [ - v - async for v in loop( + assert await alist( + loop( checks(), apps=frozenset(["hexanator"]), wait_for_units=1, idle_period=15, ) - ] == [False] * 6 + ) == [False, False, True, True] From 4276d99ca99b22cc552a1840e196da6b9088e35d Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 17:01:27 +0900 Subject: [PATCH 07/19] chore: slightly better type hints --- juju/model/_idle.py | 6 +++--- tests/unit/test_idle_loop.py | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/juju/model/_idle.py b/juju/model/_idle.py index 7ac32c76f..7ca0cac93 100644 --- a/juju/model/_idle.py +++ b/juju/model/_idle.py @@ -7,7 +7,7 @@ import logging import time from dataclasses import dataclass -from typing import AsyncIterable +from typing import AbstractSet, AsyncIterable from ..client._definitions import ( ApplicationStatus, @@ -35,7 +35,7 @@ class CheckStatus: async def loop( foo: AsyncIterable[CheckStatus | None], *, - apps: frozenset[str], + apps: AbstractSet[str], wait_for_exact_units: int | None = None, wait_for_units: int, idle_period: float, @@ -101,7 +101,7 @@ async def loop( def check( full_status: FullStatus, *, - apps: frozenset[str], + apps: AbstractSet[str], raise_on_error: bool, raise_on_blocked: bool, status: str | None, diff --git a/tests/unit/test_idle_loop.py b/tests/unit/test_idle_loop.py index eaee5aa0b..2e97bd67e 100644 --- a/tests/unit/test_idle_loop.py +++ b/tests/unit/test_idle_loop.py @@ -19,7 +19,7 @@ async def checks(): assert await alist( loop( checks(), - apps=frozenset(["a"]), + apps={"a"}, wait_for_units=0, idle_period=0, ) @@ -38,7 +38,7 @@ async def checks(): assert await alist( loop( checks(), - apps=frozenset(["u"]), + apps={"u"}, wait_for_units=2, idle_period=0, ) @@ -71,7 +71,7 @@ async def checks(): assert await alist( loop( checks(), - apps=frozenset(["u"]), + apps={"u"}, wait_for_units=1, wait_for_exact_units=2, idle_period=0, @@ -92,7 +92,7 @@ async def checks(): assert await alist( loop( checks(), - apps=frozenset(["hexanator"]), + apps={"hexanator"}, wait_for_units=1, idle_period=15, ) @@ -109,7 +109,7 @@ async def checks(): assert await alist( loop( checks(), - apps=frozenset(["hexanator"]), + apps={"hexanator"}, wait_for_units=1, idle_period=15, ) From d7abe10ed5cb7e705c432a956d10a0499ac499e2 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 17:20:16 +0900 Subject: [PATCH 08/19] chore: note on converted responses --- juju/client/facade.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/juju/client/facade.py b/juju/client/facade.py index 7ab4fddd0..6fd349347 100644 --- a/juju/client/facade.py +++ b/juju/client/facade.py @@ -499,10 +499,14 @@ def _convert_response(response: dict[str, Any], *, cls: type[Type] | None) -> An if cls is None: return response if "error" in response: + # TODO: I don't think this ever happens, + # errors are handled by Connection.rpc(), + # though, admittedly the shape is different. cls = CLASSES["Error"] if typing_inspect.is_generic_type(cls) and issubclass( typing_inspect.get_origin(cls), Sequence ): + # TODO: I'm not sure this ever happens either. parameters = typing_inspect.get_parameters(cls) result = [] item_cls = parameters[0] From c200694ab4a924dd3b29f299315d4c4e257aed94 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 19 Dec 2024 10:58:29 +0900 Subject: [PATCH 09/19] chore: refactor using plain old classes --- juju/model/__init__.py | 20 +++---- juju/model/_idle.py | 68 +++++++++++----------- tests/unit/test_idle_loop.py | 107 ++++++++++++++++++----------------- 3 files changed, 101 insertions(+), 94 deletions(-) diff --git a/juju/model/__init__.py b/juju/model/__init__.py index 951713a78..826cff886 100644 --- a/juju/model/__init__.py +++ b/juju/model/__init__.py @@ -3300,24 +3300,24 @@ async def new_wait_for_idle( started = time.monotonic() deadline = None if timeout is None else started + timeout + loop = _idle.Loop( + apps=apps, + wait_for_exact_units=wait_for_exact_units, + wait_for_units=wait_for_units, + idle_period=idle_period, + ) - async def status_on_demand(): - while True: - yield _idle.check( + while True: + done = loop.next( + _idle.check( await self.get_status(), apps=apps, raise_on_error=raise_on_error, raise_on_blocked=raise_on_blocked, status=status, ) + ) - async for done in _idle.loop( - status_on_demand(), - apps=apps, - wait_for_exact_units=wait_for_exact_units, - 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 7ca0cac93..f73539fa1 100644 --- a/juju/model/_idle.py +++ b/juju/model/_idle.py @@ -7,7 +7,7 @@ import logging import time from dataclasses import dataclass -from typing import AbstractSet, AsyncIterable +from typing import AbstractSet from ..client._definitions import ( ApplicationStatus, @@ -32,26 +32,29 @@ class CheckStatus: """Units with stable agent status (FIXME details).""" -async def loop( - foo: AsyncIterable[CheckStatus | None], - *, - apps: AbstractSet[str], - wait_for_exact_units: int | None = None, - wait_for_units: int, - idle_period: float, -) -> AsyncIterable[bool]: - """The outer, time-dependents logic of a wait_for_idle loop.""" - idle_since: dict[str, float] = {} - - async for status in foo: +class Loop: + def __init__( + self, + *, + apps: AbstractSet[str], + wait_for_exact_units: int | None = None, + wait_for_units: int, + idle_period: float, + ): + self.apps = apps + self.wait_for_exact_units = wait_for_exact_units + self.wait_for_units = wait_for_units + self.idle_period = idle_period + self.idle_since: dict[str, float] = {} + + def next(self, status: CheckStatus | None) -> bool: logger.info("wait_for_idle iteration %s", status) now = time.monotonic() if not status: - yield False - continue + return False - expected_idle_since = now - idle_period + expected_idle_since = now - self.idle_period # FIXME there's some confusion about what a "busy" unit is # are we ready when over the last idle_period, every time sampled: @@ -59,43 +62,42 @@ async def loop( # b. >=N units were ready each time for name in status.units: if name in status.idle_units: - idle_since[name] = min(now, idle_since.get(name, float("inf"))) + self.idle_since[name] = min( + now, self.idle_since.get(name, float("inf")) + ) else: - idle_since[name] = float("inf") + self.idle_since[name] = float("inf") - if busy := {n for n, t in idle_since.items() if t > expected_idle_since}: + if busy := {n for n, t in self.idle_since.items() if t > expected_idle_since}: logger.info("Waiting for units to be idle enough: %s", busy) - yield False - continue + return False - for app_name in apps: + for app_name in self.apps: ready_units = [ n for n in status.ready_units if n.startswith(f"{app_name}/") ] - if len(ready_units) < wait_for_units: + if len(ready_units) < self.wait_for_units: logger.info( "Waiting for app %r units %s >= %s", app_name, len(status.ready_units), - wait_for_units, + self.wait_for_units, ) - yield False - break + return False if ( - wait_for_exact_units is not None - and len(ready_units) != wait_for_exact_units + self.wait_for_exact_units is not None + and len(ready_units) != self.wait_for_exact_units ): logger.info( "Waiting for app %r units %s == %s", app_name, len(ready_units), - wait_for_exact_units, + self.wait_for_exact_units, ) - yield False - break - else: - yield True + return False + + return True def check( diff --git a/tests/unit/test_idle_loop.py b/tests/unit/test_idle_loop.py index 2e97bd67e..300c50f35 100644 --- a/tests/unit/test_idle_loop.py +++ b/tests/unit/test_idle_loop.py @@ -2,32 +2,45 @@ # Licensed under the Apache V2, see LICENCE file for details. from __future__ import annotations -from freezegun import freeze_time - -from juju.model._idle import CheckStatus, loop +from typing import AbstractSet, Iterable +from freezegun import freeze_time -async def alist(agen): - return [v async for v in agen] +from juju.model._idle import CheckStatus, Loop + + +def unroll( + statuses: Iterable[CheckStatus | None], + *, + apps: AbstractSet[str], + wait_for_exact_units: int | None = None, + wait_for_units: int, + idle_period: float, +) -> list[bool]: + loop = Loop( + apps=apps, + wait_for_exact_units=wait_for_exact_units, + wait_for_units=wait_for_units, + idle_period=idle_period, + ) + return [loop.next(s) for s in statuses] -async def test_wait_for_apps(): - async def checks(): +def test_wait_for_apps(): + def checks(): yield None yield None - assert await alist( - loop( - checks(), - apps={"a"}, - wait_for_units=0, - idle_period=0, - ) + assert unroll( + checks(), + apps={"a"}, + wait_for_units=0, + idle_period=0, ) == [False, False] -async def test_at_least_units(): - async def checks(): +def test_at_least_units(): + def checks(): yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0"}, {"u/0", "u/1", "u/2"}) yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0", "u/1"}, {"u/0", "u/1", "u/2"}) yield CheckStatus( @@ -35,17 +48,15 @@ async def checks(): ) with freeze_time(): - assert await alist( - loop( - checks(), - apps={"u"}, - wait_for_units=2, - idle_period=0, - ) + assert unroll( + checks(), + apps={"u"}, + wait_for_units=2, + idle_period=0, ) == [False, True, True] -async def test_for_exact_units(): +def test_for_exact_units(): good = CheckStatus( {"u/0", "u/1", "u/2"}, {"u/1", "u/2"}, @@ -62,55 +73,49 @@ async def test_for_exact_units(): {"u/0", "u/1", "u/2"}, ) - async def checks(): + def checks(): yield too_few yield good yield too_many yield good - assert await alist( - loop( - checks(), - apps={"u"}, - wait_for_units=1, - wait_for_exact_units=2, - idle_period=0, - ) + assert unroll( + checks(), + apps={"u"}, + wait_for_units=1, + wait_for_exact_units=2, + idle_period=0, ) == [False, True, False, True] -async def test_idle_ping_pong(): +def test_idle_ping_pong(): good = CheckStatus({"hexanator/0"}, {"hexanator/0"}, {"hexanator/0"}) bad = CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) - async def checks(): + def checks(): with freeze_time() as clock: for status in [good, bad, good, bad]: yield status clock.tick(10) - assert await alist( - loop( - checks(), - apps={"hexanator"}, - wait_for_units=1, - idle_period=15, - ) + assert unroll( + checks(), + apps={"hexanator"}, + wait_for_units=1, + idle_period=15, ) == [False, False, False, False] -async def test_idle_period(): - async def checks(): +def test_idle_period(): + def checks(): with freeze_time() as clock: for _ in range(4): yield CheckStatus({"hexanator/0"}, {"hexanator/0"}, {"hexanator/0"}) clock.tick(10) - assert await alist( - loop( - checks(), - apps={"hexanator"}, - wait_for_units=1, - idle_period=15, - ) + assert unroll( + checks(), + apps={"hexanator"}, + wait_for_units=1, + idle_period=15, ) == [False, False, True, True] From 7998263b0d7ed242a7f824db25f4516c508455ee Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 19 Dec 2024 11:02:51 +0900 Subject: [PATCH 10/19] chore: bump version --- juju/version.py | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/juju/version.py b/juju/version.py index c0cd876c6..0a4471059 100644 --- a/juju/version.py +++ b/juju/version.py @@ -6,4 +6,4 @@ DEFAULT_ARCHITECTURE = "amd64" -CLIENT_VERSION = "3.6.1.0rc4" +CLIENT_VERSION = "3.6.1.0rc5" diff --git a/pyproject.toml b/pyproject.toml index 6eedab937..1a651c4e3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "juju" -version = "3.6.1.0rc4" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION +version = "3.6.1.0rc5" # 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" } From cbb73efed9d2d5f40904f8a5829da44dbc911350 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 19 Dec 2024 16:05:12 +0900 Subject: [PATCH 11/19] chore: fix doc link --- tests/unit/test_idle_check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_idle_check.py b/tests/unit/test_idle_check.py index bba2cee4e..7efec3e3a 100644 --- a/tests/unit/test_idle_check.py +++ b/tests/unit/test_idle_check.py @@ -164,7 +164,7 @@ def test_workload_error(response: dict[str, Any], kwargs): def test_machine_ok(response: dict[str, Any], kwargs): app = response["response"]["applications"]["hexanator"] app["units"]["hexanator/0"]["machine"] = "42" - # https://github.com/dimaqq/juju-schema-analysis/blob/main/schemas-juju-3.5.4.txt#L3611-L3674 + # https://github.com/dimaqq/juju-schema-analysis/blob/main/schemas-juju-3.5.4.model-user.txt#L3611-L3674 response["response"]["machines"] = { "42": { "instance-status": { From 3c1ebe14c7a57b9f0ce42a062464819bc89c5f4c Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 19 Dec 2024 17:04:08 +0900 Subject: [PATCH 12/19] chore: refactor raise_on into separate helper functions --- juju/model/_idle.py | 95 ++++++++++++++++++++--------------- tests/unit/test_idle_check.py | 6 +-- 2 files changed, 57 insertions(+), 44 deletions(-) diff --git a/juju/model/_idle.py b/juju/model/_idle.py index f73539fa1..5bcbb3fdc 100644 --- a/juju/model/_idle.py +++ b/juju/model/_idle.py @@ -27,9 +27,9 @@ class CheckStatus: units: set[str] """All units visible at this point.""" ready_units: set[str] - """Units in good status (workload, agent, machine?).""" + """Units with the expected workload status.""" idle_units: set[str] - """Units with stable agent status (FIXME details).""" + """Units with stable (idle) agent status.""" class Loop: @@ -56,10 +56,6 @@ def next(self, status: CheckStatus | None) -> bool: expected_idle_since = now - self.idle_period - # 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.idle_units: self.idle_since[name] = min( @@ -114,40 +110,66 @@ def check( logger.info("Waiting for app %r", app_name) return None - # Order of errors: - # - # Machine error (any unit of any app from apps) - # Agent error (-"-) - # Workload error (-"-) - # App error (any app from apps) - # - # Workload blocked (any unit of any app from apps) - # App blocked (any app from apps) units: dict[str, UnitStatus] = {} + rv = CheckStatus(set(), set(), set()) + + for app_name in apps: + units.update(app_units(full_status, app_name)) + + if raise_on_error: + check_errors(full_status, apps, units) + + if raise_on_blocked: + check_blocked(full_status, apps, units) for app_name in apps: - units.update(_app_units(full_status, app_name)) + app = full_status.applications[app_name] + assert isinstance(app, ApplicationStatus) + + for unit_name, unit in app_units(full_status, app_name).items(): + rv.units.add(unit_name) + assert unit.agent_status + assert unit.workload_status + + if unit.agent_status.status == "idle": + rv.idle_units.add(unit_name) + + if not status or unit.workload_status.status == status: + rv.ready_units.add(unit_name) + + return rv + +def check_errors( + full_status: FullStatus, apps: AbstractSet[str], units: dict[str, UnitStatus] +) -> None: + """Check the full status for error conditions, in this order: + + - Machine error (any unit of any app from apps) + - Agent error (-"-) + - Workload error (-"-) + - App error (any app from apps) + """ for unit_name, unit in units.items(): if unit.machine: machine = full_status.machines[unit.machine] assert isinstance(machine, MachineStatus) assert machine.instance_status - if machine.instance_status.status == "error" and raise_on_error: + if machine.instance_status.status == "error": raise JujuMachineError( f"{unit_name!r} machine {unit.machine!r} has errored: {machine.instance_status.info!r}" ) for unit_name, unit in units.items(): assert unit.agent_status - if unit.agent_status.status == "error" and raise_on_error: + if unit.agent_status.status == "error": raise JujuAgentError( f"{unit_name!r} agent has errored: {unit.agent_status.info!r}" ) for unit_name, unit in units.items(): assert unit.workload_status - if unit.workload_status.status == "error" and raise_on_error: + if unit.workload_status.status == "error": raise JujuUnitError( f"{unit_name!r} workload has errored: {unit.workload_status.info!r}" ) @@ -156,12 +178,21 @@ def check( app = full_status.applications[app_name] assert isinstance(app, ApplicationStatus) assert app.status - if app.status.status == "error" and raise_on_error: + if app.status.status == "error": raise JujuAppError(f"{app_name!r} has errored: {app.status.info!r}") + +def check_blocked( + full_status: FullStatus, apps: AbstractSet[str], units: dict[str, UnitStatus] +) -> None: + """Check the full status for blocked conditions, in this order: + + - Workload blocked (any unit of any app from apps) + - App blocked (any app from apps) + """ for unit_name, unit in units.items(): assert unit.workload_status - if unit.workload_status.status == "blocked" and raise_on_blocked: + if unit.workload_status.status == "blocked": raise JujuUnitError( f"{unit_name!r} workload is blocked: {unit.workload_status.info!r}" ) @@ -170,29 +201,11 @@ def check( app = full_status.applications[app_name] assert isinstance(app, ApplicationStatus) assert app.status - if app.status.status == "blocked" and raise_on_blocked: + if app.status.status == "blocked": raise JujuAppError(f"{app_name!r} is blocked: {app.status.info!r}") - rv = CheckStatus(set(), set(), set()) - - for app_name in apps: - app = full_status.applications[app_name] - assert isinstance(app, ApplicationStatus) - for unit_name, unit in _app_units(full_status, app_name).items(): - rv.units.add(unit_name) - assert unit.agent_status - assert unit.workload_status - - if unit.agent_status.status == "idle": - rv.idle_units.add(unit_name) - - if not status or unit.workload_status.status == status: - rv.ready_units.add(unit_name) - - return rv - -def _app_units(full_status: FullStatus, app_name: str) -> dict[str, UnitStatus]: +def app_units(full_status: FullStatus, app_name: str) -> dict[str, UnitStatus]: """Fish out the app's units' status from a FullStatus response.""" rv: dict[str, UnitStatus] = {} app = full_status.applications[app_name] diff --git a/tests/unit/test_idle_check.py b/tests/unit/test_idle_check.py index 7efec3e3a..59617b894 100644 --- a/tests/unit/test_idle_check.py +++ b/tests/unit/test_idle_check.py @@ -20,19 +20,19 @@ def test_check_status(full_status: FullStatus, kwargs): status = check(full_status, **kwargs) assert status == CheckStatus( - { + units={ "grafana-agent-k8s/0", "hexanator/0", "mysql-test-app/0", "mysql-test-app/1", }, - { + ready_units={ "grafana-agent-k8s/0", "hexanator/0", "mysql-test-app/0", "mysql-test-app/1", }, - { + idle_units={ "grafana-agent-k8s/0", "hexanator/0", "mysql-test-app/0", From 57038a7a50016da4e268abc460adf97fa9175777 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 19 Dec 2024 17:11:48 +0900 Subject: [PATCH 13/19] chore: use kwargs in CheckStatus in tests --- tests/unit/test_idle_check_subordinate.py | 4 +-- tests/unit/test_idle_loop.py | 32 +++++++---------------- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/tests/unit/test_idle_check_subordinate.py b/tests/unit/test_idle_check_subordinate.py index 6d4fc2371..8e9454223 100644 --- a/tests/unit/test_idle_check_subordinate.py +++ b/tests/unit/test_idle_check_subordinate.py @@ -7,9 +7,7 @@ import pytest -from juju.client._definitions import ( - FullStatus, -) +from juju.client._definitions import FullStatus from juju.client.facade import _convert_response from juju.model._idle import CheckStatus, check diff --git a/tests/unit/test_idle_loop.py b/tests/unit/test_idle_loop.py index 300c50f35..ca4132f2b 100644 --- a/tests/unit/test_idle_loop.py +++ b/tests/unit/test_idle_loop.py @@ -41,11 +41,10 @@ def checks(): def test_at_least_units(): def checks(): - yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0"}, {"u/0", "u/1", "u/2"}) - yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0", "u/1"}, {"u/0", "u/1", "u/2"}) - yield CheckStatus( - {"u/0", "u/1", "u/2"}, {"u/0", "u/1", "u/2"}, {"u/0", "u/1", "u/2"} - ) + units = {"u/0", "u/1", "u/2"} + yield CheckStatus(units, ready_units={"u/0"}, idle_units=units) + yield CheckStatus(units, ready_units={"u/0", "u/1"}, idle_units=units) + yield CheckStatus(units, ready_units={"u/0", "u/1", "u/2"}, idle_units=units) with freeze_time(): assert unroll( @@ -57,21 +56,10 @@ def checks(): def test_for_exact_units(): - good = CheckStatus( - {"u/0", "u/1", "u/2"}, - {"u/1", "u/2"}, - {"u/0", "u/1", "u/2"}, - ) - too_few = CheckStatus( - {"u/0", "u/1", "u/2"}, - {"u/2"}, - {"u/0", "u/1", "u/2"}, - ) - too_many = CheckStatus( - {"u/0", "u/1", "u/2"}, - {"u/1", "u/2", "u/0"}, - {"u/0", "u/1", "u/2"}, - ) + units = {"u/0", "u/1", "u/2"} + good = CheckStatus(units, ready_units={"u/1", "u/2"}, idle_units=units) + too_few = CheckStatus(units, ready_units={"u/2"}, idle_units=units) + too_many = CheckStatus(units, ready_units={"u/1", "u/2", "u/0"}, idle_units=units) def checks(): yield too_few @@ -89,8 +77,8 @@ def checks(): def test_idle_ping_pong(): - good = CheckStatus({"hexanator/0"}, {"hexanator/0"}, {"hexanator/0"}) - bad = CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + good = CheckStatus({"hexanator/0"}, {"hexanator/0"}, idle_units={"hexanator/0"}) + bad = CheckStatus({"hexanator/0"}, {"hexanator/0"}, idle_units=set()) def checks(): with freeze_time() as clock: From 9f51f5be9b79dffde1052c49e7877862802dd786 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 19 Dec 2024 17:18:30 +0900 Subject: [PATCH 14/19] chore: use kwargs in CheckStatus in tests --- tests/unit/test_idle_check.py | 51 ++++++++++++----------------------- 1 file changed, 17 insertions(+), 34 deletions(-) diff --git a/tests/unit/test_idle_check.py b/tests/unit/test_idle_check.py index 59617b894..8f8541a44 100644 --- a/tests/unit/test_idle_check.py +++ b/tests/unit/test_idle_check.py @@ -5,13 +5,10 @@ import copy import json from typing import Any -from unittest.mock import ANY import pytest -from juju.client._definitions import ( - FullStatus, -) +from juju.client._definitions import FullStatus from juju.client.facade import _convert_response from juju.errors import JujuAgentError, JujuAppError, JujuMachineError, JujuUnitError from juju.model._idle import CheckStatus, check @@ -19,26 +16,13 @@ def test_check_status(full_status: FullStatus, kwargs): status = check(full_status, **kwargs) - assert status == CheckStatus( - units={ - "grafana-agent-k8s/0", - "hexanator/0", - "mysql-test-app/0", - "mysql-test-app/1", - }, - ready_units={ - "grafana-agent-k8s/0", - "hexanator/0", - "mysql-test-app/0", - "mysql-test-app/1", - }, - idle_units={ - "grafana-agent-k8s/0", - "hexanator/0", - "mysql-test-app/0", - "mysql-test-app/1", - }, - ) + units = { + "grafana-agent-k8s/0", + "hexanator/0", + "mysql-test-app/0", + "mysql-test-app/1", + } + assert status == CheckStatus(units, units, units) def test_check_status_missing_app(full_status: FullStatus, kwargs): @@ -89,18 +73,16 @@ def test_app_error(response: dict[str, Any], kwargs): 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"}, - {"mysql-test-app/0", "mysql-test-app/1"}, - ) + units = {"mysql-test-app/0", "mysql-test-app/1"} + assert status == CheckStatus(units, units, units) 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(), ANY) + units = {"mysql-test-app/0", "mysql-test-app/1"} + assert status == CheckStatus(units, ready_units=set(), idle_units=units) def test_ready_unit_requires_idle_agent(response: dict[str, Any], kwargs): @@ -115,7 +97,7 @@ def test_ready_unit_requires_idle_agent(response: dict[str, Any], kwargs): assert status == CheckStatus( {"hexanator/0", "hexanator/1"}, {"hexanator/0", "hexanator/1"}, - {"hexanator/0"}, + idle_units={"hexanator/0"}, ) @@ -128,7 +110,8 @@ 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"}, ANY) + units = {"hexanator/0", "hexanator/1"} + assert status == CheckStatus(units, ready_units={"hexanator/0"}, idle_units=units) def test_agent_error(response: dict[str, Any], kwargs): @@ -178,7 +161,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"}, ANY) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, {"hexanator/0"}) def test_machine_error(response: dict[str, Any], kwargs): @@ -240,7 +223,7 @@ def test_maintenance(response: dict[str, Any], kwargs): kwargs["status"] = "maintenance" status = check(convert(response), **kwargs) - assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, ANY) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, {"hexanator/0"}) @pytest.fixture From 995210acc72f924df07333b453b7ab6442c480f3 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 19 Dec 2024 17:40:44 +0900 Subject: [PATCH 15/19] chore: test ignoring errors --- tests/unit/test_idle_check.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/unit/test_idle_check.py b/tests/unit/test_idle_check.py index 8f8541a44..c920df530 100644 --- a/tests/unit/test_idle_check.py +++ b/tests/unit/test_idle_check.py @@ -213,6 +213,28 @@ def test_unit_blocked(response: dict[str, Any], kwargs): assert "small problem" in str(e) +def test_no_raise_on(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/0"]["workload-status"]["status"] = "blocked" + app["units"]["hexanator/0"]["workload-status"]["info"] = "small problem" + app["units"]["hexanator/0"]["machine"] = "42" + response["response"]["machines"] = { + "42": { + "instance-status": { + "status": "running", + "info": "RUNNING", + }, + }, + } + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_blocked"] = False + kwargs["raise_on_error"] = False + + status = check(convert(response), **kwargs) + assert status # didn't raise an exception + + def test_maintenance(response: dict[str, Any], kwargs): """Taken from nginx-ingress-integrator-operator integration tests.""" app = response["response"]["applications"]["hexanator"] From b328b3f4513837f712ef4a9becb8b9a3504ff069 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Thu, 19 Dec 2024 17:41:40 +0900 Subject: [PATCH 16/19] chore: call it rc6 --- juju/version.py | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/juju/version.py b/juju/version.py index 0a4471059..f50f96f54 100644 --- a/juju/version.py +++ b/juju/version.py @@ -6,4 +6,4 @@ DEFAULT_ARCHITECTURE = "amd64" -CLIENT_VERSION = "3.6.1.0rc5" +CLIENT_VERSION = "3.6.1.0rc6" diff --git a/pyproject.toml b/pyproject.toml index 1a651c4e3..66e1fbc88 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "juju" -version = "3.6.1.0rc5" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION +version = "3.6.1.0rc6" # 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" } From 044c57c1421013a1906128715e8514be9c70742a Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 20 Dec 2024 08:53:15 +0900 Subject: [PATCH 17/19] chore: release 3.6.1.0 --- juju/version.py | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/juju/version.py b/juju/version.py index f50f96f54..f74d8c299 100644 --- a/juju/version.py +++ b/juju/version.py @@ -6,4 +6,4 @@ DEFAULT_ARCHITECTURE = "amd64" -CLIENT_VERSION = "3.6.1.0rc6" +CLIENT_VERSION = "3.6.1.06" diff --git a/pyproject.toml b/pyproject.toml index 66e1fbc88..8da6954f4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "juju" -version = "3.6.1.0rc6" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION +version = "3.6.1.0" # 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" } From 37e25e2c3e32b5dbbc28b11e3474b57a8464711a Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 20 Dec 2024 08:58:21 +0900 Subject: [PATCH 18/19] chore: changelog and note supported versions to fix #1241 --- docs/changelog.rst | 12 ++++++++++++ docs/readme.rst | 3 +++ 2 files changed, 15 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index adaa842d5..8d5daec6f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,6 +1,18 @@ Changelog --------- +3.6.1.0 +^^^^^^^ + +Friday 20th Dec 2024 + +## What's Changed +* add 3.5.5 schema and update SCHEMAS.md by @james-garner-canonical in https://github.com/juju/python-libjuju/pull/1223 +* feat: larger default websockets frame size by @dimaqq in https://github.com/juju/python-libjuju/pull/1239 +* deprecate juju.jasyncio by @EdmilsonRodrigues in https://github.com/juju/python-libjuju/pull/1221 +* remove juju.loop, deprecated before 3.0 by @dimaqq in https://github.com/juju/python-libjuju/pull/1242 +* new wait for idle implementation, behind a feature flag ``JUJU_NEW_WAIT_FOR_IDLE`` in https://github.com/juju/python-libjuju/pull/1245 + 3.6.0.0 ^^^^^^^ diff --git a/docs/readme.rst b/docs/readme.rst index b237e2067..3c6567795 100644 --- a/docs/readme.rst +++ b/docs/readme.rst @@ -7,6 +7,9 @@ Bug reports: https://github.com/juju/python-libjuju/issues Documentation: https://pythonlibjuju.readthedocs.io/en/latest/ +Supported Python versions: 3.8 through 3.13 +Supported Juju versions: 3.1 through 3.6 + Design Notes ------------ From 716adb1a36587a441e75489bb9bff43fb811edf8 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 20 Dec 2024 08:59:59 +0900 Subject: [PATCH 19/19] chore: typo in the version --- juju/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/juju/version.py b/juju/version.py index f74d8c299..b4c849f95 100644 --- a/juju/version.py +++ b/juju/version.py @@ -6,4 +6,4 @@ DEFAULT_ARCHITECTURE = "amd64" -CLIENT_VERSION = "3.6.1.06" +CLIENT_VERSION = "3.6.1.0"