Merge pull request #749 from cderici/fix-series-for-local-charms-and-bundles-3.0

#749

#### Description

This change fixes the use of `base` in `CharmOrigin` in basically all code paths for local and charmhub deployments for both charms and bundles.

Initially started to fix the charm deploy issues such as in [parca-k8s](https://github.com/jnsgruk/parca-k8s-operator/actions/runs/3277144022/jobs/5394048300#step:4:1), this grew out of it pretty quickly, as I started to run tests against the 3.0 controller with the new facades/clients.

This should also fix the [LP #1992833](https://bugs.launchpad.net/juju/+bug/1992833), though we need a confirmation on that.

#### QA Steps

To qa this we need a `3.0` controller.

So for the local charm deployment issue, run `juju download parca-k8s --channel=edge` and then run:

```python
application = await model.deploy(
 './parca.charm',
 )
```

with libjuju and it should work.

For the bundle parts, I manually made sure that all of the following integration tests are working against the latest juju `3.0`. Note that in the CI below we won't see any of this because juju latest/stable is still 2.9 (i.e. almost all our tests are running against 2.9) and our latest-edge jenkins job (against 3.0) is still not working.

Here are the individual tests that need to pass (they were all failing without this change):

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

tox -e integration -- tests/integration/test_model.py::test_deploy_bundle

tox -e integration -- tests/integration/test_model.py::test_deploy_local_bundle_file

tox -e integration -- tests/integration/test_model.py::test_deploy_local_bundle_include_file

tox -e integration -- tests/integration/test_model.py::test_deploy_bundle_local_charms

tox -e integration -- tests/integration/test_model.py::test_deploy_bundle_with_multiple_overlays_with_include_files

tox -e integration -- tests/integration/test_model.py::test_deploy_local_bundle_include_base64

tox -e integration -- tests/integration/test_model.py::test_deploy_trusted_bundle

tox -e integration -- tests/integration/test_model.py::test_deploy_local_bundle_with_overlay_multi

tox -e integration -- tests/integration/test_model.py::test_deploy_bundle_local_resource_relative_path

tox -e integration -- tests/integration/test_model.py::test_deploy_local_bundle_dir

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

#### Notes & Discussion

Note that I rely on the CI tests to make sure we didn't regress on the 2.9 support. So we should be careful in looking at the CI to make sure none of the related tests are failing. Failure means that a change I made to support 3.0 broke 2.9 support. In that case we gotta fix it before we land this.

Author: jujubot

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 . import utils, jasyncio, charmhub
+from . import utils, jasyncio
 from .origin import Channel
 from .url import Schema, URL
 
@@ -129,7 +129,7 @@ async def _handle_local_charms(self, bundle, bundle_dir):
                 default_series or
                 await get_charm_series(charm_dir, self.model)
             )
-            if not series:
+            if not self.model.connection().is_using_old_client and not series:
                 raise JujuError(
                     "Couldn't determine series for charm at {}. "
                     "Add a 'series' key to the bundle.".format(charm_dir))
@@ -148,15 +148,23 @@ async def _handle_local_charms(self, bundle, bundle_dir):
             ])
 
             # Update the 'charm:' entry for each app with the new 'local:' url.
-            for app_name, charm_url, (charm_dir, _) in zip(apps, charm_urls, args):
+            for app_name, charm_url, (charm_dir, series) in zip(apps, charm_urls, args):
+                metadata = utils.get_local_charm_metadata(charm_dir)
                 resources = await self.model.add_local_resources(
                     app_name,
                     charm_url,
-                    utils.get_local_charm_metadata(charm_dir),
+                    metadata,
                     resources=bundle.get('applications', {app_name: {}})[app_name].get("resources", {}),
                 )
                 apps_dict[app_name]['charm'] = charm_url
                 apps_dict[app_name]["resources"] = resources
+                origin = client.CharmOrigin(source="local", risk="stable")
+                if not self.model.connection().is_using_old_client:
+                    origin.base = utils.get_local_charm_base(series, '',
+                                                             metadata,
+                                                             charm_dir,
+                                                             client.Base)
+                self.origins[charm_url] = {str(None): origin}
 
         return bundle
 
@@ -281,20 +289,27 @@ async def _download_bundle(self, charm_url, origin):
         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))
 
+        id = origin.id_ if origin.id_ else ''
+        hash = origin.hash_ if origin.hash_ else ''
+        charm_origin = {
+            'source': origin.source,
+            'type': origin.type_,
+            'id': id,
+            'hash': hash,
+            'revision': origin.revision,
+            'risk': origin.risk,
+            'track': origin.track,
+            'architecture': origin.architecture,
+        }
+        if self.model.connection().is_using_old_client:
+            charm_origin['os'] = origin.os
+            charm_origin['series'] = origin.series
+        else:
+            charm_origin['base'] = origin.base
+
         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,
