From e33995d9b32db5f175a5cf10943b30671ece8f50 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Fri, 13 Oct 2023 12:02:10 -0600 Subject: [PATCH 1/4] 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, 31 insertions(+), 11 deletions(-) diff --git a/juju/application.py b/juju/application.py index 6e05e091..73813d36 100644 --- a/juju/application.py +++ b/juju/application.py @@ -620,7 +620,7 @@ async def set_constraints(self, constraints): async def refresh( self, channel=None, force=False, force_series=False, force_units=False, - path=None, resources=None, revision=None, switch=None): + path=None, resources={}, revision=None, switch=None): """Refresh the charm for this application. :param str channel: Channel to use when getting the charm from the @@ -635,9 +635,6 @@ async def refresh( :param str switch: Crossgrade charm url """ - if path is None and resources is not None: - raise NotImplementedError("refreshing a non-local charm with resources option is not yet implemented") - if switch is not None and revision is not None: raise ValueError("switch and revision parameters are mutually exclusive in application refresh") @@ -727,6 +724,20 @@ async def refresh( # 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 + + # 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 + # Already prepped the charm_resources # Now get the existing resources from the ResourcesFacade request_data = [client.Entity(self.tag)] @@ -739,23 +750,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 d424bf82..2cfc643c 100644 --- a/juju/utils.py +++ b/juju/utils.py @@ -394,7 +394,7 @@ def base_channel_to_series(channel): return get_version_series(origin.Channel.parse(channel).track) -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. @@ -402,12 +402,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 9c681c91de457af1ed8ccbc8e930201e1d3bd34d Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Fri, 13 Oct 2023 12:03:56 -0600 Subject: [PATCH 2/4] Add integration test for resources argument in app refresh --- tests/integration/test_application.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/integration/test_application.py b/tests/integration/test_application.py index d9bf3d64..1637c805 100644 --- a/tests/integration/test_application.py +++ b/tests/integration/test_application.py @@ -230,6 +230,18 @@ async def test_upgrade_charm_resource(event_loop): 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): From b05623507a812029560dbfd748d0c23c85eefd0f Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Fri, 13 Oct 2023 13:22:29 -0600 Subject: [PATCH 3/4] Make argument optional to save unit tests --- juju/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/juju/utils.py b/juju/utils.py index 2cfc643c..a65d5597 100644 --- a/juju/utils.py +++ b/juju/utils.py @@ -394,7 +394,7 @@ def base_channel_to_series(channel): return get_version_series(origin.Channel.parse(channel).track) -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 6ec5c3e5aecb5fe599a9fb42ef257ea7a8bd69f9 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Fri, 13 Oct 2023 15:53:40 -0600 Subject: [PATCH 4/4] 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 73813d36..779bffe9 100644 --- a/juju/application.py +++ b/juju/application.py @@ -760,13 +760,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', ))