Merge pull request #957 from cderici/deploy-by-revision-2.9

#957

#### Description

This backports the support for deploy by revision from master branch (3.x track) (#830) onto the 2.9 track. 

#### QA Steps

So we have 1 example and 2 integration tests for this, all of them should work well (I tested it against 2.9.45 on lxd).

```sh
 $ python examples/deploywithrevision.py
```

```
tox -e integration -- tests/integration/test_model.py::test_deploy_by_revision
```

```
tox -e integration -- tests/integration/test_model.py::test_deploy_by_revision_validate_flags
```

All the rest of the CI tests need to pass.

#### Notes & Discussion

JUJU-4716

examples/annotate.py

@@ -0,0 +1,73 @@
+#!/usr/bin/python3
+
+# Copyright 2023 Canonical Ltd.
+# Licensed under the Apache V2, see LICENCE file for details.
+
+import asyncio
+import argparse
+import logging
+import sys
+
+# from juju import jasyncio
+import juju.model
+
+
+async def get_annotations(model, args):
+    logging.info('getting annotations')
+    annotations = await model.get_annotations()
+    if args.key is None:
+        for key, value in annotations.items():
+            print(f'{key}: {value!r}')
+    else:
+        value = annotations[args.key]
+        print(value)
+
+
+async def set_annotation(model, args):
+    logging.info(f'setting annotation for {args.key!r}')
+    await model.set_annotations({args.key: args.value})
+
+
+async def get_model():
+    logging.info('getting model')
+    model = juju.model.Model()
+    await model.connect()
+    return model
+
+
+async def run_func(args):
+    model = await get_model()
+    try:
+        await args.func(model, args)
+    finally:
+        logging.info('disconnecting the model')
+        await model.disconnect()
+
+
+def parse_args(args):
+    p = argparse.ArgumentParser()
+    p.add_argument('--debug', action="store_const", dest="loglevel", const=logging.DEBUG, default=logging.WARNING)
+    p.add_argument('--verbose', '-v', action="store_const", dest="loglevel", const=logging.INFO)
+    p.add_argument('--quiet', action="store_const", dest="loglevel", const=logging.CRITICAL)
+    sub = p.add_subparsers()
+    getp = sub.add_parser('get')
+    getp.set_defaults(func=get_annotations)
+    getp.add_argument("key", help='the key to set', nargs='?')
+    setp = sub.add_parser('set')
+    setp.set_defaults(func=set_annotation)
+    setp.add_argument("key", help='the key to set')
+    setp.add_argument("value", help='set the value of key to this value, empty string removes the annotation')
+    return p.parse_args(args)
+
+
+def main():
+    args = parse_args(sys.argv[1:])
+    rootLogger = logging.getLogger()
+    rootLogger.setLevel(args.loglevel)
+    asyncio.run(run_func(args))
+
+
+if __name__ == '__main__':
+    main()
+
+# vim: expandtab ts=4 sts=4 shiftwidth=4 autoindent:

examples/deploy_with_revision.py

@@ -0,0 +1,34 @@
+from juju import jasyncio
+from juju.model import Model
+
+
+async def main():
+    charm = 'juju-qa-test'
+
+    model = Model()
+    print('Connecting to model')
+    # connect to current model with current user, per Juju CLI
+    await model.connect()
+
+    try:
+        print(f'Deploying {charm} --channel latest/edge --revision 19')
+        application = await model.deploy(
+            'juju-qa-test',
+            application_name='test',
+            channel='latest/edge',
+            series='xenial',
+            revision=19,
+        )
+
+        print('Waiting for active')
+        await model.wait_for_idle(status='active')
+
+        print(f'Removing {charm}')
+        await application.remove()
+    finally:
+        print('Disconnecting from model')
+        await model.disconnect()
+
+
+if __name__ == '__main__':
+    jasyncio.run(main())

juju/model.py

@@ -452,7 +452,8 @@ class LocalDeployType:
     """LocalDeployType deals with local only deployments.
     """
 
-    async def resolve(self, url, architecture, app_name=None, channel=None, series=None, entity_url=None, force=False):
+    async def resolve(self, url, architecture, app_name=None, channel=None, series=None, revision=None,
+                      entity_url=None, force=False):
         """resolve attempts to resolve a local charm or bundle using the url
         and architecture. If information is missing, it will attempt to backfill
         that information, before sending the result back.
@@ -509,7 +510,7 @@ def _default_app_name(meta):
         suggested_name = meta.get('charm-metadata', {}).get('Name')
         return suggested_name or meta.get('id', {}).get('Name')
 
-    async def resolve(self, url, architecture, app_name=None, channel=None, series=None, entity_url=None, force=False):
+    async def resolve(self, url, architecture, app_name=None, channel=None, series=None, revision=None, entity_url=None, force=False):
         """resolve attempts to resolve charmstore charms or bundles. A request
         to the charmstore is required to get more information about the
         underlying identifier.
@@ -548,11 +549,13 @@ class CharmhubDeployType:
     def __init__(self, charm_resolver):
         self.charm_resolver = charm_resolver
 
-    async def resolve(self, url, architecture, app_name=None, channel=None, series=None, entity_url=None, force=False):
+    async def resolve(self, url, architecture, app_name=None, channel=None, series=None, revision=None, entity_url=None, force=False):
         """resolve attempts to resolve charmhub charms or bundles. A request to
         the charmhub API is required to correctly determine the charm url and
         underlying origin.
         """
+        if revision and not channel:
+            raise JujuError('specifying a revision requires a channel for future upgrades. Please use --channel')
 
         ch = Channel('latest', 'stable')
         if channel is not None:
@@ -562,9 +565,17 @@ async def resolve(self, url, architecture, app_name=None, channel=None, series=N
                                     architecture=architecture,
                                     risk=ch.risk,
                                     track=ch.track,
-                                    series=series)
+                                    series=series,
+                                    revision=revision,
+                                    )
+
+        charm_url_str, origin = await self.charm_resolver(url, origin, force)
 