-            }
+            'charm-origin': charm_origin
         }])
         if len(resp.results) != 1:
             raise JujuError("expected one result, received {}".format(resp.results))
@@ -341,10 +356,14 @@ async def _resolve_charms(self):
 
             charm_url = URL.parse(spec['charm'])
             channel = None
+            series = spec.get('series', None)
             track, risk = '', ''
             if 'channel' in spec:
                 channel = Channel.parse(spec['channel'])
                 track, risk = channel.track, channel.risk
+
+                # if not track and series:
+                #     track = utils.get_series_version(series)
             if self.charms_facade is not None:
                 if cons is not None and cons['arch'] != '':
                     architecture = cons['arch']
@@ -355,8 +374,10 @@ async def _resolve_charms(self):
                                             architecture=architecture,
                                             risk=risk,
                                             track=track)
+                if not self.model.connection().is_using_old_client and series:
+                    origin.base = client.Base(
+                        channel=utils.get_series_version(series), name='ubuntu')
                 charm_url, charm_origin, _ = await self.model._resolve_charm(charm_url, origin)
-
                 spec['charm'] = str(charm_url)
             else:
                 results = await self.model.charmstore.entity(str(charm_url))
@@ -618,9 +639,6 @@ async def run(self, context):
             resources = await context.model._add_store_resources(
                 self.application, charm, overrides=self.resources)
         elif Schema.CHARM_HUB.matches(url.schema):
-            c_hub = charmhub.CharmHub(context.model)
-            id_, _ = await c_hub.get_charm_id(url.name)
-            origin.id_ = id_
             resources = await context.model._add_charmhub_resources(
                 self.application, charm, origin, overrides=self.resources)
         else:
@@ -719,8 +737,6 @@ 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
 
         if Schema.CHARM_STORE.matches(url.schema):
@@ -741,6 +757,10 @@ async def run(self, context):
                                         architecture=arch,
                                         risk=ch.risk,
                                         track=ch.track)
+            if not context.model.connection().is_using_old_client and self.series:
+                origin.base = client.Base(
+                    channel=utils.get_series_version(self.series),
+                    name='ubuntu')
             identifier, origin, _ = await context.model._resolve_charm(url, origin)
 
         if identifier is None:
@@ -826,7 +846,12 @@ async def run(self, context):
 
         params['constraints'] = parse_constraints(self.constraints)
         params['jobs'] = params.get('jobs', ['JobHostUnits'])
-        params['series'] = self.series
+        if not context.model.connection().is_using_old_client:
+            params['base'] = client.Base(
+                channel=utils.get_series_version(self.series),
+                name='ubuntu')
+        else:
+            params['series'] = self.series
 
         if self.container_type == 'lxc':
             log.warning('Juju 2.0 does not support lxc containers. '

juju/model.py

@@ -824,7 +824,7 @@ async def add_local_charm_dir(self, charm_dir, series):
         log.debug('Uploaded local charm: %s -> %s', charm_dir, charm_url)
         return charm_url
 
-    def add_local_charm(self, charm_file, series, size=None):
+    def add_local_charm(self, charm_file, series="", size=None):
         """Upload a local charm archive to the model.
 
         Returns the 'local:...' url that should be used to deploy the charm.
@@ -1770,7 +1770,6 @@ async def deploy(
                     charm_origin = add_charm_res.get('charm_origin', res.origin)
                 else:
                     charm_origin = add_charm_res.charm_origin
-
                 if Schema.CHARM_HUB.matches(url.schema):
                     resources = await self._add_charmhub_resources(res.app_name,
                                                                    identifier,
@@ -1789,16 +1788,24 @@ async def deploy(
                 charm_dir = os.path.abspath(
                     os.path.expanduser(identifier))
                 charm_origin = res.origin
+
                 metadata = utils.get_local_charm_metadata(charm_dir)
+                # TODO (cderici) : pass the metadata into get_charm_series, as
+                #  it also reads that file redundantly
+                series = series or await get_charm_series(charm_dir, self)
+
+                # If we're using a newer client, then the CharmOrigin needs a
+                # base
+                if not self.connection().is_using_old_client:
+                    charm_origin.base = utils.get_local_charm_base(series,
+                                                                   channel,
+                                                                   metadata,
+                                                                   charm_dir,
+                                                                   client.Base)
+
                 if not application_name:
                     application_name = metadata['name']
-                series = series or await get_charm_series(charm_dir, self)
-                if not series:
-                    model_config = await self.get_config()
-                    default_series = model_config.get("default-series")
-                    if default_series:
-                        series = default_series.value
-                if not series:
+                if self.connection().is_using_old_client and not series:
                     raise JujuError(
                         "Couldn't determine series for charm at {}. "
                         "Pass a 'series' kwarg to Model.deploy().".format(
@@ -1853,14 +1860,19 @@ async def _resolve_charm(self, url, origin):
             source = "charm-store"
         else:
             source = "charm-hub"
+
+        resolve_origin = {
+            'source': source,
+            'architecture': origin.architecture,
+            'track': origin.track,
+            'risk': origin.risk,
+        }
+
+        if not self.connection().is_using_old_client:
+            resolve_origin['base'] = origin.base
         resp = await charms_facade.ResolveCharms(resolve=[{
             'reference': str(url),
-            'charm-origin': {
-                'source': source,
-                'architecture': origin.architecture,
-                'track': origin.track,
-                'risk': origin.risk,
-            }
+            'charm-origin': resolve_origin
         }])
         if len(resp.results) != 1:
             raise JujuError("expected one result, received {}".format(resp.results))

juju/utils.py

@@ -9,7 +9,7 @@
 import yaml
 import zipfile
 
-from . import jasyncio
+from . import jasyncio, origin, errors
 
 
 async def execute_process(*cmd, log=None):
@@ -234,19 +234,151 @@ def generate_user_controller_access_token(username, controller_endpoints, secret
     return base64.urlsafe_b64encode(registration_string)
 
 
-def get_local_charm_metadata(path):
+def get_local_charm_data(path, yaml_file):
     """Retrieve Metadata of a Charm from its path
 
     :patam str path: Path of charm directory or .charm file
+    :patam str yaml_file: name of the yaml file, can be either
+    "metadata.yaml", or "manifest.yaml"
 
     :return: Object of charm metadata
     """
     if str(path).endswith('.charm'):
         with zipfile.ZipFile(str(path), 'r') as charm_file:
-            metadata = yaml.load(charm_file.read('metadata.yaml'), Loader=yaml.FullLoader)
+            metadata = yaml.load(charm_file.read(yaml_file), Loader=yaml.FullLoader)
     else:
         entity_path = Path(path)
-        metadata_path = entity_path / 'metadata.yaml'
+        metadata_path = entity_path / yaml_file
         metadata = yaml.load(metadata_path.read_text(), Loader=yaml.FullLoader)
 
     return metadata
+
+
+def get_local_charm_metadata(path):
+    return get_local_charm_data(path, 'metadata.yaml')
+
+
+def get_local_charm_manifest(path):
+    return get_local_charm_data(path, 'manifest.yaml')
+
+
+PRECISE = "precise"
+QUANTAL = "quantal"
+RARING = "raring"
+SAUCY = "saucy"
+TRUSTY = "trusty"
+UTOPIC = "utopic"
+VIVID = "vivid"
+WILY = "wily"
+XENIAL = "xenial"
+YAKKETY = "yakkety"
+ZESTY = "zesty"
+ARTFUL = "artful"
+BIONIC = "bionic"
+COSMIC = "cosmic"
+DISCO = "disco"
+EOAN = "eoan"
+FOCAL = "focal"
+GROOVY = "groovy"
+HIRSUTE = "hirsute"
+IMPISH = "impish"
+JAMMY = "jammy"
+KINETIC = "kinetic"
+
+UBUNTU_SERIES = {
+    PRECISE: "12.04",
+    QUANTAL: "12.10",
+    RARING: "13.04",
+    SAUCY: "13.10",
+    TRUSTY: "14.04",
+    UTOPIC: "14.10",
+    VIVID: "15.04",
+    WILY: "15.10",
+    XENIAL: "16.04",
+    YAKKETY: "16.10",
+    ZESTY: "17.04",
+    ARTFUL: "17.10",
+    BIONIC: "18.04",
+    COSMIC: "18.10",
+    DISCO: "19.04",
+    EOAN: "19.10",
+    FOCAL: "20.04",
+    GROOVY: "20.10",
+    HIRSUTE: "21.04",
+    IMPISH: "21.10",
+    JAMMY: "22.04",
+    KINETIC: "22.10",
+}
+
+
+def get_series_version(series_name):
+    if series_name not in UBUNTU_SERIES:
+        raise errors.JujuError("Unknown series : %s", series_name)
+    return UBUNTU_SERIES[series_name]
+
+
+def get_version_series(version):
+    if version not in UBUNTU_SERIES.values():
+        raise errors.JujuError("Unknown version : %s", 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):
+    """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:
+    :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 == '':
+        channel_for_base = get_series_version(series) if series else ''
+        if channel_for_base:
+            # we currently only support ubuntu series (statically)
+            # TODO (cderici) : go juju/core/series/supported.go and get the
+            #  others here too
+            os_name_for_base = 'ubuntu'
+
+    # Check the charm manifest
+    if channel_for_base == '':
+        charm_manifest = get_local_charm_manifest(charm_path)
+        if 'bases' in charm_manifest:
+            channel_for_base = charm_manifest['bases'][0]['channel']
+            os_name_for_base = charm_manifest['bases'][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)