Merge pull request juju#953 from cderici/handle-pending-upload-resour…

…ces-deployfromrepository

juju#953

#### Description

This is the continuation of juju#949, that implements handling of the local resources that need to be uploaded after a (server-side) deploy.

In particular, this splits out the second part of the `add_local_resources` into a separate `_upload` function to use after the `DeployFromRepository` call which reports the pending file uploads if there's any.

#### QA Steps

Following the juju#949, this needs the server side deploy support from the controller (i.e. `>= 3.3`). So, 

```sh
 $ juju version
3.3-beta2-ubuntu-amd64
 $ juju bootstrap localhost lxd33 && juju add-model test
```

Now let's make a local resource to use:

```sh
 $ cd python-libjuju
 $ echo "jujurulez" > ./foo.txt
 $ cat ./foo.txt
jujurulez
```

Then just manually deploy the `juju-qa-test` charm with the local resource `foo.txt`.

```python
python -m asyncio
asyncio REPL 3.10.12 (main, Jun 11 2023, 05:26:28) [GCC 11.4.0] on linux
Use "await" directly instead of "asyncio.run()".
Type "help", "copyright", "credits" or "license" for more information.
>>> import asyncio
>>> from juju import model;m=model.Model();await m.connect();await m.deploy('juju-qa-test', application_name='j1', resources={'foo-file':'./foo.txt'}))
<Application entity_id="j1">
>>>
exiting asyncio REPL...
```

Now confirm that the resource is uploaded:

```sh
 $ juju resources j1
Resource Supplied by Revision
foo-file admin 2023-09-19T22:55
```

All CI tests need to pass.

#### Notes & Discussion

JUJU-3638

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,
@@ -2099,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,
@@ -2125,6 +2147,7 @@ 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))
+
         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):
@@ -2729,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":

tests/integration/test_application.py

@@ -14,6 +14,8 @@
 
 logger = logging.getLogger(__name__)
 
+from ..utils import INTEGRATION_TEST_DIR
+
 
 @base.bootstrapped
 async def test_action(event_loop):
@@ -191,17 +193,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']

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: