From 573a7566c444e6f1a64df977f7c910e55717ab10 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Wed, 31 Mar 2021 11:33:35 +0100
Subject: [PATCH 01/18] Deploy add charm

The following adds the ability to add a charm based on the charm facade.
The client facade is deprecated and shouldn't be used if the charms
facade is located.
---
 juju/model.py | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/juju/model.py b/juju/model.py
index 23c1a0faf..ca64df363 100644
--- a/juju/model.py
+++ b/juju/model.py
@@ -1433,8 +1433,6 @@ async def deploy(
                                                   include_stats=False)
             entity_id = entity['Id']
 
-        client_facade = client.ClientFacade.from_connection(self.connection())
-
         is_bundle = ((is_local and
                       (entity_id.endswith('.yaml') and entity_path.exists()) or
                       bundle_path.exists()) or
@@ -1463,7 +1461,8 @@ async def deploy(
                     application_name = entity['Meta']['charm-metadata']['Name']
                 if not series:
                     series = self._get_series(entity_url, entity)
-                await client_facade.AddCharm(channel=channel, url=entity_id, force=False)
+
+                self._add_charm(channel, entity_id)
                 # XXX: we're dropping local resources here, but we don't
                 # actually support them yet anyway
                 resources = await self._add_store_resources(application_name,
@@ -1506,6 +1505,17 @@ async def deploy(
                 devices=devices,
             )
 
+    async def _add_charm(self, channel, entity_id):
+        # client facade is deprecated with in Juju, and smaller, more focused
+        # facades have been created and we'll use that if it's available.
+        charms_cls = client.CharmsFacade
+        if charms_cls.best_facade_version(self.connection()) > 2:
+            charms_facade = charms_cls.from_connection(self.connection())
+            return await charms_facade.AddCharm(channel=channel, url=entity_id, force=False)
+
+        client_facade = client.ClientFacade.from_connection(self.connection())
+        await client_facade.AddCharm(channel=channel, url=entity_id, force=False)
+
     async def _add_store_resources(self, application, entity_url,
                                    overrides=None, entity=None):
         if not entity:

From 8ffa7086ccb177fbcf25f09b1e7770d519841432 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Wed, 31 Mar 2021 11:57:05 +0100
Subject: [PATCH 02/18] Add architecture and series to charm URL

The v2 charm URL scheme should support both architecture and series.
Allowing us to deploy different architectures correctly.
---
 juju/url.py            | 22 +++++++++++++++++-----
 tests/unit/test_url.py |  8 ++++++++
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/juju/url.py b/juju/url.py
index 0f7b6e622..88ea6b981 100644
--- a/juju/url.py
+++ b/juju/url.py
@@ -16,7 +16,7 @@ def __str__(self):
 
 
 class URL:
-    def __init__(self, schema, user=None, name=None, revision=None, series=None):
+    def __init__(self, schema, user=None, name=None, revision=None, series=None, architecture=None):
         self.schema = schema
         self.user = user
         self.name = name
@@ -26,6 +26,7 @@ def __init__(self, schema, user=None, name=None, revision=None, series=None):
         if revision is None:
             revision = -1
         self.revision = revision
+        self.architecture = architecture
 
     @staticmethod
     def parse(s):
@@ -57,6 +58,8 @@ def path(self):
         parts = []
         if self.user:
             parts.append("~{}".format(self.user))
+        if self.architecture:
+            parts.append(self.architecture)
         if self.series:
             parts.append(self.series)
         if self.revision is not None and self.revision >= 0:
@@ -114,10 +117,19 @@ def parse_v2_url(u, s):
     c = URL(Schema.CHARM_HUB)
 
     parts = u.path.split("/")
-    if len(parts) != 1:
-        raise JujuError("charm or bundle URL {} malformed, expected <name>".format(s))
-
-    (c.name, c.revision) = extract_revision(parts[0])
+    num = len(parts)
+    if num == 0 or num > 3:
+        raise JujuError("charm or bundle URL {} malformed".format(s))
+
+    name = ""
+    if num == 3:
+        c.architecture, c.series, name = parts[0], parts[1], parts[2]
+    elif num == 2:
+        c.architecture, name = parts[0], parts[1]
+    else:
+        name = parts[0]
+
+    (c.name, c.revision) = extract_revision(name)
     # TODO (stickupkid) - validate the name.
 
     return c
diff --git a/tests/unit/test_url.py b/tests/unit/test_url.py
index adcb6ce48..bf875fa9a 100644
--- a/tests/unit/test_url.py
+++ b/tests/unit/test_url.py
@@ -31,6 +31,14 @@ def test_parse_v1_series(self):
 
 class TestURLV2(unittest.TestCase):
     def test_parse_charmhub(self):
+        u = URL.parse("ch:arm64/bionic/mysql-1")
+        self.assertEqual(u, URL(Schema.CHARM_HUB, name="mysql", architecture="amd64", series="bionic", revision=1))
+
+    def test_parse_charmhub_with_no_series(self):
+        u = URL.parse("ch:arm64/mysql")
+        self.assertEqual(u, URL(Schema.CHARM_HUB, name="mysql", architecture="amd64"))
+
+    def test_parse_charmhub_with_no_series_arch(self):
         u = URL.parse("ch:mysql")
         self.assertEqual(u, URL(Schema.CHARM_HUB, name="mysql"))
 

From 1a0dc2f7237053064c7199124dc98c8a780adf3f Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Wed, 31 Mar 2021 13:10:37 +0100
Subject: [PATCH 03/18] Allow the deploying of a charm

There was some issue with adding a charm, but that was easily resolved.
---
 juju/model.py | 47 +++++++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/juju/model.py b/juju/model.py
index ca64df363..4918f2f65 100644
--- a/juju/model.py
+++ b/juju/model.py
@@ -37,6 +37,7 @@
 from .offerendpoints import parse_local_endpoint, parse_offer_url
 from .placement import parse as parse_placement
 from .tag import application as application_tag
+from .url import URL, Schema
 
 log = logging.getLogger(__name__)
 
@@ -1416,27 +1417,37 @@ async def deploy(
         if trust and (self.info.agent_version < client.Number.from_json('2.4.0')):
             raise NotImplementedError("trusted is not supported on model version {}".format(self.info.agent_version))
 
-        entity_url = str(entity_url)  # allow for pathlib.Path objects
-        entity_path = Path(entity_url.replace('local:', ''))
-        bundle_path = entity_path / 'bundle.yaml'
-        metadata_path = entity_path / 'metadata.yaml'
 
-        is_local = (
-            entity_url.startswith('local:') or
-            entity_path.is_dir() or
-            entity_path.is_file()
-        )
-        if is_local:
-            entity_id = entity_url.replace('local:', '')
-        else:
-            entity = await self.charmstore.entity(entity_url, channel=channel,
+        # Attempt to resolve a charm or bundle based on the URL.
+        # In an ideal world this should be moved to the controller, and we
+        # wouldn't have to deal with this at all.
+        is_local = False
+        is_bundle = False
+        identifier = None
+        url = URL.parse(str(entity_url))
+        if url.schema.matches(Schema.Local):
+            entity_url = url.path()
+            entity_path = Path(entity_url)
+            bundle_path = entity_path / 'bundle.yaml'
+
+            identifier = entity_url
+            is_local = (
+                entity_path.is_dir() or
+                entity_path.is_file()
+            )
+            is_bundle = (
+                (entity_url.endswith(".yaml") and entity_path.exists()) or 
+                bundle_path.exists()
+            )
+        elif url.schema.matches(Schema.CHARM_STORE):
+            charm = await self.charmstore.entity(str(url), channel=channel,
                                                   include_stats=False)
-            entity_id = entity['Id']
+            identifier = charm['Id']
+            is_bundle = url.series == "bundle"
+        elif url.schema.matches(Schema.CHARM_HUB):
+            # TODO (stickupkid): request to get the charm id.
+
 
-        is_bundle = ((is_local and
-                      (entity_id.endswith('.yaml') and entity_path.exists()) or
-                      bundle_path.exists()) or
-                     (not is_local and 'bundle/' in entity_id))
 
         if is_bundle:
             handler = BundleHandler(self, trusted=trust, forced=force)

From aad7761e244c836b9a04bb8f123fba04dfb8f980 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Wed, 31 Mar 2021 17:17:24 +0100
Subject: [PATCH 04/18] Allow the deploying of charms and bundles

This is still a work in progress, but essentially charms without
resources are able to be deployed and bundle machines are. There is an
issue with a missing charm revision for charmhub charms, I'm unsure
where to resolve that currently.
---
 juju/bundle.py            | 107 ++++++++++++++++++++++++++----
 juju/client/connection.py |   2 +-
 juju/model.py             | 132 +++++++++++++++++++++++++++++---------
 juju/url.py               |   6 +-
 4 files changed, 200 insertions(+), 47 deletions(-)

diff --git a/juju/bundle.py b/juju/bundle.py
index 3c68a4648..5c3c5a065 100644
--- a/juju/bundle.py
+++ b/juju/bundle.py
@@ -1,7 +1,10 @@
 import asyncio
 import logging
+import io
 import os
 import zipfile
+import requests
+from contextlib import closing
 from pathlib import Path
 
 import yaml
@@ -11,6 +14,8 @@
 from .client import client
 from .constraints import parse as parse_constraints, parse_storage_constraint, parse_device_constraint
 from .errors import JujuError
+from .origin import Channel, Risk
+from .url import Schema, URL
 
 log = logging.getLogger(__name__)
 
@@ -128,22 +133,28 @@ async def _handle_local_charms(self, bundle, bundle_dir):
 
         return bundle
 
-    async def fetch_plan(self, entity_id):
-        is_store_url = entity_id.startswith('cs:')
-        is_local = False
+    async def fetch_plan(self, charm_url, origin):
+        entity_id = charm_url.path()
+        is_local = Schema.LOCAL.matches(charm_url.schema)
         bundle_dir = None
 
-        if not is_store_url and os.path.isfile(entity_id):
+        if is_local and os.path.isfile(entity_id):
             bundle_yaml = Path(entity_id).read_text()
-            is_local = True
             bundle_dir = Path(entity_id).parent
-        elif not is_store_url and os.path.isdir(entity_id):
+        elif is_local and os.path.isdir(entity_id):
             bundle_yaml = (Path(entity_id) / "bundle.yaml").read_text()
             bundle_dir = Path(entity_id)
-        else:
+
+        if Schema.CHARM_STORE.matches(charm_url.schema):
             bundle_yaml = await self.charmstore.files(entity_id,
                                                       filename='bundle.yaml',
                                                       read_file=True)
+        elif Schema.CHARM_HUB.matches(charm_url.schema):
+            bundle_yaml = await self._download_bundle(charm_url, origin)
+
+        if not bundle_yaml:
+            raise JujuError('empty bundle, nothing to deploy')
+
         self.bundle = yaml.safe_load(bundle_yaml)
         self.bundle = await self._validate_bundle(self.bundle)
         if is_local:
@@ -156,6 +167,46 @@ async def fetch_plan(self, entity_id):
         if self.plan.errors:
             raise JujuError(self.plan.errors)
 
+    async def _download_bundle(self, charm_url, origin):
+        charms_cls = client.CharmsFacade
+        if charms_cls.best_facade_version(self.model.connection()) > 2:
+            charms_facade = charms_cls.from_connection(self.model.connection())
+            resp = await charms_facade.GetDownloadInfos(entities=[{
+                'charm-url': str(charm_url),
+                'charm-origin': {
+                    'source': origin.source,
+                    'type': origin.type_,
+                    'id': origin.id_,
+                    'hash': origin.hash_,
+                    'revision': origin.revision,
+                    'risk': origin.risk,
+                    'track': origin.track,
+                    'architecture': origin.architecture,
+                    'os': origin.os,
+                    'series': origin.series,
+                }
+            }])
+            if len(resp.results) != 1:
+                raise JujuError("expected one result, received {}".format(resp.results))
+            
+            result = resp.results[0]
+            if not result.url:
+                raise JujuError("no url found for bundle {}".format(charm_url.name))
+
+            bundle_resp = requests.get(result.url)
+            bundle_resp.raise_for_status()
+
+            with closing(bundle_resp), zipfile.ZipFile(io.BytesIO(bundle_resp.content)) as archive:
+                return self._get_bundle_yaml(archive)
+
+        raise JujuError('charm facade not supported')
+    
+    def _get_bundle_yaml(self, archive):
+        for member in archive.infolist():
+            if member.filename == "bundle.yaml":
+                return archive.read(member)
+        raise JujuError("bundle.yaml not found")
+
     async def execute_plan(self):
         changes = ChangeSet(self.plan.changes)
         for step in changes.sorted():
@@ -338,11 +389,14 @@ async def run(self, context):
             if context.model.info.agent_version < client.Number.from_json('2.4.0'):
                 raise NotImplementedError("trusted is not supported on model version {}".format(context.model.info.agent_version))
             options["trust"] = "true"
-        if not charm.startswith('local:'):
+
+        url = URL.parse(str(charm))
+        if Schema.CHARM_STORE.matches(url.schema):
             resources = await context.model._add_store_resources(
                 self.application, charm, overrides=self.resources)
         else:
             resources = {}
+
         await context.model._deploy(
             charm_url=charm,
             application=self.application,
@@ -404,6 +458,10 @@ def __init__(self, change_id, requires, params=None):
                 self.channel = params[2]
             else:
                 self.channel = None
+            if len(params) > 3 and params[3] != "":
+                self.architecture = params[3]
+            else:
+                self.architecture = None
         elif isinstance(params, dict):
             AddCharmChange.from_dict(self, params)
         else:
@@ -422,15 +480,38 @@ async def run(self, context):
         :param context: is used for any methods or properties required to
             perform a change.
         """
+
         # We don't add local charms because they've already been added
         # by self._handle_local_charms
-        if self.charm.startswith('local:'):
+        url = URL.parse(str(self.charm))
+        identifier = None
+        if Schema.LOCAL.matches(url.schema):
             return self.charm
 
-        entity_id = await context.charmstore.entityId(self.charm)
-        log.debug('Adding %s', entity_id)
-        await context.client_facade.AddCharm(channel=None, url=entity_id, force=False)
-        return entity_id
+        elif Schema.CHARM_STORE.matches(url.schema):
+            entity_id = await context.charmstore.entityId(self.charm)
+            log.debug('Adding %s', entity_id)
+            await context.client_facade.AddCharm(channel=None, url=entity_id, force=False)
+            identifier = entity_id
+
+        elif Schema.CHARM_HUB.matches(url.schema):
+            ch = Channel('latest', 'stable')
+            if self.channel:
+                ch = Channel.parse(self.channel).normalize()
+            arch = self.architecture
+            if not arch:
+                arch = await context.model._resolve_architecture(url)
+            origin = client.CharmOrigin(source="charm-hub", 
+                                        architecture=arch,
+                                        risk=ch.risk,
+                                        track=ch.track)
+            identifier, origin = await context.model._resolve_charm(url, origin)
+
+        if identifier is None:
+            raise JujuError('unknown charm {}'.format(self.charm))
+
+        await context.model._add_charm(identifier, origin)
+        return url.path()
 
     def __str__(self):
         series = ""
diff --git a/juju/client/connection.py b/juju/client/connection.py
index a328fcd2b..8daeb58a6 100644
--- a/juju/client/connection.py
+++ b/juju/client/connection.py
@@ -31,7 +31,7 @@
     'Bundle': {'versions': [1, 2, 3]},
     'CharmHub': {'versions': [1]},
     'CharmRevisionUpdater': {'versions': [2]},
-    'Charms': {'versions': [2]},
+    'Charms': {'versions': [2, 3, 4]},
     'Cleaner': {'versions': [2]},
     'Client': {'versions': [1, 2]},
     'Cloud': {'versions': [1, 2, 3, 4, 5]},
diff --git a/juju/model.py b/juju/model.py
index 4918f2f65..ba3b2a946 100644
--- a/juju/model.py
+++ b/juju/model.py
@@ -35,6 +35,7 @@
 from .names import is_valid_application
 from .offerendpoints import ParseError as OfferParseError
 from .offerendpoints import parse_local_endpoint, parse_offer_url
+from .origin import Channel, Risk
 from .placement import parse as parse_placement
 from .tag import application as application_tag
 from .url import URL, Schema
@@ -1424,34 +1425,67 @@ async def deploy(
         is_local = False
         is_bundle = False
         identifier = None
+        origin = None
+        result = None
+
         url = URL.parse(str(entity_url))
-        if url.schema.matches(Schema.Local):
+        architecture = await self._resolve_architecture(url)
+
+        if Schema.LOCAL.matches(url.schema):
             entity_url = url.path()
             entity_path = Path(entity_url)
             bundle_path = entity_path / 'bundle.yaml'
 
             identifier = entity_url
-            is_local = (
-                entity_path.is_dir() or
-                entity_path.is_file()
-            )
+            origin = client.CharmOrigin(source="local", architecture=architecture)
+            if not (entity_path.is_dir() or entity_path.is_file()):
+                raise JujuError('{} path not found'.format(entity_url))
+            
             is_bundle = (
                 (entity_url.endswith(".yaml") and entity_path.exists()) or 
                 bundle_path.exists()
             )
-        elif url.schema.matches(Schema.CHARM_STORE):
-            charm = await self.charmstore.entity(str(url), channel=channel,
+
+        elif Schema.CHARM_STORE.matches(url.schema):
+            result = await self.charmstore.entity(str(url), 
+                                                  channel=channel,
                                                   include_stats=False)
-            identifier = charm['Id']
+            identifier = result['Id']
+            origin = client.CharmOrigin(source="charm-store", 
+                                        architecture=architecture,
+                                        risk=channel)
             is_bundle = url.series == "bundle"
-        elif url.schema.matches(Schema.CHARM_HUB):
-            # TODO (stickupkid): request to get the charm id.
-
-
+            if not series:
+                series = self._get_series(entity_url, result)
+
+        elif Schema.CHARM_HUB.matches(url.schema):
+            ch = Channel('latest', 'stable')
+            if channel:
+                ch = Channel.parse(channel).normalize()
+            origin = client.CharmOrigin(source="charm-hub", 
+                                        architecture=architecture,
+                                        risk=ch.risk,
+                                        track=ch.track)
+            charm_url, origin = await self._resolve_charm(url, origin)
+
+            identifier = charm_url
+            is_bundle = origin.type_ == "bundle"
+
+        if identifier is None:
+            raise JujuError('unknown charm or bundle {}'.format(entity_url))
+
+        if not application_name:
+            if Schema.LOCAL.matches(url.schema) and Schema.CHARM_STORE.matches(url.schema):
+                application_name = result['Meta']['charm-metadata']['Name']
+            else:
+                # For charmhub charms, we don't have the metadata and we're not
+                # going to get it, so fallback to the url and use that one if a
+                # user didn't specify it.
+                application_name = url.name
 
         if is_bundle:
             handler = BundleHandler(self, trusted=trust, forced=force)
-            await handler.fetch_plan(entity_id)
+            await handler.fetch_plan(url, origin)
             await handler.execute_plan()
             extant_apps = {app for app in self.applications}
             pending_apps = set(handler.applications) - extant_apps
@@ -1467,42 +1501,47 @@ async def deploy(
             return [app for name, app in self.applications.items()
                     if name in handler.applications]
         else:
+            # XXX: we're dropping local resources here, but we don't
+            # actually support them yet anyway
             if not is_local:
-                if not application_name:
-                    application_name = entity['Meta']['charm-metadata']['Name']
-                if not series:
-                    series = self._get_series(entity_url, entity)
-
-                self._add_charm(channel, entity_id)
-                # XXX: we're dropping local resources here, but we don't
-                # actually support them yet anyway
-                resources = await self._add_store_resources(application_name,
-                                                            entity_id,
-                                                            entity=entity)
+                await self._add_charm(identifier, origin)
+
+                # TODO (stickupkid): Handle charmhub charms, for now we'll only
+                # handle charmstore charms.
+                if Schema.CHARM_STORE.matches(url.schema):
+                    resources = await self._add_store_resources(application_name,
+                                                                identifier,
+                                                                entity=result)
             else:
                 if not application_name:
+                    entity_url = url.path()
+                    entity_path = Path(entity_url)
                     if str(entity_path).endswith('.charm'):
                         with zipfile.ZipFile(entity_path, 'r') as charm_file:
                             metadata = yaml.load(charm_file.read('metadata.yaml'), Loader=yaml.FullLoader)
                     else:
+                        metadata_path = entity_path / 'metadata.yaml'
                         metadata = yaml.load(metadata_path.read_text(), Loader=yaml.FullLoader)
                     application_name = metadata['name']
+
                 # We have a local charm dir that needs to be uploaded
                 charm_dir = os.path.abspath(
-                    os.path.expanduser(entity_id))
+                    os.path.expanduser(identifier))
                 series = series or get_charm_series(charm_dir)
                 if not series:
                     raise JujuError(
                         "Couldn't determine series for charm at {}. "
                         "Pass a 'series' kwarg to Model.deploy().".format(
                             charm_dir))
-                entity_id = await self.add_local_charm_dir(charm_dir, series)
+                identifier = await self.add_local_charm_dir(charm_dir, series)
+
             if config is None:
                 config = {}
             if trust:
                 config["trust"] = "true"
+
             return await self._deploy(
-                charm_url=entity_id,
+                charm_url=identifier,
                 application=application_name,
                 series=series,
                 config=config,
@@ -1516,16 +1555,49 @@ async def deploy(
                 devices=devices,
             )
 
-    async def _add_charm(self, channel, entity_id):
+    async def _add_charm(self, charm_url, origin):
         # client facade is deprecated with in Juju, and smaller, more focused
         # facades have been created and we'll use that if it's available.
         charms_cls = client.CharmsFacade
         if charms_cls.best_facade_version(self.connection()) > 2:
             charms_facade = charms_cls.from_connection(self.connection())
-            return await charms_facade.AddCharm(channel=channel, url=entity_id, force=False)
+            return await charms_facade.AddCharm(charm_origin=origin, url=charm_url, force=False)
 
         client_facade = client.ClientFacade.from_connection(self.connection())
-        await client_facade.AddCharm(channel=channel, url=entity_id, force=False)
+        await client_facade.AddCharm(channel=origin.channel, url=charm_url, force=False)
+
+    async def _resolve_charm(self, url, origin):
+        charms_cls = client.CharmsFacade
+        if charms_cls.best_facade_version(self.connection()) < 3:
+            raise JujuError("resolve charm")
+
+        charms_facade = charms_cls.from_connection(self.connection())
+
+        resp = await charms_facade.ResolveCharms(resolve=[{
+            'reference': str(url),
+            'charm-origin': {
+                'source': 'charm-hub',
+                'architecture': origin.architecture,
+            }
+        }])
+        if len(resp.results) != 1:
+            raise JujuError("expected one result, received {}".format(resp.results))
+
+        result = resp.results[0]
+        if result.error:
+            raise JujuError(result.error.message)
+
+        return (result.url, result.charm_origin)
+
+    async def _resolve_architecture(self, url):
+        if url.architecture:
+            return url.architecture
+        
+        constraints = await self.get_constraints()
+        if 'arch' in constraints:
+            return constraints['arch']
+        
+        return "amd64"
 
     async def _add_store_resources(self, application, entity_url,
                                    overrides=None, entity=None):
diff --git a/juju/url.py b/juju/url.py
index 88ea6b981..815052391 100644
--- a/juju/url.py
+++ b/juju/url.py
@@ -9,10 +9,10 @@ class Schema(Enum):
     CHARM_HUB = "ch"
 
     def matches(self, potential):
-        return self.value == potential
+        return str(self.value) == str(potential)
 
     def __str__(self):
-        return self.value
+        return str(self.value)
 
 
 class URL:
@@ -63,7 +63,7 @@ def path(self):
         if self.series:
             parts.append(self.series)
         if self.revision is not None and self.revision >= 0:
-            parts.append("{}-{}", self.name, self.revision)
+            parts.append("{}-{}".format(self.name, self.revision))
         else:
             parts.append(self.name)
         return "/".join(parts)

From c3e4a84d11a9c7ba4196fa4f181e1219331b9b87 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Tue, 13 Apr 2021 16:32:44 +0100
Subject: [PATCH 05/18] Allow charmhub bundles to be deployed

---
 juju/application.py       |   8 +++
 juju/bundle.py            | 106 ++++++++++++++++++++++++++++++++++----
 juju/model.py             |   8 ++-
 tests/unit/test_bundle.py |   2 +-
 4 files changed, 113 insertions(+), 11 deletions(-)

diff --git a/juju/application.py b/juju/application.py
index 1ac6f8540..062c2343f 100644
--- a/juju/application.py
+++ b/juju/application.py
@@ -481,6 +481,14 @@ async def run(self, command, timeout=None):
             units=[],
         )
 
+    @property
+    def charm_url(self):
+        """Get the charm url for a given application
+
+        :return string: The charm url for an application
+        """
+        return self.safe_data['charm-url']
+
     async def get_annotations(self):
         """Get annotations on this application.
 
diff --git a/juju/bundle.py b/juju/bundle.py
index 5c3c5a065..e7138022c 100644
--- a/juju/bundle.py
+++ b/juju/bundle.py
@@ -14,7 +14,7 @@
 from .client import client
 from .constraints import parse as parse_constraints, parse_storage_constraint, parse_device_constraint
 from .errors import JujuError
-from .origin import Channel, Risk
+from .origin import Channel
 from .url import Schema, URL
 
 log = logging.getLogger(__name__)
@@ -34,10 +34,12 @@ def __init__(self, model, trusted=False, forced=False):
         self.plan = []
         self.references = {}
         self._units_by_app = {}
+        self.origins = {}
 
         for unit_name, unit in model.units.items():
             app_units = self._units_by_app.setdefault(unit.application, [])
             app_units.append(unit_name)
+
         self.bundle_facade = client.BundleFacade.from_connection(
             model.connection())
         self.client_facade = client.ClientFacade.from_connection(
@@ -47,6 +49,14 @@ def __init__(self, model, trusted=False, forced=False):
         self.ann_facade = client.AnnotationsFacade.from_connection(
             model.connection())
 
+        # Feature detect if we have the new charms facade, otherwise fallback
+        # to the client facade, when making calls.
+        if client.CharmsFacade.best_facade_version(model.connection()) > 2:
+            self.charms_facade = client.CharmsFacade.from_connection(
+                model.connection())
+        else:
+            self.charms_facade = None
+
         # This describes all the change types that the BundleHandler supports.
         change_type_cls = [AddApplicationChange,
                            AddCharmChange,
@@ -168,10 +178,8 @@ async def fetch_plan(self, charm_url, origin):
             raise JujuError(self.plan.errors)
 
     async def _download_bundle(self, charm_url, origin):
-        charms_cls = client.CharmsFacade
-        if charms_cls.best_facade_version(self.model.connection()) > 2:
-            charms_facade = charms_cls.from_connection(self.model.connection())
-            resp = await charms_facade.GetDownloadInfos(entities=[{
+        if self.charms_facade is not None:
+            resp = await self.charms_facade.GetDownloadInfos(entities=[{
                 'charm-url': str(charm_url),
                 'charm-origin': {
                     'source': origin.source,
@@ -206,8 +214,63 @@ def _get_bundle_yaml(self, archive):
             if member.filename == "bundle.yaml":
                 return archive.read(member)
         raise JujuError("bundle.yaml not found")
+    
+    async def _resolve_charms(self):
+        deployed = dict()
+
+        specs = self.applications_specs
+        for name in self.applications:
+            spec = specs[name]
+            app = self.model.applications.get(name, None)
+
+            cons = None
+            if app is not None:
+                deployed[name] = name
+
+                if is_local_charm(spec['charm']):
+                    spec.charm = self.model.applications[name]
+                    continue
+
+                if spec['charm'] == app.charm_url:
+                    continue
+
+                cons = await app.get_constraints()
+
+            if is_local_charm(spec['charm']):
+                continue
+
+            if self.charms_facade is not None:
+                charm_url = URL.parse(spec['charm'])
+                channel = None
+                track, risk = '', ''
+                if 'channel' in spec: 
+                    channel = Channel.parse(spec['channel'])
+                    track, risk = channel.track, channel.risk
+
+                if cons is not None and cons['arch'] != '':
+                    architecture = cons['arch']
+                else:
+                    architecture = await self.model._resolve_architecture(charm_url)
+
+                origin = client.CharmOrigin(source="charm-hub", 
+                                        architecture=architecture,
+                                        risk=risk,
+                                        track=track)
+                charm_url, charm_origin = await self.model._resolve_charm(charm_url, origin)
+
+                spec['charm'] = str(charm_url)
+
+                if str(channel) not in self.origins:
+                    self.origins[str(charm_url)] = {}
+                self.origins[str(charm_url)][str(channel)] = charm_origin
+            else:
+                results = await self.client_facade.ResolveCharms(references=[spec['charm']])
+                # TODO (stickupkid): Ensure that this works as expected.
+
 
     async def execute_plan(self):
+        await self._resolve_charms()
+
         changes = ChangeSet(self.plan.changes)
         for step in changes.sorted():
             change_cls = self.change_types.get(step.method)
@@ -222,8 +285,13 @@ def applications(self):
         apps_dict = self.bundle.get('applications',
                                     self.bundle.get('services', {}))
         return list(apps_dict.keys())
+    
+    @property
+    def applications_specs(self):
+        return self.bundle.get('applications',
+                                self.bundle.get('services', {}))
 
-    def resolveRelation(self, reference):
+    def resolve_relation(self, reference):
         parts = reference.split(":", maxsplit=1)
         application = self.resolve(parts[0])
         if len(parts) == 1:
@@ -238,6 +306,10 @@ def resolve(self, reference):
         return reference
 
 
+def is_local_charm(charm_url):
+    return charm_url.startswith('.') or charm_url.startswith('local:') 
+
+
 def get_charm_series(path):
     """Inspects the charm directory at ``path`` and returns a default
     series from its metadata.yaml (the first item in the 'series' list).
@@ -354,6 +426,7 @@ def __init__(self, change_id, requires, params=None):
                 self.resources = params[7]
                 self.devices = None
                 self.num_units = None
+                self.channel = None
             else:
                 # Juju 2.5+ sends devices before endpoint bindings, as well as num_units
                 # There might be placement but we need to ignore that.
@@ -361,6 +434,7 @@ def __init__(self, change_id, requires, params=None):
                 self.endpoint_bindings = params[7]
                 self.resources = params[8]
                 self.num_units = params[9]
+                self.channel = params[10]
 
         elif isinstance(params, dict):
             AddApplicationChange.from_dict(self, params)
@@ -397,6 +471,14 @@ async def run(self, context):
         else:
             resources = {}
 
+        channel = None
+        if self.channel is not None:
+            channel = Channel.parse(self.channel).normalize()
+
+        origin = context.origins[str(url)][str(channel)]
+        if origin is None:
+            raise JujuError("expected origin to be valid for application {} and charm {}".format(self.application, self.charm))
+
         await context.model._deploy(
             charm_url=charm,
             application=self.application,
@@ -484,6 +566,7 @@ async def run(self, context):
         # We don't add local charms because they've already been added
         # by self._handle_local_charms
         url = URL.parse(str(self.charm))
+        ch = None
         identifier = None
         if Schema.LOCAL.matches(url.schema):
             return self.charm
@@ -511,7 +594,12 @@ async def run(self, context):
             raise JujuError('unknown charm {}'.format(self.charm))
 
         await context.model._add_charm(identifier, origin)
-        return url.path()
+
+        if str(ch) not in context.origins:
+            context.origins[str(identifier)] = {}
+        context.origins[str(identifier)][str(ch)] = origin
+
+        return str(identifier) if identifier is not None else url.path()
 
     def __str__(self):
         series = ""
@@ -657,8 +745,8 @@ async def run(self, context):
         :param context: is used for any methods or properties required to
             perform a change.
         """
-        ep1 = context.resolveRelation(self.endpoint1)
-        ep2 = context.resolveRelation(self.endpoint2)
+        ep1 = context.resolve_relation(self.endpoint1)
+        ep2 = context.resolve_relation(self.endpoint2)
 
         log.info('Relating %s <-> %s', ep1, ep2)
         return await context.model.add_relation(ep1, ep2)
diff --git a/juju/model.py b/juju/model.py
index ba3b2a946..07458d540 100644
--- a/juju/model.py
+++ b/juju/model.py
@@ -1573,11 +1573,17 @@ async def _resolve_charm(self, url, origin):
 
         charms_facade = charms_cls.from_connection(self.connection())
 
+        if Schema.CHARM_STORE.matches(url.schema):
+            source = "charm-store"
+        else:
+            source = "charm-hub"
         resp = await charms_facade.ResolveCharms(resolve=[{
             'reference': str(url),
             'charm-origin': {
-                'source': 'charm-hub',
+                'source': source,
                 'architecture': origin.architecture,
+                'track': origin.track,
+                'risk': origin.risk,
             }
         }])
         if len(resp.results) != 1:
diff --git a/tests/unit/test_bundle.py b/tests/unit/test_bundle.py
index 77f93a552..ed8e723ba 100644
--- a/tests/unit/test_bundle.py
+++ b/tests/unit/test_bundle.py
@@ -411,7 +411,7 @@ async def test_run(self, event_loop):
         model.add_relation = base.AsyncMock(return_value="relation1")
 
         context = mock.Mock()
-        context.resolveRelation = mock.Mock(side_effect=['endpoint_1', 'endpoint_2'])
+        context.resolve_relation = mock.Mock(side_effect=['endpoint_1', 'endpoint_2'])
         context.model = model
 
         result = await change.run(context)

From e8129afac41ea25e6c7c33f75c5ce6d2b566c1f0 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Tue, 13 Apr 2021 17:00:00 +0100
Subject: [PATCH 06/18] Fix the linting issues

---
 juju/bundle.py | 27 +++++++++++++--------------
 juju/model.py  | 17 ++++++++---------
 2 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/juju/bundle.py b/juju/bundle.py
index e7138022c..52d54a7b0 100644
--- a/juju/bundle.py
+++ b/juju/bundle.py
@@ -196,7 +196,7 @@ async def _download_bundle(self, charm_url, origin):
             }])
             if len(resp.results) != 1:
                 raise JujuError("expected one result, received {}".format(resp.results))
-            
+
             result = resp.results[0]
             if not result.url:
                 raise JujuError("no url found for bundle {}".format(charm_url.name))
@@ -208,13 +208,13 @@ async def _download_bundle(self, charm_url, origin):
                 return self._get_bundle_yaml(archive)
 
         raise JujuError('charm facade not supported')
-    
+
     def _get_bundle_yaml(self, archive):
         for member in archive.infolist():
             if member.filename == "bundle.yaml":
                 return archive.read(member)
         raise JujuError("bundle.yaml not found")
-    
+
     async def _resolve_charms(self):
         deployed = dict()
 
@@ -243,7 +243,7 @@ async def _resolve_charms(self):
                 charm_url = URL.parse(spec['charm'])
                 channel = None
                 track, risk = '', ''
-                if 'channel' in spec: 
+                if 'channel' in spec:
                     channel = Channel.parse(spec['channel'])
                     track, risk = channel.track, channel.risk
 
@@ -252,10 +252,10 @@ async def _resolve_charms(self):
                 else:
                     architecture = await self.model._resolve_architecture(charm_url)
 
-                origin = client.CharmOrigin(source="charm-hub", 
-                                        architecture=architecture,
-                                        risk=risk,
-                                        track=track)
+                origin = client.CharmOrigin(source="charm-hub",
+                                            architecture=architecture,
+                                            risk=risk,
+                                            track=track)
                 charm_url, charm_origin = await self.model._resolve_charm(charm_url, origin)
 
                 spec['charm'] = str(charm_url)
@@ -264,10 +264,9 @@ async def _resolve_charms(self):
                     self.origins[str(charm_url)] = {}
                 self.origins[str(charm_url)][str(channel)] = charm_origin
             else:
-                results = await self.client_facade.ResolveCharms(references=[spec['charm']])
+                await self.client_facade.ResolveCharms(references=[spec['charm']])
                 # TODO (stickupkid): Ensure that this works as expected.
 
-
     async def execute_plan(self):
         await self._resolve_charms()
 
@@ -285,11 +284,11 @@ def applications(self):
         apps_dict = self.bundle.get('applications',
                                     self.bundle.get('services', {}))
         return list(apps_dict.keys())
-    
+
     @property
     def applications_specs(self):
         return self.bundle.get('applications',
-                                self.bundle.get('services', {}))
+                               self.bundle.get('services', {}))
 
     def resolve_relation(self, reference):
         parts = reference.split(":", maxsplit=1)
@@ -307,7 +306,7 @@ def resolve(self, reference):
 
 
 def is_local_charm(charm_url):
-    return charm_url.startswith('.') or charm_url.startswith('local:') 
+    return charm_url.startswith('.') or charm_url.startswith('local:')
 
 
 def get_charm_series(path):
@@ -584,7 +583,7 @@ async def run(self, context):
             arch = self.architecture
             if not arch:
                 arch = await context.model._resolve_architecture(url)
-            origin = client.CharmOrigin(source="charm-hub", 
+            origin = client.CharmOrigin(source="charm-hub",
                                         architecture=arch,
                                         risk=ch.risk,
                                         track=ch.track)
diff --git a/juju/model.py b/juju/model.py
index 07458d540..77ef15ae4 100644
--- a/juju/model.py
+++ b/juju/model.py
@@ -35,7 +35,7 @@
 from .names import is_valid_application
 from .offerendpoints import ParseError as OfferParseError
 from .offerendpoints import parse_local_endpoint, parse_offer_url
-from .origin import Channel, Risk
+from .origin import Channel
 from .placement import parse as parse_placement
 from .tag import application as application_tag
 from .url import URL, Schema
@@ -1418,7 +1418,6 @@ async def deploy(
         if trust and (self.info.agent_version < client.Number.from_json('2.4.0')):
             raise NotImplementedError("trusted is not supported on model version {}".format(self.info.agent_version))
 
-
         # Attempt to resolve a charm or bundle based on the URL.
         # In an ideal world this should be moved to the controller, and we
         # wouldn't have to deal with this at all.
@@ -1440,18 +1439,18 @@ async def deploy(
             origin = client.CharmOrigin(source="local", architecture=architecture)
             if not (entity_path.is_dir() or entity_path.is_file()):
                 raise JujuError('{} path not found'.format(entity_url))
-            
+
             is_bundle = (
-                (entity_url.endswith(".yaml") and entity_path.exists()) or 
+                (entity_url.endswith(".yaml") and entity_path.exists()) or
                 bundle_path.exists()
             )
 
         elif Schema.CHARM_STORE.matches(url.schema):
-            result = await self.charmstore.entity(str(url), 
+            result = await self.charmstore.entity(str(url),
                                                   channel=channel,
                                                   include_stats=False)
             identifier = result['Id']
-            origin = client.CharmOrigin(source="charm-store", 
+            origin = client.CharmOrigin(source="charm-store",
                                         architecture=architecture,
                                         risk=channel)
             is_bundle = url.series == "bundle"
@@ -1462,7 +1461,7 @@ async def deploy(
             ch = Channel('latest', 'stable')
             if channel:
                 ch = Channel.parse(channel).normalize()
-            origin = client.CharmOrigin(source="charm-hub", 
+            origin = client.CharmOrigin(source="charm-hub",
                                         architecture=architecture,
                                         risk=ch.risk,
                                         track=ch.track)
@@ -1598,11 +1597,11 @@ async def _resolve_charm(self, url, origin):
     async def _resolve_architecture(self, url):
         if url.architecture:
             return url.architecture
-        
+
         constraints = await self.get_constraints()
         if 'arch' in constraints:
             return constraints['arch']
-        
+
         return "amd64"
 
     async def _add_store_resources(self, application, entity_url,

From d81320245d925eac0f5b498d946b044fa867b06f Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Wed, 14 Apr 2021 11:39:32 +0100
Subject: [PATCH 07/18] Ensure we can talk to old controllers

The following changes ensures that if we talk to older controllers we
can correctly deploy bundles as well.
---
 examples/deploy_bundle.py | 56 ++++++++++++++++++++++-----------------
 juju/bundle.py            | 32 +++++++++++-----------
 juju/model.py             |  3 ++-
 juju/url.py               | 14 ++++++----
 juju/version.py           |  4 +++
 5 files changed, 63 insertions(+), 46 deletions(-)
 create mode 100644 juju/version.py

diff --git a/examples/deploy_bundle.py b/examples/deploy_bundle.py
index 20da7b0e0..a7af4751e 100644
--- a/examples/deploy_bundle.py
+++ b/examples/deploy_bundle.py
@@ -6,35 +6,41 @@
 3. Destroys the units and applications
 
 """
+from juju.controller import Controller
 from juju import loop
-from juju.model import Model
 
 
 async def main():
-    model = Model()
-    print('Connecting to model')
-    # Connect to current model with current user, per Juju CLI
-    await model.connect()
-
-    try:
-        print('Deploying bundle')
-        applications = await model.deploy(
-            'cs:~juju-qa/bundle/basic-0',
-            channel='beta',
-        )
-
-        print('Waiting for active')
-        await model.block_until(
-            lambda: all(unit.workload_status == 'active'
-                        for application in applications for unit in application.units))
-        print("Successfully deployed!")
-        print('Removing bundle')
-        for application in applications:
-            await application.remove()
-    finally:
-        print('Disconnecting from model')
-        await model.disconnect()
-        print("Success")
+    controller = Controller()
+    # connect to current controller with current user, per Juju CLI
+    await controller.connect()
+
+    bundles = [('juju-qa-bundle-test', None), ('cs:~juju-qa/bundle/basic-0', 'beta')]
+    for i in range(len(bundles)): 
+        deployment = bundles[i]
+        model = await controller.add_model('model{}'.format(i))
+
+        try:
+            print('Deploying bundle')
+            applications = await model.deploy(
+                deployment[0],
+                channel=deployment[1],
+            )
+
+            print('Waiting for active')
+            await model.block_until(
+                lambda: all(unit.workload_status == 'active'
+                            for application in applications for unit in application.units))
+            print("Successfully deployed!")
+            print('Removing bundle')
+            for application in applications:
+                await application.remove()
+        finally:
+            print('Disconnecting from model')
+            await model.disconnect()
+            print("Success")
+
+    await controller.disconnect()
 
 
 if __name__ == '__main__':
diff --git a/juju/bundle.py b/juju/bundle.py
index 52d54a7b0..7f9e6bdda 100644
--- a/juju/bundle.py
+++ b/juju/bundle.py
@@ -16,6 +16,7 @@
 from .errors import JujuError
 from .origin import Channel
 from .url import Schema, URL
+from .version import LTS_RELEASES
 
 log = logging.getLogger(__name__)
 
@@ -230,7 +231,6 @@ async def _resolve_charms(self):
                 if is_local_charm(spec['charm']):
                     spec.charm = self.model.applications[name]
                     continue
-
                 if spec['charm'] == app.charm_url:
                     continue
 
@@ -239,14 +239,13 @@ async def _resolve_charms(self):
             if is_local_charm(spec['charm']):
                 continue
 
+            charm_url = URL.parse(spec['charm'])
+            channel = None
+            track, risk = '', ''
+            if 'channel' in spec:
+                channel = Channel.parse(spec['channel'])
+                track, risk = channel.track, channel.risk
             if self.charms_facade is not None:
-                charm_url = URL.parse(spec['charm'])
-                channel = None
-                track, risk = '', ''
-                if 'channel' in spec:
-                    channel = Channel.parse(spec['channel'])
-                    track, risk = channel.track, channel.risk
-
                 if cons is not None and cons['arch'] != '':
                     architecture = cons['arch']
                 else:
@@ -259,13 +258,15 @@ async def _resolve_charms(self):
                 charm_url, charm_origin = await self.model._resolve_charm(charm_url, origin)
 
                 spec['charm'] = str(charm_url)
-
-                if str(channel) not in self.origins:
-                    self.origins[str(charm_url)] = {}
-                self.origins[str(charm_url)][str(channel)] = charm_origin
             else:
-                await self.client_facade.ResolveCharms(references=[spec['charm']])
-                # TODO (stickupkid): Ensure that this works as expected.
+                results = await self.model.charmstore.entity(str(charm_url))
+                charm_origin = client.CharmOrigin(source="charm-store",
+                                                  risk=risk,
+                                                  track=track)
+
+            if str(channel) not in self.origins:
+                self.origins[str(charm_url)] = {}
+            self.origins[str(charm_url)][str(channel)] = charm_origin
 
     async def execute_plan(self):
         await self._resolve_charms()
@@ -471,7 +472,7 @@ async def run(self, context):
             resources = {}
 
         channel = None
-        if self.channel is not None:
+        if self.channel is not None and self.channel != "":
             channel = Channel.parse(self.channel).normalize()
 
         origin = context.origins[str(url)][str(channel)]
@@ -575,6 +576,7 @@ async def run(self, context):
             log.debug('Adding %s', entity_id)
             await context.client_facade.AddCharm(channel=None, url=entity_id, force=False)
             identifier = entity_id
+            origin = client.CharmOrigin(source="charm-store", risk="stable")
 
         elif Schema.CHARM_HUB.matches(url.schema):
             ch = Channel('latest', 'stable')
diff --git a/juju/model.py b/juju/model.py
index 77ef15ae4..04b689a35 100644
--- a/juju/model.py
+++ b/juju/model.py
@@ -39,6 +39,7 @@
 from .placement import parse as parse_placement
 from .tag import application as application_tag
 from .url import URL, Schema
+from .version import DEFAULT_ARCHITECTURE
 
 log = logging.getLogger(__name__)
 
@@ -1602,7 +1603,7 @@ async def _resolve_architecture(self, url):
         if 'arch' in constraints:
             return constraints['arch']
 
-        return "amd64"
+        return DEFAULT_ARCHITECTURE
 
     async def _add_store_resources(self, application, entity_url,
                                    overrides=None, entity=None):
diff --git a/juju/url.py b/juju/url.py
index 815052391..42a0dd01c 100644
--- a/juju/url.py
+++ b/juju/url.py
@@ -52,15 +52,18 @@ def parse(s):
         return c
 
     def with_revision(self, rev):
-        return URL(self.schema, self.user, self.name, rev, self.series)
+        return URL(self.schema, self.user, self.name, rev, self.series, self.architecture)
+
+    def with_series(self, series):
+        return URL(self.schema, self.user, self.name, self.revision, series, self.architecture)
 
     def path(self):
         parts = []
-        if self.user:
+        if self.user is not None:
             parts.append("~{}".format(self.user))
-        if self.architecture:
+        if self.architecture is not None:
             parts.append(self.architecture)
-        if self.series:
+        if self.series is not None:
             parts.append(self.series)
         if self.revision is not None and self.revision >= 0:
             parts.append("{}-{}".format(self.name, self.revision))
@@ -74,7 +77,8 @@ def __eq__(self, other):
                 self.user == other.user and \
                 self.name == other.name and \
                 self.revision == other.revision and \
-                self.series == other.series
+                self.series == other.series and \
+                self.architecture == other.architecture
         return False
 
     def __str__(self):
diff --git a/juju/version.py b/juju/version.py
new file mode 100644
index 000000000..793237475
--- /dev/null
+++ b/juju/version.py
@@ -0,0 +1,4 @@
+
+LTS_RELEASES = ["focal", "bionic", "xenial", "trusty", "precise"]
+
+DEFAULT_ARCHITECTURE = 'amd64'
\ No newline at end of file

From 37c1e5b2e5d5ea6bee6d00421d8c72c268fd2900 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Wed, 14 Apr 2021 11:52:12 +0100
Subject: [PATCH 08/18] Fix the issue of using a REALLY old controller

Using an older controller that doesn't have the facade available causes
all sorts of issues, this cleans up the issues around that.
---
 examples/deploy_bundle.py | 2 +-
 juju/bundle.py            | 5 +++--
 juju/model.py             | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/examples/deploy_bundle.py b/examples/deploy_bundle.py
index a7af4751e..648fef690 100644
--- a/examples/deploy_bundle.py
+++ b/examples/deploy_bundle.py
@@ -15,7 +15,7 @@ async def main():
     # connect to current controller with current user, per Juju CLI
     await controller.connect()
 
-    bundles = [('juju-qa-bundle-test', None), ('cs:~juju-qa/bundle/basic-0', 'beta')]
+    bundles = [('cs:~juju-qa/bundle/basic-0', 'beta'), ('juju-qa-bundle-test', None)]
     for i in range(len(bundles)): 
         deployment = bundles[i]
         model = await controller.add_model('model{}'.format(i))
diff --git a/juju/bundle.py b/juju/bundle.py
index 7f9e6bdda..45b1d50e1 100644
--- a/juju/bundle.py
+++ b/juju/bundle.py
@@ -420,13 +420,13 @@ def __init__(self, change_id, requires, params=None):
             self.options = params[3]
             self.constraints = params[4]
             self.storage = {k: parse_storage_constraint(v) for k, v in params[5].items()}
+            self.channel = None
             if len(params) == 8:
                 # Juju 2.4 and below only sends the endpoint bindings and resources
                 self.endpoint_bindings = params[6]
                 self.resources = params[7]
                 self.devices = None
                 self.num_units = None
-                self.channel = None
             else:
                 # Juju 2.5+ sends devices before endpoint bindings, as well as num_units
                 # There might be placement but we need to ignore that.
@@ -434,7 +434,8 @@ def __init__(self, change_id, requires, params=None):
                 self.endpoint_bindings = params[7]
                 self.resources = params[8]
                 self.num_units = params[9]
-                self.channel = params[10]
+                if len(params) > 10:
+                    self.channel = params[10]
 
         elif isinstance(params, dict):
             AddApplicationChange.from_dict(self, params)
diff --git a/juju/model.py b/juju/model.py
index 04b689a35..5b902724b 100644
--- a/juju/model.py
+++ b/juju/model.py
@@ -1564,7 +1564,7 @@ async def _add_charm(self, charm_url, origin):
             return await charms_facade.AddCharm(charm_origin=origin, url=charm_url, force=False)
 
         client_facade = client.ClientFacade.from_connection(self.connection())
-        await client_facade.AddCharm(channel=origin.channel, url=charm_url, force=False)
+        await client_facade.AddCharm(channel=str(origin.risk), url=charm_url, force=False)
 
     async def _resolve_charm(self, url, origin):
         charms_cls = client.CharmsFacade

From 62517e998fb38ad3f660a2cbb41cdfc191fac8d6 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Wed, 14 Apr 2021 14:09:09 +0100
Subject: [PATCH 09/18] Fix the integration for info request

The original charm we used for the integration tests have gone, let's
pick one that we can control.
---
 tests/integration/test_charmhub.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/integration/test_charmhub.py b/tests/integration/test_charmhub.py
index ad4c95d9f..b087f1b27 100644
--- a/tests/integration/test_charmhub.py
+++ b/tests/integration/test_charmhub.py
@@ -8,18 +8,18 @@
 @pytest.mark.asyncio
 async def test_info(event_loop):
     async with base.CleanModel() as model:
-        result = await model.charmhub.info("mattermost")
+        result = await model.charmhub.info("hello-juju")
 
-        assert result.result.name == "mattermost"
+        assert result.result.name == "hello-juju"
 
 
 @base.bootstrapped
 @pytest.mark.asyncio
 async def test_info_with_channel(event_loop):
     async with base.CleanModel() as model:
-        result = await model.charmhub.info("mattermost", "latest/stable")
+        result = await model.charmhub.info("hello-juju", "latest/stable")
 
-        assert result.result.name == "mattermost"
+        assert result.result.name == "hello-juju"
         assert "latest/stable" in result.result.channel_map
 
 

From 81d22c171709220cb6bf0b15ecf464cbbee648c9 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Wed, 14 Apr 2021 16:56:20 +0100
Subject: [PATCH 10/18] Start fixing the integration tests

The integration tests do work when run as a file, but fail when
attempting to run the whole thing as one. In order to get some tests
starting to pass green I think the best way would be to run each on and
see if that helps.
---
 juju/bundle.py                        |  6 ++++--
 juju/client/connection.py             |  7 +++++++
 juju/model.py                         | 14 ++++++++++----
 juju/url.py                           |  2 +-
 tests/integration/test_application.py | 22 +++++++++++++++++++++-
 tests/integration/test_connection.py  |  2 ++
 tests/integration/test_model.py       |  2 +-
 tests/integration/test_unit.py        |  2 +-
 8 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/juju/bundle.py b/juju/bundle.py
index 45b1d50e1..ec9838a9a 100644
--- a/juju/bundle.py
+++ b/juju/bundle.py
@@ -307,7 +307,7 @@ def resolve(self, reference):
 
 
 def is_local_charm(charm_url):
-    return charm_url.startswith('.') or charm_url.startswith('local:')
+    return charm_url.startswith('.') or charm_url.startswith('local:') or os.path.isabs(charm_url)
 
 
 def get_charm_series(path):
@@ -476,7 +476,7 @@ async def run(self, context):
         if self.channel is not None and self.channel != "":
             channel = Channel.parse(self.channel).normalize()
 
-        origin = context.origins[str(url)][str(channel)]
+        origin = context.origins.get(str(url), {}).get(str(channel), None)
         if origin is None:
             raise JujuError("expected origin to be valid for application {} and charm {}".format(self.application, self.charm))
 
@@ -570,6 +570,8 @@ async def run(self, context):
         ch = None
         identifier = None
         if Schema.LOCAL.matches(url.schema):
+            origin = client.CharmOrigin(source="local", risk="stable")
+            context.origins[self.charm] = {str(None): origin}
             return self.charm
 
         elif Schema.CHARM_STORE.matches(url.schema):
diff --git a/juju/client/connection.py b/juju/client/connection.py
index 8daeb58a6..dd6e4b52c 100644
--- a/juju/client/connection.py
+++ b/juju/client/connection.py
@@ -314,14 +314,21 @@ async def connect(
             self.proxy.connect()
 
         _endpoints = [(endpoint, cacert)] if isinstance(endpoint, str) else [(e, cacert) for e in endpoint]
+        lastError = None
         for _ep in _endpoints:
             try:
                 await self._connect_with_redirect([_ep])
                 return self
+            except ssl.SSLError as e:
+                lastError = e
+                continue
             except OSError as e:
                 logging.debug(
                     "Cannot access endpoint {}: {}".format(_ep, e.strerror))
+                lastError = e
                 continue
+        if lastError is not None:
+            raise lastError
         raise Exception("Unable to connect to websocket")
 
     @property
diff --git a/juju/model.py b/juju/model.py
index 5b902724b..549ec892c 100644
--- a/juju/model.py
+++ b/juju/model.py
@@ -20,7 +20,7 @@
 
 from . import provisioner, tag, utils
 from .annotationhelper import _get_annotations, _set_annotations
-from .bundle import BundleHandler, get_charm_series
+from .bundle import BundleHandler, get_charm_series, is_local_charm
 from .charmhub import CharmHub
 from .charmstore import CharmStore
 from .client import client, connector
@@ -1428,6 +1428,11 @@ async def deploy(
         origin = None
         result = None
 
+        # Ensure what we pass in, is a string.
+        entity_url = str(entity_url)
+        if is_local_charm(entity_url) and not entity_url.startswith("local:"):
+            entity_url = "local:{}".format(entity_url)
+        print("!!!", entity_url)
         url = URL.parse(str(entity_url))
         architecture = await self._resolve_architecture(url)
 
@@ -1441,6 +1446,7 @@ async def deploy(
             if not (entity_path.is_dir() or entity_path.is_file()):
                 raise JujuError('{} path not found'.format(entity_url))
 
+            is_local = True
             is_bundle = (
                 (entity_url.endswith(".yaml") and entity_path.exists()) or
                 bundle_path.exists()
@@ -1475,13 +1481,13 @@ async def deploy(
             raise JujuError('unknown charm or bundle {}'.format(entity_url))
 
         if not application_name:
-            if Schema.LOCAL.matches(url.schema) and Schema.CHARM_STORE.matches(url.schema):
-                application_name = result['Meta']['charm-metadata']['Name']
-            else:
+            if Schema.CHARM_HUB.matches(url.schema):
                 # For charmhub charms, we don't have the metadata and we're not
                 # going to get it, so fallback to the url and use that one if a
                 # user didn't specify it.
                 application_name = url.name
+            elif result is not None and not is_bundle:
+                application_name = result['Meta']['charm-metadata']['Name']
 
         if is_bundle:
             handler = BundleHandler(self, trusted=trust, forced=force)
diff --git a/juju/url.py b/juju/url.py
index 42a0dd01c..b2b27d510 100644
--- a/juju/url.py
+++ b/juju/url.py
@@ -41,7 +41,7 @@ def parse(s):
             raise JujuError("charm or bundle URL {} has unrecognized parts".format(u))
 
         if Schema.LOCAL.matches(u.scheme):
-            c = parse_v1_url(Schema.LOCAL, u, s)
+            c = URL(Schema.LOCAL, name=u.path)
         elif Schema.CHARM_STORE.matches(u.scheme):
             c = parse_v1_url(Schema.CHARM_STORE, u, s)
         else:
diff --git a/tests/integration/test_application.py b/tests/integration/test_application.py
index a5c2518de..7b409e6ad 100644
--- a/tests/integration/test_application.py
+++ b/tests/integration/test_application.py
@@ -93,6 +93,26 @@ async def test_add_units(event_loop):
             assert isinstance(unit, Unit)
 
 
+@base.bootstrapped
+@pytest.mark.asyncio
+async def test_deploy_charmstore_charm(event_loop):
+    async with base.CleanModel() as model:
+        app = await model.deploy('cs:ubuntu-0')
+        await model.block_until(lambda: (len(app.units) > 0 and
+                                         app.units[0].machine))
+        assert app.data['charm-url'] == 'cs:ubuntu-0'
+
+
+@base.bootstrapped
+@pytest.mark.asyncio
+async def test_deploy_charmhub_charm(event_loop):
+    async with base.CleanModel() as model:
+        app = await model.deploy('hello-juju')
+        await model.block_until(lambda: (len(app.units) > 0 and
+                                         app.units[0].machine))
+        assert 'hello-juju' in app.data['charm-url'] 
+
+
 @base.bootstrapped
 @pytest.mark.asyncio
 async def test_upgrade_charm(event_loop):
@@ -135,7 +155,7 @@ async def test_upgrade_charm_revision(event_loop):
 @pytest.mark.asyncio
 async def test_upgrade_charm_switch(event_loop):
     async with base.CleanModel() as model:
-        app = await model.deploy('ubuntu-0')
+        app = await model.deploy('cs:ubuntu-0')
         await model.block_until(lambda: (len(app.units) > 0 and
                                          app.units[0].machine))
         assert app.data['charm-url'] == 'cs:ubuntu-0'
diff --git a/tests/integration/test_connection.py b/tests/integration/test_connection.py
index c26b62375..469248093 100644
--- a/tests/integration/test_connection.py
+++ b/tests/integration/test_connection.py
@@ -270,6 +270,8 @@ async def test_verify_controller_cert(event_loop):
 -----END CERTIFICATE-----
 """
 
+    print(endpoint)
+
     try:
         connection = await Connection.connect(
             endpoint=endpoint,
diff --git a/tests/integration/test_model.py b/tests/integration/test_model.py
index 5aa92b17a..24531f7d6 100644
--- a/tests/integration/test_model.py
+++ b/tests/integration/test_model.py
@@ -137,7 +137,7 @@ async def test_wait_local_charm_waiting_timeout(event_loop):
 @pytest.mark.asyncio
 async def test_deploy_bundle(event_loop):
     async with base.CleanModel() as model:
-        await model.deploy('bundle/wiki-simple')
+        await model.deploy('cs:bundle/wiki-simple')
 
         for app in ('wiki', 'mysql'):
             assert app in model.applications
diff --git a/tests/integration/test_unit.py b/tests/integration/test_unit.py
index 0c04f3667..b6daa7119 100644
--- a/tests/integration/test_unit.py
+++ b/tests/integration/test_unit.py
@@ -13,7 +13,7 @@ async def test_run(event_loop):
 
     async with base.CleanModel() as model:
         app = await model.deploy(
-            'ubuntu-0',
+            'cs:ubuntu-0',
             application_name='ubuntu',
             series='trusty',
             channel='stable',

From 3d7af12bcd80f62c3926024827d320a5f0d2f401 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Wed, 14 Apr 2021 17:09:00 +0100
Subject: [PATCH 11/18] Start to run integration tests individually

The integration tests will be better served if they're run individually
so we can try and get the signal to noise ratio lower.
---
 Makefile | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 2884c5656..2711d2527 100644
--- a/Makefile
+++ b/Makefile
@@ -20,8 +20,11 @@ client:
 	$(PY) -m juju.client.facade -s "juju/client/schemas*" -o juju/client/
 
 .PHONY: test
-test:
-	tox
+test: lint
+	tox -e py3
+	@for f in $(shell find tests/integration -maxdepth 1 -mindepth 1 -name "*.py" | grep -v "__init__.py"); do \
+		tox -e integration -- "$${f}"; \
+	done
 
 .PHONY: lint
 lint: 

From 1a2c2a0b83c4c3086a85b985f1ea13048967df72 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Thu, 15 Apr 2021 10:22:16 +0100
Subject: [PATCH 12/18] Unit test fixes

The following updates the bundle unit tests changes fixes. These should
ensure that we're at least compatible with what was previously written.
---
 juju/bundle.py            |  8 ++--
 tests/unit/test_bundle.py | 93 +++++++++++++++++++++++++++++++--------
 tests/unit/test_url.py    |  4 +-
 3 files changed, 82 insertions(+), 23 deletions(-)

diff --git a/juju/bundle.py b/juju/bundle.py
index ec9838a9a..b84fe68f8 100644
--- a/juju/bundle.py
+++ b/juju/bundle.py
@@ -389,7 +389,8 @@ class AddApplicationChange(ChangeInfo):
              'devices': 'devices',
              'endpoint-bindings': 'endpoint_bindings',
              'resources': 'resources',
-             'num-units': 'num_units'}
+             'num-units': 'num_units',
+             'channel': 'channel'}
 
     """AddApplicationChange holds a change for deploying a Juju application.
 
@@ -478,7 +479,7 @@ async def run(self, context):
 
         origin = context.origins.get(str(url), {}).get(str(channel), None)
         if origin is None:
-            raise JujuError("expected origin to be valid for application {} and charm {}".format(self.application, self.charm))
+            raise JujuError("expected origin to be valid for application {} and charm {} with channel {}".format(self.application, str(url), str(channel)))
 
         await context.model._deploy(
             charm_url=charm,
@@ -514,7 +515,8 @@ def __str__(self):
 class AddCharmChange(ChangeInfo):
     _toPy = {'charm': 'charm',
              'series': 'series',
-             'channel': 'channel'}
+             'channel': 'channel',
+             'architecture': 'architecture'}
 
     """AddCharmChange holds a change for adding a charm to the environment.
 
diff --git a/tests/unit/test_bundle.py b/tests/unit/test_bundle.py
index ed8e723ba..bea5f4cfe 100644
--- a/tests/unit/test_bundle.py
+++ b/tests/unit/test_bundle.py
@@ -77,7 +77,8 @@ def test_list_params_juju_2_4(self):
                           "endpoint_bindings": "endpoint_bindings",
                           "resources": "resources",
                           "devices": None,
-                          "num_units": None}, change.__dict__)
+                          "num_units": None,
+                          "channel": None}, change.__dict__)
 
     def test_list_params_juju_2_5(self):
         change = AddApplicationChange(1, [], params=["charm",
@@ -101,7 +102,8 @@ def test_list_params_juju_2_5(self):
                           "endpoint_bindings": "endpoint_bindings",
                           "resources": "resources",
                           "devices": {"gpu": {"type": "gpu", "count": 1, "attributes": {"attr1": "a", "attr2": "b"}}},
-                          "num_units": "num_units"}, change.__dict__)
+                          "num_units": "num_units",
+                          "channel": None}, change.__dict__)
 
     def test_dict_params(self):
         change = AddApplicationChange(1, [], params={"charm": "charm",
@@ -113,7 +115,8 @@ def test_dict_params(self):
                                                      "endpoint-bindings": "endpoint_bindings",
                                                      "resources": "resources",
                                                      "devices": "devices",
-                                                     "num-units": "num_units"})
+                                                     "num-units": "num_units",
+                                                     "channel": "channel"})
         self.assertEqual({"change_id": 1,
                           "requires": [],
                           "charm": "charm",
@@ -125,7 +128,8 @@ def test_dict_params(self):
                           "endpoint_bindings": "endpoint_bindings",
                           "resources": "resources",
                           "devices": "devices",
-                          "num_units": "num_units"}, change.__dict__)
+                          "num_units": "num_units",
+                          "channel": "channel"}, change.__dict__)
 
     def test_dict_params_missing_data(self):
         change = AddApplicationChange(1, [], params={"charm": "charm",
@@ -145,14 +149,15 @@ def test_dict_params_missing_data(self):
                           "endpoint_bindings": None,
                           "resources": None,
                           "devices": None,
-                          "num_units": None}, change.__dict__)
+                          "num_units": None,
+                          "channel": None}, change.__dict__)
 
 
 class TestAddApplicationChangeRun:
 
     @pytest.mark.asyncio
-    async def test_run(self, event_loop):
-        change = AddApplicationChange(1, [], params={"charm": "charm",
+    async def test_run_with_charmstore_charm(self, event_loop):
+        change = AddApplicationChange(1, [], params={"charm": "cs:charm",
                                                      "series": "series",
                                                      "application": "application",
                                                      "options": "options",
@@ -161,14 +166,16 @@ async def test_run(self, event_loop):
                                                      "endpoint-bindings": "endpoint_bindings",
                                                      "resources": "resources",
                                                      "devices": "devices",
-                                                     "num-units": "num_units"})
+                                                     "num-units": "num_units",
+                                                     "channel": "channel"})
 
         model = mock.Mock()
         model._deploy = base.AsyncMock(return_value=None)
         model._add_store_resources = base.AsyncMock(return_value=["resource1"])
 
         context = mock.Mock()
-        context.resolve.return_value = "charm1"
+        context.resolve.return_value = "cs:charm1"
+        context.origins = {"cs:charm1": {"channel/stable": {}}}
         context.trusted = False
         context.model = model
 
@@ -177,11 +184,11 @@ async def test_run(self, event_loop):
 
         model._add_store_resources.assert_called_once()
         model._add_store_resources.assert_called_with("application",
-                                                      "charm1",
+                                                      "cs:charm1",
                                                       overrides="resources")
 
         model._deploy.assert_called_once()
-        model._deploy.assert_called_with(charm_url="charm1",
+        model._deploy.assert_called_with(charm_url="cs:charm1",
                                          application="application",
                                          series="series",
                                          config="options",
@@ -192,6 +199,46 @@ async def test_run(self, event_loop):
                                          devices="devices",
                                          num_units="num_units")
 
+
+    @pytest.mark.asyncio
+    async def test_run_with_charmhub_charm(self, event_loop):
+        change = AddApplicationChange(1, [], params={"charm": "charm",
+                                                     "series": "series",
+                                                     "application": "application",
+                                                     "options": "options",
+                                                     "constraints": "constraints",
+                                                     "storage": "storage",
+                                                     "endpoint-bindings": "endpoint_bindings",
+                                                     "resources": "resources",
+                                                     "devices": "devices",
+                                                     "num-units": "num_units",
+                                                     "channel": "channel"})
+
+        model = mock.Mock()
+        model._deploy = base.AsyncMock(return_value=None)
+        model._add_store_resources = base.AsyncMock(return_value=["resource1"])
+
+        context = mock.Mock()
+        context.resolve.return_value = "ch:charm1"
+        context.origins = {"ch:charm1": {"channel/stable": {}}}
+        context.trusted = False
+        context.model = model
+
+        result = await change.run(context)
+        assert result == "application"
+
+        model._deploy.assert_called_once()
+        model._deploy.assert_called_with(charm_url="ch:charm1",
+                                         application="application",
+                                         series="series",
+                                         config="options",
+                                         constraints="constraints",
+                                         endpoint_bindings="endpoint_bindings",
+                                         resources={},
+                                         storage="storage",
+                                         devices="devices",
+                                         num_units="num_units")
+
     @pytest.mark.asyncio
     async def test_run_local(self, event_loop):
         change = AddApplicationChange(1, [], params={"charm": "local:charm",
@@ -240,7 +287,8 @@ def test_list_params_juju_2_6(self):
                           "requires": [],
                           "charm": "charm",
                           "series": "series",
-                          "channel": None}, change.__dict__)
+                          "channel": None,
+                          "architecture": None}, change.__dict__)
 
     def test_list_params_juju_2_7(self):
         change = AddCharmChange(1, [], params=["charm",
@@ -250,17 +298,20 @@ def test_list_params_juju_2_7(self):
                           "requires": [],
                           "charm": "charm",
                           "series": "series",
-                          "channel": "channel"}, change.__dict__)
+                          "channel": "channel",
+                          "architecture": None}, change.__dict__)
 
     def test_dict_params(self):
         change = AddCharmChange(1, [], params={"charm": "charm",
                                                "series": "series",
-                                               "channel": "channel"})
+                                               "channel": "channel",
+                                               "architecture": "architecture"})
         self.assertEqual({"change_id": 1,
                           "requires": [],
                           "charm": "charm",
                           "series": "series",
-                          "channel": "channel"}, change.__dict__)
+                          "channel": "channel",
+                          "architecture": "architecture"}, change.__dict__)
 
     def test_dict_params_missing_data(self):
         change = AddCharmChange(1, [], params={"charm": "charm",
@@ -269,14 +320,15 @@ def test_dict_params_missing_data(self):
                           "requires": [],
                           "charm": "charm",
                           "series": "series",
-                          "channel": None}, change.__dict__)
+                          "channel": None,
+                          "architecture": None}, change.__dict__)
 
 
 class TestAddCharmChangeRun:
 
     @pytest.mark.asyncio
     async def test_run(self, event_loop):
