From 06c0e7aff1c2f2c453140565a86b47eb503389ae Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Sat, 27 Jan 2024 22:58:06 +0800 Subject: [PATCH 1/5] feat: remove app block until done --- juju/application.py | 32 ++++++++++++++++++--------- tests/integration/test_application.py | 21 ++++++++++++++---- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/juju/application.py b/juju/application.py index 5f824e3a..2134fa9e 100644 --- a/juju/application.py +++ b/juju/application.py @@ -4,17 +4,20 @@ import hashlib import json import logging +import typing from pathlib import Path -from . import model, tag, utils, jasyncio -from .url import URL -from .status import derive_status +from . import jasyncio, model, tag, utils from .annotationhelper import _get_annotations, _set_annotations -from .client import client -from .errors import JujuError, JujuApplicationConfigError from .bundle import get_charm_series, is_local_charm -from .placement import parse as parse_placement +from .client import client +from .errors import JujuApplicationConfigError, JujuError from .origin import Channel, Source +from .placement import parse as parse_placement +from .relation import Relation +from .status import derive_status +from .url import URL +from .utils import block_until log = logging.getLogger(__name__) @@ -59,14 +62,14 @@ def subordinate_units(self): return [u for u in self.units if u.is_subordinate] @property - def relations(self): + def relations(self) -> typing.List[Relation]: return [rel for rel in self.model.relations if rel.matches(self.name)] def related_applications(self, endpoint_name=None): apps = {} for rel in self.relations: if rel.is_peer: - local_ep, remote_ep = rel.endpoints[0] + local_ep, remote_ep = rel.endpoints else: def is_us(ep): return ep.application.name == self.name @@ -191,12 +194,13 @@ async def scale(self, scale=None, scale_change=None): scale_change=scale_change) ]) - async def destroy_relation(self, local_relation, remote_relation): + async def destroy_relation(self, local_relation, remote_relation, block_until_done: bool = False): """Remove a relation to another application. :param str local_relation: Name of relation on this application :param str remote_relation: Name of relation on the other application in the form '[:]' + :param bool block_until_done: Wait until the relation is completely removed. """ if ':' not in local_relation: @@ -207,8 +211,16 @@ async def destroy_relation(self, local_relation, remote_relation): log.debug( 'Destroying relation %s <-> %s', local_relation, remote_relation) - return await app_facade.DestroyRelation(endpoints=[ + await app_facade.DestroyRelation(endpoints=[ local_relation, remote_relation]) + if block_until_done: + await block_until( + not any( + relation.matches(local_relation, remote_relation) + for relation in self.relations + ) + ) + remove_relation = destroy_relation async def destroy_unit(self, *unit_names): diff --git a/tests/integration/test_application.py b/tests/integration/test_application.py index 05252d46..86a21edb 100644 --- a/tests/integration/test_application.py +++ b/tests/integration/test_application.py @@ -1,15 +1,17 @@ # Copyright 2023 Canonical Ltd. # Licensed under the Apache V2, see LICENCE file for details. +import logging from pathlib import Path import pytest -import logging -from .. import base -from juju import jasyncio, errors -from juju.url import URL, Schema +from juju import errors, jasyncio +from juju.application import Application from juju.client import client +from juju.url import URL, Schema + +from .. import base MB = 1 @@ -331,3 +333,14 @@ async def test_app_charm_name(): await model.wait_for_idle(status="active") assert 'ubuntu' in app.charm_url assert 'ubuntu' == app.charm_name + + +@base.bootstrapped +async def test_app_relation_destroy_block_until_done(): + async with base.CleanModel() as model: + app: Application = await model.deploy('docker-registry') + rsa: Application = await model.deploy("easyrsa") + await app.relate("cert-provider", rsa.name) + await model.wait_for_idle(status="active") + await app.destroy_relation("cert-provider", rsa.name, block_until_done=True) + await app.relate("cert-provider", rsa.name) From 6a2a7ab0fe91c8ce8b7f4cf56733e1c99cb14920 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Sat, 27 Jan 2024 23:09:36 +0800 Subject: [PATCH 2/5] feat: remove app timeout --- juju/model.py | 14 ++++++++++++-- tests/integration/test_application.py | 11 +++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/juju/model.py b/juju/model.py index 56af31e7..59637b74 100644 --- a/juju/model.py +++ b/juju/model.py @@ -1000,7 +1000,15 @@ async def remove_storage(self, *storage_ids, force=False, destroy_storage=False) if ret.results[0].error: raise JujuError(ret.results[0].error.message) - async def remove_application(self, app_name, block_until_done=False, force=False, destroy_storage=False, no_wait=False): + async def remove_application( + self, + app_name, + block_until_done=False, + force=False, + destroy_storage=False, + no_wait=False, + timeout=None + ): """Removes the given application from the model. :param str app_name: Name of the application @@ -1009,6 +1017,8 @@ async def remove_application(self, app_name, block_until_done=False, force=False :param bool no_wait: Rush through application removal without waiting for each individual step to complete (=false) :param bool block_until_done: Ensure the app is removed from the model when returned + :param int timeout: Raise asyncio.exceptions.TimeoutError if the application is not removed + within the timeout period. """ if app_name not in self.applications: raise JujuError("Given application doesn't seem to appear in the\ @@ -1019,7 +1029,7 @@ async def remove_application(self, app_name, block_until_done=False, force=False no_wait=no_wait, ) if block_until_done: - await self.block_until(lambda: app_name not in self.applications) + await self.block_until(lambda: app_name not in self.applications, timeout=timeout) async def block_until(self, *conditions, timeout=None, wait_period=0.5): """Return only after all conditions are true. diff --git a/tests/integration/test_application.py b/tests/integration/test_application.py index 86a21edb..b00f9dd2 100644 --- a/tests/integration/test_application.py +++ b/tests/integration/test_application.py @@ -3,6 +3,7 @@ import logging from pathlib import Path +import asyncio import pytest @@ -326,6 +327,16 @@ async def test_app_remove_wait_flag(): assert a_name not in model.applications +@base.bootstrapped +async def test_app_remove_timeout(): + async with base.CleanModel() as model: + app = await model.deploy('ubuntu') + await model.wait_for_idle(status="active") + + with pytest.raises(asyncio.TimeoutError): + await model.remove_application(app.name, block_until_done=True, timeout=1) + + @base.bootstrapped async def test_app_charm_name(): async with base.CleanModel() as model: From 8df075e558f2d5d60f8e7d779ba4f7792499e273 Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Sat, 27 Jan 2024 23:09:54 +0800 Subject: [PATCH 3/5] Revert "feat: remove app timeout" This reverts commit 6a2a7ab0fe91c8ce8b7f4cf56733e1c99cb14920. --- juju/model.py | 14 ++------------ tests/integration/test_application.py | 11 ----------- 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/juju/model.py b/juju/model.py index 59637b74..56af31e7 100644 --- a/juju/model.py +++ b/juju/model.py @@ -1000,15 +1000,7 @@ async def remove_storage(self, *storage_ids, force=False, destroy_storage=False) if ret.results[0].error: raise JujuError(ret.results[0].error.message) - async def remove_application( - self, - app_name, - block_until_done=False, - force=False, - destroy_storage=False, - no_wait=False, - timeout=None - ): + async def remove_application(self, app_name, block_until_done=False, force=False, destroy_storage=False, no_wait=False): """Removes the given application from the model. :param str app_name: Name of the application @@ -1017,8 +1009,6 @@ async def remove_application( :param bool no_wait: Rush through application removal without waiting for each individual step to complete (=false) :param bool block_until_done: Ensure the app is removed from the model when returned - :param int timeout: Raise asyncio.exceptions.TimeoutError if the application is not removed - within the timeout period. """ if app_name not in self.applications: raise JujuError("Given application doesn't seem to appear in the\ @@ -1029,7 +1019,7 @@ async def remove_application( no_wait=no_wait, ) if block_until_done: - await self.block_until(lambda: app_name not in self.applications, timeout=timeout) + await self.block_until(lambda: app_name not in self.applications) async def block_until(self, *conditions, timeout=None, wait_period=0.5): """Return only after all conditions are true. diff --git a/tests/integration/test_application.py b/tests/integration/test_application.py index b00f9dd2..86a21edb 100644 --- a/tests/integration/test_application.py +++ b/tests/integration/test_application.py @@ -3,7 +3,6 @@ import logging from pathlib import Path -import asyncio import pytest @@ -327,16 +326,6 @@ async def test_app_remove_wait_flag(): assert a_name not in model.applications -@base.bootstrapped -async def test_app_remove_timeout(): - async with base.CleanModel() as model: - app = await model.deploy('ubuntu') - await model.wait_for_idle(status="active") - - with pytest.raises(asyncio.TimeoutError): - await model.remove_application(app.name, block_until_done=True, timeout=1) - - @base.bootstrapped async def test_app_charm_name(): async with base.CleanModel() as model: From 2f4813c1b3e2f7794994b5feb91e8f8127f1f10f Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Sun, 28 Jan 2024 12:54:22 +0800 Subject: [PATCH 4/5] fix: callable --- juju/application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/juju/application.py b/juju/application.py index 2134fa9e..e1e08ef2 100644 --- a/juju/application.py +++ b/juju/application.py @@ -215,7 +215,7 @@ async def destroy_relation(self, local_relation, remote_relation, block_until_do local_relation, remote_relation]) if block_until_done: await block_until( - not any( + lambda: not any( relation.matches(local_relation, remote_relation) for relation in self.relations ) From 27cae57d53a79f317abcdfd53fee233eced3187a Mon Sep 17 00:00:00 2001 From: charlie4284 Date: Wed, 31 Jan 2024 18:21:01 +0800 Subject: [PATCH 5/5] chore: use relations assertion --- tests/integration/test_application.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_application.py b/tests/integration/test_application.py index 86a21edb..b396f902 100644 --- a/tests/integration/test_application.py +++ b/tests/integration/test_application.py @@ -340,7 +340,7 @@ async def test_app_relation_destroy_block_until_done(): async with base.CleanModel() as model: app: Application = await model.deploy('docker-registry') rsa: Application = await model.deploy("easyrsa") - await app.relate("cert-provider", rsa.name) + relation = await app.relate('cert-provider', rsa.name) await model.wait_for_idle(status="active") - await app.destroy_relation("cert-provider", rsa.name, block_until_done=True) - await app.relate("cert-provider", rsa.name) + await app.destroy_relation('cert-provider', rsa.name, block_until_done=True) + assert relation not in app.relations