From 086068278ec980c3e4221464ca45671c6417b537 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Fri, 13 Oct 2023 12:02:10 -0600 Subject: [PATCH 1/6] Avoid trying to pass revision unless provided by user in app refresh Fixes #955 This also implements functionality that allows passing resources argument to application refresh. --- juju/application.py | 35 +++++++++++++++++++++++++++++------ juju/utils.py | 7 ++++++- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/juju/application.py b/juju/application.py index 83cf933b..95a4983e 100644 --- a/juju/application.py +++ b/juju/application.py @@ -736,6 +736,25 @@ async def refresh( err = charm_origin_result.error raise JujuError(f'{err.code} : {err.message}') +<<<<<<< HEAD +======= + # Now take care of the resources: + + # user supplied resources to be used in refresh, + # will override the default values if there's any + arg_resources = resources or {} + + # need to process the given resources, as they can be + # paths or revisions + _arg_res_filenames = {} + _arg_res_revisions = {} + for res, filename_or_rev in arg_resources.items(): + if isinstance(filename_or_rev, int): + _arg_res_revisions[res] = filename_or_rev + else: + _arg_res_filenames[res] = filename_or_rev + +>>>>>>> 5fbf5bc (Avoid trying to pass revision unless provided by user in app refresh) # Already prepped the charm_resources # Now get the existing resources from the ResourcesFacade request_data = [client.Entity(self.tag)] @@ -749,23 +768,27 @@ async def refresh( # Compute the difference btw resources needed and the existing resources resources_to_update = [] for resource in charm_resources: - if utils.should_upgrade_resource(resource, existing_resources): + if utils.should_upgrade_resource(resource, existing_resources, arg_resources): resources_to_update.append(resource) # Update the resources if resources_to_update: request_data = [] for resource in resources_to_update: + res_name = resource.get('Name', resource.get('name')) request_data.append(client.CharmResource( description=resource.get('Description', resource.get('description')), - fingerprint=resource.get('Fingerprint', resource.get('fingerprint')), - name=resource.get('Name', resource.get('name')), - path=resource.get('Path', resource.get('filename')), - revision=resource.get('Revision', resource.get('revision', -1)), - size=resource.get('Size', resource.get('size')), + fingerprint=resource.get('Fingerprint', resource.get('fingerprint', [])), + name=res_name, + path=_arg_res_filenames.get(res_name, + resource.get('Path', + resource.get('filename', ''))), + revision=_arg_res_revisions.get(res_name, -1), + size=resource.get('Size', resource.get('size', 0)), type_=resource.get('Type', resource.get('type')), origin='store', )) + response = await resources_facade.AddPendingResources( application_tag=self.tag, charm_url=charm_url, diff --git a/juju/utils.py b/juju/utils.py index d52e49e3..6744f31f 100644 --- a/juju/utils.py +++ b/juju/utils.py @@ -532,7 +532,7 @@ def series_selector(series_arg='', charm_url=None, model_config=None, supported_ return DEFAULT_SUPPORTED_LTS -def should_upgrade_resource(available_resource, existing_resources): +def should_upgrade_resource(available_resource, existing_resources, arg_resources): """Called in the context of upgrade_charm. Given a resource R, takes a look at the resources we already have and decides if we need to refresh R. @@ -540,12 +540,17 @@ def should_upgrade_resource(available_resource, existing_resources): charmhub api. We're considering if we need to refresh this during upgrade_charm. :param dict[str] existing_resources: The dict coming from resources_facade.ListResources representing the resources of the currently deployed charm. + :param dict[str] arg_resources: user provided resources to be refreshed :result bool: The decision to refresh the given resource """ + # should upgrade resource? res_name = available_resource.get('Name', available_resource.get('name')) + if res_name in arg_resources: + return True + # do we have it already? if res_name in existing_resources: # no upgrade, if it's upload From 871de22624a925050cd6d26eb23ac6724048e20d Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Fri, 13 Oct 2023 12:03:56 -0600 Subject: [PATCH 2/6] Add integration test for resources argument in app refresh --- tests/integration/test_application.py | 33 +++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/integration/test_application.py b/tests/integration/test_application.py index 424c5ff1..f7c795f4 100644 --- a/tests/integration/test_application.py +++ b/tests/integration/test_application.py @@ -209,6 +209,39 @@ async def test_upgrade_local_charm_resource(event_loop): @base.bootstrapped +@pytest.mark.asyncio +async def test_upgrade_charm_resource(event_loop): + async with base.CleanModel() as model: + app = await model.deploy('cs:~juju-qa/bionic/upgrade-charm-resource-test-0') + + await model.wait_for_idle(wait_for_units=1) + unit = app.units[0] + expected_message = 'I have no resource.' + assert unit.workload_status_message == expected_message + + await app.upgrade_charm(revision=1) + await model.block_until( + lambda: unit.workload_status_message != 'I have no resource.', + timeout=60, + ) + expected_message = 'My resource: I am the resource.' + assert app.units[0].workload_status_message == expected_message + + +@base.bootstrapped +@pytest.mark.asyncio +async def test_refresh_with_resource_argument(event_loop): + async with base.CleanModel() as model: + app = await model.deploy('juju-qa-test', resources={'foo-file': 2}) + res2 = await app.get_resources() + assert res2['foo-file'].revision == 2 + await app.refresh(resources={'foo-file': 4}) + res4 = await app.get_resources() + assert res4['foo-file'].revision == 4 + + +@base.bootstrapped +@pytest.mark.asyncio async def test_upgrade_charm_resource_same_rev_no_update(event_loop): async with base.CleanModel() as model: app = await model.deploy('keystone', channel='victoria/stable') From 06f4312eb3fc4b63a1f0c42a1b9e975ea7309ef2 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Fri, 13 Oct 2023 13:22:29 -0600 Subject: [PATCH 3/6] Make argument optional to save unit tests --- juju/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/juju/utils.py b/juju/utils.py index 6744f31f..30109fcc 100644 --- a/juju/utils.py +++ b/juju/utils.py @@ -421,6 +421,7 @@ def base_channel_to_series(channel): return get_version_series(origin.Channel.parse(channel).track) +<<<<<<< HEAD def parse_base_arg(base): """Parses a given base into a Client.Base object :param base str : The base to deploy a charm (e.g. ubuntu@22.04) @@ -532,7 +533,7 @@ def series_selector(series_arg='', charm_url=None, model_config=None, supported_ return DEFAULT_SUPPORTED_LTS -def should_upgrade_resource(available_resource, existing_resources, arg_resources): +def should_upgrade_resource(available_resource, existing_resources, arg_resources={}): """Called in the context of upgrade_charm. Given a resource R, takes a look at the resources we already have and decides if we need to refresh R. From f86e9561e32c5959a5bafe5e29a3cfe704073bf4 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Fri, 13 Oct 2023 15:53:40 -0600 Subject: [PATCH 4/6] Avoid providing fingerprint/size for resources with -1 revision --- juju/application.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/juju/application.py b/juju/application.py index 95a4983e..b9f11ec0 100644 --- a/juju/application.py +++ b/juju/application.py @@ -778,13 +778,11 @@ async def refresh( res_name = resource.get('Name', resource.get('name')) request_data.append(client.CharmResource( description=resource.get('Description', resource.get('description')), - fingerprint=resource.get('Fingerprint', resource.get('fingerprint', [])), name=res_name, path=_arg_res_filenames.get(res_name, resource.get('Path', resource.get('filename', ''))), revision=_arg_res_revisions.get(res_name, -1), - size=resource.get('Size', resource.get('size', 0)), type_=resource.get('Type', resource.get('type')), origin='store', )) From 300ba5da3c2bd9fa1fde9cff972f89f11f2d1d94 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Tue, 24 Oct 2023 16:36:18 -0600 Subject: [PATCH 5/6] Cleanup for application refresh with resources --- juju/application.py | 3 --- juju/utils.py | 1 - 2 files changed, 4 deletions(-) diff --git a/juju/application.py b/juju/application.py index b9f11ec0..3bda640d 100644 --- a/juju/application.py +++ b/juju/application.py @@ -686,9 +686,6 @@ async def refresh( force_units, path or switch, resources) return - if resources is not None: - raise NotImplementedError("resources option is not implemented") - # If switch is not None at this point, that means it's a switch to a store charm charm_url = switch or charm_url_origin_result.url parsed_url = URL.parse(charm_url) diff --git a/juju/utils.py b/juju/utils.py index 30109fcc..1b6854f7 100644 --- a/juju/utils.py +++ b/juju/utils.py @@ -421,7 +421,6 @@ def base_channel_to_series(channel): return get_version_series(origin.Channel.parse(channel).track) -<<<<<<< HEAD def parse_base_arg(base): """Parses a given base into a Client.Base object :param base str : The base to deploy a charm (e.g. ubuntu@22.04) From fb6099b7e7390cd17e2f90ad750787be11cd548b Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Thu, 26 Oct 2023 13:53:27 -0600 Subject: [PATCH 6/6] Cleanup conflict noise --- juju/application.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/juju/application.py b/juju/application.py index 3bda640d..4988e8a3 100644 --- a/juju/application.py +++ b/juju/application.py @@ -733,8 +733,6 @@ async def refresh( err = charm_origin_result.error raise JujuError(f'{err.code} : {err.message}') -<<<<<<< HEAD -======= # Now take care of the resources: # user supplied resources to be used in refresh, @@ -751,7 +749,6 @@ async def refresh( else: _arg_res_filenames[res] = filename_or_rev ->>>>>>> 5fbf5bc (Avoid trying to pass revision unless provided by user in app refresh) # Already prepped the charm_resources # Now get the existing resources from the ResourcesFacade request_data = [client.Entity(self.tag)]