Merge pull request #973 from cderici/app-refresh-with-resources-on-3.x

#973

#### Description

This is a forward port of #960, bringing support for refreshing application with providing `resources` as arguments.

Fixes #955 and #883 for 3.x track.

#### QA Steps

QA steps should be the same with #960:

So the fix for the #955 can be tested manually by recreating the scenario above with pylibjuju as follows:

```python
 $ python -m asyncio
>>> import asyncio
>>> from juju import model;m=model.Model();await m.connect()
>>> await m.deploy('ceph-mon', channel='octopus/stable')
```

Check the resources for that using juju-cli:

```sh
 $ juju resources ceph-mon
No resources to display.
```

Go back to the python repl:

```python
>>> await m.applications['ceph-mon'].refresh(channel='quincy/stable')
This should succeed
```

And check the resources again external to pylibjuju, you should see the resource is added:

```sh
 $ juju resources ceph-juju
Resource Supplied by Revision
alert-rules charmstore 1
```

Additionally, an integration test for the second part of the PR that adds the resource argument to refresh is added, so just run that, and maybe play with it manually for different charms:

```
tox -e integration -- tests/integration/test_application.py::test_refresh_with_resource_argument
```

All CI tests need to pass.

#### Notes & Discussion

JUJU-4736

Author: jujubot

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)
@@ -736,6 +733,22 @@ async def refresh(
             err = charm_origin_result.error
             raise JujuError(f'{err.code} : {err.message}')
 
+        # 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
+
         # Already prepped the charm_resources
         # Now get the existing resources from the ResourcesFacade
         request_data = [client.Entity(self.tag)]
@@ -749,23 +762,25 @@ 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')),
+                    name=res_name,
+                    path=_arg_res_filenames.get(res_name,
+                                                resource.get('Path',
+                                                             resource.get('filename', ''))),
+                    revision=_arg_res_revisions.get(res_name, -1),
                     type_=resource.get('Type', resource.get('type')),
                     origin='store',
                 ))
+
             response = await resources_facade.AddPendingResources(
                 application_tag=self.tag,
                 charm_url=charm_url,

juju/utils.py

@@ -532,20 +532,25 @@ 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.
 
     :param dict[str] available_resource: The dict representing the client.Resource coming from the
     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

tests/integration/test_application.py

@@ -210,6 +210,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')