-        charm_url, origin = await self.charm_resolver(url, origin, force)
+        is_bundle = origin.type_ == "bundle"
+        if is_bundle and revision and channel:
+            raise JujuError('revision and channel are mutually exclusive when deploying a bundle. Please choose one.')
+
+        charm_url = URL.parse(charm_url_str)
 
         if app_name is None:
             app_name = url.name
@@ -573,7 +584,7 @@ async def resolve(self, url, architecture, app_name=None, channel=None, series=N
             identifier=str(charm_url),
             app_name=app_name,
             origin=origin,
-            is_bundle=origin.type_ == "bundle",
+            is_bundle=is_bundle,
         )
 
 
@@ -1588,7 +1599,7 @@ def _get_series(self, entity_url, entity):
     async def deploy(
             self, entity_url, application_name=None, bind=None,
             channel=None, config=None, constraints=None, force=False,
-            num_units=1, overlays=[], plan=None, resources=None, series=None,
+            num_units=1, overlays=[], base=None, resources=None, series=None, revision=None,
             storage=None, to=None, devices=None, trust=False):
         """Deploy a new service or bundle.
 
@@ -1607,6 +1618,8 @@ async def deploy(
         :param str plan: Plan under which to deploy charm
         :param dict resources: <resource name>:<file path> pairs
         :param str series: Series on which to deploy
+        :param int revision: specifying a revision requires a channel for future upgrades for charms.
+            For bundles, revision and channel are mutually exclusive.
         :param dict storage: Storage constraints TODO how do these look?
         :param to: Placement directive as a string. For example:
 
@@ -1647,7 +1660,9 @@ async def deploy(
         if str(url.schema) not in self.deploy_types:
             raise JujuError("unknown deploy type {}, expected charmhub, charmstore or local".format(url.schema))
 
-        res = await self.deploy_types[str(url.schema)].resolve(url, architecture, application_name, channel, series, entity_url, force)
+        res = await self.deploy_types[str(url.schema)].resolve(url, architecture,
+                                                               application_name, channel, series,
+                                                               revision, entity_url, force)
 
         if res.identifier is None:
             raise JujuError('unknown charm or bundle {}'.format(entity_url))
@@ -1782,6 +1797,7 @@ async def _resolve_charm(self, url, origin, force=False):
                 'architecture': origin.architecture,
                 'track': origin.track,
                 'risk': origin.risk,
+                'revision': origin.revision,
             }
         }])
         if len(resp.results) != 1:

tests/integration/test_model.py

@@ -14,7 +14,7 @@
 
 import pylxd
 import pytest
-from juju import jasyncio
+from juju import jasyncio, url
 from juju.client.client import ApplicationFacade, ConfigValue
 from juju.errors import JujuError, JujuUnitError, JujuConnectionError
 from juju.model import Model, ModelObserver
@@ -88,6 +88,39 @@ async def test_deploy_bundle_local_resource_relative_path(event_loop):
                                 timeout=60 * 4)
 
 
+@base.bootstrapped
+@pytest.mark.asyncio
+async def test_deploy_by_revision(event_loop):
+    async with base.CleanModel() as model:
+        app = await model.deploy('juju-qa-test',
+                                 application_name='test',
+                                 channel='2.0/stable',
+                                 series='xenial',
+                                 revision=19,)
+
+        assert url.URL.parse(app.charm_url).revision == 19
+
+
+@base.bootstrapped
+@pytest.mark.asyncio
+async def test_deploy_by_revision_validate_flags(event_loop):
+    # Make sure we fail gracefully when invalid --revision/--channel
+    # flags are used
+
+    async with base.CleanModel() as model:
+        # For charms --revision requires --channel
+        with pytest.raises(JujuError):
+            await model.deploy('juju-qa-test',
+                               # channel='2.0/stable',
+                               revision=22)
+
+        # For bundles, --revision and --channel are mutually exclusive
+        with pytest.raises(JujuError):
+            await model.deploy('ch:canonical-livepatch-onprem',
+                               channel='latest/stable',
+                               revision=4)
+
+
 @base.bootstrapped
 @pytest.mark.asyncio
 async def test_deploy_local_bundle_include_file(event_loop):