Merge pull request #777 from cderici/series-or-base-for-local-charms

[JUJU-2276] Series or base for local charms

Author: juanmanuel-tirado

juju/application.py

@@ -823,9 +823,12 @@ async def local_refresh(
 
         series = (
             await self.get_series() or
-            self.model.info.get('default-series', '') or
-            await get_charm_series(charm_dir, self.model)
+            self.model.info.get('default-series', '')
         )
+        if not series:
+            metadata = utils.get_local_charm_metadata(charm_dir)
+            await get_charm_series(metadata, self.model)
+
         if not series:
             default_series = model_config.get("default-series")
             if default_series:

juju/bundle.py

@@ -124,11 +124,10 @@ async def _handle_local_charms(self, bundle, bundle_dir):
                 pass
             except FileNotFoundError:
                 continue
-            series = (
-                app_dict.get('series') or
-                default_series or
-                await get_charm_series(charm_dir, self.model)
-            )
+            series = (app_dict.get('series') or default_series)
+            if not series:
+                metadata = utils.get_local_charm_metadata(charm_dir)
+                series = await get_charm_series(metadata, self.model)
             if not self.model.connection().is_using_old_client and not series:
                 raise JujuError(
                     "Couldn't determine series for charm at {}. "
@@ -430,32 +429,17 @@ def is_local_charm(charm_url):
     return charm_url.startswith('.') or charm_url.startswith('local:') or os.path.isabs(charm_url)
 
 
-async def get_charm_series(path, model):
-    """Inspects the charm directory at ``path`` and returns a default
-    series from its metadata.yaml (the first item in the 'series' list).
+async def get_charm_series(metadata, model):
+    """Inspects the given metadata and returns a default series from its
+    metadata.yaml (the first item in the 'series' list).
 
-    Tries to extract the informiation from the given model if no
-    series is determined from the path.
+    Tries to extract the information from the given model if no
+    series is determined from the given metadata.
     Returns None if no series can be determined.
 
     """
-    path = Path(path)
-    try:
-        if path.suffix == '.charm':
-            md = "metadata.yaml in %s" % path
-            with zipfile.ZipFile(str(path), 'r') as charm_file:
-                data = yaml.safe_load(charm_file.read('metadata.yaml'))
-        else:
-            md = path / "metadata.yaml"
-            if not md.exists():
-                return None
-            data = yaml.safe_load(md.open())
-    except yaml.YAMLError as exc:
-        if hasattr(exc, "problem_mark"):
-            mark = exc.problem_mark
-            log.error("Error parsing YAML file {}, line {}, column: {}".format(md, mark.line, mark.column))
-        raise
-    _series = data.get('series')
+
+    _series = metadata.get('series')
     series = _series[0] if _series else None
 
     if series is None:

juju/model.py

@@ -1791,20 +1791,24 @@ async def deploy(
                                                                 identifier)
             else:
                 # We have a local charm dir that needs to be uploaded
-                charm_dir = os.path.abspath(
-                    os.path.expanduser(identifier))
+                charm_dir = os.path.abspath(os.path.expanduser(identifier))
                 charm_origin = res.origin
+                base = None
 
                 metadata = utils.get_local_charm_metadata(charm_dir)
-                # TODO (cderici) : pass the metadata into get_charm_series, as
-                #  it also reads that file redundantly
-                charm_series = charm_series or await get_charm_series(charm_dir,
+                charm_series = charm_series or await get_charm_series(metadata,
                                                                       self)
-
-                charm_origin.base = utils.get_local_charm_base(charm_series, channel, metadata, charm_dir, client.Base)
-
+                base = utils.get_local_charm_base(
+                    charm_series, channel, metadata, charm_dir, client.Base)
+                charm_origin.base = base
+                if not application_name:
+                    application_name = metadata['name']
                 if not application_name:
                     application_name = metadata['name']
+                if base is None and charm_series is None:
+                    raise JujuError(
+                        "Either series or base is needed to deploy the "
+                        "charm at {}. ".format(charm_dir))
 
                 identifier = await self.add_local_charm_dir(charm_dir,
                                                             charm_series)

juju/utils.py

@@ -239,7 +239,7 @@ def get_local_charm_data(path, yaml_file):
 
     :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"
+    "metadata.yaml", or "manifest.yaml", or "charmcraft.yaml"
 
     :return: Object of charm metadata
     """
@@ -249,6 +249,8 @@ def get_local_charm_data(path, yaml_file):
     else:
         entity_path = Path(path)
         metadata_path = entity_path / yaml_file
+        if not metadata_path.exists():
+            return {}
         metadata = yaml.load(metadata_path.read_text(), Loader=yaml.FullLoader)
 
     return metadata
@@ -262,6 +264,10 @@ def get_local_charm_manifest(path):
     return get_local_charm_data(path, 'manifest.yaml')
 
 
+def get_local_charm_charmcraft_yaml(path):
+    return get_local_charm_data(path, 'charmcraft.yaml')
+
+
 PRECISE = "precise"
 QUANTAL = "quantal"
 RARING = "raring"
@@ -376,6 +382,12 @@ 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']
 
     if channel_for_base == '':
         raise errors.JujuError("Unable to determine base for charm : %s" %

tests/integration/charm-base-charmcraft-yaml/charmcraft.yaml

@@ -0,0 +1,10 @@
+# Copyright 2021 Canonical Ltd.
+# See LICENSE file for licensing details.
+type: charm
+bases:
+  - build-on:
+      - name: "ubuntu"
+        channel: "20.04"
+    run-on:
+      - name: "ubuntu"
+        channel: "20.04"

tests/integration/charm-base-charmcraft-yaml/config.yaml

@@ -0,0 +1,4 @@
+options:
+  status:
+    type: string
+    default: "active"

tests/integration/charm-base-charmcraft-yaml/dispatch

@@ -0,0 +1,13 @@
+#!/bin/bash
+
+status="$(config-get status)"
+
+if [[ "$status" == "error" ]]; then
+    if [[ -e .errored ]]; then
+        status="active"
+    else
+        touch .errored
+        exit 1
+    fi
+fi
+status-set "$status"

tests/integration/charm-base-charmcraft-yaml/metadata.yaml

@@ -0,0 +1,4 @@
+name: charm
+summary: "test"
+description: "test"
+maintainers: ["test"]

tests/integration/test_model.py

@@ -162,6 +162,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_base_charmcraft_yaml(event_loop):
+    charm_path = HERE_DIR / 'charm-base-charmcraft-yaml'
+
+    async with base.CleanModel() as model:
+        await model.deploy(str(charm_path))
+        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):