-        change = AddCharmChange(1, [], params={"charm": "charm",
+        change = AddCharmChange(1, [], params={"charm": "cs:charm",
                                                "series": "series",
                                                "channel": "channel"})
 
@@ -286,15 +338,20 @@ async def test_run(self, event_loop):
         client_facade = mock.Mock()
         client_facade.AddCharm = base.AsyncMock(return_value=None)
 
+        model = mock.Mock()
+        model._add_charm = base.AsyncMock(return_value=None)
+
         context = mock.Mock()
         context.charmstore = charmstore
         context.client_facade = client_facade
+        context.origins = {}
+        context.model = model
 
         result = await change.run(context)
         assert result == "entity_id"
 
         charmstore.entityId.assert_called_once()
-        charmstore.entityId.assert_called_with("charm")
+        charmstore.entityId.assert_called_with("cs:charm")
 
         client_facade.AddCharm.assert_called_once()
         client_facade.AddCharm.assert_called_with(channel=None,
diff --git a/tests/unit/test_url.py b/tests/unit/test_url.py
index bf875fa9a..302a840eb 100644
--- a/tests/unit/test_url.py
+++ b/tests/unit/test_url.py
@@ -32,11 +32,11 @@ def test_parse_v1_series(self):
 class TestURLV2(unittest.TestCase):
     def test_parse_charmhub(self):
         u = URL.parse("ch:arm64/bionic/mysql-1")
-        self.assertEqual(u, URL(Schema.CHARM_HUB, name="mysql", architecture="amd64", series="bionic", revision=1))
+        self.assertEqual(u, URL(Schema.CHARM_HUB, name="mysql", architecture="arm64", series="bionic", revision=1))
 
     def test_parse_charmhub_with_no_series(self):
         u = URL.parse("ch:arm64/mysql")
-        self.assertEqual(u, URL(Schema.CHARM_HUB, name="mysql", architecture="amd64"))
+        self.assertEqual(u, URL(Schema.CHARM_HUB, name="mysql", architecture="arm64"))
 
     def test_parse_charmhub_with_no_series_arch(self):
         u = URL.parse("ch:mysql")

From 0e6444ecf1c1827b4afdb05baa57413deb235e1c Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Thu, 13 May 2021 11:42:31 +0100
Subject: [PATCH 13/18] Use Id from charmstore entity

When supporting older facades we need to get and use the charm Id from
the entity get.

As a driveby I've sorted out the lint issues
---
 examples/deploy_bundle.py             | 2 +-
 juju/bundle.py                        | 2 +-
 juju/model.py                         | 1 -
 juju/version.py                       | 2 +-
 tests/integration/test_application.py | 2 +-
 tests/unit/test_bundle.py             | 1 -
 6 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/examples/deploy_bundle.py b/examples/deploy_bundle.py
index 648fef690..abfb13bfb 100644
--- a/examples/deploy_bundle.py
+++ b/examples/deploy_bundle.py
@@ -16,7 +16,7 @@ async def main():
     await controller.connect()
 
     bundles = [('cs:~juju-qa/bundle/basic-0', 'beta'), ('juju-qa-bundle-test', None)]
-    for i in range(len(bundles)): 
+    for i in range(len(bundles)):
         deployment = bundles[i]
         model = await controller.add_model('model{}'.format(i))
 
diff --git a/juju/bundle.py b/juju/bundle.py
index b84fe68f8..541f21669 100644
--- a/juju/bundle.py
+++ b/juju/bundle.py
@@ -16,7 +16,6 @@
 from .errors import JujuError
 from .origin import Channel
 from .url import Schema, URL
-from .version import LTS_RELEASES
 
 log = logging.getLogger(__name__)
 
@@ -260,6 +259,7 @@ async def _resolve_charms(self):
                 spec['charm'] = str(charm_url)
             else:
                 results = await self.model.charmstore.entity(str(charm_url))
+                charm_url = results.get('Id', charm_url)
                 charm_origin = client.CharmOrigin(source="charm-store",
                                                   risk=risk,
                                                   track=track)
diff --git a/juju/model.py b/juju/model.py
index 549ec892c..7078cb3f5 100644
--- a/juju/model.py
+++ b/juju/model.py
@@ -1432,7 +1432,6 @@ async def deploy(
         entity_url = str(entity_url)
         if is_local_charm(entity_url) and not entity_url.startswith("local:"):
             entity_url = "local:{}".format(entity_url)
-        print("!!!", entity_url)
         url = URL.parse(str(entity_url))
         architecture = await self._resolve_architecture(url)
 
diff --git a/juju/version.py b/juju/version.py
index 793237475..f29839019 100644
--- a/juju/version.py
+++ b/juju/version.py
@@ -1,4 +1,4 @@
 
 LTS_RELEASES = ["focal", "bionic", "xenial", "trusty", "precise"]
 
-DEFAULT_ARCHITECTURE = 'amd64'
\ No newline at end of file
+DEFAULT_ARCHITECTURE = 'amd64'
diff --git a/tests/integration/test_application.py b/tests/integration/test_application.py
index 7b409e6ad..d66638ffa 100644
--- a/tests/integration/test_application.py
+++ b/tests/integration/test_application.py
@@ -110,7 +110,7 @@ async def test_deploy_charmhub_charm(event_loop):
         app = await model.deploy('hello-juju')
         await model.block_until(lambda: (len(app.units) > 0 and
                                          app.units[0].machine))
-        assert 'hello-juju' in app.data['charm-url'] 
+        assert 'hello-juju' in app.data['charm-url']
 
 
 @base.bootstrapped
diff --git a/tests/unit/test_bundle.py b/tests/unit/test_bundle.py
index bea5f4cfe..73a53f1cf 100644
--- a/tests/unit/test_bundle.py
+++ b/tests/unit/test_bundle.py
@@ -199,7 +199,6 @@ async def test_run_with_charmstore_charm(self, event_loop):
                                          devices="devices",
                                          num_units="num_units")
 
-
     @pytest.mark.asyncio
     async def test_run_with_charmhub_charm(self, event_loop):
         change = AddApplicationChange(1, [], params={"charm": "charm",

From 9abe4b65966e553675b924a9b24965da2fdfe032 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Tue, 18 May 2021 12:23:42 +0100
Subject: [PATCH 14/18] Clean up facade error on bundle download

The following removes an indention when checking for a facade. This
makes it a lot easier to read.

Additionally cleaned up the deploy_bundle example.
---
 examples/deploy_bundle.py | 58 +++++++++++++++++++++----------------
 juju/bundle.py            | 60 +++++++++++++++++++--------------------
 2 files changed, 64 insertions(+), 54 deletions(-)

diff --git a/examples/deploy_bundle.py b/examples/deploy_bundle.py
index abfb13bfb..f3f710e1e 100644
--- a/examples/deploy_bundle.py
+++ b/examples/deploy_bundle.py
@@ -15,33 +15,43 @@ async def main():
     # connect to current controller with current user, per Juju CLI
     await controller.connect()
 
-    bundles = [('cs:~juju-qa/bundle/basic-0', 'beta'), ('juju-qa-bundle-test', None)]
-    for i in range(len(bundles)):
-        deployment = bundles[i]
-        model = await controller.add_model('model{}'.format(i))
-
-        try:
-            print('Deploying bundle')
-            applications = await model.deploy(
-                deployment[0],
-                channel=deployment[1],
-            )
-
-            print('Waiting for active')
-            await model.block_until(
-                lambda: all(unit.workload_status == 'active'
-                            for application in applications for unit in application.units))
-            print("Successfully deployed!")
-            print('Removing bundle')
-            for application in applications:
-                await application.remove()
-        finally:
-            print('Disconnecting from model')
-            await model.disconnect()
-            print("Success")
+    # Deploy charmhub bundle
+    await deploy_bundle(controller, 'juju-qa-bundle-test')
+
+    # Deploy legacy bundle
+    await deploy_bundle(controller, 'cs:~juju-qa/bundle/basic-0', 'beta')
 
     await controller.disconnect()
 
 
+async def deploy_bundle(controller, url, channel=None):
+    models = await controller.list_models()
+    model = await controller.add_model('model{}'.format(len(models) + 1))
+
+    try:
+        print('Deploying bundle')
+
+        applications = await deploy_and_wait_for_bundle(model, url, channel)
+
+        print("Successfully deployed!")
+        print('Removing bundle')
+        for application in applications:
+            await application.remove()
+    finally:
+        print('Disconnecting from model')
+        await model.disconnect()
+        print("Success")
+
+
+async def deploy_and_wait_for_bundle(model, url, channel=None):
+    applications = await model.deploy(url, channel=channel)
+
+    print('Waiting for active')
+    await model.block_until(
+        lambda: all(unit.workload_status == 'active'
+                    for application in applications for unit in application.units))
+
+    return applications
+
 if __name__ == '__main__':
     loop.run(main())
diff --git a/juju/bundle.py b/juju/bundle.py
index 541f21669..92530cc81 100644
--- a/juju/bundle.py
+++ b/juju/bundle.py
@@ -178,36 +178,36 @@ async def fetch_plan(self, charm_url, origin):
             raise JujuError(self.plan.errors)
 
     async def _download_bundle(self, charm_url, origin):
-        if self.charms_facade is not None:
-            resp = await self.charms_facade.GetDownloadInfos(entities=[{
-                'charm-url': str(charm_url),
-                'charm-origin': {
-                    'source': origin.source,
-                    'type': origin.type_,
-                    'id': origin.id_,
-                    'hash': origin.hash_,
-                    'revision': origin.revision,
-                    'risk': origin.risk,
-                    'track': origin.track,
-                    'architecture': origin.architecture,
-                    'os': origin.os,
-                    'series': origin.series,
-                }
-            }])
-            if len(resp.results) != 1:
-                raise JujuError("expected one result, received {}".format(resp.results))
-
-            result = resp.results[0]
-            if not result.url:
-                raise JujuError("no url found for bundle {}".format(charm_url.name))
-
-            bundle_resp = requests.get(result.url)
-            bundle_resp.raise_for_status()
-
-            with closing(bundle_resp), zipfile.ZipFile(io.BytesIO(bundle_resp.content)) as archive:
-                return self._get_bundle_yaml(archive)
-
-        raise JujuError('charm facade not supported')
+        if self.charms_facade is None:
+            raise JujuError('unable to download bundle for {} using the new charms facade. Upgrade controller to proceed.'.format(charm_url))
+
+        resp = await self.charms_facade.GetDownloadInfos(entities=[{
+            'charm-url': str(charm_url),
+            'charm-origin': {
+                'source': origin.source,
+                'type': origin.type_,
+                'id': origin.id_,
+                'hash': origin.hash_,
+                'revision': origin.revision,
+                'risk': origin.risk,
+                'track': origin.track,
+                'architecture': origin.architecture,
+                'os': origin.os,
+                'series': origin.series,
+            }
+        }])
+        if len(resp.results) != 1:
+            raise JujuError("expected one result, received {}".format(resp.results))
+
+        result = resp.results[0]
+        if not result.url:
+            raise JujuError("no url found for bundle {}".format(charm_url.name))
+
+        bundle_resp = requests.get(result.url)
+        bundle_resp.raise_for_status()
+
+        with closing(bundle_resp), zipfile.ZipFile(io.BytesIO(bundle_resp.content)) as archive:
+            return self._get_bundle_yaml(archive)
 
     def _get_bundle_yaml(self, archive):
         for member in archive.infolist():

From 0e901f8583dd1f6c15bde1a63f3a0da9f9d90ee5 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Tue, 18 May 2021 14:59:57 +0100
Subject: [PATCH 15/18] Implement deployment types

The following implements deployment types so that it's easier to reason
about the logic of a deployment strategy (charmhub, charmstore and local
charms/bundles).

This is a quite a simple change, but does have a dramatic effect or
readability of the deploy method in model.
---
 juju/model.py | 240 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 156 insertions(+), 84 deletions(-)

diff --git a/juju/model.py b/juju/model.py
index 7078cb3f5..182f2d352 100644
--- a/juju/model.py
+++ b/juju/model.py
@@ -425,6 +425,139 @@ def latest(self):
         return self.model.state.get_entity(self.entity_type, self.entity_id)
 
 
+class DeployTypeResult:
+    """DeployTypeResult represents the result of a deployment type after a
+    resolution.
+    """
+
+    def __init__(self, identifier, origin, app_name, is_local=False, is_bundle=False):
+        self.identifier = identifier
+        self.origin = origin
+        self.app_name = app_name
+        self.is_local = is_local
+        self.is_bundle = is_bundle
+
+
+class LocalDeployType:
+    """LocalDeployType deals with local only deployments.
+    """
+
+    async def resolve(self, url, architecture, app_name=None, channel=None, series=None, entity_url=None):
+        """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.
+        """
+
+        entity_url = url.path()
+        entity_path = Path(entity_url)
+        bundle_path = entity_path / 'bundle.yaml'
+
+        identifier = entity_url
+        origin = client.CharmOrigin(source="local", architecture=architecture)
+        if not (entity_path.is_dir() or entity_path.is_file()):
+            raise JujuError('{} path not found'.format(entity_url))
+
+        is_bundle = (
+            (entity_url.endswith(".yaml") and entity_path.exists()) or
+            bundle_path.exists()
+        )
+
+        if app_name is None:
+            app_name = url.name
+
+            if not is_bundle:
+                entity_url = url.path()
+                entity_path = Path(entity_url)
+                if str(entity_path).endswith('.charm'):
+                    with zipfile.ZipFile(entity_path, 'r') as charm_file:
+                        metadata = yaml.load(charm_file.read('metadata.yaml'), Loader=yaml.FullLoader)
+                else:
+                    metadata_path = entity_path / 'metadata.yaml'
+                    metadata = yaml.load(metadata_path.read_text(), Loader=yaml.FullLoader)
+                app_name = metadata['name']
+
+        return DeployTypeResult(
+            identifier=identifier,
+            origin=origin,
+            app_name=app_name,
+            is_local=True,
+            is_bundle=is_bundle,
+        )
+
+
+class CharmStoreDeployType:
+    """CharmStoreDeployType defines a class for resolving and deploying charm
+    store charms and bundle.
+    """
+
+    def __init__(self, charmstore, get_series):
+        self.charmstore = charmstore
+        self.get_series = get_series
+
+    async def resolve(self, url, architecture, app_name=None, channel=None, series=None, entity_url=None):
+        """resolve attempts to resolve charmstore charms or bundles. A request
+        to the charmstore is required to get more information about the
+        underlying identifier.
+        """
+
+        result = await self.charmstore.entity(str(url),
+                                              channel=channel,
+                                              include_stats=False)
+        identifier = result['Id']
+        is_bundle = url.series == "bundle"
+        if not series:
+            series = self.get_series(entity_url, result)
+
+        if app_name is None and not is_bundle:
+            app_name = result['Meta']['charm-metadata']['Name']
+
+        origin = client.CharmOrigin(source="charm-store",
+                                    architecture=architecture,
+                                    risk=channel,
+                                    series=series)
+
+        return DeployTypeResult(
+            identifier=identifier,
+            app_name=app_name,
+            origin=origin,
+            is_bundle=is_bundle,
+        )
+
+
+class CharmhubDeployType:
+    """CharmhubDeployType defines a class for resolving and deploying charmhub
+    charms and bundles.
+    """
+
+    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):
+        """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.
+        """
+
+        ch = Channel('latest', 'stable')
+        if channel is not None:
+            ch = Channel.parse(channel).normalize()
+        origin = client.CharmOrigin(source="charm-hub",
+                                    architecture=architecture,
+                                    risk=ch.risk,
+                                    track=ch.track)
+        charm_url, origin = await self.charm_resolver(url, origin)
+
+        if app_name is None:
+            app_name = url.name
+
+        return DeployTypeResult(
+            identifier=charm_url,
+            app_name=app_name,
+            origin=origin,
+            is_bundle=origin.type_ == "bundle",
+        )
+
+
 class Model:
     """
     The main API for interacting with a Juju model.
@@ -468,6 +601,12 @@ def __init__(
         self._charmhub = CharmHub(self)
         self._charmstore = CharmStore(self._connector.loop)
 
+        self.deploy_types = {
+            "local": LocalDeployType(),
+            "cs": CharmStoreDeployType(self._charmstore, self._get_series),
+            "ch": CharmhubDeployType(self._resolve_charm),
+        }
+
     def is_connected(self):
         """Reports whether the Model is currently connected."""
         return self._connector.is_connected()
@@ -1419,15 +1558,6 @@ async def deploy(
         if trust and (self.info.agent_version < client.Number.from_json('2.4.0')):
             raise NotImplementedError("trusted is not supported on model version {}".format(self.info.agent_version))
 
-        # Attempt to resolve a charm or bundle based on the URL.
-        # In an ideal world this should be moved to the controller, and we
-        # wouldn't have to deal with this at all.
-        is_local = False
-        is_bundle = False
-        identifier = None
-        origin = None
-        result = None
-
         # Ensure what we pass in, is a string.
         entity_url = str(entity_url)
         if is_local_charm(entity_url) and not entity_url.startswith("local:"):
@@ -1435,62 +1565,18 @@ async def deploy(
         url = URL.parse(str(entity_url))
         architecture = await self._resolve_architecture(url)
 
-        if Schema.LOCAL.matches(url.schema):
-            entity_url = url.path()
-            entity_path = Path(entity_url)
-            bundle_path = entity_path / 'bundle.yaml'
-
-            identifier = entity_url
-            origin = client.CharmOrigin(source="local", architecture=architecture)
-            if not (entity_path.is_dir() or entity_path.is_file()):
-                raise JujuError('{} path not found'.format(entity_url))
-
-            is_local = True
-            is_bundle = (
-                (entity_url.endswith(".yaml") and entity_path.exists()) or
-                bundle_path.exists()
-            )
-
-        elif Schema.CHARM_STORE.matches(url.schema):
-            result = await self.charmstore.entity(str(url),
-                                                  channel=channel,
-                                                  include_stats=False)
-            identifier = result['Id']
-            origin = client.CharmOrigin(source="charm-store",
-                                        architecture=architecture,
-                                        risk=channel)
-            is_bundle = url.series == "bundle"
-            if not series:
-                series = self._get_series(entity_url, result)
-
-        elif Schema.CHARM_HUB.matches(url.schema):
-            ch = Channel('latest', 'stable')
-            if channel:
-                ch = Channel.parse(channel).normalize()
-            origin = client.CharmOrigin(source="charm-hub",
-                                        architecture=architecture,
-                                        risk=ch.risk,
-                                        track=ch.track)
-            charm_url, origin = await self._resolve_charm(url, origin)
-
-            identifier = charm_url
-            is_bundle = origin.type_ == "bundle"
-
-        if identifier is None:
+        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)
+        
+        if res.identifier is None:
             raise JujuError('unknown charm or bundle {}'.format(entity_url))
+        identifier = res.identifier
 
-        if not application_name:
-            if Schema.CHARM_HUB.matches(url.schema):
-                # For charmhub charms, we don't have the metadata and we're not
-                # going to get it, so fallback to the url and use that one if a
-                # user didn't specify it.
-                application_name = url.name
-            elif result is not None and not is_bundle:
-                application_name = result['Meta']['charm-metadata']['Name']
-
-        if is_bundle:
+        series = res.origin.series or series
+        if res.is_bundle:
             handler = BundleHandler(self, trusted=trust, forced=force)
-            await handler.fetch_plan(url, origin)
+            await handler.fetch_plan(url, res.origin)
             await handler.execute_plan()
             extant_apps = {app for app in self.applications}
             pending_apps = set(handler.applications) - extant_apps
@@ -1508,27 +1594,15 @@ async def deploy(
         else:
             # XXX: we're dropping local resources here, but we don't
             # actually support them yet anyway
-            if not is_local:
-                await self._add_charm(identifier, origin)
+            if not res.is_local:
+                await self._add_charm(identifier, res.origin)
 
                 # TODO (stickupkid): Handle charmhub charms, for now we'll only
                 # handle charmstore charms.
                 if Schema.CHARM_STORE.matches(url.schema):
-                    resources = await self._add_store_resources(application_name,
-                                                                identifier,
-                                                                entity=result)
+                    resources = await self._add_store_resources(res.app_name,
+                                                                identifier)
             else:
-                if not application_name:
-                    entity_url = url.path()
-                    entity_path = Path(entity_url)
-                    if str(entity_path).endswith('.charm'):
-                        with zipfile.ZipFile(entity_path, 'r') as charm_file:
-                            metadata = yaml.load(charm_file.read('metadata.yaml'), Loader=yaml.FullLoader)
-                    else:
-                        metadata_path = entity_path / 'metadata.yaml'
-                        metadata = yaml.load(metadata_path.read_text(), Loader=yaml.FullLoader)
-                    application_name = metadata['name']
-
                 # We have a local charm dir that needs to be uploaded
                 charm_dir = os.path.abspath(
                     os.path.expanduser(identifier))
@@ -1547,7 +1621,7 @@ async def deploy(
 
             return await self._deploy(
                 charm_url=identifier,
-                application=application_name,
+                application=res.app_name,
                 series=series,
                 config=config,
                 constraints=constraints,
@@ -1611,11 +1685,9 @@ async def _resolve_architecture(self, url):
         return DEFAULT_ARCHITECTURE
 
     async def _add_store_resources(self, application, entity_url,
-                                   overrides=None, entity=None):
-        if not entity:
-            # avoid extra charm store call if one was already made
-            entity = await self.charmstore.entity(entity_url,
-                                                  include_stats=False)
+                                   overrides=None):
+        entity = await self.charmstore.entity(entity_url,
+                                              include_stats=False)
         resources = [
             {
                 'description': resource['Description'],

From 749f140be00dcfb48588cd5d2cec8e98d1f05433 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Tue, 18 May 2021 15:05:10 +0100
Subject: [PATCH 16/18] Clean up the code for adding charm

The readability of some of the if statements was slightly hard to
comprehend because of the if, elif statements.
---
 juju/bundle.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/juju/bundle.py b/juju/bundle.py
index 92530cc81..5eee38959 100644
--- a/juju/bundle.py
+++ b/juju/bundle.py
@@ -576,14 +576,14 @@ async def run(self, context):
             context.origins[self.charm] = {str(None): origin}
             return self.charm
 
-        elif Schema.CHARM_STORE.matches(url.schema):
+        if Schema.CHARM_STORE.matches(url.schema):
             entity_id = await context.charmstore.entityId(self.charm)
             log.debug('Adding %s', entity_id)
             await context.client_facade.AddCharm(channel=None, url=entity_id, force=False)
             identifier = entity_id
             origin = client.CharmOrigin(source="charm-store", risk="stable")
 
-        elif Schema.CHARM_HUB.matches(url.schema):
+        if Schema.CHARM_HUB.matches(url.schema):
             ch = Channel('latest', 'stable')
             if self.channel:
                 ch = Channel.parse(self.channel).normalize()

From 84c0202f76b7241fbe6ad264eb74a977a86babd0 Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Tue, 18 May 2021 15:57:44 +0100
Subject: [PATCH 17/18] Remove the Makefile hack

Just make xdist work better with the libraries.
---
 Makefile | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 2711d2527..ea673dda5 100644
--- a/Makefile
+++ b/Makefile
@@ -22,9 +22,8 @@ client:
 .PHONY: test
 test: lint
 	tox -e py3
-	@for f in $(shell find tests/integration -maxdepth 1 -mindepth 1 -name "*.py" | grep -v "__init__.py"); do \
-		tox -e integration -- "$${f}"; \
-	done
+	tox -e integration
+
 
 .PHONY: lint
 lint: 

From 98bef78f1f0dc42139589e75e6085e301e06703f Mon Sep 17 00:00:00 2001
From: Simon Richardson <stickupkid@gmail.com>
Date: Tue, 18 May 2021 16:12:43 +0100
Subject: [PATCH 18/18] Remove the debug print for github actions

---
 juju/model.py                        | 2 +-
 tests/integration/test_connection.py | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/juju/model.py b/juju/model.py
index 182f2d352..4cebaa132 100644
--- a/juju/model.py
+++ b/juju/model.py
@@ -1568,7 +1568,7 @@ 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)
-        
+
         if res.identifier is None:
             raise JujuError('unknown charm or bundle {}'.format(entity_url))
         identifier = res.identifier
diff --git a/tests/integration/test_connection.py b/tests/integration/test_connection.py
index 469248093..c26b62375 100644
--- a/tests/integration/test_connection.py
+++ b/tests/integration/test_connection.py
@@ -270,8 +270,6 @@ async def test_verify_controller_cert(event_loop):
 -----END CERTIFICATE-----
 """
 
-    print(endpoint)
-
     try:
         connection = await Connection.connect(
             endpoint=endpoint,