From 67128e998e025b64016b41abf2df1e6789119177 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Tue, 19 Sep 2023 16:59:20 -0600 Subject: [PATCH 1/5] Upload pending local resources after server side deploy Splits out the second part of the add_local_resources into a separate _upload function to use after DeployFromRepository reports the pending file uploads if there's any --- juju/model.py | 76 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 49 insertions(+), 27 deletions(-) diff --git a/juju/model.py b/juju/model.py index 23b72924..928e87de 100644 --- a/juju/model.py +++ b/juju/model.py @@ -1743,6 +1743,8 @@ async def deploy( if base: charm_origin.base = utils.parse_base_arg(base) + server_side_deploy = False + if res.is_bundle: handler = BundleHandler(self, trusted=trust, forced=force) await handler.fetch_plan(url, charm_origin, overlays=overlays) @@ -1774,9 +1776,20 @@ async def deploy( else: charm_origin = add_charm_res.charm_origin if Schema.CHARM_HUB.matches(url.schema): - resources = await self._add_charmhub_resources(res.app_name, - identifier, - add_charm_res.charm_origin) + + if client.ApplicationFacade.best_facade_version(self.connection()) >= 19: + server_side_deploy = True + else: + # TODO (cderici): this is an awkward workaround for basically not calling + # the AddPendingResources in case this is a server side deploy. + # If that's the case, then the store resources (and revisioned local + # resources) are handled at the server side if this is a server side deploy + # (local uploads are handled right after we get the pendingIDs returned + # from the facade call). + resources = await self._add_charmhub_resources(res.app_name, + identifier, + add_charm_res.charm_origin) + is_sub = await self.charmhub.is_subordinate(url.name) if is_sub: if num_units > 1: @@ -1829,6 +1842,7 @@ async def deploy( charm_origin=charm_origin, attach_storage=attach_storage, force=force, + server_side_deploy=server_side_deploy, ) async def _add_charm(self, charm_url, origin): @@ -2029,42 +2043,42 @@ async def add_local_resources(self, application, entity_url, metadata, resources 'username': '', 'password': '', } - data = yaml.dump(docker_image_details) + else: + p = Path(path) + data = p.read_text() if p.exists() else '' - hash_alg = hashlib.sha3_384 - - charmresource['fingerprint'] = hash_alg(bytes(data, 'utf-8')).digest() + self._upload(data, path, application, name, resource_type, pending_id) - conn, headers, path_prefix = self.connection().https_connection() + return resource_map - query = "?pendingid={}".format(pending_id) - url = "{}/applications/{}/resources/{}{}".format( - path_prefix, application, name, query) - if resource_type == "oci-image": - disp = "multipart/form-data; filename=\"{}\"".format(path) - else: - disp = "form-data; filename=\"{}\"".format(path) + def _upload(self, data, path, app_name, res_name, res_type, pending_id): + conn, headers, path_prefix = self.connection().https_connection() - headers['Content-Type'] = 'application/octet-stream' - headers['Content-Length'] = len(data) - headers['Content-Sha384'] = charmresource['fingerprint'].hex() - headers['Content-Disposition'] = disp + query = "?pendingid={}".format(pending_id) + url = "{}/applications/{}/resources/{}{}".format(path_prefix, app_name, res_name, query) + if res_type == "oci-image": + disp = "multipart/form-data; filename=\"{}\"".format(path) + else: + disp = "form-data; filename=\"{}\"".format(path) - conn.request('PUT', url, data, headers) + headers['Content-Type'] = 'application/octet-stream' + headers['Content-Length'] = len(data) + headers['Content-Sha384'] = hashlib.sha384(bytes(data, 'utf-8')).hexdigest() + headers['Content-Disposition'] = disp - response = conn.getresponse() - result = response.read().decode() - if not response.status == 200: - raise JujuError(result) + conn.request('PUT', url, data, headers) - return resource_map + response = conn.getresponse() + result = response.read().decode() + if not response.status == 200: + raise JujuError(result) async def _deploy(self, charm_url, application, series, config, constraints, endpoint_bindings, resources, storage, channel=None, num_units=None, placement=None, devices=None, charm_origin=None, attach_storage=[], - force=False): + force=False, server_side_deploy=False): """Logic shared between `Model.deploy` and `BundleHandler.deploy`. """ log.info('Deploying %s', charm_url) @@ -2077,7 +2091,7 @@ async def _deploy(self, charm_url, application, series, config, app_facade = client.ApplicationFacade.from_connection(self.connection()) - if client.ApplicationFacade.best_facade_version(self.connection()) >= 19: + if server_side_deploy: # Call DeployFromRepository app = client.DeployFromRepositoryArg( applicationname=application, @@ -2125,6 +2139,14 @@ async def _deploy(self, charm_url, application, series, config, errors = [r.error.message for r in result.results if r.error] if errors: raise JujuError('\n'.join(errors)) + + for _result in result.results: + for pending_upload_resource in _result.pendingresourceuploads: + _path = pending_upload_resource.filename + p = Path(_path) + data = p.read_text() if p.exists() else '' + self._upload(data, _path, application, pending_upload_resource.name, 'file', '') + return await self._wait_for_new('application', application) async def destroy_unit(self, unit_id, destroy_storage=False, dry_run=False, force=False, max_wait=None): From cec424fb1101ade828979627fa2e632dd093c9ca Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Wed, 20 Sep 2023 08:32:28 -0600 Subject: [PATCH 2/5] Upload pending local resources after deploy only for server_side_deploy --- juju/model.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/juju/model.py b/juju/model.py index 928e87de..519fa07a 100644 --- a/juju/model.py +++ b/juju/model.py @@ -2113,10 +2113,18 @@ async def _deploy(self, charm_url, application, series, config, revision=charm_origin.revision, ) result = await app_facade.DeployFromRepository([app]) + # Collect the errors errors = [] for r in result.results: if r.errors: errors.extend([e.message for e in r.errors]) + # Upload pending local resources if any + for _result in result.results: + for pending_upload_resource in getattr(_result, 'pendingresourceuploads', []): + _path = pending_upload_resource.filename + p = Path(_path) + data = p.read_text() if p.exists() else '' + self._upload(data, _path, application, pending_upload_resource.name, 'file', '') else: app = client.ApplicationDeploy( charm_url=charm_url, @@ -2140,13 +2148,6 @@ async def _deploy(self, charm_url, application, series, config, if errors: raise JujuError('\n'.join(errors)) - for _result in result.results: - for pending_upload_resource in _result.pendingresourceuploads: - _path = pending_upload_resource.filename - p = Path(_path) - data = p.read_text() if p.exists() else '' - self._upload(data, _path, application, pending_upload_resource.name, 'file', '') - return await self._wait_for_new('application', application) async def destroy_unit(self, unit_id, destroy_storage=False, dry_run=False, force=False, max_wait=None): From 3d6866bb9136b446e131605dde04548b7c7d4ce3 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Wed, 20 Sep 2023 14:31:12 -0600 Subject: [PATCH 3/5] Fix file resource charm raise on error --- juju/model.py | 4 ++-- tests/integration/test_application.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/juju/model.py b/juju/model.py index 519fa07a..31facbd1 100644 --- a/juju/model.py +++ b/juju/model.py @@ -2752,10 +2752,10 @@ def _raise_for_status(entities, status): # errors to raise at the end break for unit in app.units: - if unit.machine is not None and unit.machine.status == "error": + if raise_on_error and unit.machine is not None and unit.machine.status == "error": errors.setdefault("Machine", []).append(unit.machine.id) continue - if unit.agent_status == "error": + if raise_on_error and unit.agent_status == "error": errors.setdefault("Agent", []).append(unit.name) continue if raise_on_error and unit.workload_status == "error": diff --git a/tests/integration/test_application.py b/tests/integration/test_application.py index 9e799bb3..2bc64ecb 100644 --- a/tests/integration/test_application.py +++ b/tests/integration/test_application.py @@ -14,6 +14,7 @@ logger = logging.getLogger(__name__) +from ..utils import INTEGRATION_TEST_DIR @base.bootstrapped async def test_action(event_loop): @@ -191,17 +192,16 @@ async def test_upgrade_local_charm(event_loop): @base.bootstrapped async def test_upgrade_local_charm_resource(event_loop): async with base.CleanModel() as model: - tests_dir = Path(__file__).absolute().parent - charm_path = tests_dir / 'file-resource-charm' + charm_path = INTEGRATION_TEST_DIR / 'file-resource-charm' resources = {"file-res": "test.file"} app = await model.deploy(str(charm_path), resources=resources) assert 'file-resource-charm' in model.applications - await model.wait_for_idle() + await model.wait_for_idle(raise_on_error=False) assert app.units[0].agent_status == 'idle' await app.upgrade_charm(path=charm_path, resources=resources) - await model.wait_for_idle() + await model.wait_for_idle(raise_on_error=False) ress = await app.get_resources() assert 'file-res' in ress assert ress['file-res'] From 1745baca019edb3762d75565fa7a37ef3fea1e8b Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Wed, 20 Sep 2023 14:41:31 -0600 Subject: [PATCH 4/5] Fix linter --- tests/integration/test_application.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/test_application.py b/tests/integration/test_application.py index 2bc64ecb..caee0cff 100644 --- a/tests/integration/test_application.py +++ b/tests/integration/test_application.py @@ -16,6 +16,7 @@ from ..utils import INTEGRATION_TEST_DIR + @base.bootstrapped async def test_action(event_loop): async with base.CleanModel() as model: From 8eaad01475a1ed3b932b1b76acab5145494141db Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Mon, 25 Sep 2023 11:05:26 -0600 Subject: [PATCH 5/5] Avoid raising for intermediate fails on tests --- tests/integration/test_model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_model.py b/tests/integration/test_model.py index e52a1b1f..830046e5 100644 --- a/tests/integration/test_model.py +++ b/tests/integration/test_model.py @@ -702,7 +702,7 @@ async def test_local_file_resource_charm(event_loop): app = await model.deploy(str(charm_path), resources=resources) assert 'file-resource-charm' in model.applications - await model.wait_for_idle() + await model.wait_for_idle(raise_on_error=False) assert app.units[0].agent_status == 'idle' ress = await app.get_resources() @@ -718,7 +718,7 @@ async def test_attach_resource(event_loop): app = await model.deploy(str(charm_path), resources=resources) assert 'file-resource-charm' in model.applications - await model.wait_for_idle() + await model.wait_for_idle(raise_on_error=False) assert app.units[0].agent_status == 'idle' with open(str(charm_path / 'test.file')) as f: