Merge pull request #846 from juju/fix-local-charm-base-channel-discovery

#846

#### Description

`utils.get_local_charm_base()` was incorrectly using the `--channel` argument (the charm's channel) for discovering the channel part of the base. (we should stop using the word 'channel' for two different things).

This fixes that by taking out the incorrect part of the code.

Should fix #839


#### QA Steps

So a trivial way to reproduce the #839 is to deploy a local charm with a `--channel='stable'` argument. You may try to use one of the examples to validate this. Additionally an integration test is added (see below), so passing that should be enough. I also suggest getting a confirmation from [@gcalvinos](https://github.com/gcalvinos), just in case.

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

All CI tests need to pass. We might have a couple of time-outs from previously known CI test failures.

Author: jujubot

juju/model.py

@@ -1682,13 +1682,11 @@ async def deploy(
                 # We have a local charm dir that needs to be uploaded
                 charm_dir = os.path.abspath(os.path.expanduser(identifier))
                 charm_origin = res.origin
-                base = None
 
                 metadata = utils.get_local_charm_metadata(charm_dir)
                 charm_series = charm_series or await get_charm_series(metadata,
                                                                       self)
-                base = utils.get_local_charm_base(
-                    charm_series, channel, metadata, charm_dir, client.Base)
+                base = utils.get_local_charm_base(charm_series, charm_dir, client.Base)
                 charm_origin.base = base
                 if not application_name:
                     application_name = metadata['name']

juju/utils.py

@@ -329,46 +329,21 @@ def get_version_series(version):
     return list(UBUNTU_SERIES.keys())[list(UBUNTU_SERIES.values()).index(version)]
 
 
-def get_local_charm_base(series, channel_from_arg, charm_metadata,
-                         charm_path, baseCls):
+def get_local_charm_base(series, charm_path, base_class):
     """Deduce the base [channel/osname] of a local charm based on what we
     know already
 
     :param str series: This may come from the argument or the metadata.yaml
-    :param str channel_from_arg: This is channel passed as argument, if any.
-    :param dict charm_metadata: metadata.yaml
     :param str charm_path: Path of charm directory/.charm file
-    :param class baseCls:
+    :param class base_class:
     :return: Instance of the baseCls with channel/osname informaiton
     """
 
     channel_for_base = ''
     os_name_for_base = ''
-    # If user passed a channel_arg, then check the supported series against
-    # the channel's track
-    chnl_check = origin.Channel.parse(channel_from_arg) if channel_from_arg \
-        else None
-    if chnl_check:
-        not_supported_error = errors.JujuError(
-            "Given channel [track/risk] is not supported --"
-            "\n - Given channel : %s"
-            "\n - Series in Charm Metadata : %s" %
-            (channel_from_arg, charm_metadata['series']))
-        channel_for_base = chnl_check.track
-        intented_series = get_version_series(channel_for_base)
-        if intented_series not in charm_metadata['series']:
-            raise not_supported_error
-        # Also check the manifest if there's one
-        charm_manifest = get_local_charm_manifest(charm_path)
-        if 'bases' in charm_manifest:
-            for base in charm_manifest['bases']:
-                if channel_for_base == base['channel']:
-                    break
-            else:
-                raise not_supported_error
 
-    # If we know the series, use it to get a channel
-    if channel_for_base == '':
+    # We should know the series, so use it to get a channel
+    if series:
         channel_for_base = get_series_version(series) if series else ''
         if channel_for_base:
             # we currently only support ubuntu series (statically)
@@ -382,18 +357,19 @@ def get_local_charm_base(series, channel_from_arg, charm_metadata,
         if 'bases' in charm_manifest:
             channel_for_base = charm_manifest['bases'][0]['channel']
             os_name_for_base = charm_manifest['bases'][0]['name']
-        else:
-            # Also check the charmcraft.yaml
-            charmcraft_yaml = get_local_charm_charmcraft_yaml(charm_path)
-            if 'bases' in charmcraft_yaml:
-                channel_for_base = charmcraft_yaml['bases'][0]['run-on'][0]['channel']
-                os_name_for_base = charmcraft_yaml['bases'][0]['run-on'][0]['name']
+
+    # Also check the charmcraft.yaml
+    if channel_for_base == '':
+        charmcraft_yaml = get_local_charm_charmcraft_yaml(charm_path)
+        if 'bases' in charmcraft_yaml:
+            channel_for_base = charmcraft_yaml['bases'][0]['run-on'][0]['channel']
+            os_name_for_base = charmcraft_yaml['bases'][0]['run-on'][0]['name']
 
     if channel_for_base == '':
         raise errors.JujuError("Unable to determine base for charm : %s" %
                                charm_path)
 
-    return baseCls(channel_for_base, os_name_for_base)
+    return base_class(channel_for_base, os_name_for_base)
 
 
 def base_channel_to_series(channel):

tests/integration/test_charmhub.py

@@ -103,5 +103,5 @@ async def test_subordinate_charm_zero_units(event_loop):
 @pytest.mark.asyncio
 async def test_list_resources(event_loop):
     async with base.CleanModel() as model:
-        resources = await model.charmhub.list_resources('postgresql')
+        resources = await model.charmhub.list_resources('hello-kubecon')
         assert type(resources) == list and len(resources) > 0

tests/integration/test_model.py

@@ -149,6 +149,18 @@ async def test_deploy_local_charm(event_loop):
         assert model.units['charm/0'].workload_status == 'active'
 
 
+@base.bootstrapped
+@pytest.mark.asyncio
+async def test_deploy_local_charm_channel(event_loop):
+    charm_path = TESTS_DIR / 'charm'
+
+    async with base.CleanModel() as model:
+        await model.deploy(str(charm_path), channel='stable')
+        assert 'charm' in model.applications
+        await model.wait_for_idle(status="active")
+        assert model.units['charm/0'].workload_status == 'active'
+
+
 @base.bootstrapped
 @pytest.mark.asyncio
 async def test_wait_local_charm_blocked(event_loop):
@@ -181,9 +193,10 @@ 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('canonical-livepatch-onprem', channel='edge', trust=True)
+        await model.deploy('anbox-cloud-core', channel='stable',
+                           trust=True)
 
-        for app in ('haproxy', 'livepatch', 'postgresql', 'ubuntu-advantage'):
+        for app in ('ams', 'etcd', 'ams-node-controller', 'etcd-ca', 'lxd'):
             assert app in model.applications
 
 
@@ -269,10 +282,11 @@ async def test_deploy_local_charm_folder_symlink(event_loop):
 @base.bootstrapped
 @pytest.mark.asyncio
 async def test_deploy_trusted_bundle(event_loop):
+    pytest.skip("skip until we have a deployable bundle available. Right now the landscape-dense fails because postgresql is broken")
     async with base.CleanModel() as model:
-        await model.deploy('canonical-livepatch-onprem', channel='stable', trust=True)
+        await model.deploy('landscape-dense', channel='stable', trust=True)
 
-        for app in ('haproxy', 'livepatch', 'postgresql', 'ubuntu-advantage'):
+        for app in ('haproxy', 'landscape-server', 'postgresql', 'rabbit-mq-server'):
             assert app in model.applications
 
         haproxy_app = model.applications['haproxy']